Bug 29043 - Items are processed but not displayed on request.pl before a patron is selected
Summary: Items are processed but not displayed on request.pl before a patron is selected
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 29058
  Show dependency treegraph
 
Reported: 2021-09-16 18:50 UTC by Nick Clemens
Modified: 2022-12-12 21:24 UTC (History)
11 users (show)

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


Attachments
Bug 29043: Don't load items unless a patron has been chosen on request.pl (11.10 KB, patch)
2021-09-16 18:53 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29043: Don't load items unless a patron has been chosen on request.pl (11.17 KB, patch)
2021-09-20 12:51 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29043: Don't load items unless a p[atron has been chosen on request.pl (24.12 KB, patch)
2021-11-17 14:50 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29043: Prune dead code (10.45 KB, patch)
2021-11-17 14:50 UTC, Nick Clemens
Details | Diff | Splinter Review
Here's the fixed patch for testing. (1.19 KB, patch)
2021-12-06 01:04 UTC, Samu Heiskanen
Details | Diff | Splinter Review
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title" (1.47 KB, patch)
2021-12-07 08:48 UTC, Samu Heiskanen
Details | Diff | Splinter Review
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title" (1.46 KB, patch)
2021-12-08 01:06 UTC, Samu Heiskanen
Details | Diff | Splinter Review
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title" (973 bytes, patch)
2021-12-08 01:10 UTC, Samu Heiskanen
Details | Diff | Splinter Review
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title" (10.35 KB, patch)
2021-12-08 01:34 UTC, Samu Heiskanen
Details | Diff | Splinter Review
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title" (10.35 KB, patch)
2021-12-08 01:45 UTC, Samu Heiskanen
Details | Diff | Splinter Review
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title" (977 bytes, patch)
2021-12-08 01:48 UTC, Samu Heiskanen
Details | Diff | Splinter Review
Bug 29043: Don't load items unless a p[atron has been chosen on request.pl (24.18 KB, patch)
2021-12-12 19:27 UTC, Samu Heiskanen
Details | Diff | Splinter Review
Bug 29043: Prune dead code (10.51 KB, patch)
2021-12-12 19:48 UTC, Samu Heiskanen
Details | Diff | Splinter Review
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title" (1.01 KB, patch)
2021-12-12 19:50 UTC, Samu Heiskanen
Details | Diff | Splinter Review
Bug 29043: Don't load items unless a p[atron has been chosen on request.pl (24.18 KB, patch)
2021-12-13 16:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29043: Prune dead code (10.51 KB, patch)
2021-12-13 16:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title" (1.07 KB, patch)
2021-12-13 16:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29043: Don't load items unless a p[atron has been chosen on request.pl (23.67 KB, patch)
2022-01-03 15:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29043: Prune dead code (10.51 KB, patch)
2022-01-03 15:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title" (1.07 KB, patch)
2022-01-03 15:45 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29043: [ALTERNATIVE PATCH] Don't fetch biblios info unless a patron is picked (36.42 KB, patch)
2022-01-07 16:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29043: Don't fetch biblios info unless a patron is picked (44.11 KB, patch)
2022-01-14 16:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29043: Don't fetch items if when are not on the "Place a hold on" form (26.94 KB, patch)
2022-01-17 14:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29043: Don't fetch items if when are not on the 'Place a hold on' form (24.96 KB, patch)
2022-01-17 14:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29043: Don't fetch biblios info unless a patron is picked (44.15 KB, patch)
2022-01-26 08:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29043: Don't fetch items if when are not on the 'Place a hold on' form (24.96 KB, patch)
2022-01-26 08:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29043: Don't fetch biblios info unless a patron is picked (44.20 KB, patch)
2022-01-27 17:26 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29043: Don't fetch items if when are not on the 'Place a hold on' form (25.01 KB, patch)
2022-01-27 17:26 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29043: Don't fetch biblios info unless a patron is picked (44.27 KB, patch)
2022-01-28 14:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29043: Don't fetch items if when are not on the 'Place a hold on' form (25.08 KB, patch)
2022-01-28 14:57 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 Nick Clemens 2021-09-16 18:50:20 UTC
To recreate:
1 - Browse to a bib in the staff client
2 - Click on the holds tab
3 - Note loading time
4 - Add 500 items to bib
5 - Note loading time - it takes longer
6 - Note items are not displayed
Comment 1 Nick Clemens 2021-09-16 18:53:04 UTC
Created attachment 124941 [details] [review]
Bug 29043: Don't load items unless a patron has been chosen on request.pl

