fix(inboxes): address disabled inbox review feedback
This commit is contained in:
@@ -3,7 +3,6 @@ class Api::V1::Widget::BaseController < ApplicationController
|
||||
include WebsiteTokenHelper
|
||||
|
||||
before_action :set_web_widget
|
||||
before_action :ensure_inbox_active
|
||||
before_action :set_contact
|
||||
|
||||
private
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
class Api::V1::Widget::MessagesController < Api::V1::Widget::BaseController
|
||||
before_action :ensure_inbox_active, only: [:create, :update]
|
||||
before_action :set_conversation, only: [:create]
|
||||
before_action :set_message, only: [:update]
|
||||
|
||||
|
||||
@@ -56,11 +56,12 @@ class Webhooks::InstagramEventsJob < MutexApplicationJob
|
||||
channel = find_channel(instagram_id)
|
||||
|
||||
next if channel.blank?
|
||||
next unless channel.inbox.active?
|
||||
|
||||
if (event_name = event_name(messaging))
|
||||
send(event_name, messaging, channel)
|
||||
end
|
||||
event_name = event_name(messaging)
|
||||
next if event_name.blank?
|
||||
next unless channel.inbox.active? || event_name == :read
|
||||
|
||||
send(event_name, messaging, channel)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -127,7 +128,7 @@ class Webhooks::InstagramEventsJob < MutexApplicationJob
|
||||
end
|
||||
|
||||
def event_name(messaging)
|
||||
@event_name ||= SUPPORTED_EVENTS.find { |key| messaging.key?(key) }
|
||||
SUPPORTED_EVENTS.find { |key| messaging.key?(key) }
|
||||
end
|
||||
|
||||
def message(messaging, channel)
|
||||
|
||||
@@ -21,7 +21,7 @@ class Webhooks::TiktokEventsJob < MutexApplicationJob
|
||||
def channel_is_inactive?
|
||||
return true if channel.blank?
|
||||
return true unless channel.account.active?
|
||||
return true unless channel.inbox.active?
|
||||
return true unless channel.inbox.active? || event_name == 'im_mark_read_msg'
|
||||
|
||||
false
|
||||
end
|
||||
|
||||
@@ -8,7 +8,7 @@ class Webhooks::WhatsappEventsJob < MutexApplicationJob
|
||||
def perform(params = {})
|
||||
channel = find_channel_from_whatsapp_business_payload(params)
|
||||
|
||||
if channel_is_inactive?(channel)
|
||||
if channel_is_inactive?(channel, params)
|
||||
Rails.logger.warn("Inactive WhatsApp channel: #{channel&.phone_number || "unknown - #{params[:phone_number]}"}")
|
||||
return
|
||||
end
|
||||
@@ -124,16 +124,21 @@ class Webhooks::WhatsappEventsJob < MutexApplicationJob
|
||||
].compact_blank.first
|
||||
end
|
||||
|
||||
def channel_is_inactive?(channel)
|
||||
def channel_is_inactive?(channel, params)
|
||||
return true if channel.blank?
|
||||
# Only skip for embedded signup when reauth is required; manual flow uses API keys and should still receive webhooks
|
||||
return true if channel.reauthorization_required? && embedded_signup_channel?(channel)
|
||||
return true unless channel.account.active?
|
||||
return true unless channel.inbox.active?
|
||||
return true unless channel.inbox.active? || status_update_event?(params)
|
||||
|
||||
false
|
||||
end
|
||||
|
||||
def status_update_event?(params)
|
||||
value = params.dig(:entry, 0, :changes, 0, :value) || params
|
||||
value[:statuses].present?
|
||||
end
|
||||
|
||||
def embedded_signup_channel?(channel)
|
||||
(channel.provider_config || {}).to_h['source'] == 'embedded_signup'
|
||||
end
|
||||
|
||||
@@ -6,6 +6,7 @@ class ReplyMailbox < ApplicationMailbox
|
||||
def process
|
||||
# Return early if no conversation was found (e.g., notification emails, suspended accounts)
|
||||
return unless @conversation
|
||||
return unless @conversation.inbox.active?
|
||||
|
||||
# Wrap everything in a transaction to ensure atomicity
|
||||
# This prevents orphan conversations if message/attachment creation fails
|
||||
|
||||
@@ -22,6 +22,7 @@ class Mailbox::ConversationFinderStrategies::NewConversationStrategy < Mailbox::
|
||||
# The actual persistence happens in ReplyMailbox within a transaction that includes message creation.
|
||||
def find
|
||||
return nil unless @channel # No valid channel found
|
||||
return nil unless @inbox.active?
|
||||
return nil unless incoming_email_from_valid_email? # Skip edge cases
|
||||
|
||||
# Check if conversation already exists by in_reply_to
|
||||
|
||||
@@ -8,13 +8,13 @@ class Whatsapp::IncomingMessageBaseService
|
||||
pattr_initialize [:inbox!, :params!, :outgoing_echo]
|
||||
|
||||
def perform
|
||||
return unless @inbox.active?
|
||||
|
||||
processed_params
|
||||
|
||||
if processed_params.try(:[], :statuses).present?
|
||||
process_statuses
|
||||
elsif messages_data.present?
|
||||
return unless @inbox.active?
|
||||
|
||||
process_messages
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseController
|
||||
before_action :set_call, only: %i[show accept reject terminate upload_recording]
|
||||
before_action :set_call_context, only: :initiate
|
||||
before_action :ensure_inbox_active, only: :initiate
|
||||
before_action :ensure_calling_enabled, only: :initiate
|
||||
before_action :ensure_sdp_offer, only: :initiate
|
||||
before_action :ensure_contact_phone, only: :initiate
|
||||
@@ -96,6 +97,10 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro
|
||||
render_could_not_create_error(I18n.t('errors.whatsapp.calls.not_enabled'))
|
||||
end
|
||||
|
||||
def ensure_inbox_active
|
||||
render_inbox_disabled_error unless @inbox.active?
|
||||
end
|
||||
|
||||
def ensure_sdp_offer
|
||||
return if params[:sdp_offer].present?
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ class Voice::OutboundCallBuilder
|
||||
def perform!
|
||||
raise ArgumentError, 'Contact phone number required' if contact.phone_number.blank?
|
||||
raise ArgumentError, 'Agent required' if user.blank?
|
||||
raise CustomExceptions::InboxDisabled unless inbox.active?
|
||||
|
||||
# Claim for the caller if a reused conversation is unassigned at trigger time; wins over auto-assignment.
|
||||
# New conversations set the assignee at creation instead (see create_conversation!).
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
class CustomExceptions::InboxDisabled < CustomExceptions::Base
|
||||
def initialize(data = {})
|
||||
super
|
||||
end
|
||||
|
||||
def message
|
||||
'This inbox is currently disabled'
|
||||
end
|
||||
|
||||
@@ -43,6 +43,18 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do
|
||||
expect(json_response['id']).to eq(conversation.display_id)
|
||||
expect(json_response['status']).to eq(conversation.status)
|
||||
end
|
||||
|
||||
it 'returns the conversation when the inbox is disabled' do
|
||||
web_widget.inbox.update!(active: false)
|
||||
|
||||
get '/api/v1/widget/conversations',
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
params: { website_token: web_widget.website_token },
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
expect(response.parsed_body['id']).to eq(conversation.display_id)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with a conversation but invalid source id' do
|
||||
@@ -62,6 +74,19 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do
|
||||
end
|
||||
|
||||
describe 'POST /api/v1/widget/conversations' do
|
||||
it 'does not create a conversation when the inbox is disabled' do
|
||||
web_widget.inbox.update!(active: false)
|
||||
|
||||
expect do
|
||||
post '/api/v1/widget/conversations',
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
params: conversation_params,
|
||||
as: :json
|
||||
end.not_to change(Conversation, :count)
|
||||
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
|
||||
it 'creates a conversation with correct details' do
|
||||
post '/api/v1/widget/conversations',
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
|
||||
@@ -28,6 +28,18 @@ RSpec.describe '/api/v1/widget/messages', type: :request do
|
||||
expect(json_response['meta']).not_to be_empty
|
||||
end
|
||||
|
||||
it 'returns messages when the inbox is disabled' do
|
||||
web_widget.inbox.update!(active: false)
|
||||
|
||||
get api_v1_widget_messages_url,
|
||||
params: { website_token: web_widget.website_token },
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
expect(response.parsed_body['payload'].length).to eq(4)
|
||||
end
|
||||
|
||||
it 'returns empty messages', :skip_before do
|
||||
get api_v1_widget_messages_url,
|
||||
params: { website_token: web_widget.website_token },
|
||||
@@ -43,6 +55,20 @@ RSpec.describe '/api/v1/widget/messages', type: :request do
|
||||
|
||||
describe 'POST /api/v1/widget/messages' do
|
||||
context 'when post request is made' do
|
||||
it 'does not create message in conversation when the inbox is disabled' do
|
||||
web_widget.inbox.update!(active: false)
|
||||
message_params = { content: 'hello world', timestamp: Time.current }
|
||||
|
||||
expect do
|
||||
post api_v1_widget_messages_url,
|
||||
params: { website_token: web_widget.website_token, message: message_params },
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
as: :json
|
||||
end.not_to change(Message, :count)
|
||||
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
end
|
||||
|
||||
it 'creates message in conversation' do
|
||||
conversation.destroy! # Test all params
|
||||
message_params = { content: 'hello world', timestamp: Time.current }
|
||||
@@ -213,6 +239,21 @@ RSpec.describe '/api/v1/widget/messages', type: :request do
|
||||
end
|
||||
|
||||
describe 'PUT /api/v1/widget/messages' do
|
||||
context 'when the inbox is disabled' do
|
||||
it 'does not update the message' do
|
||||
message = create(:message, content_type: 'input_email', account: account, inbox: web_widget.inbox, conversation: conversation)
|
||||
web_widget.inbox.update!(active: false)
|
||||
|
||||
put api_v1_widget_message_url(message.id),
|
||||
params: { website_token: web_widget.website_token, contact: { email: Faker::Internet.email } },
|
||||
headers: { 'X-Auth-Token' => token },
|
||||
as: :json
|
||||
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
expect(message.reload.submitted_email).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
context 'when put request targets a message from another visitor in the same inbox' do
|
||||
it 'does not update the foreign message' do
|
||||
other_contact = create(:contact, account: account, email: nil)
|
||||
|
||||
@@ -208,6 +208,18 @@ RSpec.describe 'WhatsApp Calls API', type: :request do
|
||||
expect(response).to have_http_status(:unprocessable_entity)
|
||||
expect(response.parsed_body['error']).to eq(I18n.t('errors.whatsapp.calls.not_enabled'))
|
||||
end
|
||||
|
||||
it 'returns 403 before initiating the provider call when the inbox is disabled' do
|
||||
inbox.update!(active: false)
|
||||
allow(provider_service).to receive(:initiate_call)
|
||||
|
||||
post "/api/v1/accounts/#{account.id}/whatsapp_calls/initiate",
|
||||
params: { conversation_id: initiate_conversation.display_id, sdp_offer: 'sdp_offer' },
|
||||
headers: agent.create_new_auth_token
|
||||
|
||||
expect(response).to have_http_status(:forbidden)
|
||||
expect(provider_service).not_to have_received(:initiate_call)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'POST /api/v1/accounts/:account_id/whatsapp_calls/:id/upload_recording' do
|
||||
|
||||
@@ -127,5 +127,20 @@ RSpec.describe Voice::OutboundCallBuilder do
|
||||
)
|
||||
end.to raise_error(ArgumentError, 'Agent required')
|
||||
end
|
||||
|
||||
it 'raises before initiating the provider call when the inbox is disabled' do
|
||||
inbox.update!(active: false)
|
||||
|
||||
expect do
|
||||
described_class.perform!(
|
||||
account: account,
|
||||
inbox: inbox,
|
||||
user: user,
|
||||
contact: contact
|
||||
)
|
||||
end.to raise_error(CustomExceptions::InboxDisabled)
|
||||
|
||||
expect(channel).not_to have_received(:initiate_call)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -202,6 +202,24 @@ describe Webhooks::InstagramEventsJob do
|
||||
instagram_webhook.perform_now(messaging_seen_event[:entry])
|
||||
end
|
||||
|
||||
it 'handles messaging_seen callback when the inbox is disabled' do
|
||||
messaging_seen_event = build(:messaging_seen_event).with_indifferent_access
|
||||
instagram_messenger_inbox.update!(active: false)
|
||||
|
||||
expect(Instagram::ReadStatusService).to receive(:new).with(params: messaging_seen_event[:entry][0][:messaging][0],
|
||||
channel: instagram_messenger_inbox.channel).and_call_original
|
||||
instagram_webhook.perform_now(messaging_seen_event[:entry])
|
||||
end
|
||||
|
||||
it 'does not create message callbacks when the inbox is disabled' do
|
||||
dm_event = build(:instagram_message_create_event).with_indifferent_access
|
||||
instagram_messenger_inbox.update!(active: false)
|
||||
|
||||
instagram_webhook.perform_now(dm_event[:entry])
|
||||
|
||||
expect(instagram_messenger_inbox.messages.count).to be 0
|
||||
end
|
||||
|
||||
it 'handles unsupported message' do
|
||||
unsupported_event = build(:instagram_message_unsupported_event).with_indifferent_access
|
||||
sender_id = unsupported_event[:entry][0][:messaging][0][:sender][:id]
|
||||
|
||||
@@ -42,6 +42,38 @@ RSpec.describe Webhooks::TiktokEventsJob do
|
||||
expect(read_status_service).to have_received(:perform)
|
||||
end
|
||||
|
||||
it 'processes im_mark_read_msg events when the inbox is disabled' do
|
||||
channel.inbox.update!(active: false)
|
||||
read_status_service = instance_double(Tiktok::ReadStatusService, perform: true)
|
||||
allow(Tiktok::ReadStatusService).to receive(:new).and_return(read_status_service)
|
||||
|
||||
event = {
|
||||
event: 'im_mark_read_msg',
|
||||
user_openid: 'biz-123',
|
||||
content: { conversation_id: 'tt-conv-1', read: { last_read_timestamp: 1_700_000_000_000 }, from_user: { id: 'user-1' } }.to_json
|
||||
}
|
||||
|
||||
job.perform(event)
|
||||
|
||||
expect(Tiktok::ReadStatusService).to have_received(:new).with(channel: channel, content: hash_including(conversation_id: 'tt-conv-1'))
|
||||
expect(read_status_service).to have_received(:perform)
|
||||
end
|
||||
|
||||
it 'does not process im_receive_msg events when the inbox is disabled' do
|
||||
channel.inbox.update!(active: false)
|
||||
allow(Tiktok::MessageService).to receive(:new)
|
||||
|
||||
event = {
|
||||
event: 'im_receive_msg',
|
||||
user_openid: 'biz-123',
|
||||
content: { conversation_id: 'tt-conv-1' }.to_json
|
||||
}
|
||||
|
||||
job.perform(event)
|
||||
|
||||
expect(Tiktok::MessageService).not_to have_received(:new)
|
||||
end
|
||||
|
||||
it 'ignores unsupported event types' do
|
||||
allow(Tiktok::MessageService).to receive(:new)
|
||||
|
||||
|
||||
@@ -106,6 +106,34 @@ RSpec.describe Webhooks::WhatsappEventsJob do
|
||||
job.perform_now(phone_number: unknown_phone)
|
||||
end
|
||||
|
||||
it 'processes status callbacks when the inbox is disabled' do
|
||||
wb_params = params.deep_dup
|
||||
wb_params[:entry].first[:changes].first[:value][:statuses] = [
|
||||
{ id: 'wamid-test', recipient_id: '919745786257', status: 'delivered' }
|
||||
]
|
||||
channel.inbox.update!(active: false)
|
||||
|
||||
allow(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).and_return(process_service)
|
||||
|
||||
expect(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new).with(inbox: channel.inbox, params: wb_params)
|
||||
|
||||
job.perform_now(wb_params)
|
||||
end
|
||||
|
||||
it 'does not process message callbacks when the inbox is disabled' do
|
||||
wb_params = params.deep_dup
|
||||
wb_params[:entry].first[:changes].first[:value][:messages] = [
|
||||
{ from: '919745786257', id: 'wamid-test', text: { body: 'Hello' }, type: 'text' }
|
||||
]
|
||||
channel.inbox.update!(active: false)
|
||||
|
||||
allow(Whatsapp::IncomingMessageWhatsappCloudService).to receive(:new)
|
||||
|
||||
expect(Whatsapp::IncomingMessageWhatsappCloudService).not_to receive(:new)
|
||||
|
||||
job.perform_now(wb_params)
|
||||
end
|
||||
|
||||
it 'uses from_user_id as the mutex sender for BSUID-only inbound messages' do
|
||||
bsuid = 'IN.2081978709342942'
|
||||
wb_params = params.deep_dup
|
||||
|
||||
@@ -42,6 +42,17 @@ RSpec.describe ReplyMailbox do
|
||||
end
|
||||
end
|
||||
|
||||
context 'with reply uuid present for a disabled inbox' do
|
||||
before do
|
||||
conversation.update!(uuid: '6bdc3f4d-0bec-4515-a284-5d916fdde489')
|
||||
conversation.inbox.update!(active: false)
|
||||
end
|
||||
|
||||
it 'does not add the mail content as a new message' do
|
||||
expect { described_subject }.not_to change(Message, :count)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with in reply to email' do
|
||||
let(:reply_mail_without_uuid) { create_inbound_email_from_fixture('reply_mail_without_uuid.eml') }
|
||||
let(:described_subject) { described_class.receive reply_mail_without_uuid }
|
||||
@@ -67,6 +78,27 @@ RSpec.describe ReplyMailbox do
|
||||
end
|
||||
end
|
||||
|
||||
context 'when forwarded email inbox is disabled' do
|
||||
let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) }
|
||||
let(:forwarded_mail) { create_inbound_email_from_mail(from: 'sender@example.com', to: email_channel.email, subject: 'Hello') }
|
||||
|
||||
before do
|
||||
email_channel.inbox.update!(active: false)
|
||||
end
|
||||
|
||||
it 'does not create contacts, conversations, or messages' do
|
||||
contact_count = Contact.count
|
||||
conversation_count = Conversation.count
|
||||
message_count = Message.count
|
||||
|
||||
described_class.receive forwarded_mail
|
||||
|
||||
expect(Contact.count).to eq(contact_count)
|
||||
expect(Conversation.count).to eq(conversation_count)
|
||||
expect(Message.count).to eq(message_count)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when new conversation email contains null bytes' do
|
||||
let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) }
|
||||
let(:null_byte_mail) { create_inbound_email_from_mail(from: 'sender@example.com', to: email_channel.email, subject: 'Hello') }
|
||||
|
||||
@@ -252,6 +252,18 @@ describe Whatsapp::IncomingMessageService do
|
||||
expect(message.reload.status).to eq('read')
|
||||
end
|
||||
|
||||
it 'updates message status when the inbox is disabled' do
|
||||
status_params = {
|
||||
'statuses' => [{ 'recipient_id' => from, 'id' => from, 'status' => 'read' }]
|
||||
}.with_indifferent_access
|
||||
message = Message.find_by!(source_id: from)
|
||||
whatsapp_channel.inbox.update!(active: false)
|
||||
|
||||
described_class.new(inbox: whatsapp_channel.inbox, params: status_params).perform
|
||||
|
||||
expect(message.reload.status).to eq('read')
|
||||
end
|
||||
|
||||
it 'stores BSUID source ids from status contacts' do
|
||||
bsuid = 'IN.2081978709342942'
|
||||
parent_bsuid = 'IN.ENT.9081726354'
|
||||
|
||||
Reference in New Issue
Block a user