From 206969e49d89807b6eed2a3057e77cab1ecc2e7b Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 7 Sep 2023 19:05:41 +0100 Subject: [PATCH] DEV: Replace BulkTopicSelection mixin with a helper object (#23268) Mixins are considered deprecated by Ember, and cannot be applied to modern framework objects. Also, the coupling they introduce can make things very difficult to refactor. This commit takes the state/logic from BulkTopicSelection and turns it into a helper object. This avoids it polluting any controllers/components its included in. In future, the entire helper object can be passed down to child components so that they don't need to lookup controllers using the resolver. Those kinds of changes would involve changing some very heavily-overridden templates, so they are not included in this PR. It will likely be done as part of the larger refactor in https://github.com/discourse/discourse/pull/22622. --- .../app/components/bulk-select-toggle.hbs | 6 +- .../app/components/bulk-select-toggle.js | 18 ++-- .../discourse/app/components/d-navigation.hbs | 2 +- .../app/controllers/discovery/topics.js | 50 +++++++++-- .../discourse/app/controllers/tag-show.js | 50 +++++++++-- .../app/controllers/user-topics-list.js | 41 ++++++++-- .../discourse/app/lib/bulk-select-helper.js | 66 +++++++++++++++ .../discourse/app/lib/filter-mode.js | 2 +- .../app/mixins/bulk-topic-selection.js | 82 ------------------- .../app/routes/build-category-route.js | 2 +- .../routes/build-private-messages-route.js | 2 +- .../discourse/app/routes/build-topic-route.js | 2 +- .../user-private-messages-tags-index.js | 2 +- .../discourse/app/templates/user/messages.hbs | 2 +- 14 files changed, 200 insertions(+), 127 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/lib/bulk-select-helper.js delete mode 100644 app/assets/javascripts/discourse/app/mixins/bulk-topic-selection.js diff --git a/app/assets/javascripts/discourse/app/components/bulk-select-toggle.hbs b/app/assets/javascripts/discourse/app/components/bulk-select-toggle.hbs index 2caf4cb5717..e0b31d677f3 100644 --- a/app/assets/javascripts/discourse/app/components/bulk-select-toggle.hbs +++ b/app/assets/javascripts/discourse/app/components/bulk-select-toggle.hbs @@ -1,5 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/components/bulk-select-toggle.js b/app/assets/javascripts/discourse/app/components/bulk-select-toggle.js index 80ddbc7decc..09e2a1c22f1 100644 --- a/app/assets/javascripts/discourse/app/components/bulk-select-toggle.js +++ b/app/assets/javascripts/discourse/app/components/bulk-select-toggle.js @@ -1,17 +1,15 @@ -import Component from "@ember/component"; +import Component from "@glimmer/component"; import { action } from "@ember/object"; import { getOwner } from "discourse-common/lib/get-owner"; -export default Component.extend({ - parentController: null, - +export default class BulkSelectToggle extends Component { @action toggleBulkSelect() { const controller = getOwner(this).lookup( - `controller:${this.parentController}` + `controller:${this.args.parentController}` ); - const selection = controller.selected; - controller.toggleProperty("bulkSelectEnabled"); - selection.clear(); - }, -}); + const helper = controller.bulkSelectHelper; + helper.clear(); + helper.bulkSelectEnabled = !helper.bulkSelectEnabled; + } +} diff --git a/app/assets/javascripts/discourse/app/components/d-navigation.hbs b/app/assets/javascripts/discourse/app/components/d-navigation.hbs index ae8e4a23b25..98907cbba12 100644 --- a/app/assets/javascripts/discourse/app/components/d-navigation.hbs +++ b/app/assets/javascripts/discourse/app/components/d-navigation.hbs @@ -17,7 +17,7 @@
{{#if (and this.notCategoriesRoute this.site.mobileView this.canBulk)}} - + {{/if}} {{#if this.showCategoryAdmin}} diff --git a/app/assets/javascripts/discourse/app/controllers/discovery/topics.js b/app/assets/javascripts/discourse/app/controllers/discovery/topics.js index a87580e1f05..af9ac88ddab 100644 --- a/app/assets/javascripts/discourse/app/controllers/discovery/topics.js +++ b/app/assets/javascripts/discourse/app/controllers/discovery/topics.js @@ -1,7 +1,7 @@ import { inject as controller } from "@ember/controller"; import { inject as service } from "@ember/service"; -import { alias, empty, equal, gt, readOnly } from "@ember/object/computed"; -import BulkTopicSelection from "discourse/mixins/bulk-topic-selection"; +import { alias, empty, equal, gt, or, readOnly } from "@ember/object/computed"; +import BulkSelectHelper from "discourse/lib/bulk-select-helper"; import DismissTopics from "discourse/mixins/dismiss-topics"; import DiscoveryController from "discourse/controllers/discovery"; import I18n from "I18n"; @@ -12,17 +12,18 @@ import { endWith } from "discourse/lib/computed"; import { routeAction } from "discourse/helpers/route-action"; import { userPath } from "discourse/lib/url"; import { action } from "@ember/object"; +import { filterTypeForMode } from "discourse/lib/filter-mode"; export default class TopicsController extends DiscoveryController.extend( - BulkTopicSelection, DismissTopics ) { @service router; @service composer; @controller discovery; + bulkSelectHelper = new BulkSelectHelper(this); + period = null; - selected = null; expandGloballyPinned = false; expandAllPinned = false; @@ -40,14 +41,25 @@ export default class TopicsController extends DiscoveryController.extend( @equal("period", "weekly") weekly; @equal("period", "daily") daily; - @discourseComputed("model.filter", "model.topics.length") - showDismissRead(filter, topicsLength) { - return this._isFilterPage(filter, "unread") && topicsLength > 0; + @or("currentUser.canManageTopic", "showDismissRead", "showResetNew") + canBulkSelect; + + get bulkSelectEnabled() { + return this.bulkSelectHelper.bulkSelectEnabled; + } + + get selected() { + return this.bulkSelectHelper.selected; } @discourseComputed("model.filter", "model.topics.length") - showResetNew(filter, topicsLength) { - return this._isFilterPage(filter, "new") && topicsLength > 0; + showDismissRead(filterMode, topicsLength) { + return filterTypeForMode(filterMode) === "unread" && topicsLength > 0; + } + + @discourseComputed("model.filter", "model.topics.length") + showResetNew(filterMode, topicsLength) { + return filterTypeForMode(filterMode) === "new" && topicsLength > 0; } callResetNew(dismissPosts = false, dismissTopics = false, untrack = false) { @@ -212,4 +224,24 @@ export default class TopicsController extends DiscoveryController.extend( !this.get("bulkSelectEnabled") ); } + + @action + toggleBulkSelect() { + this.bulkSelectHelper.toggleBulkSelect(); + } + + @action + dismissRead(operationType, options) { + this.bulkSelectHelper.dismissRead(operationType, options); + } + + @action + updateAutoAddTopicsToBulkSelect(value) { + this.bulkSelectHelper.autoAddTopicsToBulkSelect = value; + } + + @action + addTopicsToBulkSelect(topics) { + this.bulkSelectHelper.addTopics(topics); + } } diff --git a/app/assets/javascripts/discourse/app/controllers/tag-show.js b/app/assets/javascripts/discourse/app/controllers/tag-show.js index cf8e85514e0..5a112ebe875 100644 --- a/app/assets/javascripts/discourse/app/controllers/tag-show.js +++ b/app/assets/javascripts/discourse/app/controllers/tag-show.js @@ -1,8 +1,8 @@ import { inject as service } from "@ember/service"; -import { readOnly } from "@ember/object/computed"; +import { or, readOnly } from "@ember/object/computed"; import DiscoverySortableController from "discourse/controllers/discovery-sortable"; import discourseComputed from "discourse-common/utils/decorators"; -import BulkTopicSelection from "discourse/mixins/bulk-topic-selection"; +import BulkSelectHelper from "discourse/lib/bulk-select-helper"; import DismissTopics from "discourse/mixins/dismiss-topics"; import I18n from "I18n"; import NavItem from "discourse/models/nav-item"; @@ -14,7 +14,6 @@ import { dependentKeyCompat } from "@ember/object/compat"; import { tracked } from "@glimmer/tracking"; export default class TagShowController extends DiscoverySortableController.extend( - BulkTopicSelection, DismissTopics ) { @service dialog; @@ -26,6 +25,8 @@ export default class TagShowController extends DiscoverySortableController.exten @tracked filterType; @tracked noSubcategories; + bulkSelectHelper = new BulkSelectHelper(this); + tag = null; additionalTags = null; list = null; @@ -39,6 +40,17 @@ export default class TagShowController extends DiscoverySortableController.exten @endWith("list.filter", "top") top; + @or("currentUser.canManageTopic", "showDismissRead", "showResetNew") + canBulkSelect; + + get bulkSelectEnabled() { + return this.bulkSelectHelper.bulkSelectEnabled; + } + + get selected() { + return this.bulkSelectHelper.selected; + } + @dependentKeyCompat get filterMode() { return calculateFilterMode({ @@ -96,14 +108,14 @@ export default class TagShowController extends DiscoverySortableController.exten } } - @discourseComputed("list.filter", "list.topics.length") - showDismissRead(filter, topicsLength) { - return this._isFilterPage(filter, "unread") && topicsLength > 0; + @discourseComputed("filterType", "list.topics.length") + showDismissRead(filterType, topicsLength) { + return filterType === "unread" && topicsLength > 0; } - @discourseComputed("list.filter") - new(filter) { - return this._isFilterPage(filter, "new"); + @discourseComputed("filterType") + new(filterType) { + return filterType === "new"; } @discourseComputed("new") @@ -258,4 +270,24 @@ export default class TagShowController extends DiscoverySortableController.exten }); }); } + + @action + toggleBulkSelect() { + this.bulkSelectHelper.toggleBulkSelect(); + } + + @action + dismissRead(operationType, options) { + this.bulkSelectHelper.dismissRead(operationType, options); + } + + @action + updateAutoAddTopicsToBulkSelect(value) { + this.bulkSelectHelper.autoAddTopicsToBulkSelect = value; + } + + @action + addTopicsToBulkSelect(topics) { + this.bulkSelectHelper.addTopics(topics); + } } diff --git a/app/assets/javascripts/discourse/app/controllers/user-topics-list.js b/app/assets/javascripts/discourse/app/controllers/user-topics-list.js index 548a2e380c3..f66a0de36be 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-topics-list.js +++ b/app/assets/javascripts/discourse/app/controllers/user-topics-list.js @@ -1,7 +1,7 @@ import Controller from "@ember/controller"; import discourseComputed from "discourse-common/utils/decorators"; -import { reads } from "@ember/object/computed"; -import BulkTopicSelection from "discourse/mixins/bulk-topic-selection"; +import { or, reads } from "@ember/object/computed"; +import BulkSelectHelper from "discourse/lib/bulk-select-helper"; import { action } from "@ember/object"; import Topic from "discourse/models/topic"; @@ -11,16 +11,27 @@ import { } from "discourse/routes/build-private-messages-route"; // Lists of topics on a user's page. -export default class UserTopicsListController extends Controller.extend( - BulkTopicSelection -) { +export default class UserTopicsListController extends Controller { hideCategory = false; showPosters = false; channel = null; tagsForUser = null; + bulkSelectHelper = new BulkSelectHelper(this); + @reads("pmTopicTrackingState.newIncoming.length") incomingCount; + @or("currentUser.canManageTopic", "showDismissRead", "showResetNew") + canBulkSelect; + + get bulkSelectEnabled() { + return this.bulkSelectHelper.bulkSelectEnabled; + } + + get selected() { + return this.bulkSelectHelper.selected; + } + @discourseComputed("model.topics.length", "incomingCount") noContent(topicsLength, incomingCount) { return topicsLength === 0 && incomingCount === 0; @@ -83,4 +94,24 @@ export default class UserTopicsListController extends Controller.extend( refresh() { this.send("triggerRefresh"); } + + @action + toggleBulkSelect() { + this.bulkSelectHelper.toggleBulkSelect(); + } + + @action + dismissRead(operationType, options) { + this.bulkSelectHelper.dismissRead(operationType, options); + } + + @action + updateAutoAddTopicsToBulkSelect(value) { + this.bulkSelectHelper.autoAddTopicsToBulkSelect = value; + } + + @action + addTopicsToBulkSelect(topics) { + this.bulkSelectHelper.addTopics(topics); + } } diff --git a/app/assets/javascripts/discourse/app/lib/bulk-select-helper.js b/app/assets/javascripts/discourse/app/lib/bulk-select-helper.js new file mode 100644 index 00000000000..f6dfbdaea90 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/bulk-select-helper.js @@ -0,0 +1,66 @@ +import { NotificationLevels } from "discourse/lib/notification-levels"; +import Topic from "discourse/models/topic"; +import { inject as service } from "@ember/service"; +import { getOwner, setOwner } from "@ember/application"; +import { tracked } from "@glimmer/tracking"; +import { TrackedArray } from "@ember-compat/tracked-built-ins"; + +export default class BulkSelectHelper { + @service router; + @service modal; + @service pmTopicTrackingState; + @service topicTrackingState; + + @tracked bulkSelectEnabled = false; + @tracked autoAddTopicsToBulkSelect = false; + + selected = new TrackedArray(); + + constructor(context) { + setOwner(this, getOwner(context)); + } + + clear() { + this.selected.length = 0; + } + + addTopics(topics) { + this.selected.concat(topics); + } + + toggleBulkSelect() { + this.bulkSelectEnabled = !this.bulkSelectEnabled; + this.clear(); + } + + dismissRead(operationType, options) { + const operation = + operationType === "posts" + ? { type: "dismiss_posts" } + : { + type: "change_notification_level", + notification_level_id: NotificationLevels.REGULAR, + }; + + const isTracked = + (this.router.currentRoute.queryParams["f"] || + this.router.currentRoute.queryParams["filter"]) === "tracked"; + + const promise = this.selected.length + ? Topic.bulkOperation(this.selected, operation, isTracked) + : Topic.bulkOperationByFilter("unread", operation, options, isTracked); + + promise.then((result) => { + if (result?.topic_ids) { + if (options.private_message_inbox) { + this.pmTopicTrackingState.removeTopics(result.topic_ids); + } else { + this.topicTrackingState.removeTopics(result.topic_ids); + } + } + + this.modal.close(); + this.router.refresh(); + }); + } +} diff --git a/app/assets/javascripts/discourse/app/lib/filter-mode.js b/app/assets/javascripts/discourse/app/lib/filter-mode.js index 46ad69e00f1..1b839457c6e 100644 --- a/app/assets/javascripts/discourse/app/lib/filter-mode.js +++ b/app/assets/javascripts/discourse/app/lib/filter-mode.js @@ -11,5 +11,5 @@ export function calculateFilterMode({ category, filterType, noSubcategories }) { } export function filterTypeForMode(mode) { - return mode.split("/").pop(); + return mode?.split("/").pop(); } diff --git a/app/assets/javascripts/discourse/app/mixins/bulk-topic-selection.js b/app/assets/javascripts/discourse/app/mixins/bulk-topic-selection.js deleted file mode 100644 index fe1980ed824..00000000000 --- a/app/assets/javascripts/discourse/app/mixins/bulk-topic-selection.js +++ /dev/null @@ -1,82 +0,0 @@ -import Mixin from "@ember/object/mixin"; -import { or } from "@ember/object/computed"; -import { on } from "discourse-common/utils/decorators"; -import { NotificationLevels } from "discourse/lib/notification-levels"; -import Topic from "discourse/models/topic"; -import { inject as service } from "@ember/service"; - -export default Mixin.create({ - router: service(), - - bulkSelectEnabled: false, - autoAddTopicsToBulkSelect: false, - selected: null, - lastChecked: null, - - canBulkSelect: or( - "currentUser.canManageTopic", - "showDismissRead", - "showResetNew" - ), - - @on("init") - resetSelected() { - this.set("selected", []); - }, - - _isFilterPage(filter, filterType) { - if (!filter) { - return false; - } - return new RegExp(filterType + "$", "gi").test(filter); - }, - - actions: { - toggleBulkSelect() { - this.toggleProperty("bulkSelectEnabled"); - this.selected.clear(); - }, - - dismissRead(operationType, options) { - const operation = - operationType === "posts" - ? { type: "dismiss_posts" } - : { - type: "change_notification_level", - notification_level_id: NotificationLevels.REGULAR, - }; - - const tracked = - (this.router.currentRoute.queryParams["f"] || - this.router.currentRoute.queryParams["filter"]) === "tracked"; - - const promise = this.selected.length - ? Topic.bulkOperation(this.selected, operation, tracked) - : Topic.bulkOperationByFilter("unread", operation, options, tracked); - - promise.then((result) => { - if (result && result.topic_ids) { - if (options.private_message_inbox) { - this.pmTopicTrackingState.removeTopics(result.topic_ids); - } else { - this.topicTrackingState.removeTopics(result.topic_ids); - } - } - - this.send("closeModal"); - this.send( - "refresh", - tracked ? { skipResettingParams: ["filter", "f"] } : {} - ); - }); - }, - - updateAutoAddTopicsToBulkSelect(newVal) { - this.set("autoAddTopicsToBulkSelect", newVal); - }, - - addTopicsToBulkSelect(topics) { - this.selected.pushObjects(topics); - }, - }, -}); diff --git a/app/assets/javascripts/discourse/app/routes/build-category-route.js b/app/assets/javascripts/discourse/app/routes/build-category-route.js index d48e6d21e3b..06df29fc466 100644 --- a/app/assets/javascripts/discourse/app/routes/build-category-route.js +++ b/app/assets/javascripts/discourse/app/routes/build-category-route.js @@ -162,7 +162,6 @@ class AbstractCategoryRoute extends DiscourseRoute { period: topics.get("for_period") || (model.modelParams && model.modelParams.period), - selected: [], noSubcategories: this.routeConfig && !!this.routeConfig.no_subcategories, expandAllPinned: true, }; @@ -178,6 +177,7 @@ class AbstractCategoryRoute extends DiscourseRoute { } this.controllerFor("discovery/topics").setProperties(topicOpts); + this.controllerFor("discovery/topics").bulkSelectHelper.clear(); this.searchService.searchContext = category.get("searchContext"); this.set("topics", null); } diff --git a/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js b/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js index ec85ccfb901..a588a923a01 100644 --- a/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js +++ b/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js @@ -60,12 +60,12 @@ export default (inboxType, path, filter) => { hideCategory: true, showPosters: true, tagsForUser: this.modelFor("user").get("username_lower"), - selected: [], showToggleBulkSelect: true, filter, group: null, inbox: inboxType, }); + userTopicsListController.bulkSelectHelper.clear(); userTopicsListController.subscribe(); diff --git a/app/assets/javascripts/discourse/app/routes/build-topic-route.js b/app/assets/javascripts/discourse/app/routes/build-topic-route.js index 7cd5085e2f7..13dba923b4f 100644 --- a/app/assets/javascripts/discourse/app/routes/build-topic-route.js +++ b/app/assets/javascripts/discourse/app/routes/build-topic-route.js @@ -139,12 +139,12 @@ class AbstractTopicRoute extends DiscourseRoute { model, category: null, period: model.get("for_period") || model.get("params.period"), - selected: [], expandAllPinned: false, expandGloballyPinned: true, }; this.controllerFor("discovery/topics").setProperties(topicOpts); + this.controllerFor("discovery/topics").bulkSelectHelper.clear(); this.controllerFor("navigation/default").set( "canCreateTopic", diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-tags-index.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-tags-index.js index 869b2a9bf4f..4e74c4dfb7f 100644 --- a/app/assets/javascripts/discourse/app/routes/user-private-messages-tags-index.js +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-tags-index.js @@ -30,7 +30,7 @@ export default DiscourseRoute.extend({ this.controllerFor("user-topics-list").setProperties({ showToggleBulkSelect: false, - selected: [], }); + this.controllerFor("user-topics-list").bulkSelectHelper.clear(); }, }); diff --git a/app/assets/javascripts/discourse/app/templates/user/messages.hbs b/app/assets/javascripts/discourse/app/templates/user/messages.hbs index 410e1974bb9..3473acc7161 100644 --- a/app/assets/javascripts/discourse/app/templates/user/messages.hbs +++ b/app/assets/javascripts/discourse/app/templates/user/messages.hbs @@ -25,7 +25,7 @@
{{#if this.site.mobileView}} {{#if this.currentUser.admin}} - + {{/if}} {{/if}}