This page wraps the item processing into an 'if( $patron )' conditional
to boost performance before a patron has been located.

To test:
 1 - Browse to a bib in the staff client
 2 - Click on the holds tab
 3 - Note loading time
 4 - Add 500 items to bib
 5 - Note loading time - it takes longer
 6 - Note items are not displayed
 7 - Apply patch
 8 - Reload page - is much faster
 9 - Find/choose a patron
10 - Confirm items are loaded correctly
Comment 2 Andrew Fuerste-Henry 2021-09-20 12:51:47 UTC
Created attachment 125060 [details] [review]
Bug 29043: Don't load items unless a patron has been chosen on request.pl

This page wraps the item processing into an 'if( $patron )' conditional
to boost performance before a patron has been located.

To test:
 1 - Browse to a bib in the staff client
 2 - Click on the holds tab
 3 - Note loading time
 4 - Add 500 items to bib
 5 - Note loading time - it takes longer
 6 - Note items are not displayed
 7 - Apply patch
 8 - Reload page - is much faster
 9 - Find/choose a patron
10 - Confirm items are loaded correctly

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 3 Joonas Kylmälä 2021-09-26 17:22:28 UTC
I think this makes the code harder to understand by adding one more indentation layer to the giant loop. Could I propose that in this outer loop "foreach my $biblionumber (@biblionumbers) {" all the non-patron requiring code comes first and calls next unless we have a $patron? This way we we are not adding in all the different places calls to if ($patron) but only in one place. This would help also to make the multi-holds work for clubs – bug 29117.

