Bug 29660 - reserve/request.pl should not deal with biblioitem
Summary: reserve/request.pl should not deal with biblioitem
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 29562
Blocks: 23659 28782 30395 30769
  Show dependency treegraph
 
Reported: 2021-12-08 15:26 UTC by Jonathan Druart
Modified: 2022-07-06 09:07 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00


Attachments
Bug 29660: Remove hostitemsflag (2.45 KB, patch)
2021-12-08 15:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29660: Unused code in placerequest.pl (2.32 KB, patch)
2021-12-08 15:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29660: Remove biblioitem loop from request.pl (62.30 KB, patch)
2021-12-08 15:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29660: Remove hostitemsflag (2.66 KB, patch)
2022-01-28 12:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29660: Unused code in placerequest.pl (2.32 KB, patch)
2022-01-28 12:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29660: Remove biblioitem loop from request.pl (66.52 KB, patch)
2022-01-28 12:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29660: Remove hostitemsflag (2.71 KB, patch)
2022-02-09 12:43 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29660: Unused code in placerequest.pl (2.37 KB, patch)
2022-02-09 12:43 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29660: Remove biblioitem loop from request.pl (66.57 KB, patch)
2022-02-09 12:43 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29660: Fix conflict with bug 29844 (1.42 KB, patch)
2022-02-10 13:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29660: Remove hostitemsflag (2.77 KB, patch)
2022-02-17 13:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29660: Unused code in placerequest.pl (2.43 KB, patch)
2022-02-17 13:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29660: Remove biblioitem loop from request.pl (66.63 KB, patch)
2022-02-17 13:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29660: Fix conflict with bug 29844 (1.48 KB, patch)
2022-02-17 13:07 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2021-12-08 15:26:09 UTC
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.
Comment 1 Jonathan Druart 2021-12-08 15:43:18 UTC
Created attachment 128366 [details] [review]
Bug 29660: Remove hostitemsflag
Comment 2 Jonathan Druart 2021-12-08 15:43:22 UTC
Created attachment 128367 [details] [review]
Bug 29660: Unused code in placerequest.pl
Comment 3 Jonathan Druart 2021-12-08 15:43:26 UTC
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)
Comment 4 Jonathan Druart 2021-12-08 15:45:40 UTC
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.
Comment 5 Jonathan Druart 2022-01-28 12:56:36 UTC
Created attachment 129934 [details] [review]
Bug 29660: Remove hostitemsflag
Comment 6 Jonathan Druart 2022-01-28 12:56:44 UTC
Created attachment 129935 [details] [review]
Bug 29660: Unused code in placerequest.pl
Comment 7 Jonathan Druart 2022-01-28 12:56:51 UTC
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)
Comment 8 Jonathan Druart 2022-01-28 12:57:46 UTC
Patches have been rebased on top of other patches that were in the queue. Ready for testing!
Comment 9 Jonathan Druart 2022-01-28 12:58:27 UTC
The whole tree is pushed to https://gitlab.com/joubu/Koha/-/tree/bug_29660
Comment 10 Nick Clemens 2022-02-09 12:43:47 UTC
Created attachment 130352 [details] [review]
Bug 29660: Remove hostitemsflag

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens 2022-02-09 12:43:51 UTC
Created attachment 130353 [details] [review]
Bug 29660: Unused code in placerequest.pl

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Nick Clemens 2022-02-09 12:43:56 UTC
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>
Comment 13 Nick Clemens 2022-02-09 12:54:32 UTC
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
Comment 14 Jonathan Druart 2022-02-10 13:25:25 UTC
Created attachment 130447 [details] [review]
Bug 29660: Fix conflict with bug 29844
Comment 15 Martin Renvoize 2022-02-17 13:07:10 UTC
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>
Comment 16 Martin Renvoize 2022-02-17 13:07:14 UTC
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>
Comment 17 Martin Renvoize 2022-02-17 13:07:19 UTC
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>
Comment 18 Martin Renvoize 2022-02-17 13:07:23 UTC
Created attachment 130734 [details] [review]
Bug 29660: Fix conflict with bug 29844

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Martin Renvoize 2022-02-17 13:09:08 UTC
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
Comment 20 Fridolin Somers 2022-02-23 08:28:04 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