Description
Alex Buckley
2017-03-12 10:01:29 UTC
Created attachment 61014 [details] [review] Bug 18247 - Moved the SQL queries out of the branch_transfer_limits.pl script into the Koha::AuthorisedValue, Koha::Libraries, Koha::ItemType perl modules All the queries run successfully in the perl modules. Created attachment 61015 [details] [review] Bug 18247 - Added 3 successfully working unit tests for each new subroutine Test plan: 1. Restart memcached: sudo service memcached restart 2. Drop and recreate the Koha instance database 3. Go through the web installer, selecting all data to be installed in step 3 of the web installer 4. After the web installer is finished log in with the Koha database administrator credentials 5. Create yourself a patron account 6. Set the patron account to have superlibrarian privileges 7. Log out and back in as your newly created patron 8. Navigate to the Administrator interface and type in the url: cgi-bin/koha/admin/branch_transfer_limits.pl 9. Observe the list of default installed libraries displayed 10. Change the url to cgi-bin/koha/admin/branch_transfer_limits.pl?limitType=ccode 11. Observe the same list of libraries is displayed 12. Apply all the patches attached to this bug report 13. Repeat steps 1 through to 11 observing that all the libraries in the list are still visible with the patches applied when repeating steps 9 and 11 14. In your terminal navigate from your Koha instance root directory to t/db_dependent 15. Enter koha shell: sudo koha-shell <instancename> 16. Run AuthorisedValues unit tests: prove -v AuthorisedValues.t 17. The tests should run successful 18. Exit the koha shell 19. cd into the 'Koha' directory from where you are 20. Enter the koha shell and run the ItemTypes.t and Libraries.t unit tests by writing in: prove -v filename.t (both of which should be successful) Sorry, I get a merge conflict in t/db_dependent/Koha/ItemTypes.t Created attachment 62583 [details] [review] Bug 18247 - Moved the SQL queries out of the branch_transfer_limits.pl script into the Koha::AuthorisedValue, Koha::Libraries, Koha::ItemType perl modules. Also added 3 successfully working unit tests for the new subroutines (In reply to Marc Veron in comment 4) Hi Marc I have updated my patches, rebased them into a single patch and have just attached that to the bug report. So should now apply successfully. Please continue to use the test plan in comment 3 Created attachment 62586 [details] [review] Bug 18247 - Moved the SQL queries out of the branch_transfer_limits.pl script into the Koha::AuthorisedValue, Koha::Libraries, Koha::ItemType perl modules. Also added 3 successfully working unit tests for the new subroutines Followed test plan from comment #3, works as expected Signed-off-by: Marc Véron <veron@veron.ch> Alex, same as bug 18291 comment 6: you should not need to add new method. Ping me on IRC if you need more details. The Get* method you added with this patch can be replaced with Koha::YourObjects->find, or ->search No need to add new methods. For instance GetBranchCodes can be replaced with my @branchcodes = Koha::Libraries->search->get_column('branchcode'); Created attachment 138946 [details] [review] Bug 18247: Remove SQL queries from branch_transfer_limit.pl 1. Visit Administration > Patrons and circulation > Limit transfer limits 2. Observe collection codes are displayed as tabs on the Limit transfer limits page when the BranchTransferLimitsType syspref = 'Collection code' 3. Switch the BranchTransferLimitsType syspref = 'item type'. Refresh the Limit transfer limits page and observe the tabs have changed to item type codes 4. Apply patchset and restart services 5. Reload the Limit transfer limits page and confirm it displays correctly with the BranchTransferLimitsType = 'collection code' and 'item type' 6. Confirm you can successfully save transfer policies Sponsored-by: Catalyst IT I've replaced the Get* methods in my original patch with Koha::YourObjects->find, or ->search and you suggested Jonathan in comment #9 Ready for testing! Many thanks, Alex Created attachment 144691 [details] [review] Bug 18247: Remove SQL queries from branch_transfer_limit.pl 1. Visit Administration > Patrons and circulation > Limit transfer limits 2. Observe collection codes are displayed as tabs on the Limit transfer limits page when the BranchTransferLimitsType syspref = 'Collection code' 3. Switch the BranchTransferLimitsType syspref = 'item type'. Refresh the Limit transfer limits page and observe the tabs have changed to item type codes 4. Apply patchset and restart services 5. Reload the Limit transfer limits page and confirm it displays correctly with the BranchTransferLimitsType = 'collection code' and 'item type' 6. Confirm you can successfully save transfer policies Sponsored-by: Catalyst IT Signed-off-by: Jacob Omara <jacob.omara@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed to master for 23.05. Nice work everyone, thanks! Nice work everyone! Pushed to stable for 22.11.x Backported to 22.05.x for upcoming 22.05.10 applied to 21.11.x for 21.11.16 Not backported to 21.05.x |