If anyhow other people think it is okay to go with the patch here then "my $items_any_available;" and the comment above it should be inside the loop as well, and probably the outer loop should also actually be in this if condition.
Comment 4 Marcel de Rooy 2021-10-07 13:42:23 UTC
+        if ( $patron ) {
+            $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblioitem->{biblionumber}, patron => $patron });
+
+            foreach my $itemnumber ( @{ $itemnumbers_of_biblioitem{$biblioitemnumber} } )    {
+                my $item = $iteminfos_of->{$itemnumber};
+                my $do_check;
+                if ( $patron ) {


Double condition
Comment 5 Nick Clemens 2021-11-17 14:50:53 UTC
Created attachment 127731 [details] [review]
Bug 29043: Don't load items unless a p[atron has been chosen on request.pl

This patch moves the code that we sh9ould run if there is no patron to the head of
the biblio loop. If we have no patron we execute this and go to the next loop.

A few variables that were being passed to the template in the loop are moved outside - no
reason to overwrite each loop

To test:
 1 - Browse to a bib in the staff client
 2 - Click on the holds tab
 3 - Note loading time
 4 - Add 500 items to bib
 5 - Note loading time - it takes longer
 6 - Note items are not displayed
 7 - Apply patch
 8 - Reload page - is much faster
 9 - Find/choose a patron
10 - Confirm items are loaded correctly
Comment 6 Nick Clemens 2021-11-17 14:50:56 UTC
Created attachment 127732 [details] [review]
Bug 29043: Prune dead code

Remove sort_borrowerlist as it was not used
Remove "time" for form name
    I don't see nay reason we needed this
Remove messagetransfert from template
    Never passed in code
Remove totalcount variable
    Assigned and never referenced
Remove 'alreadyres'/'alreadyreserved' from template
    All references in coide were in template - never set or passed
Comment 7 Fridolin Somers 2021-11-19 07:07:29 UTC
Hi,

There is a small typo in commit message :
Don't load items unless a p[atron

Causing previous patch to not be obsolete.
Comment 8 Samu Heiskanen 2021-11-29 01:19:42 UTC
Load time is much faster but the title has changed to: Place a hold on No title
Comment 9 Katrin Fischer 2021-11-29 09:51:08 UTC
Hi Samu, thx for testing! If you find an issue it's ok to set status to failed QA to alert the deveveloper.
Comment 10 Samu Heiskanen 2021-12-05 05:52:56 UTC
If you move the line:

323    my $biblio = Koha::Biblios->find( $biblionumber );

just before the foreach loop and add the line:

biblio             => $biblio,

to:

738 # display infos
739 $template->param(
740    reserveloop        => 1,
741    itemdata_enumchron => $itemdata_enumchron,
742    itemdata_ccode     => $itemdata_ccode,
743    date               => $date,
744    biblionumber       => $biblionumber,
745    findborrower       => $findborrower,
746    holdsview          => 1,
747    C4::Search::enabled_staff_search_views,
748 );

like:

# display infos
$template->param(
    reserveloop        => 1,
    itemdata_enumchron => $itemdata_enumchron,
    itemdata_ccode     => $itemdata_ccode,
    date               => $date,
    biblionumber       => $biblionumber,
    findborrower       => $findborrower,
    biblio             => $biblio,
    holdsview          => 1,
    C4::Search::enabled_staff_search_views,
);

The patch works. What is your opinion, have I got it done correcly?
Comment 11 Samu Heiskanen 2021-12-06 01:04:18 UTC
Created attachment 128254 [details] [review]
Here's the fixed patch for testing.
Comment 12 Lari Taskula 2021-12-06 06:02:16 UTC
Hi Samu and welcome,

I can confirm the "No title" bug from Nick's patches.

I tested Samu's patch and noticed that your patch is introducing a new bug - batch placement of holds gets broken, follow this plan to replicate:
  0. Have at least two bibliographic records in your Koha installation
  1. In staff client, perform a search that returns at least two results
  2. On search result list there are checkboxes, check them for two or more results
  3. Click "Place hold" button
  4. Observe the following error: "Cannot place hold: one or more records don't exist."

This is because at line 326 (with your patch included) we are checking if any of the biblios in batch request is missing. The loop that starts at line 321 is handling each biblio of the batch request individually and each iteration requires that specific biblio to be loaded with "my $biblio = Koha::Biblios->find( $biblionumber );" so that required steps can be performed on it.

Also, patch/commit title should be "Bug 29043: subject". See https://wiki.koha-community.org/wiki/Commit_messages#Subject_line
Comment 13 Samu Heiskanen 2021-12-07 08:48:44 UTC Comment hidden (obsolete)
Comment 14 Samu Heiskanen 2021-12-08 01:06:18 UTC Comment hidden (obsolete)
Comment 15 Samu Heiskanen 2021-12-08 01:10:43 UTC Comment hidden (obsolete)
Comment 16 Samu Heiskanen 2021-12-08 01:34:12 UTC Comment hidden (obsolete)
Comment 17 Samu Heiskanen 2021-12-08 01:45:39 UTC Comment hidden (obsolete)
Comment 18 Samu Heiskanen 2021-12-08 01:48:48 UTC
Created attachment 128340 [details] [review]
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title"

This version takes account of the batch-mode.

I only just add the line:

$template->param( biblio => $biblio ) if scalar @biblionumbers == 1;

inside the foreach loop. All my other patch-fixes are obsolete.
Comment 19 Samu Heiskanen 2021-12-09 22:11:05 UTC
Sorry, I got a little carried away with this bug. But original assignee, Nick please continue from here.
Comment 20 Katrin Fischer 2021-12-12 17:09:00 UTC
Hi Samu, do you want to sign-off on the first 2 patches in combination with yours? Then we could ask Nick to sign-off on yours and move to Signed off.
Comment 21 Samu Heiskanen 2021-12-12 19:08:22 UTC
Ok, I'll do that. Thx Katrin!
Comment 22 Samu Heiskanen 2021-12-12 19:27:10 UTC
Created attachment 128421 [details] [review]
Bug 29043: Don't load items unless a p[atron has been chosen on request.pl

This patch moves the code that we sh9ould run if there is no patron to the head of
the biblio loop. If we have no patron we execute this and go to the next loop.

A few variables that were being passed to the template in the loop are moved outside - no
reason to overwrite each loop

To test:
 1 - Browse to a bib in the staff client
 2 - Click on the holds tab
 3 - Note loading time
 4 - Add 500 items to bib
 5 - Note loading time - it takes longer
 6 - Note items are not displayed
 7 - Apply patch
 8 - Reload page - is much faster
 9 - Find/choose a patron
10 - Confirm items are loaded correctly

Signed-off-by: Samu Heiskanen <samu.heiskanen@hypernova.fi>
Comment 23 Samu Heiskanen 2021-12-12 19:48:17 UTC
Created attachment 128422 [details] [review]
Bug 29043: Prune dead code

Remove sort_borrowerlist as it was not used
Remove "time" for form name
    I don't see nay reason we needed this
Remove messagetransfert from template
    Never passed in code
Remove totalcount variable
    Assigned and never referenced
Remove 'alreadyres'/'alreadyreserved' from template
    All references in coide were in template - never set or passed

Signed-off-by: Samu Heiskanen <samu.heiskanen@hypernova.fi>
Comment 24 Samu Heiskanen 2021-12-12 19:50:35 UTC
Created attachment 128423 [details] [review]
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title"

This version takes account of the batch-mode.

I only just add the line:

$template->param( biblio => $biblio ) if scalar @biblionumbers == 1;

inside the foreach loop. All my other patch-fixes are obsolete.

Signed-off-by: Samu Heiskanen <samu.heiskanen@hypernova.fi>
Comment 25 Nick Clemens 2021-12-13 16:07:38 UTC
Created attachment 128474 [details] [review]
Bug 29043: Don't load items unless a p[atron has been chosen on request.pl

This patch moves the code that we sh9ould run if there is no patron to the head of
the biblio loop. If we have no patron we execute this and go to the next loop.

A few variables that were being passed to the template in the loop are moved outside - no
reason to overwrite each loop

To test:
 1 - Browse to a bib in the staff client
 2 - Click on the holds tab
 3 - Note loading time
 4 - Add 500 items to bib
 5 - Note loading time - it takes longer
 6 - Note items are not displayed
 7 - Apply patch
 8 - Reload page - is much faster
 9 - Find/choose a patron
10 - Confirm items are loaded correctly

Signed-off-by: Samu Heiskanen <samu.heiskanen@hypernova.fi>
Comment 26 Nick Clemens 2021-12-13 16:07:42 UTC
Created attachment 128475 [details] [review]
Bug 29043: Prune dead code

Remove sort_borrowerlist as it was not used
Remove "time" for form name
    I don't see nay reason we needed this
Remove messagetransfert from template
    Never passed in code
Remove totalcount variable
    Assigned and never referenced
Remove 'alreadyres'/'alreadyreserved' from template
    All references in coide were in template - never set or passed

Signed-off-by: Samu Heiskanen <samu.heiskanen@hypernova.fi>
Comment 27 Nick Clemens 2021-12-13 16:07:45 UTC
Created attachment 128476 [details] [review]
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title"

This version takes account of the batch-mode.

I only just add the line:

$template->param( biblio => $biblio ) if scalar @biblionumbers == 1;

inside the foreach loop. All my other patch-fixes are obsolete.

Signed-off-by: Samu Heiskanen <samu.heiskanen@hypernova.fi>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 28 Katrin Fischer 2022-01-02 16:39:30 UTC
Sorry Nick, can you please rebase?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 29043: Don't load items unless a p[atron has been chosen on request.pl
Using index info to reconstruct a base tree...
M	reserve/request.pl
Falling back to patching base and 3-way merge...
Auto-merging reserve/request.pl
CONFLICT (content): Merge conflict in reserve/request.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 29043: Don't load items unless a p[atron has been chosen on request.pl
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-29043-Dont-load-items-unless-a-patron-has-been-vtn8py.patch
Comment 29 Nick Clemens 2022-01-03 15:45:26 UTC
Created attachment 128971 [details] [review]
Bug 29043: Don't load items unless a p[atron has been chosen on request.pl

This patch moves the code that we sh9ould run if there is no patron to the head of
the biblio loop. If we have no patron we execute this and go to the next loop.

A few variables that were being passed to the template in the loop are moved outside - no
reason to overwrite each loop

To test:
 1 - Browse to a bib in the staff client
 2 - Click on the holds tab
 3 - Note loading time
 4 - Add 500 items to bib
 5 - Note loading time - it takes longer
 6 - Note items are not displayed
 7 - Apply patch
 8 - Reload page - is much faster
 9 - Find/choose a patron
10 - Confirm items are loaded correctly

Signed-off-by: Samu Heiskanen <samu.heiskanen@hypernova.fi>
Comment 30 Nick Clemens 2022-01-03 15:45:30 UTC
Created attachment 128972 [details] [review]
Bug 29043: Prune dead code

Remove sort_borrowerlist as it was not used
Remove "time" for form name
    I don't see nay reason we needed this
Remove messagetransfert from template
    Never passed in code
Remove totalcount variable
    Assigned and never referenced
Remove 'alreadyres'/'alreadyreserved' from template
    All references in coide were in template - never set or passed

Signed-off-by: Samu Heiskanen <samu.heiskanen@hypernova.fi>
Comment 31 Nick Clemens 2022-01-03 15:45:34 UTC
Created attachment 128973 [details] [review]
Bug 29043: Remaining bug is the incorrect title "Place a hold on No title"

This version takes account of the batch-mode.

I only just add the line:

$template->param( biblio => $biblio ) if scalar @biblionumbers == 1;

inside the foreach loop. All my other patch-fixes are obsolete.

Signed-off-by: Samu Heiskanen <samu.heiskanen@hypernova.fi>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 32 Jonathan Druart 2022-01-07 16:58:40 UTC
Created attachment 129200 [details] [review]
Bug 29043: [ALTERNATIVE PATCH] Don't fetch biblios info unless a patron is picked

If we haven't picked a patron or a club yet we don't need to fetch
biblios, items and holdability infos
Comment 33 Jonathan Druart 2022-01-07 17:02:19 UTC
I am late here and I was not aware of these patches when I wrote bug 29660. I have tried to rebase bug 29660 on top of this but it's very tricky.
This patch is suggesting to do the minimum required to not fetch the infos if we don't need to (no club or patron selected).

Please read this patch with `git show -w HEAD` and you will notice there are only ~10 lines. It will help a lot if we can move forward with this version as I will be able to rebase easily bug 29660 on top of it.

I haven't stressed the patch much but the simple use cases work so far. Will be back to it next week.
Comment 34 Jonathan Druart 2022-01-07 17:02:58 UTC Comment hidden (obsolete)
Comment 35 Jonathan Druart 2022-01-10 13:28:12 UTC
Nick, will this alternative patch answers your needs?
Comment 36 Jonathan Druart 2022-01-14 16:40:31 UTC
Created attachment 129498 [details] [review]
Bug 29043: Don't fetch biblios info unless a patron is picked

If we haven't picked a patron or a club yet we don't need to fetch
biblios, items and holdability infos
Comment 37 Jonathan Druart 2022-01-14 16:41:56 UTC
@ reserve/request.pl:199 @ $template->param(
     multi_hold => $multi_hold,
 );
 
+# If we are coming from the search result and only 1 is selected
+$biblionumber ||= $biblionumbers[0] unless $multi_hold;
 
 # If we have the borrowernumber because we've performed an action, then we
 # don't want to try to place another reserve.
 if ($borrowernumber_hold && !$action) {
@ reserve/request.pl:297 @ $template->param(
     messageclub     => $messageclub
 );
 
+# Load the hold list if
+#  - we are searching for a patron or club and found one
+#  - we are not searching for anything
+if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
+    || ( !$findborrower && !$findclub ) )
+{
     # FIXME launch another time GetMember perhaps until (Joubu: Why?)
     my $patron = Koha::Patrons->find( $borrowernumber_hold );
 
@ reserve/request.pl:771 @ foreach my $biblionumber (@biblionumbers) {
 
     $template->param( biblioloop => \@biblioloop );
     $template->param( no_reserves_allowed => $no_reserves_allowed );
-$template->param( biblionumbers => join('/', @biblionumbers) );
     $template->param( exceeded_maxreserves => $exceeded_maxreserves );
     $template->param( exceeded_holds_per_record => $exceeded_holds_per_record );
     $template->param( subscriptionsnumber => CountSubscriptionFromBiblionumber($biblionumber));
+} elsif ( ! $multi_hold ) {
+    my $biblio = Koha::Biblios->find( $biblionumber );
+    $template->param( biblio => $biblio );
+}
 
+if ( $multi_hold ) {
+    $template->param( biblionumbers => join('/', @biblionumbers) );
+} else {
+    $template->param( biblionumber => $biblionumber || $biblionumbers[0] );
+}
 
 # pass the userenv branch if no pickup location selected
 $template->param( pickup => $pickup || C4::Context->userenv->{branch} );
Comment 38 Jonathan Druart 2022-01-14 16:42:08 UTC
Rebased and fixed a bug.
Comment 39 Jonathan Druart 2022-01-17 14:15:32 UTC
Created attachment 129530 [details] [review]
Bug 29043: Don't fetch items if when are not on the "Place a hold on" form
Comment 40 Jonathan Druart 2022-01-17 14:18:36 UTC
Created attachment 129531 [details] [review]
Bug 29043: Don't fetch items if when are not on the 'Place a hold on' form
Comment 41 Jonathan Druart 2022-01-17 14:19:05 UTC
@ reserve/request.pl:440 @ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
               }
         };
 
+        if ( $club_hold or $borrowernumber_hold ) {
             my @bibitemloop;
 
             my @available_itemtypes;
@ reserve/request.pl:673 @ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
             }
 
             @available_itemtypes = uniq( @available_itemtypes );
-        $template->param( available_itemtypes => \@available_itemtypes );
+            $template->param(
+                bibitemloop         => \@bibitemloop,
+                available_itemtypes => \@available_itemtypes
+            );
+        }
 
         # existingreserves building
         my @reserveloop;
