From 354ec6694aac30191c9438139bf9f873c8be479f Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Tue, 9 Feb 2021 10:39:30 +1100 Subject: [PATCH] FEATURE: Ability to dismiss new topics in a specific tag (#11968) * FEATURE: Ability to dismiss new topics in a specific tag Follow up of https://github.com/discourse/discourse/pull/11927 Using the same mechanism to disable new topics in a tag. * FIX: respect when category and tag is selected --- .../discourse/app/controllers/tag-show.js | 24 +++++++++ .../app/models/topic-tracking-state.js | 31 ++++++----- .../javascripts/discourse/app/models/topic.js | 6 ++- .../discourse/app/templates/tags/show.hbs | 9 ++++ .../unit/models/topic-tracking-state-test.js | 52 +++++++++++++++++++ app/controllers/topics_controller.rb | 15 +++++- app/models/topic_tracking_state.rb | 6 ++- app/services/dismiss_topics.rb | 7 ++- spec/requests/topics_controller_spec.rb | 32 +++++++++++- spec/services/dismiss_topics_spec.rb | 6 +++ 10 files changed, 168 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/tag-show.js b/app/assets/javascripts/discourse/app/controllers/tag-show.js index 9fbc7ffc396..bb2997e20e2 100644 --- a/app/assets/javascripts/discourse/app/controllers/tag-show.js +++ b/app/assets/javascripts/discourse/app/controllers/tag-show.js @@ -4,6 +4,7 @@ import BulkTopicSelection from "discourse/mixins/bulk-topic-selection"; import FilterModeMixin from "discourse/mixins/filter-mode"; import I18n from "I18n"; import NavItem from "discourse/models/nav-item"; +import Topic from "discourse/models/topic"; import { alias } from "@ember/object/computed"; import bootbox from "bootbox"; import { queryParams } from "discourse/controllers/discovery-sortable"; @@ -109,11 +110,34 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, { return this.isFilterPage(filter, "unread") && topicsLength > 0; }, + @discourseComputed("list.filter", "list.topics.length") + showResetNew(filter, topicsLength) { + return this.isFilterPage(filter, "new") && topicsLength > 0; + }, + actions: { dismissReadPosts() { showModal("dismiss-read", { title: "topics.bulk.dismiss_read" }); }, + resetNew() { + const tracked = + (this.router.currentRoute.queryParams["f"] || + this.router.currentRoute.queryParams["filter"]) === "tracked"; + + Topic.resetNew( + this.category, + !this.noSubcategories, + tracked, + this.tag + ).then(() => + this.send( + "refresh", + tracked ? { skipResettingParams: ["filter", "f"] } : {} + ) + ); + }, + changeSort(order) { if (order === this.order) { this.toggleProperty("ascending"); diff --git a/app/assets/javascripts/discourse/app/models/topic-tracking-state.js b/app/assets/javascripts/discourse/app/models/topic-tracking-state.js index 71a2bb709b5..5909ed63ef6 100644 --- a/app/assets/javascripts/discourse/app/models/topic-tracking-state.js +++ b/app/assets/javascripts/discourse/app/models/topic-tracking-state.js @@ -106,19 +106,7 @@ const TopicTrackingState = EmberObject.extend({ } if (data.message_type === "dismiss_new") { - Object.keys(tracker.states).forEach((k) => { - const topic = tracker.states[k]; - if ( - !data.payload.category_id || - topic.category_id === parseInt(data.payload.category_id, 0) - ) { - tracker.states[k] = Object.assign({}, topic, { - is_seen: true, - }); - } - }); - tracker.notifyPropertyChange("states"); - tracker.incrementMessageCount(); + tracker.dismissNewTopic(data); } if (["new_topic", "unread", "read"].includes(data.message_type)) { @@ -193,6 +181,23 @@ const TopicTrackingState = EmberObject.extend({ this.currentUser && this.currentUser.set(key, topics); }, + dismissNewTopic(data) { + Object.keys(this.states).forEach((k) => { + const topic = this.states[k]; + if ( + (!data.payload.category_id || + topic.category_id === parseInt(data.payload.category_id, 10)) && + (!data.payload.tag_id || topic.tags.includes(data.payload.tag_id)) + ) { + this.states[k] = Object.assign({}, topic, { + is_seen: true, + }); + } + }); + this.notifyPropertyChange("states"); + this.incrementMessageCount(); + }, + pruneOldMutedAndUnmutedTopics() { const now = Date.now(); let mutedTopics = this.mutedTopics().filter( diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js index 95b147922a0..572a4168b11 100644 --- a/app/assets/javascripts/discourse/app/models/topic.js +++ b/app/assets/javascripts/discourse/app/models/topic.js @@ -766,12 +766,16 @@ Topic.reopenClass({ }); }, - resetNew(category, include_subcategories, tracked = false) { + resetNew(category, include_subcategories, tracked = false, tag = false) { const data = { tracked }; if (category) { data.category_id = category.id; data.include_subcategories = include_subcategories; } + if (tag) { + data.tag_id = tag.id; + } + return ajax("/topics/reset-new", { type: "PUT", data }); }, diff --git a/app/assets/javascripts/discourse/app/templates/tags/show.hbs b/app/assets/javascripts/discourse/app/templates/tags/show.hbs index 6c03574cc33..dbb03bbb563 100644 --- a/app/assets/javascripts/discourse/app/templates/tags/show.hbs +++ b/app/assets/javascripts/discourse/app/templates/tags/show.hbs @@ -74,6 +74,15 @@ label="topics.bulk.dismiss_button"}} {{/if}} + {{#if showResetNew}} + {{d-button + class="btn-default dismiss-read" + action=(action "resetNew") + id="dismiss-new" + icon="check" + label="topics.bulk.dismiss_new"}} + {{/if}} + {{#footer-message education=footerEducation message=footerMessage}} {{#link-to "tags"}} {{i18n "topic.browse_all_tags"}}{{/link-to}} {{i18n "or"}} {{#link-to "discovery.latest"}}{{i18n "topic.view_latest_topics"}}{{/link-to}}. {{/footer-message}} diff --git a/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js b/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js index 44cceda109e..8e5a0e70e9d 100644 --- a/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js +++ b/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js @@ -328,6 +328,58 @@ module("Unit | Model | topic-tracking-state", function (hooks) { assert.equal(state.countNew(4), 0); }); + test("dismissNew", function (assert) { + let currentUser = User.create({ + username: "chuck", + }); + + const state = TopicTrackingState.create({ currentUser }); + + state.states["t112"] = { + last_read_post_number: null, + id: 112, + notification_level: NotificationLevels.TRACKING, + category_id: 1, + is_seen: false, + tags: ["foo"], + }; + + state.dismissNewTopic({ + message_type: "dismiss_new", + topic_id: 112, + payload: { category_id: 2 }, + }); + assert.equal(state.states["t112"].is_seen, false); + state.dismissNewTopic({ + message_type: "dismiss_new", + topic_id: 112, + payload: { category_id: 1 }, + }); + assert.equal(state.states["t112"].is_seen, true); + + state.states["t112"].is_seen = false; + state.dismissNewTopic({ + message_type: "dismiss_new", + topic_id: 112, + payload: { tag_id: "bar" }, + }); + assert.equal(state.states["t112"].is_seen, false); + state.dismissNewTopic({ + message_type: "dismiss_new", + topic_id: 112, + payload: { tag_id: "foo" }, + }); + assert.equal(state.states["t112"].is_seen, true); + + state.states["t112"].is_seen = false; + state.dismissNewTopic({ + message_type: "dismiss_new", + topic_id: 112, + payload: {}, + }); + assert.equal(state.states["t112"].is_seen, true); + }); + test("mute and unmute topic", function (assert) { let currentUser = User.create({ username: "chuck", diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 8c47cf47cfe..aa3c6c57bb7 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -898,10 +898,21 @@ class TopicsController < ApplicationController if params[:include_subcategories] == 'true' category_ids = category_ids.concat(Category.where(parent_category_id: params[:category_id]).pluck(:id)) end - DismissTopics.new(current_user, Topic.where(category_id: category_ids)).perform! + + topic_scope = + if params[:tag_id] + Topic.where(category_id: category_ids).joins(:tags).where(tags: { name: params[:tag_id] }) + else + Topic.where(category_id: category_ids) + end + + DismissTopics.new(current_user, topic_scope).perform! category_ids.each do |category_id| - TopicTrackingState.publish_dismiss_new(current_user.id, category_id) + TopicTrackingState.publish_dismiss_new(current_user.id, category_id: category_id, tag_id: params[:tag_id]) end + elsif params[:tag_id].present? + DismissTopics.new(current_user, Topic.joins(:tags).where(tags: { name: params[:tag_id] })).perform! + TopicTrackingState.publish_dismiss_new(current_user.id, tag_id: params[:tag_id]) else if params[:tracked].to_s == "true" topics = TopicQuery.tracked_filter(TopicQuery.new(current_user).new_results, current_user.id) diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb index ad7c5a7b4d4..78e4c7073d8 100644 --- a/app/models/topic_tracking_state.rb +++ b/app/models/topic_tracking_state.rb @@ -215,8 +215,10 @@ class TopicTrackingState MessageBus.publish(self.unread_channel_key(user_id), message.as_json, user_ids: [user_id]) end - def self.publish_dismiss_new(user_id, category_id = nil) - payload = category_id ? { category_id: category_id } : {} + def self.publish_dismiss_new(user_id, category_id: nil, tag_id: nil) + payload = {} + payload[:category_id] = category_id if category_id + payload[:tag_id] = tag_id if tag_id message = { message_type: "dismiss_new", payload: payload diff --git a/app/services/dismiss_topics.rb b/app/services/dismiss_topics.rb index b8ce5bc86d5..2b439944631 100644 --- a/app/services/dismiss_topics.rb +++ b/app/services/dismiss_topics.rb @@ -13,7 +13,12 @@ class DismissTopics private def rows - @rows ||= @topics_scope.where("created_at >= ?", since_date).order(created_at: :desc).limit(SiteSetting.max_new_topics).map do |topic| + @rows ||= @topics_scope + .joins("LEFT JOIN topic_users ON topic_users.topic_id = topics.id AND topic_users.user_id = #{@user.id}") + .where("topics.created_at >= ?", since_date) + .where("topic_users.id IS NULL") + .order("topics.created_at DESC") + .limit(SiteSetting.max_new_topics).map do |topic| { topic_id: topic.id, user_id: @user.id, diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 2eaf88bc74a..71a7b9ba9e9 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -2873,7 +2873,7 @@ RSpec.describe TopicsController do it 'dismisses topics for main category' do sign_in(user) - TopicTrackingState.expects(:publish_dismiss_new).with(user.id, category.id.to_s) + TopicTrackingState.expects(:publish_dismiss_new).with(user.id, category_id: category.id.to_s, tag_id: nil) put "/topics/reset-new.json?category_id=#{category.id}" @@ -2887,6 +2887,36 @@ RSpec.describe TopicsController do expect(DismissedTopicUser.where(user_id: user.id).pluck(:topic_id).sort).to eq([category_topic.id, subcategory_topic.id].sort) end end + + context 'tag' do + fab!(:tag) { Fabricate(:tag) } + fab!(:tag_topic) { Fabricate(:topic) } + fab!(:topic_tag) { Fabricate(:topic_tag, topic: tag_topic, tag: tag) } + fab!(:topic) { Fabricate(:topic) } + + it 'dismisses topics for tag' do + sign_in(user) + TopicTrackingState.expects(:publish_dismiss_new).with(user.id, tag_id: tag.name) + put "/topics/reset-new.json?tag_id=#{tag.name}" + expect(DismissedTopicUser.where(user_id: user.id).pluck(:topic_id)).to eq([tag_topic.id]) + end + end + + context 'tag and category' do + fab!(:tag) { Fabricate(:tag) } + fab!(:tag_topic) { Fabricate(:topic) } + fab!(:category) { Fabricate(:category) } + fab!(:tag_and_category_topic) { Fabricate(:topic, category: category) } + fab!(:topic_tag) { Fabricate(:topic_tag, topic: tag_topic, tag: tag) } + fab!(:topic_tag2) { Fabricate(:topic_tag, topic: tag_and_category_topic, tag: tag) } + + it 'dismisses topics for tag' do + sign_in(user) + TopicTrackingState.expects(:publish_dismiss_new).with(user.id, tag_id: tag.name, category_id: category.id.to_s) + put "/topics/reset-new.json?tag_id=#{tag.name}&category_id=#{category.id}" + expect(DismissedTopicUser.where(user_id: user.id).pluck(:topic_id)).to eq([tag_and_category_topic.id]) + end + end end describe '#feature_stats' do diff --git a/spec/services/dismiss_topics_spec.rb b/spec/services/dismiss_topics_spec.rb index ad3741163f4..925a573051c 100644 --- a/spec/services/dismiss_topics_spec.rb +++ b/spec/services/dismiss_topics_spec.rb @@ -28,6 +28,12 @@ describe DismissTopics do expect(dismissed_topic_user.created_at).not_to be_nil end + it 'respects seen topics' do + Fabricate(:topic_user, user: user, topic: topic1) + Fabricate(:topic_user, user: user, topic: topic2) + expect { described_class.new(user, Topic.all).perform! }.to change { DismissedTopicUser.count }.by(0) + end + it 'respects new_topic_duration_minutes' do user.user_option.update!(new_topic_duration_minutes: 70)