Bug 7178 - Improve order item creation
Summary: Improve order item creation
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low normal (vote)
Assignee: Julian Maurice
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
: 7173 (view as bug list)
Depends on: 8135
Blocks: 7163 8684 21182
  Show dependency treegraph
 
Reported: 2011-11-06 04:54 UTC by Julian Maurice
Modified: 2018-08-08 17:57 UTC (History)
7 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
neworderempty.pl (37.42 KB, image/png)
2011-11-06 04:54 UTC, Julian Maurice
Details
Bug 7178: Acquisition item creation improvement (37.02 KB, patch)
2012-01-19 16:09 UTC, Julian Maurice
Details | Diff | Splinter Review
no item form when ordering (20.82 KB, image/png)
2012-01-22 19:35 UTC, Katrin Fischer
Details
7249: Bug 7178: Acquisition item creation improvement (38.87 KB, patch)
2012-02-03 15:56 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7178: Acquisition item creation improvement (45.96 KB, patch)
2012-02-22 15:49 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7178: Acquisition item creation improvement (45.74 KB, patch)
2012-02-22 16:04 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7178: Acquisition item creation improvement (45.85 KB, patch)
2012-02-26 16:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
no item block quantity=0 (199.14 KB, image/jpeg)
2012-03-01 12:46 UTC, Marcel de Rooy
Details
Bug 7178: Acquisition item creation improvement (46.01 KB, patch)
2012-03-23 09:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Follow-up Improve order item creation (3.03 KB, patch)
2012-03-28 15:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Follow-up Improve order item creation (45.74 KB, patch)
2012-03-30 12:08 UTC, Julian Maurice
Details | Diff | Splinter Review
Follow-up Improve order item creation (5.94 KB, patch)
2012-04-04 07:13 UTC, Julian Maurice
Details | Diff | Splinter Review
[PASSED-QA][SIGNED-OFF] Bug 7178: Follow-up Improve order item creation (6.17 KB, patch)
2012-04-04 19:57 UTC, Katrin Fischer
Details | Diff | Splinter Review
Patch clearing a warning (1.06 KB, patch)
2012-05-11 12:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 7178 follow-up: Fix check_additem when UniqueItemFields is empty (1.21 KB, patch)
2012-05-11 14:43 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7178 follow-up: Do not fill items table with "(undefined)" (904 bytes, patch)
2012-05-22 08:51 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2011-11-06 04:54:06 UTC
Created attachment 6233 [details]
neworderempty.pl

This development change the behaviour of item creation while ordering or receiving. Only one item block is displayed at a time, and the others are "minimized" in a table row. Moreover, adding a new item no longer clone the html block, but it call PrepareItemRecordDisplay via ajax, so the fields ids are different and cataloguing plugins can be used on these fields.
Only items shown in items list will be created, so it requires to click the '+' button at least one time, even for creating a single item.
See screenshot attached to see what it looks like.

BibLibre infos:
  Repository: git://git.biblibre.com/koha_biblibre.git
  Branch:     stetienne/ft/MT6523
Comment 1 Julian Maurice 2012-01-19 16:09:34 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2012-01-22 19:26:58 UTC
Hi Julian, 

I did some tests and looked at the code. I found some things:

1) File name an function of check_unicity.pl
create mode 100755 acqui/check_unicity.pl
I think perhaps it should be check_uniqueness? 
How does this work? Can you add some notes about this feature?
I see it includes copynumber. Is it configurable which fields will be checked for uniqueness? 
Barcode should always be unique. But for stocknumber and copynumber I think it depends on the library.


2) Untranslatable strings
koha-tmpl/intranet-tmpl/prog/en/js/additem.js contains some strings. In order to be translatable those must be moved into the template, some examples:
+            alert(_("You can't receive any more items."));
+    var show_link = "<a href='#items' onclick='showItem(\"" + index + "\");'>Show</a>";
+    var del_link = "<a href='#' onclick='deleteItemBlock(this, \"" + index + "\");'>Delete</a>";
+            var buttonClear = '<a name="buttonClear" style="cursor:pointer; color:grey; font-size:180%" onclick="clearItemBlock(this)">Clear</a>';


3) Typo? koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
+    [% END %][%# IF (AcqCreateItemOrdering) %]


4) I have set a see also to bug 7175 - I think both features are related affecting different settings of AcqCreateItem and item creation.


5) AcqCreateItem set to 'on order'
The item form does not show for me at all. I have an empty 'item' section under 'catalog details'. FireBug shows a javascript error:
var clone = original.cloneNode(true);	

Failing QA here.
Comment 3 Katrin Fischer 2012-01-22 19:35:15 UTC Comment hidden (obsolete)
Comment 4 Julian Maurice 2012-02-03 15:56:29 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2012-02-12 21:13:58 UTC
Hi Julian, 

thank you for the new patch.

About 5) - I still have problems here. Sometimes the item form appears after I hit F5 various times. When I don't add an item and click on save, the item form is gone again. F5 brings it back. Something still seems to go wrong here.

