Bug 34668 - Add syspref to popup warn librarians of waiting holds for the patrons whose items they are checking out
Summary: Add syspref to popup warn librarians of waiting holds for the patrons whose i...
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Shi Yao Wang
QA Contact: Lucas Gass
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-08-30 20:07 UTC by Shi Yao Wang
Modified: 2024-01-08 23:45 UTC (History)
10 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 34668: Add popup warn librarians of waiting holds when checking out (7.73 KB, patch)
2023-08-31 19:46 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 34668: Add popup warn librarians of waiting holds when checking out (9.29 KB, patch)
2023-08-31 19:56 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 34668: Add popup warn librarians of waiting holds when checking out (9.31 KB, patch)
2023-09-01 17:37 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 34668: Add popup warn librarians of waiting holds when checking out (9.39 KB, patch)
2023-09-04 08:48 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 34668: remove binding/unbinding event for one submit event (3.15 KB, patch)
2023-10-12 19:53 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 34668: remove binding/unbinding event for one submit event (3.33 KB, patch)
2023-10-17 12:53 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 34668: remove binding/unbinding event for one submit event (3.47 KB, patch)
2023-10-18 12:57 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 34668: Add popup warn librarians of waiting holds when checking out (9.42 KB, patch)
2023-10-18 13:13 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 34668: remove binding/unbinding event for one submit event (3.38 KB, patch)
2023-10-18 13:13 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 34668: pass whole waiting_holds rs (2.75 KB, patch)
2023-11-03 17:16 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 34668: Add popup warn librarians of waiting holds when checking out (9.47 KB, patch)
2023-11-06 14:18 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 34668: remove binding/unbinding event for one submit event (3.43 KB, patch)
2023-11-06 14:18 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 34668: pass whole waiting_holds rs (2.80 KB, patch)
2023-11-06 14:18 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 34668: Add popup warn librarians of waiting holds when checking out (9.53 KB, patch)
2024-01-08 23:44 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 34668: remove binding/unbinding event for one submit event (3.48 KB, patch)
2024-01-08 23:44 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 34668: pass whole waiting_holds rs (2.85 KB, patch)
2024-01-08 23:44 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 34668: (QA follow-up) Change file permission (737 bytes, patch)
2024-01-08 23:45 UTC, Lucas Gass
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Shi Yao Wang 2023-08-30 20:07:16 UTC
Add a modal popup when checking out items of a patron that has waiting holds available with a syspref to enable or disable the feature. Would make it more obvious than the simple display of "Holds waiting here" comment on top right of the checkout page.
Comment 1 Shi Yao Wang 2023-08-31 19:46:23 UTC Comment hidden (obsolete)
Comment 2 Shi Yao Wang 2023-08-31 19:56:37 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2023-09-01 12:13:21 UTC
This is really interesting, and seems to work well. I wonder if we should include an option to check a box in the modal that says "Don't warn me again" which would  suppress the warning for the current session?
Comment 4 Blou 2023-09-01 12:19:16 UTC
I think what would happen is that the clerks would hit it automatically (and blindly) after a while, especially those that were targeted by this library request.

Considering each popup would be for a different user, with different books listed, this is always new information.  Turning it off seems counterintuitive, in my personnal opinion.

Humans, humans...
Comment 5 Lucas Gass 2023-09-01 15:59:40 UTC
I think is going to be useful and it seems to work well. 

Can you check these QA errors:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
   FAIL	  filters
		missing_filter at line 967 (                    waiting_holds_barcodes.push("[% b %]");)


 WARN	circ/circulation.pl
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 240, now: 244)


For the perltidy warn maybe this is helpful: https://wiki.koha-community.org/wiki/Perltidy
Comment 6 Shi Yao Wang 2023-09-01 17:37:52 UTC Comment hidden (obsolete)
Comment 7 Shi Yao Wang 2023-09-01 17:38:44 UTC
QA errors should be fixed.
Comment 8 Matt Blenkinsop 2023-09-04 08:48:50 UTC
Created attachment 155188 [details] [review]
Bug 34668: Add popup warn librarians of waiting holds when checking out

When there are holds waiting for patrons, sometimes the librarian misses
the "Holds waiting here" display. This patch adds a modal popup warning
when checking out an item for a patron with waiting holds.

Test plan:
1) find a user (user1)
2) find a biblio (biblio1)
3) add a hold for biblio1 to user1
   (search biblio1 > Holds > find user1 > Place hold)
4) checkout biblio1 to another user if not already checked out
   and checkin through circulation page (not through the user page)
   > confirm hold
5) there should be a "Holds waiting here (1)" section added on user1 page

6) checkout any items that isn't the one on hold for user1
   -> notice it just checks out as normal
7) apply patch and update database
8) go to administration > system preferences > switch "WaitingNotifyAtCheckout" to "Notify"
9) refresh page and redo step 6
   -> notice there is now a modal to warn the librarian of a waiting hold
   click Ok to proceed with the checkout, click outside the modal to not do anything
10) additionnally, the popup shouldn't appear if we checkout any of
    user1's waiting hold

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 9 Nick Clemens 2023-10-12 17:31:27 UTC
Rather than unbinding/rebinding events, could we have an action bound to submit on the form, and then display the modal or not as needed? That would be more in line with current code I believe
Comment 10 Shi Yao Wang 2023-10-12 18:26:29 UTC
(In reply to Nick Clemens from comment #9)
> Rather than unbinding/rebinding events, could we have an action bound to
> submit on the form, and then display the modal or not as needed? That would
> be more in line with current code I believe

>$('#circ-warnwaitingholds-modal .btn-primary').on('click',function() {
>    $('#mainform').submit();
>});

This is the action bound to (confirm) submit the usual checkout form from the modal.


The modal display logic depends on what is written in the barcode input, it needs to display the modal if the barcode about to be checked out is not among waiting holds, else it will submit the form as usual.

I currently do not see how this can be done without binding/unbinding events of the "Check out" button depending on the barcode input. Can I get more details on how to do what you suggest?
Comment 11 Nick Clemens 2023-10-12 19:18:12 UTC
(In reply to Shi Yao Wang from comment #10)
> (In reply to Nick Clemens from comment #9)
> I currently do not see how this can be done without binding/unbinding events
> of the "Check out" button depending on the barcode input. Can I get more
> details on how to do what you suggest?

Something like this:

                $("#mainform").on('submit', function(){
                    if( $("#checkout_confirmed").length > 0 ){
                        return true;
                    }
                    if ( waiting_holds_barcodes.includes($('#barcode').val().trim()) ) {
                        return true;
                    } else {
                        $('#circ-warnwaitingholds-modal').modal();
                        return false;
                    }
                });
                $("#circ-warnwaitingholds-modal").on('hidden.bs.modal',function(){
                    $("#mainform").append('<input type="hidden" id="checkout_confirmed" value=1>').submit();
                });
Comment 12 Shi Yao Wang 2023-10-12 19:53:56 UTC Comment hidden (obsolete)
Comment 13 Mia Kujala 2023-10-17 10:39:13 UTC
I tried to signoff: Popup is showing but clicking Ok doesn't proceed the checkout, only popup disappears.
Comment 14 Shi Yao Wang 2023-10-17 12:53:41 UTC Comment hidden (obsolete)
Comment 15 Shi Yao Wang 2023-10-17 13:00:15 UTC
(In reply to Mia Kujala from comment #13)
> I tried to signoff: Popup is showing but clicking Ok doesn't proceed the
> checkout, only popup disappears.

Thank you for testing. It was indeed not proceeding with the checkout. It should be fixed now.
Comment 16 Mia Kujala 2023-10-18 11:57:49 UTC
Tested again. Now the popup is not showing but it proceed the checkout
Comment 17 Shi Yao Wang 2023-10-18 12:55:30 UTC
(In reply to Mia Kujala from comment #16)
> Tested again. Now the popup is not showing but it proceed the checkout

The popup shows up on my end... is the syspref active?
Also, the popup won't show if checking out an item waiting on hold.
Comment 18 Shi Yao Wang 2023-10-18 12:57:17 UTC
Created attachment 157312 [details] [review]
Bug 34668: remove binding/unbinding event for one submit event

step 9) of previous patch is a bit different now, if we click outside of
the modal it will proceed with the checkout instead of not doing
anything
Comment 19 Shi Yao Wang 2023-10-18 12:57:47 UTC
No new code changes, just changed the commit message.
Comment 20 PTFS Europe Sandboxes 2023-10-18 13:13:17 UTC
Created attachment 157315 [details] [review]
Bug 34668: Add popup warn librarians of waiting holds when checking out

When there are holds waiting for patrons, sometimes the librarian misses
the "Holds waiting here" display. This patch adds a modal popup warning
when checking out an item for a patron with waiting holds.

Test plan:
1) find a user (user1)
2) find a biblio (biblio1)
3) add a hold for biblio1 to user1
   (search biblio1 > Holds > find user1 > Place hold)
4) checkout biblio1 to another user if not already checked out
   and checkin through circulation page (not through the user page)
   > confirm hold
5) there should be a "Holds waiting here (1)" section added on user1 page

6) checkout any items that isn't the one on hold for user1
   -> notice it just checks out as normal
7) apply patch and update database
8) go to administration > system preferences > switch "WaitingNotifyAtCheckout" to "Notify"
9) refresh page and redo step 6
   -> notice there is now a modal to warn the librarian of a waiting hold
   click Ok to proceed with the checkout, click outside the modal to not do anything
10) additionnally, the popup shouldn't appear if we checkout any of
    user1's waiting hold

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Mia Kujala <mia.kujala@xamk.fi>
Comment 21 PTFS Europe Sandboxes 2023-10-18 13:13:19 UTC
Created attachment 157316 [details] [review]
Bug 34668: remove binding/unbinding event for one submit event

