From 973a0028b450acc4c40bad794bb53a9f5df0f336 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 12 Apr 2024 13:10:14 +1000 Subject: [PATCH] FEATURE: Bulk topic tagging allowing restricted operations on sole categories (#26602) The bulk actions menu for topics has multiple options to work with tags on topics (append, replace, remove). Our tagging system along with categories allows for some complicated tag restrictions to be applied via tag groups. This was a problem for the topic bulk actions because you couldn't append restricted tags to topics. This commit allows restricted tags to be used in bulk tagging actions as long as all selected topics are for a sole category. The category information will be shown in the modal, and the category ID is used for the tag search. --- .../components/modal/bulk-topic-actions.gjs | 103 +++++++++---- .../discourse/app/lib/bulk-select-helper.js | 4 + .../common/modal/modal-overrides.scss | 10 ++ config/locales/client.en.yml | 3 + .../page_objects/components/category_badge.rb | 17 +++ .../page_objects/components/topic_list.rb | 4 +- .../components/topic_list_header.rb | 7 + .../page_objects/modals/topic_bulk_actions.rb | 36 +++++ spec/system/topic_bulk_select_spec.rb | 135 +++++++++++++++--- 9 files changed, 266 insertions(+), 53 deletions(-) create mode 100644 spec/system/page_objects/components/category_badge.rb create mode 100644 spec/system/page_objects/modals/topic_bulk_actions.rb diff --git a/app/assets/javascripts/discourse/app/components/modal/bulk-topic-actions.gjs b/app/assets/javascripts/discourse/app/components/modal/bulk-topic-actions.gjs index 0e4d2987408..3aedaf99115 100644 --- a/app/assets/javascripts/discourse/app/components/modal/bulk-topic-actions.gjs +++ b/app/assets/javascripts/discourse/app/components/modal/bulk-topic-actions.gjs @@ -2,14 +2,16 @@ import Component from "@glimmer/component"; import { tracked } from "@glimmer/tracking"; import { Input } from "@ember/component"; import { on } from "@ember/modifier"; -import { action, computed } from "@ember/object"; +import { action } from "@ember/object"; import { service } from "@ember/service"; import { Promise } from "rsvp"; import ConditionalLoadingSection from "discourse/components/conditional-loading-section"; import DButton from "discourse/components/d-button"; import DModal from "discourse/components/d-modal"; import RadioButton from "discourse/components/radio-button"; +import { categoryBadgeHTML } from "discourse/helpers/category-link"; import { topicLevels } from "discourse/lib/notification-levels"; +import Category from "discourse/models/category"; import Topic from "discourse/models/topic"; import autoFocus from "discourse/modifiers/auto-focus"; import htmlSafe from "discourse-common/helpers/html-safe"; @@ -39,9 +41,9 @@ export default class BulkTopicActions extends Component { constructor() { super(...arguments); - if (this.args.model.initialAction === "set-component") { - if (this.args.model.initialActionLabel in _customActions) { - _customActions[this.args.model.initialActionLabel]({ + if (this.model.initialAction === "set-component") { + if (this.model.initialActionLabel in _customActions) { + _customActions[this.model.initialActionLabel]({ setComponent: this.setComponent.bind(this), }); } @@ -49,7 +51,7 @@ export default class BulkTopicActions extends Component { } async perform(operation) { - if (this.args.model.bulkSelectHelper.selected.length > 20) { + if (this.model.bulkSelectHelper.selected.length > 20) { this.showProgress = true; } @@ -78,7 +80,7 @@ export default class BulkTopicActions extends Component { } _processChunks(operation) { - const allTopics = this.args.model.bulkSelectHelper.selected; + const allTopics = this.model.bulkSelectHelper.selected; const topicChunks = this._generateTopicChunks(allTopics); const topicIds = []; const options = {}; @@ -134,7 +136,7 @@ export default class BulkTopicActions extends Component { @action performAction() { this.loading = true; - switch (this.args.model.action) { + switch (this.model.action) { case "close": this.forEachPerformed({ type: "close" }, (t) => t.set("closed", true)); break; @@ -192,7 +194,7 @@ export default class BulkTopicActions extends Component { if (this.customAction) { this.customAction(this.performAndRefresh.bind(this)); } else { - _customActions[this.args.model.initialActionLabel](this); + _customActions[this.model.initialActionLabel](this); } } } @@ -218,9 +220,9 @@ export default class BulkTopicActions extends Component { if (topics) { topics.forEach(cb); - this.args.model.refreshClosure?.(); + this.model.refreshClosure?.(); this.args.closeModal(); - this.args.model.bulkSelectHelper.toggleBulkSelect(); + this.model.bulkSelectHelper.toggleBulkSelect(); this.showToast(); } } @@ -229,33 +231,30 @@ export default class BulkTopicActions extends Component { async performAndRefresh(operation) { await this.perform(operation); - this.args.model.refreshClosure?.(); - this.args.closeModal(); - this.args.model.bulkSelectHelper.toggleBulkSelect(); - this.showToast(); + this.model.refreshClosure?.().then(() => { + this.args.closeModal(); + this.model.bulkSelectHelper.toggleBulkSelect(); + this.showToast(); + }); } - @computed("action") get isTagAction() { return ( - this.args.model.action === "append-tags" || - this.args.model.action === "replace-tags" + this.model.action === "append-tags" || + this.model.action === "replace-tags" ); } - @computed("action") get isNotificationAction() { - return this.args.model.action === "update-notifications"; + return this.model.action === "update-notifications"; } - @computed("action") get isCategoryAction() { - return this.args.model.action === "update-category"; + return this.model.action === "update-category"; } - @computed("action") get isCloseAction() { - return this.args.model.action === "close"; + return this.model.action === "close"; } @action @@ -264,6 +263,10 @@ export default class BulkTopicActions extends Component { this.closeNote = event.target.value; } + get model() { + return this.args.model; + } + get notificationLevels() { return topicLevels.map((level) => ({ id: level.id.toString(), @@ -272,6 +275,32 @@ export default class BulkTopicActions extends Component { })); } + get soleCategoryId() { + if (this.model.bulkSelectHelper.selectedCategoryIds.length === 1) { + return this.model.bulkSelectHelper.selectedCategoryIds[0]; + } + + return null; + } + + get soleCategory() { + if (!this.soleCategoryId) { + return null; + } + + return Category.findById(this.soleCategoryId); + } + + get soleCategoryBadgeHTML() { + return categoryBadgeHTML(this.soleCategory, { + allowUncategorized: true, + }); + } + + get showSoleCategoryTip() { + return this.soleCategory && this.isTagAction; + } + @action onCategoryChange(categoryId) { this.categoryId = categoryId; @@ -289,13 +318,25 @@ export default class BulkTopicActions extends Component { @isLoading={{this.loading}} @title={{i18n "topics.bulk.performing"}} > -
- {{htmlSafe - (i18n - "topics.bulk.selected" - count=@model.bulkSelectHelper.selected.length - ) - }} +
+ + {{#if this.showSoleCategoryTip}} + {{htmlSafe + (i18n + "topics.bulk.selected_sole_category" + count=@model.bulkSelectHelper.selected.length + ) + }} + {{htmlSafe this.soleCategoryBadgeHTML}} + {{else}} + {{htmlSafe + (i18n + "topics.bulk.selected" + count=@model.bulkSelectHelper.selected.length + ) + }} + + {{/if}}
{{#if this.isCategoryAction}} @@ -330,7 +371,7 @@ export default class BulkTopicActions extends Component { {{#if this.isTagAction}}

{{/if}} diff --git a/app/assets/javascripts/discourse/app/lib/bulk-select-helper.js b/app/assets/javascripts/discourse/app/lib/bulk-select-helper.js index 75558fdec5b..743ad117dc7 100644 --- a/app/assets/javascripts/discourse/app/lib/bulk-select-helper.js +++ b/app/assets/javascripts/discourse/app/lib/bulk-select-helper.js @@ -29,6 +29,10 @@ export default class BulkSelectHelper { this.selected.concat(topics); } + get selectedCategoryIds() { + return this.selected.mapBy("category_id").uniq(); + } + @bind toggleBulkSelect() { this.bulkSelectEnabled = !this.bulkSelectEnabled; diff --git a/app/assets/stylesheets/common/modal/modal-overrides.scss b/app/assets/stylesheets/common/modal/modal-overrides.scss index db219f56d7e..9865f5bf79e 100644 --- a/app/assets/stylesheets/common/modal/modal-overrides.scss +++ b/app/assets/stylesheets/common/modal/modal-overrides.scss @@ -218,7 +218,17 @@ } } +.topic-bulk-actions-modal { + &__selection-info { + margin-bottom: 0.5em; + } +} + .d-modal.topic-bulk-actions-modal { + .d-modal__title-text { + font-size: var(--font-up-2); + } + .d-modal { &__container { display: flex; diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 58385fe6eab..0bbb98fea3a 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3000,6 +3000,9 @@ en: selected: one: "You have selected %{count} topic." other: "You have selected %{count} topics." + selected_sole_category: + one: "You have selected %{count} topic from category:" + other: "You have selected %{count} topics from category:" selected_count: one: "%{count} selected" other: "%{count} selected" diff --git a/spec/system/page_objects/components/category_badge.rb b/spec/system/page_objects/components/category_badge.rb new file mode 100644 index 00000000000..929ad39f9ec --- /dev/null +++ b/spec/system/page_objects/components/category_badge.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module PageObjects + module Components + class CategoryBadge < PageObjects::Components::Base + SELECTOR = ".badge-category__wrapper" + + def find_for_category(category) + find(category_selector(category)) + end + + def category_selector(category) + "#{SELECTOR} .badge-category[data-category-id='#{category.id}']" + end + end + end +end diff --git a/spec/system/page_objects/components/topic_list.rb b/spec/system/page_objects/components/topic_list.rb index 9be201d9b7f..7b5e4626c09 100644 --- a/spec/system/page_objects/components/topic_list.rb +++ b/spec/system/page_objects/components/topic_list.rb @@ -75,12 +75,12 @@ module PageObjects find("#topic-entrance button.jump-top").native.send_keys(:return) end - private - def topic_list_item_class(topic) "#{TOPIC_LIST_ITEM_SELECTOR}[data-topic-id='#{topic.id}']" end + private + def topic_list_item_closed(topic) "#{topic_list_item_class(topic)} .topic-statuses .topic-status svg.locked" end diff --git a/spec/system/page_objects/components/topic_list_header.rb b/spec/system/page_objects/components/topic_list_header.rb index 79a96430984..2d689a26a8b 100644 --- a/spec/system/page_objects/components/topic_list_header.rb +++ b/spec/system/page_objects/components/topic_list_header.rb @@ -30,6 +30,12 @@ module PageObjects ).click end + def click_bulk_button(name) + find(bulk_select_dropdown_item(name)).click + end + + # TODO (martin) Remove all this once discourse-assign is using the new bulk select + # modal page object in specs. def has_close_topics_button? page.has_css?(bulk_select_dropdown_item("close-topics")) end @@ -57,6 +63,7 @@ module PageObjects def click_dismiss_read_confirm find("#dismiss-read-confirm").click end + ### /TODO private diff --git a/spec/system/page_objects/modals/topic_bulk_actions.rb b/spec/system/page_objects/modals/topic_bulk_actions.rb new file mode 100644 index 00000000000..f0b541d81c0 --- /dev/null +++ b/spec/system/page_objects/modals/topic_bulk_actions.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true +module PageObjects + module Modals + class TopicBulkActions < PageObjects::Modals::Base + MODAL_SELECTOR = ".topic-bulk-actions-modal" + + def tag_selector + PageObjects::Components::SelectKit.new(".tag-chooser") + end + + def click_bulk_topics_confirm + find("#bulk-topics-confirm").click + end + + def click_silent + find("#topic-bulk-action-options__silent").click + end + + def fill_in_close_note(message) + find("#bulk-close-note").set(message) + end + + def has_category_badge?(category) + within(MODAL_SELECTOR) do + PageObjects::Components::CategoryBadge.new.find_for_category(category) + end + end + + def has_no_category_badge?(category) + within(MODAL_SELECTOR) do + has_no_css?(PageObjects::Components::CategoryBadge.new.category_selector(category)) + end + end + end + end +end diff --git a/spec/system/topic_bulk_select_spec.rb b/spec/system/topic_bulk_select_spec.rb index 17d365c8a92..51cf3dca3b2 100644 --- a/spec/system/topic_bulk_select_spec.rb +++ b/spec/system/topic_bulk_select_spec.rb @@ -3,19 +3,115 @@ describe "Topic bulk select", type: :system do before { SiteSetting.experimental_topic_bulk_actions_enabled_groups = "1" } fab!(:topics) { Fabricate.times(10, :post).map(&:topic) } + fab!(:admin) + fab!(:user) + let(:topic_list_header) { PageObjects::Components::TopicListHeader.new } let(:topic_list) { PageObjects::Components::TopicList.new } let(:topic_page) { PageObjects::Pages::Topic.new } + let(:topic_bulk_actions_modal) { PageObjects::Modals::TopicBulkActions.new } - context "when in topic" do - fab!(:admin) - fab!(:user) + context "when appending tags" do + fab!(:tag1) { Fabricate(:tag) } + fab!(:tag2) { Fabricate(:tag) } + fab!(:tag3) { Fabricate(:tag) } + before { SiteSetting.tagging_enabled = true } + + def open_append_modal(topics_to_select = nil) + sign_in(admin) + visit("/latest") + + topic_list_header.click_bulk_select_button + + if !topics_to_select + topic_list.click_topic_checkbox(topics.last) + else + topics_to_select.each { |topic| topic_list.click_topic_checkbox(topic) } + end + + topic_list_header.click_bulk_select_topics_dropdown + topic_list_header.click_bulk_button("append-tags") + expect(topic_bulk_actions_modal).to be_open + end + + it "appends tags to selected topics" do + open_append_modal + + topic_bulk_actions_modal.tag_selector.expand + topic_bulk_actions_modal.tag_selector.search(tag1.name) + topic_bulk_actions_modal.tag_selector.select_row_by_value(tag1.name) + topic_bulk_actions_modal.tag_selector.search(tag2.name) + topic_bulk_actions_modal.tag_selector.select_row_by_value(tag2.name) + + topic_bulk_actions_modal.click_bulk_topics_confirm + + expect( + find(topic_list.topic_list_item_class(topics.last)).find(".discourse-tags"), + ).to have_content(tag1.name) + expect( + find(topic_list.topic_list_item_class(topics.last)).find(".discourse-tags"), + ).to have_content(tag2.name) + end + + context "when selecting topics in different categories" do + before do + topics + .last(2) + .each do |topic| + topic.update!(category: Fabricate(:category)) + topic.update!(category: Fabricate(:category)) + end + end + + it "does not show an additional note about the category in the modal" do + open_append_modal(topics.last(2)) + + expect(topic_bulk_actions_modal).to have_no_category_badge(topics.last.reload.category) + end + end + + context "when selecting topics that are all in the same category" do + fab!(:category) + + before { topics.last.update!(category_id: category.id) } + + it "shows an additional note about the category in the modal" do + open_append_modal + expect(topic_bulk_actions_modal).to have_category_badge(category) + end + + it "allows for searching restricted tags for that category and other tags too if the category allows it" do + restricted_tag_group = Fabricate(:tag_group) + restricted_tag = Fabricate(:tag) + TagGroupMembership.create!(tag: restricted_tag, tag_group: restricted_tag_group) + CategoryTagGroup.create!(category: category, tag_group: restricted_tag_group) + category.update!(allow_global_tags: true) + + open_append_modal + + topic_bulk_actions_modal.tag_selector.expand + topic_bulk_actions_modal.tag_selector.search(restricted_tag.name) + topic_bulk_actions_modal.tag_selector.select_row_by_value(restricted_tag.name) + topic_bulk_actions_modal.tag_selector.search(tag1.name) + topic_bulk_actions_modal.tag_selector.select_row_by_value(tag1.name) + + topic_bulk_actions_modal.click_bulk_topics_confirm + + expect( + find(topic_list.topic_list_item_class(topics.last)).find(".discourse-tags"), + ).to have_content(restricted_tag.name) + expect( + find(topic_list.topic_list_item_class(topics.last)).find(".discourse-tags"), + ).to have_content(tag1.name) + end + end + end + + context "when closing" do it "closes multiple topics" do sign_in(admin) visit("/latest") - expect(page).to have_css(".topic-list button.bulk-select") - expect(topic_list_header).to have_bulk_select_button # Click bulk select button topic_list_header.click_bulk_select_button @@ -30,16 +126,15 @@ describe "Topic bulk select", type: :system do topic_list_header.click_bulk_select_topics_dropdown # Clicking the close button opens up the modal - expect(topic_list_header).to have_close_topics_button - topic_list_header.click_close_topics_button - expect(topic_list_header).to have_bulk_select_modal + topic_list_header.click_bulk_button("close-topics") + expect(topic_bulk_actions_modal).to be_open # Closes the selected topics - topic_list_header.click_bulk_topics_confirm + topic_bulk_actions_modal.click_bulk_topics_confirm expect(topic_list).to have_closed_status(topics.first) end - it "closes topics normally" do + it "closes single topic" do # Watch the topic as a user sign_in(user) visit("/latest") @@ -54,8 +149,8 @@ describe "Topic bulk select", type: :system do topic_list_header.click_bulk_select_button topic_list.click_topic_checkbox(topics.third) topic_list_header.click_bulk_select_topics_dropdown - topic_list_header.click_close_topics_button - topic_list_header.click_bulk_topics_confirm + topic_list_header.click_bulk_button("close-topics") + topic_bulk_actions_modal.click_bulk_topics_confirm # Check that the user did receive a new post notification badge sign_in(user) @@ -78,9 +173,9 @@ describe "Topic bulk select", type: :system do topic_list_header.click_bulk_select_button topic_list.click_topic_checkbox(topics.first) topic_list_header.click_bulk_select_topics_dropdown - topic_list_header.click_close_topics_button - topic_list_header.click_silent # Check Silent - topic_list_header.click_bulk_topics_confirm + topic_list_header.click_bulk_button("close-topics") + topic_bulk_actions_modal.click_silent # Check Silent + topic_bulk_actions_modal.click_bulk_topics_confirm # Check that the user didn't receive a new post notification badge sign_in(user) @@ -96,15 +191,15 @@ describe "Topic bulk select", type: :system do topic_list_header.click_bulk_select_button topic_list.click_topic_checkbox(topics.first) topic_list_header.click_bulk_select_topics_dropdown - topic_list_header.click_close_topics_button + topic_list_header.click_bulk_button("close-topics") # Fill in message - topic_list_header.fill_in_close_note("My message") - topic_list_header.click_bulk_topics_confirm + topic_bulk_actions_modal.fill_in_close_note("None of these are useful") + topic_bulk_actions_modal.click_bulk_topics_confirm # Check that the topic now has the message visit("/t/#{topic.slug}/#{topic.id}") - expect(topic_page).to have_content("My message") + expect(topic_page).to have_content("None of these are useful") end it "works with keyboard shortcuts" do @@ -134,7 +229,7 @@ describe "Topic bulk select", type: :system do send_keys("x") send_keys([:shift, "d"]) - topic_list_header.click_dismiss_read_confirm + click_button("dismiss-read-confirm") expect(topic_list).to have_no_topics end