diff --git a/app/assets/javascripts/discourse/app/models/private-message-topic-tracking-state.js b/app/assets/javascripts/discourse/app/models/private-message-topic-tracking-state.js index c8a8d0fcb44..a7fafaeb279 100644 --- a/app/assets/javascripts/discourse/app/models/private-message-topic-tracking-state.js +++ b/app/assets/javascripts/discourse/app/models/private-message-topic-tracking-state.js @@ -168,18 +168,12 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({ this._notifyIncoming(message.topic_id); } - break; - case "archive": - if ( - [INBOX_FILTER, ARCHIVE_FILTER].includes(this.filter) && - ["user", "all"].includes(this.inbox) - ) { - this._notifyIncoming(message.topic_id); - } break; case "group_archive": if ( [INBOX_FILTER, ARCHIVE_FILTER].includes(this.filter) && + (!message.payload.acting_user_id || + message.payload.acting_user_id !== this.currentUser.id) && (this.inbox === "all" || this._displayMessageForGroupInbox(message)) ) { this._notifyIncoming(message.topic_id); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js index 5157fa71e77..29372f68225 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js @@ -235,16 +235,6 @@ acceptance( ); }; - const publishArchiveToMessageBus = function (opts) { - publishToMessageBus( - `/private-message-topic-tracking-state/user/${opts.userId || 5}`, - { - topic_id: opts.topicId, - message_type: "archive", - } - ); - }; - const publishGroupArchiveToMessageBus = function (opts) { publishToMessageBus( `/private-message-topic-tracking-state/group/${opts.groupIds[0]}`, @@ -253,6 +243,7 @@ acceptance( message_type: "group_archive", payload: { group_ids: opts.groupIds, + acting_user_id: opts.actingUserId, }, } ); @@ -289,42 +280,6 @@ acceptance( ); }; - test("incoming archive message on all and archive filter", async function (assert) { - for (const url of [ - "/u/charlie/messages", - "/u/charlie/messages/archive", - "/u/charlie/messages/personal", - "/u/charlie/messages/personal/archive", - ]) { - await visit(url); - - publishArchiveToMessageBus({ topicId: 1 }); - - await visit(url); // wait for re-render - - assert.ok( - exists(".show-mores"), - `${url} displays the topic incoming info` - ); - } - - for (const url of [ - "/u/charlie/messages/group/awesome_group/archive", - "/u/charlie/messages/group/awesome_group", - ]) { - await visit(url); - - publishArchiveToMessageBus({ topicId: 1 }); - - await visit(url); // wait for re-render - - assert.ok( - !exists(".show-mores"), - `${url} does not display the topic incoming info` - ); - } - }); - test("incoming read message on unread filter", async function (assert) { await visit("/u/charlie/messages/unread"); @@ -335,6 +290,23 @@ acceptance( assert.ok(exists(".show-mores"), `displays the topic incoming info`); }); + test("incoming group archive message acted by current user", async function (assert) { + await visit("/u/charlie/messages"); + + publishGroupArchiveToMessageBus({ + groupIds: [14], + topicId: 1, + actingUserId: 5, + }); + + await visit("/u/charlie/messages"); // wait for re-render + + assert.ok( + !exists(".show-mores"), + `does not display the topic incoming info` + ); + }); + test("incoming group archive message on all and archive filter", async function (assert) { for (const url of [ "/u/charlie/messages", diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 1f9ab77c30f..710632bae0b 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -548,12 +548,22 @@ class TopicsController < ApplicationController if group_ids.present? allowed_groups = topic.allowed_groups .where('topic_allowed_groups.group_id IN (?)', group_ids).pluck(:id) + allowed_groups.each do |id| if archive - GroupArchivedMessage.archive!(id, topic) + GroupArchivedMessage.archive!( + id, + topic, + acting_user_id: current_user.id + ) + group_id = id else - GroupArchivedMessage.move_to_inbox!(id, topic) + GroupArchivedMessage.move_to_inbox!( + id, + topic, + acting_user_id: current_user.id + ) end end end diff --git a/app/models/group_archived_message.rb b/app/models/group_archived_message.rb index 3b481c86f12..ea5eb6a9abb 100644 --- a/app/models/group_archived_message.rb +++ b/app/models/group_archived_message.rb @@ -9,7 +9,7 @@ class GroupArchivedMessage < ActiveRecord::Base destroyed = GroupArchivedMessage.where(group_id: group_id, topic_id: topic_id).destroy_all trigger(:move_to_inbox, group_id, topic_id) MessageBus.publish("/topic/#{topic_id}", { type: "move_to_inbox" }, group_ids: [group_id]) - publish_topic_tracking_state(topic, group_id) + publish_topic_tracking_state(topic, group_id, opts[:acting_user_id]) set_imap_sync(topic_id) if !opts[:skip_imap_sync] && destroyed.present? end @@ -19,7 +19,7 @@ class GroupArchivedMessage < ActiveRecord::Base GroupArchivedMessage.create!(group_id: group_id, topic_id: topic_id) trigger(:archive_message, group_id, topic_id) MessageBus.publish("/topic/#{topic_id}", { type: "archived" }, group_ids: [group_id]) - publish_topic_tracking_state(topic, group_id) + publish_topic_tracking_state(topic, group_id, opts[:acting_user_id]) set_imap_sync(topic_id) if !opts[:skip_imap_sync] && destroyed.blank? end @@ -39,8 +39,12 @@ class GroupArchivedMessage < ActiveRecord::Base end private_class_method :set_imap_sync - def self.publish_topic_tracking_state(topic, group_id) - PrivateMessageTopicTrackingState.publish_group_archived(topic, group_id) + def self.publish_topic_tracking_state(topic, group_id, acting_user_id = nil) + PrivateMessageTopicTrackingState.publish_group_archived( + topic: topic, + group_id: group_id, + acting_user_id: acting_user_id + ) end private_class_method :publish_topic_tracking_state end diff --git a/app/models/private_message_topic_tracking_state.rb b/app/models/private_message_topic_tracking_state.rb index 0c33e97b2d3..74ad8d5f12b 100644 --- a/app/models/private_message_topic_tracking_state.rb +++ b/app/models/private_message_topic_tracking_state.rb @@ -21,7 +21,6 @@ class PrivateMessageTopicTrackingState NEW_MESSAGE_TYPE = "new_topic" UNREAD_MESSAGE_TYPE = "unread" READ_MESSAGE_TYPE = "read" - ARCHIVE_MESSAGE_TYPE = "archive" GROUP_ARCHIVE_MESSAGE_TYPE = "group_archive" def self.report(user) @@ -163,29 +162,23 @@ class PrivateMessageTopicTrackingState end end - def self.publish_group_archived(topic, group_id) + def self.publish_group_archived(topic:, group_id:, acting_user_id: nil) return unless topic.private_message? message = { message_type: GROUP_ARCHIVE_MESSAGE_TYPE, topic_id: topic.id, payload: { - group_ids: [group_id] + group_ids: [group_id], + acting_user_id: acting_user_id } }.as_json - MessageBus.publish(self.group_channel(group_id), message, group_ids: [group_id]) - end - - def self.publish_user_archived(topic, user_id) - return unless topic.private_message? - - message = { - message_type: ARCHIVE_MESSAGE_TYPE, - topic_id: topic.id, - }.as_json - - MessageBus.publish(self.user_channel(user_id), message, user_ids: [user_id]) + MessageBus.publish( + self.group_channel(group_id), + message, + group_ids: [group_id] + ) end def self.publish_read(topic_id, last_read_post_number, user, notification_level = nil) diff --git a/app/models/user_archived_message.rb b/app/models/user_archived_message.rb index f5ee006dfbe..2410f50e2e2 100644 --- a/app/models/user_archived_message.rb +++ b/app/models/user_archived_message.rb @@ -16,7 +16,6 @@ class UserArchivedMessage < ActiveRecord::Base UserArchivedMessage.where(user_id: user_id, topic_id: topic_id).destroy_all trigger(:move_to_inbox, user_id, topic_id) MessageBus.publish("/topic/#{topic_id}", { type: "move_to_inbox" }, user_ids: [user_id]) - publish_topic_tracking_state(topic, user_id) end def self.archive!(user_id, topic) @@ -25,7 +24,6 @@ class UserArchivedMessage < ActiveRecord::Base UserArchivedMessage.create!(user_id: user_id, topic_id: topic_id) trigger(:archive_message, user_id, topic_id) MessageBus.publish("/topic/#{topic_id}", { type: "archived" }, user_ids: [user_id]) - publish_topic_tracking_state(topic, user_id) end def self.trigger(event, user_id, topic_id) @@ -35,11 +33,6 @@ class UserArchivedMessage < ActiveRecord::Base DiscourseEvent.trigger(event, user: user, topic: topic) end end - - def self.publish_topic_tracking_state(topic, user_id) - PrivateMessageTopicTrackingState.publish_user_archived(topic, user_id) - end - private_class_method :publish_topic_tracking_state end # == Schema Information diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 69a94bdc806..3bedd2a1d10 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -480,7 +480,7 @@ class PostCreator end GroupArchivedMessage.where(topic_id: @topic.id).pluck(:group_id).each do |group_id| - GroupArchivedMessage.move_to_inbox!(group_id, @topic) + GroupArchivedMessage.move_to_inbox!(group_id, @topic, acting_user_id: @user.id) end end diff --git a/lib/topics_bulk_action.rb b/lib/topics_bulk_action.rb index e0a82daa4c8..f38f4b17cef 100644 --- a/lib/topics_bulk_action.rb +++ b/lib/topics_bulk_action.rb @@ -47,7 +47,7 @@ class TopicsBulkAction topics.each do |t| if guardian.can_see?(t) && t.private_message? if group - GroupArchivedMessage.move_to_inbox!(group.id, t) + GroupArchivedMessage.move_to_inbox!(group.id, t, acting_user_id: @user.id) else UserArchivedMessage.move_to_inbox!(@user.id, t) end @@ -60,7 +60,7 @@ class TopicsBulkAction topics.each do |t| if guardian.can_see?(t) && t.private_message? if group - GroupArchivedMessage.archive!(group.id, t) + GroupArchivedMessage.archive!(group.id, t, acting_user_id: @user.id) else UserArchivedMessage.archive!(@user.id, t) end diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 165162953e5..bf677b39df9 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -1134,6 +1134,26 @@ describe PostCreator do expect(target_user1.notifications.count).to eq(1) expect(target_user2.notifications.count).to eq(1) + + GroupArchivedMessage.create!(group: group, topic: post.topic) + + message = MessageBus.track_publish( + PrivateMessageTopicTrackingState.group_channel(group.id) + ) do + PostCreator.create!(user, + raw: "this is a reply to the group message", + topic_id: post.topic_id + ) + end.first + + expect(message.data["message_type"]).to eq( + PrivateMessageTopicTrackingState::GROUP_ARCHIVE_MESSAGE_TYPE + ) + + expect(message.data["payload"]["acting_user_id"]).to eq(user.id) + + expect(GroupArchivedMessage.exists?(group: group, topic: post.topic)) + .to eq(false) end end diff --git a/spec/models/private_message_topic_tracking_state_spec.rb b/spec/models/private_message_topic_tracking_state_spec.rb index f4aa96f3235..f0fdb380a91 100644 --- a/spec/models/private_message_topic_tracking_state_spec.rb +++ b/spec/models/private_message_topic_tracking_state_spec.rb @@ -168,26 +168,17 @@ describe PrivateMessageTopicTrackingState do end end - describe '.publish_user_archived' do - it 'should publish the right message_bus message' do - message = MessageBus.track_publish(described_class.user_channel(user.id)) do - described_class.publish_user_archived(private_message, user.id) - end.first - - data = message.data - - expect(data['topic_id']).to eq(private_message.id) - expect(data['message_type']).to eq(described_class::ARCHIVE_MESSAGE_TYPE) - end - end - describe '.publish_group_archived' do it 'should publish the right message_bus message' do user_3 = Fabricate(:user) group.add(user_3) messages = MessageBus.track_publish do - described_class.publish_group_archived(group_message, group.id) + described_class.publish_group_archived( + topic: group_message, + group_id: group.id, + acting_user_id: user_3.id + ) end expect(messages.map(&:channel)).to contain_exactly( @@ -201,6 +192,7 @@ describe PrivateMessageTopicTrackingState do expect(data['message_type']).to eq(described_class::GROUP_ARCHIVE_MESSAGE_TYPE) expect(data['topic_id']).to eq(group_message.id) expect(data['payload']['group_ids']).to contain_exactly(group.id) + expect(data['payload']['acting_user_id']).to eq(user_3.id) end end diff --git a/spec/models/user_archived_message_spec.rb b/spec/models/user_archived_message_spec.rb index dcfd7743e17..8a2f5a0795d 100644 --- a/spec/models/user_archived_message_spec.rb +++ b/spec/models/user_archived_message_spec.rb @@ -20,11 +20,7 @@ describe UserArchivedMessage do UserArchivedMessage.archive!(user.id, private_message) expect do - messages = MessageBus.track_publish(PrivateMessageTopicTrackingState.user_channel(user.id)) do - UserArchivedMessage.move_to_inbox!(user.id, private_message) - end - - expect(messages.present?).to eq(true) + UserArchivedMessage.move_to_inbox!(user.id, private_message) end.to change { private_message.message_archived?(user) }.from(true).to(false) end @@ -40,14 +36,4 @@ describe UserArchivedMessage do end end - describe '.archive' do - it 'archives message correctly' do - messages = MessageBus.track_publish(PrivateMessageTopicTrackingState.user_channel(user.id)) do - UserArchivedMessage.archive!(user.id, private_message) - end - - expect(messages.present?).to eq(true) - end - end - end diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 6cd81254d66..f904694a6bd 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -4458,4 +4458,43 @@ RSpec.describe TopicsController do .to eq(true) end end + + describe '#archive_message' do + fab!(:group) do + Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]).tap do |g| + g.add(user) + end + end + + fab!(:group_message) do + create_post( + user: user, + target_group_names: [group.name], + archetype: Archetype.private_message + ).topic + end + + it 'should be able to archive a private message' do + sign_in(user) + + message = MessageBus.track_publish( + PrivateMessageTopicTrackingState.group_channel(group.id) + ) do + + put "/t/#{group_message.id}/archive-message.json" + + expect(response.status).to eq(200) + end.first + + expect(message.data["message_type"]).to eq( + PrivateMessageTopicTrackingState::GROUP_ARCHIVE_MESSAGE_TYPE + ) + + expect(message.data["payload"]["acting_user_id"]).to eq(user.id) + + body = response.parsed_body + + expect(body["group_name"]).to eq(group.name) + end + end end