February 2018 - Koha-bugs - lists.koha-community.org

[Bug 14769] Authorities merge: Set correct indicators in biblio field
by bugzilla-daemon＠bugs.koha-community.org 09 Feb '18

09 Feb '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14769 --- Comment #36 from Julian Maurice <julian.maurice(a)biblibre.com> --- (In reply to Marcel de Rooy from comment #35) > > - In Koha::Authority::ControlledIndicators, there is a package variable > > $cached_indicators. Maybe this module should use Koha::Cache instead ? > > I do not really see the advantage. We do not win time imo by saving the > structure to the L2 cache. We could argue about putting it in L1 only (and > check if we would gain time), but that seems not possible now. Since the > internal structure is kept only during the lifetime of the object, the name > could be misleading. If required, I could rename it. I'm just saying that maybe we should use Koha::Cache for consistency (I can totally be wrong :)) You're saying that the structure is kept only during the lifetime of the object, but with Plack, isn't the object's lifetime infinite ? > > > Also, I'm not sure if this is a problem, but it seems that there is no way > > to tell Koha to change only one of the two indicators. > > I am not sure if I understand this correctly. The current approach with the > pref allows for great flexibility. > If you say e.g. MARC21,100,ind1:auth1 you only copy the first indicator. The > second indicator will be empty when creating a new field or will be taken > from the biblio record when updating/merging. > But if you say e.g. MARC21,830,,ind2:auth2 you only copy the second > indicator. One of the unit tests contains an example where indicators are > swapped. With "MARC21,100,ind1:auth1" for instance, if I manually put something in the 2nd indicator, when I choose an authority it erases the content of the 2nd indicator. -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 14769] Authorities merge: Set correct indicators in biblio field
by bugzilla-daemon＠bugs.koha-community.org 09 Feb '18

09 Feb '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14769 --- Comment #35 from Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> --- (In reply to Julian Maurice from comment #33) Thanks Julian! > A few comments on the code: > - I think that the subroutine controlled_indicators doesn't belong in > Koha::Authority. The FIXME added in C4::AuthoritiesMarc strengthens that > opinion I removed the FIXME and optimized it a bit in the last follow-up. I thought about where to put this code when developing, and it is somewhat harder. This translated in putting the essential code in a separate object. (Parsing the pref and applying its rules on a MARC record should imo not be in Koha::Authority.) But since the indicators really are determined by the specific values of a given authority record, I decided to add the main controlled_indicators method to Koha::Authority. > - In Koha::Authority::ControlledIndicators, there is a package variable > $cached_indicators. Maybe this module should use Koha::Cache instead ? I do not really see the advantage. We do not win time imo by saving the structure to the L2 cache. We could argue about putting it in L1 only (and check if we would gain time), but that seems not possible now. Since the internal structure is kept only during the lifetime of the object, the name could be misleading. If required, I could rename it. > Also, I'm not sure if this is a problem, but it seems that there is no way > to tell Koha to change only one of the two indicators. I am not sure if I understand this correctly. The current approach with the pref allows for great flexibility. If you say e.g. MARC21,100,ind1:auth1 you only copy the first indicator. The second indicator will be empty when creating a new field or will be taken from the biblio record when updating/merging. But if you say e.g. MARC21,830,,ind2:auth2 you only copy the second indicator. One of the unit tests contains an example where indicators are swapped. So for UNIMARC you can also make all changes you want by adding specific rules. In comment27 I proposed to open a new report if we need further refinements for UNIMARC specifically. Hope this helps. -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 14769] Authorities merge: Set correct indicators in biblio field
by bugzilla-daemon＠bugs.koha-community.org 09 Feb '18

09 Feb '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14769 --- Comment #34 from Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> --- Created attachment 71366 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=71366&action=ed… Bug 14769: (Follow-up) Reduce number of preference parses In sub merge the Koha::Authority object was already created, so it was kind of silly to recreate it each time with new. In Koha::Authority, the ControlledIndicators object can be saved in the object too for reuse during its life time. Similarly, we prevent repeated lookups for the reporting tag (via authority type). Test plan: [1] Run t/db_dependent/Authority/Merge.t [2] Run t/db_dependent/Koha/Authorities.t Signed-off-by: Marcel de Rooy <m.de.rooy(a)rijksmuseum.nl> Limited benchmarking shows me a reduction of 11% on Merge.t and a reduction of 4% on Authorities.t when comparing with and without this last patch. The larger impact on Merge.t could be expected. -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 9302] Add ability to merge patron records
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '18

08 Feb '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9302 George Williams (NEKLS) <george(a)nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |george(a)nekls.org -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 14570] Make it possible to add multiple guarantors to a record
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '18

08 Feb '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14570 George Williams (NEKLS) <george(a)nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|gwilliams(a)nekls.org |george(a)nekls.org -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 12904] Force browser to load new javascript files after upgrade
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '18

08 Feb '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12904 --- Comment #98 from Jonathan Druart <jonathan.druart(a)bugs.koha-community.org> --- 1. Must misc4dev deal with apache *-git.conf files? If so, how? I think we will have to ask devs to add these 2 lines to /etc/koha/apache-shared-*-git.conf 2. <Joubu> 20 RewriteRule ^(.*)_(.*).js$ $1.js [N,L]. <Joubu> [N,L]? <tcohen> it looks like L shouldn't be there? or N? <tcohen> L = Last <tcohen> N = Next <tcohen> using N is very tricky <tcohen> and need to have a perfect understanding of how all defined rules make sense together Does someone know? -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 15191] Serials display improvements
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '18

08 Feb '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15191 Chris Slone <cslone(a)camdencountylibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |cslone@camdencountylibrary. | |org -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 14962] Temp Shelving Location
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '18

08 Feb '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14962 Jessie Zairo <jzairo(a)bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jzairo(a)bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 10858] Browse selected biblios - OPAC
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '18

08 Feb '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10858 Jonathan Druart <jonathan.druart(a)bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion -- You are receiving this mail because: You are watching all bug changes.

1 0

[Bug 14769] Authorities merge: Set correct indicators in biblio field
by bugzilla-daemon＠bugs.koha-community.org 08 Feb '18

08 Feb '18

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14769 --- Comment #33 from Julian Maurice <julian.maurice(a)biblibre.com> --- Patches work as expected. I signed off the patches but I think another sign-off from a MARC21 expert would be great. A few comments on the code: - I think that the subroutine controlled_indicators doesn't belong in Koha::Authority. The FIXME added in C4::AuthoritiesMarc strengthens that opinion - In Koha::Authority::ControlledIndicators, there is a package variable $cached_indicators. Maybe this module should use Koha::Cache instead ? Also, I'm not sure if this is a problem, but it seems that there is no way to tell Koha to change only one of the two indicators. -- You are receiving this mail because: You are watching all bug changes.

1 0