I see some other problems with the new interface:
 
a) I am missing some information in the item table:
- itemtype
- collection
- the not for loan value (often set to 'on order')

I think ideally the table should display all fields I filled out in the item form. Or alternatively all fields that are not hidden in the item framework (default or ACQ).

b) The item table should display the descriptions, not the codes.

c) I fill out the item form, click '+'. Next I click 'Show' and I can make changes. But it's still '+'. If I click it once - it will make my changes. But when I click it a second time, it will add a dupliate of the item. It's not clear when I change an existing item on the record and when I am adding a new item. It would be good if the link showed different labels for 'updating' an item and 'adding' a completely new item ot the order.

d) The form says + instead of 'Add' at the end of the form (says Add on master now) and the font is very big.

Not sure this is enough for a 'fail qa', but don't feel comfortable signing off for the points raised above.
Comment 6 Jonathan Druart 2012-02-17 08:26:37 UTC
*** Bug 7173 has been marked as a duplicate of this bug. ***
Comment 7 Paul Poulain 2012-02-17 14:39:05 UTC
The script devel/misc/testbugzillapatches.pl has detected that this bug does not apply anymore, so i'm marking this bug as "does not apply".

The script is supposed to handle conflicts related to updatedatabase, but no other cases of merge conflict. So the conflict may be trivial to fix.

If it happens that the patch attached does apply cleanly, please send me an email, there's something wrong in the script. It's possible, as it's a recent script.
If the patch applies does not apply as I think, please rebase and resubmit, thanks
Comment 8 Julian Maurice 2012-02-22 15:49:42 UTC Comment hidden (obsolete)
Comment 9 Julian Maurice 2012-02-22 16:04:05 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2012-02-26 16:37:44 UTC
Hi Julian, 

another round of testing :)

1) Items created 'on order'

Items table 1: I like the changes you made for the labels 'Edit', 'Update' and 'Add' works nicely for me. The The item table has a lot more information and shows correct descriptions for authorised values, branches an itemtypes.

Items table 2: Suggestion: Make the table so that it only shows columns that were used for at least one item, like we do for the item forms in OPAC and staff catalog.

Items table 3: Suggestion: Rename Stock number to 'Inventory number' to match the English description in the frameworks.

Duplication check 1: Suggestion: I added duplicate barcodes and on saving the order I got an error message. The message does not contain any information about the field that contained the duplicate. It would be cool if it would show the field and duplicate value. So when you created a lot of items, you know where to look.

Duplication check 2: Suggestion: First time around I missed the message for the duplicate barcode on top of the screen. Could we move that between the item table and the item form? I think it make it more visible when you are working with a lot of items and a longer item form.

UniqueItemFields 1: Suggestion: We are using | or commas for most system preferences which require a list of values. This adds a new variant 'whitespace'. Not sure it's worth changing, but wanted to add a note. 

UniqueItemFields 2: OK :) I added stocknumber to the list of fields, because I wanted to enforce duplicate detection. Entry was: 'barcode stocknumber'. I created a new order line adding the same inventory number for 2 items. On saving I got an error message for stocknumbers. When adding more than one item the field is cleared each time. Works nicely.

Item creation 1: OK :) I checked items were created correctly for each of my items. Works nicely.

Receiving items 1: Works nicely. Item form is not shown there (yet).

2) Items created 'on receive'

Items table 2 and Items table 3 suggestions also apply here.

Duplication check error message is more visible here, but could perhaps be moved down too - not sure.

Item creation 2: The form does not allow to enter more items than were ordered. I am not sure if this is a bug or 'by design'. I think it would cause no problems for 'on receive' while it is problematic for 'on order'.

Receiving items 2: OK :) Items were created correctly and partial receive works too.

Great work!
Comment 11 Katrin Fischer 2012-02-26 16:39:45 UTC Comment hidden (obsolete)
Comment 12 Marcel de Rooy 2012-03-01 12:46:55 UTC Comment hidden (obsolete)
Comment 13 Marcel de Rooy 2012-03-01 12:51:15 UTC
Julian look at this: 

                    [% IF (AcqCreateItemOrdering) %]
                        <input type="text" readonly="readonly" size="20" id="quantity" name="quantity" value="0" onchange="updateCosts();" />
                    [% ELSE %]
                        <input type="text" size="20" id="quantity" name="quantity" value="[% quantityrec %]" onchange="calcNeworderTotal();" />
                    [% END %]
  


readonly +0 is my problem
Comment 14 Marcel de Rooy 2012-03-01 13:22:37 UTC
Errors on master

