Bug 33945 - Add ability to delay the loading of the current checkouts table on the checkouts page
Summary: Add ability to delay the loading of the current checkouts table on the checko...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 36540 33528
  Show dependency treegraph
 
Reported: 2023-06-07 18:15 UTC by Kyle M Hall
Modified: 2024-04-08 05:31 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/852
Text to go in the release notes:
Version(s) released in:
23.11.00


Attachments
Bug 33945: Add ability to delay the loading of the current checkouts table on the checkouts page (7.06 KB, patch)
2023-06-07 18:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 33945: Add ability to delay the loading of the current checkouts table on the checkouts page (7.10 KB, patch)
2023-06-07 19:08 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 33945: Add ability to delay the loading of the current checkouts table on the checkouts page (7.13 KB, patch)
2023-06-08 11:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 33945: (QA follow-up) Add delay spinner and message (1.47 KB, patch)
2023-06-08 11:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 33945: Add ability to delay the loading of the current checkouts table on the checkouts page (7.19 KB, patch)
2023-06-08 12:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33945: (QA follow-up) Add delay spinner and message (1.52 KB, patch)
2023-06-08 12:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33945: (follow-up) Avoid breaking checkouts table when pref is empty (1.30 KB, patch)
2023-06-08 12:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33945: (QA follow-up) Don't show delay message on patron details page (4.74 KB, patch)
2023-06-08 12:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 33945: Add ability to delay the loading of the current checkouts table on the checkouts page (7.19 KB, patch)
2023-06-08 12:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33945: (QA follow-up) Add delay spinner and message (1.52 KB, patch)
2023-06-08 12:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33945: (follow-up) Avoid breaking checkouts table when pref is empty (1.36 KB, patch)
2023-06-08 12:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33945: (QA follow-up) Don't show delay message on patron details page (4.79 KB, patch)
2023-06-08 12:36 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2023-06-07 18:15:38 UTC
If a librarian has opted to load the checkouts table automatically on the checkouts page, it will trigger a call to svc/checkouts. If a librarian is checkout out 10 items to a patron, that means svc/checkouts is called uselessly 9 times, with only the 10th time being used to display the checkouts table.

It would be useful to add a delay such that the table only load if the page has been display for a given number of seconds. That way the continuous scanning does not trigger useless svc/checkouts calls, but the librarian also does not need to click the load checkouts button manually.
Comment 1 Kyle M Hall 2023-06-07 18:31:31 UTC
Created attachment 152130 [details] [review]
Bug 33945: Add ability to delay the loading of the current checkouts table on the checkouts page

If a librarian has opted to load the checkouts table automatically on the checkouts page, it will trigger a call to svc/checkouts. If a librarian is checkout out 10 items to a patron, that means svc/checkouts is called uselessly 9 times, with only the 10th time being used to display the checkouts table.

It would be useful to add a delay such that the table only load if the page has been display for a given number of seconds. That way the continuous scanning does not trigger useless svc/checkouts calls, but the librarian also does not need to click the load checkouts button manually.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Verify "Always show checkouts immediately" retains its' original
   behavior of loading the checkouts table immediately
4) Set LoadIssuesTableDelay to a non-zero integer
5) Verify the automatic table loading is delayed by that number of
   seconds
Comment 2 Sam Lau 2023-06-07 19:08:08 UTC
Created attachment 152134 [details] [review]
Bug 33945: Add ability to delay the loading of the current checkouts table on the checkouts page

If a librarian has opted to load the checkouts table automatically on the checkouts page, it will trigger a call to svc/checkouts. If a librarian is checkout out 10 items to a patron, that means svc/checkouts is called uselessly 9 times, with only the 10th time being used to display the checkouts table.

It would be useful to add a delay such that the table only load if the page has been display for a given number of seconds. That way the continuous scanning does not trigger useless svc/checkouts calls, but the librarian also does not need to click the load checkouts button manually.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Verify "Always show checkouts immediately" retains its' original
   behavior of loading the checkouts table immediately
4) Set LoadIssuesTableDelay to a non-zero integer
5) Verify the automatic table loading is delayed by that number of
   seconds

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 3 Katrin Fischer 2023-06-08 10:47:04 UTC
I understand that this is to fix a serious problem, but the solution doesn't feel quite right.

I am not going to block this, but it feels like there should be a better solution.

Could we differentiate between loading the whole table and adding an additional line to it somehow? So the call would not be as extensive?

Or maybe load the full checkouts list immediately but have an (optional) update button that will load the rest of the table after checkouts? 

Brain storming, will still continue QA.
Comment 4 Kyle M Hall 2023-06-08 11:00:23 UTC
> Could we differentiate between loading the whole table and adding an
> additional line to it somehow? So the call would not be as extensive?
> 
> Or maybe load the full checkouts list immediately but have an (optional)
> update button that will load the rest of the table after checkouts? 
> 
> Brain storming, will still continue QA.

Unfortunately, those solutions cannot work with circulation.pl as it currently is. That page reloads for each checkout, so we cannot have an kind of persistence.

The best solution is in line with your thoughts. We need circulation.pl to use API calls to check out items and build the table from those API call responses. This is big work and something I fully intend to make happen, but in the mean time this is our best solution.
Comment 5 Katrin Fischer 2023-06-08 11:08:29 UTC
1) LoadIssuesTableDelay should be LoadCheckoutsTableDelay 
  All descriptions etc. use checkouts, it was only missed in the pref name itself.

