Description
Jonathan Druart
2016-01-20 14:15:19 UTC
Created attachment 46997 [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 Created attachment 46998 [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 Created attachment 46999 [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. Created attachment 47000 [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. Created attachment 47001 [details] [review] Bug 15629: Koha::Libraries - Remove ModBranch This subroutine was only used in tests to add/update a library. Created attachment 47002 [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. Created attachment 47518 [details] [review] [SIGNED-OFF] 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> Created attachment 47519 [details] [review] [SIGNED-OFF] 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> Created attachment 47520 [details] [review] [SIGNED-OFF] 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> Created attachment 47521 [details] [review] [SIGNED-OFF] 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> Created attachment 47522 [details] [review] [SIGNED-OFF] 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> Created attachment 47523 [details] [review] [SIGNED-OFF] 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> Created attachment 47553 [details] [review] Bug 15629: Fix tests - Koha::Library does not have ->categories but ->get_categories Created attachment 47701 [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 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> 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> 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> 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> 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> 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> 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> 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> Pushed to Master - Should be in the May 2016 Release. Thanks! |