Bug 36060 - If issues table includes overdues 'Renew selected items' button is disabled
Summary: If issues table includes overdues 'Renew selected items' button is disabled
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Lucas Gass (lukeg)
QA Contact: Victor Grousset/tuxayo
URL:
Keywords: rel_23_11_candidate
: 35709 (view as bug list)
Depends on: 35068 36418
Blocks:
  Show dependency treegraph
 
Reported: 2024-02-09 17:37 UTC by Lucas Gass (lukeg)
Modified: 2024-09-30 17:32 UTC (History)
7 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:
24.05.00
Circulation function:


Attachments
Bug 36060: Simplify logic to enable/disable CheckinChecked/RenewChecked buttons (6.18 KB, patch)
2024-02-09 18:20 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36060: Simplify logic to enable/disable CheckinChecked/RenewChecked buttons (6.23 KB, patch)
2024-02-26 15:43 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 36060: Simplify logic to enable/disable CheckinChecked/RenewChecked buttons (6.18 KB, patch)
2024-03-01 15:42 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36060: (follow-up) Remove unnessesary code (1.63 KB, patch)
2024-03-27 21:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36060: (follow-up) Remember selections when refreshing the table (1.17 KB, patch)
2024-05-14 18:18 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 36060: Simplify logic to enable/disable CheckinChecked/RenewChecked buttons (6.23 KB, patch)
2024-05-20 19:25 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36060: (follow-up) Remember selections when refreshing the table (1.23 KB, patch)
2024-05-20 19:25 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass (lukeg) 2024-02-09 17:37:45 UTC
Bug 35068 split the renewal and checkout buttons. I did not account for the fact that if items are overdue the 'Renew' button for that item is checked by default. 

Once the issue table is loaded there should be a check to see if either renew or checkin checkboxes are checked and enable or disable the 'Renew selected items' and 'Check in selected items'

To recreate: 
1. Have overdues 
2. Go to a patron account and load the issue table 
3. The items that are overdue are checked by default
4. 'Renew selected items' is disabled, you have to toggle the checkboxes to enable the button.
Comment 1 Lucas Gass (lukeg) 2024-02-09 18:20:31 UTC
Created attachment 161991 [details] [review]
Bug 36060: Simplify logic to enable/disable CheckinChecked/RenewChecked buttons

This patch attempts to simplify for disabling/enabling the 'Check in selected items' and 'Renewal selected items' buttons.

To test:
1. APPLY patch, clear browser's cache
2. Have more than 1 checkout issued to a borrower.
3. Load the checkout table on circ/circulation.pl
4. Notice that there is now 'Renew selected items' and 'Checkin
   selected items' button.
5. Play around with each making sure that they work as excepted when
   checkouts are checked in either the 'Check in' or 'Renew' columns.
6. Uncheck everything in the 'Renew column', the 'Renew selected items'
   button should be disabled.
7. Uncheck everything in the 'Checkin column', the 'Checkin selected
   items' button should be disabled.
8. Try unchecking some of these to make sure the Renew/Checkin button is
   properly disabled when no related checkboxes are checked.
9. Try using the select all/none options at the top of both the 'Renew'
   and 'Checkin' columns. Make sure the 'Renew selected items' and
   'Checkin selected items' become properly disabled/enabled.
10. Make sure the 'Renew all' button still works properly.

11. Have two checkouts. Click one for Renew and one for Checkin. Click on Check in selected items.
12. When the table reloads the 'renew' selections should be retained
13. Have two checkouts. Click one for Renew and one for Checkin. Click on Renew selected items.
14. When the table reloads the 'check in' selections should be retained

15. Have overdues
16. Go to a patron account and load the issue table
17. The items that are overdue are checked for renewal by default
16. The 'Renew selected items' button show be enabled.
Comment 2 Brendan Lawlor 2024-02-26 15:43:56 UTC
Created attachment 162459 [details] [review]
Bug 36060: Simplify logic to enable/disable CheckinChecked/RenewChecked buttons

This patch attempts to simplify for disabling/enabling the 'Check in selected items' and 'Renewal selected items' buttons.

