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.
This commit is contained in:
Martin Brennan 2024-04-12 13:10:14 +10:00 committed by GitHub
parent a96367e56a
commit 973a0028b4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 266 additions and 53 deletions

View File

@ -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"}}
>
<div>
{{htmlSafe
(i18n
"topics.bulk.selected"
count=@model.bulkSelectHelper.selected.length
)
}}
<div class="topic-bulk-actions-modal__selection-info">
{{#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}}
</div>
{{#if this.isCategoryAction}}
@ -330,7 +371,7 @@ export default class BulkTopicActions extends Component {
{{#if this.isTagAction}}
<p><TagChooser
@tags={{this.tags}}
@categoryId={{@categoryId}}
@categoryId={{this.soleCategoryId}}
/></p>
{{/if}}

View File

@ -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;

View File

@ -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;

View File

@ -3000,6 +3000,9 @@ en:
selected:
one: "You have selected <b>%{count}</b> topic."
other: "You have selected <b>%{count}</b> topics."
selected_sole_category:
one: "You have selected <b>%{count}</b> topic from category:"
other: "You have selected <b>%{count}</b> topics from category:"
selected_count:
one: "%{count} selected"
other: "%{count} selected"

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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