| Summary: | Order lines can be duplicated in acqui scripts spent.pl and ordered.pl | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
| Component: | Acquisitions | Assignee: | Nick Clemens (kidclamp) <nick> |
| Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
| Severity: | major | ||
| Priority: | P5 - low | CC: | fridolin.somers, jonathan.druart, m.de.rooy |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
19.11.00,19.05.04
|
|
| Circulation function: | |||
| Attachments: |
Bug 23397: Fix grouping of orders in acqui scripts
Bug 23397: Fix grouping of orders in acqui scripts Bug 23397: (follow-up) Split and display itemtypes Bug 23397: Fix grouping of orders in acqui scripts Bug 23397: (follow-up) Split and display itemtypes Bug 23397: (QA follow-up) Replace mod 2 by last |
||
|
Description
Nick Clemens (kidclamp)
2019-07-29 18:11:06 UTC
Created attachment 91855 [details] [review] Bug 23397: Fix grouping of orders in acqui scripts Bug 21622 attempted to fix some grouping for strict mode, however, it failed to take into account that single order can have multiple item types. To recreate: 1 - Place an order for 4 copies of a title in acq 2 - Make sure you are creating orders when ordering (AcqCreateItems syspref and/or when creating basket) 3 - Set three items to one type, and one to another 4 - Go to Acqui-Home and click 'ordered' for the used budget 5 - Note the line is duplicated for each itemtype in the order 6 - Recieve the items and note the same issue on 'Spent' 7 - Place another order as before so you have the problem in both ordered.pl and spent/pl 8 - Apply patch 9 - Note orders are no longer duplicated Created attachment 92344 [details] [review] Bug 23397: Fix grouping of orders in acqui scripts Bug 21622 attempted to fix some grouping for strict mode, however, it failed to take into account that single order can have multiple item types. To recreate: 1 - Place an order for 4 copies of a title in acq 2 - Make sure you are creating orders when ordering (AcqCreateItems syspref and/or when creating basket) 3 - Set three items to one type, and one to another 4 - Go to Acqui-Home and click 'ordered' for the used budget 5 - Note the line is duplicated for each itemtype in the order 6 - Recieve the items and note the same issue on 'Spent' 7 - Place another order as before so you have the problem in both ordered.pl and spent/pl 8 - Apply patch 9 - Note orders are no longer duplicated Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Form shows itemtype, which is empty now. Template contains: ItemTypes.GetDescription( order.itype ) If you would add AS itype to the GROUP_CONCAT you added, it still does not work, since the concatenated string is no itype anymore.. Remove it from the form ? BTW While testing: DBD::mysql::st execute failed: Incorrect double value: '' for column `koha_master`.`aqorders`.`discount` at row 1 [for Statement "UPDATE aqorders SET basketno=?, biblionumber=?, quantity=?, budget_id=?, currency=?, listprice=?, discount=?, rrp=?, replacementprice=?, ecost=?, unitprice=?, order_internalnote=?, order_vendornote=?, sort1=?, sort2=?, subscriptionid=?, uncertainprice=?, tax_rate_on_ordering=?, rrp_tax_excluded=?, rrp_tax_included=?, ecost_tax_excluded=?, ecost_tax_included=?, tax_value_on_ordering=?, timestamp=NOW() WHERE ordernumber=?" with ParamValues: 0="67", 1="2578", 2="2", 3="123", 4="USD", 5="10", 6="", 7="10.00", 8="10.00", 9="10.00", 10="0.00", 11="", 12="", 13="", 14="", 15=undef, 16="0", 17="0", 18="10.00", 19="10", 20="10", 21="10", 22="0", 23="15"] at /usr/share/koha/C4/Acquisition.pm line 1358. With strict SQL saving the order failed invisibly.. Could we deal with that in the template, splitting up the string and displaying all itemtypes of an order? Worried about this one as it makes the amounts not add up... and libraries don't like that. (In reply to Katrin Fischer from comment #5) > Could we deal with that in the template, splitting up the string and > displaying all itemtypes of an order? > > Worried about this one as it makes the amounts not add up... and libraries > don't like that. We could show the concatenated itypes if GetDescription does not come up with a value? I think if we can't fix showing the description, nothing is better right now and matches the behaviour before as far as I can tell from our installations where we have a mix of empty and itemtype description. What I was thinking was: if I understood GROUP_CONCAT right it gives you a comma separated string of values? Could we not split by , and loop through it template side? Created attachment 92446 [details] [review] Bug 23397: (follow-up) Split and display itemtypes (In reply to Nick Clemens from comment #8) > Created attachment 92446 [details] [review] [review] > Bug 23397: (follow-up) Split and display itemtypes Looking at it again Created attachment 92448 [details] [review] Bug 23397: Fix grouping of orders in acqui scripts Bug 21622 attempted to fix some grouping for strict mode, however, it failed to take into account that single order can have multiple item types. To recreate: 1 - Place an order for 4 copies of a title in acq 2 - Make sure you are creating orders when ordering (AcqCreateItems syspref and/or when creating basket) 3 - Set three items to one type, and one to another 4 - Go to Acqui-Home and click 'ordered' for the used budget 5 - Note the line is duplicated for each itemtype in the order 6 - Recieve the items and note the same issue on 'Spent' 7 - Place another order as before so you have the problem in both ordered.pl and spent/pl 8 - Apply patch 9 - Note orders are no longer duplicated Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 92449 [details] [review] Bug 23397: (follow-up) Split and display itemtypes Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 92450 [details] [review] Bug 23397: (QA follow-up) Replace mod 2 by last Looks better: Maps | Books | Continuing Resources Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> I was completely stuck with circ/pendingreserves.pl because of the use of GROUP_CONCAT. It makes things super hard to rewrite. Do not we have another option? I do not have much to suggest apart from rewriting the scripts from scratch, which seems quite out of the scope ;) Nice work! Pushed to master for 19.11.00 Pushed to 19.05.x for 19.05.04 |