Bug 15758 - Move the C4::Branch related code to Koha::Libraries - part 4
Summary: Move the C4::Branch related code to Koha::Libraries - part 4
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: Marcel de Rooy
URL:
Keywords:
Depends on: 15407 15731 15742 15746
Blocks: 10300 11426 15293 17285 17295 17537 18307 18534 18884 19073 19769 20282 28622
  Show dependency treegraph
 
Reported: 2016-02-08 15:27 UTC by Jonathan Druart
Modified: 2021-06-23 14:08 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Large patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15758: Koha::Libraries - Remove GetBranchName (83.05 KB, patch)
2016-02-08 17:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchesLoop (132.47 KB, patch)
2016-02-08 17:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move mybranch to C4::Context (4.50 KB, patch)
2016-02-08 17:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranches (134.85 KB, patch)
2016-02-08 17:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.15 KB, patch)
2016-02-08 17:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.72 KB, patch)
2016-02-08 17:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (2.70 KB, patch)
2016-02-08 17:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.35 KB, patch)
2016-02-08 17:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchesLoop (132.46 KB, patch)
2016-02-08 17:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move mybranch to C4::Context (4.50 KB, patch)
2016-02-08 17:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranches (134.85 KB, patch)
2016-02-08 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.15 KB, patch)
2016-02-08 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.72 KB, patch)
2016-02-08 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (2.70 KB, patch)
2016-02-08 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.35 KB, patch)
2016-02-08 17:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchName (82.58 KB, patch)
2016-06-23 14:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchesLoop (132.09 KB, patch)
2016-06-23 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move mybranch to C4::Context (4.50 KB, patch)
2016-06-23 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranches (133.52 KB, patch)
2016-06-23 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.15 KB, patch)
2016-06-23 14:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.70 KB, patch)
2016-06-23 14:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (4.90 KB, patch)
2016-06-23 14:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.32 KB, patch)
2016-06-23 14:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Add missing use of Branches TT plugin in template (872 bytes, patch)
2016-06-23 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchName (87.65 KB, patch)
2016-06-24 12:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchesLoop (132.55 KB, patch)
2016-06-24 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move mybranch to C4::Context (5.09 KB, patch)
2016-06-24 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranches (134.97 KB, patch)
2016-06-24 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.15 KB, patch)
2016-06-24 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.70 KB, patch)
2016-06-24 12:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (4.90 KB, patch)
2016-06-24 12:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.32 KB, patch)
2016-06-24 12:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchName (87.65 KB, patch)
2016-06-28 13:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchesLoop (133.53 KB, patch)
2016-06-28 13:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move mybranch to C4::Context (5.09 KB, patch)
2016-06-28 13:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranches (134.60 KB, patch)
2016-06-28 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.15 KB, patch)
2016-06-28 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.70 KB, patch)
2016-06-28 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (4.90 KB, patch)
2016-06-28 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.32 KB, patch)
2016-06-28 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchName (87.68 KB, patch)
2016-08-02 12:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchesLoop (133.47 KB, patch)
2016-08-02 12:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move mybranch to C4::Context (5.09 KB, patch)
2016-08-02 12:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranches (134.59 KB, patch)
2016-08-02 12:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.15 KB, patch)
2016-08-02 12:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.72 KB, patch)
2016-08-02 12:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (4.90 KB, patch)
2016-08-02 12:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.32 KB, patch)
2016-08-02 12:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchName (89.95 KB, patch)
2016-08-04 16:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchesLoop (135.03 KB, patch)
2016-08-04 16:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move mybranch to C4::Context (5.09 KB, patch)
2016-08-04 16:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranches (137.31 KB, patch)
2016-08-04 16:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.15 KB, patch)
2016-08-04 16:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.72 KB, patch)
2016-08-04 16:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (4.90 KB, patch)
2016-08-04 16:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.32 KB, patch)
2016-08-04 16:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchName (89.96 KB, patch)
2016-08-05 12:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchesLoop (135.06 KB, patch)
2016-08-05 12:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move mybranch to C4::Context (5.09 KB, patch)
2016-08-05 12:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranches (137.79 KB, patch)
2016-08-05 12:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.15 KB, patch)
2016-08-05 12:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.72 KB, patch)
2016-08-05 12:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (4.90 KB, patch)
2016-08-05 12:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.32 KB, patch)
2016-08-05 12:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15758: Koha::Libraries - Remove GetBranchName (90.07 KB, patch)
2016-08-05 12:43 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15758: Koha::Libraries - Remove GetBranchesLoop (135.16 KB, patch)
2016-08-05 12:44 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15758: Koha::Libraries - Move mybranch to C4::Context (5.20 KB, patch)
2016-08-05 12:44 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15758: Koha::Libraries - Remove GetBranches (137.90 KB, patch)
2016-08-05 12:44 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.25 KB, patch)
2016-08-05 12:44 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.83 KB, patch)
2016-08-05 12:44 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (5.00 KB, patch)
2016-08-05 12:44 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.42 KB, patch)
2016-08-05 12:44 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchName (90.00 KB, patch)
2016-08-25 12:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchesLoop (135.12 KB, patch)
2016-08-25 12:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move mybranch to C4::Context (5.14 KB, patch)
2016-08-25 12:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranches (137.75 KB, patch)
2016-08-25 12:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.20 KB, patch)
2016-08-25 12:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.78 KB, patch)
2016-08-25 12:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (4.95 KB, patch)
2016-08-25 12:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.37 KB, patch)
2016-08-25 12:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: [QA Follow-up] Remove onlymine from Plugin/Branches.pm (3.11 KB, patch)
2016-08-26 08:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: [QA Follow-up] Some corrections related to GetBranchName (4.53 KB, patch)
2016-08-26 08:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: [QA Follow-up] Corrections related to GetBranches (1.82 KB, patch)
2016-08-26 12:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: Fix variable name, it's a branchcode (2.25 KB, patch)
2016-08-26 13:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchName (90.24 KB, patch)
2016-08-31 14:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchesLoop (135.37 KB, patch)
2016-08-31 14:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move mybranch to C4::Context (5.26 KB, patch)
2016-08-31 14:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranches (138.02 KB, patch)
2016-08-31 14:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.31 KB, patch)
2016-08-31 14:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.93 KB, patch)
2016-08-31 14:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (5.04 KB, patch)
2016-08-31 14:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.47 KB, patch)
2016-08-31 14:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: [QA Follow-up] Remove onlymine from Plugin/Branches.pm (3.12 KB, patch)
2016-08-31 14:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: [QA Follow-up] Some corrections related to GetBranchName (4.53 KB, patch)
2016-08-31 14:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: [QA Follow-up] Corrections related to GetBranches (1.82 KB, patch)
2016-08-31 14:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: Fix variable name, it's a branchcode (2.36 KB, patch)
2016-08-31 14:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: [QA Follow-up] Final touch? (3.43 KB, patch)
2016-08-31 14:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchName (90.00 KB, patch)
2016-09-08 15:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranchesLoop (134.93 KB, patch)
2016-09-08 15:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move mybranch to C4::Context (5.14 KB, patch)
2016-09-08 15:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Remove GetBranches (137.50 KB, patch)
2016-09-08 15:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library (4.20 KB, patch)
2016-09-08 15:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch (27.79 KB, patch)
2016-09-08 15:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin (4.95 KB, patch)
2016-09-08 15:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Koha::Libraries - Do not select an option if selected is defined (2.37 KB, patch)
2016-09-08 15:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: [QA Follow-up] Remove onlymine from Plugin/Branches.pm (3.07 KB, patch)
2016-09-08 15:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: [QA Follow-up] Some corrections related to GetBranchName (4.48 KB, patch)
2016-09-08 15:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: [QA Follow-up] Corrections related to GetBranches (1.77 KB, patch)
2016-09-08 15:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: Fix variable name, it's a branchcode (2.25 KB, patch)
2016-09-08 15:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: [QA Follow-up] Final touch? (3.38 KB, patch)
2016-09-08 15:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15758: (QA followup) Remove leftover reference to C4::Branch (802 bytes, patch)
2016-09-09 15:33 UTC, Tomás Cohen Arazi
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-02-08 15:27:00 UTC
This fourth and last part will remove:
- GetBranchName
- GetBranchesLoop
- GetBranches
- GetBranch

will move mybranch and onlymine to C4::Context
and finally remove all occurrences of C4::Branch.
Comment 1 Jonathan Druart 2016-02-08 17:32:39 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-02-08 17:32:42 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-02-08 17:32:44 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-02-08 17:32:47 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2016-02-08 17:32:50 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2016-02-08 17:32:53 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-02-08 17:32:55 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2016-02-08 17:32:58 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2016-02-08 17:45:55 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2016-02-08 17:45:59 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2016-02-08 17:46:13 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2016-02-08 17:46:21 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2016-02-08 17:46:26 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2016-02-08 17:46:32 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2016-02-08 17:46:38 UTC Comment hidden (obsolete)
Comment 16 Héctor Eduardo Castro Avalos 2016-06-22 17:16:40 UTC
Applying: Bug 15758: Koha::Libraries - Remove GetBranchName
fatal: sha1 information is lacking or useless (C4/Circulation.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 15758: Koha::Libraries - Remove GetBranchName
Comment 17 Jonathan Druart 2016-06-23 14:06:19 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2016-06-23 14:07:04 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2016-06-23 14:08:06 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2016-06-23 14:09:01 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2016-06-23 14:10:03 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2016-06-23 14:11:03 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2016-06-23 14:15:16 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2016-06-23 14:16:59 UTC Comment hidden (obsolete)
Comment 25 Jonathan Druart 2016-06-23 14:29:46 UTC Comment hidden (obsolete)
Comment 26 Héctor Eduardo Castro Avalos 2016-06-23 16:17:26 UTC
Hi Jonathan

Sorry but patch fail in C4/XSLT

When you make a search you obtain as follow:

Software error: at <http://dev.koha:8080/cgi-bin/koha/catalogue/search.pl?q=au%3AMaxwell>

Can't use string ("Midway") as a HASH ref while "strict refs" in use at /home/koha/kohaclone/C4/XSLT.pm line 308.

Or when you try to see the detail page of a bib-record:

Software error: <http://dev.koha:8080/cgi-bin/koha/catalogue/detail.pl?biblionumber=2>

Can't use string ("Midway") as a HASH ref while "strict refs" in use at /home/koha/kohaclone/C4/XSLT.pm line 308.

I think fails IMHO in this attachment <https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52727&action=diff>

So change bug status to "In Discussion" since I'm not a QA like you

Hard work. Congrats!

Regards
Comment 27 Owen Leonard 2016-06-23 16:23:29 UTC
I think this should continue to be marked "needs signoff" because it requires so much testing.
Comment 28 Owen Leonard 2016-06-23 16:59:28 UTC
I've made some progress on testing this. These are the pages I've tested which appear to be fine:

admin/branches.pl
admin/smart-rules.pl
admin/patron-attr-types.pl
admin/aqplan.pl
admin/aqbudgets.pl
admin/edi_ean_accounts.pl
acqui/addorderiso2709.pl
acqui/neworderbiblio.pl
acqui/neworderempty.pl
acqui/newordersubscription.pl
acqui/newordersuggestion.pl
acqui/basketgroup.pl
acqui/basketheader.pl
acqui/parcel.pl
catalogue/issuehistory.pl
catalogue/moredetail.pl
circ/circulation.pl
circ/reserveratios.pl
circ/selectbranchprinter.pl
circ/transferstoreceive.pl
circ/view_holdsqueue.pl
circ/waitingreserves.pl
course_reserves/course-details.pl
members/members-home.pl 
members/memberentry.pl
suggestion/suggestion.pl

These are the problems I encountered:

admin/patron-attr-types.pl - Limiting by library works (data is saved) but correct library is not selected when editing that attribute again.

admin/branch_transfer_limits.pl - "Undefined subroutine &main::mybranch called at /admin/branch_transfer_limits.pl line 43."

admin/transport-cost-matrix.pl - Table rows display library names correctly, but table columns are blank

admin/authorised_values.pl - Editing any authorized value "Not a HASH reference at /admin/authorised_values.pl line 61."

admin/item_circulation_alerts.pl - No libraries listed in "select a library" dropdown

admin/categories.pl - Editing any patron category - "Not a HASH reference at /admin/categories.pl line 62.""

acqui/basket.pl?basketno=X - "Not a HASH reference at /acqui/basket.pl line 251.""

opac-detail.pl and catalogue/detail.pl - Can't use string ( [my library's name] ) as a HASH ref while "strict refs" in use at /C4/XSLT.pm line 308.

members/moremember.pl - sidebar: Logged-in branch is displayed instead of patron home library
patron_lists/list.pl

reserve/request.pl - Pickup library is saved correctly but not displayed correctly for existing holds

circ/returns.pl - If I check in an item which belongs at another library, the holding library is shown as the destination library rather than the library where the item was checked in

Several of those problems are blockers for this patch, but I think it's still worthwhile to keep this bug open to encourage others to test the areas which haven't been tested yet.
Comment 29 Jonathan Druart 2016-06-24 12:46:30 UTC Comment hidden (obsolete)
Comment 30 Jonathan Druart 2016-06-24 12:47:07 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2016-06-24 12:47:24 UTC Comment hidden (obsolete)
Comment 32 Jonathan Druart 2016-06-24 12:47:41 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2016-06-24 12:47:59 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2016-06-24 12:48:08 UTC Comment hidden (obsolete)
Comment 35 Jonathan Druart 2016-06-24 12:48:22 UTC Comment hidden (obsolete)
Comment 36 Jonathan Druart 2016-06-24 12:48:30 UTC Comment hidden (obsolete)
Comment 37 Jonathan Druart 2016-06-24 12:50:12 UTC
(In reply to Owen Leonard from comment #28)

Thanks!
Everything is fixed, unless:

> circ/returns.pl - If I check in an item which belongs at another library,
> the holding library is shown as the destination library rather than the
> library where the item was checked in

It seems to be the behavior in master, could you confirm?
Comment 38 Owen Leonard 2016-06-27 18:32:09 UTC
There is a conflict in catalogue/search.pl.
Comment 39 Jonathan Druart 2016-06-28 13:08:42 UTC Comment hidden (obsolete)
Comment 40 Jonathan Druart 2016-06-28 13:08:50 UTC Comment hidden (obsolete)
Comment 41 Jonathan Druart 2016-06-28 13:08:54 UTC Comment hidden (obsolete)
Comment 42 Jonathan Druart 2016-06-28 13:09:04 UTC Comment hidden (obsolete)
Comment 43 Jonathan Druart 2016-06-28 13:09:09 UTC Comment hidden (obsolete)
Comment 44 Jonathan Druart 2016-06-28 13:09:14 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2016-06-28 13:09:18 UTC Comment hidden (obsolete)
Comment 46 Jonathan Druart 2016-06-28 13:09:24 UTC Comment hidden (obsolete)
Comment 47 Jonathan Druart 2016-08-02 12:42:50 UTC Comment hidden (obsolete)
Comment 48 Jonathan Druart 2016-08-02 12:42:56 UTC Comment hidden (obsolete)
Comment 49 Jonathan Druart 2016-08-02 12:43:00 UTC Comment hidden (obsolete)
Comment 50 Jonathan Druart 2016-08-02 12:43:06 UTC Comment hidden (obsolete)
Comment 51 Jonathan Druart 2016-08-02 12:43:09 UTC Comment hidden (obsolete)
Comment 52 Jonathan Druart 2016-08-02 12:43:13 UTC Comment hidden (obsolete)
Comment 53 Jonathan Druart 2016-08-02 12:43:18 UTC Comment hidden (obsolete)
Comment 54 Jonathan Druart 2016-08-02 12:43:21 UTC Comment hidden (obsolete)
Comment 55 Owen Leonard 2016-08-04 14:59:22 UTC
I've done a lot of testing now and have only run into small problems:

These pages have a sidebar filter which includes a library selector but no libraries are listed:

-  acqui/invoices.pl
-  admin/aqbudgets.pl
-  opac/opac-topissues.pl
-  serials/serials-search.pl

These pages don't show the patron's library in the sidebar:

-  members/discharge.pl
-  members/member-flags.pl
-  members/member-password.pl

Other issues:

- In both the staff client and OPAC, library names don't appear in the facets. Instead there is a list of asterisks.

-  admin/clone-rules.pl -- When you submit a library to clone rules to, the next step asks you to select a library. Your original selection seems to be getting lost.

- circ/returns.pl -- When a hold is found, the line at the bottom of the dialog, "Hold at [library]" shows the library code instead of name.

- members/paycollect.pl -- Doesn't show patron category description in the sidebar (maybe that's from Bug 15407?)

- reports/guided_reports.pl -- When running an SQL report which uses the <<branches>> placeholder, the libraries are shown in order by code rather than name.

- tools/holidays.pl -- The library name doesn't appear in the page heading.
Comment 56 Jonathan Druart 2016-08-04 16:14:50 UTC Comment hidden (obsolete)
Comment 57 Jonathan Druart 2016-08-04 16:14:56 UTC Comment hidden (obsolete)
Comment 58 Jonathan Druart 2016-08-04 16:15:00 UTC Comment hidden (obsolete)
Comment 59 Jonathan Druart 2016-08-04 16:15:07 UTC Comment hidden (obsolete)
Comment 60 Jonathan Druart 2016-08-04 16:15:11 UTC Comment hidden (obsolete)
Comment 61 Jonathan Druart 2016-08-04 16:15:15 UTC Comment hidden (obsolete)
Comment 62 Jonathan Druart 2016-08-04 16:15:19 UTC Comment hidden (obsolete)
Comment 63 Jonathan Druart 2016-08-04 16:15:23 UTC Comment hidden (obsolete)
Comment 64 Jonathan Druart 2016-08-04 16:17:22 UTC
(In reply to Owen Leonard from comment #55)

Excellent testing session Owen :)

> I've done a lot of testing now and have only run into small problems:
> 
> These pages have a sidebar filter which includes a library selector but no
> libraries are listed:
> 
> -  acqui/invoices.pl
> -  admin/aqbudgets.pl
> -  opac/opac-topissues.pl
> -  serials/serials-search.pl
> 
> These pages don't show the patron's library in the sidebar:
> 
> -  members/discharge.pl
> -  members/member-flags.pl
> -  members/member-password.pl

Everything fixed.
members/discharge.pl was actually moremember.pl

> Other issues:
> 
> - In both the staff client and OPAC, library names don't appear in the
> facets. Instead there is a list of asterisks.

Erk, I missed something in one rebase. Fixed now.

> -  admin/clone-rules.pl -- When you submit a library to clone rules to, the
> next step asks you to select a library. Your original selection seems to be
> getting lost.
> 
> - circ/returns.pl -- When a hold is found, the line at the bottom of the
> dialog, "Hold at [library]" shows the library code instead of name.

Fixed!

> - members/paycollect.pl -- Doesn't show patron category description in the
> sidebar (maybe that's from Bug 15407?)

In master, will fix it separately.

> - reports/guided_reports.pl -- When running an SQL report which uses the
> <<branches>> placeholder, the libraries are shown in order by code rather
> than name.
> 
> - tools/holidays.pl -- The library name doesn't appear in the page heading.

Fixed!
Comment 65 Jonathan Druart 2016-08-04 16:23:18 UTC
(In reply to Jonathan Druart from comment #64)
> > - members/paycollect.pl -- Doesn't show patron category description in the
> > sidebar (maybe that's from Bug 15407?)
> 
> In master, will fix it separately.

See bug 17052.
Comment 66 Jonathan Druart 2016-08-05 12:00:35 UTC Comment hidden (obsolete)
Comment 67 Jonathan Druart 2016-08-05 12:00:40 UTC Comment hidden (obsolete)
Comment 68 Jonathan Druart 2016-08-05 12:00:44 UTC Comment hidden (obsolete)
Comment 69 Jonathan Druart 2016-08-05 12:00:49 UTC Comment hidden (obsolete)
Comment 70 Jonathan Druart 2016-08-05 12:00:53 UTC Comment hidden (obsolete)
Comment 71 Jonathan Druart 2016-08-05 12:00:57 UTC Comment hidden (obsolete)
Comment 72 Jonathan Druart 2016-08-05 12:01:01 UTC Comment hidden (obsolete)
Comment 73 Jonathan Druart 2016-08-05 12:01:05 UTC Comment hidden (obsolete)
Comment 74 Owen Leonard 2016-08-05 12:43:58 UTC Comment hidden (obsolete)
Comment 75 Owen Leonard 2016-08-05 12:44:06 UTC Comment hidden (obsolete)
Comment 76 Owen Leonard 2016-08-05 12:44:13 UTC Comment hidden (obsolete)
Comment 77 Owen Leonard 2016-08-05 12:44:22 UTC Comment hidden (obsolete)
Comment 78 Owen Leonard 2016-08-05 12:44:29 UTC Comment hidden (obsolete)
Comment 79 Owen Leonard 2016-08-05 12:44:37 UTC Comment hidden (obsolete)
Comment 80 Owen Leonard 2016-08-05 12:44:43 UTC Comment hidden (obsolete)
Comment 81 Owen Leonard 2016-08-05 12:44:50 UTC Comment hidden (obsolete)
Comment 82 Jonathan Druart 2016-08-25 12:54:35 UTC Comment hidden (obsolete)
Comment 83 Jonathan Druart 2016-08-25 12:54:40 UTC Comment hidden (obsolete)
Comment 84 Jonathan Druart 2016-08-25 12:54:45 UTC Comment hidden (obsolete)
Comment 85 Jonathan Druart 2016-08-25 12:54:50 UTC Comment hidden (obsolete)
Comment 86 Jonathan Druart 2016-08-25 12:54:56 UTC Comment hidden (obsolete)
Comment 87 Jonathan Druart 2016-08-25 12:55:01 UTC Comment hidden (obsolete)
Comment 88 Jonathan Druart 2016-08-25 12:55:07 UTC Comment hidden (obsolete)
Comment 89 Jonathan Druart 2016-08-25 12:55:12 UTC Comment hidden (obsolete)
Comment 90 Marcel de Rooy 2016-08-25 13:04:09 UTC
QA: On my list for tomorrow..
Comment 91 Marcel de Rooy 2016-08-26 08:47:04 UTC Comment hidden (obsolete)
Comment 92 Marcel de Rooy 2016-08-26 08:47:09 UTC Comment hidden (obsolete)
Comment 93 Marcel de Rooy 2016-08-26 08:47:35 UTC
No not ready yet (by far..)
Comment 94 Marcel de Rooy 2016-08-26 09:59:22 UTC
Impressive work, Jonathan.

Some findings so far for first two patches. Looks like I will not finish this one today.

=== Patch 1
C4/Labels/Label.pm
L100 require Koha::Libraries;
Remember you failed patches before for that? :)
L101 # FIXME Is this used??
Not sure. But can you use those fields when you define a label in the editor or something?
One of the templates contains the string in text and implicitly seems to suggest that.
t/db_dependent/Reserves.t still fails (cause somewhere else)

=== Patch 2
Auth.pm: use for Branches removed; but still called. (Will be resolved, yes)
TODO in C4/Search.pm L337
koha-tmpl/intranet-tmpl/prog/en/modules/reports/borrowers_stats.tt: Exceptional [% l.branchname || 'UNKNOWN' %] Inconsistent?
t/db_dependent/Holds/RevertWaitingStatus.t
DBIx::Class::ResultSet::next(): [SQL::Abstract::__ANON__] Fatal: unexpected operator 'cancellationdate' with undef operand at /usr/share/koha/masterclone/Koha/Objects.pm line 168
Probably something similar as in Reserves.t
Comment 95 Jonathan Druart 2016-08-26 10:16:16 UTC
(In reply to Marcel de Rooy from comment #94)
> === Patch 1
> C4/Labels/Label.pm
> L100 require Koha::Libraries;
> Remember you failed patches before for that? :)

Yes :)
But I have replaced an existing require with another one, not added a new one.

> L101 # FIXME Is this used??
> Not sure. But can you use those fields when you define a label in the editor
> or something?
> One of the templates contains the string in text and implicitly seems to
> suggest that.
> t/db_dependent/Reserves.t still fails (cause somewhere else)

Yes, see bug 16942.

> === Patch 2
> Auth.pm: use for Branches removed; but still called. (Will be resolved, yes)
> TODO in C4/Search.pm L337

Yes it's a TODO, for later.

> koha-tmpl/intranet-tmpl/prog/en/modules/reports/borrowers_stats.tt:
> Exceptional [% l.branchname || 'UNKNOWN' %] Inconsistent?

No, the UNKNOWN comes from the pl. Now it's translatable.

> t/db_dependent/Holds/RevertWaitingStatus.t
> DBIx::Class::ResultSet::next(): [SQL::Abstract::__ANON__] Fatal: unexpected
> operator 'cancellationdate' with undef operand at
> /usr/share/koha/masterclone/Koha/Objects.pm line 168
> Probably something similar as in Reserves.t

Same as before, see bug 16942.


Thanks for QAing, really much appreciated!
Comment 96 Marcel de Rooy 2016-08-26 12:02:53 UTC Comment hidden (obsolete)
Comment 97 Marcel de Rooy 2016-08-26 12:04:40 UTC
Some more tiny comments from the last big patch:

You say that this (horrible??) category key within the branches hash is not used anymore due to bug 15295 ? I think so too. More evidence to present?

FIXME in transferbook is fine with me. Just leave it. This PE stuff should perhaps be removed, but not on this report.

circ/branchtransfers.pl
+        $item{'tobrname'}              = $tobranchcd;
and
+    $item{'tobrname'}              = $tobcd;
With a corresponding: <td class="tf-destination">[% Branches.GetName( trsfitemloo.tobrname ) %]</td>
Do not really look good.. Name versus code?
L194 Do we still need this warn BTW ?

koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
+                        <p class="problem">Please return item to: [% Branches.GetName( errmsgloo.msg ) %]</p>
Hmm.

I will finish it later next week. I guess that splitting up this patch set in two parts would have been better.
Comment 98 Jonathan Druart 2016-08-26 13:08:30 UTC
(In reply to Marcel de Rooy from comment #97)
> Some more tiny comments from the last big patch:
> 
> You say that this (horrible??) category key within the branches hash is not
> used anymore due to bug 15295 ? I think so too. More evidence to present?

Everywhere it was used, Koha::LibraryCategory is now used

> FIXME in transferbook is fine with me. Just leave it. This PE stuff should
> perhaps be removed, but not on this report.

Definitely not :)

> circ/branchtransfers.pl
> +        $item{'tobrname'}              = $tobranchcd;
> and
> +    $item{'tobrname'}              = $tobcd;
> With a corresponding: <td class="tf-destination">[% Branches.GetName(
> trsfitemloo.tobrname ) %]</td>
> Do not really look good.. Name versus code?

Yes the var name is wrong. Patch is coming.

> L194 Do we still need this warn BTW ?

No idea. In doubt, let's keep it.

> koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
> +                        <p class="problem">Please return item to: [%
> Branches.GetName( errmsgloo.msg ) %]</p>
> Hmm.

Ugly but correct.

> I will finish it later next week. I guess that splitting up this patch set
> in two parts would have been better.

Yes it's what I did for C4::Members. But here for libraries it looked easier to get 1 big test than having to test everything for each patch.
Comment 99 Jonathan Druart 2016-08-26 13:08:53 UTC Comment hidden (obsolete)
Comment 100 Marcel de Rooy 2016-08-31 14:10:24 UTC Comment hidden (obsolete)
Comment 101 Marcel de Rooy 2016-08-31 14:10:30 UTC Comment hidden (obsolete)
Comment 102 Marcel de Rooy 2016-08-31 14:10:35 UTC Comment hidden (obsolete)
Comment 103 Marcel de Rooy 2016-08-31 14:10:40 UTC Comment hidden (obsolete)
Comment 104 Marcel de Rooy 2016-08-31 14:10:45 UTC Comment hidden (obsolete)
Comment 105 Marcel de Rooy 2016-08-31 14:10:50 UTC Comment hidden (obsolete)
Comment 106 Marcel de Rooy 2016-08-31 14:10:55 UTC Comment hidden (obsolete)
Comment 107 Marcel de Rooy 2016-08-31 14:11:01 UTC Comment hidden (obsolete)
Comment 108 Marcel de Rooy 2016-08-31 14:11:06 UTC Comment hidden (obsolete)
Comment 109 Marcel de Rooy 2016-08-31 14:11:11 UTC Comment hidden (obsolete)
Comment 110 Marcel de Rooy 2016-08-31 14:11:16 UTC Comment hidden (obsolete)
Comment 111 Marcel de Rooy 2016-08-31 14:11:20 UTC Comment hidden (obsolete)
Comment 112 Marcel de Rooy 2016-08-31 14:11:26 UTC Comment hidden (obsolete)
Comment 113 Marcel de Rooy 2016-08-31 14:15:32 UTC
QA Final comments:
Good job, Jonathan.
Code looks good to me. Hard to test every corner. Hopefully we tackled most issues now.
See also comments in last commit message.

RM: Please push 15407 + this one 15758 now early to have some more test coverage and prevent rebasing.

Passed QA
Comment 114 Jonathan Druart 2016-09-08 15:32:22 UTC
Created attachment 55346 [details] [review]
Bug 15758: Koha::Libraries - Remove GetBranchName

This is the fourth and last patch set to remove C4::Branch.
The real purpose of this patch is to standardise and refactor some code
which is related to the libraries selection/display.
Its unconfessed purpose is to remove the C4::Branch package.

Before this patch set, only 6 subroutines still existed in the C4::Branch
package:
- GetBranchName
- GetBranchesLoop
- mybranch
- onlymine
- GetBranches
- GetBranch

GetBranchName basically returns the branchname for a given branchcode.
The branchname is only used for a display purpose and we don't need to
retrieve it in package or pl scripts (unless for a few exceptions).
We have a `Branches` template plugin with a `GetName` method which does
exactly this job.
To achieve this removal, we will use this template plugin and delete the
GetBranchName from pl and pm files.
The `Branches.all()` will now select the library of the logged in user
if no `selected` parameter has been passed.
This new behavior could cause regressions, for instance there are some
places where we do not want an option preselected (batch item
modification for instance), keep that in mind when testing.

GetBranchesLoop took 3 parameters: $branch and $onlymine.
The first one was used to set a "selected" flag, for a display purpose:
select an option in the libraries dropdown lists.
The second one was useless: If not passed or set to 0, the
`C4::Branch::onlymine` subroutine was called.
This onlymine flag was use to know if the logged in user was able to see
other libraries infos.
A patron can see the infos from other libraries if IndependentBranches
is not set OR if he has the superlibrarian permission.
Prior to this patch set, the "onlymine test" was done on different
places (neworderempty.pl, additem.pl, holidays.pl, etc.), including the
Branches TT plugin. In this patch set, this test is only done on one
place (C4::Context::only_my_library, code moved from
C4::Branch::onlymine).
To accomplish the same job as this subroutine, we just need to call the
`Branches.all()` method from the `Branches` TT plugin. It already
accepts a `selected` parameter to set a flag on the option to select.
To avoid the repetitive
  [% IF selected %]<option selected="selected">[% ELSE %]<option>[% END %]
pattern, a new `html_helpers` TT include file has been created, it
defines an `options_for_libraries` block, which takes a `selected`
parameter. We could imagine to use this include file for other
selects.

The 'mybranch` and `onlymine` subroutines of the C4::Branch package have
been moved to C4::Context. onlymine has been renamed with
only_my_library. There are only 4 occurrences of it, against 11 before
this patch set.
There 2 subroutines are Context-centric and it makes sense to put them
in `C4::Context` (at least it's the least worst place!)

GetBranches is the tricky part of this patch set: It retrieves all the
libraries, independently of the value of IndependentBranches.
To keep the same way as the existing calls of `Branches.all()`, I have
added a `unfiltered` parameter. If set, the `Branches.all()` will call
a usual Koha::Libraries->search method, otherwise
Koha::Libraries->search_filtered will be called. This new method will
check if the logged in user is allowed to see other libraries or only
its library.
Note that this `GetBranches` subroutine also created a `category` key:
it allowed to get the list of groups (of libraries) where this library
existed. Thanks to a previous patch set (bug 15295), this value was
not used anymore (I may have missed something!).

Note that the only use of `GetBranch` was buggy (see bug 15746).

Test plan (for the whole patch set):
The best way to test this whole patch set is to test with 2 instances: 1
with the patch set applied, 1 using master, to be sure there is no
regression.
It would be good to test the same with `IndependentBranches` and the
without `IndependentBranches`.
No difference should be found.
The tester must focus on the library dropdowns on as many forms as
possible.
You will notice changes in the order of the options: the libraries will
now be ordered by branchname (instead of branchcode in some places).
A special attention will be given to the following page:
- acqui/neworderempty.pl
- catalogue/search.pl
- members/members-home.pl (header?)
- opac/opac-topissues.pl
- tools/holidays.pl
- admin/branch_transfer_limits.pl
- admin/item_circulation_alerts.pl
- rotating_collections/transferCollection.pl
- suggestion/suggestion.pl
- tools/export.pl

Notes for QA:
- There are 2 FIXMEs in the patch set, I have kept the existing behavior,
but I am not sure it's the good one. Feel free to open a bug report and
I will fill a patch if you think it's not correct. Otherwise, remove the
FIXME lines in a follow-up patch.
- The whole patch set is huge and makes a lot of changes.
But it finally will tremendously reduce the number of lines:
716 insertions for 1910 deletions

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 115 Jonathan Druart 2016-09-08 15:32:29 UTC
Created attachment 55347 [details] [review]
Bug 15758: Koha::Libraries - Remove GetBranchesLoop

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 116 Jonathan Druart 2016-09-08 15:32:34 UTC
Created attachment 55348 [details] [review]
Bug 15758: Koha::Libraries - Move mybranch to C4::Context

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 117 Jonathan Druart 2016-09-08 15:32:40 UTC
Created attachment 55349 [details] [review]
Bug 15758: Koha::Libraries - Remove GetBranches

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 118 Jonathan Druart 2016-09-08 15:32:46 UTC
Created attachment 55350 [details] [review]
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 119 Jonathan Druart 2016-09-08 15:32:51 UTC
Created attachment 55351 [details] [review]
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 120 Jonathan Druart 2016-09-08 15:32:56 UTC
Created attachment 55352 [details] [review]
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 121 Jonathan Druart 2016-09-08 15:33:00 UTC
Created attachment 55353 [details] [review]
Bug 15758: Koha::Libraries - Do not select an option if selected is defined

This will certainly need a few more tweaks.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 122 Jonathan Druart 2016-09-08 15:33:06 UTC
Created attachment 55354 [details] [review]
Bug 15758: [QA Follow-up] Remove onlymine from Plugin/Branches.pm

Hard to miss that one:
Template process failed: plugin error - Bareword "C4::Branch::onlymine" not allowed while "strict subs" in use at Koha/Template/Plugin/Branches.pm line 59.
Compilation failed in require at /usr/lib/perl5/Template/Plugins.pm line 206.

Removed OnlyMine from Plugin/Branches. Replaced by a template var in
cleanborrowers; it was used only once.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 123 Jonathan Druart 2016-09-08 15:33:13 UTC
Created attachment 55355 [details] [review]
Bug 15758: [QA Follow-up] Some corrections related to GetBranchName

[1] Acquisition.pm
The lines filling $row in GetBasketGroupAsCSV may have side-effects when
the library name is not found. This change restores former behavior. Just
theoretically more safe.
Note that it also contained a typo: $row->{deliveryplace} should have been
$row->{$place}.

[2] Auth.pm
checkauth: $branchname = Koha::Libraries->find($branchcode)->branchname;
Should normally be fine, but I rather have an empty string here than
crashing on "Can't call method branchname on undefined value".
Same for sub check_api_auth.
Note that this holds for a larger number of calls, but I am adding a check
here because it is checkauth.
Also removed a duplicate use Koha::Libraries-statement.

[3] Search.pm
Also removed a duplicate use statement for Libraries.

[4] svc/holds
Added an (explicit) use statement for Koha::Libraries.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 124 Jonathan Druart 2016-09-08 15:33:18 UTC
Created attachment 55356 [details] [review]
Bug 15758: [QA Follow-up] Corrections related to GetBranches

[1] C4/Search
A call to Koha::Libraries is added to routine pazGetRecords, but the
results of that call are not used. So removing it again.

[2] catalogue/itemsearch.pl
Although A=>B=>C=>D works, we'd better use here A=>B, C=>D.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 125 Jonathan Druart 2016-09-08 15:33:23 UTC
Created attachment 55357 [details] [review]
Bug 15758: Fix variable name, it's a branchcode
Comment 126 Jonathan Druart 2016-09-08 15:33:28 UTC
Created attachment 55358 [details] [review]
Bug 15758: [QA Follow-up] Final touch?

From qa tools:
reports/cat_issues_top.pl: pod: =over on line 39 without closing =back

Note: qa tools does also report "options_for_libraries: not found" on two
template includes. Considering this as a false alarm, since the block is
defined in another include loaded in doc-head-open.
[1] koha-tmpl/intranet-tmpl/prog/en/includes/subscriptions-search.inc
[2] koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-topissues.inc

Found a template where a USE Branches was missing:
koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt
Easy to spot since the libraries combo was empty.
Tried to find similar occurrences with few greps, but couldn't find any.

I did find duplicate USE Branches though in:
koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
koha-tmpl/intranet-tmpl/prog/en/modules/serials/checkexpiration.tt
This patch removes them.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 127 Jonathan Druart 2016-09-08 15:34:02 UTC
Yet another rebase
Comment 128 Kyle M Hall 2016-09-08 16:19:44 UTC
Pushed to master for 16.11, thanks Jonathan, Marcel!
Comment 129 Tomás Cohen Arazi 2016-09-09 15:33:10 UTC
Created attachment 55445 [details] [review]
Bug 15758: (QA followup) Remove leftover reference to C4::Branch

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 130 Mason James 2016-09-29 09:18:50 UTC
(In reply to Kyle M Hall from comment #128)
> Pushed to master for 16.11, thanks Jonathan, Marcel!

hi there..
this patchset seems to cause the t/00-load.t test to fail?

root@xen1:/home/mason/g/k/master# koha-shell -c 'prove' mast3
...
Too late to run INIT block at /usr/share/koha/lib/C4/External/BakerTaylor.pm line 42.
t/00-load.t ......................... 117/?
#   Failed test 'use Koha::Libraries;'
#   at t/00-load.t line 65.
#     Tried to use 'Koha::Libraries'.
#     Error:  Bareword "C4::Context::only_my_library" not allowed while "strict subs" in use at Koha/Libraries.pm line 52.
# Compilation failed in require at t/00-load.t line 65.
# BEGIN failed--compilation aborted at t/00-load.t line 65.
Bailout called.  Further testing stopped:  ***** PROBLEMS LOADING FILE 'Koha::Libraries'
FAILED--Further testing stopped: ***** PROBLEMS LOADING FILE 'Koha::Libraries'
root@xen1:/home/mason/g/k/master#
Comment 131 Jonathan Druart 2016-09-29 11:16:38 UTC
(In reply to Mason James from comment #130)
> (In reply to Kyle M Hall from comment #128)
> > Pushed to master for 16.11, thanks Jonathan, Marcel!
> 
> hi there..
> this patchset seems to cause the t/00-load.t test to fail?

% prove t/00-load.t
t/00-load.t .. 1/? DBIx::Class::Storage::DBI::sql_maker(): Your storage class (DBIx::Class::Storage::DBI::Mock) does not set sql_limit_dialect and you have not supplied an explicit limit_dialect in your connection_info. DBIC will attempt to use the GenericSubQ dialect, which works on most databases but can be (and often is) painfully slow. Please file an RT ticket against 'DBIx::Class::Storage::DBI::Mock' at Koha/Objects.pm line 83
t/00-load.t .. ok     
All tests successful.
Files=1, Tests=523,  2 wallclock secs ( 0.08 usr  0.00 sys +  1.98 cusr  0.06 csys =  2.12 CPU)
Result: PASS

The C4::Context::only_my_library call is at line 48, not 52 on current master:
 48     if ( C4::Context::only_my_library ) {
Comment 132 Mason James 2017-05-24 01:17:50 UTC
(In reply to Jonathan Druart from comment #131)
> (In reply to Mason James from comment #130)
> > (In reply to Kyle M Hall from comment #128)
> > > Pushed to master for 16.11, thanks Jonathan, Marcel!
> > 
> > hi there..
> > this patchset seems to cause the t/00-load.t test to fail?
> 
> % prove t/00-load.t
> ..
> Result: PASS
> 

thanks Jonathan, I confirm it's working now too

(ps: I think I forgot to export my dev $PERL5LIB previously)
Comment 133 Mason James 2017-05-24 01:18:31 UTC
(In reply to Kyle M Hall from comment #128)
> Pushed to master for 16.11, thanks Jonathan, Marcel!

Enhancement, skipping for 16.05.x
Comment 134 David Cook 2017-10-10 00:21:44 UTC
This patch introduces a regression to the branch limit in the staff client advanced search. (The OPAC masthead search is ok and I think OPAC advanced search is too, but need to double-check.)

More details at Bug 18884...