[Bug 15758] New: Move the C4::Branch related code to Koha::Libraries - part 4 - Koha-bugs

newer
[Bug 15156] Get all Borrowers with...

[Bug 15758] New: Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

8 Feb 2016 8 Feb '16

3:27 p.m.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Bug ID: 15758 Summary: Move the C4::Branch related code to Koha::Libraries - part 4 Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Depends on: 15629 Blocks: 15293 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. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15293 [Bug 15293] Move the C4::Branch related code to Koha::Libraries https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15629 [Bug 15629] Move the C4::Branch related code to Koha::Libraries - part 3 -- You are receiving this mail because: You are watching all bug changes.

Show replies by date

bugzilla-daemon＠bugs.koha-community.org

8 Feb 8 Feb

bugzilla-daemon＠bugs.koha-community.org

5:49 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

12 Feb 12 Feb

9:10 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=15807 -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

5:01 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Heather Braum <hbraum@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hbraum@nekls.org -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

22 Jun 22 Jun

6:16 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

23 Jun 23 Jun

3:05 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

3:06 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #47746|0 |1 is obsolete| | --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 52723 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52723&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

3:07 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

3:08 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

3:09 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

3:10 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

3:11 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

3:15 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

3:16 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

3:29 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #25 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 52736 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52736&action=edit Bug 15758: Add missing use of Branches TT plugin in template -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

5:17 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion --- Comment #26 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- 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 -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

5:23 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

5:59 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #28 from Owen Leonard <oleonard@myacpl.org> --- 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. -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

bugzilla-daemon＠bugs.koha-community.org

1:50 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #37 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (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? -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

27 Jun 27 Jun

7:32 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

28 Jun 28 Jun

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.

5:14 p.m.

5:17 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

...

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! -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

5:23 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #65 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (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. -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

5 Aug 5 Aug

1 p.m.

1:43 p.m.

1:45 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

25 Aug 25 Aug

1:55 p.m.

bugzilla-daemon＠bugs.koha-community.org

26 Aug 26 Aug

9:47 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #91 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54908 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54908&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

9:47 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #92 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54909 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54909&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

9:47 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #93 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- No not ready yet (by far..) -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

10:59 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #94 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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 -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

11:16 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #95 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (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! -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

1:02 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #96 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 54921 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54921&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

1:04 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #97 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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. -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

2:08 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #98 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (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. -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

2:08 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #99 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 54925 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=54925&action=edit Bug 15758: Fix variable name, it's a branchcode -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

31 Aug 31 Aug

3:10 p.m.

3:11 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #112 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 55073 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55073&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

3:15 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

5 Sep 5 Sep

7:19 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |10300 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10300 [Bug 10300] Allow transferring of items to be have separate IndependentBranches syspref -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

8 Sep 8 Sep

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

bugzilla-daemon＠bugs.koha-community.org

4:34 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

5:19 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #128 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks Jonathan, Marcel! -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

9 Sep 9 Sep

4:33 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #129 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 55445 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55445&action=edit Bug 15758: (QA followup) Remove leftover reference to C4::Branch Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

7:14 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17285 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17285 [Bug 17285] Rancor - Advanded editor fails or broken -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

13 Sep 13 Sep

3:12 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17295 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17295 [Bug 17295] Missed variable removal in subscription-add.pl from Bug 15758 -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

29 Sep 29 Sep

10:18 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com --- Comment #130 from Mason James <mtj@kohaaloha.com> --- (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# -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

12:16 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #131 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (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 ) { -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

2 Nov 2 Nov

10:54 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17537 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17537 [Bug 17537] xt/author/valid-templates.t is broken -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

28 Nov 28 Nov

10:39 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

bugzilla-daemon＠bugs.koha-community.org

22 Mar 22 Mar

2:52 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18307 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18307 [Bug 18307] Branchname is no more displayed in subscription tab wiew -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

5 May 5 May

11:57 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18534 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18534 [Bug 18534] When IndependentBranches is enabled the pickup location displayed incorrectly on request.pl -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

24 May 24 May

2:17 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #132 from Mason James <mtj@kohaaloha.com> --- (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) -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

2:18 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 --- Comment #133 from Mason James <mtj@kohaaloha.com> --- (In reply to Kyle M Hall from comment #128)

...

Pushed to master for 16.11, thanks Jonathan, Marcel!

Enhancement, skipping for 16.05.x -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

10 Aug 10 Aug

7:25 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19073 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19073 [Bug 19073] Can't change library with patron batch modification tool -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

10 Oct 10 Oct

1:21 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #134 from David Cook <dcook@prosentient.com.au> --- 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... -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

2:03 a.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=18884 -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

27 Oct 27 Oct

3:49 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18884 See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=18884 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18884 [Bug 18884] Advanced search on staff client, Availability limit not properly limiting -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

11 Dec 11 Dec

4:16 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19769 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19769 [Bug 19769] 'Pickup library is different' message does not display library branch name when placing hold -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

2 Apr 2 Apr

8:49 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |20282 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20282 [Bug 20282] Wrong attribute in template calls to match holding branch when adding/editing a course reserve item -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

23 Oct 23 Oct

3:41 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |11426 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11426 [Bug 11426] Make HighlightOwnItemsOnOPAC work with XSLT -- You are receiving this mail because: You are watching all bug changes.

bugzilla-daemon＠bugs.koha-community.org

23 Jun 23 Jun

3:08 p.m.

New subject: [Bug 15758] Move the C4::Branch related code to Koha::Libraries - part 4

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15758 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28622 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28622 [Bug 28622] selected branchcode not passed to adv search -- You are receiving this mail because: You are watching all bug changes.

1848

Age (days ago)

3810

Last active (days ago)

List overview

Download

169 comments

1 participants

participants (1)

bugzilla-daemon＠bugs.koha-community.org