refactor: 💡 logically separate save steps for targets (#1604)

* 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

@@ -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

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;
+    if (target.address) {
-    const address = target.address;
+      const { address } = target;
-    if (address && numHostSources) {
+      // Remove host sources if necessary.  This is cancelable by the user.
-      try {
+      await this.removeHostSources(target);
-        await this.confirm.confirm(
+      // After removing host sources, the model is reset to an empty address,
-          this.intl.t(
+      // so we need to update the address with the previous value.
-            '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
       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