From e768f682c8a33aecba40e669c6fe329737bbad81 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 7 Nov 2019 17:41:02 +0100 Subject: [PATCH] Bug 23991: Move SearchSuggestion to Koha::Suggestions --- Koha/Suggestion.pm | 36 ++++++++++ acqui/newordersuggestion.pl | 18 ++--- .../prog/en/modules/acqui/newordersuggestion.tt | 50 ++++++------- .../en/modules/members/purchase-suggestions.tt | 10 +-- .../bootstrap/en/modules/opac-suggestions.tt | 62 ++++++++-------- members/purchase-suggestions.pl | 5 +- opac/opac-suggestions.pl | 82 ++++++++++------------ 7 files changed, 138 insertions(+), 125 deletions(-) diff --git a/Koha/Suggestion.pm b/Koha/Suggestion.pm index 2fef425197..cf47960382 100644 --- a/Koha/Suggestion.pm +++ b/Koha/Suggestion.pm @@ -53,6 +53,42 @@ sub store { return $self->SUPER::store(); } +sub manager { + my ($self) = @_; + my $manager_rs = $self->_result->managedby; + return unless $manager_rs; + return Koha::Patron->_new_from_dbic($manager_rs); +} + +sub suggester { + my ($self) = @_; + my $suggester_rs = $self->_result->managedby; + return unless $suggester_rs; + return Koha::Patron->_new_from_dbic($suggester_rs); +} + +sub rejecter { + my ($self) = @_; + my $rejecter_rs = $self->_result->managedby; + return unless $rejecter_rs; + return Koha::Patron->_new_from_dbic($rejecter_rs); +} + +sub last_modifier { + my ($self) = @_; + my $last_modifier_rs = $self->_result->managedby; + return unless $last_modifier_rs; + return Koha::Patron->_new_from_dbic($last_modifier_rs); +} + +sub fund { + my ($self) = @_; + my $fund_rs = $self->_result->budgetid; + return unless $fund_rs; + return Koha::Acquisition::Fund->_new_from_dbic($fund_rs); +} + + =head3 type =cut diff --git a/acqui/newordersuggestion.pl b/acqui/newordersuggestion.pl index 54934967ab..9220e5c55a 100755 --- a/acqui/newordersuggestion.pl +++ b/acqui/newordersuggestion.pl @@ -98,6 +98,7 @@ use C4::Biblio; use C4::Budgets; use Koha::Acquisition::Booksellers; +use Koha::Suggestions; my $input = new CGI; @@ -128,23 +129,22 @@ if ( $op eq 'connectDuplicate' ) { ConnectSuggestionAndBiblio( $suggestionid, $duplicateNumber ); } -# getting all suggestions. -my $suggestions_loop = SearchSuggestion( +my $suggestions = Koha::Suggestions->search( { - author => $author, - title => $title, - publishercode => $publishercode, - STATUS => 'ACCEPTED' - } + ( $author ? ( author => $author ) : () ), + ( $title ? ( title => $title ) : () ), + ( $publishercode ? ( publishercode => $publishercode ) : () ), + STATUS => 'ACCEPTED' + }, + { prefetch => ['managedby', 'suggestedby'] }, ); my $vendor = Koha::Acquisition::Booksellers->find( $booksellerid ); $template->param( - suggestions_loop => $suggestions_loop, + suggestions => $suggestions, basketno => $basketno, booksellerid => $booksellerid, name => $vendor->name, - loggedinuser => $borrowernumber, "op_$op" => 1, ); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/newordersuggestion.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/newordersuggestion.tt index cad0dd26e0..145c27f30a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/newordersuggestion.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/newordersuggestion.tt @@ -20,7 +20,7 @@

Suggestions

- [% IF ( suggestions_loop ) %] + [% IF suggestions.count %] Show only mine | Show all suggestions @@ -38,49 +38,45 @@ - [% FOREACH suggestions_loo IN suggestions_loop %] + [% FOREACH suggestion IN suggestions %] - + + + - - @@ -114,7 +110,7 @@ })); $("#show_only_mine").on('click', function(e){ e.preventDefault(); - suggestionst.fnFilter('^[% loggedinuser | html %]$', 0, true); + suggestionst.fnFilter('^[% logged_in_user.borrowernumber | html %]$', 0, true); }); $("#show_all").on('click', function(e){ e.preventDefault(); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt index dc8afa0283..7fb653dc91 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt @@ -31,7 +31,7 @@ New purchase suggestion - [% IF suggestions %] + [% IF suggestions.count %]
[% suggestions_loo.managedby | html %][% suggestion.managedby | html %] -

[% suggestions_loo.title | html %] - [% suggestions_loo.author | html %]

+

[% suggestion.title | html %] - [% suggestion.author | html %]

- [% IF ( suggestions_loo.copyrightdate ) %]© [% suggestions_loo.copyrightdate | html %] [% END %] - [% IF ( suggestions_loo.volumedesc ) %]volume: [% suggestions_loo.volumedesc | html %] [% END %] - [% IF ( suggestions_loo.isbn ) %]ISBN: [% suggestions_loo.isbn | html %] [% END %] - [% IF ( suggestions_loo.publishercode ) %]
published by: [% suggestions_loo.publishercode | html %] [% END %] - [% IF ( suggestions_loo.publicationyear ) %] in [% suggestions_loo.publicationyear | html %] [% END %] - [% IF ( suggestions_loo.place ) %] in [% suggestions_loo.place | html %] [% END %] - [% IF ( suggestions_loo.note ) %]

([% suggestions_loo.note | html %])

[% END %] + [% IF ( suggestion.copyrightdate ) %]© [% suggestion.copyrightdate | html %] [% END %] + [% IF ( suggestion.volumedesc ) %]volume: [% suggestion.volumedesc | html %] [% END %] + [% IF ( suggestion.isbn ) %]ISBN: [% suggestion.isbn | html %] [% END %] + [% IF ( suggestion.publishercode ) %]
published by: [% suggestion.publishercode | html %] [% END %] + [% IF ( suggestion.publicationyear ) %] in [% suggestion.publicationyear | html %] [% END %] + [% IF ( suggestion.place ) %] in [% suggestion.place | html %] [% END %] + [% IF ( suggestion.note ) %]

([% suggestion.note | html %])

[% END %]

[% INCLUDE 'patron-title.inc' patron => suggestion.suggester %][% INCLUDE 'patron-title.inc' patron => suggestion.manager %] - [% suggestions_loo.surnamesuggestedby | html %][% IF ( suggestions_loo.firstnamesuggestedby ) %],[% END %] [% suggestions_loo.firstnamesuggestedby | html %] + [% Branches.GetName(suggestion.branchcode) | html %] - [% suggestions_loo.surnamemanagedby | html %][% IF ( suggestions_loo.firstnamemanagedby ) %],[% END %] [% suggestions_loo.firstnamemanagedby | html %] + [% suggestion.fund.budget_name | html %] - [% Branches.GetName(suggestions_loo.branchcode) | html %] + [% suggestion.price | $Price %] - [% suggestions_loo.budget_name | html %] - - [% suggestions_loo.price | $Price %] - - [% IF (suggestions_loo.quantity > 0) %] - [% suggestions_loo.quantity | html %] + [% IF (suggestion.quantity > 0) %] + [% suggestion.quantity | html %] [% END %] - [% suggestions_loo.total | $Price %] + [% suggestion.total | $Price %] - [% IF ( suggestions_loo.biblionumber ) %] - Order + [% IF ( suggestion.biblionumber ) %] + Order [% ELSE %] - Order + Order [% END %]
@@ -66,13 +66,7 @@ - [% FOREACH suggestions_loo IN suggestions_loop %] + [% FOREACH suggestion IN suggestions %] - [% IF ( loggedinusername ) %] + [% IF logged_in_user %] [% END %] [% IF Koha.Preference( 'OPACViewOthersSuggestions' ) == 1 %] [% END %] [% IF Koha.Preference( 'OpacSuggestionManagedBy' ) %] [% END %] - [% END # / FOREACH suggestions_loo %] + [% END # / FOREACH suggestions %]
[% s.note | html %] - [% IF ( s.surnamemanagedby ) %] - [% s.surnamemanagedby | html %] - [% IF ( s.firstnamemanagedby ) %],[% END %] - [% s.firstnamemanagedby | html %] - [% ELSE %] -   - [% END %] + [% INCLUDE 'patron-title.inc' patron => s.manager %] [% IF s.manageddate %] - [% s.manageddate | $KohaDates %][% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt index 2f906637f6..86f2d69daa 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt @@ -250,7 +250,7 @@ [% IF ( deleted ) %]
The selected suggestions have been deleted.
[% END %] - [% IF ( suggestions_loop ) %] + [% IF suggestions.count %]
@@ -309,66 +309,64 @@
- [% IF ( suggestions_loo.showcheckbox ) %] - + [% IF logged_in_user.borrowernumber == suggestion.suggestedby %] + [% END %] -

[% suggestions_loo.title | html %]

-

[% IF ( suggestions_loo.author ) %][% suggestions_loo.author | html %],[% END %] - [% IF ( suggestions_loo.copyrightdate ) %] - [% suggestions_loo.copyrightdate | html %],[% END %] - [% IF ( suggestions_loo.publishercode ) %] - [% suggestions_loo.publishercode | html %][% END %] - [% IF ( suggestions_loo.place ) %]([% suggestions_loo.place | html %])[% END %] - [% IF ( suggestions_loo.collectiontitle ) %] , [% suggestions_loo.collectiontitle | html %][% END %] - [% IF ( suggestions_loo.itemtype ) %] - [% AuthorisedValues.GetByCode( 'SUGGEST_FORMAT', suggestions_loo.itemtype, 1 ) | html %][% END %] +

[% suggestion.title | html %]

+

[% IF ( suggestion.author ) %][% suggestion.author | html %],[% END %] + [% IF ( suggestion.copyrightdate ) %] - [% suggestion.copyrightdate | html %],[% END %] + [% IF ( suggestion.publishercode ) %] - [% suggestion.publishercode | html %][% END %] + [% IF ( suggestion.place ) %]([% suggestion.place | html %])[% END %] + [% IF ( suggestion.collectiontitle ) %] , [% suggestion.collectiontitle | html %][% END %] + [% IF ( suggestion.itemtype ) %] - [% AuthorisedValues.GetByCode( 'SUGGEST_FORMAT', suggestion.itemtype, 1 ) | html %][% END %]

- [% IF ( suggestions_loo.suggesteddate ) %][% suggestions_loo.suggesteddate |$KohaDates %][% END %] + [% IF ( suggestion.suggesteddate ) %][% suggestion.suggesteddate |$KohaDates %][% END %] - [% IF ( suggestions_loo.note ) %] + [% IF ( suggestion.note ) %] Note: - [% suggestions_loo.note | html %] + [% suggestion.note | html %] [% END %] - [% IF ( suggestions_loo.branchcodesuggestedby ) %] + [% IF suggestion.suggestedby %] Suggested for: - [% suggestions_loo.branchcodesuggestedby | html %] + [% Branches.GetName(suggestion.suggester.branchcode) | html %] [% END %] - [% IF ( suggestions_loo.surnamemanagedby ) %] + [% IF suggestion.managedby %] Managed by: - [% suggestions_loo.surnamemanagedby | html %] - [% IF ( suggestions_loo.firstnamemanagedby ) %] , [% suggestions_loo.firstnamemanagedby | html %] - [% END %] - [% IF ( suggestions_loo.manageddate ) %] - [% suggestions_loo.manageddate | $KohaDates %][% END %] + [% INCLUDE 'patron-title.inc' patron = suggestion.manager %] + [% IF ( suggestion.manageddate ) %] - [% suggestion.manageddate | $KohaDates %][% END %] [% END %] Status: - [% IF ( suggestions_loo.ASKED ) %]Requested - [% ELSIF ( suggestions_loo.CHECKED ) %]Checked by the library - [% ELSIF ( suggestions_loo.ACCEPTED ) %]Accepted by the library - [% ELSIF ( suggestions_loo.ORDERED ) %]Ordered by the library - [% ELSIF ( suggestions_loo.REJECTED ) %]Suggestion declined - [% ELSIF ( suggestions_loo.AVAILABLE ) %]Available in the library - [% ELSE %] [% AuthorisedValues.GetByCode( 'SUGGEST_STATUS', suggestions_loo.STATUS, 1 ) | html %] [% END %] - [% IF ( suggestions_loo.reason ) %]([% suggestions_loo.reason | html %])[% END %] + [% IF ( suggestion.STATUS == 'ASKED' ) %]Requested + [% ELSIF ( suggestion.STATUS == 'CHECKED' ) %]Checked by the library + [% ELSIF ( suggestion.STATUS == 'ACCEPTED' ) %]Accepted by the library + [% ELSIF ( suggestion.STATUS == 'ORDERED' ) %]Ordered by the library + [% ELSIF ( suggestion.STATUS == 'REJECTED' ) %]Suggestion declined + [% ELSIF ( suggestion.STATUS == 'AVAILABLE' ) %]Available in the library + [% ELSE %] [% AuthorisedValues.GetByCode( 'SUGGEST_STATUS', suggestion.STATUS, 1 ) | html %] [% END %] + [% IF ( suggestion.reason ) %]([% suggestion.reason | html %])[% END %]
@@ -391,7 +389,7 @@

New purchase suggestion

[% END %] [% END %] - [% END # / IF suggestions_loop %] + [% END # / IF suggestions.count %] [% END # IF op_else %] diff --git a/members/purchase-suggestions.pl b/members/purchase-suggestions.pl index 67c021e020..5aa2315dca 100755 --- a/members/purchase-suggestions.pl +++ b/members/purchase-suggestions.pl @@ -23,9 +23,8 @@ use CGI qw ( -utf8 ); use C4::Auth; use C4::Context; use C4::Output; -use C4::Members; -use C4::Suggestions; use Koha::Patrons; +use Koha::Suggestions; my $input = new CGI; @@ -51,7 +50,7 @@ $template->param( suggestionsview => 1, ); -my $suggestions = SearchSuggestion( { suggestedby => $borrowernumber } ); +my $suggestions = Koha::Suggestions->search( { suggestedby => $borrowernumber }, { prefetch => 'managedby' } ); $template->param( suggestions => $suggestions ); diff --git a/opac/opac-suggestions.pl b/opac/opac-suggestions.pl index b502e92737..39797487a3 100755 --- a/opac/opac-suggestions.pl +++ b/opac/opac-suggestions.pl @@ -38,7 +38,6 @@ use Koha::DateUtils qw( dt_from_string ); my $input = new CGI; my $op = $input->param('op'); my $biblionumber = $input->param('biblionumber'); -my $suggestion = $input->Vars; my $negcaptcha = $input->param('negcap'); my $suggested_by_anyone = $input->param('suggested_by_anyone') || 0; my $need_confirm = 0; @@ -47,10 +46,6 @@ my $need_confirm = 0; if ($negcaptcha ) { print $input->redirect("/cgi-bin/koha/opac-suggestions.pl"); exit; -} else { - # don't pass 'negcap' column to DB, else DBI::Class will error - # DBIx::Class::Row::store_column(): No such column 'negcap' on Koha::Schema::Result::Suggestion at Koha/C4/Suggestions.pm - delete $suggestion->{negcap}; } #If suggestions are turned off we redirect to 404 error. This will also redirect guest suggestions @@ -59,7 +54,6 @@ if ( ! C4::Context->preference('suggestion') ) { exit; } -delete $suggestion->{$_} foreach qw; $op = 'else' unless $op; my ( $template, $borrowernumber, $cookie, @messages ); @@ -87,36 +81,44 @@ else { ); } +my $suggested_by; if ( $op eq 'else' ) { if ( C4::Context->preference("OPACViewOthersSuggestions") ) { if ( $borrowernumber ) { # A logged in user is able to see suggestions from others - $suggestion->{suggestedby} = $suggested_by_anyone + $suggested_by = $suggested_by_anyone ? undef : $borrowernumber; } - else { - # Non logged in user is able to see all suggestions - $suggestion->{suggestedby} = undef; - } + # else: Non logged in user is able to see all suggestions } else { if ( $borrowernumber ) { - $suggestion->{suggestedby} = $borrowernumber; + $suggested_by = $borrowernumber; } else { - $suggestion->{suggestedby} = -1; + $suggested_by = -1; } } } else { if ( $borrowernumber ) { - $suggestion->{suggestedby} = $borrowernumber; + $suggested_by = $borrowernumber; } else { - $suggestion->{suggestedby} = C4::Context->preference("AnonymousPatron"); + $suggested_by = C4::Context->preference("AnonymousPatron"); } } +my @suggestion_fields = + qw( title author copyrightdate isbn publishercode collectiontitle place quantity itemtype patronreason note ); +my $suggestion = { + map { + # Keep parameters that are not an empty string + my $p = $input->param($_); + ( defined $p && $p ne '' ? ( $_ => $p ) : () ) + } @suggestion_fields +}; + if ( $op eq "add_validate" && not $biblionumber ) { # If we are creating the suggestion from an existing record we do not want to search for duplicates $op = 'add_confirm'; my $biblio = MarcRecordFromNewSuggestion($suggestion); @@ -129,24 +131,29 @@ if ( $op eq "add_validate" && not $biblionumber ) { # If we are creating the sug my $patrons_pending_suggestions_count = 0; if ( $borrowernumber && C4::Context->preference("MaxOpenSuggestions") ne '' ) { - $patrons_pending_suggestions_count = scalar @{ SearchSuggestion( { suggestedby => $borrowernumber, STATUS => 'ASKED' } ) } ; + $patrons_pending_suggestions_count = Koha::Suggestions->search( + { + suggestedby => $borrowernumber, + STATUS => 'ASKED', + } + )->count; } if ( $op eq "add_confirm" ) { - my $suggestions_loop = &SearchSuggestion($suggestion); + my $suggestions = Koha::Suggestions->search($suggestion); if ( C4::Context->preference("MaxOpenSuggestions") ne '' && $patrons_pending_suggestions_count >= C4::Context->preference("MaxOpenSuggestions") ) #only check limit for signed in borrowers { push @messages, { type => 'error', code => 'too_many' }; } - elsif ( @$suggestions_loop >= 1 ) { + elsif ( $suggestions->count >= 1 ) { #some suggestion are answering the request Donot Add - for my $suggestion (@$suggestions_loop) { + while ( my $suggestion = $suggestions->next ) { push @messages, { type => 'error', code => 'already_exists', - id => $suggestion->{suggestionid} + id => $suggestion->suggestionid }; last; } @@ -165,19 +172,20 @@ if ( $op eq "add_confirm" ) { &NewSuggestion($suggestion); $patrons_pending_suggestions_count++; - # delete empty fields, to avoid filter in "SearchSuggestion" - foreach my $field ( qw( title author publishercode copyrightdate place collectiontitle isbn STATUS ) ) { - delete $suggestion->{$field}; #clear search filters (except borrower related) to show all suggestions after placing a new one - } - $suggestions_loop = &SearchSuggestion($suggestion); - push @messages, { type => 'info', code => 'success_on_inserted' }; } $op = 'else'; } -my $suggestions_loop = &SearchSuggestion({suggestedby => $suggestion->{suggestedby}}); +my $suggestions = Koha::Suggestions->search( + { + $suggestion->{suggestedby} + ? ( suggestedby => $suggestion->{suggestedby} ) + : () + } +); + if ( $op eq "delete_confirm" ) { my @delete_field = $input->multi_param("delete_field"); foreach my $delete_field (@delete_field) { @@ -188,24 +196,6 @@ if ( $op eq "delete_confirm" ) { exit; } -map{ - my $s = $_; - my $library = Koha::Libraries->find($s->{branchcodesuggestedby}); - $library ? $s->{branchcodesuggestedby} = $library->branchname : () -} @$suggestions_loop; - -foreach my $suggestion(@$suggestions_loop) { - if($suggestion->{'suggestedby'} == $borrowernumber) { - $suggestion->{'showcheckbox'} = $borrowernumber; - } else { - $suggestion->{'showcheckbox'} = 0; - } - if($suggestion->{'patronreason'}){ - my $av = Koha::AuthorisedValues->search({ category => 'OPAC_SUG', authorised_value => $suggestion->{patronreason} }); - $suggestion->{'patronreason'} = $av->count ? $av->next->opac_description : ''; - } -} - my $patron_reason_loop = GetAuthorisedValues("OPAC_SUG"); # Is the person allowed to choose their branch @@ -247,7 +237,7 @@ my @mandatoryfields; $template->param( %$suggestion, - suggestions_loop => $suggestions_loop, + suggestions => $suggestions, patron_reason_loop => $patron_reason_loop, "op_$op" => 1, $op => 1, -- 2.11.0