FIX: Base topic details message on current category and tag tracking state (#12937)

FIX: Base topic details message on current category and tag tracking state (#12937)

The user may have changed their category or tag tracking settings since a topic was tracked/watched based on those settings in the past. In that case we need to alter the reason message we show them otherwise it is very confusing for the end user to be told they are tracking a topic because of a category, when they are no longer tracking that category.

For example: “You will see a count of new replies because you are tracking this category.” becomes: “You will see a count of new replies because you were tracking this category in the past.”

To do this, it was necessary to add tag and category tracking info to current user serializer. I improved the serializer code so it only does 3 SQL queries instead of 9 to get the tracking information for tags and categories for the current user.

diff --git a/app/assets/javascripts/discourse/app/models/topic-details.js b/app/assets/javascripts/discourse/app/models/topic-details.js
index 8162a05..c96e73c 100644
--- a/app/assets/javascripts/discourse/app/models/topic-details.js
+++ b/app/assets/javascripts/discourse/app/models/topic-details.js
@@ -1,6 +1,4 @@
 import EmberObject from "@ember/object";
-import I18n from "I18n";
-import { NotificationLevels } from "discourse/lib/notification-levels";
 import RestModel from "discourse/models/rest";
 import User from "discourse/models/user";
 import { ajax } from "discourse/lib/ajax";
@@ -9,8 +7,6 @@ import { ajax } from "discourse/lib/ajax";
   A model representing a Topic's details that aren't always present, such as a list of participants.
   When showing topics in lists and such this information should not be required.
 **/
-import discourseComputed from "discourse-common/utils/decorators";
-import getURL from "discourse-common/lib/get-url";
 
 const TopicDetails = RestModel.extend({
   loaded: false,
@@ -35,34 +31,6 @@ const TopicDetails = RestModel.extend({
     this.set("loaded", true);
   },
 
-  @discourseComputed("notification_level", "notifications_reason_id")
-  notificationReasonText(level, reason) {
-    if (typeof level !== "number") {
-      level = 1;
-    }
-
-    let localeString = `topic.notifications.reasons.${level}`;
-    if (typeof reason === "number") {
-      const tmp = localeString + "_" + reason;
-      // some sane protection for missing translations of edge cases
-      if (I18n.lookup(tmp, { locale: "en" })) {
-        localeString = tmp;
-      }
-    }
-
-    if (
-      User.currentProp("mailing_list_mode") &&
-      level > NotificationLevels.MUTED
-    ) {
-      return I18n.t("topic.notifications.reasons.mailing_list_mode");
-    } else {
-      return I18n.t(localeString, {
-        username: User.currentProp("username_lower"),
-        basePath: getURL(""),
-      });
-    }
-  },
-
   updateNotifications(level) {
     return ajax(`/t/${this.get("topic.id")}/notifications`, {
       type: "POST",
diff --git a/app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-button-test.js b/app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-button-test.js
index a939555..c6e1e9f 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-button-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-button-test.js
@@ -3,19 +3,25 @@ import componentTest, {
 } from "discourse/tests/helpers/component-test";
 import I18n from "I18n";
 import Topic from "discourse/models/topic";
-import { discourseModule } from "discourse/tests/helpers/qunit-helpers";
+import {
+  discourseModule,
+  queryAll,
+} from "discourse/tests/helpers/qunit-helpers";
 import hbs from "htmlbars-inline-precompile";
 import selectKit from "discourse/tests/helpers/select-kit-helper";
 
-const buildTopic = function (level, archetype = "regular") {
+const buildTopic = function (opts) {
   return Topic.create({
     id: 4563,
   }).updateFromJson({
     title: "Qunit Test Topic",
     details: {
-      notification_level: level,
+      notification_level: opts.level,
+      notifications_reason_id: opts.reason || null,
     },
-    archetype,
+    archetype: opts.archetype || "regular",
+    category_id: opts.category_id || null,
+    tags: opts.tags || [],
   });
 };
 
@@ -40,7 +46,7 @@ discourseModule(
       `,
 
       beforeEach() {
-        this.set("topic", buildTopic(1));
+        this.set("topic", buildTopic({ level: 1 }));
       },
 
       async test(assert) {
@@ -50,7 +56,7 @@ discourseModule(
           "it has the correct label"
         );
 
-        await this.set("topic", buildTopic(2));
+        await this.set("topic", buildTopic({ level: 2 }));
 
         assert.equal(
           selectKit().header().label(),
@@ -70,7 +76,10 @@ discourseModule(
 
       beforeEach() {
         I18n.translations.en.js.topic.notifications.tracking_pm.title = `${originalTranslation} PM`;
-        this.set("topic", buildTopic(2, "private_message"));
+        this.set(
+          "topic",
+          buildTopic({ level: 2, archetype: "private_message" })
+        );
       },
 
       test(assert) {
@@ -81,5 +90,194 @@ discourseModule(
         );
       },
     });
+
+    componentTest("notification reason text - user mailing list mode", {
+      template: hbs`
+        {{topic-notifications-button
+          notificationLevel=topic.details.notification_level
+          topic=topic
+        }}
+      `,
+
+      beforeEach() {
+        this.currentUser.set("mailing_list_mode", true);
+        this.set("topic", buildTopic({ level: 2 }));
+      },
+
+      test(assert) {
+        assert.equal(
+          queryAll(".topic-notifications-button .text").text(),
+          I18n.t("topic.notifications.reasons.mailing_list_mode"),
+          "mailing_list_mode enabled for the user shows unique text"
+        );
+      },
+    });
+
+    componentTest("notification reason text - bad notification reason", {
+      template: hbs`
+        {{topic-notifications-button
+          notificationLevel=topic.details.notification_level
+          topic=topic
+        }}
+      `,
+
+      beforeEach() {
+        this.set("topic", buildTopic({ level: 2 }));
+      },
+
+      test(assert) {
+        this.set("topic", buildTopic({ level: 3, reason: 999 }));
+
+        assert.equal(
+          queryAll(".topic-notifications-button .text").text(),
+          I18n.t("topic.notifications.reasons.3"),
+          "fallback to regular level translation if reason does not exist"
+        );
+      },
+    });
+
+    componentTest("notification reason text - user tracking category", {
+      template: hbs`
+        {{topic-notifications-button
+          notificationLevel=topic.details.notification_level
+          topic=topic
+        }}
+      `,
+
+      beforeEach() {
+        this.currentUser.set("tracked_category_ids", [88]);
+        this.set("topic", buildTopic({ level: 2, reason: 8, category_id: 88 }));
+      },
+
+      test(assert) {
+        assert.equal(
+          queryAll(".topic-notifications-button .text").text(),
+          I18n.t("topic.notifications.reasons.2_8"),
+          "use 2_8 notification if user is still tracking category"
+        );
+      },
+    });
+
+    componentTest(
+      "notification reason text - user no longer tracking category",
+      {
+        template: hbs`
+        {{topic-notifications-button
+          notificationLevel=topic.details.notification_level
+          topic=topic
+        }}
+      `,
+
+        beforeEach() {
+          this.currentUser.set("tracked_category_ids", []);
+          this.set(
+            "topic",
+            buildTopic({ level: 2, reason: 8, category_id: 88 })
+          );
+        },
+
+        test(assert) {
+          assert.equal(
+            queryAll(".topic-notifications-button .text").text(),
+            I18n.t("topic.notifications.reasons.2_8_stale"),
+            "use _stale notification if user is no longer tracking category"
+          );
+        },
+      }
+    );
+
+    componentTest("notification reason text - user watching category", {
+      template: hbs`
+        {{topic-notifications-button
+          notificationLevel=topic.details.notification_level
+          topic=topic
+        }}
+      `,
+
+      beforeEach() {
+        this.currentUser.set("watched_category_ids", [88]);
+        this.set("topic", buildTopic({ level: 3, reason: 6, category_id: 88 }));
+      },
+
+      test(assert) {
+        assert.equal(
+          queryAll(".topic-notifications-button .text").text(),
+          I18n.t("topic.notifications.reasons.3_6"),
+          "use 3_6 notification if user is still watching category"
+        );
+      },
+    });
+
+    componentTest(

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

GitHub sha: 72648dd5

This commit appears in #12937 which was approved by eviltrout. It was merged by martin.