Bug 15629

Summary: Move the C4::Branch related code to Koha::Libraries - part 3
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: brendan
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 15628    
Bug Blocks: 15293, 15407, 16198    
Attachments: Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (1)
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (2)
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (3)
Bug 15629: Koha::Libraries - Remove GetBranchDetail
Bug 15629: Koha::Libraries - Remove ModBranch
Bug 15629: Koha::Libraries - Remove GetBranchInfo
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (1)
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (2)
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (3)
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove GetBranchDetail
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove ModBranch
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove GetBranchInfo
Bug 15629: Fix tests - Koha::Library does not have ->categories but ->get_categories
Bug 15629 [QA Followup]
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (1)
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (2)
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (3)
Bug 15629: Koha::Libraries - Remove GetBranchDetail
Bug 15629: Koha::Libraries - Remove ModBranch
Bug 15629: Koha::Libraries - Remove GetBranchInfo
Bug 15629: Fix tests - Koha::Library does not have ->categories but ->get_categories
Bug 15629 [QA Followup]

Description Jonathan Druart 2016-01-20 14:15:19 UTC
Part 1 (bug 15294) and part 2 (bug 15295) have been pushed.
This third part will remove some C4::Branch subroutines:
- GetBranchesInCategory
- GetBranchDetail
- ModBranch
- GetBranchInfo
Comment 1 Jonathan Druart 2016-01-20 14:36:35 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-01-20 14:36:37 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-01-20 14:36:40 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-01-20 14:36:46 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2016-01-20 14:36:49 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2016-01-20 14:36:51 UTC Comment hidden (obsolete)
Comment 7 Owen Leonard 2016-02-01 16:47:16 UTC Comment hidden (obsolete)
Comment 8 Owen Leonard 2016-02-01 16:47:22 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2016-02-01 16:47:30 UTC Comment hidden (obsolete)
Comment 10 Owen Leonard 2016-02-01 16:47:39 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2016-02-01 16:47:45 UTC Comment hidden (obsolete)
Comment 12 Owen Leonard 2016-02-01 16:48:45 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2016-02-02 14:02:06 UTC Comment hidden (obsolete)
Comment 14 Kyle M Hall 2016-02-05 14:48:41 UTC Comment hidden (obsolete)
Comment 15 Kyle M Hall 2016-02-05 16:22:13 UTC
Created attachment 47706 [details] [review]
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (1)

For consistency, the branchcodes method of Koha::LibraryCategory should be
named libraries.
At the moment, there is only one occurrence, in admin/branches.pl

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 16 Kyle M Hall 2016-02-05 16:22:21 UTC
Created attachment 47707 [details] [review]
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (2)

C4::Branch::GetBranchesInCategory can be replaced with
Koha::LibraryCategory->libraries

Test plan:
1/ Define some 1+ group of libraries with 1+ libraries each
2/ Go on the advanced search (OPAC and Staff) and select a group of
libraries
3/ The result should be consistent and only include record from these
libraries

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 17 Kyle M Hall 2016-02-05 16:22:23 UTC
Created attachment 47708 [details] [review]
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (3)

C4::Branch::GetBranchesInCategory is not used anymore and can be
removed.

Test plan:
  git grep GetBranchesInCategory
should not return any result.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Kyle M Hall 2016-02-05 16:22:26 UTC
Created attachment 47709 [details] [review]
Bug 15629: Koha::Libraries - Remove GetBranchDetail

C4::Branch::GetBranchDetail retrieved library infos, it could be easily
replaced with Koha::Libraries->find

When this change needs other big changes, the unblessed method is
called, to manipulate a hashref (as before) instead of a Koha::Library
object (for instance when $library is sent to GetPreparedLetter).

Test plan:
1/ Print a basket group, the library names should be correctly
displayed.
2/ Enable emailLibrarianWhenHoldIsPlaced and place a hold, a HOLDPLACED
notice will be generated (focus on the library name)
3/ Edit a patron and change his/her library
4/ Generate the advanced notices (misc/cronjobs/advance_notices.pl) and
have a look at the generated notices
5/ Same of overdues notices
6/ Set IndependentBranches and use a non superlibrarian user to place a
hold. The "pickup at" should be correctly filled.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 19 Kyle M Hall 2016-02-05 16:22:28 UTC
Created attachment 47710 [details] [review]
Bug 15629: Koha::Libraries - Remove ModBranch

This subroutine was only used in tests to add/update a library.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 20 Kyle M Hall 2016-02-05 16:22:31 UTC
Created attachment 47711 [details] [review]
Bug 15629: Koha::Libraries - Remove GetBranchInfo

C4::Branch::GetBranchInfo was only used once in opac/opac-suggestions.pl
The occurrence can easily be replaced by a call to Koha::Libraries->find

Test plan:
1/ Create a suggestion for library A
2/ Enable OPACViewOthersSuggestions
3/ Use a patron from library B to see the suggestions
4/ The "Suggested for" value should be correctly filled with the library
A name.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 21 Kyle M Hall 2016-02-05 16:22:33 UTC
Created attachment 47712 [details] [review]
Bug 15629: Fix tests - Koha::Library does not have ->categories but ->get_categories

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 22 Kyle M Hall 2016-02-05 16:22:36 UTC
Created attachment 47713 [details] [review]
Bug 15629 [QA Followup]

* Use ->id instead of ->branchcode when possible to eliminate use of that nomenclature
* Fix bad use of ->branchcode to ->{branchcode} for unblessed hashref  version of Koha::Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 23 Brendan Gallagher 2016-02-24 04:08:35 UTC
Pushed to Master - Should be in the May 2016 Release. Thanks!