From aa3a9b6fea9190eadd4e0152f1b8e26cee3de0be Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Wed, 14 Dec 2022 10:22:26 +1100 Subject: [PATCH] FEATURE: Differentiate notification type for directly vs indirectly watched topic (#19433) When user is watching category or tag (watching or watching first post) notifications are moved to other tab. To achieve that and distinguish between post create to directly watched topics and indirectly watched topics, new notification type called `watching_category_or_tag` was introduced. --- .../addon/lib/icon-library.js | 1 + .../app/components/user-menu/menu.js | 11 ++--------- .../components/user-menu/menu-test.js | 7 +++---- app/models/notification.rb | 1 + app/services/notification_emailer.rb | 5 ++++- app/services/post_alerter.rb | 19 ++++++++++++++----- config/locales/client.en.yml | 2 ++ .../api/schemas/json/site_response.json | 4 ++++ spec/services/post_alerter_spec.rb | 12 ++++++------ 9 files changed, 37 insertions(+), 25 deletions(-) diff --git a/app/assets/javascripts/discourse-common/addon/lib/icon-library.js b/app/assets/javascripts/discourse-common/addon/lib/icon-library.js index 8882d7c8d6..34d6b6912c 100644 --- a/app/assets/javascripts/discourse-common/addon/lib/icon-library.js +++ b/app/assets/javascripts/discourse-common/addon/lib/icon-library.js @@ -28,6 +28,7 @@ const REPLACEMENTS = { "notification.quoted": "quote-right", "notification.replied": "reply", "notification.posted": "discourse-bell-exclamation", + "notification.watching_category_or_tag": "discourse-bell-exclamation", "notification.edited": "pencil-alt", "notification.bookmark_reminder": "discourse-bookmark-clock", "notification.liked": "heart", diff --git a/app/assets/javascripts/discourse/app/components/user-menu/menu.js b/app/assets/javascripts/discourse/app/components/user-menu/menu.js index fc963747a3..cd79a6c7b0 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/menu.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/menu.js @@ -49,19 +49,12 @@ const CORE_TOP_TABS = [ this.getUnreadCountForType("mentioned") + this.getUnreadCountForType("posted") + this.getUnreadCountForType("quoted") + - this.getUnreadCountForType("replied") + - this.getUnreadCountForType("watching_first_post") + this.getUnreadCountForType("replied") ); } get notificationTypes() { - return [ - "mentioned", - "posted", - "quoted", - "replied", - "watching_first_post", - ]; + return ["mentioned", "posted", "quoted", "replied"]; } get linkWhenActive() { diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js index 562784895f..36026d2511 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js @@ -222,8 +222,7 @@ module("Integration | Component | user-menu", function (hooks) { }, ]; } else if ( - queryParams.filter_by_types === - "mentioned,posted,quoted,replied,watching_first_post" + queryParams.filter_by_types === "mentioned,posted,quoted,replied" ) { data = [ { @@ -281,8 +280,8 @@ module("Integration | Component | user-menu", function (hooks) { assert.ok(exists("#quick-access-replies.quick-access-panel")); assert.strictEqual( queryParams.filter_by_types, - "mentioned,posted,quoted,replied,watching_first_post", - "request params has filter_by_types set to `mentioned`, `posted`, `quoted`, `replied` and `watching_first_post`" + "mentioned,posted,quoted,replied", + "request params has filter_by_types set to `mentioned`, `posted`, `quoted` and `replied`" ); assert.strictEqual(queryParams.silent, "true"); activeTabs = queryAll(".top-tabs .btn.active"); diff --git a/app/models/notification.rb b/app/models/notification.rb index b6e95aa256..350de63720 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -138,6 +138,7 @@ class Notification < ActiveRecord::Base chat_quoted: 33, assigned: 34, question_answer_user_commented: 35, # Used by https://github.com/discourse/discourse-question-answer + watching_category_or_tag: 36, ) end diff --git a/app/services/notification_emailer.rb b/app/services/notification_emailer.rb index 05e7a8b435..b768ae5b68 100644 --- a/app/services/notification_emailer.rb +++ b/app/services/notification_emailer.rb @@ -22,6 +22,10 @@ class NotificationEmailer enqueue :user_posted end + def watching_category_or_tag + enqueue :user_posted + end + def quoted enqueue :user_quoted end @@ -134,7 +138,6 @@ class NotificationEmailer email_user = EmailUser.new(notification, no_delay: no_delay) email_method = Notification.types[notification.notification_type] - email_user.public_send(email_method) if email_user.respond_to? email_method end diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index c4c930b5ca..9b9545ea0c 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -178,7 +178,8 @@ class PostAlerter notified += notify_pm_users(post, reply_to_user, quoted_users, notified) elsif notify_about_reply?(post) # posts - notified += notify_post_users(post, notified, new_record: new_record) + notified += notify_post_users(post, notified, new_record: new_record, include_category_watchers: false, include_tag_watchers: false) + notified += notify_post_users(post, notified, new_record: new_record, include_topic_watchers: false, notification_type: :watching_category_or_tag) end end @@ -400,6 +401,7 @@ class PostAlerter Notification.types[:replied], Notification.types[:posted], Notification.types[:private_message], + Notification.types[:watching_category_or_tag], ] def create_notification(user, type, post, opts = {}) @@ -779,7 +781,7 @@ class PostAlerter emails_to_skip_send.uniq end - def notify_post_users(post, notified, group_ids: nil, include_topic_watchers: true, include_category_watchers: true, include_tag_watchers: true, new_record: false) + def notify_post_users(post, notified, group_ids: nil, include_topic_watchers: true, include_category_watchers: true, include_tag_watchers: true, new_record: false, notification_type: nil) return [] unless post.topic warn_if_not_sidekiq @@ -871,10 +873,17 @@ class PostAlerter ) each_user_in_batches(notify) do |user| - notification_type = !new_record && already_seen_user_ids.include?(user.id) ? Notification.types[:edited] : Notification.types[:posted] + calculated_type = + if !new_record && already_seen_user_ids.include?(user.id) + Notification.types[:edited] + elsif notification_type + Notification.types[notification_type] + else + Notification.types[:posted] + end opts = {} - opts[:display_username] = post.last_editor.username if notification_type == Notification.types[:edited] - create_notification(user, notification_type, post, opts) + opts[:display_username] = post.last_editor.username if calculated_type == Notification.types[:edited] + create_notification(user, calculated_type, post, opts) end notify diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 5b2362e0be..e0a97877dc 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2507,6 +2507,7 @@ en: bookmark_reminder: "%{username} %{description}" replied: "%{username} %{description}" posted: "%{username} %{description}" + watching_category_or_tag: "%{username} %{description}" edited: "%{username} %{description}" liked: "%{username} %{description}" liked_2: "%{username}, %{username2} %{description}" @@ -2579,6 +2580,7 @@ en: invited_to_private_message: "invited to private message" invitee_accepted: "invite accepted" posted: "new post" + watching_category_or_tag: "new post" moved_post: "post moved" linked: "linked" bookmark_reminder: "bookmark reminder" diff --git a/spec/requests/api/schemas/json/site_response.json b/spec/requests/api/schemas/json/site_response.json index 16bc845a5d..ebe85af1b0 100644 --- a/spec/requests/api/schemas/json/site_response.json +++ b/spec/requests/api/schemas/json/site_response.json @@ -35,6 +35,9 @@ "posted": { "type": "integer" }, + "watching_category_or_tag": { + "type": "integer" + }, "moved_post": { "type": "integer" }, @@ -124,6 +127,7 @@ "invited_to_private_message", "invitee_accepted", "posted", + "watching_category_or_tag", "moved_post", "linked", "granted_badge", diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index 4fb69e2a06..ca484b6efc 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -1395,10 +1395,10 @@ RSpec.describe PostAlerter do whispered_post = Fabricate(:post, user: Fabricate(:admin), topic: topic, post_type: Post.types[:whisper]) expect { PostAlerter.post_created(whispered_post) - }.to add_notification(admin, :posted) + }.to add_notification(admin, :watching_category_or_tag) expect { PostAlerter.post_created(whispered_post) - }.not_to add_notification(user, :posted) + }.not_to add_notification(user, :watching_category_or_tag) end it "notifies a staged user about a private post, but only if the user has access" do @@ -1424,8 +1424,8 @@ RSpec.describe PostAlerter do expect { PostAlerter.post_created(post) - }.to add_notification(staged_member, :posted) - .and not_add_notification(staged_non_member, :posted) + }.to add_notification(staged_member, :watching_category_or_tag) + .and not_add_notification(staged_non_member, :watching_category_or_tag) end it "does not update existing unread notification" do @@ -1622,7 +1622,7 @@ RSpec.describe PostAlerter do end context "with :watching notification level" do - include_examples "tag user with notification level", :watching, :posted + include_examples "tag user with notification level", :watching, :watching_category_or_tag end context "with :watching_first_post notification level" do @@ -2089,7 +2089,7 @@ RSpec.describe PostAlerter do notification = Notification.last expect(notification.user).to eq(user) - expect(notification.notification_type).to eq(Notification.types[:posted]) + expect(notification.notification_type).to eq(Notification.types[:watching_category_or_tag]) expect(notification.topic).to eq(post.topic) expect(notification.post_number).to eq(1) end