Bug 18247 - Remove SQL queries from branch_transfer_limit.pl administrative script
Summary: Remove SQL queries from branch_transfer_limit.pl administrative script
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low trivial (vote)
Assignee: Alex Buckley
QA Contact: Kyle M Hall
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2017-03-12 10:01 UTC by Alex Buckley
Modified: 2023-12-28 20:44 UTC (History)
8 users (show)

See Also:
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


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 (3.84 KB, patch)
2017-03-12 12:24 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18247 - Added 3 successfully working unit tests for each new subroutine (4.39 KB, patch)
2017-03-12 12:24 UTC, Alex Buckley
Details | Diff | Splinter 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. (7.88 KB, patch)
2017-04-24 02:45 UTC, Alex Buckley
Details | Diff | Splinter 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. (8.06 KB, patch)
2017-04-24 06:44 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18247: Remove SQL queries from branch_transfer_limit.pl (2.17 KB, patch)
2022-08-10 01:08 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18247: Remove SQL queries from branch_transfer_limit.pl (2.28 KB, patch)
2022-12-16 15:38 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 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 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 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 wainuiwitikapark 2023-03-15 01:41:31 UTC
Not backported to 21.05.x