Bug 15629 - Move the C4::Branch related code to Koha::Libraries - part 3
Summary: Move the C4::Branch related code to Koha::Libraries - part 3
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 15628
Blocks: 15293 15407 16198
  Show dependency treegraph
 
Reported: 2016-01-20 14:15 UTC by Jonathan Druart
Modified: 2017-06-14 22:11 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (1) (2.39 KB, patch)
2016-01-20 14:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (2) (2.07 KB, patch)
2016-01-20 14:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (3) (3.20 KB, patch)
2016-01-20 14:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15629: Koha::Libraries - Remove GetBranchDetail (39.56 KB, patch)
2016-01-20 14:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15629: Koha::Libraries - Remove ModBranch (25.73 KB, patch)
2016-01-20 14:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15629: Koha::Libraries - Remove GetBranchInfo (4.83 KB, patch)
2016-01-20 14:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (1) (2.50 KB, patch)
2016-02-01 16:47 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (2) (2.17 KB, patch)
2016-02-01 16:47 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (3) (3.30 KB, patch)
2016-02-01 16:47 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove GetBranchDetail (39.66 KB, patch)
2016-02-01 16:47 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove ModBranch (25.83 KB, patch)
2016-02-01 16:47 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15629: Koha::Libraries - Remove GetBranchInfo (4.93 KB, patch)
2016-02-01 16:48 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 15629: Fix tests - Koha::Library does not have ->categories but ->get_categories (1.17 KB, patch)
2016-02-02 14:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15629 [QA Followup] (2.81 KB, patch)
2016-02-05 14:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (1) (2.50 KB, patch)
2016-02-05 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (2) (2.17 KB, patch)
2016-02-05 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15629: Koha::Libraries - Remove GetBranchesInCategory (3) (3.30 KB, patch)
2016-02-05 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15629: Koha::Libraries - Remove GetBranchDetail (39.66 KB, patch)
2016-02-05 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15629: Koha::Libraries - Remove ModBranch (25.83 KB, patch)
2016-02-05 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15629: Koha::Libraries - Remove GetBranchInfo (4.93 KB, patch)
2016-02-05 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15629: Fix tests - Koha::Library does not have ->categories but ->get_categories (1.22 KB, patch)
2016-02-05 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15629 [QA Followup] (2.86 KB, patch)
2016-02-05 16:22 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 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!