From c895a8d5633fdda1793099fa063010b5da5ace2b Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Tue, 16 Jul 2024 15:42:10 -0300
Subject: [PATCH] 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>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
---
 C4/Biblio.pm            | 27 ---------------------------
 catalogue/export.pl     | 12 ++++++------
 opac/opac-ISBDdetail.pl |  7 ++++---
 opac/opac-MARCdetail.pl | 17 +++++++++--------
 opac/opac-detail.pl     | 11 ++++++-----
 opac/opac-export.pl     | 12 ++++++------
 t/Biblio.t              | 10 ++--------
 t/db_dependent/Biblio.t | 21 +++------------------
 8 files changed, 36 insertions(+), 81 deletions(-)

diff --git a/C4/Biblio.pm b/C4/Biblio.pm
index 063a07a765..2ab15c5aa0 100644
--- a/C4/Biblio.pm
+++ b/C4/Biblio.pm
@@ -30,7 +30,6 @@ BEGIN {
         AddBiblio
         GetBiblioData
         GetISBDView
-        GetMarcControlnumber
         GetMarcISBN
         GetMarcISSN
         GetMarcSubjects
@@ -1528,32 +1527,6 @@ sub GetAuthorisedValueDesc {
     }
 }
 
-=head2 GetMarcControlnumber
-
-  $marccontrolnumber = GetMarcControlnumber($record,$marcflavour);
-
-Get the control number / record Identifier from the MARC record and return it.
-
-=cut
-
-sub GetMarcControlnumber {
-    my ( $record, $marcflavour ) = @_;
-    if (!$record) {
-        carp 'GetMarcControlnumber called on undefined record';
-        return;
-    }
-    my $controlnumber = "";
-    # Control number or Record identifier are the same field in MARC21 and UNIMARC
-    # Keep $marcflavour for possible later use
-    if ($marcflavour eq "MARC21" || $marcflavour eq "UNIMARC" ) {
-        my $controlnumberField = $record->field('001');
-        if ($controlnumberField) {
-            $controlnumber = $controlnumberField->data();
-        }
-    }
-    return $controlnumber;
-}
-
 =head2 GetMarcISBN
 
   $marcisbnsarray = GetMarcISBN( $record, $marcflavour );
diff --git a/catalogue/export.pl b/catalogue/export.pl
index ff02cea67f..eb54b3683b 100755
--- a/catalogue/export.pl
+++ b/catalogue/export.pl
@@ -4,7 +4,6 @@ use Modern::Perl;
 use C4::Record;
 use C4::Auth qw( get_template_and_user );
 use C4::Output;
-use C4::Biblio qw( GetMarcControlnumber );
 use CGI qw ( -utf8 );
 use C4::Ris qw( marc2ris );
 
@@ -30,11 +29,12 @@ if ($op eq "export") {
             my $biblio = Koha::Biblios->find($biblionumber);
             my $marc   = $biblio->metadata->record({ embed_items => 1 });
 
-            if( C4::Context->preference('DefaultSaveRecordFileID') eq 'controlnumber' ){
-                my $marcflavour = C4::Context->preference('marcflavour'); #FIXME This option is required but does not change control num behaviour
-                my $control_num = GetMarcControlnumber( $marc, $marcflavour );
-                if( $control_num ){
-                    $file_id = $control_num;
+            my $metadata_extractor = $biblio->metadata_extractor;
+
+            if ( C4::Context->preference('DefaultSaveRecordFileID') eq 'controlnumber' ) {
+                my $control_num = $metadata_extractor->get_control_number();
+                if ($control_num) {
+                    $file_id  = $control_num;
                     $file_pre = "record-";
                 }
             }
diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl
index a70d2ccf76..1b863efeb3 100755
--- a/opac/opac-ISBDdetail.pl
+++ b/opac/opac-ISBDdetail.pl
@@ -48,7 +48,6 @@ use CGI qw ( -utf8 );
 use C4::Biblio qw(
     CountItemsIssued
     GetISBDView
-    GetMarcControlnumber
     GetMarcISSN
     TransformMarcToKoha
 );
@@ -102,6 +101,8 @@ unless ( $patron and $patron->category->override_hidden_items ) {
 my $record = $biblio->metadata->record;
 my @items  = $biblio->items->filter_by_visible_in_opac({ patron => $patron })->as_list;
 
+my $metadata_extractor = $biblio->metadata_extractor;
+
 my $record_processor = Koha::RecordProcessor->new(
     {   filters => [ 'EmbedItems', 'ViewPolicy' ],
         options => {
@@ -195,7 +196,7 @@ $template->param(
 );
 
 #Search for title in links
-my $marccontrolnumber   = GetMarcControlnumber ($record, $marcflavour);
+my $control_number = $metadata_extractor->get_control_number();
 my $marcissns = GetMarcISSN ( $record, $marcflavour );
 my $issn = $marcissns->[0] || '';
 
@@ -210,7 +211,7 @@ if (my $search_for_title = C4::Context->preference('OPACSearchForTitleIn')){
             AUTHOR        => $dat->{author},
             ISBN          => $isbn,
             ISSN          => $issn,
-            CONTROLNUMBER => $marccontrolnumber,
+            CONTROLNUMBER => $control_number,
             BIBLIONUMBER  => $biblionumber,
             OCLC_NO       => $oclc_no,
         }
diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl
index 406d2d4ceb..34ddd83877 100755
--- a/opac/opac-MARCdetail.pl
+++ b/opac/opac-MARCdetail.pl
@@ -52,7 +52,6 @@ use CGI qw ( -utf8 );
 use C4::Biblio qw(
     CountItemsIssued
     GetAuthorisedValueDesc
-    GetMarcControlnumber
     GetMarcFromKohaField
     GetMarcISSN
     GetMarcStructure
@@ -108,6 +107,8 @@ unless ( $patron and $patron->category->override_hidden_items ) {
     }
 }
 
+my $metadata_extractor = $biblio->metadata_extractor;
+
 my $items = $biblio->items->filter_by_visible_in_opac({ patron => $patron });
 my $framework = $biblio ? $biblio->frameworkcode : q{};
 my $tagslib   = &GetMarcStructure( 0, $framework );
@@ -339,12 +340,12 @@ if ( C4::Context->preference("OPACISBD") ) {
 }
 
 #Search for title in links
-my $marcflavour  = C4::Context->preference("marcflavour");
-my $dat = TransformMarcToKoha({ record => $record });
-my $isbn = GetNormalizedISBN(undef,$record,$marcflavour);
-my $marccontrolnumber   = GetMarcControlnumber ($record, $marcflavour);
-my $marcissns = GetMarcISSN( $record, $marcflavour );
-my $issn = $marcissns->[0] || '';
+my $marcflavour    = C4::Context->preference("marcflavour");
+my $dat            = TransformMarcToKoha( { record => $record } );
+my $isbn           = GetNormalizedISBN( undef, $record, $marcflavour );
+my $control_number = $metadata_extractor->get_control_number();
+my $marcissns      = GetMarcISSN( $record, $marcflavour );
+my $issn           = $marcissns->[0] || '';
 
 if (my $search_for_title = C4::Context->preference('OPACSearchForTitleIn')){
     $dat->{title} =~ s/\/+$//; # remove trailing slash
@@ -357,7 +358,7 @@ if (my $search_for_title = C4::Context->preference('OPACSearchForTitleIn')){
             AUTHOR        => $dat->{author},
             ISBN          => $isbn,
             ISSN          => $issn,
-            CONTROLNUMBER => $marccontrolnumber,
+            CONTROLNUMBER => $control_number,
             BIBLIONUMBER  => $biblionumber,
             OCLC_NO       => $oclc_no,
         }
diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
index 59a7d5ec57..e9f188da2b 100755
--- a/opac/opac-detail.pl
+++ b/opac/opac-detail.pl
@@ -38,7 +38,6 @@ use C4::Output qw( parametrized_url output_html_with_http_headers );
 use C4::Biblio qw(
     CountItemsIssued
     GetBiblioData
-    GetMarcControlnumber
     GetMarcISBN
     GetMarcISSN
     GetMarcSeries
@@ -112,6 +111,8 @@ unless ( $biblio && $record ) {
     exit;
 }
 
+my $metadata_extractor = $biblio->metadata_extractor;
+
 my $items = $biblio->items->search_ordered;
 if ($specific_item) {
     $items = $items->search( { itemnumber => scalar $query->param('itemnumber') } );
@@ -1166,9 +1167,9 @@ if ( C4::Context->preference('OpacStarRatings') !~ /disable/ ) {
 }
 
 #Search for title in links
-my $marccontrolnumber   = GetMarcControlnumber ($record, $marcflavour);
-my $marcissns = GetMarcISSN ( $record, $marcflavour );
-my $issn = $marcissns->[0] || '';
+my $control_number = $metadata_extractor->get_control_number();
+my $marcissns      = GetMarcISSN( $record, $marcflavour );
+my $issn           = $marcissns->[0] || '';
 
 if (my $search_for_title = C4::Context->preference('OPACSearchForTitleIn')){
     $dat->{title} =~ s/\/+$//; # remove trailing slash
@@ -1181,7 +1182,7 @@ if (my $search_for_title = C4::Context->preference('OPACSearchForTitleIn')){
             AUTHOR        => $dat->{author},
             ISBN          => $isbn,
             ISSN          => $issn,
-            CONTROLNUMBER => $marccontrolnumber,
+            CONTROLNUMBER => $control_number,
             BIBLIONUMBER  => $biblionumber,
             OCLC_NO       => $oclc_no,
         }
diff --git a/opac/opac-export.pl b/opac/opac-export.pl
index a39c804695..f5c363fc20 100755
--- a/opac/opac-export.pl
+++ b/opac/opac-export.pl
@@ -25,7 +25,6 @@ use C4::Output;
 use C4::Biblio qw(
     GetFrameworkCode
     GetISBDView
-    GetMarcControlnumber
 );
 use CGI qw ( -utf8 );
 use C4::Auth;
@@ -85,13 +84,14 @@ if(!$marc) {
     exit;
 }
 
+my $metadata_extractor = $biblio->metadata_extractor;
+
 my $file_id = $biblionumber;
 my $file_pre = "bib-";
-if( C4::Context->preference('DefaultSaveRecordFileID') eq 'controlnumber' ){
-    my $marcflavour = C4::Context->preference('marcflavour'); #FIXME This option is required but does not change control num behaviour
-    my $control_num = GetMarcControlnumber( $marc, $marcflavour );
-    if( $control_num ){
-        $file_id = $control_num;
+if ( C4::Context->preference('DefaultSaveRecordFileID') eq 'controlnumber' ) {
+    my $control_number = $metadata_extractor->get_control_number();
+    if ($control_number) {
+        $file_id  = $control_number;
         $file_pre = "record-";
     }
 }
diff --git a/t/Biblio.t b/t/Biblio.t
index c298349e18..3fbf90402a 100755
--- a/t/Biblio.t
+++ b/t/Biblio.t
@@ -21,10 +21,10 @@ use Test::More;
 use Test::MockModule;
 use Test::Warn;
 
-plan tests => 34;
+plan tests => 32;
 
 
-use_ok('C4::Biblio', qw( AddBiblio ModBiblio BiblioAutoLink LinkBibHeadingsToAuthorities GetMarcPrice GetMarcQuantity GetMarcControlnumber GetMarcISBN GetMarcISSN GetMarcSubjects GetMarcUrls GetMarcSeries TransformMarcToKoha ModBiblioMarc RemoveAllNsb UpdateTotalIssues ));
+use_ok('C4::Biblio', qw( AddBiblio ModBiblio BiblioAutoLink LinkBibHeadingsToAuthorities GetMarcPrice GetMarcQuantity GetMarcISBN GetMarcISSN GetMarcSubjects GetMarcUrls GetMarcSeries TransformMarcToKoha ModBiblioMarc RemoveAllNsb UpdateTotalIssues ));
 
 my $db = Test::MockModule->new('Koha::Database');
 $db->mock( _new_schema => sub { return Schema(); } );
@@ -70,12 +70,6 @@ warning_is { $ret = GetMarcQuantity(undef, 'MARC21') }
 
 ok( !defined $ret, 'GetMarcQuantity returns undef if not passed rec');
 
-warning_is { $ret = GetMarcControlnumber() }
-           { carped => 'GetMarcControlnumber called on undefined record'},
-           "GetMarcControlnumber returns carped warning on undef record";
-
-ok( !defined $ret, 'GetMarcControlnumber returns undef if not passed rec');
-
 warning_is { $ret = GetMarcISBN() }
            { carped => 'GetMarcISBN called on undefined record'},
            "GetMarcISBN returns carped warning on undef record";
diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t
index ef8eb0ca54..9f470409d2 100755
--- a/t/db_dependent/Biblio.t
+++ b/t/db_dependent/Biblio.t
@@ -34,7 +34,7 @@ use Koha::MarcSubfieldStructures;
 use C4::Linker::Default qw( get_link );
 
 BEGIN {
-    use_ok('C4::Biblio', qw( AddBiblio GetMarcFromKohaField BiblioAutoLink GetMarcSubfieldStructure GetMarcSubfieldStructureFromKohaField LinkBibHeadingsToAuthorities GetBiblioData ModBiblio GetMarcISSN GetMarcControlnumber GetMarcISBN GetMarcPrice GetFrameworkCode GetMarcUrls IsMarcStructureInternal GetMarcStructure GetXmlBiblio DelBiblio ));
+    use_ok('C4::Biblio', qw( AddBiblio GetMarcFromKohaField BiblioAutoLink GetMarcSubfieldStructure GetMarcSubfieldStructureFromKohaField LinkBibHeadingsToAuthorities GetBiblioData ModBiblio GetMarcISSN GetMarcISBN GetMarcPrice GetFrameworkCode GetMarcUrls IsMarcStructureInternal GetMarcStructure GetXmlBiblio DelBiblio ));
 }
 
 my $schema = Koha::Database->new->schema;
@@ -417,21 +417,6 @@ sub run_tests {
     is( scalar @$issns, 4,
         'GetMARCISSN skips empty ISSN fields (Bug 12674)');
 
-    ## Testing GetMarcControlnumber
-    my $controlnumber;
-    $controlnumber = GetMarcControlnumber( $marc_record, $marcflavour );
-    is( $controlnumber, '', 'GetMarcControlnumber handles records without 001' );
-
-    $field = MARC::Field->new( '001', '' );
-    $marc_record->append_fields($field);
-    $controlnumber = GetMarcControlnumber( $marc_record, $marcflavour );
-    is( $controlnumber, '', 'GetMarcControlnumber handles records with empty 001' );
-
-    $field = $marc_record->field('001');
-    $field->update('123456789X');
-    $controlnumber = GetMarcControlnumber( $marc_record, $marcflavour );
-    is( $controlnumber, '123456789X', 'GetMarcControlnumber handles records with 001' );
-
     ## Testing GetMarcISBN
     my $record_for_isbn = MARC::Record->new();
     my $isbns = GetMarcISBN( $record_for_isbn, $marcflavour );
@@ -632,14 +617,14 @@ sub create_author_field {
 }
 
 subtest 'MARC21' => sub {
-    plan tests => 46;
+    plan tests => 43;
     run_tests('MARC21');
     $schema->storage->txn_rollback;
     $schema->storage->txn_begin;
 };
 
 subtest 'UNIMARC' => sub {
-    plan tests => 46;
+    plan tests => 43;
 
     # Mock the auth type data for UNIMARC
     $dbh->do("UPDATE auth_types SET auth_tag_to_report = '106' WHERE auth_tag_to_report = '100'") or die $dbh->errstr;
-- 
2.39.5