fix(crm): Notificame fallback_title uses content[:fileName] (EVO-1130)#69
Conversation
…k_title (EVO-1130) Inbound Notificame webhook delivers the real filename in `content[:fileName]` (mirrors the outbound serializer and the Z-API inbound handler). Falling back to URL basename produced opaque hashes for proxied/signed URLs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reviewer's GuideAdjusts Notificame WhatsApp inbound file attachment handling to prefer the provided fileName for fallback_title and adds targeted specs for the new behavior. Sequence diagram for updated Notificame inbound file fallback_title resolutionsequenceDiagram
participant NotificameWebhook
participant IncomingMessageNotificameService
participant Message
participant Attachment
NotificameWebhook ->> IncomingMessageNotificameService: attach_file(message, content)
IncomingMessageNotificameService ->> Message: attachments
activate Message
Message ->> Attachment: new(file_type, fallback_title, file)
deactivate Message
rect rgb(230,230,230)
IncomingMessageNotificameService ->> IncomingMessageNotificameService: [content[:fileName].present?]
Note over IncomingMessageNotificameService: fallback_title = content[:fileName]
end
rect rgb(230,230,230)
IncomingMessageNotificameService ->> IncomingMessageNotificameService: [content[:fileName] blank]
IncomingMessageNotificameService ->> IncomingMessageNotificameService: URI.parse(content[:fileUrl])
Note over IncomingMessageNotificameService: fallback_title = basename from content[:fileUrl]
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 left some high level feedback:
- The
fallback_titleexpression is getting a bit dense with thepresencechecks andrescue; consider extracting this into a small helper method (e.g.resolved_fallback_title(content)) to improve readability and make the error handling more explicit. - The inline
rescuein thefallback_titleURL basename logic will catch all exceptions fromURI.parse; it might be safer to rescue only the expected parsing errors or handle invalid URLs earlier to avoid silently swallowing unexpected issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `fallback_title` expression is getting a bit dense with the `presence` checks and `rescue`; consider extracting this into a small helper method (e.g. `resolved_fallback_title(content)`) to improve readability and make the error handling more explicit.
- The inline `rescue` in the `fallback_title` URL basename logic will catch all exceptions from `URI.parse`; it might be safer to rescue only the expected parsing errors or handle invalid URLs earlier to avoid silently swallowing unexpected issues.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dpaes
left a comment
There was a problem hiding this comment.
Approved.
ACs validated against the diff:
- fallback_title now prefers content[:fileName] (line 308)
- URL basename retained as second-level fallback via || chain
- New spec covers fileName present / missing / blank (3 examples)
Convention corroborated by Z-API inbound handler (incoming_message_zapi_service.rb:209) and Notificame outbound serializer (providers/notificame_service.rb:63, 431). Cross-gateway symmetry is strong evidence.
One Medium observation logged on the Linear card (EVO-1130) about the spec scaffold's rescue LoadError + return unless defined?(Rails) guards — non-blocking, capped Medium by project convention since specs aren't blockers.
Sourcery's helper-extraction and rescue-scoping points are valid but pre-existing / out of scope for this single-line fix.
Real Notificame document webhook payload still worth capturing in production logs to close the inference loop — already flagged in the PR body as non-blocking.
Summary
Whatsapp::IncomingMessageNotificameService#attach_filenow preferscontent[:fileName]forfallback_title, falling back to URL basename when absent or blank.providers/notificame_service.rb:63,431) and the Z-API inbound handler (incoming_message_zapi_service.rb:209), which already consume the same:fileNamecamelCase key.attach_file, covering the three branches of the resolution (fileNamepresent, absent, blank).Changed Files
app/services/whatsapp/incoming_message_notificame_service.rb— single-line change at thefallback_titleassignmentspec/services/whatsapp/incoming_message_notificame_service_spec.rb— new file, 3 examplesValidation
ruby -c app/services/whatsapp/incoming_message_notificame_service.rb→ Syntax OKruby -c spec/services/whatsapp/incoming_message_notificame_service_spec.rb→ Syntax OKbundle exec rspec spec/services/whatsapp/incoming_message_notificame_service_spec.rb→ 3 examples, 0 failuresNotes
:fileNameconvention is inferred from API symmetry and corroborated by the Z-API sibling handler. Capturing a real Notificame document webhook in production logs would close the loop — non-blocking, but worth checking once a real document message lands.Linked Issue
🤖 Generated with Claude Code
Summary by Sourcery
Prefer Notificame payload fileName for WhatsApp incoming attachment fallback titles, with URL basename as a secondary fallback.
Bug Fixes:
Tests: