Bug 12550 - Add ability to delay loading of patron's checkouts in circulation and patron details
Summary: Add ability to delay loading of patron's checkouts in circulation and patron ...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 11703
Blocks: 13020 13196
  Show dependency treegraph
 
Reported: 2014-07-10 14:03 UTC by Kyle M Hall
Modified: 2015-12-03 22:11 UTC (History)
7 users (show)

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


Attachments
Bug 12550 - Add ability to delay loading of patron's checkouts in circulation and patron details (33.59 KB, patch)
2014-07-10 14:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12550 - Add ability to delay loading of patron's checkouts in circulation and patron details (33.59 KB, patch)
2014-07-10 14:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12550 - Add ability to delay loading of patron's checkouts in circulation and patron details (28.59 KB, patch)
2014-07-10 16:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12550 - Add ability to delay loading of patron's checkouts in circulation and patron details (28.77 KB, patch)
2014-07-11 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 12550 - Add ability to delay loading of patron's checkouts in circulation and patron details (28.85 KB, patch)
2014-08-26 13:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12550 - Add ability to delay loading of patron's checkouts in circulation and patron details (30.78 KB, patch)
2014-10-01 12:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12550 [QA Followup] - Make checking the checkbox load the table of checkouts (1.04 KB, patch)
2014-10-01 13:14 UTC, Kyle M Hall
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 2014-07-10 14:03:42 UTC
Now that a patron's checkouts are loaded asynchronously, we can further improve Koha's performance by not loading the checkouts table when it is not needed. For example, if a librarian is checking out 5 items to a patron, we really don't need to load the table during the scanning of items 1 to 4, just for item 5. Another example would be browsing to the patron details table in order to change a patron's password.

I propose we add two separate system preferences to add a timer for loading the patron's checkouts table, one for circulation and one for patron details. That way a library can choose to have enough delay so that most librarians can scan items repeatedly without loading the checkouts table, and can have the patron details table load more quickly if they wish. In addition, setting one or both timeouts to 0 will cause the system to act as it currently does.
Comment 1 Kyle M Hall 2014-07-10 14:30:36 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2014-07-10 14:35:57 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2014-07-10 15:59:36 UTC
Quick feedback:

Joubu: khall: don't you think it would be better to display the tables on demand? 
Joubu: with a link "load checkout table"
gmcharlt: I agree with Joubu - and maybe make it so that a circ desk operator can use a checkbox or a toggle that sets a cookie to determine if the table displays automatically when a patron is retrieved
Comment 4 Kyle M Hall 2014-07-10 16:43:29 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2014-07-11 07:18:02 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2014-08-18 12:10:49 UTC
This one is on top now.
Will add this report to my QA to-do list (for nontrivial patches :)
But if someone else wants to QA this first, no problem whatsoever..
Comment 7 Jonathan Druart 2014-08-18 12:33:19 UTC
(In reply to M. de Rooy from comment #6)
> Will add this report to my QA to-do list (for nontrivial patches :)

If I remember correctly, this is more a simple patch than a medium. Most of the part of the changes is indentation.
Comment 8 Marcel de Rooy 2014-08-18 12:36:00 UTC
(In reply to Jonathan Druart from comment #7)
> (In reply to M. de Rooy from comment #6)
> > Will add this report to my QA to-do list (for nontrivial patches :)
> 
> If I remember correctly, this is more a simple patch than a medium. Most of
> the part of the changes is indentation.

Could be. I just looked at the number of lines while scrolling through.. Thx
Comment 9 Martin Renvoize 2014-08-26 13:52:15 UTC Comment hidden (obsolete)
Comment 10 Martin Renvoize 2014-08-26 13:54:52 UTC
Happy to pass this.  It, is indeed, actually a pretty trivial patch; some minor re-factoring which knocks out allot of indenting and then a few extra bits of code for functionality.

I'm happy there are no regressions and it's a very worthwhile addition.

I do agree with what Jonathan says about the check box not feeling quite right.. but can't come up with a better position either.. I'll ask Owen to take a look at some point ;)
Comment 11 Owen Leonard 2014-08-26 15:42:48 UTC
(In reply to Martin Renvoize from comment #10)

> I do agree with what Jonathan says about the check box not feeling quite
> right..

I think the checkbox placement seems logical. I think the only change I would make is that checking the check box should immediately load the table of checkouts. I don't believe the opposite should happen though (hiding the table if you uncheck it).
Comment 12 Kyle M Hall 2014-10-01 12:35:49 UTC
Created attachment 31965 [details] [review]
Bug 12550 - Add ability to delay loading of patron's checkouts in circulation and patron details

Rebased patch.
Comment 13 Kyle M Hall 2014-10-01 13:14:06 UTC
Created attachment 31968 [details] [review]
Bug 12550 [QA Followup] - Make checking the checkbox load the table of checkouts
Comment 14 Tomás Cohen Arazi 2014-10-19 14:44:10 UTC
Patches pushed to master.

Thanks Kyle!
Comment 15 Mason James 2014-12-22 17:22:44 UTC
skipping ENH bug for inclusion in 3.16.x