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

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 135232 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135232&action=edit Bug 30813: Update TransformMarcToKoha to accept a hashref This patch updates all the calls to pass a hasref rather than an array It also removes the no longer used framework parameter To test: prove -v t/Biblio.t t/db_dependent/Biblio/TransformMarcToKoha.t -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 135234 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135234&action=edit Bug 30813: Remove TransformMarcToKohaOneField To test: 1 - prove -v t/db_dependent/Biblio/TransformMarcToKoha.t t/db_dependent/Breeding.t 2 - Test Z3950 search in advanced catalog editor -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135233|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 135240 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135240&action=edit Bug 30813: Adjust TransformMarcToKoha to take kohafields parameter This adjusts the routine to accept an arrayref of koha fields to process To test: prove -v t/db_dependent/Biblio/TransformMarcToKoha.t t/Biblio.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135235|0 |1 is obsolete| | --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 135242 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135242&action=edit Bug 30813: Remove Koha::MetadataRecord::getKohaField This was essentially a wrapper around TransformKohaToMarcOneField After changing and updating, it seems easier not to create the intermediary object and to just transform the data Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 135243 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135243&action=edit Bug 30813: (follow-up) ImportBatch updates Missed these in the original commits -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30824 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30824 [Bug 30824] Improve performance of BatchCommitItems -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135240|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 135303 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135303&action=edit Bug 30813: Adjust TransformMarcToKoha to take kohafields parameter This adjusts the routine to accept an arrayref of koha fields to process To test: prove -v t/db_dependent/Biblio/TransformMarcToKoha.t t/Biblio.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135242|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 135305 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135305&action=edit Bug 30813: Remove Koha::MetadataRecord::getKohaField This was essentially a wrapper around TransformKohaToMarcOneField After changing and updating, it seems easier not to create the intermediary object and to just transform the data Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |joonas.kylmala@iki.fi --- Comment #16 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- If you go to

/cgi-bin/koha/cataloguing/addbooks.pl

and perform a search using "New from Z39.50" you get:

Undefined subroutine &C4::Breeding::TransformMarcToKoha called at /kohadevbox/koha/C4/Breeding.pm line 311

to reproduce the error in the search pop-up put as the Title "aa". If you fix the missing import then the search results are still empty however (they should not be empty for this keyword), so there is a bit more to fix this, I will leave it to the author of the patch set. - Koha/MetaSearcher.pm doesn't import C4::Biblio - Koha/MetadataRecord.pm imports now C4::Biblio unnecessarily -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #17 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 135371 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135371&action=edit Bug 30813: (follow-up) Correct C4::Breeding call This patch changes the call to use the fully qualified name and adds import to C4::Breeding, Koha::MetaSearcher, and removes import from Koha::MetadataRecord Additionally it replaces missing fields from the update to using TransformMarcToKoha Lastly, it reduces the fields used when saving the bredding record to the reservoir -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #19 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 135375 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135375&action=edit Bug 30813: Adjust TransformMarcToKoha to take kohafields parameter This adjusts the routine to accept an arrayref of koha fields to process To test: prove -v t/db_dependent/Biblio/TransformMarcToKoha.t t/Biblio.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #21 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 135377 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135377&action=edit Bug 30813: Remove Koha::MetadataRecord::getKohaField This was essentially a wrapper around TransformKohaToMarcOneField After changing and updating, it seems easier not to create the intermediary object and to just transform the data Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #23 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 135379 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135379&action=edit Bug 30813: (follow-up) Correct C4::Breeding call This patch changes the call to use the fully qualified name and adds import to C4::Breeding, Koha::MetaSearcher, and removes import from Koha::MetadataRecord Additionally it replaces missing fields from the update to using TransformMarcToKoha Lastly, it reduces the fields used when saving the bredding record to the reservoir Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |BLOCKED --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Taking a look here too -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #26)

my $mss = GetMarcSubfieldStructure( '', { unsafe => 1 } ); # Do not change framework @{$kohafields} = keys %{ $mss } unless $kohafields; foreach my $kohafield ( @{$kohafields} ) { Hmm. I dont understand: Why pass kohafields and still do a GetMarcSubfieldSttructure call ?

Misreading the change. Sorry. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #29 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #28)

Coming back here very soon. Please keep status.

Ok, do you have an idea of the timing for this one? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135375|0 |1 is obsolete| | --- Comment #32 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 135764 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135764&action=edit Bug 30813: Adjust TransformMarcToKoha to take kohafields parameter This adjusts the routine to accept an arrayref of koha fields to process To test: prove -v t/db_dependent/Biblio/TransformMarcToKoha.t t/Biblio.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135377|0 |1 is obsolete| | --- Comment #34 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 135766 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135766&action=edit Bug 30813: Remove Koha::MetadataRecord::getKohaField This was essentially a wrapper around TransformKohaToMarcOneField After changing and updating, it seems easier not to create the intermediary object and to just transform the data Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135379|0 |1 is obsolete| | --- Comment #36 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 135768 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135768&action=edit Bug 30813: (follow-up) Correct C4::Breeding call This patch changes the call to use the fully qualified name and adds import to C4::Breeding, Koha::MetaSearcher, and removes import from Koha::MetadataRecord Additionally it replaces missing fields from the update to using TransformMarcToKoha Lastly, it reduces the fields used when saving the bredding record to the reservoir Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #38 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Great work, Nick. Removing that framework parameter was still something far down on my todo list ;) Might still be a report for; will close it. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #40 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Nick Clemens from comment #39)

Why add the qw(); to the use statements?

That is essential to prevent needlessly importing all default exported routines from the module. (EXPORT_OK routines) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #42 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Nick Clemens from comment #41)

(In reply to Marcel de Rooy from comment #40)

...

(In reply to Nick Clemens from comment #39)

...

Why add the qw(); to the use statements?

That is essential to prevent needlessly importing all default exported routines from the module. (EXPORT_OK routines)

From bug 17600 I understood that EXPORT_OK was 'on demand' i.e. only explicitly named subroutines are imported. Is that incorrect?

Ah. You might very well be right. EXPORT versus EXPORT_OK So yes, might not be needed then. But explicitly stating that you dont want to import cannot be wrong either. I would recommend keeping them. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #44 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #43)

Please explicitly import the subroutines that are used in the scripts/modules, that's the point of bug 17600. That helps to clean the import (remove the 'use' statements when no subroutines are imported).

No. You may want to require a module without importing its names. So you do use Module qw(); -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #46 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #45)

(In reply to Marcel de Rooy from comment #44)

...

(In reply to Jonathan Druart from comment #43)

...

Please explicitly import the subroutines that are used in the scripts/modules, that's the point of bug 17600. That helps to clean the import (remove the 'use' statements when no subroutines are imported).

No. You may want to require a module without importing its names. So you do use Module qw();

No? I can assure you that it's why I wrote bug 17600. We want to EXPLICITLY export the subroutines we will be using in the scripts/modules.

You dont understand what I mean. Please read it again. No refers to the case where we do not import. I say no, do not remove the use statement when you want to require the module anyway. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135769|0 |1 is obsolete| | --- Comment #48 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 135803 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135803&action=edit Bug 30813: (QA follow-up) Adjust three use statements Altough we used the namespace when calling, the intention of bug 17600 was that we do import the routines that we are calling here. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30813 --- Comment #49 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.