DEV: Wire up the dimiss button in the new notifications menu (#17745)

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

@@ -4,4 +4,8 @@ export default class UserMenuLikesNotificationsList extends UserMenuNotification
   get filterByTypes() {
     return ["liked", "liked_consolidated"];
   }
+
+  dismissWarningModal() {
+    return null;
+  }
 }

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

@@ -4,4 +4,8 @@ export default class UserMenuMentionsNotificationsList extends UserMenuNotificat
   get filterByTypes() {
     return ["mentioned"];
   }
+
+  dismissWarningModal() {
+    return null;
+  }
 }

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

@@ -1,6 +1,9 @@
 import UserMenuItemsList from "discourse/components/user-menu/items-list";
 import I18n from "I18n";
 import { action } from "@ember/object";
+import { ajax } from "discourse/lib/ajax";
+import { postRNWebviewMessage } from "discourse/lib/utilities";
+import showModal from "discourse/lib/show-modal";
 
 export default class UserMenuNotificationsList extends UserMenuItemsList {
   get filterByTypes() {
@@ -60,13 +63,34 @@ 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;
+    if (this.currentUser.unread_high_priority_notifications > 0) {
+      const modalController = showModal("dismiss-notification-confirmation");
+      modalController.set(
+        "count",
+        this.currentUser.unread_high_priority_notifications
+      );
+      return modalController;
+    }
   }
 
   @action
   dismissButtonClick() {
-    // TODO
+    const opts = { type: "PUT" };
+    const dismissTypes = this.filterByTypes;
+    if (dismissTypes?.length > 0) {
+      opts.data = { dismiss_types: dismissTypes.join(",") };
+    }
+    const modalController = this.dismissWarningModal();
+    const modalCallback = () => {
+      ajax("/notifications/mark-read", opts).then(() => {
+        this.refreshList();
+        postRNWebviewMessage("markRead", "1");
+      });
+    };
+    if (modalController) {
+      modalController.set("dismissNotifications", modalCallback);
+    } else {
+      modalCallback();
+    }
   }
 }

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

@@ -4,4 +4,8 @@ export default class UserMenuRepliesNotificationsList extends UserMenuNotificati
   get filterByTypes() {
     return ["replied"];
   }
+
+  dismissWarningModal() {
+    return null;
+  }
 }

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

@@ -0,0 +1,97 @@
+import { click, visit } from "@ember/test-helpers";
+import {
+  acceptance,
+  query,
+  updateCurrentUser,
+} from "discourse/tests/helpers/qunit-helpers";
+import { test } from "qunit";
+import { cloneJSON } from "discourse-common/lib/object";
+import TopicFixtures from "discourse/tests/fixtures/topic";
+import I18n from "I18n";
+
+acceptance("User menu", function (needs) {
+  needs.user({ redesigned_user_menu_enabled: true });
+  let requestHeaders = {};
+
+  needs.pretender((server, helper) => {
+    server.get("/t/1234.json", (request) => {
+      const json = cloneJSON(TopicFixtures["/t/130.json"]);
+      json.id = 1234;
+      json.post_stream.posts.forEach((post) => {
+        post.topic_id = 1234;
+      });
+      requestHeaders = request.requestHeaders;
+      return helper.response(json);
+    });
+  });
+
+  needs.hooks.afterEach(() => {
+    requestHeaders = {};
+  });
+
+  test("clicking on an unread notification", async function (assert) {
+    await visit("/");
+    await click(".d-header-icons .current-user");
+    await click(".user-menu ul li.replied a");
+
+    assert.strictEqual(
+      requestHeaders["Discourse-Clear-Notifications"],
+      123, // id is from the fixtures in fixtures/notification-fixtures.js
+      "the Discourse-Clear-Notifications request header is set to the notification id in the next ajax request"
+    );
+  });
+});
+
+acceptance("User menu - Dismiss button", function (needs) {
+  needs.user({
+    redesigned_user_menu_enabled: true,
+    unread_high_priority_notifications: 10,
+  });
+
+  let markRead = false;
+
+  needs.pretender((server, helper) => {
+    server.put("/notifications/mark-read", () => {
+      markRead = true;
+      return helper.response({ success: true });
+    });
+  });
+
+  needs.hooks.afterEach(() => {
+    markRead = false;
+  });
+
+  test("shows confirmation modal for the all-notifications list", async function (assert) {
+    await visit("/");
+    await click(".d-header-icons .current-user");
+
+    await click(".user-menu .notifications-dismiss");
+    assert.strictEqual(
+      query(".dismiss-notification-confirmation").textContent.trim(),
+      I18n.t("notifications.dismiss_confirmation.body", { count: 10 }),
+      "confirmation modal is shown when there are unread high pri notifications"
+    );
+    assert.notOk(markRead, "mark-read request isn't sent");
+
+    await click(".modal-footer .btn-default"); // click cancel on the dismiss modal
+
+    updateCurrentUser({ unread_high_priority_notifications: 0 });
+    await click(".user-menu .notifications-dismiss");
+    assert.ok(
+      markRead,
+      "mark-read request is sent without a confirmation modal when there are no unread high pri notifications"
+    );
+  });
+
+  test("doesn't show confirmation modal for the likes notifications panel/list", async function (assert) {
+    await visit("/");
+    await click(".d-header-icons .current-user");
+
+    await click("#user-menu-button-likes");
+    await click(".user-menu .notifications-dismiss");
+    assert.ok(
+      markRead,
+      "mark-read request is sent without a confirmation modal"
+    );
+  });
+});

