The patron search results in the patron card creator card batch doesn't include the value of the PatronsPerPage system preference in the per page drop down, which means it isn't using the system preference. Also, on the Patrons page, the patron search includes the PatronsPerPage value, but doesn't use it as the default.
Created attachment 123848 [details] [review] Proposed patch
Created attachment 124787 [details] [review] Proposed patch
Micheal, I'm happy to test this if you could add a test plan.
The test plan is in the patch, but it isn't complete. Here it is for you. 1. Set the PatronsPerPage system preference to any integer other than 20. 2. Go to the Patrons page and search for patrons. 3. Observe that the per page drop down defaults to 20. 4. Go to Tools -> Patron card creator. 5. Start a new Card Batch. 6. Click the 'Add patron(s)' button to open the patron search window. 7. Do a search with more then 20 results. 8. Observe that the per page drop down defaults to 20. 8b. If the PatronsPerPage was set to a value other than 10, 20, 50, or 100 observe that that value is not in the per page drop down. 9. Add patrons to the batch. 10. Close patron search window. 11. Click the 'Add patron(s)' button again to add the patrons to the batch. Observe that the per page defaults to 20. 12. Delete the card batch. 13. Apply patch. 14. Start a new card batch. Click the 'Add patron(s)' button again to open the patron search window. 15. Do a search. 16. Observe that the per page drop down includes the value of PatronsPerPage and that value is selected by default. 17. Add patrons to batch and close the search window. 18. Click the 'Add patron(s)' button again and observe that the per page drop down includes and defaults to the PatronsPerPage value. 19. Delete the card batch. 17. Go back to the Patrons page and search for patrons. 18. Observe that the per page drop down defaults to the value of the PatronsPerPage system preference.
Created attachment 134004 [details] [review] Bug 28864: Have patron card creator use PatronsPerPage Rebase after update to how patron search works elsewhere. Add aLengthMenu, pageLength, and iDisplayLength to patroncards/edit-batch Test plan: 1. Set the PatronsPerPage system preference to any integer other than 20. 2. Go to Tools -> Patron card creator. 3. Start a new Card Batch. 4. Click the 'Add patron(s)' button to open the patron search window. 5. Do a search. 6. Add more than 20 patrons to the batch and close the search window. 7. Observe that the per page drop down defaults to 20. 8. If the PatronsPerPage was set to a value other than 10, 20, 50, or 100 observe that that value is not in the per page drop down. 9. Apply patch. 10. Reload the Patron Card Batch 11. Observe that the per page drop down includes the value of PatronsPerPage and that value is selected by default.
I think this has since been fixed; I can't replicate the reported issue on the current branch or on 20.11.
My test plan missed hitting the 'add patrons' button the second time. The patron table in the search window that open the first time you hit the 'add patrons' button is fine, it honors the PatronsPerPage setting. After adding the patrons to the batch though, the patron table that appears on the main windows does not honor the setting. I just loaded the patron card creator on my workstation, which is running 22.05.3-1, and checked. My PatronsPerPage setting is 50. I created a batch with more than 50 patrons. In the patrons table in the card batch after adding patrons to the batch, the default per page selected is 20, not 50 as it should be.
Created attachment 138194 [details] [review] Bug 28864: Have patron search use PatronsPerPage better. Add pageLength to members/member.tt Add aLengthMenu and pageLength to patroncards/add_user_search (common/patron_search.tt). Add aLengthMenu, pageLength, and iDisplayLength to patroncards/edit-batch Test plan: 1. Set the PatronsPerPage system preference to any integer other than 20. 2. Go to the Patrons page and search for patrons. 3. Observe that the per page drop down defaults to 20. 4. Go to Tools -> Patron card creator. 5. Start a new Card Batch. 6. Click the 'Add patron(s)' button to open the patron search window. 7. Do a search. 8. Observe that the per page drop down defaults to 20. 8b. If the PatronsPerPage was set to a value other than 10, 20, 50, or 100 observe that that value is not in the per page drop down. 9. Close patron search window. 10. Apply patch. 11. Click the 'Add patron(s)' button again to open the patron search window. 12. Do a search. 13. Observe that the per page drop down includes the value of PatronsPerPage and that value is selected by default. 14. Go back to the Patrons page and search for patrons. 15. Observe that the per page drop down defaults to the value of the PatronsPerPage system preference. Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Created attachment 138195 [details] [review] Bug 28864: Have patron search use PatronsPerPage better. Add pageLength to members/member.tt Add aLengthMenu and pageLength to patroncards/add_user_search (common/patron_search.tt). Add aLengthMenu, pageLength, and iDisplayLength to patroncards/edit-batch Test plan: 1. Set the PatronsPerPage system preference to any integer other than 20. 2. Go to the Patrons page and search for patrons. 3. Observe that the per page drop down defaults to 20. 4. Go to Tools -> Patron card creator. 5. Start a new Card Batch. 6. Click the 'Add patron(s)' button to open the patron search window. 7. Do a search. 8. Observe that the per page drop down defaults to 20. 8b. If the PatronsPerPage was set to a value other than 10, 20, 50, or 100 observe that that value is not in the per page drop down. 9. Close patron search window. 10. Apply patch. 11. Click the 'Add patron(s)' button again to open the patron search window. 12. Do a search. 13. Observe that the per page drop down includes the value of PatronsPerPage and that value is selected by default. 14. Go back to the Patrons page and search for patrons. 15. Observe that the per page drop down defaults to the value of the PatronsPerPage system preference. Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Created attachment 138196 [details] [review] Bug 31120: Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value If the rule renewalperiod is the blank empty string instead of being null/undefined or non-existant, Koha will interpret the renewal period as being zero days instead of falling back to the issuelength rule. It makes sense to me that a literal 0 here should make it renew for zero days even though that is nonsensical. Test Plan: 1) Delete all your rules 2) Create an all/all/all rules with an empty string for renewal base period 3) Note that renewing an item does nothing 4) Apply this patch 5) Restart all the things! 6) Renew again 7) Note the renewal uses the issuelength rule as intended Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Created attachment 138197 [details] [review] Bug 28864: Have patron search use PatronsPerPage better. Add pageLength to members/member.tt Add aLengthMenu and pageLength to patroncards/add_user_search (common/patron_search.tt). Add aLengthMenu, pageLength, and iDisplayLength to patroncards/edit-batch Test plan: 1. Set the PatronsPerPage system preference to any integer other than 20. 2. Go to the Patrons page and search for patrons. 3. Observe that the per page drop down defaults to 20. 4. Go to Tools -> Patron card creator. 5. Start a new Card Batch. 6. Click the 'Add patron(s)' button to open the patron search window. 7. Do a search. 8. Observe that the per page drop down defaults to 20. 8b. If the PatronsPerPage was set to a value other than 10, 20, 50, or 100 observe that that value is not in the per page drop down. 9. Close patron search window. 10. Apply patch. 11. Click the 'Add patron(s)' button again to open the patron search window. 12. Do a search. 13. Observe that the per page drop down includes the value of PatronsPerPage and that value is selected by default. 14. Go back to the Patrons page and search for patrons. 15. Observe that the per page drop down defaults to the value of the PatronsPerPage system preference. Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
(In reply to Michael Hafen from comment #7) > My test plan missed hitting the 'add patrons' button the second time. Great, thank you. Patch works as described with the updated test plan - so I've signed off. Unfortunately, the sandbox is playing havoc at the moment and won't sign off both of the patches. After signing one off twice, it has now added a patch from a different bug - rather than signing off the two patches Michael added. Nightmare. :( But the two patches from Michael both work here.
Comment on attachment 138196 [details] [review] Bug 31120: Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value A patch for Bug 31120 was attached by mistake here.
I have tested without applying the patch and this works nicely on master. I believe this might be because of the standardizing of patron search that has taken place since this patch was written.
(In reply to Katrin Fischer from comment #14) > I have tested without applying the patch and this works nicely on master. I > believe this might be because of the standardizing of patron search that has > taken place since this patch was written. The test plan, since the standardizing of the patron search, is wrong. This is still an issue on the edit-batch page. The contents of the patch reflect this though I neglected to update the test plan. Here is the new test plan: Test plan: 1. Set the PatronsPerPage system preference to any integer other than 20. 2. Go to Tools -> Patron card creator. 3. Start a new Card Batch. 4. Click the 'Add patron(s)' button to open the patron search window. 5. Do a search. 6. Add more than 20 patrons to the batch. 7. Close patron search window. 8. Click the 'Add patron(s)' button to add the selected patrons to the batch. 9. Observe that the table of patrons is has 20 as the default per page setting. 10. Apply patch. 11. Reload the batch. 12. Observe that the per page drop down includes the value of PatronsPerPage and that value is selected by default.
The commits Bug 28864: Have patron card creator use PatronsPerPage Bug 28864: Have patron search use PatronsPerPage better. are duplicate, which the is the right one? Thanks Michael for the correct test plan. Which patch name? With this, we will be able to cleanup this. The name and test plan was changed on the 1st signoff via a sandbox. I don't know how is that possible to change a commit message via a sandbox.
Email sent to Sally to ask if their testing matches the new test plan. To know if the signoff is still valid.
(In reply to Victor Grousset/tuxayo from comment #16) > The commits > Bug 28864: Have patron card creator use PatronsPerPage > Bug 28864: Have patron search use PatronsPerPage better. > are duplicate, which the is the right one? > > Thanks Michael for the correct test plan. Which patch name? With this, we > will be able to cleanup this. > > The name and test plan was changed on the 1st signoff via a sandbox. I don't > know how is that possible to change a commit message via a sandbox. The line numbers are the only difference. It seems that the commit "Bug 28864: Have patron search use PatronsPerPage better." is the more correct, looking at the line numbers (this is the only attachment that isn't marked obsolete); though I like the commit message of the other commit better, but if you can't change the commit message, then that's ok.
(In reply to Victor Grousset/tuxayo from comment #17) > Email sent to Sally to ask if their testing matches the new test plan. To > know if the signoff is still valid. Yes, it's valid. I used the new test plan and checked the bug was valid on master. Then I used the new test plan against the patch, and it worked perfectly - so that's why I added the sign off. I'll change the status back.
Michael, please don't forget to assign yourself!
Created attachment 138379 [details] [review] Bug 28864: Have patron card creator use PatronsPerPage Add pageLength to members/member.tt Add aLengthMenu and pageLength to patroncards/add_user_search (common/patron_search.tt). Add aLengthMenu, pageLength, and iDisplayLength to patroncards/edit-batch Test plan: 1. Set the PatronsPerPage system preference to any integer other than 20. 2. Go to Tools -> Patron card creator. 3. Start a new Card Batch. 4. Click the 'Add patron(s)' button to open the patron search window. 5. Do a search. 6. Add more than 20 patrons to the batch. 7. Close patron search window. 8. Click the 'Add patron(s)' button to add the selected patrons to the batch. 9. Observe that the table of patrons is has 20 as the default per page setting. 10. Apply patch. 11. Reload the batch. 12. Observe that the per page drop down includes the value of PatronsPerPage and that value is selected by default. Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
(In reply to Michael Hafen from comment #18) > though I like the commit message of the other commit better, but > if you can't change the commit message, then that's ok. Ok I changed to "Bug 28864: Have patron card creator use PatronsPerPage" and added test plan from Comment 15
Thanks for clearing that up!
Created attachment 138388 [details] [review] Bug 28864: Have patron card creator use PatronsPerPage Add pageLength to members/member.tt Add aLengthMenu and pageLength to patroncards/add_user_search (common/patron_search.tt). Add aLengthMenu, pageLength, and iDisplayLength to patroncards/edit-batch Test plan: 1. Set the PatronsPerPage system preference to any integer other than 20. 2. Go to Tools -> Patron card creator. 3. Start a new Card Batch. 4. Click the 'Add patron(s)' button to open the patron search window. 5. Do a search. 6. Add more than 20 patrons to the batch. 7. Close patron search window. 8. Click the 'Add patron(s)' button to add the selected patrons to the batch. 9. Observe that the table of patrons is has 20 as the default per page setting. 10. Apply patch. 11. Reload the batch. 12. Observe that the per page drop down includes the value of PatronsPerPage and that value is selected by default. Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.06
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document it seems, marking resolved.
Thanks! Pushed to 21.11 for 21.11.12