Signed-off-by: Mia Kujala <mia.kujala@xamk.fi>
Comment 22 Mia Kujala 2023-10-18 13:15:38 UTC
Works as designed and described in the test plan.
Comment 23 Kelly McElligott 2023-10-18 13:27:01 UTC
My sandbox is not sending the sign off through- but I have tested and it works as expected.
Comment 24 Kyle M Hall 2023-11-03 16:39:48 UTC
Comment on attachment 157315 [details] [review]
Bug 34668: Add popup warn librarians of waiting holds when checking out

Review of attachment 157315 [details] [review]:
-----------------------------------------------------------------

::: circ/circulation.pl
@@ +552,5 @@
> +        #Check for waiting holds
> +        my $waiting_holds          = $patron->holds->search( { found => 'W', branchcode => $branch } );
> +        my @waiting_holds_barcodes = ();
> +        while ( my $hold = $waiting_holds->next ) {
> +            push( @waiting_holds_barcodes, $hold->item->barcode );

There is no need to make a list of the barcodes, just pass the entire waiting_holds rs to the template and let it loop over them.
Comment 25 Shi Yao Wang 2023-11-03 17:16:50 UTC
Created attachment 158359 [details] [review]
Bug 34668: pass whole waiting_holds rs
Comment 26 ByWater Sandboxes 2023-11-06 14:18:13 UTC
Created attachment 158528 [details] [review]
Bug 34668: Add popup warn librarians of waiting holds when checking out

When there are holds waiting for patrons, sometimes the librarian misses
the "Holds waiting here" display. This patch adds a modal popup warning
when checking out an item for a patron with waiting holds.

Test plan:
1) find a user (user1)
2) find a biblio (biblio1)
3) add a hold for biblio1 to user1
   (search biblio1 > Holds > find user1 > Place hold)
4) checkout biblio1 to another user if not already checked out
   and checkin through circulation page (not through the user page)
   > confirm hold
5) there should be a "Holds waiting here (1)" section added on user1 page

6) checkout any items that isn't the one on hold for user1
   -> notice it just checks out as normal
7) apply patch and update database
8) go to administration > system preferences > switch "WaitingNotifyAtCheckout" to "Notify"
9) refresh page and redo step 6
   -> notice there is now a modal to warn the librarian of a waiting hold
   click Ok to proceed with the checkout, click outside the modal to not do anything
10) additionnally, the popup shouldn't appear if we checkout any of
    user1's waiting hold

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Mia Kujala <mia.kujala@xamk.fi>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Comment 27 ByWater Sandboxes 2023-11-06 14:18:15 UTC
Created attachment 158529 [details] [review]
Bug 34668: remove binding/unbinding event for one submit event

Signed-off-by: Mia Kujala <mia.kujala@xamk.fi>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Comment 28 ByWater Sandboxes 2023-11-06 14:18:18 UTC
Created attachment 158530 [details] [review]
Bug 34668: pass whole waiting_holds rs

Signed-off-by: Kelly <kelly@bywatersolutions.com>
Comment 29 Lucas Gass 2024-01-08 23:44:16 UTC
Working as described. QA script is mostly happy, 1 issue with file permissions that I am fixing in a follow-up. The rest of the code looks good. DBUpdate looks good.
Comment 30 Lucas Gass 2024-01-08 23:44:54 UTC
Created attachment 160673 [details] [review]
Bug 34668: Add popup warn librarians of waiting holds when checking out

When there are holds waiting for patrons, sometimes the librarian misses
the "Holds waiting here" display. This patch adds a modal popup warning
when checking out an item for a patron with waiting holds.

Test plan:
1) find a user (user1)
2) find a biblio (biblio1)
3) add a hold for biblio1 to user1
   (search biblio1 > Holds > find user1 > Place hold)
4) checkout biblio1 to another user if not already checked out
   and checkin through circulation page (not through the user page)
   > confirm hold
5) there should be a "Holds waiting here (1)" section added on user1 page

6) checkout any items that isn't the one on hold for user1
   -> notice it just checks out as normal
7) apply patch and update database
8) go to administration > system preferences > switch "WaitingNotifyAtCheckout" to "Notify"
9) refresh page and redo step 6
   -> notice there is now a modal to warn the librarian of a waiting hold
   click Ok to proceed with the checkout, click outside the modal to not do anything
10) additionnally, the popup shouldn't appear if we checkout any of
    user1's waiting hold

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Mia Kujala <mia.kujala@xamk.fi>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 31 Lucas Gass 2024-01-08 23:44:56 UTC
Created attachment 160674 [details] [review]
Bug 34668: remove binding/unbinding event for one submit event

Signed-off-by: Mia Kujala <mia.kujala@xamk.fi>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 32 Lucas Gass 2024-01-08 23:44:59 UTC
Created attachment 160675 [details] [review]
Bug 34668: pass whole waiting_holds rs

Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 33 Lucas Gass 2024-01-08 23:45:02 UTC
Created attachment 160676 [details] [review]
Bug 34668: (QA follow-up) Change file permission

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>