From 52c1d7337ec5863b65a557a7f4d02af9bd0b7300 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Thu, 23 Apr 2020 14:57:35 +1000 Subject: [PATCH] FEATURE: don't display new/unread notification for muted topics (#9482) * FEATURE: don't display new/unread notification for muted topics Currently, even if user mute topic, when a new reply to that topic arrives, the user will get "See 1 new or updated topic" message. After clicking on that link, nothing is visible (because the topic is muted) To solve that problem, we will send background message to all users who recently muted that topic that update is coming and they can ignore the next message about that topic. --- .../discourse/models/topic-tracking-state.js | 35 ++++++++++++++++ app/models/topic_tracking_state.rb | 17 ++++++++ lib/post_jobs_enqueuer.rb | 5 ++- spec/models/topic_tracking_state_spec.rb | 40 +++++++++++++++++++ .../models/topic-tracking-state-test.js | 30 +++++++++++++- 5 files changed, 125 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/models/topic-tracking-state.js b/app/assets/javascripts/discourse/models/topic-tracking-state.js index 7debe3f4ce7..bccb7747ebf 100644 --- a/app/assets/javascripts/discourse/models/topic-tracking-state.js +++ b/app/assets/javascripts/discourse/models/topic-tracking-state.js @@ -53,6 +53,17 @@ const TopicTrackingState = EmberObject.extend({ const tracker = this; const process = data => { + if (data.message_type === "muted") { + tracker.trackMutedTopic(data.topic_id); + return; + } + + tracker.pruneOldMutedTopics(); + + if (tracker.isMutedTopic(data.topic_id)) { + return; + } + if (data.message_type === "delete") { tracker.removeTopic(data.topic_id); tracker.incrementMessageCount(); @@ -132,6 +143,30 @@ const TopicTrackingState = EmberObject.extend({ }); }, + mutedTopics() { + return (this.currentUser && this.currentUser.muted_topics) || []; + }, + + trackMutedTopic(topicId) { + let mutedTopics = this.mutedTopics().concat({ + topicId: topicId, + createdAt: Date.now() + }); + this.currentUser && this.currentUser.set("muted_topics", mutedTopics); + }, + + pruneOldMutedTopics() { + const now = Date.now(); + let mutedTopics = this.mutedTopics().filter( + mutedTopic => now - mutedTopic.createdAt < 60000 + ); + this.currentUser && this.currentUser.set("muted_topics", mutedTopics); + }, + + isMutedTopic(topicId) { + return !!this.mutedTopics().findBy("topicId", topicId); + }, + updateSeen(topicId, highestSeen) { if (!topicId || !highestSeen) { return; diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb index 49ffeb97512..91838da99bb 100644 --- a/app/models/topic_tracking_state.rb +++ b/app/models/topic_tracking_state.rb @@ -12,6 +12,7 @@ class TopicTrackingState CHANNEL = "/user-tracking" UNREAD_MESSAGE_TYPE = "unread".freeze LATEST_MESSAGE_TYPE = "latest".freeze + MUTED_MESSAGE_TYPE = "muted".freeze attr_accessor :user_id, :topic_id, @@ -71,6 +72,22 @@ class TopicTrackingState "/unread/#{user_id}" end + def self.publish_muted(post) + user_ids = post.topic.topic_users + .where(notification_level: NotificationLevels.all[:muted]) + .joins(:user) + .where("users.last_seen_at > ?", 7.days.ago) + .order("users.last_seen_at DESC") + .limit(100) + .pluck(:user_id) + return if user_ids.blank? + message = { + topic_id: post.topic_id, + message_type: MUTED_MESSAGE_TYPE, + } + MessageBus.publish("/latest", message.as_json, user_ids: user_ids) + end + def self.publish_unread(post) return unless post.topic.regular? # TODO at high scale we are going to have to defer this, diff --git a/lib/post_jobs_enqueuer.rb b/lib/post_jobs_enqueuer.rb index 6f95899bb42..d539a4d6402 100644 --- a/lib/post_jobs_enqueuer.rb +++ b/lib/post_jobs_enqueuer.rb @@ -57,7 +57,10 @@ class PostJobsEnqueuer end def after_post_create - TopicTrackingState.publish_unread(@post) if @post.post_number > 1 + if @post.post_number > 1 + TopicTrackingState.publish_muted(@post) + TopicTrackingState.publish_unread(@post) + end TopicTrackingState.publish_latest(@topic, @post.whisper?) Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes, diff --git a/spec/models/topic_tracking_state_spec.rb b/spec/models/topic_tracking_state_spec.rb index a68239512d3..25bc127ba63 100644 --- a/spec/models/topic_tracking_state_spec.rb +++ b/spec/models/topic_tracking_state_spec.rb @@ -72,6 +72,46 @@ describe TopicTrackingState do end end + describe '#publish_muted' do + let(:user) do + Fabricate(:user, last_seen_at: Date.today) + end + let(:post) do + create_post(user: user) + end + + it 'can correctly publish muted' do + TopicUser.find_by(topic: post.topic, user: post.user).update(notification_level: 0) + messages = MessageBus.track_publish("/latest") do + TopicTrackingState.publish_muted(post) + end + + muted_message = messages.find { |message| message.data["message_type"] == "muted" } + + expect(muted_message.data["topic_id"]).to eq(topic.id) + expect(muted_message.data["message_type"]).to eq(described_class::MUTED_MESSAGE_TYPE) + end + + it 'should not publish any message when notification level is not muted' do + messages = MessageBus.track_publish("/latest") do + TopicTrackingState.publish_muted(post) + end + muted_messages = messages.select { |message| message.data["message_type"] == "muted" } + + expect(muted_messages).to eq([]) + end + + it 'should not publish any message when the user was not seen in the last 7 days' do + TopicUser.find_by(topic: post.topic, user: post.user).update(notification_level: 0) + post.user.update(last_seen_at: 8.days.ago) + messages = MessageBus.track_publish("/latest") do + TopicTrackingState.publish_muted(post) + end + muted_messages = messages.select { |message| message.data["message_type"] == "muted" } + expect(muted_messages).to eq([]) + end + end + describe '#publish_private_message' do fab!(:admin) { Fabricate(:admin) } diff --git a/test/javascripts/models/topic-tracking-state-test.js b/test/javascripts/models/topic-tracking-state-test.js index 0d958b8503d..709928c9329 100644 --- a/test/javascripts/models/topic-tracking-state-test.js +++ b/test/javascripts/models/topic-tracking-state-test.js @@ -2,8 +2,17 @@ import TopicTrackingState from "discourse/models/topic-tracking-state"; import createStore from "helpers/create-store"; import Category from "discourse/models/category"; import { NotificationLevels } from "discourse/lib/notification-levels"; +import User from "discourse/models/user"; -QUnit.module("model:topic-tracking-state"); +QUnit.module("model:topic-tracking-state", { + beforeEach() { + this.clock = sinon.useFakeTimers(new Date(2012, 11, 31, 12, 0).getTime()); + }, + + afterEach() { + this.clock.restore(); + } +}); QUnit.test("sync", function(assert) { const state = TopicTrackingState.create(); @@ -168,3 +177,22 @@ QUnit.test("countNew", assert => { assert.equal(state.countNew(2), 2); assert.equal(state.countNew(3), 1); }); + +QUnit.test("mute topic", function(assert) { + let currentUser = User.create({ + username: "chuck", + muted_category_ids: [] + }); + + const state = TopicTrackingState.create({ currentUser }); + + state.trackMutedTopic(1); + assert.equal(currentUser.muted_topics[0].topicId, 1); + + state.pruneOldMutedTopics(); + assert.equal(state.isMutedTopic(1), true); + + this.clock.tick(60000); + state.pruneOldMutedTopics(); + assert.equal(state.isMutedTopic(1), false); +});