Bug 33497 - Reduce DB calls on staff detail page
Summary: Reduce DB calls on staff detail page
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 33167 33493 33496
Blocks: 33568
  Show dependency treegraph
 
Reported: 2023-04-11 18:51 UTC by Nick Clemens
Modified: 2023-05-31 12:06 UTC (History)
4 users (show)

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


Attachments
Bug 33497: Use 'host_items' param to fetch all items at once (2.66 KB, patch)
2023-04-11 18:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33497: Add a filter relationship for branchtransfers (2.01 KB, patch)
2023-04-11 18:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33497: Use 'host_items' param to fetch all items at once (2.71 KB, patch)
2023-04-12 18:00 UTC, David Nind
Details | Diff | Splinter Review
Bug 33497: Add a filter relationship for branchtransfers (2.06 KB, patch)
2023-04-12 18:01 UTC, David Nind
Details | Diff | Splinter Review
Bug 33497: Use 'host_items' param to fetch all items at once (2.73 KB, patch)
2023-05-31 11:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33497: Add a filter relationship for branchtransfers (2.08 KB, patch)
2023-05-31 11:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33497: (follow-up) Remove unecessary scalar (900 bytes, patch)
2023-05-31 11:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33497: (follow-up) Adjust relationship name from bug 33493 (1.07 KB, patch)
2023-05-31 12:06 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2023-04-11 18:51:45 UTC

    
Comment 1 Nick Clemens 2023-04-11 18:57:03 UTC
Created attachment 149467 [details] [review]
Bug 33497: Use 'host_items' param to fetch all items at once

This patch adjusts the detail page to fetch items and host items
together, and prefetches transfers and checkouts

To test:
1 - Enable easy analytics
2 - Attach some items to a bib
3 - Checkout an item on the bib
4 - View the details page
5 - Apply patch
6 - View details page, confirm nothing has changed
Comment 2 Nick Clemens 2023-04-11 18:57:05 UTC
Created attachment 149468 [details] [review]
Bug 33497: Add a filter relationship for branchtransfers

This uses the new relationship from bug 33493 to fetch the transfers for items

To test:
1 - Transfer some items on a bib
2 - View the biblio details page in the staff interface
3 - Apply patch
4 - Confirm  the view is the same
Comment 3 David Nind 2023-04-11 19:12:36 UTC
Added assignee.
Comment 4 David Nind 2023-04-12 18:00:59 UTC
Created attachment 149573 [details] [review]
Bug 33497: Use 'host_items' param to fetch all items at once

This patch adjusts the detail page to fetch items and host items
together, and prefetches transfers and checkouts

To test:
1 - Enable easy analytics
2 - Attach some items to a bib
3 - Checkout an item on the bib
4 - View the details page
5 - Apply patch
6 - View details page, confirm nothing has changed

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2023-04-12 18:01:01 UTC
Created attachment 149574 [details] [review]
Bug 33497: Add a filter relationship for branchtransfers

This uses the new relationship from bug 33493 to fetch the transfers for items

To test:
1 - Transfer some items on a bib
2 - View the biblio details page in the staff interface
3 - Apply patch
4 - Confirm  the view is the same

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 Jonathan Druart 2023-04-14 13:28:47 UTC
Nick, I have a plan to rewrite the items table on the staff detail page to make it use the REST API. I will base my patches on top of this bug, but keep it in mind in case you would like to provide more things. Maybe we are going to collide :)
Comment 7 Nick Clemens 2023-04-14 13:48:39 UTC
(In reply to Jonathan Druart from comment #6)
> Nick, I have a plan to rewrite the items table on the staff detail page to
> make it use the REST API. I will base my patches on top of this bug, but
> keep it in mind in case you would like to provide more things. Maybe we are
> going to collide :)

Excellent! I had started trying to clean the page, then realized it would just be better to move to the API, so switched to setting the stage for that. Let me know if any of this set needs adjustment or how I can help. I won't file anymore touching this one for now :-)
Comment 8 Jonathan Druart 2023-04-18 13:56:30 UTC
+my $items = $biblio->items({ host_items => 1 })->search_ordered( $params, { prefetch => ['issue','branchtransfers'] } );
[...]
+$dat->{'count'} = $biblio->items({ host_items => 1 })->count;
+$dat->{'showncount'} = $items->count;

Are not we going to always get count == showncount?
Comment 9 Jonathan Druart 2023-04-18 15:20:23 UTC
Nick answered me, itemlost is passed in $params
Comment 10 Jonathan Druart 2023-04-19 12:26:48 UTC
$dat->{'count'} = $biblio->items->count + $hostitems->count;

Should we apply the same count (ie. include host items) in other catalogue scripts as well? See catalog-strings.inc
  var count = [% count | html %];
Comment 11 Nick Clemens 2023-04-19 17:15:53 UTC
(In reply to Jonathan Druart from comment #10)
> $dat->{'count'} = $biblio->items->count + $hostitems->count;
> 
> Should we apply the same count (ie. include host items) in other catalogue
> scripts as well? See catalog-strings.inc
>   var count = [% count | html %];

moredetail needs it, the other areas using catalog-strings don't actually seemd to display count or items
Comment 12 Jonathan Druart 2023-05-25 12:01:02 UTC
+if (C4::Context->preference("AlternateHoldingsField") && scalar $items->count == 0) {

scalar should be removed here.
Comment 13 Nick Clemens 2023-05-31 11:33:15 UTC
Created attachment 151861 [details] [review]
Bug 33497: Use 'host_items' param to fetch all items at once

This patch adjusts the detail page to fetch items and host items
together, and prefetches transfers and checkouts

To test:
1 - Enable easy analytics
2 - Attach some items to a bib
3 - Checkout an item on the bib
4 - View the details page
5 - Apply patch
6 - View details page, confirm nothing has changed

Signed-off-by: David Nind <david@davidnind.com>
Comment 14 Nick Clemens 2023-05-31 11:33:17 UTC
Created attachment 151862 [details] [review]
Bug 33497: Add a filter relationship for branchtransfers

This uses the new relationship from bug 33493 to fetch the transfers for items

To test:
1 - Transfer some items on a bib
2 - View the biblio details page in the staff interface
3 - Apply patch
4 - Confirm  the view is the same

Signed-off-by: David Nind <david@davidnind.com>
Comment 15 Nick Clemens 2023-05-31 11:33:19 UTC
Created attachment 151863 [details] [review]
Bug 33497: (follow-up) Remove unecessary scalar
Comment 16 Nick Clemens 2023-05-31 12:06:56 UTC
Created attachment 151866 [details] [review]
Bug 33497: (follow-up) Adjust relationship name from bug 33493