[Bug 35142] New: Koha::Util::oclc_number should be removed in favor of Koha::Biblio::Metadata::Extractor
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 Bug ID: 35142 Summary: Koha::Util::oclc_number should be removed in favor of Koha::Biblio::Metadata::Extractor 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: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 30678, 34828 See bug 34828. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30678 [Bug 30678] Add OCLC_NO as option to OPACSearchForTitleIn https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34828 [Bug 34828] Add Koha::Biblio::Metadata::Extractor* classes -- 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=35142 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- \o/ -- 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=35142 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |ASSIGNED -- 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=35142 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198873 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198873&action=edit Bug 35142: Remove GetNormalizedOCLCNumber and oclc_number Unifies OCLC number extraction into the MetadataExtractor pattern. The get_normalized_oclc method now lives in the base ::MARC class (035$a is flavor-independent) with improved matching: - Case-insensitive OCoLC detection (matches ocolc, OCOLC, etc.) - Generic orgcode prefix stripping (any parenthesized prefix, not just literal "(OCoLC)") for resilience against cataloging typos This resolves a pre-existing inconsistency where different code paths produced different results for the same record depending on which implementation was used. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/Koha/Util/MARC.t \ t/db_dependent/Koha.t \ t/db_dependent/Koha/Biblio.t \ t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC/MARC21.t => SUCCESS: Tests pass 3. Verify OCLC numbers still display in OPAC and staff detail pages 4. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42566 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42566 [Bug 42566] Remove C4::Koha::GetNormalizedUPC in favor of MetadataExtractor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- Is anything special required to get the OCLC number to display in the staff interface or OPAC record? Note: The number of tests for t/db_dependent/Koha/Biblio.t needs updating (these pass once I change the number of tests from 44 to 43). Testing notes (using KTD - MARC21): 1. I updated the BK framework so that 035 $6, $8, $a and $z are visible in the editor. 2. I added an OCLC number to Programming Perl (262): (OCoLC)902632762 3. Nothing shows in the staff interface or OPAC -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@openfifth.c | |o.uk --- Comment #4 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- There's a possible regression here lurking.. raw vs. ViewPolicy-filtered records in the OPAC displays. I don't think it actually matters in this case specifically.. whose going to hide the 035?, but it may in subsequent such migrations. The old callers passed a local $record that had been processed in place by $record_processor->process($record) (ViewPolicy filter) before OCLC extraction. The new $biblio->normalized_oclc reads $biblio->metadata->record — and neither Koha::Biblio::metadata nor Koha::Biblio::Metadata::record caches, so it rebuilds a fresh, unfiltered MARC::Record from XML. Affected sites where the source record genuinely differs: - opac/opac-detail.pl:146 — $record_processor->process($record) mutates the record previously used for OCLC. - opac/opac-ISBDdetail.pl:119 — same in-place ViewPolicy processing. - opac/opac-user.pl — old code used metadata_record({ interface => 'opac', ... }), an OPAC-filtered copy. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Koha/Biblio.pm:28 still imports GetNormalizedUPC, which is no longer used (normalized_upc delegates to the extractor). It's pre-existing, but since the patch already edits this exact use C4::Koha line, dropping GetNormalizedUPC here too is a free cleanup. (GetNormalizedISBN is still used at line 1656 — keep it.) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198873|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 200672 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200672&action=edit Bug 35142: Remove GetNormalizedOCLCNumber and oclc_number Unifies OCLC number extraction into the MetadataExtractor pattern. The get_normalized_oclc method now lives in the base ::MARC class (035$a is flavor-independent) with improved matching: - Case-insensitive OCoLC detection (matches ocolc, OCOLC, etc.) - Generic orgcode prefix stripping (any parenthesized prefix, not just literal "(OCoLC)") for resilience against cataloging typos This resolves a pre-existing inconsistency where different code paths produced different results for the same record depending on which implementation was used. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/Koha/Util/MARC.t \ t/db_dependent/Koha.t \ t/db_dependent/Koha/Biblio.t \ t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC/MARC21.t => SUCCESS: Tests pass 3. Verify OCLC numbers still display in OPAC and staff detail pages 4. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 200673 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200673&action=edit Bug 35142: (follow-up) Fix test plan count in Koha/Biblio.t The normalized_oclc subtest was expanded from 1 to 3 internal tests, but this does not change the top-level plan count (a subtest counts as a single test). The top-level plan was incorrectly bumped from 43 to 44, causing: Bad plan. You planned 44 tests but ran 43. Revert the plan to 43. Test plan: 1. prove t/db_dependent/Koha/Biblio.t => FAIL before, PASS after Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 --- Comment #8 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 200674 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200674&action=edit Bug 35142: (follow-up) Remove unused GetNormalizedUPC import Koha::Biblio::normalized_upc delegates to the metadata extractor, so GetNormalizedUPC is no longer called anywhere in Koha/Biblio.pm. Drop it from the C4::Koha import list. GetNormalizedISBN is still used by normalized_isbn and is retained. Test plan: 1. perl -c Koha/Biblio.pm => syntax OK 2. git grep GetNormalizedUPC Koha/Biblio.pm => no matches Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 --- Comment #9 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- One last thing.. which I think is a bit more open to discussion: DRY: the new get_normalized_oclc inlines s/^\([^)]*\)\s*//, which is byte-for-byte identical to the surviving Koha::Util::MARC::strip_orgcode, and inlines /OCoLC/i, which was the removed OCLC_REGEX. The removed Koha::Util::MARC::oclc_number already expressed exactly this as strip_orgcode(scalar find_marc_info({... match => qr/OCoLC/i})). Reusing those tested helpers would keep a single source of truth rather than re-duplicating the logic the patch set out to de-duplicate. But, the counter-argument is that the inlined version is self-contained and readable.? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200672|0 |1 is obsolete| | Attachment #200673|0 |1 is obsolete| | Attachment #200674|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 --- Comment #10 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201770 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201770&action=edit Bug 35142: Move get_normalized_oclc to base MARC extractor Moves get_normalized_oclc from the MARC21-specific extractor to the base ::MARC class (035$a is flavor-independent) with improved matching: - Case-insensitive OCoLC detection (matches ocolc, OCOLC, etc.) - Generic orgcode prefix stripping (any parenthesized prefix, not just literal "(OCoLC)") for resilience against cataloging typos Test plan: $ prove t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC/MARC21.t Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 --- Comment #11 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201771 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201771&action=edit Bug 35142: Migrate callers to use metadata extractor Replaces all calls to GetNormalizedOCLCNumber() and Koha::Util::MARC::oclc_number() with the extractor pattern. OPAC scripts pass their already ViewPolicy-filtered $record directly to the extractor. This preserves the existing filtering behavior without introducing new abstraction layers: Koha::Biblio::Metadata::Extractor->new( { metadata => $record } )->get_normalized_oclc Intranet scripts and the Koha::Biblio convenience method use the raw cached extractor (035 visibility is not affected by ViewPolicy in practice, and intranet doesn't filter): $biblio->normalized_oclc Test plan: 1. Verify OCLC numbers display on OPAC detail, ISBD, MARC detail 2. Verify staff catalogue detail and search results 3. prove t/db_dependent/Koha/Biblio.t Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 --- Comment #12 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201772 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201772&action=edit Bug 35142: Remove GetNormalizedOCLCNumber, oclc_number, strip_orgcode All callers migrated. Removes dead code: - C4::Koha::GetNormalizedOCLCNumber - Koha::Util::MARC::oclc_number - Koha::Util::MARC::strip_orgcode - Koha::Util::MARC::OCLC_REGEX constant Test plan: $ prove t/Koha/Util/MARC.t t/db_dependent/Koha.t Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 --- Comment #13 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Martin Renvoize (ashimema) from comment #4)
There's a possible regression here lurking.. raw vs. ViewPolicy-filtered records in the OPAC displays.
Solved this. Multi-tasking had me have a branch with this fixed but never submitted! (In reply to Martin Renvoize (ashimema) from comment #9)
Reusing those tested helpers would keep a single source of truth rather than re-duplicating the logic the patch set out to de-duplicate. But, the counter-argument is that the inlined version is self-contained and readable.?
I like the end result, and the are no more places using that anymore. If we find a good use case for having helper subs we can move the snippets into class methods. But I doubt it for now! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 David Nind <david@davidnind.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=35142 --- Comment #14 from David Nind <david@davidnind.com> --- Created attachment 201841 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201841&action=edit Bug 35142: Move get_normalized_oclc to base MARC extractor Moves get_normalized_oclc from the MARC21-specific extractor to the base ::MARC class (035$a is flavor-independent) with improved matching: - Case-insensitive OCoLC detection (matches ocolc, OCOLC, etc.) - Generic orgcode prefix stripping (any parenthesized prefix, not just literal "(OCoLC)") for resilience against cataloging typos Test plan: $ prove t/db_dependent/Koha/Biblio/Metadata/Extractor/MARC/MARC21.t Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: David Nind <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=35142 --- Comment #16 from David Nind <david@davidnind.com> --- Created attachment 201843 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201843&action=edit Bug 35142: Remove GetNormalizedOCLCNumber, oclc_number, strip_orgcode All callers migrated. Removes dead code: - C4::Koha::GetNormalizedOCLCNumber - Koha::Util::MARC::oclc_number - Koha::Util::MARC::strip_orgcode - Koha::Util::MARC::OCLC_REGEX constant Test plan: $ prove t/Koha/Util/MARC.t t/db_dependent/Koha.t Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: David Nind <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=35142 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201770|0 |1 is obsolete| | Attachment #201771|0 |1 is obsolete| | Attachment #201772|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35142 --- Comment #15 from David Nind <david@davidnind.com> --- Created attachment 201842 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201842&action=edit Bug 35142: Migrate callers to use metadata extractor Replaces all calls to GetNormalizedOCLCNumber() and Koha::Util::MARC::oclc_number() with the extractor pattern. OPAC scripts pass their already ViewPolicy-filtered $record directly to the extractor. This preserves the existing filtering behavior without introducing new abstraction layers: Koha::Biblio::Metadata::Extractor->new( { metadata => $record } )->get_normalized_oclc Intranet scripts and the Koha::Biblio convenience method use the raw cached extractor (035 visibility is not affected by ViewPolicy in practice, and intranet doesn't filter): $biblio->normalized_oclc Test plan: 1. Verify OCLC numbers display on OPAC detail, ISBD, MARC detail 2. Verify staff catalogue detail and search results 3. prove t/db_dependent/Koha/Biblio.t Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: David Nind <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=35142 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org