From f71e9aad60a63c5b98a6f9e20988874d01e97df4 Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Fri, 8 Mar 2024 15:14:46 -0700 Subject: [PATCH] FEATURE: Silence Close Notifications User Setting (#26072) This change creates a user setting that they can toggle if they don't want to receive unread notifications when someone closes a topic they have read and are watching/tracking it. --- .../app/controllers/preferences/tracking.js | 1 + .../javascripts/discourse/app/models/user.js | 1 + .../app/templates/preferences/tracking.hbs | 5 +++ app/models/post_timing.rb | 21 ++++++--- app/models/user_option.rb | 1 + app/serializers/user_option_serializer.rb | 3 +- app/services/topic_status_updater.rb | 15 +++++++ app/services/user_updater.rb | 1 + config/locales/client.en.yml | 1 + ...pics_unread_when_closed_to_user_options.rb | 7 +++ .../api/schemas/json/user_get_response.json | 6 ++- spec/system/page_objects/pages/topic.rb | 9 ++++ spec/system/topics_unread_when_closed_spec.rb | 44 +++++++++++++++++++ 13 files changed, 106 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20240307231053_add_topics_unread_when_closed_to_user_options.rb create mode 100644 spec/system/topics_unread_when_closed_spec.rb diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/tracking.js b/app/assets/javascripts/discourse/app/controllers/preferences/tracking.js index 13619807f1d..80ec9b54599 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/tracking.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/tracking.js @@ -167,6 +167,7 @@ export default class extends Controller { "tracked_category_ids", "watched_first_post_category_ids", "watched_precedence_over_muted", + "topics_unread_when_closed", ]; if (this.siteSettings.tagging_enabled) { diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index 323743f26b6..931c8b73bde 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -130,6 +130,7 @@ let userOptionFields = [ "sidebar_link_to_filtered_list", "sidebar_show_count_of_new_items", "watched_precedence_over_muted", + "topics_unread_when_closed", ]; export function addSaveableUserOptionField(fieldName) { diff --git a/app/assets/javascripts/discourse/app/templates/preferences/tracking.hbs b/app/assets/javascripts/discourse/app/templates/preferences/tracking.hbs index 735555bfebf..943ddddc306 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/tracking.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/tracking.hbs @@ -49,6 +49,11 @@ }} /> + + diff --git a/app/models/post_timing.rb b/app/models/post_timing.rb index 1a3863672f5..44d683db50d 100644 --- a/app/models/post_timing.rb +++ b/app/models/post_timing.rb @@ -7,26 +7,33 @@ class PostTiming < ActiveRecord::Base validates_presence_of :post_number validates_presence_of :msecs - def self.pretend_read(topic_id, actual_read_post_number, pretend_read_post_number) + def self.pretend_read(topic_id, actual_read_post_number, pretend_read_post_number, user_ids = nil) # This is done in SQL cause the logic is quite tricky and we want to do this in one db hit # - DB.exec( - "INSERT INTO post_timings(topic_id, user_id, post_number, msecs) + user_ids_condition = user_ids.present? ? "AND user_id = ANY(ARRAY[:user_ids]::int[])" : "" + sql_query = <<-SQL + INSERT INTO post_timings(topic_id, user_id, post_number, msecs) SELECT :topic_id, user_id, :pretend_read_post_number, 1 FROM post_timings pt WHERE topic_id = :topic_id AND - post_number = :actual_read_post_number AND - NOT EXISTS ( + post_number = :actual_read_post_number + #{user_ids_condition} + AND NOT EXISTS ( SELECT 1 FROM post_timings pt1 WHERE pt1.topic_id = pt.topic_id AND pt1.post_number = :pretend_read_post_number AND pt1.user_id = pt.user_id ) - ", + SQL + + params = { pretend_read_post_number: pretend_read_post_number, topic_id: topic_id, actual_read_post_number: actual_read_post_number, - ) + } + params[:user_ids] = user_ids if user_ids.present? + + DB.exec(sql_query, params) TopicUser.ensure_consistency!(topic_id) end diff --git a/app/models/user_option.rb b/app/models/user_option.rb index e20a556459b..70c24af6a38 100644 --- a/app/models/user_option.rb +++ b/app/models/user_option.rb @@ -292,6 +292,7 @@ end # sidebar_show_count_of_new_items :boolean default(FALSE), not null # watched_precedence_over_muted :boolean # chat_separate_sidebar_mode :integer default(0), not null +# topics_unread_when_closed :boolean default(TRUE), not null # # Indexes # diff --git a/app/serializers/user_option_serializer.rb b/app/serializers/user_option_serializer.rb index 562a06ba8c3..97804335402 100644 --- a/app/serializers/user_option_serializer.rb +++ b/app/serializers/user_option_serializer.rb @@ -39,7 +39,8 @@ class UserOptionSerializer < ApplicationSerializer :seen_popups, :sidebar_link_to_filtered_list, :sidebar_show_count_of_new_items, - :watched_precedence_over_muted + :watched_precedence_over_muted, + :topics_unread_when_closed def auto_track_topics_after_msecs object.auto_track_topics_after_msecs || SiteSetting.default_other_auto_track_topics_after_msecs diff --git a/app/services/topic_status_updater.rb b/app/services/topic_status_updater.rb index 7008c3223a5..04998dfe40c 100644 --- a/app/services/topic_status_updater.rb +++ b/app/services/topic_status_updater.rb @@ -90,6 +90,21 @@ TopicStatusUpdater = # actually read the topic PostTiming.pretend_read(topic.id, old_highest_read, topic.highest_post_number) end + + if status.closed? && status.enabled? + sql_query = <<-SQL + SELECT DISTINCT post_timings.user_id + FROM post_timings + JOIN user_options ON user_options.user_id = post_timings.user_id + WHERE post_timings.topic_id = :topic_id + AND user_options.topics_unread_when_closed = 'f' + SQL + user_ids = DB.query_single(sql_query, topic_id: topic.id) + + if user_ids.present? + PostTiming.pretend_read(topic.id, old_highest_read, topic.highest_post_number, user_ids) + end + end end def message_for(status) diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index 4b6e4bc6781..2c7acd0bd9f 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -52,6 +52,7 @@ class UserUpdater sidebar_link_to_filtered_list sidebar_show_count_of_new_items watched_precedence_over_muted + topics_unread_when_closed ] NOTIFICATION_SCHEDULE_ATTRS = -> do diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6d823a16fe6..5e1da9a15eb 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1774,6 +1774,7 @@ en: after_10_minutes: "after 10 minutes" notification_level_when_replying: "When I post in a topic, set that topic to" + topics_unread_when_closed: "Consider topics unread when they are closed" invited: title: "Invites" diff --git a/db/migrate/20240307231053_add_topics_unread_when_closed_to_user_options.rb b/db/migrate/20240307231053_add_topics_unread_when_closed_to_user_options.rb new file mode 100644 index 00000000000..f2f36ce1edf --- /dev/null +++ b/db/migrate/20240307231053_add_topics_unread_when_closed_to_user_options.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddTopicsUnreadWhenClosedToUserOptions < ActiveRecord::Migration[7.0] + def change + add_column :user_options, :topics_unread_when_closed, :boolean, default: true, null: false + end +end diff --git a/spec/requests/api/schemas/json/user_get_response.json b/spec/requests/api/schemas/json/user_get_response.json index 24f7063b391..0f19c6d1b56 100644 --- a/spec/requests/api/schemas/json/user_get_response.json +++ b/spec/requests/api/schemas/json/user_get_response.json @@ -794,6 +794,9 @@ }, "seen_popups": { "type": ["array", "null"] + }, + "topics_unread_when_closed": { + "type": "boolean" } }, "required": [ @@ -828,7 +831,8 @@ "text_size_seq", "title_count_mode", "timezone", - "skip_new_user_tips" + "skip_new_user_tips", + "topics_unread_when_closed" ] } }, diff --git a/spec/system/page_objects/pages/topic.rb b/spec/system/page_objects/pages/topic.rb index 044e0f69dfa..eb0654e7000 100644 --- a/spec/system/page_objects/pages/topic.rb +++ b/spec/system/page_objects/pages/topic.rb @@ -214,11 +214,20 @@ module PageObjects find(".topic-notifications-button .select-kit-header").click end + def click_admin_menu_button + find("#topic-footer-buttons .topic-admin-menu-button").click + end + def watch_topic click_notifications_button find('li[data-name="watching"]').click end + def close_topic + click_admin_menu_button + find(".topic-admin-popup-menu ul.topic-admin-menu-topic li.topic-admin-close").click + end + def has_read_post?(post) post_by_number(post).has_css?(".read-state.read", visible: :all, wait: 3) end diff --git a/spec/system/topics_unread_when_closed_spec.rb b/spec/system/topics_unread_when_closed_spec.rb new file mode 100644 index 00000000000..8b180c78d98 --- /dev/null +++ b/spec/system/topics_unread_when_closed_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +describe "Topics unread when closed", type: :system do + fab!(:topics) { Fabricate.times(10, :post).map(&:topic) } + let(:topic_list) { PageObjects::Components::TopicList.new } + let(:topic_page) { PageObjects::Pages::Topic.new } + + context "when closing a topic" do + fab!(:admin) + fab!(:user) + + it "close notifications do not appear when disabled" do + user.user_option.update!(topics_unread_when_closed: false) + sign_in(user) + topic = topics.third + topic_page.visit_topic(topic) + topic_page.watch_topic + expect(topic_page).to have_read_post(1) + + # Close the topic as an admin + TopicStatusUpdater.new(topic, admin).update!("closed", true) + + # Check that the user did not receive a new post notification badge + visit("/latest") + expect(topic_list).to have_no_unread_badge(topics.third) + end + + it "close notifications appear when enabled (the default)" do + user.user_option.update!(topics_unread_when_closed: true) + sign_in(user) + topic = topics.third + topic_page.visit_topic(topic) + topic_page.watch_topic + expect(topic_page).to have_read_post(1) + + # Close the topic as an admin + TopicStatusUpdater.new(topic, admin).update!("closed", true) + + # Check that the user did receive a new post notification badge + visit("/latest") + expect(topic_list).to have_unread_badge(topics.third) + end + end +end