DEV: Add likes, mentions and replies tabs to the new user menu (#17623)

This commit is a subset of the changes proposed in https://github.com/discourse/discourse/pull/17379.

app/assets/javascripts/discourse/app/components/user-menu/likes-notifications-list.js

@@ -0,0 +1,7 @@
+import UserMenuNotificationsList from "discourse/components/user-menu/notifications-list";
+
+export default class UserMenuLikesNotificationsList extends UserMenuNotificationsList {
+  get filterByTypes() {
+    return ["liked", "liked_consolidated"];
+  }
+}

app/assets/javascripts/discourse/app/components/user-menu/mentions-notifications-list.js

@@ -0,0 +1,7 @@
+import UserMenuNotificationsList from "discourse/components/user-menu/notifications-list";
+
+export default class UserMenuMentionsNotificationsList extends UserMenuNotificationsList {
+  get filterByTypes() {
+    return ["mentioned"];
+  }
+}

app/assets/javascripts/discourse/app/components/user-menu/menu.js

@@ -1,23 +1,98 @@
 import GlimmerComponent from "discourse/components/glimmer";
 import { tracked } from "@glimmer/tracking";
 import { action } from "@ember/object";
+import UserMenuTab from "discourse/lib/user-menu/tab";
 
 const DEFAULT_TAB_ID = "all-notifications";
 const DEFAULT_PANEL_COMPONENT = "user-menu/notifications-list";
 
+const CORE_TOP_TABS = [
+  class extends UserMenuTab {
+    get id() {
+      return DEFAULT_TAB_ID;
+    }
+
+    get icon() {
+      return "bell";
+    }
+
+    get panelComponent() {
+      return DEFAULT_PANEL_COMPONENT;
+    }
+  },
+
+  class extends UserMenuTab {
+    get id() {
+      return "replies";
+    }
+
+    get icon() {
+      return "reply";
+    }
+
+    get panelComponent() {
+      return "user-menu/replies-notifications-list";
+    }
+  },
+
+  class extends UserMenuTab {
+    get id() {
+      return "mentions";
+    }
+
+    get icon() {
+      return "at";
+    }
+
+    get panelComponent() {
+      return "user-menu/mentions-notifications-list";
+    }
+  },
+
+  class extends UserMenuTab {
+    get id() {
+      return "likes";
+    }
+
+    get icon() {
+      return "heart";
+    }
+
+    get panelComponent() {
+      return "user-menu/likes-notifications-list";
+    }
+
+    get shouldDisplay() {
+      return !this.currentUser.likes_notifications_disabled;
+    }
+  },
+];
+
 export default class UserMenu extends GlimmerComponent {
   @tracked currentTabId = DEFAULT_TAB_ID;
   @tracked currentPanelComponent = DEFAULT_PANEL_COMPONENT;
 
-  get topTabs() {
+  constructor() {
-    const tabs = this._coreTopTabs;
+    super(...arguments);
+    this.topTabs = this._topTabs;
+    this.bottomTabs = this._bottomTabs;
+  }
+
+  get _topTabs() {
+    const tabs = [];
+    CORE_TOP_TABS.forEach((tabClass) => {
+      const tab = new tabClass(this.currentUser, this.siteSettings, this.site);
+      if (tab.shouldDisplay) {
+        tabs.push(tab);
+      }
+    });
     return tabs.map((tab, index) => {
       tab.position = index;
       return tab;
     });
   }
 
-  get bottomTabs() {
+  get _bottomTabs() {
     const topTabsLength = this.topTabs.length;
     return this._coreBottomTabs.map((tab, index) => {
       tab.position = index + topTabsLength;
@@ -25,16 +100,6 @@ export default class UserMenu extends GlimmerComponent {
     });
   }
 
-  get _coreTopTabs() {
-    return [
-      {
-        id: DEFAULT_TAB_ID,
-        icon: "bell",
-        panelComponent: DEFAULT_PANEL_COMPONENT,
-      },
-    ];
-  }
-
   get _coreBottomTabs() {
     return [
       {

app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js

@@ -29,9 +29,9 @@ export default class UserMenuNotificationsList extends UserMenuItemsList {
 
   get itemsCacheKey() {
     let key = "recent-notifications";
-    const types = this.filterByTypes?.toString();
+    const types = this.filterByTypes;
-    if (types) {
+    if (types?.length > 0) {
-      key += `-type-${types}`;
+      key += `-type-${types.join(",")}`;
     }
     return key;
   }
@@ -52,9 +52,10 @@ export default class UserMenuNotificationsList extends UserMenuItemsList {
       silent: this.currentUser.enforcedSecondFactor,
     };
 
-    const types = this.filterByTypes?.toString();
+    const types = this.filterByTypes;
-    if (types) {
+    if (types?.length > 0) {
-      params.filter_by_types = types;
+      params.filter_by_types = types.join(",");
+      params.silent = true;
     }
     return this.store
       .findStale("notification", params)
@@ -64,6 +65,7 @@ export default class UserMenuNotificationsList extends UserMenuItemsList {
 
   dismissWarningModal() {
     // TODO: add warning modal when there are unread high pri notifications
+    // TODO: review child components and override if necessary
     return null;
   }
 

app/assets/javascripts/discourse/app/components/user-menu/replies-notifications-list.js

@@ -0,0 +1,7 @@
+import UserMenuNotificationsList from "discourse/components/user-menu/notifications-list";
+
+export default class UserMenuRepliesNotificationsList extends UserMenuNotificationsList {
+  get filterByTypes() {
+    return ["replied"];
+  }
+}

app/assets/javascripts/discourse/app/lib/user-menu/tab.js

@@ -0,0 +1,27 @@
+export default class UserMenuTab {
+  constructor(currentUser, siteSettings, site) {
+    this.currentUser = currentUser;
+    this.siteSettings = siteSettings;
+    this.site = site;
+  }
+
+  get shouldDisplay() {
+    return true;
+  }
+
+  get count() {
+    return 0;
+  }
+
+  get panelComponent() {
+    throw new Error("not implemented");
+  }
+
+  get id() {
+    throw new Error("not implemented");
+  }
+
+  get icon() {
+    throw new Error("not implemented");
+  }
+}

app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js

@@ -1,14 +1,26 @@
 import { module, test } from "qunit";
 import { setupRenderingTest } from "discourse/tests/helpers/component-test";
-import { query, queryAll } from "discourse/tests/helpers/qunit-helpers";
+import { exists, query, queryAll } from "discourse/tests/helpers/qunit-helpers";
-import { render } from "@ember/test-helpers";
+import { click, render } from "@ember/test-helpers";
+import { NOTIFICATION_TYPES } from "discourse/tests/fixtures/concerns/notification-types";
 import { hbs } from "ember-cli-htmlbars";
+import pretender from "discourse/tests/helpers/create-pretender";
 
 module("Integration | Component | user-menu", function (hooks) {
   setupRenderingTest(hooks);
 
   const template = hbs`<UserMenu::Menu/>`;
 
+  test("default tab is all notifications", async function (assert) {
+    await render(template);
+    const activeTab = query(".top-tabs.tabs-list .btn.active");
+    assert.strictEqual(activeTab.id, "user-menu-button-all-notifications");
+    const notifications = queryAll("#quick-access-all-notifications ul li");
+    assert.strictEqual(notifications[0].className, "edited");
+    assert.strictEqual(notifications[1].className, "replied");
+    assert.strictEqual(notifications[2].className, "liked-consolidated");
+  });
+
   test("notifications panel has a11y attributes", async function (assert) {
     await render(template);
     const panel = query("#quick-access-all-notifications");
@@ -26,21 +38,27 @@ module("Integration | Component | user-menu", function (hooks) {
     assert.strictEqual(activeTab.getAttribute("aria-selected"), "true");
   });
 
+  test("inactive tab has a11y attributes that indicate it's inactive", async function (assert) {
+    await render(template);
+    const inactiveTab = query(".top-tabs.tabs-list .btn:not(.active)");
+    assert.strictEqual(inactiveTab.getAttribute("tabindex"), "-1");
+    assert.strictEqual(inactiveTab.getAttribute("aria-selected"), "false");
+  });
+
   test("the menu has a group of tabs at the top", async function (assert) {
     await render(template);
     const tabs = queryAll(".top-tabs.tabs-list .btn");
-    assert.strictEqual(tabs.length, 1);
+    assert.strictEqual(tabs.length, 4);
-    ["all-notifications"].forEach((tab, index) => {
+    ["all-notifications", "replies", "mentions", "likes"].forEach(
-      assert.strictEqual(tabs[index].id, `user-menu-button-${tab}`);
+      (tab, index) => {
-      assert.strictEqual(
+        assert.strictEqual(tabs[index].id, `user-menu-button-${tab}`);
-        tabs[index].getAttribute("data-tab-number"),
+        assert.strictEqual(tabs[index].dataset.tabNumber, index.toString());
-        index.toString()
+        assert.strictEqual(
-      );
+          tabs[index].getAttribute("aria-controls"),
-      assert.strictEqual(
+          `quick-access-${tab}`
-        tabs[index].getAttribute("aria-controls"),
+        );
-        `quick-access-${tab}`
+      }
-      );
+    );
-    });
   });
 
   test("the menu has a group of tabs at the bottom", async function (assert) {
@@ -49,7 +67,162 @@ module("Integration | Component | user-menu", function (hooks) {
     assert.strictEqual(tabs.length, 1);
     const preferencesTab = tabs[0];
     assert.ok(preferencesTab.href.endsWith("/u/eviltrout/preferences"));
-    assert.strictEqual(preferencesTab.getAttribute("data-tab-number"), "1");
+    assert.strictEqual(preferencesTab.dataset.tabNumber, "4");
     assert.strictEqual(preferencesTab.getAttribute("tabindex"), "-1");
   });
+
+  test("likes tab is hidden if current user's like notifications frequency is 'never'", async function (assert) {
+    this.currentUser.set("likes_notifications_disabled", true);
+    await render(template);
+    assert.ok(!exists("#user-menu-button-likes"));
+
+    const tabs = Array.from(queryAll(".tabs-list .btn")); // top and bottom tabs
+    assert.strictEqual(tabs.length, 4);
+
+    assert.deepEqual(
+      tabs.map((t) => t.dataset.tabNumber),
+      [...Array(4).keys()].map((n) => n.toString()),
+      "data-tab-number of the tabs has no gaps when the likes tab is hidden"
+    );
+  });
+
+  test("changing tabs", async function (assert) {
+    await render(template);
+    let queryParams;
+    pretender.get("/notifications", (request) => {
+      queryParams = request.queryParams;
+      let data;
+      if (queryParams.filter_by_types === "mentioned") {
+        data = [
+          {
+            id: 6,
+            user_id: 1,
+            notification_type: NOTIFICATION_TYPES.mentioned,
+            read: true,
+            high_priority: false,
+            created_at: "2021-11-25T19:31:13.241Z",
+            post_number: 6,
+            topic_id: 10,
+            fancy_title: "Greetings!",
+            slug: "greetings",
+            data: {
+              topic_title: "Greetings!",
+              original_post_id: 20,
+              original_post_type: 1,
+              original_username: "discobot",
+              revision_number: null,
+              display_username: "discobot",
+            },
+          },
+        ];
+      } else if (queryParams.filter_by_types === "liked,liked_consolidated") {
+        data = [
+          {
+            id: 60,
+            user_id: 1,
+            notification_type: NOTIFICATION_TYPES.liked,
+            read: true,
+            high_priority: false,
+            created_at: "2021-11-25T19:31:13.241Z",
+            post_number: 6,
+            topic_id: 10,
+            fancy_title: "Greetings!",
+            slug: "greetings",
+            data: {
+              topic_title: "Greetings!",
+              original_post_id: 20,
+              original_post_type: 1,
+              original_username: "discobot",
+              revision_number: null,
+              display_username: "discobot",
+            },
+          },
+          {
+            id: 63,
+            user_id: 1,
+            notification_type: NOTIFICATION_TYPES.liked,
+            read: true,
+            high_priority: false,
+            created_at: "2021-11-25T19:31:13.241Z",
+            post_number: 6,
+            topic_id: 10,
+            fancy_title: "Greetings!",
+            slug: "greetings",
+            data: {
+              topic_title: "Greetings!",
+              original_post_id: 20,
+              original_post_type: 1,
+              original_username: "discobot",
+              revision_number: null,
+              display_username: "discobot",
+            },
+          },
+          {
+            id: 20,
+            user_id: 1,
+            notification_type: NOTIFICATION_TYPES.liked_consolidated,
+            read: true,
+            high_priority: false,
+            created_at: "2021-11-25T19:31:13.241Z",
+            post_number: 6,
+            topic_id: 10,
+            fancy_title: "Greetings 123!",
+            slug: "greetings 123",
+            data: {
+              topic_title: "Greetings 123!",
+              original_post_id: 20,
+              original_post_type: 1,
+              original_username: "discobot",
+              revision_number: null,
+              display_username: "discobot",
+            },
+          },
+        ];
+      } else {
+        throw new Error(
+          `unexpected notification type ${queryParams.filter_by_types}`
+        );
+      }
+
+      return [
+        200,
+        { "Content-Type": "application/json" },
+        { notifications: data },
+      ];
+    });
+
+    await click("#user-menu-button-mentions");
+    assert.ok(exists("#quick-access-mentions.quick-access-panel"));
+    assert.strictEqual(
+      queryParams.filter_by_types,
+      "mentioned",
+      "request params has filter_by_types set to `mentioned`"
+    );
+    assert.strictEqual(queryParams.silent, "true");
+    let activeTabs = queryAll(".top-tabs .btn.active");
+    assert.strictEqual(activeTabs.length, 1);
+    assert.strictEqual(
+      activeTabs[0].id,
+      "user-menu-button-mentions",
+      "active tab is now the mentions tab"
+    );
+    assert.strictEqual(queryAll("#quick-access-mentions ul li").length, 1);
+
+    await click("#user-menu-button-likes");
+    assert.ok(exists("#quick-access-likes.quick-access-panel"));
+    assert.strictEqual(
+      queryParams.filter_by_types,
+      "liked,liked_consolidated",
+      "request params has filter_by_types set to `liked` and `liked_consolidated"
+    );
+    assert.strictEqual(queryParams.silent, "true");
+    activeTabs = queryAll(".top-tabs .btn.active");
+    assert.strictEqual(activeTabs.length, 1);
+    assert.strictEqual(
+      activeTabs[0].id,
+      "user-menu-button-likes",
+      "active tab is now the likes tab"
+    );
+    assert.strictEqual(queryAll("#quick-access-likes ul li").length, 3);
+  });
 });

app/assets/javascripts/discourse/tests/integration/components/user-menu/notifications-list-test.js

@@ -53,6 +53,11 @@ module(
       );
     });
 
+    test("doesn't request the full notifications list in silent mode", async function (assert) {
+      await render(template);
+      assert.strictEqual(queryParams.silent, undefined);
+    });
+
     test("displays a show all button that takes to the notifications page of the current user", async function (assert) {
       await render(template);
       const showAllBtn = query(".panel-body-bottom .btn.show-all");

app/controllers/notifications_controller.rb

@@ -18,11 +18,19 @@ class NotificationsController < ApplicationController
 
     guardian.ensure_can_see_notifications!(user)
 
+    if notification_types = params[:filter_by_types]&.split(",").presence
+      notification_types.map! do |type|
+        Notification.types[type.to_sym] || (
+          raise Discourse::InvalidParameters.new("invalid notification type: #{type}")
+        )
+      end
+    end
+
     if params[:recent].present?
       limit = (params[:limit] || 15).to_i
       limit = 50 if limit > 50
 
-      notifications = Notification.recent_report(current_user, limit)
+      notifications = Notification.recent_report(current_user, limit, notification_types)
       changed = false
 
       if notifications.present? && !(params.has_key?(:silent) || @readonly_mode)
@@ -31,11 +39,15 @@ class NotificationsController < ApplicationController
         changed = current_user.saw_notification_id(max_id)
       end
 
-      user.reload
+      if changed
-      user.publish_notifications_state if changed
+        current_user.reload
+        current_user.publish_notifications_state
+      end
 
-      render_json_dump(notifications: serialize_data(notifications, NotificationSerializer),
+      render_json_dump(
-                       seen_notification_id: current_user.seen_notification_id)
+        notifications: serialize_data(notifications, NotificationSerializer),
+        seen_notification_id: current_user.seen_notification_id
+      )
     else
       offset = params[:offset].to_i
 

app/models/notification.rb

@@ -205,7 +205,7 @@ class Notification < ActiveRecord::Base
     Post.find_by(topic_id: topic_id, post_number: post_number)
   end
 
-  def self.recent_report(user, count = nil)
+  def self.recent_report(user, count = nil, types = [])
     return unless user && user.user_option
 
     count ||= 10
@@ -214,6 +214,7 @@ class Notification < ActiveRecord::Base
       .recent(count)
       .includes(:topic)
 
+    notifications = notifications.where(notification_type: types) if types.present?
     if user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:never]
       [
         Notification.types[:liked],
@@ -228,17 +229,23 @@ class Notification < ActiveRecord::Base
     notifications = notifications.to_a
 
     if notifications.present?
-
+      builder = DB.build(<<~SQL)
-      ids = DB.query_single(<<~SQL, limit: count.to_i)
          SELECT n.id FROM notifications n
-         WHERE
+         /*where*/
-           n.high_priority = TRUE AND
-           n.user_id = #{user.id.to_i} AND
-           NOT read
         ORDER BY n.id ASC
-        LIMIT :limit
+        /*limit*/
       SQL
 
+      builder.where(<<~SQL, user_id: user.id)
+        n.high_priority = TRUE AND
+        n.user_id = :user_id AND
+        NOT read
+      SQL
+      builder.where("notification_type IN (:types)", types: types) if types.present?
+      builder.limit(count.to_i)
+
+      ids = builder.query_single
+
       if ids.length > 0
         notifications += user
           .notifications

app/models/user_option.rb

@@ -200,6 +200,10 @@ class UserOption < ActiveRecord::Base
       email_messages_level == UserOption.email_level_types[:never]
   end
 
+  def likes_notifications_disabled?
+    like_notification_frequency == UserOption.like_notification_frequency_type[:never]
+  end
+
   def self.user_tzinfo(user_id)
     timezone = UserOption.where(user_id: user_id).pluck(:timezone).first || 'UTC'
 

app/serializers/current_user_serializer.rb

@@ -75,6 +75,7 @@ class CurrentUserSerializer < BasicUserSerializer
              :status,
              :sidebar_category_ids,
              :sidebar_tag_names,
+             :likes_notifications_disabled,
              :redesigned_user_menu_enabled
 
   delegate :user_stat, to: :object, private: true
@@ -346,4 +347,8 @@ class CurrentUserSerializer < BasicUserSerializer
     end
     @redesigned_user_menu_enabled = object.redesigned_user_menu_enabled?
   end
+
+  def likes_notifications_disabled
+    object.user_option&.likes_notifications_disabled?
+  end
 end

spec/requests/notifications_controller_spec.rb

@@ -111,6 +111,75 @@ describe NotificationsController do
           expect(JSON.parse(response.body)['notifications'][0]['read']).to eq(false)
         end
 
+        context "when filter_by_types param is present" do
+          fab!(:liked1) do
+            Fabricate(
+              :notification,
+              user: user,
+              notification_type: Notification.types[:liked],
+              created_at: 2.minutes.ago
+            )
+          end
+          fab!(:liked2) do
+            Fabricate(
+              :notification,
+              user: user,
+              notification_type: Notification.types[:liked],
+              created_at: 10.minutes.ago
+            )
+          end
+          fab!(:replied) do
+            Fabricate(
+              :notification,
+              user: user,
+              notification_type: Notification.types[:replied],
+              created_at: 7.minutes.ago
+            )
+          end
+          fab!(:mentioned) do
+            Fabricate(
+              :notification,
+              user: user,
+              notification_type: Notification.types[:mentioned]
+            )
+          end
+
+          it "correctly filters notifications to the type(s) given" do
+            get "/notifications.json", params: { recent: true, filter_by_types: "liked,replied" }
+            expect(response.status).to eq(200)
+            expect(
+              response.parsed_body["notifications"].map { |n| n["id"] }
+            ).to eq([liked1.id, replied.id, liked2.id])
+
+            get "/notifications.json", params: { recent: true, filter_by_types: "replied" }
+            expect(response.status).to eq(200)
+            expect(
+              response.parsed_body["notifications"].map { |n| n["id"] }
+            ).to eq([replied.id])
+          end
+
+          it "doesn't include notifications from other users" do
+            Fabricate(
+              :notification,
+              user: Fabricate(:user),
+              notification_type: Notification.types[:liked]
+            )
+            get "/notifications.json", params: { recent: true, filter_by_types: "liked" }
+            expect(response.status).to eq(200)
+            expect(
+              response.parsed_body["notifications"].map { |n| n["id"] }
+            ).to eq([liked1.id, liked2.id])
+          end
+
+          it "limits the number of returned notifications according to the limit param" do
+            get "/notifications.json", params: { recent: true, filter_by_types: "liked", limit: 1 }
+            expect(response.status).to eq(200)
+            expect(
+              response.parsed_body["notifications"].map { |n| n["id"] }
+            ).to eq([liked1.id])
+          end
+        end
+
         context 'when username params is not valid' do
           it 'should raise the right error' do
             get "/notifications.json", params: { username: 'somedude' }

spec/serializers/current_user_serializer_spec.rb

@@ -298,4 +298,20 @@ RSpec.describe CurrentUserSerializer do
       )
     end
   end
+
+  describe "#likes_notifications_disabled" do
+    it "is true if the user disables likes notifications" do
+      user.user_option.update!(like_notification_frequency: UserOption.like_notification_frequency_type[:never])
+      expect(serializer.as_json[:likes_notifications_disabled]).to eq(true)
+    end
+
+    it "is false if the user doesn't disable likes notifications" do
+      user.user_option.update!(like_notification_frequency: UserOption.like_notification_frequency_type[:always])
+      expect(serializer.as_json[:likes_notifications_disabled]).to eq(false)
+      user.user_option.update!(like_notification_frequency: UserOption.like_notification_frequency_type[:first_time_and_daily])
+      expect(serializer.as_json[:likes_notifications_disabled]).to eq(false)
+      user.user_option.update!(like_notification_frequency: UserOption.like_notification_frequency_type[:first_time])
+      expect(serializer.as_json[:likes_notifications_disabled]).to eq(false)
+    end
+  end
 end