2) It's a little irritating that the table disappears during the delay and only reappears after the delay. Maybe we could do better with some spinning thing and keeping the table visible until reloaded?
Comment 6 Katrin Fischer 2023-06-08 11:09:05 UTC
Thx for the reply Kyle, can you have a look at my 2 testing comments?
Comment 7 Kyle M Hall 2023-06-08 11:55:41 UTC
Created attachment 152170 [details] [review]
Bug 33945: Add ability to delay the loading of the current checkouts table on the checkouts page

If a librarian has opted to load the checkouts table automatically on the checkouts page, it will trigger a call to svc/checkouts. If a librarian is checkout out 10 items to a patron, that means svc/checkouts is called uselessly 9 times, with only the 10th time being used to display the checkouts table.

It would be useful to add a delay such that the table only load if the page has been display for a given number of seconds. That way the continuous scanning does not trigger useless svc/checkouts calls, but the librarian also does not need to click the load checkouts button manually.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Verify "Always show checkouts immediately" retains its' original
   behavior of loading the checkouts table immediately
4) Set LoadCheckoutsTableDelay to a non-zero integer
5) Verify the automatic table loading is delayed by that number of
   seconds

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 8 Kyle M Hall 2023-06-08 11:55:48 UTC
Created attachment 152171 [details] [review]
Bug 33945: (QA follow-up) Add delay spinner and message
Comment 9 Kyle M Hall 2023-06-08 11:56:51 UTC
(In reply to Katrin Fischer from comment #5)
> 1) LoadIssuesTableDelay should be LoadCheckoutsTableDelay 
>   All descriptions etc. use checkouts, it was only missed in the pref name
> itself.

Fixed! Changed in the original patch.

> 2) It's a little irritating that the table disappears during the delay and
> only reappears after the delay. Maybe we could do better with some spinning
> thing and keeping the table visible until reloaded?

Added as a followup patch!
Comment 10 Katrin Fischer 2023-06-08 12:14:27 UTC
Created attachment 152173 [details] [review]
Bug 33945: Add ability to delay the loading of the current checkouts table on the checkouts page

If a librarian has opted to load the checkouts table automatically on the checkouts page, it will trigger a call to svc/checkouts. If a librarian is checkout out 10 items to a patron, that means svc/checkouts is called uselessly 9 times, with only the 10th time being used to display the checkouts table.

It would be useful to add a delay such that the table only load if the page has been display for a given number of seconds. That way the continuous scanning does not trigger useless svc/checkouts calls, but the librarian also does not need to click the load checkouts button manually.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Verify "Always show checkouts immediately" retains its' original
   behavior of loading the checkouts table immediately
4) Set LoadCheckoutsTableDelay to a non-zero integer
5) Verify the automatic table loading is delayed by that number of
   seconds

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Katrin Fischer 2023-06-08 12:14:30 UTC
Created attachment 152174 [details] [review]
Bug 33945: (QA follow-up) Add delay spinner and message

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2023-06-08 12:22:14 UTC
Created attachment 152180 [details] [review]
Bug 33945: (follow-up) Avoid breaking checkouts table when pref is empty

When the pref is empty, the checkouts table would break.
This avoids it.
Comment 13 Kyle M Hall 2023-06-08 12:32:45 UTC
Created attachment 152181 [details] [review]
Bug 33945: (QA follow-up) Don't show delay message on patron details page
Comment 14 Katrin Fischer 2023-06-08 12:36:32 UTC
Created attachment 152184 [details] [review]
Bug 33945: Add ability to delay the loading of the current checkouts table on the checkouts page

If a librarian has opted to load the checkouts table automatically on the checkouts page, it will trigger a call to svc/checkouts. If a librarian is checkout out 10 items to a patron, that means svc/checkouts is called uselessly 9 times, with only the 10th time being used to display the checkouts table.

It would be useful to add a delay such that the table only load if the page has been display for a given number of seconds. That way the continuous scanning does not trigger useless svc/checkouts calls, but the librarian also does not need to click the load checkouts button manually.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Verify "Always show checkouts immediately" retains its' original
   behavior of loading the checkouts table immediately
4) Set LoadCheckoutsTableDelay to a non-zero integer
5) Verify the automatic table loading is delayed by that number of
   seconds

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2023-06-08 12:36:34 UTC
Created attachment 152185 [details] [review]
Bug 33945: (QA follow-up) Add delay spinner and message

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Katrin Fischer 2023-06-08 12:36:37 UTC
Created attachment 152186 [details] [review]
Bug 33945: (follow-up) Avoid breaking checkouts table when pref is empty

When the pref is empty, the checkouts table would break.
This avoids it.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Katrin Fischer 2023-06-08 12:36:40 UTC
Created attachment 152187 [details] [review]
Bug 33945: (QA follow-up) Don't show delay message on patron details page

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Tomás Cohen Arazi 2023-06-09 11:43:00 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 19 Martin Renvoize 2023-07-12 07:13:31 UTC
Opting not to backport this enhancement to 23.05.x series (although I do love it, it certainly falls into the enhancement category)
Comment 20 Caroline Cyr La Rose 2024-04-05 19:51:57 UTC
I added the new syspref in the manual to the best of my comprehension of it. It's a bit technical for me. Feel free to let me know if it's totally wrong!

Here is what I wrote, in case the gitlab merge request is not easy to understand.

-  This system preference is used to delay the loading of the checkouts table
   in a patron's account to prevent too many service queries when
   checking out a number of items in a row.

-  Enter a positive integer, such as 5, to delay the loading of the table
   and prevent it from reloading after each check out.