Bug 18247

Summary: Remove SQL queries from branch_transfer_limit.pl administrative script
Product: Koha Reporter: Alex Buckley <alexbuckley>
Component: Architecture, internals, and plumbingAssignee: Alex Buckley <alexbuckley>
Status: RESOLVED FIXED QA Contact: Kyle M Hall (khall) <kyle>
Severity: trivial    
Priority: P5 - low CC: arthur.suzuki, jacob.omara, jonathan.druart, kyle, lucas, martin.renvoize, veron, wainuiwitikapark
Version: unspecifiedKeywords: Academy
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.03,22.05.10, 21.11.16
Circulation function:
Attachments: Bug 18247 - Moved the SQL queries out of the branch_transfer_limits.pl script into the Koha::AuthorisedValue, Koha::Libraries, Koha::ItemType perl modules
Bug 18247 - Added 3 successfully working unit tests for each new subroutine
Bug 18247 - Moved the SQL queries out of the branch_transfer_limits.pl script into the Koha::AuthorisedValue, Koha::Libraries, Koha::ItemType perl modules.
Bug 18247 - Moved the SQL queries out of the branch_transfer_limits.pl script into the Koha::AuthorisedValue, Koha::Libraries, Koha::ItemType perl modules.
Bug 18247: Remove SQL queries from branch_transfer_limit.pl
Bug 18247: Remove SQL queries from branch_transfer_limit.pl

Description Alex Buckley 2017-03-12 10:01:29 UTC
This bug report requires all three SQL queries to be removed from branch_transfer_limit.pl administrative script and placed into suitable perl modules in the Koha or C4 directory.
Comment 1 Alex Buckley 2017-03-12 12:24:14 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.
Comment 2 Alex Buckley 2017-03-12 12:24:50 UTC
Created attachment 61015 [details] [review]
Bug 18247 - Added 3 successfully working unit tests for each new subroutine
Comment 3 Alex Buckley 2017-03-12 12:35:20 UTC
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)
Comment 4 Marc Véron 2017-04-21 06:08:40 UTC
Sorry, I get a merge conflict in t/db_dependent/Koha/ItemTypes.t
Comment 5 Alex Buckley 2017-04-24 02:45:26 UTC
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
Comment 6 Alex Buckley 2017-04-24 02:48:07 UTC
(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
Comment 7 Marc Véron 2017-04-24 06:44:33 UTC
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>
Comment 8 Jonathan Druart 2017-04-24 17:46:02 UTC
Alex, same as bug 18291 comment 6: you should not need to add new method.

Ping me on IRC if you need more details.
Comment 9 Jonathan Druart 2017-05-02 22:01:09 UTC
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');
Comment 10 Alex Buckley 2022-08-10 01:08:19 UTC
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
Comment 11 Alex Buckley 2022-08-10 01:09:36 UTC
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
Comment 12 Kyle M Hall (khall) 2022-12-16 15:38:26 UTC
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>
Comment 13 Tomás Cohen Arazi (tcohen) 2023-01-27 19:19:19 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 14 Matt Blenkinsop 2023-01-31 14:28:37 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 15 Lucas Gass (lukeg) 2023-02-01 18:46:11 UTC
Backported to 22.05.x for upcoming 22.05.10
Comment 16 Arthur Suzuki 2023-02-17 09:38:02 UTC
applied to 21.11.x for 21.11.16
Comment 17 Wainui Witika-Park 2023-03-15 01:41:31 UTC
Not backported to 21.05.x