From c741d8314a2a5d56e0bd54c8258c3120810a3a18 Mon Sep 17 00:00:00 2001 From: Baptiste Wojtkowski Date: Thu, 10 Apr 2025 18:06:29 +0200 Subject: [PATCH] Bug 39732: Remove MarcRecordFromNewSuggestion from C4/Suggestions.pm Note: This function was necessary for a very weird purpose. The whole process was: 1 - Have a suggestion as an hashref 2 - Convert it into a record 3 - Pass it to FindDuplicate 4 - C4/search exctract the exact same hashref as in 1 and performs the search Hence we extracted from FindDuplicate the part that uses the hashref and called it directly --- C4/Search.pm | 52 ++++++++++++++++++++++-------------- C4/Suggestions.pm | 41 ---------------------------- opac/opac-suggestions.pl | 14 ++++------ suggestion/suggestion.pl | 18 ++++++------- t/db_dependent/Suggestions.t | 50 ++-------------------------------- 5 files changed, 47 insertions(+), 128 deletions(-) diff --git a/C4/Search.pm b/C4/Search.pm index 6ae9ddcc..4631ec65 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -48,6 +48,7 @@ BEGIN { @ISA = qw(Exporter); @EXPORT_OK = qw( FindDuplicate + FindBiblioFromMetadata SimpleSearch searchResults getRecords @@ -80,7 +81,7 @@ This module provides searching functions for Koha's bibliographic databases =head2 FindDuplicate -($biblionumber,$biblionumber,$title) = FindDuplicate($record); +($biblionumber,$title) = FindDuplicate($record); This function attempts to find duplicate records using a hard-coded, fairly simplistic algorithm @@ -88,37 +89,48 @@ This function attempts to find duplicate records using a hard-coded, fairly simp sub FindDuplicate { my ($record) = @_; - my $dbh = C4::Context->dbh; - my $result = TransformMarcToKoha( { record => $record } ); - my $sth; + my $result = TransformMarcToKoha( { record => $record } ); + return FindBiblioFromMetadata($result); +} + +=head2 FindBiblioFromMetadata + +($biblionumber, $title) = FindBiblioFromData($data); + +This function attempts to find records matching data using a hard-coded, fairly simplistic algorithm + +=cut + +sub FindBiblioFromMetadata { + my $data = shift; my $query; # search duplicate on ISBN, easy and fast.. # ... normalize first - if ( $result->{isbn} ) { - $result->{isbn} =~ s/\(.*$//; - $result->{isbn} =~ s/\s+$//; - $result->{isbn} =~ s/\|/OR/; - $query = "isbn:$result->{isbn}"; + if ( $data->{isbn} ) { + $data->{isbn} =~ s/\(.*$//; + $data->{isbn} =~ s/\s+$//; + $data->{isbn} =~ s/\|/OR/; + $query = "isbn:$data->{isbn}"; } else { my $titleindex = 'ti,ext'; my $authorindex = 'au,ext'; my $op = 'AND'; - $result->{title} =~ s /\\//g; - $result->{title} =~ s /\"//g; - $result->{title} =~ s /\(//g; - $result->{title} =~ s /\)//g; + $data->{title} =~ s /\\//g; + $data->{title} =~ s /\"//g; + $data->{title} =~ s /\(//g; + $data->{title} =~ s /\)//g; - $query = "$titleindex:\"$result->{title}\""; - if ( $result->{author} ) { - $result->{author} =~ s /\\//g; - $result->{author} =~ s /\"//g; - $result->{author} =~ s /\(//g; - $result->{author} =~ s /\)//g; + $query = "$titleindex:\"$data->{title}\""; + if ( $data->{author} ) { + $data->{author} =~ s /\\//g; + $data->{author} =~ s /\"//g; + $data->{author} =~ s /\(//g; + $data->{author} =~ s /\)//g; - $query .= " $op $authorindex:\"$result->{author}\""; + $query .= " $op $authorindex:\"$data->{author}\""; } } diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm index d7454cbd..06f72379 100644 --- a/C4/Suggestions.pm +++ b/C4/Suggestions.pm @@ -34,7 +34,6 @@ use base qw(Exporter); our @EXPORT = qw( ModStatus ModSuggestion - MarcRecordFromNewSuggestion ); =head1 NAME @@ -133,46 +132,6 @@ sub ModSuggestion { return 1; # No useful if the exception is raised earlier } -=head2 MarcRecordFromNewSuggestion - - $record = MarcRecordFromNewSuggestion ( $suggestion ) - -This function build a marc record object from a suggestion - -=cut - -sub MarcRecordFromNewSuggestion { - my ($suggestion) = @_; - my $record = MARC::Record->new(); - - if ( my $isbn = $suggestion->{isbn} ) { - for my $field (qw(biblioitems.isbn biblioitems.issn)) { - my ( $tag, $subfield ) = GetMarcFromKohaField($field); - $record->append_fields( MARC::Field->new( $tag, ' ', ' ', $subfield => $isbn ) ); - } - } else { - my ( $title_tag, $title_subfield ) = GetMarcFromKohaField('biblio.title'); - $record->append_fields( MARC::Field->new( $title_tag, ' ', ' ', $title_subfield => $suggestion->{title} ) ); - - my ( $author_tag, $author_subfield ) = GetMarcFromKohaField('biblio.author'); - if ( $record->field($author_tag) ) { - $record->field($author_tag)->add_subfields( $author_subfield => $suggestion->{author} ); - } else { - $record->append_fields( - MARC::Field->new( $author_tag, ' ', ' ', $author_subfield => $suggestion->{author} ) ); - } - } - - my ( $it_tag, $it_subfield ) = GetMarcFromKohaField('biblioitems.itemtype'); - if ( $record->field($it_tag) ) { - $record->field($it_tag)->add_subfields( $it_subfield => $suggestion->{itemtype} ); - } else { - $record->append_fields( MARC::Field->new( $it_tag, ' ', ' ', $it_subfield => $suggestion->{itemtype} ) ); - } - - return $record; -} - 1; __END__ diff --git a/opac/opac-suggestions.pl b/opac/opac-suggestions.pl index 4f1f9473..15c9ccb7 100755 --- a/opac/opac-suggestions.pl +++ b/opac/opac-suggestions.pl @@ -21,14 +21,11 @@ use CGI qw ( -utf8 ); use Encode; use C4::Auth qw( get_template_and_user ); use C4::Members; -use C4::Koha qw( GetAuthorisedValues ); -use C4::Output qw( output_html_with_http_headers ); -use C4::Suggestions qw( - MarcRecordFromNewSuggestion -); -use C4::Koha qw( GetAuthorisedValues ); +use C4::Koha qw( GetAuthorisedValues ); +use C4::Output qw( output_html_with_http_headers ); +use C4::Koha qw( GetAuthorisedValues ); use C4::Scrubber; -use C4::Search qw( FindDuplicate ); +use C4::Search qw( FindBiblioFromMetadata ); use Koha::AuthorisedValues; use Koha::Libraries; @@ -145,8 +142,7 @@ $suggestion->{suggestedby} = $borrowernumber; if ( $op eq "cud-add_validate" && not $biblionumber ) { # If we are creating the suggestion from an existing record we do not want to search for duplicates $op = 'cud-add_confirm'; - my $biblio = MarcRecordFromNewSuggestion($suggestion); - if ( my ( $duplicatebiblionumber, $duplicatetitle ) = FindDuplicate($biblio) ) { + if ( my ( $duplicatebiblionumber, $duplicatetitle ) = FindBiblioFromMetadata($suggestion) ) { push @messages, { type => 'error', code => 'biblio_exists', id => $duplicatebiblionumber, title => $duplicatetitle }; $need_confirm = 1; diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl index 1aedcedb..46cdbe49 100755 --- a/suggestion/suggestion.pl +++ b/suggestion/suggestion.pl @@ -25,7 +25,7 @@ use C4::Output qw( output_html_with_http_headers output_and_exit_if_error ); use C4::Suggestions; use C4::Koha qw( GetAuthorisedValues ); use C4::Budgets qw( GetBudget GetBudgets GetBudgetHierarchy CanUserUseBudget ); -use C4::Search qw( FindDuplicate GetDistinctValues ); +use C4::Search qw( FindBiblioFromMetadata GetDistinctValues ); use C4::Members; use Koha::DateUtils qw( dt_from_string ); use Koha::AuthorisedValues; @@ -156,14 +156,12 @@ if ( $op =~ /cud-save/ ) { my @messages; - my $biblio = MarcRecordFromNewSuggestion( - { - title => $suggestion_only->{title}, - author => $suggestion_only->{author}, - itemtype => $suggestion_only->{itemtype}, - isbn => $suggestion_only->{isbn}, - } - ); + my $metadata = { + title => $suggestion_only->{title}, + author => $suggestion_only->{author}, + itemtype => $suggestion_only->{itemtype}, + isbn => $suggestion_only->{isbn}, + }; my $manager = Koha::Patrons->find( $suggestion_only->{managedby} ); if ( $manager && not $manager->has_permission( { suggestions => 'suggestions_manage' } ) ) { @@ -176,7 +174,7 @@ if ( $op =~ /cud-save/ ) { delete $suggestion_ref->{managedby}; Init($suggestion_ref); } elsif ( !$suggestion_only->{suggestionid} - && ( my ( $duplicatebiblionumber, $duplicatetitle ) = FindDuplicate($biblio) ) + && ( my ( $duplicatebiblionumber, $duplicatetitle ) = FindBiblioFromMetadata($metadata) ) && !$save_confirmed ) { push @messages, diff --git a/t/db_dependent/Suggestions.t b/t/db_dependent/Suggestions.t index 5f16132a..e3951c45 100755 --- a/t/db_dependent/Suggestions.t +++ b/t/db_dependent/Suggestions.t @@ -19,7 +19,7 @@ use Modern::Perl; use DateTime::Duration; use Test::NoWarnings; -use Test::More tests => 44; +use Test::More tests => 40; use Test::Warn; use t::lib::Mocks; @@ -38,7 +38,7 @@ use Koha::Suggestions; BEGIN { use_ok( 'C4::Suggestions', - qw( ModSuggestion MarcRecordFromNewSuggestion ) + qw( ModSuggestion ) ); } @@ -340,24 +340,6 @@ my $suggestion2 = { itemtype => "LIV" }; -my $record = MarcRecordFromNewSuggestion($suggestion2); - -is( "MARC::Record", ref($record), "MarcRecordFromNewSuggestion should return a MARC::Record object" ); - -my ( $title_tag, $title_subfield ) = C4::Biblio::GetMarcFromKohaField('biblio.title'); - -is( - $record->field($title_tag)->subfield($title_subfield), "Cuisine d'automne", - "Record from suggestion title should be 'Cuisine d'automne'" -); - -my ( $author_tag, $author_subfield ) = C4::Biblio::GetMarcFromKohaField('biblio.author'); - -is( - $record->field($author_tag)->subfield($author_subfield), "Catherine", - "Record from suggestion author should be 'Catherine'" -); - subtest 'EmailPurchaseSuggestions' => sub { plan tests => 11; @@ -553,32 +535,4 @@ subtest 'place_hold tests' => sub { }; -subtest 'Suggestion with ISBN' => sub { - my $suggestion_with_isbn = { - isbn => '1940997232', - title => "The Clouds", - author => "Aristophanes", - }; - my $record = MarcRecordFromNewSuggestion($suggestion_with_isbn); - is( "MARC::Record", ref($record), "MarcRecordFromNewSuggestion should return a MARC::Record object" ); - - my ( $isbn_tag, $isbn_subfield ) = C4::Biblio::GetMarcFromKohaField('biblioitems.isbn'); - is( - $record->field($isbn_tag)->subfield($isbn_subfield), "1940997232", - "ISBN Record from suggestion ISBN should be '1940997232'" - ); - - my ( $issn_tag, $issn_subfield ) = C4::Biblio::GetMarcFromKohaField('biblioitems.issn'); - is( - $record->field($issn_tag)->subfield($issn_subfield), "1940997232", - "ISSN Record from suggestion ISBN should also be '1940997232'" - ); - - my ( $title_tag, $title_subfield ) = C4::Biblio::GetMarcFromKohaField('biblio.title'); - is( $record->field($title_tag), undef, "Record from suggestion title should be empty" ); - - my ( $author_tag, $author_subfield ) = C4::Biblio::GetMarcFromKohaField('biblio.author'); - is( $record->field($author_tag), undef, "Record from suggestion author should be empty" ); -}; - $schema->storage->txn_rollback; -- 2.30.2