fix: try to disconnect baileys before destroy inbox #29
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 WalkthroughA Changes
Suggested labels
Poem
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:
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 disconnection callback to ensure that a channel provider using the "baileys" integration is disconnected before the channel is destroyed. The key changes include:
- Adding a before_destroy callback in the Whatsapp channel model for baileys providers.
- Defining a helper method (baileys_provider?) to conditionally trigger the callback.
- Adding tests to verify callback behavior for both baileys and non-baileys providers.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| spec/models/channel/whatsapp_spec.rb | Adds tests for the disconnect callback based on provider type. |
| app/models/channel/whatsapp.rb | Introduces a before_destroy callback and a helper method for baeleys providers. |
Comments suppressed due to low confidence (1)
spec/models/channel/whatsapp_spec.rb:72
- [nitpick] Consider renaming the locally defined 'channel' variable to avoid shadowing the let(:channel) declaration for improved clarity.
channel = create(:channel_whatsapp, provider: 'whatsapp_cloud', validate_provider_config: false, sync_templates: false)
There was a problem hiding this comment.
-(review needed) +(questions/changes requested)
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @CayoPOliveira)
app/models/channel/whatsapp.rb line 40 at r1 (raw file):
after_create :sync_templates before_destroy :disconnect_channel_provider, if: :baileys_provider?
Helper method is not needed here, due to low complexity for condition.
Suggestion:
before_destroy :disconnect_channel_provider, if: -> { provider == 'baileys' }spec/models/channel/whatsapp_spec.rb line 93 at r1 (raw file):
end end end
Carefully note on all suggested changes.
For some context:
ActiveRecordobjects have a.destroyed?methodexpect(object).to be_somethingis shorthand forexpect(object.something?).to be(true)
Suggestion:
describe 'callbacks' do
describe '#disconnect_channel_provider' do
context 'when provider is baileys' do
let(:channel) { create(:channel_whatsapp, provider: 'baileys', validate_provider_config: false, sync_templates: false) }
let(:disconnect_url) { "#{channel.provider_config['provider_url']}/connections/#{channel.phone_number}" }
it 'destroys the channel on successful disconnect' do
stub_request(:delete, disconnect_url).to_return(status: 200)
channel.destroy!
expect(channel).to be_destroyed
end
it 'destroys the channel on failure to disconnect' do
stub_request(:delete, disconnect_url).to_return(status: 404, body: 'error message')
channel.destroy!
expect(channel).to be_destroyed
end
end
context 'when provider is not baileys' do
let(:channel) { create(:channel_whatsapp, provider: 'whatsapp_cloud', validate_provider_config: false, sync_templates: false) }
it 'does not invoke callback' do
expect(channel).not_to receive(:disconnect_channel_provider)
channel.destroy!
end
end
end
endThere was a problem hiding this comment.
-(questions/changes requested) +(review needed)
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gabrieljablonski)
spec/models/channel/whatsapp_spec.rb line 93 at r1 (raw file):
Previously, gabrieljablonski (Gabriel Jablonski) wrote…
Carefully note on all suggested changes.
For some context:
ActiveRecordobjects have a.destroyed?methodexpect(object).to be_somethingis shorthand forexpect(object.something?).to be(true)
Done.
There was a problem hiding this comment.
-(review needed) +(approved for merge)
Reviewed 2 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @CayoPOliveira)
4e86e88
into
feat/process-baileys-images-in-message-upsert
…dle unsupported messages (#26) * fix: return not_found status for missing messages in WhatsApp webhook * feat: enhance message handling to support image attachments * chore: Handle incoming whatsapp baileys service attachments and implement specs for images and videos * fix: add file_content_type method to incoming baileys messages * chore: specs for stickers, audio and files * fix: handle media attachment errors * chore: convert file_content_type to string when attaching media * fix: add handling for attachment not found error in incoming message service * chore: refactor file_content_type method and simplify filename method * chore: update media attachment tests * feat: baileys unsupported message alert (#27) * feat: create alert message for unsupported message types * chore: fail message when try send not supported type in baileys * chore: add tests for unsupported message handling in Baileys service * chore: correct spelling * chore: NIT in spec name * chore: remove unnecessary message reload in unsupported message type test * feat: baileys support to send attachments (#28) * feat: enhance message sending logic with support for attachments and interactive messages * fix: update message format to use messageContent for text messages * feat: attachment message sending * fix: use strict encoding for attachment file download * chore: streamline message sending logic and remove unused error classes * chore: remove type from message sending logic * chore: update message sending specs * chore: raise MessageNotSentError instead of updating message status to failed * chore: change baileys contact name preferences (#30) * refactor: improve contact name handling and extraction from JID * test: enhance message handling to update contact names based on pushName and verifiedBizName * chore: update contact name condition to match phone number from JID * chore: correct method name typo * chore: correct variable names for phone number consistency in tests * chore: update message payload structure and improve error handling in send_message * test: re-add testing for error handling * feat: enhance contact name handling and add self-message detection * fix: ensure presence check for verified business name in contact name retrieval * test: improve specs --------- Co-authored-by: gabrieljablonski <contact@gabrieljablonski.com> --------- Co-authored-by: gabrieljablonski <contact@gabrieljablonski.com> Co-authored-by: Gabriel Jablonski <gabriel@fazer.ai> --------- Co-authored-by: gabrieljablonski <contact@gabrieljablonski.com> Co-authored-by: Gabriel Jablonski <gabriel@fazer.ai> * fix: try to disconnect baileys before destroy inbox (#29) * fix: ensure proper disconnection of Baileys provider on channel destruction * test: add callback tests for disconnecting channel provider in Whatsapp spec * refactor: simplify condition for disconnecting Baileys provider on channel destruction * refactor: enhance disconnect_channel_provider specs for Baileys provider * test: verify channel destruction does not call disconnect_channel_provider --------- Co-authored-by: gabrieljablonski <contact@gabrieljablonski.com> --------- Co-authored-by: gabrieljablonski <contact@gabrieljablonski.com> Co-authored-by: Gabriel Jablonski <gabriel@fazer.ai>
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
Bug Fixes
Tests