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/assets/javascripts/discourse/app/components/bulk-select-toggle.hbs

@@ -1,5 +1 @@
-<DButton
+<DButton class="bulk-select" @action={{this.toggleBulkSelect}} @icon="list" />
-  class="bulk-select"
-  @action={{action "toggleBulkSelect"}}
-  @icon="list"
-/>

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({
+export default class BulkSelectToggle extends Component {
-  parentController: null,
-
   @action
   toggleBulkSelect() {
     const controller = getOwner(this).lookup(
-      `controller:${this.parentController}`
+      `controller:${this.args.parentController}`
     );
-    const selection = controller.selected;
+    const helper = controller.bulkSelectHelper;
-    controller.toggleProperty("bulkSelectEnabled");
+    helper.clear();
-    selection.clear();
+    helper.bulkSelectEnabled = !helper.bulkSelectEnabled;
-  },
+  }
-});
+}

app/assets/javascripts/discourse/app/components/d-navigation.hbs

@@ -17,7 +17,7 @@
 
 <div class="navigation-controls">
   {{#if (and this.notCategoriesRoute this.site.mobileView this.canBulk)}}
-    <BulkSelectToggle @parentController="discovery/topics" @tagName="" />
+    <BulkSelectToggle @parentController="discovery/topics" />
   {{/if}}
 
   {{#if this.showCategoryAdmin}}

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 { alias, empty, equal, gt, or, readOnly } from "@ember/object/computed";
-import BulkTopicSelection from "discourse/mixins/bulk-topic-selection";
+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")
+  @or("currentUser.canManageTopic", "showDismissRead", "showResetNew")
-  showDismissRead(filter, topicsLength) {
+  canBulkSelect;
-    return this._isFilterPage(filter, "unread") && topicsLength > 0;
+
+  get bulkSelectEnabled() {
+    return this.bulkSelectHelper.bulkSelectEnabled;
+  }
+
+  get selected() {
+    return this.bulkSelectHelper.selected;
   }
 
   @discourseComputed("model.filter", "model.topics.length")
-  showResetNew(filter, topicsLength) {
+  showDismissRead(filterMode, topicsLength) {
-    return this._isFilterPage(filter, "new") && topicsLength > 0;
+    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);
+  }
 }

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")
+  @discourseComputed("filterType", "list.topics.length")
-  showDismissRead(filter, topicsLength) {
+  showDismissRead(filterType, topicsLength) {
-    return this._isFilterPage(filter, "unread") && topicsLength > 0;
+    return filterType === "unread" && topicsLength > 0;
   }
 
-  @discourseComputed("list.filter")
+  @discourseComputed("filterType")
-  new(filter) {
+  new(filterType) {
-    return this._isFilterPage(filter, "new");
+    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);
+  }
 }

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 { or, reads } from "@ember/object/computed";
-import BulkTopicSelection from "discourse/mixins/bulk-topic-selection";
+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(
+export default class UserTopicsListController extends Controller {
-  BulkTopicSelection
-) {
   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);
+  }
 }

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();
+    });
+  }
+}

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();
 }

app/assets/javascripts/discourse/app/mixins/bulk-topic-selection.js

@@ -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);
-    },
-  },
-});

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);
   }

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();
 

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",

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();
   },
 });

app/assets/javascripts/discourse/app/templates/user/messages.hbs

@@ -25,7 +25,7 @@
   <div class="navigation-controls">
     {{#if this.site.mobileView}}
       {{#if this.currentUser.admin}}
-        <BulkSelectToggle @parentController="user-topics-list" @tagName="" />
+        <BulkSelectToggle @parentController="user-topics-list" />
       {{/if}}
     {{/if}}