Forestall accidental deletion of ordered lineitems from POs

Surely you'd hardly ever (never?) want to delete an ordered lineitem
from a PO.  In practice it seems that users are doing this by accident
and/or through lack of intuitiveness in the Acq interfaces.

Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.com>

diff --git a/Open-ILS/web/js/dojo/openils/acq/nls/acq.js b/Open-ILS/web/js/dojo/openils/acq/nls/acq.js
index 040bec3..2d89f12 100644 (file)
--- a/Open-ILS/web/js/dojo/openils/acq/nls/acq.js
+++ b/Open-ILS/web/js/dojo/openils/acq/nls/acq.js
@@ -77,5 +77,6 @@
     "NUM_CLAIMS_EXISTING" : "Claims (${0} existing)",
     "LOAD_TERMS_FIRST" : "You can't retrieve records until you've loaded a CSV file\nwith bibliographic IDs in the first column.",
     "SELECT_SEARCH_FIELD": "Select Search Field",
-    "LIBRARY_INITIATED": "Library Initiated"
+    "LIBRARY_INITIATED": "Library Initiated",
+    "DEL_LI_FROM_PO": "That item has already been ordered!  Deleting it now will not revoke or modify any order that has been placed with a vendor.  Deleting the item may put the system's idea of your purchase order in a state that is inconsistent with reality.  Are you sure you mean to do this?"
 }

diff --git a/Open-ILS/web/js/ui/default/acq/common/li_table.js b/Open-ILS/web/js/ui/default/acq/common/li_table.js
index 4068294..a7cafc8 100644 (file)
--- a/Open-ILS/web/js/ui/default/acq/common/li_table.js
+++ b/Open-ILS/web/js/ui/default/acq/common/li_table.js
@@ -771,6 +771,25 @@ function AcqLiTable() {
     }
 
     this.removeLineitem = function(liId) {
+        if (this.isPO && (
+            this.liCache[liId].state() == "on-order" ||
+            this.liCache[liId].state() == "received"
+        )) {
+            /* It makes little sense to delete a lineitem from a PO that has
+             * already been marked 'on-order'.  Especially if EDI is in use,
+             * such a purchase order will probably have already been shipped
+             * off to a vendor, and mucking with it at this point could leave
+             * your data in a bad state that doesn't jive with reality.
+             *
+             * I could see making this restriction even firmer.
+             *
+             * I could also see adjusting the li state comparisons, extending
+             * the comparison to the PO's state, and/or providing functions
+             * that house the logic for comparing states in a single location.
+             */
+            if (!confirm(localeStrings.DEL_LI_FROM_PO)) return;
+        }
+
         this.tbody.removeChild(dojo.query('[li='+liId+']', this.tbody)[0]);
         delete this.liCache[liId];
         //selected.push(self.liCache[i.parentNode.parentNode.getAttribute('li')]);