[14:19:13.432] GET http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=5&basketno=527 [HTTP/1.1 200 OK 1872ms]
[14:19:15.326] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/ui.tabs.css [HTTP/1.1 200 OK 15ms]
[14:19:15.330] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/print.css [HTTP/1.1 200 OK 15ms]
[14:19:15.332] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css [HTTP/1.1 200 OK 31ms]
[14:19:15.334] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/jquery.js [HTTP/1.1 200 OK 31ms]
[14:19:15.336] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/ui.tabs.js [HTTP/1.1 200 OK 15ms]
[14:19:15.338] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/jquery.hotkeys.min.js [HTTP/1.1 200 OK 15ms]
[14:19:15.341] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/jquery.cookie.min.js [HTTP/1.1 200 OK 15ms]
[14:19:15.343] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/jquery.highlight-3.js [HTTP/1.1 200 OK 15ms]
[14:19:15.345] GET http://yui.yahooapis.com/2.5.1/build/utilities/utilities.js [HTTP/1.1 200 OK 62ms]
[14:19:15.347] GET http://yui.yahooapis.com/2.5.1/build/datasource/datasource-min.js [HTTP/1.1 404 Not Found 249ms]
[14:19:15.350] GET http://yui.yahooapis.com/2.5.1/build/autocomplete/autocomplete-min.js [HTTP/1.1 200 OK 47ms]
[14:19:15.352] GET http://yui.yahooapis.com/2.5.1/build/button/button-min.js [HTTP/1.1 200 OK 93ms]
[14:19:15.354] GET http://yui.yahooapis.com/2.5.1/build/container/container_core-min.js [HTTP/1.1 200 OK 47ms]
[14:19:15.356] GET http://yui.yahooapis.com/2.5.1/build/menu/menu-min.js [HTTP/1.1 200 OK 109ms]
[14:19:15.360] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/js/staff-global.js [HTTP/1.1 200 OK 47ms]
[14:19:15.363] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/js/basket.js [HTTP/1.1 200 OK 47ms]
[14:19:15.366] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/js/acq.js [HTTP/1.1 200 OK 62ms]
[14:19:15.368] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/js/additem.js [HTTP/1.1 200 OK 62ms]
[14:19:15.481] ‘html’ gevonden waar ‘,’ of ‘{’ verwacht.  Regelset genegeerd vanwege foute selector. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:475
[14:19:15.485] Fout tijdens het parsen van waarde voor ‘padding-left’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:482
[14:19:15.488] Fout tijdens het parsen van waarde voor ‘background’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1206
[14:19:15.495] Fout tijdens het parsen van waarde voor ‘filter’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1210
[14:19:15.499] Fout tijdens het parsen van waarde voor ‘background-position’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1221
[14:19:15.507] Fout tijdens het parsen van waarde voor ‘background-image’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1268
[14:19:15.520] Onbekende eigenschap ‘-moz-border-top-left-radius’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1336
[14:19:15.523] Onbekende eigenschap ‘-moz-border-bottom-left-radius’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1338
[14:19:15.537] Fout tijdens het parsen van waarde voor ‘padding’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1889
[14:19:15.542] Fout tijdens het parsen van waarde voor ‘filter’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/ui.tabs.css:63
[14:19:15.547] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/reset-fonts-grids.css [HTTP/1.1 200 OK 0ms]
[14:19:15.550] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css [HTTP/1.1 200 OK 32ms]
[14:19:15.563] Fout tijdens het parsen van waarde voor ‘line-height’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/reset-fonts-grids.css:7
[14:19:15.566] ‘*’ gevonden waar declaratie verwacht.  Naar volgende declaratie gesprongen. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/reset-fonts-grids.css:8
[14:19:15.570] Fout tijdens het parsen van waarde voor ‘font’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/reset-fonts-grids.css:8
[14:19:15.602] Onbekende eigenschap ‘zoom’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/reset-fonts-grids.css:9
[14:19:15.604] Onbekende eigenschap ‘-moz-opacity’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:7
[14:19:15.607] Fout tijdens het parsen van waarde voor ‘filter’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:7
[14:19:15.609] ‘*’ gevonden waar declaratie verwacht.  Naar volgende declaratie gesprongen. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:8
[14:19:15.619] Onbekende eigenschap ‘zoom’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:9
[14:19:15.624] Onbekende eigenschap ‘-moz-outline’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:10
[14:19:15.627] ‘html’ gevonden waar ‘,’ of ‘{’ verwacht.  Regelset genegeerd vanwege foute selector. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:10
[14:19:15.647] ‘right’ gevonden waar einde van waarde verwacht.  Fout tijdens het parsen van waarde voor ‘display’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:13
[14:19:15.675] Fout tijdens het parsen van waarde voor ‘white-space’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:16
[14:19:15.688] Fout tijdens het parsen van waarde voor ‘font’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:18
[14:19:15.734] Fout tijdens het parsen van waarde voor ‘padding’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:21
[14:19:15.741] Onbekende eigenschap ‘Xposition’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:21
[14:19:15.745] Fout tijdens het parsen van waarde voor ‘cursor’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:21
[14:19:15.787] GET http://yui.yahooapis.com/2.5.1/build/datasource/datasource-min.js [HTTP/1.1 404 Not Found 187ms]
[14:19:16.089] Lege string doorgegeven aan getElementById(). @ http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=5&basketno=527
[14:19:16.093] document.getElementById("") is null @ http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=5&basketno=527:781
[14:19:16.096] Lege string doorgegeven aan getElementById(). @ http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=5&basketno=527
[14:19:16.098] document.getElementById("") is null @ http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=5&basketno=527:928
[14:19:16.135] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/img/cart-small.gif [HTTP/1.1 200 OK 16ms]
[14:19:16.138] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/img/more-arrow.gif [HTTP/1.1 200 OK 16ms]
[14:19:16.141] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/img/koha-logo-medium.gif [HTTP/1.1 200 OK 16ms]


Errors for patch:

[14:21:41.034] GET http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=5&basketno=527 [HTTP/1.1 200 OK 1748ms]
[14:21:42.826] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/ui.tabs.css [HTTP/1.1 200 OK 15ms]
[14:21:42.830] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/print.css [HTTP/1.1 200 OK 16ms]
[14:21:42.832] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css [HTTP/1.1 200 OK 32ms]
[14:21:42.835] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/jquery.js [HTTP/1.1 200 OK 47ms]
[14:21:42.837] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/ui.tabs.js [HTTP/1.1 200 OK 32ms]
[14:21:42.840] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/jquery.hotkeys.min.js [HTTP/1.1 200 OK 16ms]
[14:21:42.843] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/jquery.cookie.min.js [HTTP/1.1 200 OK 16ms]
[14:21:42.845] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/jquery.highlight-3.js [HTTP/1.1 200 OK 16ms]
[14:21:42.848] GET http://yui.yahooapis.com/2.5.1/build/utilities/utilities.js [HTTP/1.1 200 OK 125ms]
[14:21:42.850] GET http://yui.yahooapis.com/2.5.1/build/datasource/datasource-min.js [HTTP/1.1 404 Not Found 47ms]
[14:21:42.852] GET http://yui.yahooapis.com/2.5.1/build/autocomplete/autocomplete-min.js [HTTP/1.1 200 OK 125ms]
[14:21:42.854] GET http://yui.yahooapis.com/2.5.1/build/button/button-min.js [HTTP/1.1 200 OK 94ms]
[14:21:42.856] GET http://yui.yahooapis.com/2.5.1/build/container/container_core-min.js [HTTP/1.1 200 OK 110ms]
[14:21:42.859] GET http://yui.yahooapis.com/2.5.1/build/menu/menu-min.js [HTTP/1.1 200 OK 125ms]
[14:21:42.861] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/js/staff-global.js [HTTP/1.1 200 OK 47ms]
[14:21:42.864] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/js/basket.js [HTTP/1.1 200 OK 47ms]
[14:21:42.867] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/js/acq.js [HTTP/1.1 200 OK 63ms]
[14:21:42.869] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/js/additem.js [HTTP/1.1 200 OK 63ms]
[14:21:42.911] Fout tijdens het parsen van waarde voor ‘filter’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/jquery/plugins/ui.tabs.css:63
[14:21:42.926] ‘html’ gevonden waar ‘,’ of ‘{’ verwacht.  Regelset genegeerd vanwege foute selector. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:475
[14:21:42.929] Fout tijdens het parsen van waarde voor ‘padding-left’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:482
[14:21:42.931] Fout tijdens het parsen van waarde voor ‘background’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1206
[14:21:42.936] Fout tijdens het parsen van waarde voor ‘filter’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1210
[14:21:42.940] Fout tijdens het parsen van waarde voor ‘background-position’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1221
[14:21:42.948] Fout tijdens het parsen van waarde voor ‘background-image’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1268
[14:21:42.961] Onbekende eigenschap ‘-moz-border-top-left-radius’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1336
[14:21:42.963] Onbekende eigenschap ‘-moz-border-bottom-left-radius’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1338
[14:21:42.977] Fout tijdens het parsen van waarde voor ‘padding’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/css/staff-global.css:1889
[14:21:43.049] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/reset-fonts-grids.css [HTTP/1.1 200 OK 16ms]
[14:21:43.054] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css [HTTP/1.1 200 OK 15ms]
[14:21:43.071] Fout tijdens het parsen van waarde voor ‘line-height’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/reset-fonts-grids.css:7
[14:21:43.074] ‘*’ gevonden waar declaratie verwacht.  Naar volgende declaratie gesprongen. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/reset-fonts-grids.css:8
[14:21:43.077] Fout tijdens het parsen van waarde voor ‘font’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/reset-fonts-grids.css:8
[14:21:43.109] Onbekende eigenschap ‘zoom’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/reset-fonts-grids.css:9
[14:21:43.111] Onbekende eigenschap ‘-moz-opacity’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:7
[14:21:43.114] Fout tijdens het parsen van waarde voor ‘filter’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:7
[14:21:43.116] ‘*’ gevonden waar declaratie verwacht.  Naar volgende declaratie gesprongen. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:8
[14:21:43.126] Onbekende eigenschap ‘zoom’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:9
[14:21:43.131] Onbekende eigenschap ‘-moz-outline’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:10
[14:21:43.134] ‘html’ gevonden waar ‘,’ of ‘{’ verwacht.  Regelset genegeerd vanwege foute selector. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:10
[14:21:43.154] ‘right’ gevonden waar einde van waarde verwacht.  Fout tijdens het parsen van waarde voor ‘display’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:13
[14:21:43.183] Fout tijdens het parsen van waarde voor ‘white-space’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:16
[14:21:43.196] Fout tijdens het parsen van waarde voor ‘font’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:18
[14:21:43.242] Fout tijdens het parsen van waarde voor ‘padding’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:21
[14:21:43.249] Onbekende eigenschap ‘Xposition’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:21
[14:21:43.253] Fout tijdens het parsen van waarde voor ‘cursor’.  Declaratie genegeerd. @ http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/en/lib/yui/skin.css:21
[14:21:43.294] GET http://yui.yahooapis.com/2.5.1/build/datasource/datasource-min.js [HTTP/1.1 404 Not Found 203ms]
[14:21:43.621] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/img/cart-small.gif [HTTP/1.1 200 OK 16ms]
[14:21:43.624] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/img/more-arrow.gif [HTTP/1.1 200 OK 16ms]
[14:21:43.626] GET http://libdevelop.rijksmuseum.nl:8009/intranet-tmpl/prog/img/koha-logo-medium.gif [HTTP/1.1 200 OK 16ms]
[14:21:43.632] GET http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/services/itemrecorddisplay.pl?frameworkcode=ACQ [HTTP/1.1 500 Internal Server Error 1279ms]
Comment 15 Marcel de Rooy 2012-03-01 13:41:09 UTC
Got rid of a selinux error already.
Still have two perl warnings:

[Thu Mar 01 14:35:23 2012] [error] [client 192.87.126.61] [Thu Mar  1 14:35:23 2012] neworderempty.pl: Use of uninitialized value in sprintf at /usr/share/koha/testclone/acqui/neworderempty.pl line 330., referer: http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/acqui/basket.pl?basketno=527

[Thu Mar 01 14:35:23 2012] [error] [client 192.87.126.61] [Thu Mar  1 14:35:23 2012] neworderempty.pl: Use of uninitialized value in sprintf at /usr/share/koha/testclone/acqui/neworderempty.pl line 330., referer: http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/acqui/basket.pl?basketno=527
Comment 16 Marcel de Rooy 2012-03-01 13:58:49 UTC
But still struggling with the 0 in quantity while having an item box (one item opened).
Note that there is a javascript error (it is listed twice per call):

Fout: document.getElementById("") is null
Bronbestand: http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=5&basketno=527
Regel: 4

Cannot easily find this line, but could this be when it tries to update quantity but cannot find the element or so?
Comment 17 Julian Maurice 2012-03-02 12:56:29 UTC
Is this still a "Failed QA"? In this case, could you be more explicit on what causes this fail?

Note: I don't see any javascript errors with firefox (and this patch doesn't add any call to getElementById, in fact it removes some of them)
Comment 18 Marcel de Rooy 2012-03-02 14:15:16 UTC
Without your patch I can order items, and with your patch it is not possible. So I would suggest to leave the status as it is for now. I retried it now on another OS with FF:

When creating new basket and clicking on new order from empty record:
	js error: additem.js line 60 node is null
On the next neworderempty.pl screen, I have one item but quantity=0 (also after refreshing).
At the same time I have two repeated js errors:
	js error: document.getElementById("") is null
	Source: http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=5&basketno=528
 Line: 4

If I had more time, I would love to start debugging this extensively but unfortunately ... 
Hope you can find the source of this problem even faster as patch author ;)
Comment 19 Julian Maurice 2012-03-02 15:26:08 UTC
Hmm... I'm quite sure there is a problem with some caching somewhere.
In patch, additem.js:60 contains:

        var field_elt = $("#" + index)
            .find("[name='kohafield'][value='items."+field+"']")
            .prevAll("[name='field_value']")[0];

(no 'node' variable here)
while in master, additem.js:60 is

        var origin = node.getElementsByTagName("select");

(here is the 'node'!)

Same for the document.getElementById which is not anymore in additem.js with the patch

So, if it's not from your browser, maybe the server is caching static files.
If you want to check easily, additem.js with the patch starts with

        function addItem( node, unique_item_fields ) {

while in master it's:

        function deleteItemBlock(index) {

About the quantity, items are counted only after clicking on "Add" (and the item is displayed in the items list/table). This is a behaviour change. You should consider the item form as an 'Edit box' where changes are applied only after clicking on 'Add' or 'Update'. And the items list/table is what will be saved.
Comment 20 Marcel de Rooy 2012-03-02 18:41:17 UTC
I checked additem.js. It is correct. Maybe the only first time this came up.

Note that if I add a return statement in the cloneItemBlock function, I do not have the js errors. So I guess that it is there somewhere.

As you will understand, the error for getElementById may not be in a line that you changed, but something you changed could trigger it now. Although this function is not used in additem.js, it is regularly used in acq.js (just to mention one).

About the quantity, did you test Add order from new empty record? I have the impression that you should start with 1 instead of 0. When I am playing around with Add, Edit, Clear and Delete, I can manage to save an item. But just from the start, it should be possible just using the first item block that appeared.
Comment 21 Julian Maurice 2012-03-08 10:56:59 UTC
Marcel, I see in the log you pasted that you do not receive one of the yui file:

[14:21:42.850] GET http://yui.yahooapis.com/2.5.1/build/datasource/datasource-min.js [HTTP/1.1 404 Not Found 47ms]

If you try to set syspref yuipath to 'local', do you still have js errors?
Comment 22 Marcel de Rooy 2012-03-12 09:47:55 UTC
Julian: That error was also at the first time. Did tests with both yui choices but with same result. Only two document.getElementById("") is null-errors remain. See further comment 20.
Comment 23 Owen Leonard 2012-03-23 09:43:19 UTC
I've tried to reproduce the error described here but I could not. I don't know if I'm not getting the error or I'm just not looking at the right thing. This seems like a really nice feature for 3.8 so I'm anxious to find out whether there is a specific sequence of steps we can use to reproduce the problem. OS and browser information might also be relevant. (I tested in Firefox 11 on Ubuntu).
Comment 24 Katrin Fischer 2012-03-23 09:44:06 UTC Comment hidden (obsolete)
Comment 25 Katrin Fischer 2012-03-23 10:18:34 UTC
I also retested the feature and kept an eye on firebug for any errors but it all worked nicely. I have an idea about the 0 though:
In current master we start with a quantity of 1 and you can save immediately, even with an empty item form. The new display after this patch requires you to click 'Add' at least once, to create your item. I know that this is one click more, but the behaviour seems consistent to me. Perhaps it could act differently, but I am not sure if this should prevent the feature from going in.
What we could do is have an item added already (table shows one empty line) and the form in 'Update' mode.
Comment 26 Francois Charbonnier 2012-03-23 11:13:56 UTC
Katrin and I retested this feature and like Katrin just said, we couldn't reproduce the javascript error. I think Owen did tests as well and nor did he reproduce the bug.

We worked on the ordering page and I just tested the receiving page and to me, it looks like everything works fine. I also keft an eye on firebug but didn't track anything in particular.

To me, this feature works fine.

Concerning the "0" default value in the quantity field. I agree with katrin. It's one more click but I think this feature is worth the change. 

About your proposition, Katrin, it will only prevent librarian from this one more click, if they order 1 item and have default value in their ACQ framework and don"t change them. If not, librarian will have to click on "update" anyway. So I would rather prefer having no empty line within the table. The risk is to add an empty item during the ordering.

In any case, I think this new behaviour (having 0 within quantity) should not block the signing off process.

My2cts ;^)
Comment 27 Marcel de Rooy 2012-03-23 11:15:48 UTC
If there is no further activity on this report, I will take a look again. Please respond if I should wait..
Comment 28 Marcel de Rooy 2012-03-23 13:19:05 UTC
Larger patch. Code looks good to me, apart from some remarks below. Has got a lot of attention in testing providing some more grounds to push it.

check_uniqueness.pl: I do not really object to this code, but this script contains SQL code and injects CGI params into SQL (now guarded fine by grep, but what happens later?). Would it be better to call a function in a module and have your code inspect the CGI params somewhat more explicitly in terms of maintainability? I am not blocking this, leave it up to RM ;)

About the javascript errors: I am still quite sure that this code produces a few js errors. (Already when I open form New order from empty record). But I do not have the time to debug it. If you change part A, part B could start generating errors now. Programming is fun ;) It does not prevent me from entering an order. So we could find it later. No reason to block a lot of work either.

Finally, the discussion on quantity zero and the extra click. Personally I would not be satisfied with leaving it that way. But perhaps we can still work on it when it reaches master.

So updating status to Passed QA and give Paul a chance to judge it ...
Comment 29 Paul Poulain 2012-03-26 12:16:41 UTC
(In reply to comment #28)
> Larger patch. Code looks good to me, apart from some remarks below. Has got
> a lot of attention in testing providing some more grounds to push it.
> 
> check_uniqueness.pl: I do not really object to this code, but this script
> contains SQL code and injects CGI params into SQL (now guarded fine by grep,
> but what happens later?). Would it be better to call a function in a module
> and have your code inspect the CGI params somewhat more explicitly in terms
> of maintainability? I am not blocking this, leave it up to RM ;)
> 
> About the javascript errors: I am still quite sure that this code produces a
> few js errors. (Already when I open form New order from empty record). But I
> do not have the time to debug it. If you change part A, part B could start
> generating errors now. Programming is fun ;) It does not prevent me from
> entering an order. So we could find it later. No reason to block a lot of
> work either.
> 
> Finally, the discussion on quantity zero and the extra click. Personally I
> would not be satisfied with leaving it that way. But perhaps we can still
> work on it when it reaches master.
> 
> So updating status to Passed QA and give Paul a chance to judge it ...

