In this second part, the following subroutines from C4::Branch will be removed by using methods from Koha::Libraries. - DelBranch - DelBranchCategory - CheckCategoryUnique - CheckBranchCategorycode - GetBranchCategory - GetBranchCategories - GetCategoryTypes - GetBranchesCount - ModBranchCategoryInfo
Created attachment 45368 [details] [review] Bug 15295: Koha::Libraries - Remove DelBranch and DelBranchCategory These 2 subroutines are not used anymore, there were only used from the admin script rewrote on bug 15294.
Created attachment 45369 [details] [review] Bug 15295: Koha::Libraries - Remove CheckCategoryUnique This subroutine is not used anymore and was not really useful. The branchcategories table has a primary key defined on categorycode.
Created attachment 45370 [details] [review] Bug 15295: Koha::Libraries - Remove CheckBranchCategorycode This verification is now done in admin/branches.pl, no need for a special subroutine/method, it's 1 line only called once.
Created attachment 45371 [details] [review] Bug 15295: Koha::Libraries - Remove GetBranchCategory This has been replaced with Koha::Libraries->find
Created attachment 45372 [details] [review] Bug 15295: Koha::Libraries - Remove GetBranchCategories Test plan 1/ enable OpacAddMastheadLibraryPulldown 2/ Defined a group of libraries as searchdomain and tick 'show in pull down' 3/ At the OPAC, go on the advanced search form, limit by the group of libraries you have just created. 4/ The group should be selected by default in the dropdown list
Created attachment 45373 [details] [review] Bug 15295: Koha::Libraries - Remove GetCategoryTypes 'searchdomain' and 'properties' were hardcoded in this subroutine. Now there are in the admin script. Not a big deal, we could improve that later if someone wants to add a third values.
Created attachment 45374 [details] [review] Bug 15295: Koha::Libraries - Remove GetBranchesCount This is replaced with Koha::Libraries->search->count.
Created attachment 45375 [details] [review] Bug 15295! Koha::Libraries - Remove ModBranchCategoryInfo This has been replaced with Koha::Library->update_categories and Koha::Library->add_to_categories
Created attachment 45752 [details] [review] Bug 15294: Koha::Libraries - Rewrite the admin scripts This patch rewrites the admin/branches.pl script to use the new modules instead of C4::Branches. Test plan: 1/ Create libraries using all the fields available 2/ Create groups of libraries 3/ Assign 1+ libraries to some groups 4/ Delete libraries and groups of libraries You should not able to delete a library if items or patrons use it. You should not able to delete a group of libraries if there are still libraries using it. 5/ Update libraries and groups of libraries Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Comment on attachment 45752 [details] [review] Bug 15294: Koha::Libraries - Rewrite the admin scripts Sorry, that was an error, I tried to attach multiple patches.
Created attachment 45754 [details] [review] Bug 15295: Koha::Libraries - Remove DelBranch and DelBranchCategory These 2 subroutines are not used anymore, there were only used from the admin script rewrote on bug 15294.
Created attachment 45755 [details] [review] Bug 15295: Koha::Libraries - Remove CheckCategoryUnique This subroutine is not used anymore and was not really useful. The branchcategories table has a primary key defined on categorycode. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 45756 [details] [review] Bug 15295: Koha::Libraries - Remove CheckBranchCategorycode This verification is now done in admin/branches.pl, no need for a special subroutine/method, it's 1 line only called once. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 45757 [details] [review] Bug 15295: Koha::Libraries - Remove GetBranchCategory This has been replaced with Koha::Libraries->find Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 45760 [details] [review] Bug 15295: Koha::Libraries - Remove GetBranchCategories Test plan 1/ enable OpacAddMastheadLibraryPulldown 2/ Defined a group of libraries as searchdomain and tick 'show in pull down' 3/ At the OPAC, go on the advanced search form, limit by the group of libraries you have just created. 4/ The group should be selected by default in the dropdown list Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 45763 [details] [review] Bug 15295: Koha::Libraries - Remove GetCategoryTypes 'searchdomain' and 'properties' were hardcoded in this subroutine. Now there are in the admin script. Not a big deal, we could improve that later if someone wants to add a third values. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 45764 [details] [review] Bug 15295: Koha::Libraries - Remove GetBranchesCount This is replaced with Koha::Libraries->search->count. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 45765 [details] [review] Bug 15295! Koha::Libraries - Remove ModBranchCategoryInfo This has been replaced with Koha::Library->update_categories and Koha::Library->add_to_categories
Note: I tested all patches together. Worked as expected.
(In reply to Marc Véron from comment #19) > Note: I tested all patches together. Worked as expected. And that is exactly the way to do. I add a test plan on the patch to highlight the changes done for each one, but they have to be tested together. Thanks a lot for testing and signoffing!
Created attachment 45924 [details] [review] Bug 15295: Koha::Libraries - Remove DelBranch and DelBranchCategory These 2 subroutines are not used anymore, there were only used from the admin script rewrote on bug 15294. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 45925 [details] [review] Bug 15295: Koha::Libraries - Remove CheckCategoryUnique This subroutine is not used anymore and was not really useful. The branchcategories table has a primary key defined on categorycode. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> http://bugs.koha-community.org/show_bug.cgi?id=15294
Created attachment 45926 [details] [review] Bug 15295: Koha::Libraries - Remove CheckBranchCategorycode This verification is now done in admin/branches.pl, no need for a special subroutine/method, it's 1 line only called once. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> http://bugs.koha-community.org/show_bug.cgi?id=15294
Created attachment 45927 [details] [review] Bug 15295: Koha::Libraries - Remove GetBranchCategory This has been replaced with Koha::Libraries->find Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> http://bugs.koha-community.org/show_bug.cgi?id=15294
Created attachment 45928 [details] [review] Bug 15295: Koha::Libraries - Remove GetBranchCategories Test plan 1/ enable OpacAddMastheadLibraryPulldown 2/ Defined a group of libraries as searchdomain and tick 'show in pull down' 3/ At the OPAC, go on the advanced search form, limit by the group of libraries you have just created. 4/ The group should be selected by default in the dropdown list Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> http://bugs.koha-community.org/show_bug.cgi?id=15294
Created attachment 45929 [details] [review] Bug 15295: Koha::Libraries - Remove GetCategoryTypes 'searchdomain' and 'properties' were hardcoded in this subroutine. Now there are in the admin script. Not a big deal, we could improve that later if someone wants to add a third values. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> http://bugs.koha-community.org/show_bug.cgi?id=15294
Created attachment 45930 [details] [review] Bug 15295: Koha::Libraries - Remove GetBranchesCount This is replaced with Koha::Libraries->search->count. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> http://bugs.koha-community.org/show_bug.cgi?id=15294
Created attachment 45931 [details] [review] Bug 15295! Koha::Libraries - Remove ModBranchCategoryInfo This has been replaced with Koha::Library->update_categories and Koha::Library->add_to_categories Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> http://bugs.koha-community.org/show_bug.cgi?id=15294
More foundational work, well done Jonathan. Not found any regressions and code is much tidier now :) Passing QA
Pushed to Master - Should be in the May 2016 release
Created attachment 46684 [details] [review] Bug 15295: (follow-up) Koha::Libraries - Remove GetBranchesCount Fix conflict with bug 11625
Created attachment 46696 [details] [review] [SIGNED-OFF] Bug 15295: (follow-up) Koha::Libraries - Remove GetBranchesCount Fix conflict with bug 11625 Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Follow pushed to master!