feat: mark message as read #43
Conversation
WalkthroughThis change introduces a mechanism for dispatching and handling message read events, specifically for WhatsApp channels. It adds event dispatching when messages are read, a listener method to process these events, a channel method to forward read receipts, and a provider service method to send read status to the WhatsApp API. Tests are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller
participant Dispatcher
participant ChannelListener
participant ChannelWhatsapp
participant WhatsappProvider
User->>Controller: Marks conversation as read
Controller->>Dispatcher: Dispatch MESSAGES_READ event with conversation
Dispatcher->>ChannelListener: messages_read(event)
ChannelListener->>ChannelWhatsapp: send_read_messages(messages)
ChannelWhatsapp->>WhatsappProvider: send_read_messages(phone_number, messages)
WhatsappProvider->>WhatsappProvider: POST /read-messages (with message keys)
WhatsappProvider-->>ChannelWhatsapp: Process response
Suggested labels
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new feature to mark messages as read by adding a new API endpoint and updating the corresponding service, model, listener, and associated tests.
- Updated test to include a source_id for messages.
- Implemented the send_read_messages method in the WhatsApp Baileys service and added a corresponding model method.
- Extended the channel listener to dispatch a messages read event.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| spec/services/whatsapp/providers/whatsapp_baileys_service_spec.rb | Updated test data for messages and added tests for send_read_messages |
| lib/events/types.rb | Added new MESSAGES_READ event type |
| app/services/whatsapp/providers/whatsapp_baileys_service.rb | Added implementation for sending read messages via API |
| app/models/channel/whatsapp.rb | Added a wrapper for send_read_messages on the channel model |
| app/listeners/channel_listener.rb | Added handling for messages_read events and calling send_read_messages |
| app/controllers/api/v1/accounts/conversations_controller.rb | Triggered the new event dispatch using MESSAGES_READ |
Comments suppressed due to low confidence (3)
app/services/whatsapp/providers/whatsapp_baileys_service.rb:66
- The variable 'remote_jid' is used without a definition in this context. Define or compute 'remote_jid' based on the phone number or other available data before its usage.
remoteJid: remote_jid,
app/listeners/channel_listener.rb:24
- The call to send_read_messages passes an extra argument (event.name) and a keyword argument 'messages', but the corresponding method expects only a single messages parameter. Adjust the method call to match the expected signature.
channel.send_read_messages(event.name, messages: messages) if messages.present?
app/listeners/channel_listener.rb:21
- Using 'map' without filtering out nil values may include nil entries in the messages array. Consider using 'compact' or a 'select' filter to ensure only valid messages are passed.
message if message.status != 'read'
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/controllers/api/v1/accounts/conversations_controller.rb(1 hunks)app/listeners/channel_listener.rb(1 hunks)app/models/channel/whatsapp.rb(1 hunks)app/services/whatsapp/providers/whatsapp_baileys_service.rb(1 hunks)lib/events/types.rb(1 hunks)spec/services/whatsapp/providers/whatsapp_baileys_service_spec.rb(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/models/channel/whatsapp.rb (1)
app/services/whatsapp/providers/whatsapp_baileys_service.rb (1)
send_read_messages(56-74)
app/services/whatsapp/providers/whatsapp_baileys_service.rb (1)
app/models/channel/whatsapp.rb (1)
send_read_messages(82-86)
🔇 Additional comments (5)
lib/events/types.rb (1)
39-39: LGTM: New event type for read messagesThe new event type
MESSAGES_READfollows the established naming pattern and is placed appropriately with other message event constants. This will serve as the identifier for the message read notification system.app/models/channel/whatsapp.rb (1)
82-86: LGTM: Well-structured method for handling read messagesThis implementation follows the same pattern as other methods in this class, like
toggle_typing_status. It properly checks if the provider service supports the required method before delegating to it, which allows for graceful degradation when using providers that don't support read receipts.app/controllers/api/v1/accounts/conversations_controller.rb (1)
144-144: LGTM: Appropriate event dispatch locationThe event is dispatched immediately after updating the last seen timestamps, which is the logical place to trigger marking messages as read.
spec/services/whatsapp/providers/whatsapp_baileys_service_spec.rb (2)
7-7: Clear test fixture setup.Explicitly setting the message
source_idto 'msg_123' ensures deterministic behavior for the tests that rely on this attribute, particularly the newsend_read_messagestest.
366-376: Well-structured test for the new read messages functionality.This test properly verifies that the
send_read_messagesmethod makes a POST request to the correct endpoint with the appropriate payload structure. It confirms the method returnstruewhen the API responds with a 200 status code.A few observations:
- The test validates the correct formatting of message keys with id, remoteJid, and fromMe attributes
- The test uses the standard stub_request pattern consistent with other tests in this file
- The assertion correctly verifies the return value of the method
…essages read event
2427eec to
a39a791
Compare
There was a problem hiding this comment.
-work-in-progress -(review needed) +(approved for merge)
Reviewed 3 of 6 files at r1, 2 of 3 files at r2, 5 of 6 files at r3, 1 of 1 files at r5, 4 of 4 files at r6, all commit messages.
Dismissed @coderabbitai[bot] from 2 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @CayoPOliveira)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/services/whatsapp/providers/whatsapp_baileys_service.rb(3 hunks)spec/controllers/api/v1/accounts/conversations_controller_spec.rb(1 hunks)spec/listeners/channel_listener_spec.rb(1 hunks)spec/models/channel/whatsapp_spec.rb(3 hunks)spec/services/whatsapp/providers/whatsapp_baileys_service_spec.rb(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- spec/services/whatsapp/providers/whatsapp_baileys_service_spec.rb
- app/services/whatsapp/providers/whatsapp_baileys_service.rb
🧰 Additional context used
🧬 Code Graph Analysis (1)
spec/models/channel/whatsapp_spec.rb (2)
app/services/whatsapp/providers/whatsapp_baileys_service.rb (2)
update_presence(89-105)send_read_messages(107-126)app/models/channel/whatsapp.rb (2)
update_presence(88-92)send_read_messages(94-98)
🔇 Additional comments (3)
spec/controllers/api/v1/accounts/conversations_controller_spec.rb (1)
691-703: Looks good - event dispatch for message read status.The test properly verifies that when a conversation's last seen timestamp is updated, a
messages.readevent is dispatched with the correct parameters. This supports the new WhatsApp read receipt functionality.spec/listeners/channel_listener_spec.rb (1)
94-117: Well-structured test for message read event handling.The tests verify both the happy path (sending read receipts for the appropriate messages) and graceful handling when the channel doesn't support read receipts. This approach matches the pattern used for other channel events in this file.
spec/models/channel/whatsapp_spec.rb (1)
71-72: Good improvement to existing tests.Adding explicit expectations for the method calls makes these tests more robust by ensuring proper interactions with the provider service.
Also applies to: 100-100, 107-107
|
|
||
| it 'calls provider service method' do | ||
| provider_double = instance_double(Whatsapp::Providers::WhatsappBaileysService, send_read_messages: nil) | ||
| allow(provider_double).to receive(:send_read_messages).with([message], conversation.contact.phone_number) |
There was a problem hiding this comment.
Fix parameter order in test stub.
The parameter order in the test stub doesn't match the actual implementation in the Channel::Whatsapp model.
According to the provided code snippet from app/models/channel/whatsapp.rb, the method calls the provider with:
provider_service.send_read_messages(conversation.contact.phone_number, messages)But the test is setting up the stub with parameters in reverse order:
-allow(provider_double).to receive(:send_read_messages).with([message], conversation.contact.phone_number)
+allow(provider_double).to receive(:send_read_messages).with(conversation.contact.phone_number, [message])
Pull Request Template
Description
Please include a summary of the change and issue(s) fixed. Also, mention relevant motivation, context, and any dependencies that this change requires.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Checklist:
This change is
Summary by CodeRabbit