Description
Philip Orr
2025-03-31 13:31:47 UTC
I will write a patch to correct this and add it here. Test plan: 1. Before applying the patch, check how the display is currently. First set syspref PatronRestrictionTypes to "Allow" 2. Add two different new restriction types, fill out both "Code" and "Label" 3. Go to a patron's checkout page and add one of each of these restrictions, but do not fill out the comment field when adding them! 4. Observe that in the checkouts page and details page of the patron, it just says "Restricted since..." and does not show the restriction types. 5. Add another restriction, this time fill out the comment field 6. Observe that now in checkout and detail pages, the labels from the restriction types are listed and the comment shown. 7. Apply patch 8. Go to patron's details and checkout pages and check the restrictions are now displayed correctly, that is the text "with the explanation" is only shown for the restrictions that actually have a comment 8b. Log in to the OPAC with that patron and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 9. Go to a different patron's account and add a restriction without a comment 10. Check it is now displayed correctly 11. Add more restrictions and check they are displayed correctly. Also add an OVERDUES restriction: a. find a patron with an overdue item checked out (or create an overdue checkout on a patron's account) b. go to Tools -> Overdue notice/status triggers c. set up a rule for that patron's category, e.g. "First" -> Delay: 1, make sure to mark the check box for "Restrict" and "Print" d. run misc/cronjobs/overdue_notices.pl e. that patron should now also have an OVERDUES restriction 12. Turn PatronRestrictionTypes off again and check the patrons if everything is still displayed correctly 13. Go to another different patron and add a restriction with no comment, check if it is displayed correctly 14. Add another restriction with a comment and check it is displayed correctly 15. Log in to the OPAC with the patron that had OVERDUES restriction and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 16. Same thing for the other patron without the OVERDUES restriction. Created attachment 180126 [details] [review] Bug 39498: correct patron restrictions displays Test plan: 1. Before applying the patch, check how the display is currently. First set syspref PatronRestrictionTypes to "Allow" 2. Add two different new restriction types, fill out both "Code" and "Label" 3. Go to a patron's checkout page and add one of each of these restrictions, but do not fill out the comment field when adding them! 4. Observe that in the checkouts page and details page of the patron, it just says "Restricted since..." and does not show the restriction types. 5. Add another restriction, this time fill out the comment field 6. Observe that now in checkout and detail pages, the labels from the restriction types are listed and the comment shown. 7. Apply patch 8. Go to patron's details and checkout pages and check the restrictions are now displayed correctly, that is the text "with the explanation" is only shown for the restrictions that actually have a comment 8b. Log in to the OPAC with that patron and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 9. Go to a different patron's account and add a restriction without a comment 10. Check it is now displayed correctly 11. Add more restrictions and check they are displayed correctly. Also add an OVERDUES restriction: a. find a patron with an overdue item checked out (or create an overdue checkout on a patron's account) b. go to Tools -> Overdue notice/status triggers c. set up a rule for that patron's category, e.g. "First" -> Delay: 1, make sure to mark the check box for "Restrict" and "Print" d. run misc/cronjobs/overdue_notices.pl e. that patron should now also have an OVERDUES restriction 12. Turn PatronRestrictionTypes off again and check the patrons if everything is still displayed correctly 13. Go to another different patron and add a restriction with no comment, check if it is displayed correctly 14. Add another restriction with a comment and check it is displayed correctly 15. Log in to the OPAC with the patron that had OVERDUES restriction and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 16. Same thing for the other patron without the OVERDUES restriction. c. set up a rule for that patron's category, e.g. "First" -> Delay: 1, make sure to mark the check box for "Restrict" and "Print" You can add Letter = ODUE Created attachment 180272 [details] [review] Bug 39498: correct patron restrictions displays Test plan: 1. Before applying the patch, check how the display is currently. First set syspref PatronRestrictionTypes to "Allow" 2. Add two different new restriction types, fill out both "Code" and "Label" 3. Go to a patron's checkout page and add one of each of these restrictions, but do not fill out the comment field when adding them! 4. Observe that in the checkouts page and details page of the patron, it just says "Restricted since..." and does not show the restriction types. 5. Add another restriction, this time fill out the comment field 6. Observe that now in checkout and detail pages, the labels from the restriction types are listed and the comment shown. 7. Apply patch 8. Go to patron's details and checkout pages and check the restrictions are now displayed correctly, that is the text "with the explanation" is only shown for the restrictions that actually have a comment 8b. Log in to the OPAC with that patron and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 9. Go to a different patron's account and add a restriction without a comment 10. Check it is now displayed correctly 11. Add more restrictions and check they are displayed correctly. Also add an OVERDUES restriction: a. find a patron with an overdue item checked out (or create an overdue checkout on a patron's account) b. go to Tools -> Overdue notice/status triggers c. set up a rule for that patron's category, e.g. "First" -> Delay: 1, make sure to mark the check box for "Restrict" and "Print" d. run misc/cronjobs/overdue_notices.pl e. that patron should now also have an OVERDUES restriction 12. Turn PatronRestrictionTypes off again and check the patrons if everything is still displayed correctly 13. Go to another different patron and add a restriction with no comment, check if it is displayed correctly 14. Add another restriction with a comment and check it is displayed correctly 15. Log in to the OPAC with the patron that had OVERDUES restriction and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 16. Same thing for the other patron without the OVERDUES restriction. Signed-off-by: Claire Hernandez <claire.hernandez@biblibre.com> Created attachment 182301 [details] [review] Bug 39498: correct patron restrictions displays Test plan: 1. Before applying the patch, check how the display is currently. First set syspref PatronRestrictionTypes to "Allow" 2. Add two different new restriction types, fill out both "Code" and "Label" 3. Go to a patron's checkout page and add one of each of these restrictions, but do not fill out the comment field when adding them! 4. Observe that in the checkouts page and details page of the patron, it just says "Restricted since..." and does not show the restriction types. 5. Add another restriction, this time fill out the comment field 6. Observe that now in checkout and detail pages, the labels from the restriction types are listed and the comment shown. 7. Apply patch 8. Go to patron's details and checkout pages and check the restrictions are now displayed correctly, that is the text "with the explanation" is only shown for the restrictions that actually have a comment 8b. Log in to the OPAC with that patron and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 9. Go to a different patron's account and add a restriction without a comment 10. Check it is now displayed correctly 11. Add more restrictions and check they are displayed correctly. Also add an OVERDUES restriction: a. find a patron with an overdue item checked out (or create an overdue checkout on a patron's account) b. go to Tools -> Overdue notice/status triggers c. set up a rule for that patron's category, e.g. "First" -> Delay: 1, make sure to mark the check box for "Restrict" and "Print" d. run misc/cronjobs/overdue_notices.pl e. that patron should now also have an OVERDUES restriction 12. Turn PatronRestrictionTypes off again and check the patrons if everything is still displayed correctly 13. Go to another different patron and add a restriction with no comment, check if it is displayed correctly 14. Add another restriction with a comment and check it is displayed correctly 15. Log in to the OPAC with the patron that had OVERDUES restriction and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 16. Same thing for the other patron without the OVERDUES restriction. Signed-off-by: Claire Hernandez <claire.hernandez@biblibre.com> Hi Claire, it looks like you have signed off this bug (which is great, thanks!). To get the credit on the dashboard, please change the status to Signed Off. David Claire had set it to signed off, I set the status back to "Needs Signoff" by accident after I rebased this one - sorry about that - setting back to "Signed Off" :) Oh and I guess I should mention, it's rebased now. Excellent!. Thanks Philip. Created attachment 182425 [details]
Screenshots
I'm not sure I like this change to be honest.. I find it less clear..
I'd be interested in others opinions.. I'd be tempted to just drop the 'with explanation' text from the top and display the original list as a list as before.. I likes that the dates/indefinite lined up the the beginning, followed by restriction type, followed by the optional comment.
Your change, to me, makes it harder to parse at a glance to me... seeking others opinions here.
Created attachment 182426 [details] [review] Bug 39498: correct patron restrictions displays Test plan: 1. Before applying the patch, check how the display is currently. First set syspref PatronRestrictionTypes to "Allow" 2. Add two different new restriction types, fill out both "Code" and "Label" 3. Go to a patron's checkout page and add one of each of these restrictions, but do not fill out the comment field when adding them! 4. Observe that in the checkouts page and details page of the patron, it just says "Restricted since..." and does not show the restriction types. 5. Add another restriction, this time fill out the comment field 6. Observe that now in checkout and detail pages, the labels from the restriction types are listed and the comment shown. 7. Apply patch 8. Go to patron's details and checkout pages and check the restrictions are now displayed correctly, that is the text "with the explanation" is only shown for the restrictions that actually have a comment 8b. Log in to the OPAC with that patron and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 9. Go to a different patron's account and add a restriction without a comment 10. Check it is now displayed correctly 11. Add more restrictions and check they are displayed correctly. Also add an OVERDUES restriction: a. find a patron with an overdue item checked out (or create an overdue checkout on a patron's account) b. go to Tools -> Overdue notice/status triggers c. set up a rule for that patron's category, e.g. "First" -> Delay: 1, make sure to mark the check box for "Restrict" and "Print" d. run misc/cronjobs/overdue_notices.pl e. that patron should now also have an OVERDUES restriction 12. Turn PatronRestrictionTypes off again and check the patrons if everything is still displayed correctly 13. Go to another different patron and add a restriction with no comment, check if it is displayed correctly 14. Add another restriction with a comment and check it is displayed correctly 15. Log in to the OPAC with the patron that had OVERDUES restriction and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 16. Same thing for the other patron without the OVERDUES restriction. Signed-off-by: Claire Hernandez <claire.hernandez@biblibre.com> To get this in a state ready for QA I had to rework it very slightly as we had invalid HTML getting generated and thus the tidy scripts weren't applying. I have to admit that the first version seems easier to scan at a busy circulation desk. I'll see if I can find time next week to try and rework the display. .. and thanks Martin for the QA rework! :) Created attachment 182437 [details]
Another proposal
I've been tinkering with this too, trying to make it clearer.
I notice it displays expired restrictions. Has it always been that way?
Still haven't had a proper look but quick reply here... (In reply to Owen Leonard from comment #17) > I notice it displays expired restrictions. Has it always been that way? I believe so. The Manual says: "Expired restrictions are marked as such as of Koha version 23.11. In prior versions, if a restriction was expired, it looked the same as an active restriction, but it stopped blocking circulation nonetheless." https://koha-community.org/manual/latest/en/html/patrons.html#restrictions Some libraries use cleanup_database.pl to delete old expired restrictions. Thanks Owen, that looks great to me - reads easier to me at a glance than the way it was before! And definitely better than my own attempt. Created attachment 182491 [details] [review] Bug 39498: correct patron restrictions displays This patch updates the way patron restrictions are displayed in the OPAC and staff interface, with the intention of making the details of each restriction more clear. In the OPAC, a new include file is created to be reused on the holds page and the user summary page. opac-user.pl is updated to allow a parameter to be passed to automatically open the "Overdues" tab. Test plan (copied from the original patch): 1. Before applying the patch, check how the display is currently. First set syspref PatronRestrictionTypes to "Allow" 2. Add two different new restriction types, fill out both "Code" and "Label" 3. Go to a patron's checkout page and add one of each of these restrictions, but do not fill out the comment field when adding them! 4. Observe that in the checkouts page and details page of the patron, it just says "Restricted since..." and does not show the restriction types. 5. Add another restriction, this time fill out the comment field 6. Observe that now in checkout and detail pages, the labels from the restriction types are listed and the comment shown. 7. Apply patch 8. Go to patron's details and checkout pages and check the restrictions are now displayed correctly, that is the text "with the explanation" is only shown for the restrictions that actually have a comment 8b. Log in to the OPAC with that patron and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 9. Go to a different patron's account and add a restriction without a comment 10. Check it is now displayed correctly 11. Add more restrictions and check they are displayed correctly. Also add an OVERDUES restriction: 11a. Find a patron with an overdue item checked out (or create an overdue checkout on a patron's account) 11b. Go to Tools -> Overdue notice/status triggers 11c. Set up a rule for that patron's category, e.g. "First" -> Delay: 1, make sure to mark the check box for "Restrict" and "Print" 11d. Run misc/cronjobs/overdue_notices.pl 11e. That patron should now also have an OVERDUES restriction 12. Turn PatronRestrictionTypes off again and check the patrons if everything is still displayed correctly 13. Go to another different patron and add a restriction with no comment, check if it is displayed correctly 14. Add another restriction with a comment and check it is displayed correctly 15. Log in to the OPAC with the patron that had OVERDUES restriction and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 16. Same thing for the other patron without the OVERDUES restriction. Created attachment 182609 [details] [review] Bug 39498: Correct patron restrictions displays This patch updates the way patron restrictions are displayed in the OPAC and staff interface, with the intention of making the details of each restriction more clear. In the OPAC, a new include file is created to be reused on the holds page and the user summary page. opac-user.pl is updated to allow a parameter to be passed to automatically open the "Overdues" tab. Test plan: 1. Before applying the patch, check how the display is currently. First set syspref PatronRestrictionTypes to "Allow" 2. Add two different new restriction types, fill out both "Code" and "Label" 3. Go to a patron's checkout page and add one of each of these restrictions, but do not fill out the comment field when adding them! 4. Observe that in the checkouts page and details page of the patron, it just says "Restricted since..." and does not show the restriction types. 5. Add another restriction, this time fill out the comment field 6. Observe that now in checkout and detail pages, the labels from the restriction types are listed and the comment shown. 7. Apply patch 8. Go to patron's details and checkout pages and check the restrictions are now displayed correctly, that is the text "with the explanation" is only shown for the restrictions that actually have a comment 8b. Log in to the OPAC with that patron and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 9. Go to a different patron's account and add a restriction without a comment 10. Check it is now displayed correctly 11. Add more restrictions and check they are displayed correctly. Also add an OVERDUES restriction: 11a. Find a patron with an overdue item checked out (or create an overdue checkout on a patron's account) 11b. Go to Tools -> Overdue notice/status triggers 11c. Set up a rule for that patron's category, e.g. "First" -> Delay: 1, make sure to mark the check box for "Restrict" and "Print" 11d. Run misc/cronjobs/overdue_notices.pl 11e. That patron should now also have an OVERDUES restriction 12. Turn PatronRestrictionTypes off again and check the patrons if everything is still displayed correctly 13. Go to another different patron and add a restriction with no comment, check if it is displayed correctly 14. Add another restriction with a comment and check it is displayed correctly 15. Log in to the OPAC with the patron that had OVERDUES restriction and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 16. Same thing for the other patron without the OVERDUES restriction. Signed-off-by: David Nind <david@davidnind.com> Wow, Owens looks great and I love that it puts it up front and centre how to resolve the restrictions. Created attachment 183539 [details] [review] Bug 39498: Correct patron restrictions displays This patch updates the way patron restrictions are displayed in the OPAC and staff interface, with the intention of making the details of each restriction more clear. In the OPAC, a new include file is created to be reused on the holds page and the user summary page. opac-user.pl is updated to allow a parameter to be passed to automatically open the "Overdues" tab. Test plan: 1. Before applying the patch, check how the display is currently. First set syspref PatronRestrictionTypes to "Allow" 2. Add two different new restriction types, fill out both "Code" and "Label" 3. Go to a patron's checkout page and add one of each of these restrictions, but do not fill out the comment field when adding them! 4. Observe that in the checkouts page and details page of the patron, it just says "Restricted since..." and does not show the restriction types. 5. Add another restriction, this time fill out the comment field 6. Observe that now in checkout and detail pages, the labels from the restriction types are listed and the comment shown. 7. Apply patch 8. Go to patron's details and checkout pages and check the restrictions are now displayed correctly, that is the text "with the explanation" is only shown for the restrictions that actually have a comment 8b. Log in to the OPAC with that patron and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 9. Go to a different patron's account and add a restriction without a comment 10. Check it is now displayed correctly 11. Add more restrictions and check they are displayed correctly. Also add an OVERDUES restriction: 11a. Find a patron with an overdue item checked out (or create an overdue checkout on a patron's account) 11b. Go to Tools -> Overdue notice/status triggers 11c. Set up a rule for that patron's category, e.g. "First" -> Delay: 1, make sure to mark the check box for "Restrict" and "Print" 11d. Run misc/cronjobs/overdue_notices.pl 11e. That patron should now also have an OVERDUES restriction 12. Turn PatronRestrictionTypes off again and check the patrons if everything is still displayed correctly 13. Go to another different patron and add a restriction with no comment, check if it is displayed correctly 14. Add another restriction with a comment and check it is displayed correctly 15. Log in to the OPAC with the patron that had OVERDUES restriction and check they are displayed correctly in their summary page and when the patron is trying to place a hold. 16. Same thing for the other patron without the OVERDUES restriction. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Nice work everyone! Pushed to main for 25.11 |