From 9332a68d06cb6caa0af3f560d5acfaaf6b716be7 Mon Sep 17 00:00:00 2001 From: Steven Chan Date: Wed, 18 Jul 2012 11:22:14 -0700 Subject: [PATCH] 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 Signed-off-by: Lebbeous Fogle-Weekley --- .../web/js/dojo/openils/widget/EditPane.js | 16 +++++ .../web/js/ui/default/acq/invoice/receive.js | 2 +- .../web/js/ui/default/acq/invoice/view.js | 64 ++++++++++--------- 3 files changed, 52 insertions(+), 30 deletions(-) diff --git a/Open-ILS/web/js/dojo/openils/widget/EditPane.js b/Open-ILS/web/js/dojo/openils/widget/EditPane.js index 1fd41adb75..d7c54b8375 100644 --- 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 d414676bc1..1ed65be7c3 100644 --- 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 7f2a2ddfac..2e8f499eeb 100644 --- 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'], { -- 2.43.2