[Bug 37380] New: Move GetMarcControlnumber to Koha::*
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Bug ID: 37380 Summary: Move GetMarcControlnumber to Koha::* Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org Easy one to move. -- 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=37380 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- 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=37380 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 169058 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169058&action=edit Bug 37380: Unit tests -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 169059 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169059&action=edit Bug 37380: Add get_control_number() to the metadata extractor This simple patch implements C4::Biblio::GetMarcControlnumber in the right spot on the Koha::* namespace. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC.t => SUCCESS: Tests pass! 3. Sign off :-D -- 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=37380 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 169060 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169060&action=edit Bug 37380: Replace uses of GetMarcControlnumber for get_control_number() -- 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=37380 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17628 CC| |tomascohen@gmail.com Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17628 [Bug 17628] Move bibliographic related code to Koha::Biblio -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |Academy, Sandbox CC| |david@davidnind.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|Sandbox |no-sandbox -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Sam Lau <samalau@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Sam Lau <samalau@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169058|0 |1 is obsolete| | --- Comment #4 from Sam Lau <samalau@gmail.com> --- Created attachment 169777 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169777&action=edit Bug 37380: Unit tests Signed-off-by: Sam Lau <samalau@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Sam Lau <samalau@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169059|0 |1 is obsolete| | --- Comment #5 from Sam Lau <samalau@gmail.com> --- Created attachment 169778 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169778&action=edit Bug 37380: Add get_control_number() to the metadata extractor This simple patch implements C4::Biblio::GetMarcControlnumber in the right spot on the Koha::* namespace. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Sam Lau <samalau@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Sam Lau <samalau@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169060|0 |1 is obsolete| | --- Comment #6 from Sam Lau <samalau@gmail.com> --- Created attachment 169779 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169779&action=edit Bug 37380: Replace uses of GetMarcControlnumber for get_control_number() Signed-off-by: Sam Lau <samalau@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@gmail.com --- Comment #7 from Jonathan Druart <jonathan.druart@gmail.com> --- + my $control_num = $biblio->get_control_number(); This is not correct. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 --- Comment #8 from Jonathan Druart <jonathan.druart@gmail.com> --- + is( $extractor->get_control_number, undef, 'Returns undef if no 001' ); - is( $controlnumber, '', 'GetMarcControlnumber handles records without 001' ); Change in behaviour here, did you investigate it? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #7)
+ my $control_num = $biblio->get_control_number();
This is not correct.
You're right! (In reply to Jonathan Druart from comment #8)
+ is( $extractor->get_control_number, undef, 'Returns undef if no 001' );
- is( $controlnumber, '', 'GetMarcControlnumber handles records without 001' );
Change in behaviour here, did you investigate it?
I did! But I will re-do just in case! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169777|0 |1 is obsolete| | Attachment #169778|0 |1 is obsolete| | Attachment #169779|0 |1 is obsolete| | --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 171099 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171099&action=edit Bug 37380: Unit tests Signed-off-by: Sam Lau <samalau@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 171100 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171100&action=edit Bug 37380: Add get_control_number() to the metadata extractor This simple patch implements C4::Biblio::GetMarcControlnumber in the right spot on the Koha::* namespace. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Sam Lau <samalau@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 171101 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171101&action=edit Bug 37380: Replace uses of GetMarcControlnumber for get_control_number() Signed-off-by: Sam Lau <samalau@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Hi, I rolled back with making it return undef. As I noticed the tradeoff was not good. We ended up having to add `// q{}` in a bunch of places, which is not worth. Tests and code have been adjusting to go back to the old behavior. I wanted to share that I made some exhaustive tests on MARC::Record and MARC::Field behavior... ``` perl -MDDP \ -MMARC::File::XML \ -MMARC::Record \ -MMARC::Field \ -e 'my $f = MARC::Field->new( "001", undef); p($f->data); my $r = MARC::Record->new(); $r->append_fields($f); $r->append_fields(MARC::Field->new("245", "", "", a => "Stuff")); my $xml = $r->as_xml; $r = MARC::Record->new_from_xml($xml); p($r->as_xml);' ``` My conclusion is that MARC::Field allows adding `undef` values but it will then warn when appending them. But I also found out that once the record gets serialized as XML, the `undef` is replaced by an empty string and that's how it is going to stay forever, with our code. So having `undef` is a weird artificially created scenario. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Jonathan Druart <jonathan.druart@gmail.com> 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=37380 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #171099|0 |1 is obsolete| | Attachment #171100|0 |1 is obsolete| | Attachment #171101|0 |1 is obsolete| | --- Comment #14 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 171189 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171189&action=edit Bug 37380: Unit tests Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 --- Comment #15 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 171190 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171190&action=edit Bug 37380: Add get_control_number() to the metadata extractor This simple patch implements C4::Biblio::GetMarcControlnumber in the right spot on the Koha::* namespace. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 --- Comment #16 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 171191 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171191&action=edit Bug 37380: Replace uses of GetMarcControlnumber for get_control_number() Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 --- Comment #17 from Jonathan Druart <jonathan.druart@gmail.com> --- Looking at catalogue/export.pl: my $marc = $biblio->metadata->record({ embed_items => 1 }); my $metadata_extractor = $biblio->metadata_extractor; We are going to fetch metadata twice. We should certainly improve it at some point. Not considering blocker for this change however. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |Main Summary|Move GetMarcControlnumber |Move GetMarcControlnumber |to Koha::* |to Koha namespace QA Contact|testopia@bugs.koha-communit |jonathan.druart@gmail.com |y.org | Keywords|Academy, no-sandbox | --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I originally introduced this method in bug 5430 which which seems like ages ago now. But I won't miss it :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Pushed to main |Needs documenting -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Skipping for release notes as this is quite technical and kind of self-explanatory to developers. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37380 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED --- Comment #21 from David Nind <david@davidnind.com> --- This is more developer focused, and is not covered in the manual - so no update to the manual required. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org