From af74cf5c7730a7a989fde36c3f534ae927aabd09 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Wed, 7 Jun 2023 10:06:57 +1000 Subject: [PATCH] FEATURE: new dismiss button for combined new and unread view (#21817) Display modal for combined new and unread view with options: - [x] Dismiss new topics - [x] Dismiss new posts - [ ] Stop tracking these topics so they stop appearing in my new list --- .../app/controllers/discovery/topics.js | 53 +++--- .../discourse/app/controllers/dismiss-new.js | 13 ++ .../discourse/app/controllers/tag-show.js | 17 +- .../discourse/app/mixins/dismiss-topics.js | 30 ++++ .../javascripts/discourse/app/models/topic.js | 12 ++ .../app/templates/modal/dismiss-new.hbs | 29 ++++ app/controllers/topics_controller.rb | 42 ++++- config/locales/client.en.yml | 5 + spec/requests/topics_controller_spec.rb | 162 ++++++++++++++++++ spec/system/dismiss_topics_spec.rb | 23 +++ .../system/page_objects/modals/dismiss_new.rb | 19 ++ 11 files changed, 374 insertions(+), 31 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/controllers/dismiss-new.js create mode 100644 app/assets/javascripts/discourse/app/mixins/dismiss-topics.js create mode 100644 app/assets/javascripts/discourse/app/templates/modal/dismiss-new.hbs create mode 100644 spec/system/dismiss_topics_spec.rb create mode 100644 spec/system/page_objects/modals/dismiss_new.rb diff --git a/app/assets/javascripts/discourse/app/controllers/discovery/topics.js b/app/assets/javascripts/discourse/app/controllers/discovery/topics.js index 2b2f5e1c71e..d5c13c8451d 100644 --- a/app/assets/javascripts/discourse/app/controllers/discovery/topics.js +++ b/app/assets/javascripts/discourse/app/controllers/discovery/topics.js @@ -1,5 +1,6 @@ import { alias, empty, equal, gt, not, readOnly } from "@ember/object/computed"; import BulkTopicSelection from "discourse/mixins/bulk-topic-selection"; +import DismissTopics from "discourse/mixins/dismiss-topics"; import DiscoveryController from "discourse/controllers/discovery"; import I18n from "I18n"; import Topic from "discourse/models/topic"; @@ -56,6 +57,32 @@ const controllerOpts = { return this._isFilterPage(filter, "new") && topicsLength > 0; }, + callResetNew(dismissPosts = false, dismissTopics = false, untrack = false) { + const tracked = + (this.router.currentRoute.queryParams["f"] || + this.router.currentRoute.queryParams["filter"]) === "tracked"; + + let topicIds = this.selected + ? this.selected.map((topic) => topic.id) + : null; + + Topic.resetNew(this.category, !this.noSubcategories, { + tracked, + topicIds, + dismissPosts, + dismissTopics, + untrack, + }).then((result) => { + if (result.topic_ids) { + this.topicTrackingState.removeTopics(result.topic_ids); + } + this.send( + "refresh", + tracked ? { skipResettingParams: ["filter", "f"] } : {} + ); + }); + }, + // Show newly inserted topics @action showInserted(event) { @@ -114,26 +141,6 @@ const controllerOpts = { } }); }, - - resetNew() { - const tracked = - (this.router.currentRoute.queryParams["f"] || - this.router.currentRoute.queryParams["filter"]) === "tracked"; - - let topicIds = this.selected - ? this.selected.map((topic) => topic.id) - : null; - - Topic.resetNew(this.category, !this.noSubcategories, { - tracked, - topicIds, - }).then(() => - this.send( - "refresh", - tracked ? { skipResettingParams: ["filter", "f"] } : {} - ) - ); - }, }, afterRefresh(filter, list, listModel = list) { @@ -213,4 +220,8 @@ const controllerOpts = { }, }; -export default DiscoveryController.extend(controllerOpts, BulkTopicSelection); +export default DiscoveryController.extend( + controllerOpts, + BulkTopicSelection, + DismissTopics +); diff --git a/app/assets/javascripts/discourse/app/controllers/dismiss-new.js b/app/assets/javascripts/discourse/app/controllers/dismiss-new.js new file mode 100644 index 00000000000..3b847e8bc19 --- /dev/null +++ b/app/assets/javascripts/discourse/app/controllers/dismiss-new.js @@ -0,0 +1,13 @@ +import Controller from "@ember/controller"; +import ModalFunctionality from "discourse/mixins/modal-functionality"; +import { action } from "@ember/object"; + +export default class DismissNewController extends Controller.extend( + ModalFunctionality +) { + @action + dismiss() { + this.dismissCallback(); + this.send("closeModal"); + } +} diff --git a/app/assets/javascripts/discourse/app/controllers/tag-show.js b/app/assets/javascripts/discourse/app/controllers/tag-show.js index 48a3964fec9..d1737593b3d 100644 --- a/app/assets/javascripts/discourse/app/controllers/tag-show.js +++ b/app/assets/javascripts/discourse/app/controllers/tag-show.js @@ -2,6 +2,7 @@ import DiscoverySortableController from "discourse/controllers/discovery-sortabl import { inject as controller } from "@ember/controller"; import discourseComputed, { observes } from "discourse-common/utils/decorators"; import BulkTopicSelection from "discourse/mixins/bulk-topic-selection"; +import DismissTopics from "discourse/mixins/dismiss-topics"; import FilterModeMixin from "discourse/mixins/filter-mode"; import I18n from "I18n"; import NavItem from "discourse/models/nav-item"; @@ -13,6 +14,7 @@ import { inject as service } from "@ember/service"; export default DiscoverySortableController.extend( BulkTopicSelection, + DismissTopics, FilterModeMixin, { application: controller(), @@ -91,8 +93,7 @@ export default DiscoverySortableController.extend( return this._isFilterPage(filter, "new") && topicsLength > 0; }, - @action - resetNew() { + callResetNew(dismissPosts = false, dismissTopics = false, untrack = false) { const tracked = (this.router.currentRoute.queryParams["f"] || this.router.currentRoute.queryParams["filter"]) === "tracked"; @@ -103,9 +104,15 @@ export default DiscoverySortableController.extend( tracked, tag: this.tag, topicIds, - }).then(() => - this.refresh(tracked ? { skipResettingParams: ["filter", "f"] } : {}) - ); + dismissPosts, + dismissTopics, + untrack, + }).then((result) => { + if (result.topic_ids) { + this.topicTrackingState.removeTopics(result.topic_ids); + } + this.refresh(tracked ? { skipResettingParams: ["filter", "f"] } : {}); + }); }, @action diff --git a/app/assets/javascripts/discourse/app/mixins/dismiss-topics.js b/app/assets/javascripts/discourse/app/mixins/dismiss-topics.js new file mode 100644 index 00000000000..00c5a7620a3 --- /dev/null +++ b/app/assets/javascripts/discourse/app/mixins/dismiss-topics.js @@ -0,0 +1,30 @@ +import Mixin from "@ember/object/mixin"; +import User from "discourse/models/user"; +import showModal from "discourse/lib/show-modal"; +import I18n from "I18n"; + +export default Mixin.create({ + actions: { + resetNew() { + const user = User.current(); + if (!user.new_new_view_enabled) { + return this.callResetNew(); + } + const controller = showModal("dismiss-new", { + model: { + dismissTopics: true, + dismissPosts: true, + }, + titleTranslated: I18n.t("topics.bulk.dismiss_new_modal.title"), + }); + + controller.set("dismissCallback", () => { + this.callResetNew( + controller.model.dismissPosts, + controller.model.dismissTopics, + controller.model.untrack + ); + }); + }, + }, +}); diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js index 9d4d22c4983..2c66fdb759b 100644 --- a/app/assets/javascripts/discourse/app/models/topic.js +++ b/app/assets/javascripts/discourse/app/models/topic.js @@ -851,6 +851,18 @@ Topic.reopenClass({ data.topic_ids = topicIds; } + if (opts.dismissPosts) { + data.dismiss_posts = opts.dismissPosts; + } + + if (opts.dismissTopics) { + data.dismiss_topics = opts.dismissTopics; + } + + if (opts.untrack) { + data.untrack = opts.untrack; + } + return ajax("/topics/reset-new", { type: "PUT", data }); }, diff --git a/app/assets/javascripts/discourse/app/templates/modal/dismiss-new.hbs b/app/assets/javascripts/discourse/app/templates/modal/dismiss-new.hbs new file mode 100644 index 00000000000..4d40ce221d8 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/modal/dismiss-new.hbs @@ -0,0 +1,29 @@ + +

+ + + +

+ + +
+ +
\ No newline at end of file diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index e8cfbea0998..314bcc7f939 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -1059,7 +1059,20 @@ class TopicsController < ApplicationController elsif params[:tag_id].present? Topic.joins(:tags).where(tags: { name: params[:tag_id] }) else - new_results = TopicQuery.new(current_user).new_results(limit: false) + new_results = + if current_user.new_new_view_enabled? + if (params[:dismiss_topics] && params[:dismiss_posts]) + TopicQuery.new(current_user).new_and_unread_results(limit: false) + elsif params[:dismiss_topics] + TopicQuery.new(current_user).new_results(limit: false) + elsif params[:dismiss_posts] + TopicQuery.new(current_user).unread_results(limit: false) + else + Topic.none + end + else + TopicQuery.new(current_user).new_results(limit: false) + end if params[:tracked].to_s == "true" TopicQuery.tracked_filter(new_results, current_user.id) else @@ -1077,11 +1090,30 @@ class TopicsController < ApplicationController topic_scope = topic_scope.where(id: topic_ids) end - dismissed_topic_ids = - TopicsBulkAction.new(current_user, topic_scope.pluck(:id), type: "dismiss_topics").perform! - TopicTrackingState.publish_dismiss_new(current_user.id, topic_ids: dismissed_topic_ids) + dismissed_topic_ids = [] + dismissed_post_topic_ids = [] + if !current_user.new_new_view_enabled? || params[:dismiss_topics] + dismissed_topic_ids = + TopicsBulkAction.new(current_user, topic_scope.pluck(:id), type: "dismiss_topics").perform! + TopicTrackingState.publish_dismiss_new(current_user.id, topic_ids: dismissed_topic_ids) + end - render body: nil + if params[:dismiss_posts] + if params[:untrack] + dismissed_post_topic_ids = + TopicsBulkAction.new( + current_user, + topic_scope.pluck(:id), + type: "change_notification_level", + notification_level_id: NotificationLevels.topic_levels[:regular], + ).perform! + else + dismissed_post_topic_ids = + TopicsBulkAction.new(current_user, topic_scope.pluck(:id), type: "dismiss_posts").perform! + end + end + + render_json_dump topic_ids: dismissed_topic_ids.concat(dismissed_post_topic_ids).uniq end def convert_topic diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 939e47ffe9e..2964e59e4d0 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2866,6 +2866,11 @@ en: dismiss_tooltip: "Dismiss just new posts or stop tracking topics" also_dismiss_topics: "Stop tracking these topics so they never show up as unread for me again" dismiss_new: "Dismiss New" + dismiss_new_modal: + title: "Dismiss new" + topics: "Dismiss new topics" + posts: "Dismiss new posts" + untrack: "Stop tracking these topics so they stop appearing in my new list" dismiss_new_with_selected: one: "Dismiss New (%{count})" other: "Dismiss New (%{count})" diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 0548fbf74ee..745cd75a448 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -4061,6 +4061,168 @@ RSpec.describe TopicsController do end end end + + describe "new and unread" do + fab!(:group) { Fabricate(:group) } + fab!(:new_topic) { Fabricate(:topic) } + fab!(:unread_topic) { Fabricate(:topic, highest_post_number: 3) } + fab!(:topic_user) do + Fabricate( + :topic_user, + topic: unread_topic, + user: user, + notification_level: NotificationLevels.topic_levels[:tracking], + last_read_post_number: 1, + ) + end + + before do + create_post(topic: unread_topic) + create_post(topic: unread_topic) + user.groups << group + SiteSetting.experimental_new_new_view_groups = group.id + sign_in(user) + end + + it "dismisses new topics" do + put "/topics/reset-new.json" + topics = TopicQuery.new(user).new_and_unread_results(limit: false) + expect(topics).to eq([unread_topic, new_topic]) + expect(response.status).to eq(200) + expect(response.parsed_body["topic_ids"]).to eq([]) + + put "/topics/reset-new.json", params: { dismiss_topics: true } + expect(response.status).to eq(200) + expect(response.parsed_body["topic_ids"]).to eq([new_topic.id]) + + topics = TopicQuery.new(user).new_and_unread_results(limit: false) + expect(topics).to eq([unread_topic]) + expect(DismissedTopicUser.where(user: user).count).to eq(1) + expect(DismissedTopicUser.where(user: user).first.topic_id).to eq(new_topic.id) + expect(topic_user.reload.notification_level).to eq( + NotificationLevels.topic_levels[:tracking], + ) + end + + it "dismisses unread topics" do + put "/topics/reset-new.json" + expect(response.status).to eq(200) + expect(response.parsed_body["topic_ids"]).to eq([]) + topics = TopicQuery.new(user).new_and_unread_results(limit: false) + expect(topics).to eq([unread_topic, new_topic]) + + put "/topics/reset-new.json", params: { dismiss_posts: true } + expect(response.status).to eq(200) + expect(response.parsed_body["topic_ids"]).to eq([unread_topic.id]) + + topics = TopicQuery.new(user).new_and_unread_results(limit: false) + expect(topics).to eq([new_topic]) + expect(DismissedTopicUser.count).to eq(0) + expect(topic_user.reload.notification_level).to eq( + NotificationLevels.topic_levels[:tracking], + ) + end + + it "untrack topics" do + expect(topic_user.notification_level).to eq(NotificationLevels.topic_levels[:tracking]) + put "/topics/reset-new.json", params: { dismiss_posts: true, untrack: true } + expect(response.status).to eq(200) + expect(response.parsed_body["topic_ids"]).to eq([unread_topic.id]) + + expect(topic_user.reload.notification_level).to eq( + NotificationLevels.topic_levels[:regular], + ) + end + + it "dismisses new topics, unread posts and untrack" do + put "/topics/reset-new.json", + params: { + dismiss_topics: true, + dismiss_posts: true, + untrack: true, + } + expect(response.status).to eq(200) + expect(response.parsed_body["topic_ids"]).to eq([new_topic.id, unread_topic.id]) + + topics = TopicQuery.new(user).new_and_unread_results(limit: false) + expect(topics).to be_empty + expect(DismissedTopicUser.where(user: user).count).to eq(1) + expect(DismissedTopicUser.where(user: user).first.topic_id).to eq(new_topic.id) + + expect(user.topic_users.map(&:notification_level).uniq).to eq( + [NotificationLevels.topic_levels[:regular]], + ) + end + + context "when category" do + fab!(:category) { Fabricate(:category) } + fab!(:new_topic_2) { Fabricate(:topic, category: category) } + fab!(:unread_topic_2) { Fabricate(:topic, category: category, highest_post_number: 3) } + fab!(:topic_user) do + Fabricate( + :topic_user, + topic: unread_topic_2, + user: user, + notification_level: NotificationLevels.topic_levels[:tracking], + last_read_post_number: 1, + ) + end + + it "dismisses new topics, unread posts and untrack for specific category" do + topics = TopicQuery.new(user).new_and_unread_results(limit: false) + expect(topics).to match_array([new_topic, new_topic_2, unread_topic, unread_topic_2]) + + put "/topics/reset-new.json", + params: { + dismiss_topics: true, + dismiss_posts: true, + untrack: true, + category_id: category.id, + } + expect(response.status).to eq(200) + expect(response.parsed_body["topic_ids"]).to eq([new_topic_2.id, unread_topic_2.id]) + + topics = TopicQuery.new(user).new_and_unread_results(limit: false) + expect(topics).to match_array([new_topic, unread_topic]) + end + end + + context "when tag" do + fab!(:tag) { Fabricate(:tag) } + fab!(:new_topic_2) { Fabricate(:topic) } + fab!(:unread_topic_2) { Fabricate(:topic, highest_post_number: 3) } + fab!(:topic_user) do + Fabricate( + :topic_user, + topic: unread_topic_2, + user: user, + notification_level: NotificationLevels.topic_levels[:tracking], + last_read_post_number: 1, + ) + end + fab!(:topic_tag) { Fabricate(:topic_tag, topic: new_topic_2, tag: tag) } + fab!(:topic_tag_2) { Fabricate(:topic_tag, topic: unread_topic_2, tag: tag) } + + it "dismisses new topics, unread posts and untrack for specific tag" do + topics = TopicQuery.new(user).new_and_unread_results(limit: false) + expect(topics).to match_array([new_topic, new_topic_2, unread_topic, unread_topic_2]) + + put "/topics/reset-new.json", + params: { + dismiss_topics: true, + dismiss_posts: true, + untrack: true, + tag_id: tag.name, + } + + expect(response.status).to eq(200) + expect(response.parsed_body["topic_ids"]).to eq([new_topic_2.id, unread_topic_2.id]) + + topics = TopicQuery.new(user).new_and_unread_results(limit: false) + expect(topics).to match_array([new_topic, unread_topic]) + end + end + end end describe "#feature_stats" do diff --git a/spec/system/dismiss_topics_spec.rb b/spec/system/dismiss_topics_spec.rb new file mode 100644 index 00000000000..c28c6bc75dc --- /dev/null +++ b/spec/system/dismiss_topics_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +describe "Filtering topics", type: :system, js: true do + fab!(:user) { Fabricate(:user) } + let(:topic_list) { PageObjects::Components::TopicList.new } + let(:dismiss_new_modal) { PageObjects::Modals::DismissNew.new } + fab!(:group) { Fabricate(:group).tap { |g| g.add(user) } } + fab!(:topic) { Fabricate(:topic) } + + before { SiteSetting.experimental_new_new_view_groups = group.id } + + it "displays confirmation modal with preselected options" do + sign_in(user) + + visit("/new") + + expect(topic_list).to have_topic(topic) + find(".dismiss-read").click + expect(dismiss_new_modal).to have_dismiss_topics_checked + expect(dismiss_new_modal).to have_dismiss_posts_checked + expect(dismiss_new_modal).to have_untrack_unchecked + end +end diff --git a/spec/system/page_objects/modals/dismiss_new.rb b/spec/system/page_objects/modals/dismiss_new.rb new file mode 100644 index 00000000000..52ae68142bd --- /dev/null +++ b/spec/system/page_objects/modals/dismiss_new.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module PageObjects + module Modals + class DismissNew < PageObjects::Modals::Base + def has_dismiss_topics_checked? + find(".dismiss-topics label").has_checked_field? + end + + def has_dismiss_posts_checked? + find(".dismiss-posts label").has_checked_field? + end + + def has_untrack_unchecked? + find(".untrack label").has_no_checked_field? + end + end + end +end