fix: search returning 500 when a conversation has no messages (#15328)
## Description
`SearchService#filter_conversations` matches conversations on the
display id and on the contact name, email, phone number and identifier.
It never looks at message content, so a conversation with no messages is
a valid result whenever its contact matches.
The search views did not account for that. They rendered
`conversation.messages.try(:first)`, which is `nil` for such a
conversation, and `api/v1/models/_message` calls `message.id` on it:
```
ActionView::Template::Error (undefined method 'id' for nil):
1: json.id message.id
app/views/api/v1/models/_message.json.jbuilder:1
app/views/api/v1/accounts/search/_message.json.jbuilder:1
app/views/api/v1/accounts/search/conversations.json.jbuilder:8
```
A single conversation without messages is enough to turn the whole
search request into a 500 for that query, so the agent loses
conversation search entirely until that conversation gets a message.
Both views that render a conversation search result were affected, so
this applies to `GET /search/conversations` and to the combined `GET
/search`.
The fix guards the message partial the same way the neighbouring
`contact`, `inbox` and `agent` partials in those same views are already
guarded. When there is no message the key is rendered as an empty
object, which is what already happens for a missing contact, inbox or
assignee.
**How to reproduce**
1. Create a conversation without any message (for example via `POST
/api/v1/accounts/{id}/conversations` without a `message`).
2. Search for the contact's name or phone number: `GET
/api/v1/accounts/{id}/search/conversations?q=<phone>`.
3. The request returns 500.
## Type of change
- [x] Bug fix (non-breaking change which fixes an issue)
## How Has This Been Tested?
Added one spec per affected endpoint in
`spec/controllers/api/v1/accounts/search_controller_spec.rb`, each
creating a conversation with no messages whose contact matches the query
and asserting that it is returned. Both fail with a 500 before the
change.
Also reproduced manually on a running instance: searching a contact that
had a conversation with no messages returned 500, and returns 200 with
`"message": {}` after the change.
## Checklist:
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my code
- [x] I have commented on my code, particularly in hard-to-understand
areas
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] Any dependent changes have been merged and published in downstream
modules
---
Related: #15289 documents the atomic `POST /conversations` with an
inline `message`, which avoids creating conversations without messages
in the first place. This fix is independent of it — it protects the
search regardless of how the conversation ended up without messages
(created by an agent before replying, by an API integration, by campaign
tooling, or by a flow that did not complete).
Co-authored-by: Sojan Jose <sojan@pepalo.com>
This commit is contained in:
@@ -44,6 +44,23 @@ RSpec.describe 'Search', type: :request do
|
||||
expect(response_data[:payload][:contacts].length).to eq 1
|
||||
expect(response_data[:payload][:articles].length).to eq 1
|
||||
end
|
||||
|
||||
it 'returns matching conversations that have no messages' do
|
||||
empty_contact = create(:contact, name: 'test empty', account: account)
|
||||
empty_conversation = create(:conversation, account: account, contact: empty_contact)
|
||||
create(:inbox_member, user: agent, inbox: empty_conversation.inbox)
|
||||
|
||||
get "/api/v1/accounts/#{account.id}/search",
|
||||
headers: agent.create_new_auth_token,
|
||||
params: { q: 'test' },
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
response_data = JSON.parse(response.body, symbolize_names: true)
|
||||
|
||||
conversation_ids = response_data[:payload][:conversations].pluck(:id)
|
||||
expect(conversation_ids).to include(empty_conversation.display_id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -169,6 +186,23 @@ RSpec.describe 'Search', type: :request do
|
||||
expect(response_data[:payload][:conversations].length).to eq 1
|
||||
end
|
||||
|
||||
it 'returns matching conversations that have no messages' do
|
||||
empty_contact = create(:contact, name: 'test empty', account: account)
|
||||
empty_conversation = create(:conversation, account: account, contact: empty_contact)
|
||||
create(:inbox_member, user: agent, inbox: empty_conversation.inbox)
|
||||
|
||||
get "/api/v1/accounts/#{account.id}/search/conversations",
|
||||
headers: agent.create_new_auth_token,
|
||||
params: { q: 'test' },
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
response_data = JSON.parse(response.body, symbolize_names: true)
|
||||
|
||||
conversation_ids = response_data[:payload][:conversations].pluck(:id)
|
||||
expect(conversation_ids).to include(empty_conversation.display_id)
|
||||
end
|
||||
|
||||
context 'with advanced_search feature enabled', :opensearch do
|
||||
before do
|
||||
account.enable_features!('advanced_search')
|
||||
|
||||
Reference in New Issue
Block a user