From 422f395042ef7f32d528a2c11e8da619fdbd5641 Mon Sep 17 00:00:00 2001
From: Osama Sayegh <asooomaasoooma90@gmail.com>
Date: Mon, 11 Jan 2021 18:29:12 +0300
Subject: [PATCH] FIX: Show unassigned component warning when installing
 multiple components successively (#11675)

A while ago we made a change to display a warning after installing a theme component when the admin tries to leave the page without adding the new installed component to any themes (see https://github.com/discourse/discourse/commit/5e29ae3ef5f7ad33906d00314cbdfb3a2813c529).

However there is an edge case that we forgot to address, and that's when an admin installs a component and then immediately opens the install modal again to install another one which can result in the warning being shown twice at the same time.

This PR prevents that by showing the warning when opening the install modal if the conditions are met (new component and not added to any themes) instead of showing it after installing the second component.
---
 .../javascripts/admin/addon/models/theme.js   |  5 +++
 .../routes/admin-customize-themes-show.js     | 32 ++++++++++++-------
 .../addon/routes/admin-customize-themes.js    | 13 +++++++-
 3 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/app/assets/javascripts/admin/addon/models/theme.js b/app/assets/javascripts/admin/addon/models/theme.js
index 9783b07e00f..d41ecb7f5f7 100644
--- a/app/assets/javascripts/admin/addon/models/theme.js
+++ b/app/assets/javascripts/admin/addon/models/theme.js
@@ -253,6 +253,11 @@ const Theme = RestModel.extend({
     }
   },
 
+  @discourseComputed("recentlyInstalled", "component", "hasParents")
+  warnUnassignedComponent(recent, component, hasParents) {
+    return recent && component && !hasParents;
+  },
+
   removeChildTheme(theme) {
     const childThemes = this.childThemes;
     childThemes.removeObject(theme);
diff --git a/app/assets/javascripts/admin/addon/routes/admin-customize-themes-show.js b/app/assets/javascripts/admin/addon/routes/admin-customize-themes-show.js
index 6311e6100e5..5bcf95d3d2d 100644
--- a/app/assets/javascripts/admin/addon/routes/admin-customize-themes-show.js
+++ b/app/assets/javascripts/admin/addon/routes/admin-customize-themes-show.js
@@ -3,6 +3,20 @@ import I18n from "I18n";
 import Route from "@ember/routing/route";
 import { scrollTop } from "discourse/mixins/scroll-top";
 
+export function showUnassignedComponentWarning(theme, callback) {
+  bootbox.confirm(
+    I18n.t("admin.customize.theme.unsaved_parent_themes"),
+    I18n.t("admin.customize.theme.discard"),
+    I18n.t("admin.customize.theme.stay"),
+    (result) => {
+      if (!result) {
+        theme.set("recentlyInstalled", false);
+      }
+      callback(result);
+    }
+  );
+}
+
 export default Route.extend({
   serialize(model) {
     return { theme_id: model.get("id") };
@@ -11,7 +25,7 @@ export default Route.extend({
   model(params) {
     const all = this.modelFor("adminCustomizeThemes");
     const model = all.findBy("id", parseInt(params.theme_id, 10));
-    return model ? model : this.replaceWith("adminCustomizeTheme.index");
+    return model ? model : this.replaceWith("adminCustomizeThemes.index");
   },
 
   setupController(controller, model) {
@@ -55,19 +69,13 @@ export default Route.extend({
     },
     willTransition(transition) {
       const model = this.controller.model;
-      if (model.recentlyInstalled && !model.hasParents && model.component) {
+      if (model.warnUnassignedComponent) {
         transition.abort();
-        bootbox.confirm(
-          I18n.t("admin.customize.theme.unsaved_parent_themes"),
-          I18n.t("admin.customize.theme.discard"),
-          I18n.t("admin.customize.theme.stay"),
-          (result) => {
-            if (!result) {
-              this.controller.model.setProperties({ recentlyInstalled: false });
-              transition.retry();
-            }
+        showUnassignedComponentWarning(model, (result) => {
+          if (!result) {
+            transition.retry();
           }
-        );
+        });
       }
     },
   },
diff --git a/app/assets/javascripts/admin/addon/routes/admin-customize-themes.js b/app/assets/javascripts/admin/addon/routes/admin-customize-themes.js
index 2da0266f429..9397c938a9b 100644
--- a/app/assets/javascripts/admin/addon/routes/admin-customize-themes.js
+++ b/app/assets/javascripts/admin/addon/routes/admin-customize-themes.js
@@ -1,5 +1,6 @@
 import Route from "@ember/routing/route";
 import showModal from "discourse/lib/show-modal";
+import { showUnassignedComponentWarning } from "admin/routes/admin-customize-themes-show";
 
 export default Route.extend({
   model() {
@@ -13,7 +14,17 @@ export default Route.extend({
 
   actions: {
     installModal() {
-      showModal("admin-install-theme", { admin: true });
+      const currentTheme = this.controllerFor("adminCustomizeThemes.show")
+        .model;
+      if (currentTheme && currentTheme.warnUnassignedComponent) {
+        showUnassignedComponentWarning(currentTheme, (result) => {
+          if (!result) {
+            showModal("admin-install-theme", { admin: true });
+          }
+        });
+      } else {
+        showModal("admin-install-theme", { admin: true });
+      }
     },
 
     addTheme(theme) {