Bugzilla – Attachment 195128 Details for
Bug 39732
Remove MarcRecordFromNewSuggestion from C4/Suggestions.pm
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 39732: Remove MarcRecordFromNewSuggestion from C4/Suggestions.pm
Bug-39732-Remove-MarcRecordFromNewSuggestion-from-.patch (text/plain), 11.98 KB, created by
Baptiste Wojtkowski (bwoj)
on 2026-03-11 15:04:04 UTC
(
hide
)
Description:
Bug 39732: Remove MarcRecordFromNewSuggestion from C4/Suggestions.pm
Filename:
MIME Type:
Creator:
Baptiste Wojtkowski (bwoj)
Created:
2026-03-11 15:04:04 UTC
Size:
11.98 KB
patch
obsolete
>From eddc2e7ae4a20d345834ff019c1b072c61af869e Mon Sep 17 00:00:00 2001 >From: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> >Date: Wed, 11 Mar 2026 15:01:33 +0000 >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 extract 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 > >Test plan: >1 - Find the ISBN of a book in the database >2 - Make a suggestion with this isbn through the intranet >3 - Make a suggestion with this isbn through the OPAC >--- > C4/Search.pm | 54 +++++++++++++++++++++++------------- > C4/Suggestions.pm | 41 --------------------------- > opac/opac-suggestions.pl | 8 ++---- > suggestion/suggestion.pl | 21 +++++++------- > t/db_dependent/Suggestions.t | 50 ++------------------------------- > 5 files changed, 48 insertions(+), 126 deletions(-) > >diff --git a/C4/Search.pm b/C4/Search.pm >index de8162cb721..7f7762ae7dc 100644 >--- a/C4/Search.pm >+++ b/C4/Search.pm >@@ -21,6 +21,7 @@ use base 'Exporter'; > BEGIN { > our @EXPORT_OK = qw( > FindDuplicate >+ FindBiblioFromMetadata > SimpleSearch > searchResults > getRecords >@@ -90,35 +91,48 @@ sub FindDuplicate { > my ($record) = @_; > my $dbh = C4::Context->dbh; > my $result = TransformMarcToKoha( { record => $record } ); >+ return FindBiblioFromMetadata($result); >+} >+ >+=head2 FindBiblioFromMetadata >+ >+($biblionumber, $title) = FindBiblioFromMetadata($data); >+ >+This function attempts to find records matching data using a hard-coded, fairly simplistic algorithm >+ >+=cut >+ >+sub FindBiblioFromMetadata { >+ my $metadata = shift; > my $sth; > 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 ( $metadata->{isbn} ) { >+ $metadata->{isbn} =~ s/\(.*$//; >+ $metadata->{isbn} =~ s/\s+$//; >+ $metadata->{isbn} =~ s/\|/OR/; >+ $query = "isbn:$metadata->{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; >+ $metadata->{title} =~ s /\\//g; >+ $metadata->{title} =~ s /\"//g; >+ $metadata->{title} =~ s /\(//g; >+ $metadata->{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:\"$metadata->{title}\""; >+ if ( $metadata->{author} ) { >+ $metadata->{author} =~ s /\\//g; >+ $metadata->{author} =~ s /\"//g; >+ $metadata->{author} =~ s /\(//g; >+ $metadata->{author} =~ s /\)//g; > >- $query .= " $op $authorindex:\"$result->{author}\""; >+ $query .= " $op $authorindex:\"$metadata->{author}\""; > } > } > >@@ -132,12 +146,12 @@ sub FindDuplicate { > $possible_duplicate_record > ); > >- my $result = TransformMarcToKoha( { record => $marcrecord } ); >+ my $metadata = TransformMarcToKoha( { record => $marcrecord } ); > > # FIXME :: why 2 $biblionumber ? >- if ($result) { >- push @results, $result->{'biblionumber'}; >- push @results, $result->{'title'}; >+ if ($metadata) { >+ push @results, $metadata->{'biblionumber'}; >+ push @results, $metadata->{'title'}; > } > } > } >diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm >index f7b8e3f359d..9428fc4880f 100644 >--- a/C4/Suggestions.pm >+++ b/C4/Suggestions.pm >@@ -26,7 +26,6 @@ BEGIN { > GetSuggestion > ModStatus > ModSuggestion >- MarcRecordFromNewSuggestion > ); > } > >@@ -136,46 +135,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 ae1c6f022ce..4fb17b3ecd7 100755 >--- a/opac/opac-suggestions.pl >+++ b/opac/opac-suggestions.pl >@@ -23,12 +23,9 @@ 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::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 06ab5c9448d..cab8d11e700 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; >@@ -144,14 +144,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' } ) ) { >@@ -160,15 +158,16 @@ if ( $op =~ /cud-save/ ) { > messages => \@messages, > ); > } elsif ( !$suggestion_only->{suggestionid} >- && ( my ( $duplicatebiblionumber, $duplicatetitle ) = FindDuplicate($biblio) ) >+ && ( my ( $duplicatebiblionumber, $duplicatetitle ) = FindBiblioFromMetadata($metadata) ) > && !$save_confirmed ) > { > push @messages, > { type => 'error', code => 'biblio_exists', id => $duplicatebiblionumber, title => $duplicatetitle }; > $template->param( > messages => \@messages, >- need_confirm => 1 >+ need_confirm => 1, > ); >+ $stored_suggestion = $suggestion_only; > $op = 'save'; > } else { > >diff --git a/t/db_dependent/Suggestions.t b/t/db_dependent/Suggestions.t >index ac1e8470138..bfa3e1e710e 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 ) > ); > } > >@@ -328,24 +328,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; > >@@ -541,32 +523,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.43.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 39732
:
181412
|
181425
| 195128