Fix LP800480, ACQ - Vendor Invoice Won't Save

1. Upon submitting the form, the client prepares two auxiliary data
objects containing service charges and line items. The two objects are
normally 'fleshed', ie, contain references to other data objects, but in
the versions sent to the server, they must be 'unfleshed', ie, the
referent objects are converted back to ID values. If the user resubmits
the form, the client should not unflesh again. However, the software for
preparing line items contain lines of code that unfleshes without
checking if the objects are already unflesh. When unfleshing a second
time, we get an uncaught reference error. Ironically, the software also
contains lines of code to do it correctly, so the errant lines of code
are also duplicates.

To fix problem 1, we remove the duplicate errant lines of code. We
define a helper unflesh() method and use it to replace the current lines
of code to unflesh.

2. When the user submits the form without filling in required data
fields in the invoice, including Vendor Invoice ID, the client does not
validate before making a service request. The server tries to complete
the database transaction, but gets an error.  When the response comes
back, the client shows the same form so that the user can retry.
However, the message alert to the user is not informative; it indicates
an error at the database level, but does not indicate the probable
reason.

To fix problem 2, We move the lines of code preparing the invoice object
earlier in the sequence.  We define a helper method mapValues() to
prepare the invoice object from the values in the UI widget object.
mapValues() will return an error object if it detects that required
values are null. We check for the error object, and will show an alert
message to the user and abort the submit operation early, allowing the
user to retry.

3. When an invoice with line items is saved, the invoice is re-rendered
with new buttons to allow receiving. It also contains a new button to
enable the user to view the line items in list format. However, it does
not work in a non-Firefox browser, because a debug statement using the
'dump()' function is left uncommented. (Inspection of all other
appearances of dump() show they are all commented out.) There is an
uncaught reference because dump() is not found.

Signed-off-by: James Fournie <jfournie@sitka.bclibraries.ca>
Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.com>

diff --git a/Open-ILS/web/js/dojo/openils/widget/EditPane.js b/Open-ILS/web/js/dojo/openils/widget/EditPane.js
index 1fd41ad..d7c54b8 100644 (file)
--- a/Open-ILS/web/js/dojo/openils/widget/EditPane.js
+++ b/Open-ILS/web/js/dojo/openils/widget/EditPane.js
@@ -206,6 +206,22 @@ if(!dojo._hasResource['openils.widget.EditPane']) {
                 return this.fieldList.map(function(a) { return a.name });
             },
 
+            // Apply a function for the name and formatted value of each field
+            // in this edit pane.  If any required value is null, then return
+            // an error object.
+            mapValues: function (fn) {
+                var e = 0, msg = this.fmIDL.label + ' ';
+                dojo.forEach(this.fieldList, function (f) {
+                    var v, w = f.widget;
+                    if ((v = w.getFormattedValue()) === null && w.isRequired()) { e++; }
+                    fn(f.name, v);
+                });
+                if (e > 0) {
+                    msg += 'edit pane has ' + e + ' required field(s) that contain no value(s)';
+                    return new Error(msg);
+                }
+            },
+
             getFieldValue : function(field) {
                 for(var i in this.fieldList) {
                     if(field == this.fieldList[i].name) {

diff --git a/Open-ILS/web/js/ui/default/acq/invoice/receive.js b/Open-ILS/web/js/ui/default/acq/invoice/receive.js
index d414676..1ed65be 100644 (file)
--- a/Open-ILS/web/js/ui/default/acq/invoice/receive.js
+++ b/Open-ILS/web/js/ui/default/acq/invoice/receive.js
@@ -161,7 +161,7 @@ function ReceivableCopyTable() {
     this._add_lineitem_list_mode = function(details, li, preselect_count) {
         details.forEach(
             function(lid) {
-                dump("preselect_count "+ preselect_count+"\n");
+                //dump("preselect_count "+ preselect_count+"\n");
                 self.add_lineitem_detail(
                     lid, li, Boolean(preselect_count-- > 0)
                 );

diff --git a/Open-ILS/web/js/ui/default/acq/invoice/view.js b/Open-ILS/web/js/ui/default/acq/invoice/view.js
index 7f2a2dd..2e8f499 100644 (file)
--- a/Open-ILS/web/js/ui/default/acq/invoice/view.js
+++ b/Open-ILS/web/js/ui/default/acq/invoice/view.js
@@ -609,16 +609,45 @@ function saveChanges(doProrate, doClose, doReopen) {
     );
 }
 
+// Define a helper function to 'unflesh' sub-objects from an fmclass object.
+// 'this' specifies the object; the arguments specify a list of names of
+// sub-objects.
+function unflesh() {
+    var _, $ = this;
+    dojo.forEach(arguments, function (n) {
+        _ = $[n]();
+        if (_ !== null && typeof _ === 'object')
+            $[n]( _.id() );
+    });
+}
+
 function saveChangesPartTwo(doProrate, doClose, doReopen) {
     
-    progressDialog.show(true);
 
     if(doReopen) {
         invoice.complete('f');
 
     } else {
 
+        // Prepare an invoice for submission
+        if(!invoice) {
+            invoice = new fieldmapper.acqinv();
+            invoice.isnew(true);
+        } else {
+            invoice.ischanged(true); // for now, just always update
+        }
+
+        var e = invoicePane.mapValues(function (n, v) { invoice[n](v); });
+        if (e instanceof Error) {
+            alert(e.message);
+            return;
+        }
+
+        if(doClose)
+            invoice.complete('t');
+
 
+        // Prepare any charge items
         var updateItems = [];
         for(var id in widgetRegistry.acqii) {
             var reg = widgetRegistry.acqii[id];
@@ -631,19 +660,17 @@ function saveChangesPartTwo(doProrate, doClose, doReopen) {
                         item[field]( reg[field].getFormattedValue() );
                 }
                 
-                // unflesh
-                if(item.purchase_order() != null && typeof item.purchase_order() == 'object')
-                    item.purchase_order( item.purchase_order().id() );
+                unflesh.call(item, 'purchase_order');
+
             }
         }
 
+        // Prepare any line items
         var updateEntries = [];
         for(var id in widgetRegistry.acqie) {
             var reg = widgetRegistry.acqie[id];
             var entry = reg._object;
             if(entry.ischanged() || entry.isnew() || entry.isdeleted()) {
-                entry.lineitem(entry.lineitem().id());
-                entry.purchase_order(entry.purchase_order().id());
                 updateEntries.push(entry);
                 if(entry.isnew()) entry.id(null);
 
@@ -652,33 +679,12 @@ function saveChangesPartTwo(doProrate, doClose, doReopen) {
                         entry[field]( reg[field].getFormattedValue() );
                 }
                 
-                // unflesh
-                dojo.forEach(['purchase_order', 'lineitem'],
-                    function(field) {
-                        if(entry[field]() != null && typeof entry[field]() == 'object')
-                            entry[field]( entry[field]().id() );
-                    }
-                );
+                unflesh.call(entry, 'purchase_order', 'lineitem');
             }
         }
-
-        if(!invoice) {
-            invoice = new fieldmapper.acqinv();
-            invoice.isnew(true);
-        } else {
-            invoice.ischanged(true); // for now, just always update
-        }
-
-        dojo.forEach(invoicePane.fieldList, 
-            function(field) {
-                invoice[field.name]( field.widget.getFormattedValue() );
-            }
-        );
-
-        if(doClose) 
-            invoice.complete('t');
     }
 
+    progressDialog.show(true);
     fieldmapper.standardRequest(
         ['open-ils.acq', 'open-ils.acq.invoice.update'],
         {