chore: identify and permit only direct users jid types #14
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update enhances the WhatsApp message processing service by introducing a new method to determine the type of JID from incoming messages. The service now processes only messages from user-type JIDs by adding an early return condition. It also refines the logic for extracting phone numbers and setting contact names, as well as simplifying the text message identification. Additionally, corresponding tests have been added and improved to cover non-user message scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant M as Message
participant S as IncomingMessageService
participant C as Contact
M->>S: Receive message with remoteJid
S->>S: Evaluate jid_type(remoteJid)
alt JID type is not 'user'
S-->>M: Early return (skip processing)
else JID type is 'user'
S->>S: Extract & set contact using jid and updated logic
S->>S: Process text message (simplified check)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 refines incoming message handling by allowing only direct user jid types while improving test coverage and contact name management.
- Adds new tests to verify that non-user messages are ignored.
- Updates jid parsing logic to correctly extract phone numbers and updates contacts when an incoming message has a new pushName.
- Introduces a jid_type method to classify message sources.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| spec/services/whatsapp/incoming_message_baileys_service_spec.rb | Adds tests for non-user jid types and validates contact name updates. |
| app/services/whatsapp/incoming_message_baileys_service.rb | Restricts message processing to direct users, refines jid parsing, and updates contact names if needed. |
There was a problem hiding this comment.
-(review needed) +(questions/changes requested)
Reviewed 2 of 2 files at r1, all commit messages.
Dismissed @copilot-pull-request-reviewer[bot] from a discussion.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @broadcast and @CayoPOliveira)
spec/services/whatsapp/incoming_message_baileys_service_spec.rb line 195 at r2 (raw file):
end it 'does not create a conversation or message' do
NIT
Suggestion:
it 'does not create a conversation' dospec/services/whatsapp/incoming_message_baileys_service_spec.rb line 197 at r2 (raw file):
it 'does not create a conversation or message' do described_class.new(inbox: inbox, params: params).perform expect(inbox.conversations).to be_empty
NIT
Suggestion:
described_class.new(inbox: inbox, params: params).perform
expect(inbox.conversations).to be_emptyapp/services/whatsapp/incoming_message_baileys_service.rb line 66 at r2 (raw file):
def set_contact # NOTE: jid shape is `<user (like phone-number)>_<agent_id>:<device_id>@<server (like s.whatsapp.net)>`
NIT
Can be simplified.
Suggestion:
# NOTE: jid shape is `<user>_<agent>:<device>@<server>`app/services/whatsapp/incoming_message_baileys_service.rb line 67 at r2 (raw file):
def set_contact # NOTE: jid shape is `<user (like phone-number)>_<agent_id>:<device_id>@<server (like s.whatsapp.net)>` # https://github.com/WhiskeySockets/Baileys/blob/d5dc75887493f25877028d43b81101e050a3695b/src/WABinary/jid-utils.ts#L21
NIT
Prefer using tagged branches when available vs specific commits (but never main/master since those change over time!)
v6.7.16 is the version we're currently using.
Suggestion:
# https://github.com/WhiskeySockets/Baileys/blob/v6.7.16/src/WABinary/jid-utils.ts#L19app/services/whatsapp/incoming_message_baileys_service.rb line 101 at r2 (raw file):
return 'lid' if jid.include? '@lid' return 'broadcast' if jid.include? '@broadcast' return 'status' if jid.include? 'status@broadcast'
Have you found any other situations in which @broadcast is used other than for status@broadcast?
app/services/whatsapp/incoming_message_baileys_service.rb line 106 at r2 (raw file):
'unknown' end
Avoid using "broad matches" (using include?) for stuff like this, better to be highly specific.
Suggestion:
def jid_type # rubocop:disable Metrics/CyclomaticComplexity
jid = @raw_message[:key][:remoteJid]
server = jid.split('@').last
case server
when 's.whatsapp.net', 'c.us'
'user'
when 'g.us'
'group'
when 'lid'
'lid'
when 'broadcast'
jid.start_with?('status@') ? 'status' : 'broadcast'
when 'newsletter'
'newsletter'
when 'call'
'call'
else
'unknown'
end
endapp/services/whatsapp/incoming_message_baileys_service.rb line 111 at r2 (raw file):
msg = @raw_message[:message] return 'text' if msg.key?(:conversation) || msg.key?(:extendedTextMessage)
Are you sure this change is correct?
… processing for non-user jids
…ve message type detection
… extendedTextMessage
70a7c97 to
de63144
Compare
There was a problem hiding this comment.
-(questions/changes requested) +(review needed)
Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @gabrieljablonski)
app/services/whatsapp/incoming_message_baileys_service.rb line 101 at r2 (raw file):
Previously, gabrieljablonski (Gabriel Jablonski) wrote…
Have you found any other situations in which
@broadcastis used other than forstatus@broadcast?
Not yet. I based myself on WhiskeySockets Baileys, which has the isJidBroadcast and isJidStatusBroadcast functions as separate ones, maybe there a reason for this.
app/services/whatsapp/incoming_message_baileys_service.rb line 106 at r2 (raw file):
Previously, gabrieljablonski (Gabriel Jablonski) wrote…
Avoid using "broad matches" (using
include?) for stuff like this, better to be highly specific.
Done.
app/services/whatsapp/incoming_message_baileys_service.rb line 111 at r2 (raw file):
Previously, gabrieljablonski (Gabriel Jablonski) wrote…
Are you sure this change is correct?
My intention was to standardize the conditions with boolean values, I believe that this way works too, but I found a different previous solution
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1.
Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @gabrieljablonski)
There was a problem hiding this comment.
LGTM. Please check pending comments, no need to re-review.
-(review needed) +(approved for merge) +(questions/changes requested)
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @CayoPOliveira)
app/services/whatsapp/incoming_message_baileys_service.rb line 101 at r2 (raw file):
Previously, CayoPOliveira (Cayo P. R. Oliveira) wrote…
Not yet. I based myself on WhiskeySockets Baileys, which has the
isJidBroadcastandisJidStatusBroadcastfunctions as separate ones, maybe there a reason for this.
Cool. Please add comment referencing that.
app/services/whatsapp/incoming_message_baileys_service.rb line 82 at r3 (raw file):
@contact = contact_inbox.contact @contact.update(name: push_name) if @contact[:name] == phone_number_formatted && !@raw_message[:key][:fromMe]
NIT
Reminder to always favor update! over update, and object.field over object[:field] (when working with ActiveRecords)
Suggestion:
@contact.update!(name: push_name) if @contact.name == phone_number_formatted && !@raw_message[:key][:fromMe]There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @CayoPOliveira)
app/services/whatsapp/incoming_message_baileys_service.rb line 82 at r3 (raw file):
Previously, gabrieljablonski (Gabriel Jablonski) wrote…
NIT
Reminder to always favor
update!overupdate, andobject.fieldoverobject[:field](when working withActiveRecords)
About update! vs update, I'll add a Rubocop rule to warn about it. Still change it here though.
There was a problem hiding this comment.
-(questions/changes requested)
Reviewed all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @CayoPOliveira)
There was a problem hiding this comment.
Reviewed 1 of 2 files at r3.
Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @gabrieljablonski)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @CayoPOliveira)
app/services/whatsapp/incoming_message_baileys_service.rb line 116 at r4 (raw file):
'unknown' end end
Suggestion:
server = jid.split('@').last
# NOTE: Based on Baileys internal functions
# https://github.com/WhiskeySockets/Baileys/blob/v6.7.16/src/WABinary/jid-utils.ts#L48-L58
case server
when 's.whatsapp.net', 'c.us'
'user'
when 'g.us'
'group'
when 'lid'
'lid'
when 'broadcast'
jid.start_with?('status@') ? 'status' : 'broadcast'
when 'newsletter'
'newsletter'
when 'call'
'call'
else
'unknown'
end
endThere was a problem hiding this comment.
Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status:
d101fe1
into
fix/contact-without-push-name
* fix: resolve bug for contact name when user stats a new conversation with a new contact * chore: update specs to cover the bug resolution * chore: correct variable name * chore: simplify update contact name handling * chore: ensure pushName is always a string and leave a comment to keep an eye if always pushName will be present * fix: update contact name access * chore: identify and permit only direct users jid types (#14) * fix: add jid_type method to classify message sender types and prevent processing for non-user jids * chore: update jid parsing to correctly extract phone number and improve message type detection * chore: add spec to ensure no conversation is created for non-user messages * chore: update jid shape comment * chore: refactor jid_type method * chore: update message_type logic for classify text messages when is a extendedTextMessage * refactor: update spec that no conversation is created for non-user messages * fix: use update! method for contact name update * chore: add note on distinguishing broadcast and status JIDs in Baileys * chore: add note on Baileys internal function for JID classification
* fix: resolve bug for contact name when user stats a new conversation with a new contact * chore: update specs to cover the bug resolution * chore: correct variable name * chore: simplify update contact name handling * chore: ensure pushName is always a string and leave a comment to keep an eye if always pushName will be present * fix: update contact name access * chore: identify and permit only direct users jid types (#14) * fix: add jid_type method to classify message sender types and prevent processing for non-user jids * chore: update jid parsing to correctly extract phone number and improve message type detection * chore: add spec to ensure no conversation is created for non-user messages * chore: update jid shape comment * chore: refactor jid_type method * chore: update message_type logic for classify text messages when is a extendedTextMessage * refactor: update spec that no conversation is created for non-user messages * fix: use update! method for contact name update * chore: add note on distinguishing broadcast and status JIDs in Baileys * chore: add note on Baileys internal function for JID classification
This change is
Summary by CodeRabbit
New Features
Tests