From de986c3df49c0b72f36a4d3b67883dcb756bc77e Mon Sep 17 00:00:00 2001
From: Randall Morey <randall@randallmorey.com>
Date: Wed, 25 Jan 2023 12:48:23 -0800
Subject: [PATCH] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20logically=20separate?=
 =?UTF-8?q?=20save=20steps=20for=20targets=20(#1604)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* refactor: 💡 logically separate save steps for targets

* Update ui/admin/app/routes/scopes/scope/targets.js

Oops, delete actions should catch.

* chore: 🤖 rename ids var
---
 addons/core/translations/resources/en-us.yaml |  2 +-
 ui/admin/app/routes/scopes/scope/targets.js   | 87 ++++++++++++-------
 2 files changed, 59 insertions(+), 30 deletions(-)

diff --git a/addons/core/translations/resources/en-us.yaml b/addons/core/translations/resources/en-us.yaml
index 5fb976c04..c6bc36ef9 100644
--- a/addons/core/translations/resources/en-us.yaml
+++ b/addons/core/translations/resources/en-us.yaml
@@ -321,7 +321,7 @@ target:
   questions:
     delete-host-sources:
       title: Remove associated host sources?
-      message: You have {numHostSources, plural, one {# host source} other {# host sources}} associated with this target. Adding an address will remove these host sources when you save your changes.
+      message: You have {hostSourceCount, plural, one {# host source} other {# host sources}} associated with this target. Adding an address will remove these host sources when you save your changes.
   host-source:
     title: Host Source
     title_plural: Host Sources
diff --git a/ui/admin/app/routes/scopes/scope/targets.js b/ui/admin/app/routes/scopes/scope/targets.js
index 1adab4283..6f76337b3 100644
--- a/ui/admin/app/routes/scopes/scope/targets.js
+++ b/ui/admin/app/routes/scopes/scope/targets.js
@@ -50,13 +50,12 @@ export default class ScopesScopeTargetsRoute extends Route {
   }
 
   /**
-   * Handle save.
+   * Saves the target and refreshes the list.
    * @param {TargetModel} target
-   * @param {Event} e
    */
   @action
   @loading
-  @notifyError(({ message }) => message, { catch: true })
+  @notifyError(({ message }) => message)
   @notifySuccess(({ isNew }) =>
     isNew ? 'notifications.create-success' : 'notifications.save-success'
   )
@@ -70,40 +69,70 @@ export default class ScopesScopeTargetsRoute extends Route {
     this.refresh();
   }
 
+  /**
+   * Targets may have either an `address` _or_ host sources, but not both.
+   *
+   * In order to save a target with an `address`, any existing host sources
+   * must first be removed.  Once host sources are removed, the target with
+   * `address` may be saved via the standard `save` action.
+   *
+   * If `address` and host sources are both present, the user is asked to
+   * confirm that they wish to remove all host sources.  If the user declines,
+   * no changes are persisted; the form remains editable and unsaved.
+   *
+   * If `address` is set and the target has no host sources, the save proceeds
+   * as normal.
+   *
+   * If neither `address` nor host sources are set, the save proceeds as normal.
+   *
+   * @param {TargetModel} target
+   */
   @action
-  @loading
-  @notifyError(({ message }) => message)
   async saveWithAddress(target) {
-    const numHostSources = target.host_sources?.length;
-    const address = target.address;
-    if (address && numHostSources) {
-      try {
-        await this.confirm.confirm(
-          this.intl.t(
-            'resources.target.questions.delete-host-sources.message',
-            { numHostSources }
-          ),
-          {
-            title: 'resources.target.questions.delete-host-sources.title',
-            confirm: 'actions.remove-resources',
-          }
-        );
-      } catch (e) {
-        // if the user denies, do nothing and return
-        return;
-      }
-
-      await target.removeHostSources(
-        target.host_sources.map((hs) => hs.host_source_id)
-      );
-      // After saving the host sources, the model gets reset to an empty address,
-      // so we need to update the address with the previous value before saving
+    if (target.address) {
+      const { address } = target;
+      // Remove host sources if necessary.  This is cancelable by the user.
+      await this.removeHostSources(target);
+      // After removing host sources, the model is reset to an empty address,
+      // so we need to update the address with the previous value.
       target.address = address;
     }
 
+    // Proceed with standard save.
     await this.save(target);
   }
 
+  /**
+   * If the passed target has host sources:
+   *   - Request confirmation from the user for host source removal.
+   *   - Persist removal of all host sources.
+   * @param {TargetModel} target
+   */
+  @action
+  @loading
+  @notifyError(({ message }) => message)
+  async removeHostSources(target) {
+    const hostSourceCount = target.host_sources?.length;
+
+    if (hostSourceCount) {
+      const hostSourceIDs = target.host_sources.map(
+        ({ host_source_id }) => host_source_id
+      );
+      const confirmMessage = this.intl.t(
+        'resources.target.questions.delete-host-sources.message',
+        { hostSourceCount }
+      );
+
+      // Ask for confirmation
+      await this.confirm.confirm(confirmMessage, {
+        title: 'resources.target.questions.delete-host-sources.title',
+        confirm: 'actions.remove-resources',
+      });
+      // Remove host sources.  This step is reached only if the user accepts.
+      await target.removeHostSources(hostSourceIDs);
+    }
+  }
+
   /**
    * Deletes a target and redirects to targets index.
    * @param {TargetModel} target