Bug 33955 - Add Koha::Biblio->normalized_upc
Summary: Add Koha::Biblio->normalized_upc
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on: 33954
Blocks: 33949 33958
  Show dependency treegraph
 
Reported: 2023-06-08 10:12 UTC by Jonathan Druart
Modified: 2023-10-14 12:26 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00


Attachments
Bug 33955: Add a test (1.24 KB, patch)
2023-06-08 10:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33955: Koha::Biblio::normalized_upc (1.59 KB, patch)
2023-06-08 10:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33955: Add a test (1.29 KB, patch)
2023-09-12 08:49 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 33955: Koha::Biblio::normalized_upc (1.58 KB, patch)
2023-09-12 08:50 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 33955: Add a test (1.35 KB, patch)
2023-09-15 16:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 33955: Koha::Biblio::normalized_upc (1.64 KB, patch)
2023-09-15 16:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
[ALTERNATE] Bug 33955: Introduce Koha::MetadataExtractor and ->get_normalized_upc (8.28 KB, patch)
2023-09-18 14:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2023-06-08 10:12:23 UTC

    
Comment 1 Jonathan Druart 2023-06-08 10:56:52 UTC
Created attachment 152152 [details] [review]
Bug 33955: Add a test
Comment 2 Jonathan Druart 2023-06-08 10:56:55 UTC
Created attachment 152153 [details] [review]
Bug 33955: Koha::Biblio::normalized_upc
Comment 3 Chris Cormack 2023-09-12 08:49:50 UTC
Created attachment 155542 [details] [review]
Bug 33955: Add a test

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 4 Chris Cormack 2023-09-12 08:50:06 UTC
Created attachment 155543 [details] [review]
Bug 33955: Koha::Biblio::normalized_upc

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 5 Kyle M Hall 2023-09-15 16:56:09 UTC
Created attachment 155699 [details] [review]
Bug 33955: Add a test

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2023-09-15 16:56:34 UTC
Created attachment 155700 [details] [review]
Bug 33955: Koha::Biblio::normalized_upc

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Tomás Cohen Arazi 2023-09-18 13:27:03 UTC
I have doubts about the current approach to extracting things from MARC records. It feels really bad that we call $self->metadata->record each time, parsing the MARCXML (I don't claim we hit the DB, but it is probably the case most of the time).

I'm setting this in discussion, and will provide an alternate patch for discussing the pattern entirely.
Comment 8 Tomás Cohen Arazi 2023-09-18 14:10:36 UTC
Created attachment 155819 [details] [review]
[ALTERNATE] Bug 33955: Introduce Koha::MetadataExtractor and ->get_normalized_upc

This patch introduces a new pattern for the different ->get_<thing>
methods we've been adding. The aim is that code will look more like:

my $metadata_extractor = Koha::MetadataExtractor->new;

while ( my $biblio = $biblios->next ) {
    my $record = $biblio->record;
    my $schema = $biblio->record_schema;

    $data->{$biblio->id}->{normalized_upc} =
$metadata_extractor->get_normalized_upc( { record => $record, schema =>
$schema } );
    $data->{$biblio->id}->{normalized_ean} =
$metadata_extractor->get_normalized_ean( { record => $record, schema =>
$schema } );
}

The key is that we are actually reusing the MARC::Record, and code for
each schema is organized cleanly so easier to maintain.
Comment 9 Nick Clemens (kidclamp) 2023-09-18 14:33:48 UTC
I like where Tomas is going with this, but I wonder if we could make this simpler by storing this info in the DB.

We have need for a UPC field in any case, why not have a normalized_upc (and isbn, issn....)

If we don't quiote go that far, we could still normalize from the parsed DB value, rather than the record
Comment 10 Tomás Cohen Arazi 2023-09-18 14:40:41 UTC
(In reply to Nick Clemens from comment #9)
> We have need for a UPC field in any case, why not have a normalized_upc (and
> isbn, issn....)

I like that...
Comment 11 Tomás Cohen Arazi 2023-09-18 14:45:08 UTC
(In reply to Tomás Cohen Arazi from comment #10)
> (In reply to Nick Clemens from comment #9)
> > We have need for a UPC field in any case, why not have a normalized_upc (and
> > isbn, issn....)
> 
> I like that...

Do we need a Koha to MARC mapping?
Comment 12 Jonathan Druart 2023-09-18 14:59:06 UTC
This was really a simple patch, as simple as possible to reuse the existing C4 sub, instead of duplicating the code.

I think it's a good idea to have something more robust, but this is not the place for that.

We should implement it either after the different "Add Koha::Biblio->normalized_*" bugs, or before.
Comment 13 Tomás Cohen Arazi 2023-09-18 15:31:11 UTC
(In reply to Jonathan Druart from comment #12)
> This was really a simple patch, as simple as possible to reuse the existing
> C4 sub, instead of duplicating the code.

I can see that.
Comment 14 Tomás Cohen Arazi 2023-09-19 12:29:19 UTC
Comment on attachment 155819 [details] [review]
[ALTERNATE] Bug 33955: Introduce Koha::MetadataExtractor and ->get_normalized_upc

Moving the Koha::MetadataExtractor work to its own bug to let this move forward.
Comment 15 Tomás Cohen Arazi 2023-09-19 19:35:39 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 16 Fridolin Somers 2023-09-21 06:19:27 UTC
Enhancement not pushed to 23.05.x