diff --git a/app/assets/javascripts/discourse/app/components/user-menu/likes-notifications-list.js b/app/assets/javascripts/discourse/app/components/user-menu/likes-notifications-list.js index 9deb8a7dd3c..bf1b654adc9 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/likes-notifications-list.js +++ b/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; + } } diff --git a/app/assets/javascripts/discourse/app/components/user-menu/mentions-notifications-list.js b/app/assets/javascripts/discourse/app/components/user-menu/mentions-notifications-list.js index a6974ab267d..8b8a2bad668 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/mentions-notifications-list.js +++ b/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; + } } diff --git a/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js b/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js index bb698a5b05f..4e5c59670dd 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js +++ b/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(); + } } } diff --git a/app/assets/javascripts/discourse/app/components/user-menu/replies-notifications-list.js b/app/assets/javascripts/discourse/app/components/user-menu/replies-notifications-list.js index 3403dac0d64..f747b81e919 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/replies-notifications-list.js +++ b/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; + } } diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js new file mode 100644 index 00000000000..1a26181412b --- /dev/null +++ b/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" + ); + }); +}); diff --git a/app/assets/javascripts/discourse/tests/fixtures/notification-fixtures.js b/app/assets/javascripts/discourse/tests/fixtures/notification-fixtures.js index b912424c3e2..a3ff25c5d88 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/notification-fixtures.js +++ b/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" }, diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/notifications-list-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/notifications-list-test.js index 082201aec06..cf1baf30b83 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/user-menu/notifications-list-test.js +++ b/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``; @@ -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" + ); + }); } ); diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 21a1a662216..0f8eb4126db 100644 --- a/app/controllers/notifications_controller.rb +++ b/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 diff --git a/app/models/notification.rb b/app/models/notification.rb index 022745b4472..1989a07d474 100644 --- a/app/models/notification.rb +++ b/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) diff --git a/spec/requests/notifications_controller_spec.rb b/spec/requests/notifications_controller_spec.rb index 4b87537ba61..85f5f2ddb2e 100644 --- a/spec/requests/notifications_controller_spec.rb +++ b/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