FIX: Don't publish PM archive events to acting user. (#14291)

FIX: Don’t publish PM archive events to acting user. (#14291)

When a user archives a personal message, they are redirected back to the inbox and will refresh the list of the topics for the given filter. Publishing an event to the user results in an incorrect incoming message because the list of topics has already been refreshed.

This does mean that if a user has two tabs opened, the non-active tab will not receive the incoming message but at this point we do not think the technical trade-offs are worth it to support this feature. We basically have to somehow exclude a client from an incoming message which is not easy to do.

Follow-up to fc1fd1b41689694b3916dc4e10605eb9b8bb89b7

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 c8a8d0f..a7fafae 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
@@ -169,17 +169,11 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({
         }
 
         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 5157fa7..29372f6 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 1f9ab77..710632b 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 3b481c8..ea5eb6a 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 0c33e97..74ad8d5 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)

[... diff too long, it was truncated ...]

GitHub sha: bc23dcd30b1106bfd9a2ebeff4a7add21b23a353

This commit appears in #14291 which was approved by eviltrout. It was merged by tgxworld.