fix(bulk-actions): return per-item success/failed ids (EVO-1011)#58
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideBulkActionsJob and BulkActionsController were updated so that bulk conversation updates now track per-conversation successes and failures and return this result from the job, which the controller includes in the API response for the frontend to surface per-item failure information. Sequence diagram for bulk conversation update result propagationsequenceDiagram
actor Frontend
participant BulkActionsController as Api_V1_BulkActionsController
participant BulkActionsJob
Frontend->>BulkActionsController: create
BulkActionsController->>BulkActionsController: type_matches?
alt type_matches
BulkActionsController->>BulkActionsJob: perform_now(user, params)
BulkActionsJob->>BulkActionsJob: bulk_update
BulkActionsJob->>BulkActionsJob: bulk_conversation_update
loop each conversation
BulkActionsJob->>BulkActionsJob: bulk_add_labels(conversation)
BulkActionsJob->>BulkActionsJob: bulk_snoozed_until(conversation)
BulkActionsJob->>BulkActionsJob: conversation.update!(params)
alt update succeeds
BulkActionsJob->>BulkActionsJob: success_ids << display_id
else update fails
BulkActionsJob->>BulkActionsJob: failed_ids << display_id
end
end
BulkActionsJob-->>BulkActionsController: { success_ids, failed_ids }
BulkActionsController->>BulkActionsController: success_response(data: result)
BulkActionsController-->>Frontend: 201 Created with result
else type does not match
BulkActionsController-->>Frontend: error response
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
bulk_update, the Contact branch now always returns an empty{ success_ids: [], failed_ids: [] }hash, which is inconsistent with the conversation branch and may mislead the caller/front-end; consider either returning a meaningful result for contacts or returningnil/a distinct structure and handling that explicitly in the controller. - In
bulk_conversation_update, therescue StandardErrorinside the loop swallows all exceptions without any logging or context; consider at least logging the exception (with conversation ID) so operational issues can be traced while still allowing the bulk job to continue.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `bulk_update`, the Contact branch now always returns an empty `{ success_ids: [], failed_ids: [] }` hash, which is inconsistent with the conversation branch and may mislead the caller/front-end; consider either returning a meaningful result for contacts or returning `nil`/a distinct structure and handling that explicitly in the controller.
- In `bulk_conversation_update`, the `rescue StandardError` inside the loop swallows all exceptions without any logging or context; consider at least logging the exception (with conversation ID) so operational issues can be traced while still allowing the bulk job to continue.
## Individual Comments
### Comment 1
<location path="app/jobs/bulk_actions_job.rb" line_range="21" />
<code_context>
def bulk_update
if @params[:type] == 'Contact'
bulk_contact_update
+ { success_ids: [], failed_ids: [] }
else
bulk_remove_labels
</code_context>
<issue_to_address>
**issue (bug_risk):** The Contact branch now returns an always-empty result, which is inconsistent with conversation updates and likely breaks result reporting.
For the Contact path, `bulk_update` now always returns `{ success_ids: [], failed_ids: [] }`, while the conversation path returns real IDs from `bulk_conversation_update`. As a result, the client will see empty arrays even when Contact updates succeed or fail. Either have `bulk_contact_update` return a similar success/failed ID structure and propagate it, or return `nil` and let the controller branch by type so it doesn’t promise per-record results for contacts.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def bulk_update | ||
| if @params[:type] == 'Contact' | ||
| bulk_contact_update | ||
| { success_ids: [], failed_ids: [] } |
There was a problem hiding this comment.
issue (bug_risk): The Contact branch now returns an always-empty result, which is inconsistent with conversation updates and likely breaks result reporting.
For the Contact path, bulk_update now always returns { success_ids: [], failed_ids: [] }, while the conversation path returns real IDs from bulk_conversation_update. As a result, the client will see empty arrays even when Contact updates succeed or fail. Either have bulk_contact_update return a similar success/failed ID structure and propagate it, or return nil and let the controller branch by type so it doesn’t promise per-record results for contacts.
… ids (EVO-1011) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
533733a to
56171e0
Compare
Summary
BulkActionsJob#bulk_conversation_updatenow collectssuccess_idsandfailed_idsper conversation using a rescue clause inside the iterationBulkActionsController#createcaptures the job result and includes it in the API response payloadValidation
evo-ai-crm-community: bundle exec ruby -c app/jobs/bulk_actions_job.rbevo-ai-crm-community: bundle exec ruby -c app/controllers/api/v1/bulk_actions_controller.rbChanged Files
app/jobs/bulk_actions_job.rbapp/controllers/api/v1/bulk_actions_controller.rbRelated PRs
Linked Issue
Summary by Sourcery
Report per-item success and failure IDs from bulk conversation updates and expose them in the bulk actions API response.
New Features: