Created attachment 7249 [details] [review] Bug 7178: Acquisition item creation improvement - Display a unique item block at once On orderreceive.pl when AcqCreateItem is 'receiving', and on neworderempty.pl when AcqCreateItem is 'ordering' it displays an item block with item infos to fill, and a '+' button. When user clicks on '+', the block is hidden and a list shows up with the items that will be received. User can then edit or delete items in the list and click 'Save' to receive items. - PrepareItemrecordDisplay is now used for cloning block Previous cloning function was duplicating ids, the side effect is that plugins didn't work when several items were displayed. PrepareItemrecordDisplay regenerate the form with new ids 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.
Created attachment 7279 [details]
no item form when ordering
Created attachment 7450 [details] [review] 7249: Bug 7178: Acquisition item creation improvement 1) check_unicity.pl renamed to check_uniqueness.pl stocknumber and copynumber checks have been deactivated, configuration of this feature can come later. 2) created additems.js.inc which contains translatable messages 3) Not a typo, it's a Template::Toolkit comment. 5) I've rebased the patch on master, and can't reproduce the problem. Could you test this patch once again please? 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. *** Bug 7173 has been marked as a duplicate of this bug. *** 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 Created attachment 7806 [details] [review] Bug 7178: Acquisition item creation improvement New rebased patch with following changes: - problem with item form fixed, we have to remove it in order for form check to work correctly, but it was not reattached if form check failed. Now it is done and you should not be able to reproduce this bug. - a lot of items infos added to the items table (with a scrollbar if there are too many infos) - items table now display the descriptions instead of the codes (for drop-down lists) - '+' renamed to 'Add' when the form is for a new item, 'Update' when it's an edit (the 'Show' link in table has been renamed to 'Edit') - a new syspref UniqueItemFields inside which we can configure which fields we want to check for duplicates (these fields are not duplicated when clicking on 'Add'). Fields are separated by space and must be sql column name of items table. Defaults to 'barcode'. Created attachment 7807 [details] [review] Bug 7178: Acquisition item creation improvement I forgot to include a change in previous patch, sorry. 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! Created attachment 7892 [details] [review] [SIGNED-OFF] Bug 7178: Acquisition item creation improvement - Display a unique item block at once On orderreceive.pl when AcqCreateItem is 'receiving', and on neworderempty.pl when AcqCreateItem is 'ordering' it displays an item block with item infos to fill, and a '+' button. When user clicks on '+', the block is hidden and a list shows up with the items that will be received. User can then edit or delete items in the list and click 'Save' to receive items. - PrepareItemrecordDisplay is now used for cloning block Previous cloning function was duplicating ids, the side effect is that plugins didn't work when several items were displayed. PrepareItemrecordDisplay regenerate the form with new ids - New system preference UniqueItemFields Contains a space-separated list of sql column names (of items table). This syspref is used in two ways: - Values corresponding to fields in syspref are not duplicated when adding a new item (button 'Add') - When saving the form, a check is made on fields in syspref for detecting duplicate (in DB and in the form) Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests done are noted on the bug report. Created attachment 7963 [details]
no item block quantity=0
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
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] 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 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?
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) 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 ;)
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.
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. 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? 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.
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). Created attachment 8558 [details] [review] Bug 7178: Acquisition item creation improvement - Display a unique item block at once On orderreceive.pl when AcqCreateItem is 'receiving', and on neworderempty.pl when AcqCreateItem is 'ordering' it displays an item block with item infos to fill, and a '+' button. When user clicks on '+', the block is hidden and a list shows up with the items that will be received. User can then edit or delete items in the list and click 'Save' to receive items. - PrepareItemrecordDisplay is now used for cloning block Previous cloning function was duplicating ids, the side effect is that plugins didn't work when several items were displayed. PrepareItemrecordDisplay regenerate the form with new ids - New system preference UniqueItemFields Contains a space-separated list of sql column names (of items table). This syspref is used in two ways: - Values corresponding to fields in syspref are not duplicated when adding a new item (button 'Add') - When saving the form, a check is made on fields in syspref for detecting duplicate (in DB and in the form) Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests done are noted on the bug report. 2012-03-23: Fixed conflict in updatedatabase. 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. 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 ;^) If there is no further activity on this report, I will take a look again. Please respond if I should wait.. 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 ... (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) switching back to assigned for the requested follow-up 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! Created attachment 8672 [details] [review] Follow-up Improve order item creation Move SQL code from Perl script to Perl module Replace SHOW COLUMNS by $dbh->column_info() This patch fixes #1 and #3. Since I can't reproduce #2, I'm not able to fix it. Created attachment 8715 [details] [review] Follow-up Improve order item creation Move SQL code from Perl script to Perl module Replace SHOW COLUMNS by $dbh->column_info() Update total on neworderempty.pl when adding or deleting items Same follow-up as above with a bug fix included. Total was not updated on neworderempty.pl 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 Created attachment 8857 [details] [review] Follow-up Improve order item creation Oops, sorry I uploaded the wrong patch. This is the follow-up. Status should probably be Needs signoff Created attachment 8886 [details] [review] [PASSED-QA][SIGNED-OFF] Bug 7178: Follow-up Improve order item creation Move SQL code from Perl script to Perl module Replace SHOW COLUMNS by $dbh->column_info() Update total on neworderempty.pl when adding or deleting items Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tested ok for ordering and receiving items. Total updated correctly. Note: There are lots of errors in the logs before and after applying the patch. A follow up is needed. 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. 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. 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. (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) Created attachment 9511 [details] [review] Patch clearing a warning Still having trouble creating items. Adding this patch just to eliminate some warnings on sprintf in log. (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 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 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 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 on attachment 8886 [details] [review] [PASSED-QA][SIGNED-OFF] Bug 7178: Follow-up Improve order item creation Marking this patch as Passed QA Comment on attachment 8886 [details] [review] [PASSED-QA][SIGNED-OFF] Bug 7178: Follow-up Improve order item creation This patch has been pushed Comment on attachment 9511 [details] [review] Patch clearing a warning This tiny and obviously correct patch has been pushed with the previous one 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. both follow-ups are tiny, they fixes the problem, doing signed-off and QA in one go 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 Ok in master and 3.8.x, marked as resolved. |
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