From bd614587206792ad9e6c0896717525a0f19d446d Mon Sep 17 00:00:00 2001 From: Gabriel Jablonski Date: Sun, 19 Apr 2026 14:06:02 -0300 Subject: [PATCH] fix(internal-chat): use internal_chat_channel_id in delete payloads (#270) Backend broadcast payloads for internal_chat.message.deleted and internal_chat.reaction.deleted used channel_id as the key, but the frontend ActionCable handlers (and all other internal-chat events) expect internal_chat_channel_id. This caused deleted messages and removed reactions to stay visible on screen until a manual refresh. Rename the key on the backend so the payloads match the convention shared with message.created/updated and reaction.created, and drop the defensive fallback on the frontend reaction-deleted handler. --- .../internal_chat/messages_controller.rb | 2 +- .../internal_chat/reactions_controller.rb | 2 +- app/javascript/dashboard/helper/actionCable.js | 2 +- app/listeners/internal_chat_listener.rb | 4 ++-- spec/listeners/internal_chat_listener_spec.rb | 16 ++++++++-------- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/controllers/api/v1/accounts/internal_chat/messages_controller.rb b/app/controllers/api/v1/accounts/internal_chat/messages_controller.rb index 9f0495177..b0c514639 100644 --- a/app/controllers/api/v1/accounts/internal_chat/messages_controller.rb +++ b/app/controllers/api/v1/accounts/internal_chat/messages_controller.rb @@ -40,7 +40,7 @@ class Api::V1::Accounts::InternalChat::MessagesController < Api::V1::Accounts::I authorize @message, :destroy?, policy_class: InternalChat::MessagePolicy message_data = { id: @message.id, - channel_id: @message.internal_chat_channel_id, + internal_chat_channel_id: @message.internal_chat_channel_id, account_id: @message.account_id } @message.update!(content: I18n.t('internal_chat.messages.deleted'), content_attributes: { deleted: true }) diff --git a/app/controllers/api/v1/accounts/internal_chat/reactions_controller.rb b/app/controllers/api/v1/accounts/internal_chat/reactions_controller.rb index ef8c95ec7..17ee5b18f 100644 --- a/app/controllers/api/v1/accounts/internal_chat/reactions_controller.rb +++ b/app/controllers/api/v1/accounts/internal_chat/reactions_controller.rb @@ -17,7 +17,7 @@ class Api::V1::Accounts::InternalChat::ReactionsController < Api::V1::Accounts:: reaction_data = { id: @reaction.id, message_id: @reaction.internal_chat_message_id, - channel_id: @message.internal_chat_channel_id, + internal_chat_channel_id: @message.internal_chat_channel_id, account_id: @message.account_id, user_id: @reaction.user_id, emoji: @reaction.emoji diff --git a/app/javascript/dashboard/helper/actionCable.js b/app/javascript/dashboard/helper/actionCable.js index c2a201136..4300d8748 100644 --- a/app/javascript/dashboard/helper/actionCable.js +++ b/app/javascript/dashboard/helper/actionCable.js @@ -342,7 +342,7 @@ class ActionCableConnector extends BaseActionCableConnector { onInternalChatReactionDeleted = data => { this.app.$store.dispatch('internalChat/messages/removeReactionFromCable', { - channelId: data.internal_chat_channel_id || data.channel_id, + channelId: data.internal_chat_channel_id, messageId: data.message_id, reactionId: data.id, }); diff --git a/app/listeners/internal_chat_listener.rb b/app/listeners/internal_chat_listener.rb index ee32bb87d..cbe78cff6 100644 --- a/app/listeners/internal_chat_listener.rb +++ b/app/listeners/internal_chat_listener.rb @@ -24,7 +24,7 @@ class InternalChatListener < BaseListener def internal_chat_message_deleted(event) message_data = event.data[:message_data] account = Account.find_by(id: message_data[:account_id]) - channel = InternalChat::Channel.find_by(id: message_data[:channel_id]) + channel = InternalChat::Channel.find_by(id: message_data[:internal_chat_channel_id]) return if account.blank? || channel.blank? return unless channel.account_id == account.id @@ -93,7 +93,7 @@ class InternalChatListener < BaseListener def internal_chat_reaction_deleted(event) reaction_data = event.data[:reaction_data] account = Account.find_by(id: reaction_data[:account_id]) - channel = InternalChat::Channel.find_by(id: reaction_data[:channel_id]) + channel = InternalChat::Channel.find_by(id: reaction_data[:internal_chat_channel_id]) return if account.blank? || channel.blank? return unless channel.account_id == account.id diff --git a/spec/listeners/internal_chat_listener_spec.rb b/spec/listeners/internal_chat_listener_spec.rb index 57c50602a..04b972db8 100644 --- a/spec/listeners/internal_chat_listener_spec.rb +++ b/spec/listeners/internal_chat_listener_spec.rb @@ -148,7 +148,7 @@ describe InternalChatListener do describe '#internal_chat_message_deleted' do let!(:channel) { create(:internal_chat_channel, :public_channel, account: account) } - let(:message_data) { { account_id: account.id, channel_id: channel.id, id: 999 } } + let(:message_data) { { account_id: account.id, internal_chat_channel_id: channel.id, id: 999 } } let!(:event) { Events::Base.new(:'internal_chat.message.deleted', Time.zone.now, message_data: message_data) } it 'broadcasts to all channel members' do @@ -157,14 +157,14 @@ describe InternalChatListener do 'internal_chat.message.deleted', hash_including( account_id: account.id, - channel_id: channel.id + internal_chat_channel_id: channel.id ) ) listener.internal_chat_message_deleted(event) end context 'when account or channel is not found' do - let(:message_data) { { account_id: 0, channel_id: 0, id: 999 } } + let(:message_data) { { account_id: 0, internal_chat_channel_id: 0, id: 999 } } it 'does not broadcast' do expect(ActionCableBroadcastJob).not_to receive(:perform_later) @@ -175,7 +175,7 @@ describe InternalChatListener do context 'when account and channel belong to different accounts' do let(:other_account) { create(:account) } let(:other_channel) { create(:internal_chat_channel, :public_channel, account: other_account) } - let(:message_data) { { account_id: account.id, channel_id: other_channel.id, id: 999 } } + let(:message_data) { { account_id: account.id, internal_chat_channel_id: other_channel.id, id: 999 } } it 'does not broadcast' do expect(ActionCableBroadcastJob).not_to receive(:perform_later) @@ -186,7 +186,7 @@ describe InternalChatListener do describe '#internal_chat_reaction_deleted' do let!(:channel) { create(:internal_chat_channel, :public_channel, account: account) } - let(:reaction_data) { { account_id: account.id, channel_id: channel.id, id: 999 } } + let(:reaction_data) { { account_id: account.id, internal_chat_channel_id: channel.id, id: 999 } } let!(:event) { Events::Base.new(:'internal_chat.reaction.deleted', Time.zone.now, reaction_data: reaction_data) } it 'broadcasts to all channel members' do @@ -195,14 +195,14 @@ describe InternalChatListener do 'internal_chat.reaction.deleted', hash_including( account_id: account.id, - channel_id: channel.id + internal_chat_channel_id: channel.id ) ) listener.internal_chat_reaction_deleted(event) end context 'when account or channel is not found' do - let(:reaction_data) { { account_id: 0, channel_id: 0, id: 999 } } + let(:reaction_data) { { account_id: 0, internal_chat_channel_id: 0, id: 999 } } it 'does not broadcast' do expect(ActionCableBroadcastJob).not_to receive(:perform_later) @@ -213,7 +213,7 @@ describe InternalChatListener do context 'when account and channel belong to different accounts' do let(:other_account) { create(:account) } let(:other_channel) { create(:internal_chat_channel, :public_channel, account: other_account) } - let(:reaction_data) { { account_id: account.id, channel_id: other_channel.id, id: 999 } } + let(:reaction_data) { { account_id: account.id, internal_chat_channel_id: other_channel.id, id: 999 } } it 'does not broadcast' do expect(ActionCableBroadcastJob).not_to receive(:perform_later)