Well... with great power comes great responsibilities...

I should mark failed QA for the following reason:
#1 SQL inside a .pl, that is forbidden
#2 Marcel has some js errors that others can't reproduce (including me), those errors don't prevent the feature to work
#3 the SHOW COLUMN is now to be avoided (see http://wiki.koha-community.org/wiki/PostgreSQL)
(#4 the "0/1" thing is secondary, it's an improvement)

BUT:
* This patch contains an interesting feature, that works from a user point of view
* we've almost reached feature freeze, and I think it's worth adding it.

So = patch pushed, but Julian, please provide a follow-up for #1 and #2

Julian: for the record: in french, the : has a space before and after. But in english, it has a space just after.
So i've added a tiny follow-up to remove a space (this follow-up also fixes some ucfirst-ed words that should not be)
Comment 30 Paul Poulain 2012-03-26 12:17:13 UTC
switching back to assigned for the requested follow-up
Comment 31 Francois Charbonnier 2012-03-26 15:40:46 UTC
Sorry, I can't do more tests right to understand where doest it come from. Is it my installation, the patch... I don't know but I retested very briefly for another purpose this patch and when I created the item when receiving, I noticed that the item wasn't display at the biblio level... 
I will keep an eye on this and retest this patch as soon as I can but if someone can do it before somebody work on it, It will be great!
Comment 32 Julian Maurice 2012-03-28 15:05:01 UTC Comment hidden (obsolete)
Comment 33 Julian Maurice 2012-03-28 15:17:44 UTC
This patch fixes #1 and #3.
Since I can't reproduce #2, I'm not able to fix it.
Comment 34 Julian Maurice 2012-03-30 12:08:37 UTC Comment hidden (obsolete)
Comment 35 Katrin Fischer 2012-04-03 21:34:53 UTC
Hi Julian, 

your follow-up seems to include all changes again? The feature was already pushed to master, so I have obsoleted the signed off earlier patch, but not sure what to do about your second. Could you check and resubmit with only the bugfixes?

http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=eefc774e274ce8167f1063be5f9204a0d5cf5650
Comment 36 Julian Maurice 2012-04-04 07:13:07 UTC Comment hidden (obsolete)
Comment 37 Marcel de Rooy 2012-04-04 11:13:54 UTC
Status should probably be Needs signoff
Comment 38 Katrin Fischer 2012-04-04 19:57:42 UTC Comment hidden (obsolete)
Comment 39 Katrin Fischer 2012-04-04 19:58:38 UTC
I see a lot of warnings in my logs when creating items on order:

[Wed Apr 04 21:30:30 2012] [error] [client 127.0.0.1] [Wed Apr  4 21:30:30 2012] basket.pl: Use of uninitialized value $total_est_gste in sprintf at /home/katrin/kohaclone/acqui/basket.pl line 370., referer: http://localhost:8080/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=3&basketno=5&biblionumber=1
[Wed Apr 04 21:30:30 2012] [error] [client 127.0.0.1] [Wed Apr  4 21:30:30 2012] basket.pl: Use of uninitialized value $total_est_gste in sprintf at /home/katrin/kohaclone/acqui/basket.pl line 370., referer: http://localhost:8080/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=3&basketno=5&biblionumber=1
[Wed Apr 04 21:30:30 2012] [error] [client 127.0.0.1] [Wed Apr  4 21:30:30 2012] basket.pl: Use of uninitialized value $gist_est in sprintf at /home/katrin/kohaclone/acqui/basket.pl line 370., referer: http://localhost:8080/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=3&basketno=5&biblionumber=1
[Wed Apr 04 21:30:30 2012] [error] [client 127.0.0.1] [Wed Apr  4 21:30:30 2012] basket.pl: Use of uninitialized value $gist_est in sprintf at /home/katrin/kohaclone/acqui/basket.pl line 370., referer: http://localhost:8080/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=3&basketno=5&biblionumber=1

And when creating items on receive:
[Wed Apr 04 21:51:21 2012] [error] [client 127.0.0.1] [Wed Apr  4 21:51:21 2012] finishreceive.pl: Use of uninitialized value in substr at /home/katrin/kohaclone/C4/Biblio.pm line 2193., referer: http://localhost:8080/cgi-bin/koha/acqui/orderreceive.pl?ordernumber=2&invoice=Test&gst=0&booksellerid=3
[Wed Apr 04 21:51:21 2012] [error] [client 127.0.0.1] [Wed Apr  4 21:51:21 2012] finishreceive.pl: Use of uninitialized value in substr at /home/katrin/kohaclone/C4/Biblio.pm line 2194., referer: http://localhost:8080/cgi-bin/koha/acqui/orderreceive.pl?ordernumber=2&invoice=Test&gst=0&booksellerid=3
[Wed Apr 04 21:51:21 2012] [error] [client 127.0.0.1] [Wed Apr  4 21:51:21 2012] finishreceive.pl: substr outside of string at /home/katrin/kohaclone/C4/Biblio.pm line 2194., referer: http://localhost:8080/cgi-bin/koha/acqui/orderreceive.pl?ordernumber=2&invoice=Test&gst=0&booksellerid=3
[Wed Apr 04 21:51:21 2012] [error] [client 127.0.0.1] [Wed Apr  4 21:51:21 2012] finishreceive.pl: Indicator in 952 is empty at /home/katrin/kohaclone/C4/Biblio.pm line 2200., referer: http://localhost:8080/cgi-bin/koha/acqui/orderreceive.pl?ordernumber=2&invoice=Test&gst=0&booksellerid=3
[Wed Apr 04 21:51:22 2012] [error] [client 127.0.0.1] [Wed Apr  4 21:51:22 2012] finishreceive.pl: Use of uninitialized value $error_url_str in concatenation (.) or string at /home/katrin/kohaclone/acqui/finishreceive.pl line 104., referer: http://localhost:8080/cgi-bin/koha/acqui/orderreceive.pl?ordernumber=2&invoice=Test&gst=0&booksellerid=3

Functionality looks ok.
Comment 40 Marcel de Rooy 2012-05-10 07:06:13 UTC
Julian/Katrin:
If this patch introduces lots of warnings, wouldn't it be better to adjust the patch instead of waiting for a followup?
Changing status for need of clarification.
Comment 41 Katrin Fischer 2012-05-10 07:19:32 UTC
Hi Marcel,
the follow-up fixes a few problems noted by Paul, so it's an improvement, while it's not causing new problems. The warns and errors I noted were there before and after applying the patch, but I think they should also be fixed before this bug can be closed.
Comment 42 Marcel de Rooy 2012-05-10 08:43:27 UTC
(In reply to comment #41)
> Hi Marcel,
> the follow-up fixes a few problems noted by Paul, so it's an improvement,
> while it's not causing new problems. The warns and errors I noted were there
> before and after applying the patch, but I think they should also be fixed
> before this bug can be closed.

So. Back to Signed off (for now)
Comment 43 Marcel de Rooy 2012-05-11 12:49:15 UTC Comment hidden (obsolete)
Comment 44 Marcel de Rooy 2012-05-11 13:56:11 UTC
(In reply to comment #43)
> Still having trouble creating items.
With the patch from Julian, adding an order with AcqCreateItem set to Receive or Catalog, works fine.
With AcqCreateItem to Order, I had problems. Found out that they came from some local plugins copied from the std framework. There was only a js error "document.getElementById("tag_952_subfield_o_994963") is null", but that referred somehow to the plugin for full call number.  After removing two local plugins from acq framework, these errors did not occur anymore.
I still see a js error on Element not found from fetch_sort_dropbox.pl.
After saving the new order, I see another two js errors: Syntax error (from check_budget_total.pl) and Syntax error, unrecognized expression: [value=items.] from jquery. (Note that I left UniqueItemsField empty here.)
Comment 45 Marcel de Rooy 2012-05-11 14:04:46 UTC
Comment 44 may make clear that some followup could still take care of some issues in acquisition javascript lines.
But attachment 8886 [details] [review] looks good to me (and works!)
Leaving the trivial change in 9511 for Paul's eyes.
Passed QA
Comment 46 Julian Maurice 2012-05-11 14:43:07 UTC
Created attachment 9518 [details] [review]
Bug 7178 follow-up: Fix check_additem when UniqueItemFields is empty

This patch prevent the JS error when submitting the form while UniqueItemFields is empty
Comment 47 Julian Maurice 2012-05-11 14:46:23 UTC
I reset the status of this bug to signed off, as there are still patches that need a sign off, but I will mark previous patch as 'passed qa' so we can see that it can be pushed to master.
Comment 48 Julian Maurice 2012-05-11 14:47:10 UTC
Comment on attachment 8886 [details] [review]
[PASSED-QA][SIGNED-OFF] Bug 7178: Follow-up Improve order item creation

Marking this patch as Passed QA
Comment 49 Paul Poulain 2012-05-14 14:18:49 UTC
Comment on attachment 8886 [details] [review]
[PASSED-QA][SIGNED-OFF] Bug 7178: Follow-up Improve order item creation

This patch has been pushed
Comment 50 Paul Poulain 2012-05-14 14:19:18 UTC
Comment on attachment 9511 [details] [review]
Patch clearing a warning

This tiny and obviously correct patch has been pushed with the previous one
Comment 51 Julian Maurice 2012-05-22 08:51:06 UTC
Created attachment 9690 [details] [review]
Bug 7178 follow-up: Do not fill items table with "(undefined)"

If the cataloging framework doesn't contain a subfield linked to one of the items table columns, these columns are filled with "(undefined)". This patch fix that and replace "(undefined)" by an empty string.
Comment 52 Paul Poulain 2012-05-24 13:01:55 UTC
both follow-ups are tiny, they fixes the problem, doing signed-off and QA in one go
Comment 53 Chris Cormack 2012-05-24 21:14:24 UTC
Switching from enhancement as the feature is in master and 3.8.x now, and these follow ups are bugfixes.

As such, pushed to stable and will be in 3.8.2
Comment 54 Julian Maurice 2012-05-29 11:29:41 UTC
Ok in master and 3.8.x, marked as resolved.