app/assets/javascripts/discourse/tests/fixtures/notification-fixtures.js

@@ -23,7 +23,7 @@ export default {
         id: 123,
         notification_type: NOTIFICATION_TYPES.replied,
         read: false,
-        post_number: 2,
+        post_number: 1,
         topic_id: 1234,
         slug: "a-slug",
         data: { topic_title: "some title", display_username: "velesin" },

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

@@ -1,7 +1,7 @@
 import { module, test } from "qunit";
 import { setupRenderingTest } from "discourse/tests/helpers/component-test";
 import { exists, query } from "discourse/tests/helpers/qunit-helpers";
-import { render } from "@ember/test-helpers";
+import { click, render } from "@ember/test-helpers";
 import { cloneJSON } from "discourse-common/lib/object";
 import NotificationFixtures from "discourse/tests/fixtures/notification-fixtures";
 import { hbs } from "ember-cli-htmlbars";
@@ -19,20 +19,26 @@ module(
 
     let notificationsData = getNotificationsData();
     let queryParams = null;
+    let markRead = false;
+    let notificationsFetches = 0;
     hooks.beforeEach(() => {
       pretender.get("/notifications", (request) => {
         queryParams = request.queryParams;
+        notificationsFetches++;
         return response({ notifications: notificationsData });
       });
 
-      pretender.put("/notifications/mark-read", () =>
-        response({ success: true })
-      );
+      pretender.put("/notifications/mark-read", () => {
+        markRead = true;
+        return response({ success: true });
+      });
     });
 
     hooks.afterEach(() => {
       notificationsData = getNotificationsData();
       queryParams = null;
+      markRead = false;
+      notificationsFetches = 0;
     });
 
     const template = hbs`<UserMenu::NotificationsList/>`;
@@ -100,5 +106,25 @@ module(
       await render(template);
       assert.ok(!exists(".panel-body-bottom .btn.notifications-dismiss"));
     });
+
+    test("dismiss button makes a request to the server and then refreshes the notifications list", async function (assert) {
+      await render(template);
+      notificationsData = getNotificationsData();
+      notificationsData.forEach((notification) => {
+        notification.read = true;
+      });
+      assert.strictEqual(notificationsFetches, 1);
+      await click(".panel-body-bottom .btn.notifications-dismiss");
+      assert.ok(markRead, "request to the server is made");
+      assert.strictEqual(
+        notificationsFetches,
+        2,
+        "notifications list is refreshed"
+      );
+      assert.ok(
+        !exists(".panel-body-bottom .btn.notifications-dismiss"),
+        "dismiss button is not shown"
+      );
+    });
   }
 );

