Bug 33955

Summary: Add Koha::Biblio->normalized_upc
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Kyle M Hall (khall) <kyle>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, kyle, m.de.rooy, martin.renvoize, nick, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34828
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00
Circulation function:
Bug Depends on: 33954    
Bug Blocks: 33949, 33958    
Attachments: Bug 33955: Add a test
Bug 33955: Koha::Biblio::normalized_upc
Bug 33955: Add a test
Bug 33955: Koha::Biblio::normalized_upc
Bug 33955: Add a test
Bug 33955: Koha::Biblio::normalized_upc
[ALTERNATE] Bug 33955: Introduce Koha::MetadataExtractor and ->get_normalized_upc

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 (khall) 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 (khall) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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