From c17285cdd9961e6ea23848a6b20e090034d2d237 Mon Sep 17 00:00:00 2001
From: Jeff Hitchcock <mail@command-center.org>
Date: Sat, 27 Aug 2022 08:51:04 -0700
Subject: [PATCH] [#1626, 1689] Adjust how some drop errors are handled, remove
 development logging

---
 module/advancement/advancement-config.mjs   | 22 ++++++++++-----------
 module/advancement/types/item-grant.mjs     |  5 ++---
 templates/advancement/item-grant-config.hbs |  2 +-
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/module/advancement/advancement-config.mjs b/module/advancement/advancement-config.mjs
index 8211ca61ba..c163524168 100644
--- a/module/advancement/advancement-config.mjs
+++ b/module/advancement/advancement-config.mjs
@@ -84,7 +84,7 @@ export default class AdvancementConfig extends FormApplication {
     super.activateListeners(html);
 
     // Remove an item from the list
-    if ( this.options.dropKeyPath ) html.on("click", ".item-delete", this._onItemDelete.bind(this));
+    if ( this.options.dropKeyPath ) html.on("click", "[data-action='delete']", this._onItemDelete.bind(this));
   }
 
   /* -------------------------------------------- */
@@ -132,7 +132,6 @@ export default class AdvancementConfig extends FormApplication {
     const updates = { configuration: await this.prepareConfigurationUpdate({
       [this.options.dropKeyPath]: items.filter(uuid => uuid !== uuidToDelete)
     }) };
-    console.log(updates);
     await this.advancement.update(updates);
     this.render();
   }
@@ -148,7 +147,7 @@ export default class AdvancementConfig extends FormApplication {
 
   /** @inheritdoc */
   async _onDrop(event) {
-    if ( !this.options.dropKeyPath ) return console.error(
+    if ( !this.options.dropKeyPath ) throw new Error(
       "AdvancementConfig#options.dropKeyPath must be configured or #_onDrop must be overridden to support"
       + " drag and drop on advancement config items."
     );
@@ -164,14 +163,17 @@ export default class AdvancementConfig extends FormApplication {
     if ( data.type !== "Item" ) return false;
     const item = await Item.implementation.fromDropData(data);
 
-    const verified = this._verifyDroppedItem(event, item);
-    if ( !verified ) return false;
+    try {
+      this._validateDroppedItem(event, item);
+    } catch(err) {
+      return ui.notifications.error(err.message);
+    }
 
     const existingItems = foundry.utils.getProperty(this.advancement.data.configuration, this.options.dropKeyPath);
 
     // Abort if this uuid is the parent item
     if ( item.uuid === this.item.uuid ) {
-      return ui.notifications.warn(game.i18n.localize("DND5E.AdvancementItemGrantRecursiveWarning"));
+      return ui.notifications.error(game.i18n.localize("DND5E.AdvancementItemGrantRecursiveWarning"));
     }
 
     // Abort if this uuid exists already
@@ -186,14 +188,12 @@ export default class AdvancementConfig extends FormApplication {
   /* -------------------------------------------- */
 
   /**
-   * Called when an item is dropped to verify the Item before it is saved.
+   * Called when an item is dropped to validate the Item before it is saved. An error should be thrown
+   * if the item is invalid.
    * @param {Event} event  Triggering drop event.
    * @param {Item5e} item  The materialized Item that was dropped.
-   * @returns {boolean}    Is the dropped Item valid?
    * @protected
    */
-  _verifyDroppedItem(event, item) {
-    return true;
-  }
+  _validateDroppedItem(event, item) {}
 
 }
diff --git a/module/advancement/types/item-grant.mjs b/module/advancement/types/item-grant.mjs
index acd648904b..1e28d36340 100644
--- a/module/advancement/types/item-grant.mjs
+++ b/module/advancement/types/item-grant.mjs
@@ -161,11 +161,10 @@ export class ItemGrantConfig extends AdvancementConfig {
   /* -------------------------------------------- */
 
   /** @inheritdoc */
-  _verifyDroppedItem(event, item) {
+  _validateDroppedItem(event, item) {
     if ( this.advancement.constructor.VALID_TYPES.has(item.type) ) return true;
     const type = game.i18n.localize(`ITEM.Type${item.type.capitalize()}`);
-    ui.notifications.warn(game.i18n.format("DND5E.AdvancementItemTypeInvalidWarning", { type }));
-    return false;
+    throw new Error(game.i18n.format("DND5E.AdvancementItemTypeInvalidWarning", { type }));
   }
 }
 
diff --git a/templates/advancement/item-grant-config.hbs b/templates/advancement/item-grant-config.hbs
index abf5a08216..f520ade670 100644
--- a/templates/advancement/item-grant-config.hbs
+++ b/templates/advancement/item-grant-config.hbs
@@ -20,7 +20,7 @@
         <li class="item flexrow" data-item-uuid="{{this}}">
           <div class="item-name">{{{dnd5e-linkForUuid this}}}</div>
           <div class="item-controls flexrow">
-            <a class="item-control item-delete" title="{{localize 'DND5E.ItemDelete'}}">
+            <a class="item-control item-action" data-action="delete" title="{{localize 'DND5E.ItemDelete'}}">
               <i class="fas fa-trash"></i>
             </a>
           </div>