app/controllers/notifications_controller.rb

@@ -90,7 +90,19 @@ class NotificationsController < ApplicationController
     if params[:id]
       Notification.read(current_user, [params[:id].to_i])
     else
-      Notification.where(user_id: current_user.id).includes(:topic).where(read: false).update_all(read: true)
+      if types = params[:dismiss_types]&.split(",").presence
+        invalid = []
+        types.map! do |type|
+          type_id = Notification.types[type.to_sym]
+          invalid << type if !type_id
+          type_id
+        end
+        if invalid.size > 0
+          raise Discourse::InvalidParameters.new("invalid notification types: #{invalid.inspect}")
+        end
+      end
+
+      Notification.read_types(current_user, types)
       current_user.saw_notification_id(Notification.recent_report(current_user, 1).max.try(:id))
     end
 

app/models/notification.rb

@@ -148,6 +148,12 @@ class Notification < ActiveRecord::Base
       .update_all(read: true)
   end
 
+  def self.read_types(user, types = nil)
+    query = Notification.where(user_id: user.id, read: false)
+    query = query.where(notification_type: types) if types
+    query.update_all(read: true)
+  end
+
   def self.interesting_after(min_date)
     result = where("created_at > ?", min_date)
       .includes(:topic)

spec/requests/notifications_controller_spec.rb

@@ -288,6 +288,81 @@ RSpec.describe NotificationsController do
           delete_notification(403, :to)
         end
       end
+
+      describe '#mark_read' do
+        context "when targeting a notification by id" do
+          it 'can mark a notification as read' do
+            expect {
+              put "/notifications/mark-read.json", params: { id: notification.id }
+              expect(response.status).to eq(200)
+              notification.reload
+            }.to change { notification.read }.from(false).to(true)
+          end
+
+          it "doesn't mark a notification of another user as read" do
+            notification.update!(user_id: Fabricate(:user).id, read: false)
+            expect {
+              put "/notifications/mark-read.json", params: { id: notification.id }
+              expect(response.status).to eq(200)
+              notification.reload
+            }.not_to change { notification.read }
+          end
+        end
+
+        context "when targeting notifications by type" do
+          it "can mark notifications as read" do
+            replied1 = notification
+            replied1.update!(notification_type: Notification.types[:replied])
+            mentioned = Fabricate(
+              :notification,
+              user: user,
+              notification_type: Notification.types[:mentioned],
+              read: false
+            )
+            liked = Fabricate(
+              :notification,
+              user: user,
+              notification_type: Notification.types[:liked],
+              read: false
+            )
+            replied2 = Fabricate(
+              :notification,
+              user: user,
+              notification_type: Notification.types[:replied],
+              read: true
+            )
+            put "/notifications/mark-read.json", params: {
+              dismiss_types: "replied,mentioned"
+            }
+            expect(response.status).to eq(200)
+            expect(replied1.reload.read).to eq(true)
+            expect(replied2.reload.read).to eq(true)
+            expect(mentioned.reload.read).to eq(true)
+
+            expect(liked.reload.read).to eq(false)
+          end
+
+          it "doesn't mark notifications of another user as read" do
+            mentioned1 = Fabricate(
+              :notification,
+              user: user,
+              notification_type: Notification.types[:mentioned],
+              read: false
+            )
+            mentioned2 = Fabricate(
+              :notification,
+              user: Fabricate(:user),
+              notification_type: Notification.types[:mentioned],
+              read: false
+            )
+            put "/notifications/mark-read.json", params: {
+              dismiss_types: "mentioned"
+            }
+            expect(mentioned1.reload.read).to eq(true)
+            expect(mentioned2.reload.read).to eq(false)
+          end
+        end
+      end
     end
 
     context 'as admin' do