REFACTOR: Improve reusability by Decoupling flag modal from flag target. (#18251)

* REFACTOR: Improve reusability by Decoupling flag modal from flag target.

We want chat message's flags to have the same features as topic and posts' flags, but we prefer not having to duplicate core's logic. This PR moves target specific bits to different classes, allowing plugins to flag custom things by
providing their own.

* A couple of fixes for the flag modal:

- Make sure buttons are disabled until a flag type is selected.
- Don't throw an error when checking if the user can undo an action on a deleted topic.
- Disable flagging on deleted topics.

app/assets/javascripts/discourse/app/controllers/flag.js

@@ -1,7 +1,5 @@
 import { schedule } from "@ember/runloop";
-import ActionSummary from "discourse/models/action-summary";
 import Controller from "@ember/controller";
-import EmberObject from "@ember/object";
 import I18n from "I18n";
 import { MAX_MESSAGE_LENGTH } from "discourse/models/post-action-type";
 import ModalFunctionality from "discourse/mixins/modal-functionality";
@@ -10,19 +8,18 @@ import User from "discourse/models/user";
 import discourseComputed, { bind } from "discourse-common/utils/decorators";
 import { not } from "@ember/object/computed";
 import optionalService from "discourse/lib/optional-service";
-import { popupAjaxError } from "discourse/lib/ajax-error";
 import { classify } from "@ember/string";
 
 export default Controller.extend(ModalFunctionality, {
   adminTools: optionalService(),
   userDetails: null,
   selected: null,
-  flagTopic: null,
   message: null,
   isWarning: false,
   topicActionByName: null,
   spammerDetails: null,
   flagActions: null,
+  flagTarget: null,
 
   init() {
     this._super(...arguments);
@@ -76,13 +73,14 @@ export default Controller.extend(ModalFunctionality, {
   _penalize(adminToolMethod, performAction) {
     if (this.adminTools) {
       return User.findByUsername(this.model.username).then((createdBy) => {
-        let postId = this.model.id;
-        let postEdit = this.model.cooked;
-        return this.adminTools[adminToolMethod](createdBy, {
-          postId,
-          postEdit,
-          before: performAction,
-        });
+        const opts = { before: performAction };
+
+        if (this.flagTarget.editable()) {
+          opts.postId = this.model.id;
+          opts.postEdit = this.model.cooked;
+        }
+
+        return this.adminTools[adminToolMethod](createdBy, opts);
       });
     }
   },
@@ -115,47 +113,25 @@ export default Controller.extend(ModalFunctionality, {
     return canDeleteSpammer && nameKey === "spam";
   },
 
-  @discourseComputed("flagTopic")
-  title(flagTopic) {
-    return flagTopic ? "flagging_topic.title" : "flagging.title";
+  @discourseComputed("flagTarget")
+  title(flagTarget) {
+    return flagTarget.title();
   },
 
-  @discourseComputed("post", "flagTopic", "model.actions_summary.@each.can_act")
+  @discourseComputed(
+    "post",
+    "flagTarget",
+    "model.actions_summary.@each.can_act"
+  )
   flagsAvailable() {
-    if (!this.flagTopic) {
-      // flagging post
-      let flagsAvailable = this.get("model.flagsAvailable");
-
-      // "message user" option should be at the top
-      const notifyUserIndex = flagsAvailable.indexOf(
-        flagsAvailable.filterBy("name_key", "notify_user")[0]
-      );
-      if (notifyUserIndex !== -1) {
-        const notifyUser = flagsAvailable[notifyUserIndex];
-        flagsAvailable.splice(notifyUserIndex, 1);
-        flagsAvailable.splice(0, 0, notifyUser);
-      }
-      return flagsAvailable;
-    } else {
-      // flagging topic
-      let lookup = EmberObject.create();
-      let model = this.model;
-      model.get("actions_summary").forEach((a) => {
-        a.flagTopic = model;
-        a.actionType = this.site.topicFlagTypeById(a.id);
-        lookup.set(a.actionType.get("name_key"), ActionSummary.create(a));
-      });
-      this.set("topicActionByName", lookup);
-
-      return this.site.get("topic_flag_types").filter((item) => {
-        return this.get("model.actions_summary").some((a) => {
-          return a.id === item.get("id") && a.can_act;
-        });
-      });
-    }
+    return this.flagTarget.flagsAvailable(this, this.site, this.model);
   },
 
-  @discourseComputed("post", "flagTopic", "model.actions_summary.@each.can_act")
+  @discourseComputed(
+    "post",
+    "flagTarget",
+    "model.actions_summary.@each.can_act"
+  )
   staffFlagsAvailable() {
     return (
       this.get("model.flagsAvailable") &&
@@ -190,9 +166,13 @@ export default Controller.extend(ModalFunctionality, {
   },
 
   // Staff accounts can "take action"
-  @discourseComputed("flagTopic", "selected.is_custom_flag")
-  canTakeAction(flagTopic, isCustomFlag) {
-    return !flagTopic && !isCustomFlag && this.currentUser.get("staff");
+  @discourseComputed("flagTarget", "selected.is_custom_flag")
+  canTakeAction(flagTarget, isCustomFlag) {
+    return (
+      !flagTarget.targetsTopic() &&
+      !isCustomFlag &&
+      this.currentUser.get("staff")
+    );
   },
 
   @discourseComputed("selected.is_custom_flag")
@@ -200,14 +180,13 @@ export default Controller.extend(ModalFunctionality, {
     return isCustomFlag ? "envelope" : "flag";
   },
 
-  @discourseComputed("selected.is_custom_flag", "flagTopic")
-  submitLabel(isCustomFlag, flagTopic) {
+  @discourseComputed("selected.is_custom_flag", "flagTarget")
+  submitLabel(isCustomFlag, flagTarget) {
     if (isCustomFlag) {
-      return flagTopic
-        ? "flagging_topic.notify_action"
-        : "flagging.notify_action";
+      return flagTarget.customSubmitLabel();
     }
-    return flagTopic ? "flagging_topic.action" : "flagging.action";
+
+    return flagTarget.submitLabel();
   },
 
   actions: {
@@ -243,59 +222,13 @@ export default Controller.extend(ModalFunctionality, {
     },
 
     createFlag(opts) {
-      let postAction; // an instance of ActionSummary
+      const params = opts || {};
 
-      if (!this.flagTopic) {
-        postAction = this.get("model.actions_summary").findBy(
-          "id",
-          this.get("selected.id")
-        );
-      } else {
-        postAction = this.get(
-          "topicActionByName." + this.get("selected.name_key")
-        );
+      if (this.get("selected.is_custom_flag")) {
+        params.message = this.message;
       }
 
-      let params = this.get("selected.is_custom_flag")
-        ? { message: this.message }
-        : {};
-
-      if (opts) {
-        params = Object.assign(params, opts);
-      }
-
-      this.appEvents.trigger(
-        this.flagTopic ? "topic:flag-created" : "post:flag-created",
-        this.model,
-        postAction,
-        params
-      );
-
-      this.send("hideModal");
-
-      postAction
-        .act(this.model, params)
-        .then(() => {
-          if (this.isDestroying || this.isDestroyed) {
-            return;
-          }
-
-          if (!params.skipClose) {
-            this.send("closeModal");
-          }
-          if (params.message) {
-            this.set("message", "");
-          }
-          this.appEvents.trigger("post-stream:refresh", {
-            id: this.get("model.id"),
-          });
-        })
-        .catch((error) => {
-          if (!this.isDestroying && !this.isDestroyed) {
-            this.send("closeModal");
-          }
-          popupAjaxError(error);
-        });
+      this.flagTarget.create(this, params);
     },
 
     createFlagAsWarning() {
@@ -304,7 +237,10 @@ export default Controller.extend(ModalFunctionality, {
     },
 
     flagForReview() {
-      this.set("selected", this.get("notifyModeratorsFlag"));
+      if (!this.selected) {
+        this.set("selected", this.get("notifyModeratorsFlag"));
+      }
+
       this.send("createFlag", { queue_for_review: true });
       this.set("model.hidden", true);
     },
@@ -314,10 +250,12 @@ export default Controller.extend(ModalFunctionality, {
     },
   },
 
-  @discourseComputed("flagTopic", "selected.name_key")
-  canSendWarning(flagTopic, nameKey) {
+  @discourseComputed("flagTarget", "selected.name_key")
+  canSendWarning(flagTarget, nameKey) {
     return (
-      !flagTopic && this.currentUser.get("staff") && nameKey === "notify_user"
+      !flagTarget.targetsTopic() &&
+      this.currentUser.get("staff") &&
+      nameKey === "notify_user"
     );
   },
 });

app/assets/javascripts/discourse/app/lib/flag-targets/flag.js

@@ -0,0 +1,49 @@
+import { popupAjaxError } from "discourse/lib/ajax-error";
+
+export default class Flag {
+  targetsTopic() {
+    return false;
+  }
+
+  editable() {
+    return true;
+  }
+
+  create(controller, opts) {
+    // an instance of ActionSummary
+    let postAction = this.postActionFor(controller);
+
+    controller.appEvents.trigger(
+      this.flagCreatedEvent,
+      controller.model,
+      postAction,
+      opts
+    );
+
+    controller.send("hideModal");
+
+    postAction
+      .act(controller.model, opts)
+      .then(() => {
+        if (controller.isDestroying || controller.isDestroyed) {
+          return;
+        }
+
+        if (!opts.skipClose) {
+          controller.send("closeModal");
+        }
+        if (opts.message) {
+          controller.set("message", "");
+        }
+        controller.appEvents.trigger("post-stream:refresh", {
+          id: controller.get("model.id"),
+        });
+      })
+      .catch((error) => {
+        if (!controller.isDestroying && !controller.isDestroyed) {
+          controller.send("closeModal");
+        }
+        popupAjaxError(error);
+      });
+  }
+}

app/assets/javascripts/discourse/app/lib/flag-targets/post-flag.js

@@ -0,0 +1,42 @@
+import Flag from "discourse/lib/flag-targets/flag";
+
+export default class PostFlag extends Flag {
+  title() {
+    return "flagging.title";
+  }
+
+  customSubmitLabel() {
+    return "flagging.notify_action";
+  }
+
+  submitLabel() {
+    return "flagging.action";
+  }
+
+  flagCreatedEvent() {
+    return "post:flag-created";
+  }
+
+  flagsAvailable(_flagController, _site, model) {
+    let flagsAvailable = model.flagsAvailable;
+
+    // "message user" option should be at the top
+    const notifyUserIndex = flagsAvailable.indexOf(
+      flagsAvailable.filterBy("name_key", "notify_user")[0]
+    );
+
+    if (notifyUserIndex !== -1) {
+      const notifyUser = flagsAvailable[notifyUserIndex];
+      flagsAvailable.splice(notifyUserIndex, 1);
+      flagsAvailable.splice(0, 0, notifyUser);
+    }
+
+    return flagsAvailable;
+  }
+
+  postActionFor(controller) {
+    return controller
+      .get("model.actions_summary")
+      .findBy("id", controller.get("selected.id"));
+  }
+}

app/assets/javascripts/discourse/app/lib/flag-targets/topic-flag.js

@@ -0,0 +1,46 @@
+import ActionSummary from "discourse/models/action-summary";
+import EmberObject from "@ember/object";
+import Flag from "discourse/lib/flag-targets/flag";
+
+export default class TopicFlag extends Flag {
+  title() {
+    return "flagging_topic.title";
+  }
+
+  targetsTopic() {
+    return true;
+  }
+
+  customSubmitLabel() {
+    return "flagging_topic.notify_action";
+  }
+
+  submitLabel() {
+    return "flagging_topic.action";
+  }
+
+  flagCreatedEvent() {
+    return "topic:flag-created";
+  }
+
+  flagsAvailable(flagController, site, model) {
+    let lookup = EmberObject.create();
+
+    model.actions_summary.forEach((a) => {
+      a.flagTopic = model;
+      a.actionType = site.topicFlagTypeById(a.id);
+      lookup.set(a.actionType.name_key, ActionSummary.create(a));
+    });
+    flagController.set("topicActionByName", lookup);
+
+    return site.topic_flag_types.filter((item) => {
+      return model.actions_summary.some((a) => {
+        return a.id === item.id && a.can_act;
+      });
+    });
+  }
+
+  postActionFor(controller) {
+    return controller.get(`topicActionByName.${controller.selected.name_key}`);
+  }
+}

app/assets/javascripts/discourse/app/lib/transform-post.js

@@ -57,7 +57,7 @@ export function transformBasicPost(post) {
     showFlagDelete: false,
     canRecover: post.can_recover,
     canEdit: post.can_edit,
-    canFlag: !isEmpty(post.get("flagsAvailable")),
+    canFlag: !post.get("topic.deleted") && !isEmpty(post.get("flagsAvailable")),
     canReviewTopic: false,
     reviewableId: post.reviewable_id,
     reviewableScoreCount: post.reviewable_score_count,

app/assets/javascripts/discourse/app/routes/topic.js

@@ -8,6 +8,8 @@ import { isEmpty } from "@ember/utils";
 import { inject as service } from "@ember/service";
 import { setTopicId } from "discourse/lib/topic-list-tracker";
 import showModal from "discourse/lib/show-modal";
+import TopicFlag from "discourse/lib/flag-targets/topic-flag";
+import PostFlag from "discourse/lib/flag-targets/post-flag";
 
 const SCROLL_DELAY = 500;
 
@@ -94,14 +96,14 @@ const TopicRoute = DiscourseRoute.extend({
   @action
   showFlags(model) {
     let controller = showModal("flag", { model });
-    controller.setProperties({ flagTopic: false });
+    controller.setProperties({ flagTarget: new PostFlag() });
   },
 
   @action
   showFlagTopic() {
     const model = this.modelFor("topic");
     let controller = showModal("flag", { model });
-    controller.setProperties({ flagTopic: true });
+    controller.setProperties({ flagTarget: new TopicFlag() });
   },
 
   @action

app/assets/javascripts/discourse/app/templates/modal/flag.hbs

@@ -18,7 +18,7 @@
   {{#if this.canTakeAction}}
     <ReviewableBundledAction @bundle={{this.flagActions}} @performAction={{action "takeAction"}} @reviewableUpdating={{this.submitDisabled}} />
 
-    <DButton @class="btn-danger" @action={{action "flagForReview"}} @disabled={{this.cantFlagForReview}} @icon="exclamation-triangle" @label="flagging.flag_for_review" />
+    <DButton @class="btn-danger" @action={{action "flagForReview"}} @disabled={{or this.submitDisabled this.cantFlagForReview}} @icon="exclamation-triangle" @label="flagging.flag_for_review" />
   {{/if}}
 
   {{#if this.showDeleteSpammer}}

app/assets/javascripts/select-kit/addon/components/select-kit.js

@@ -294,6 +294,7 @@ export default Component.extend(
       mobilePlacementStrategy: null,
       desktopPlacementStrategy: null,
       hiddenValues: null,
+      disabled: false,
     },
 
     autoFilterable: computed("content.[]", "selectKit.filter", function () {

lib/guardian/post_guardian.rb

@@ -232,7 +232,7 @@ module PostGuardian
   def can_delete_post_action?(post_action)
     return false unless is_my_own?(post_action) && !post_action.is_private_message?
 
-    post_action.created_at > SiteSetting.post_undo_action_window_mins.minutes.ago && !post_action.post.topic&.archived?
+    post_action.created_at > SiteSetting.post_undo_action_window_mins.minutes.ago && !post_action.post&.topic&.archived?
   end
 
   def can_see_post?(post)