To test:
1. APPLY patch, clear browser's cache
2. Have more than 1 checkout issued to a borrower.
3. Load the checkout table on circ/circulation.pl
4. Notice that there is now 'Renew selected items' and 'Checkin
   selected items' button.
5. Play around with each making sure that they work as excepted when
   checkouts are checked in either the 'Check in' or 'Renew' columns.
6. Uncheck everything in the 'Renew column', the 'Renew selected items'
   button should be disabled.
7. Uncheck everything in the 'Checkin column', the 'Checkin selected
   items' button should be disabled.
8. Try unchecking some of these to make sure the Renew/Checkin button is
   properly disabled when no related checkboxes are checked.
9. Try using the select all/none options at the top of both the 'Renew'
   and 'Checkin' columns. Make sure the 'Renew selected items' and
   'Checkin selected items' become properly disabled/enabled.
10. Make sure the 'Renew all' button still works properly.

11. Have two checkouts. Click one for Renew and one for Checkin. Click on Check in selected items.
12. When the table reloads the 'renew' selections should be retained
13. Have two checkouts. Click one for Renew and one for Checkin. Click on Renew selected items.
14. When the table reloads the 'check in' selections should be retained

15. Have overdues
16. Go to a patron account and load the issue table
17. The items that are overdue are checked for renewal by default
16. The 'Renew selected items' button show be enabled.

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 3 Brendan Lawlor 2024-02-26 15:54:30 UTC
This worked as described, but I just noticed one small edge case with the overdues.

Because the renew boxes are checked by default, if you change the selection on an overdue to checkin and then click the renew selected items it reloads with the default renew checkboxes checked for the overdues, but also retains the change you made to select the checkin checkbox as selected. 

It's not a problem if you don't have overdues, or if after you check a checkin boxes for overdues you use the checkin selected button first before using the renew selected button.
Comment 4 Lucas Gass (lukeg) 2024-02-28 17:43:26 UTC
(In reply to Brendan Lawlor from comment #3)
> This worked as described, but I just noticed one small edge case with the
> overdues.
> 
> Because the renew boxes are checked by default, if you change the selection
> on an overdue to checkin and then click the renew selected items it reloads
> with the default renew checkboxes checked for the overdues, but also retains
> the change you made to select the checkin checkbox as selected. 
> 
> It's not a problem if you don't have overdues, or if after you check a
> checkin boxes for overdues you use the checkin selected button first before
> using the renew selected button.

Brendan,

With this patch applied I am unable to recreate this problem.
Comment 5 Brendan Lawlor 2024-02-29 14:52:27 UTC
I'm not sure, but maybe the bug I saw where the checkout table reloaded with both the renew and checkin box checked for a record could have been caused by circ rules that I set up when I tested. I set the circ rule unit to hours. I am going to test again with the default ktd circ rules and see if the behavior is different.

If it's not the circ rules, maybe it is just caused by the user following a specific, admittedly goofy sequence of actions.


To reproduce:
1. Set the circ rule unit to hours
2. Have a few checkouts with at least one overdue
3. Load the checkout table and see the overdue item's renew checkbox is checked by default
4. Change the overdue item's checkbox to check the Check In checkbox instead
5. Check the Renew checkbox for a different item
6. Click the Renew selected items button
7. When the table reloads see both the Renew and Check In checkboxes are checked for the overdue item
Comment 6 Lucas Gass (lukeg) 2024-02-29 21:32:51 UTC
(In reply to Brendan Lawlor from comment #5)
> I'm not sure, but maybe the bug I saw where the checkout table reloaded with
> both the renew and checkin box checked for a record could have been caused
> by circ rules that I set up when I tested. I set the circ rule unit to
> hours. I am going to test again with the default ktd circ rules and see if
> the behavior is different.
> 
> If it's not the circ rules, maybe it is just caused by the user following a
> specific, admittedly goofy sequence of actions.
> 
> 
> To reproduce:
> 1. Set the circ rule unit to hours
> 2. Have a few checkouts with at least one overdue
> 3. Load the checkout table and see the overdue item's renew checkbox is
> checked by default
> 4. Change the overdue item's checkbox to check the Check In checkbox instead
> 5. Check the Renew checkbox for a different item
> 6. Click the Renew selected items button
> 7. When the table reloads see both the Renew and Check In checkboxes are
> checked for the overdue item

Sorry Brendan, I am still unable to reproduce this problem following your test plan.
Comment 7 Lucas Gass (lukeg) 2024-02-29 21:55:05 UTC
I've tried reproducing the problem as Brendan describes in both Firefox and Chrome without success.
Comment 8 Brendan Lawlor 2024-03-01 14:35:45 UTC
Hi Lucas, sorry for the false report and thanks for your time chatting with me.

I retested this bug this morning and could not reproduce it. So then I reset git and tested again without the bug applied and then I could see it.

So I think what happened was I didn't have the bug applied or had applied the wrong bug by accident when I first saw the bug.

My bad, status back to signed off.
Comment 9 Lucas Gass (lukeg) 2024-03-01 14:41:52 UTC
(In reply to Brendan Lawlor from comment #8)
> Hi Lucas, sorry for the false report and thanks for your time chatting with
> me.
> 
> I retested this bug this morning and could not reproduce it. So then I reset
> git and tested again without the bug applied and then I could see it.
> 
> So I think what happened was I didn't have the bug applied or had applied
> the wrong bug by accident when I first saw the bug.
> 
> My bad, status back to signed off.

I appreciate your testing, Brendan. Thanks!
Comment 10 Lucas Gass (lukeg) 2024-03-01 15:08:15 UTC
Needs rebased now that Bug 34478 is in,
Comment 11 Lucas Gass (lukeg) 2024-03-01 15:42:41 UTC
Created attachment 162673 [details] [review]
Bug 36060: Simplify logic to enable/disable CheckinChecked/RenewChecked buttons

This patch attempts to simplify for disabling/enabling the 'Check in selected items' and 'Renewal selected items' buttons.

To test:
1. APPLY patch, clear browser's cache
2. Have more than 1 checkout issued to a borrower.
3. Load the checkout table on circ/circulation.pl
4. Notice that there is now 'Renew selected items' and 'Checkin
   selected items' button.
5. Play around with each making sure that they work as excepted when
   checkouts are checked in either the 'Check in' or 'Renew' columns.
6. Uncheck everything in the 'Renew column', the 'Renew selected items'
   button should be disabled.
7. Uncheck everything in the 'Checkin column', the 'Checkin selected
   items' button should be disabled.
8. Try unchecking some of these to make sure the Renew/Checkin button is
   properly disabled when no related checkboxes are checked.
9. Try using the select all/none options at the top of both the 'Renew'
   and 'Checkin' columns. Make sure the 'Renew selected items' and
   'Checkin selected items' become properly disabled/enabled.
10. Make sure the 'Renew all' button still works properly.

11. Have two checkouts. Click one for Renew and one for Checkin. Click on Check in selected items.
12. When the table reloads the 'renew' selections should be retained
13. Have two checkouts. Click one for Renew and one for Checkin. Click on Renew selected items.
14. When the table reloads the 'check in' selections should be retained

15. Have overdues
16. Go to a patron account and load the issue table
17. The items that are overdue are checked for renewal by default
16. The 'Renew selected items' button show be enabled.

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 12 Victor Grousset/tuxayo 2024-03-24 04:09:26 UTC
Found unused variables left:
    var renewchecked
    var checkinchecked

---

minor thing: I don't get how `$("#RenewChecked").on("click"` works correctly without CheckRenewCheckinBoxes() when `$("#CheckinChecked").on("click"` needs it.
Comment 13 Lucas Gass (lukeg) 2024-03-25 16:38:04 UTC
Went to look at QA concerns and noticed Bug 36418, making this one a dependency of that one.
Comment 14 Lucas Gass (lukeg) 2024-03-27 21:13:34 UTC
(In reply to Victor Grousset/tuxayo from comment #12)
> Found unused variables left:
>     var renewchecked
>     var checkinchecked
> 
> ---
> 
> minor thing: I don't get how `$("#RenewChecked").on("click"` works correctly
> without CheckRenewCheckinBoxes() when `$("#CheckinChecked").on("click"`
> needs it.

Neither should actually need it because CheckRenewCheckinBoxes() is called in RefreshIssuesTable(). We also don't need the following in either of those on click functions:

$('#RenewChecked, #CheckinChecked').prop('disabled' , true );


Thanks Victor!
Comment 15 Lucas Gass (lukeg) 2024-03-27 21:15:43 UTC
Created attachment 164036 [details] [review]
Bug 36060: (follow-up) Remove unnessesary code
Comment 16 Victor Grousset/tuxayo 2024-03-31 21:05:31 UTC
> 13. Have two checkouts. Click one for Renew and one for Checkin. Click on Renew selected items.
> 14. When the table reloads the 'check in' selections should be retained

Oh, no :(

I tried many combinations and I never got checkin to stay when having done a renew.
Comment 17 Lucas Gass (lukeg) 2024-05-14 18:18:55 UTC
Created attachment 166723 [details] [review]
Bug 36060: (follow-up) Remember selections when refreshing the table
Comment 18 Victor Grousset/tuxayo 2024-05-20 19:25:56 UTC
Created attachment 166942 [details] [review]
Bug 36060: Simplify logic to enable/disable CheckinChecked/RenewChecked buttons

This patch attempts to simplify for disabling/enabling the 'Check in selected items' and 'Renewal selected items' buttons.

To test:
1. APPLY patch, clear browser's cache
2. Have more than 1 checkout issued to a borrower.
3. Load the checkout table on circ/circulation.pl
4. Notice that there is now 'Renew selected items' and 'Checkin
   selected items' button.
5. Play around with each making sure that they work as excepted when
   checkouts are checked in either the 'Check in' or 'Renew' columns.
6. Uncheck everything in the 'Renew column', the 'Renew selected items'
   button should be disabled.
7. Uncheck everything in the 'Checkin column', the 'Checkin selected
   items' button should be disabled.
8. Try unchecking some of these to make sure the Renew/Checkin button is
   properly disabled when no related checkboxes are checked.
9. Try using the select all/none options at the top of both the 'Renew'
   and 'Checkin' columns. Make sure the 'Renew selected items' and
   'Checkin selected items' become properly disabled/enabled.
10. Make sure the 'Renew all' button still works properly.

11. Have two checkouts. Click one for Renew and one for Checkin. Click on Check in selected items.
12. When the table reloads the 'renew' selections should be retained
13. Have two checkouts. Click one for Renew and one for Checkin. Click on Renew selected items.
14. When the table reloads the 'check in' selections should be retained

15. Have overdues
16. Go to a patron account and load the issue table
17. The items that are overdue are checked for renewal by default
16. The 'Renew selected items' button show be enabled.

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 19 Victor Grousset/tuxayo 2024-05-20 19:25:58 UTC
Created attachment 166943 [details] [review]
Bug 36060: (follow-up) Remember selections when refreshing the table

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 20 Victor Grousset/tuxayo 2024-05-20 19:27:40 UTC
> 4. Notice that there is now 'Renew selected items' and 'Checkin
>   selected items' button.

I'm assuming it's a copypasta from Bug 35068 test plan.



Works, makes sense, QA script happy, code looks good, passing QA :)
Comment 21 Katrin Fischer 2024-05-21 11:17:35 UTC
No string changes, decided to push.
Comment 22 Katrin Fischer 2024-05-21 11:41:14 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 23 Fridolin Somers 2024-05-29 13:31:35 UTC
Depends on Bug 36418 not in 23.11.x
Comment 24 Baptiste Wojtkowski (bwoj) 2024-09-27 08:38:38 UTC
*** Bug 35709 has been marked as a duplicate of this bug. ***
Comment 25 Lucas Gass (lukeg) 2024-09-30 17:32:47 UTC
Fixes a bug, nothing to document.