From ce347f778433077bd711f6582688164a9f1ed6fd Mon Sep 17 00:00:00 2001 From: Baptiste Wojtkowski Date: Thu, 10 Apr 2025 16:10:48 +0200 Subject: [PATCH] Bug 39730: Remove DelSuggestion from C4/Suggestions.pm To test: Delete a suggestion from the opac and from the intranet. --- C4/Suggestions.pm | 39 ------------------------------------ Koha/Suggestion.pm | 14 +++++++++++++ opac/opac-suggestions.pl | 8 ++++++-- suggestion/suggestion.pl | 2 +- t/db_dependent/Suggestions.t | 15 ++------------ 5 files changed, 23 insertions(+), 55 deletions(-) diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm index cdd8d285..d7454cbd 100644 --- a/C4/Suggestions.pm +++ b/C4/Suggestions.pm @@ -32,7 +32,6 @@ use C4::Log qw(logaction); use base qw(Exporter); our @EXPORT = qw( - DelSuggestion ModStatus ModSuggestion MarcRecordFromNewSuggestion @@ -134,44 +133,6 @@ sub ModSuggestion { return 1; # No useful if the exception is raised earlier } -=head2 DelSuggestion - -&DelSuggestion($borrowernumber,$ordernumber) - -Delete a suggestion. A borrower can delete a suggestion only if they are its owner. - -=cut - -sub DelSuggestion { - my ( $borrowernumber, $suggestionid, $type ) = @_; - my $dbh = C4::Context->dbh; - - # check that the suggestion comes from the suggestor - my $query = q{ - SELECT suggestedby - FROM suggestions - WHERE suggestionid=? - }; - my $sth = $dbh->prepare($query); - $sth->execute($suggestionid); - my ($suggestedby) = $sth->fetchrow; - $suggestedby //= ''; - $borrowernumber //= ''; - - if ( defined $type && $type eq 'intranet' || $suggestedby eq $borrowernumber ) { - my $queryDelete = q{ - DELETE FROM suggestions - WHERE suggestionid=? - }; - $sth = $dbh->prepare($queryDelete); - my $suggestiondeleted = $sth->execute($suggestionid); - if ( C4::Context->preference("SuggestionsLog") ) { - logaction( 'SUGGESTION', 'DELETE', $suggestionid, '' ); - } - return $suggestiondeleted; - } -} - =head2 MarcRecordFromNewSuggestion $record = MarcRecordFromNewSuggestion ( $suggestion ) diff --git a/Koha/Suggestion.pm b/Koha/Suggestion.pm index 8a098189..9430707e 100644 --- a/Koha/Suggestion.pm +++ b/Koha/Suggestion.pm @@ -123,6 +123,20 @@ sub store { return $result; } +=head3 delete + +Overloads the default behavior so that it becomes logged + +=cut + +sub delete { + my ($self) = @_; + if ( C4::Context->preference("SuggestionsLog") ) { + logaction( 'SUGGESTION', 'DELETE', $self->suggestionid, '' ); + } + $self->SUPER::delete(); +} + =head3 library my $library = $suggestion->library; diff --git a/opac/opac-suggestions.pl b/opac/opac-suggestions.pl index 775affbe..4f1f9473 100755 --- a/opac/opac-suggestions.pl +++ b/opac/opac-suggestions.pl @@ -24,7 +24,6 @@ use C4::Members; use C4::Koha qw( GetAuthorisedValues ); use C4::Output qw( output_html_with_http_headers ); use C4::Suggestions qw( - DelSuggestion MarcRecordFromNewSuggestion ); use C4::Koha qw( GetAuthorisedValues ); @@ -251,7 +250,12 @@ my $suggestions = [ if ( $op eq "cud-delete" ) { my @delete_field = $input->multi_param("delete_field"); foreach my $delete_field (@delete_field) { - &DelSuggestion( $borrowernumber, $delete_field ); + Koha::Suggestions->search( + { + suggestedby => $borrowernumber, + suggestionid => $delete_field + } + )->single->delete; } $op = 'else'; print $input->redirect("/cgi-bin/koha/opac-suggestions.pl?op=else"); diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl index 84d5dcf6..1aedcedb 100755 --- a/suggestion/suggestion.pl +++ b/suggestion/suggestion.pl @@ -347,7 +347,7 @@ if ( $op =~ /cud-save/ ) { } elsif ( $op eq "cud-delete" ) { if ( $librarian->has_permission( { 'suggestions' => 'suggestions_delete' } ) ) { foreach my $delete_field (@editsuggestions) { - &DelSuggestion( $borrowernumber, $delete_field, 'intranet' ); + Koha::Suggestions->find( { suggestionid => $delete_field } )->delete(); } redirect_with_params($input); } else { diff --git a/t/db_dependent/Suggestions.t b/t/db_dependent/Suggestions.t index 03d6f168..5f16132a 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 => 50; +use Test::More tests => 44; use Test::Warn; use t::lib::Mocks; @@ -38,7 +38,7 @@ use Koha::Suggestions; BEGIN { use_ok( 'C4::Suggestions', - qw( ModSuggestion DelSuggestion MarcRecordFromNewSuggestion ) + qw( ModSuggestion MarcRecordFromNewSuggestion ) ); } @@ -320,18 +320,7 @@ my $del_suggestion = { my $del_suggestion_object = Koha::Suggestion->new($del_suggestion)->store(); my $del_suggestionid = $del_suggestion_object->id; -is( DelSuggestion(), '0E0', 'DelSuggestion without arguments returns 0E0' ); -is( DelSuggestion($borrowernumber), '', 'DelSuggestion without the suggestion id returns an empty string' ); -is( - DelSuggestion( undef, $my_suggestionid ), '', - 'DelSuggestion with an invalid borrower number returns an empty string' -); -$suggestion = DelSuggestion( $borrowernumber, $my_suggestionid ); -is( $suggestion, 1, 'DelSuggestion deletes one suggestion' ); - my $suggestions = Koha::Suggestions->search( { STATUS => 'CHECKED' } )->as_list; -is( @$suggestions, 2, 'DelSuggestion deletes one suggestion' ); -is( $suggestions->[1]->unblessed()->{title}, $del_suggestion->{title}, 'DelSuggestion deletes the correct suggestion' ); # Test budgetid fk $my_suggestion->{budgetid} = ''; # If budgetid == '', NULL should be set in DB -- 2.30.2