The way reserve/request.pl is written is wrong, hard to read and to maintain. We should not use a biblioitemloop variable as there is a 1-1 link between biblio and biblioitem That would simplify the code a lot and make it more readable.
Created attachment 128366 [details] [review] Bug 29660: Remove hostitemsflag
Created attachment 128367 [details] [review] Bug 29660: Unused code in placerequest.pl
Created attachment 128368 [details] [review] Bug 29660: Remove biblioitem loop from request.pl It seems that hiddencount didn't work prior to this patch (to confirm)
Nick, Joonas, what about this approach? I haven't tested it yet much. Only tried with a record with 1500+ items and the load time is the same. If you are with me I can continue and 1. clean a bit more the controller and template then 2. try to improve the perf for big biblio records.
Created attachment 129934 [details] [review] Bug 29660: Remove hostitemsflag
Created attachment 129935 [details] [review] Bug 29660: Unused code in placerequest.pl
Created attachment 129936 [details] [review] Bug 29660: Remove biblioitem loop from request.pl It seems that hiddencount didn't work prior to this patch (to confirm)
Patches have been rebased on top of other patches that were in the queue. Ready for testing!
The whole tree is pushed to https://gitlab.com/joubu/Koha/-/tree/bug_29660
Created attachment 130352 [details] [review] Bug 29660: Remove hostitemsflag Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 130353 [details] [review] Bug 29660: Unused code in placerequest.pl Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 130354 [details] [review] Bug 29660: Remove biblioitem loop from request.pl It seems that hiddencount didn't work prior to this patch (to confirm) Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This works well, it also fixes what I th8ink is a bug Test plan: 1 - Enable EasyAnalyticalRecords 2 - Find a record, Edit->Link to host record 3 - Paste a barcode from another record 4 - The item now shows in the holdings list 5 - Attempt to place hold 6 - Linked item shows in holdings block 7 - Alter rules to prevent linked item from being holdable, only linked item holdable etc. 8 - All works as expected 9 - Also test placing holds on two records, including the record with linked item 10 - Can place hold as long as 1 item available on eahc, or can only hold record with holdable items Before these patches: - The linked item appeared in its own table, when not holdable there was a warning that the hold could not be placed as no items were available even when the regular items were available and it was possible to place a hold. - This seems to exist in 19.11 and forwards - I did not check farther back
Created attachment 130447 [details] [review] Bug 29660: Fix conflict with bug 29844
Created attachment 130731 [details] [review] Bug 29660: Remove hostitemsflag Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 130732 [details] [review] Bug 29660: Unused code in placerequest.pl Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 130733 [details] [review] Bug 29660: Remove biblioitem loop from request.pl It seems that hiddencount didn't work prior to this patch (to confirm) Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 130734 [details] [review] Bug 29660: Fix conflict with bug 29844 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
That took some wrapping my head around.. and a good chunk of testing. I'm happy with the results. QA scripts are happy too. Passing QA
Pushed to master for 22.05, thanks to everybody involved [U+1F984]