@ reserve/request.pl:744 @ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
 
         # display infos
         $template->param(
-                         bibitemloop       => \@bibitemloop,
                          itemdata_enumchron => $itemdata_enumchron,
                          itemdata_ccode    => $itemdata_ccode,
                          date              => $date,
Comment 42 Jonathan Druart 2022-01-26 08:32:07 UTC
Created attachment 129820 [details] [review]
Bug 29043: Don't fetch biblios info unless a patron is picked

If we haven't picked a patron or a club yet we don't need to fetch
biblios, items and holdability infos
Comment 43 Jonathan Druart 2022-01-26 08:32:26 UTC
Created attachment 129821 [details] [review]
Bug 29043: Don't fetch items if when are not on the 'Place a hold on' form
Comment 44 Nick Clemens 2022-01-27 17:26:31 UTC
Created attachment 129915 [details] [review]
Bug 29043: Don't fetch biblios info unless a patron is picked

If we haven't picked a patron or a club yet we don't need to fetch
biblios, items and holdability infos

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 45 Nick Clemens 2022-01-27 17:26:37 UTC
Created attachment 129916 [details] [review]
Bug 29043: Don't fetch items if when are not on the 'Place a hold on' form

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 46 Martin Renvoize 2022-01-28 14:57:36 UTC
Created attachment 129949 [details] [review]
Bug 29043: Don't fetch biblios info unless a patron is picked

If we haven't picked a patron or a club yet we don't need to fetch
biblios, items and holdability infos

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Martin Renvoize 2022-01-28 14:57:43 UTC
Created attachment 129950 [details] [review]
Bug 29043: Don't fetch items if when are not on the 'Place a hold on' form

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 48 Martin Renvoize 2022-01-28 14:58:41 UTC
Nice bit of cleaning up here, thanks.. some pretty involved code, but everything appears to work as expected and there's a noticeable performance gain.

QA scripts happy

Passing QA
Comment 49 Fridolin Somers 2022-02-01 07:56:43 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 50 Kyle M Hall 2022-02-07 16:18:29 UTC
Pushed to 21.11.x for 21.11.03
Comment 51 Andrew Fuerste-Henry 2022-02-21 15:31:42 UTC
Pushed to 21.05.x for 21.05.11
Comment 52 Victor Grousset/tuxayo 2022-02-24 22:33:10 UTC
Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed.