Bug 39624

Summary: Add an "All" patron category option for the "Default open article requests limit" circulation rules
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: CirculationAssignee: Marcel de Rooy <m.de.rooy>
Status: Pushed to main --- QA Contact: Lucas Gass (lukeg) <lucas>
Severity: enhancement    
Priority: P5 - low CC: david, gmcharlt, kyle.m.hall, lucas
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
This fixes the "Default open article requests limit" for the circulation and fine rules, so that the patron category drop-down list has an "All" option. Previously, there was no "All" patron category option, and you needed to set rules individually for each patron category. This requires the `ArticleRequests` system preference enabled.
Version(s) released in:
25.05.00
Circulation function: Article requests
Bug Depends on: 27945    
Bug Blocks:    
Attachments: Bug 39624: Add patron category * (All) to open_article_requests limit
Bug 39624: (follow-up) Fix deleting the All line
Bug 39624: Add patron category * (All) to open_article_requests limit
Bug 39624: (follow-up) Fix deleting the All line
Bug 39624: Add patron category * (All) to open_article_requests limit
Bug 39624: (follow-up) Fix deleting the All line

Description Marcel de Rooy 2025-04-14 08:00:58 UTC
All code is present to let this work. We only need a few lines in the template to get it working.
Comment 1 Marcel de Rooy 2025-04-14 08:07:30 UTC
Created attachment 180901 [details] [review]
Bug 39624: Add patron category * (All) to open_article_requests limit

Test plan:
Go to smart rules.
Add a limit for All.
Refresh and check if it is still there.
Enable ArticleRequests and verify if the limit is working.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 David Nind 2025-04-14 11:07:07 UTC
Could you please expand the test plan, it is not clear to me at all. 

Where do you add a "limit for All"? 

There is no "open_article_requests" on the page (Koha administration > Patrons and circulation > Circulation and fine rules).
Comment 3 Marcel de Rooy 2025-04-14 11:37:56 UTC
(In reply to David Nind from comment #2)
> Could you please expand the test plan, it is not clear to me at all. 
> 
> Where do you add a "limit for All"? 
> 
> There is no "open_article_requests" on the page (Koha administration >
> Patrons and circulation > Circulation and fine rules).

Hi David,
Scroll down on the admin/smart-rules.pl form.
Look for: Default open article requests limit
Comment 4 David Nind 2025-04-14 22:06:49 UTC
I have Failed QA.

With the patch applied, you cannot delete the "All" rule:
1. If you attempt to delete the "All" rule: the page refreshes, but the rule is still there.
2. It works correctly before the patch for individual patron category rules.
3. It still works correctly after the patch for individual patron category rules.
4. It does not work after the patch for the "All" rule.

In addition, both before and after the patch, when cancelling article requests using the staff interface, a progress spinner is displayed (this should probably be a separate bug):
1. Place some article requests
2. Go to Circulation > Patron request > Article requests
3. Select a request and use either the "Cancel request" option in the action column or the "Actions > Cancel request" button next to the filters
4. Note that a progress spinner is displayed, and the page does not refresh
5. If you refresh the page (or click the sidebar menu), the cancelled request is no longer shown

Testing notes (using KTD):
1. Enable the ArticleRequests system preference (without this, you cannot see the settings for "Default open article requests limit" in step 3.
2. Go to Koha administration > Patrons and circulation > Circulation and fine rules.
3. Go to the section on the page with the heading "Default open article requests limit".
4. Note that for the dropdown list for "Patron category" there is no "All" option.
5. Apply the patch.
6. Repeat steps 3 and 4 and note that there is now an "All" option.
7. Test that the "All" limit works as expected:
   7.1 Add an "All" limit for article requests:
       - Patron category: select "All" (the default)
       - Total article requests: enter "2"
       - Click "+ Add"
   7.2 Place multiple requests in the OPAC, for example on record 5, Perl best practices, for a patron (I used the koha user):
       - Once the limit is exceeded (on your third request) you should get a message "You reached your open article requests limit."
8. Test that the "All" limit and a different limit for another patron category works as expected:
   8.1 Cancel all the requests already placed
   8.2 Add a limit for another patron category, I used "Staff"
       - Patron category: select "Staff"
       - Total article requests: enter "1"
       - Click "+ Add"
       (We now have two rules - All = 2, Staff = 1)
   8.3 As the koha user, attempt to place 2 holds:
       ==> On the second request you should get the message "You reached your open article requests limit."
   8.4 As another user in any patron category except "Staff", attempt to place 3 holds (you will need to change the password so you can log in to the OPAC, I used Mary Burton):
       ==> On the third request you should get the message "You reached your open article requests limit."
Comment 5 Marcel de Rooy 2025-04-15 13:28:43 UTC
Created attachment 180961 [details] [review]
Bug 39624: (follow-up) Fix deleting the All line

Test plan:
Add a line for All categories.
Delete it again. Check when refreshing the page.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2025-04-15 13:33:22 UTC
(In reply to David Nind from comment #4)

Thx for testing.

> With the patch applied, you cannot delete the "All" rule:
> 1. If you attempt to delete the "All" rule: the page refreshes, but the rule
> is still there.
> 4. It does not work after the patch for the "All" rule.

Should be fixed now.

> In addition, both before and after the patch, when cancelling article
> requests using the staff interface, a progress spinner is displayed (this
> should probably be a separate bug):
> 1. Place some article requests
> 2. Go to Circulation > Patron request > Article requests
> 3. Select a request and use either the "Cancel request" option in the action
> column or the "Actions > Cancel request" button next to the filters
> 4. Note that a progress spinner is displayed, and the page does not refresh
> 5. If you refresh the page (or click the sidebar menu), the cancelled
> request is no longer shown

I saw the spinner a very short time. Request was cancelled. Seems to be OK for me.
Not sure, but do you need to run build resources again for js stuff?
Comment 7 Brendan Gallagher 2025-04-15 22:36:19 UTC
Created attachment 180997 [details] [review]
Bug 39624: Add patron category * (All) to open_article_requests limit

Test plan:
Go to smart rules.
Add a limit for All.
Refresh and check if it is still there.
Enable ArticleRequests and verify if the limit is working.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 8 Brendan Gallagher 2025-04-15 22:36:21 UTC
Created attachment 180998 [details] [review]
Bug 39624: (follow-up) Fix deleting the All line

Test plan:
Add a line for All categories.
Delete it again. Check when refreshing the page.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 9 Lucas Gass (lukeg) 2025-04-17 19:36:39 UTC
Created attachment 181137 [details] [review]
Bug 39624: Add patron category * (All) to open_article_requests limit

Test plan:
Go to smart rules.
Add a limit for All.
Refresh and check if it is still there.
Enable ArticleRequests and verify if the limit is working.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 10 Lucas Gass (lukeg) 2025-04-17 19:36:41 UTC
Created attachment 181138 [details] [review]
Bug 39624: (follow-up) Fix deleting the All line

Test plan:
Add a line for All categories.
Delete it again. Check when refreshing the page.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 11 Marcel de Rooy 2025-04-18 06:03:40 UTC
(In reply to Brendan Gallagher from comment #8)
> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Thanks!
Comment 12 Marcel de Rooy 2025-04-18 06:04:11 UTC
(In reply to Lucas Gass (lukeg) from comment #10)

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

Thanks! Did you mean to pass QA too btw ?
Comment 13 Lucas Gass (lukeg) 2025-04-18 14:12:11 UTC
(In reply to Marcel de Rooy from comment #12)
> (In reply to Lucas Gass (lukeg) from comment #10)
> 
> > Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
> 
> Thanks! Did you mean to pass QA too btw ?

Yes, I did! Setting it PQA now.
Comment 14 Katrin Fischer 2025-04-22 06:41:16 UTC
I agree that this is an improvement, but I am not sure it qualifies as a bug fix. :) Still up to to RMaints to backport anyway!
Comment 15 Katrin Fischer 2025-04-22 06:50:50 UTC
Pushed for 25.05!

Well done everyone, thank you!