Attachment 128071 Details for Bug 23991 – Bug 23991: Move SearchSuggestion to Koha::Suggestions

[patch] Bug 23991: Move SearchSuggestion to Koha::Suggestions

Bug-23991-Move-SearchSuggestion-to-KohaSuggestions.patch (text/plain), 61.28 KB, created by Tomás Cohen Arazi (tcohen) on 2021-11-29 13:53:20 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2021-11-29 13:53:20 UTC

Size: 61.28 KB

patch

obsolete

>From d3eac98d7f7b2dccc09852b29896557d1dda908f Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Thu, 7 Nov 2019 17:41:02 +0100
>Subject: [PATCH] Bug 23991: Move SearchSuggestion to Koha::Suggestions
>
>The C4::Suggestions::SearchSuggestion subroutine is badly written and
>can be replaced by calls to Koha::Suggestions->search.
>The hard part in this patch is suggestion.pl, the other occurrences have
>been replaced easily.
>
>Test plan:
>The idea is to test the whole suggestion workflow.
>1. Create a suggestion on OPAC
>2. Create a suggestion on the staff interface
>3. Edit suggestions
>4. Filter suggestions (use the different filters and "organize by"
>values)
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>
>Bug 23991: Remove unused variable
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> C4/Suggestions.pm                             | 150 ------------------
> Koha/Suggestions.pm                           |  21 +++
> acqui/newordersuggestion.pl                   |  18 +--
> .../en/modules/acqui/newordersuggestion.tt    |  50 +++---
> .../modules/members/purchase-suggestions.tt   |  10 +-
> .../prog/en/modules/suggestion/suggestion.tt  | 113 ++++++-------
> .../bootstrap/en/modules/opac-suggestions.tt  |  68 ++++----
> members/deletemem.pl                          |   8 +-
> members/purchase-suggestions.pl               |   5 +-
> opac/opac-suggestions.pl                      |  73 ++++-----
> suggestion/suggestion.pl                      |  51 ++++--
> 11 files changed, 219 insertions(+), 348 deletions(-)
>
>diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm
>index ba72a1888a..a9db5d86a8 100644
>--- a/C4/Suggestions.pm
>+++ b/C4/Suggestions.pm
>@@ -41,7 +41,6 @@ our @EXPORT  = qw(
>   ModStatus
>   ModSuggestion
>   NewSuggestion
>-  SearchSuggestion
>   DelSuggestionsOlderThan
>   GetUnprocessedSuggestions
>   MarcRecordFromNewSuggestion
>@@ -72,155 +71,6 @@ Suggestions done by other borrowers can be seen when not "AVAILABLE"
> 
> =head1 FUNCTIONS
> 
>-=head2 SearchSuggestion
>-
>-(\@array) = &SearchSuggestion($suggestionhashref_to_search)
>-
>-searches for a suggestion
>-
>-return :
>-C<\@array> : the aqorders found. Array of hash.
>-Note the status is stored twice :
>-* in the status field
>-* as parameter ( for example ASKED => 1, or REJECTED => 1) . This is for template & translation purposes.
>-
>-=cut
>-
>-sub SearchSuggestion {
>-    my ($suggestion) = @_;
>-    my $dbh = C4::Context->dbh;
>-    my @sql_params;
>-    my @query = (
>-        q{
>-        SELECT suggestions.*,
>-            U1.branchcode       AS branchcodesuggestedby,
>-            B1.branchname       AS branchnamesuggestedby,
>-            U1.surname          AS surnamesuggestedby,
>-            U1.firstname        AS firstnamesuggestedby,
>-            U1.cardnumber       AS cardnumbersuggestedby,
>-            U1.email            AS emailsuggestedby,
>-            U1.borrowernumber   AS borrnumsuggestedby,
>-            U1.categorycode     AS categorycodesuggestedby,
>-            C1.description      AS categorydescriptionsuggestedby,
>-            U2.surname          AS surnamemanagedby,
>-            U2.firstname        AS firstnamemanagedby,
>-            B2.branchname       AS branchnamesuggestedby,
>-            U2.email            AS emailmanagedby,
>-            U2.branchcode       AS branchcodemanagedby,
>-            U2.borrowernumber   AS borrnummanagedby,
>-            U3.surname          AS surnamelastmodificationby,
>-            U3.firstname        AS firstnamelastmodificationby,
>-            BU.budget_name      AS budget_name
>-        FROM suggestions
>-            LEFT JOIN borrowers     AS U1 ON suggestedby=U1.borrowernumber
>-            LEFT JOIN branches      AS B1 ON B1.branchcode=U1.branchcode
>-            LEFT JOIN categories    AS C1 ON C1.categorycode=U1.categorycode
>-            LEFT JOIN borrowers     AS U2 ON managedby=U2.borrowernumber
>-            LEFT JOIN branches      AS B2 ON B2.branchcode=U2.branchcode
>-            LEFT JOIN categories    AS C2 ON C2.categorycode=U2.categorycode
>-            LEFT JOIN borrowers     AS U3 ON lastmodificationby=U3.borrowernumber
>-            LEFT JOIN aqbudgets     AS BU ON budgetid=BU.budget_id
>-        WHERE 1=1
>-    }
>-    );
>-
>-    # filter on biblio informations
>-    foreach my $field (
>-        qw( title author isbn publishercode copyrightdate collectiontitle ))
>-    {
>-        if ( $suggestion->{$field} ) {
>-            push @sql_params, '%' . $suggestion->{$field} . '%';
>-            push @query,      qq{ AND suggestions.$field LIKE ? };
>-        }
>-    }
>-
>-    # filter on user branch
>-    if (   C4::Context->preference('IndependentBranches')
>-        && !C4::Context->IsSuperLibrarian() )
>-    {
>-        # If IndependentBranches is set and the logged in user is not superlibrarian
>-        # Then we want to filter by the user's library (i.e. cannot see suggestions from other libraries)
>-        my $userenv = C4::Context->userenv;
>-        if ($userenv) {
>-            {
>-                push @sql_params, $$userenv{branch};
>-                push @query,      q{
>-                    AND (suggestions.branchcode=? OR suggestions.branchcode='')
>-                };
>-            }
>-        }
>-    }
>-    elsif (defined $suggestion->{branchcode}
>-        && $suggestion->{branchcode}
>-        && $suggestion->{branchcode} ne '__ANY__' )
>-    {
>-        # If IndependentBranches is not set OR the logged in user is not superlibrarian
>-        # AND the branchcode filter is passed and not '__ANY__'
>-        # Then we want to filter using this parameter
>-        push @sql_params, $suggestion->{branchcode};
>-        push @query,      qq{ AND suggestions.branchcode=? };
>-    }
>-
>-    # filter on nillable fields
>-    foreach my $field (
>-        qw( STATUS itemtype suggestedby managedby acceptedby budgetid biblionumber )
>-      )
>-    {
>-        if ( exists $suggestion->{$field}
>-                and defined $suggestion->{$field}
>-                and $suggestion->{$field} ne '__ANY__'
>-                and (
>-                    $suggestion->{$field} ne q||
>-                        or $field eq 'STATUS'
>-                )
>-        ) {
>-            if ( $suggestion->{$field} eq '__NONE__' ) {
>-                push @query, qq{ AND (suggestions.$field = '' OR suggestions.$field IS NULL) };
>-            }
>-            else {
>-                push @sql_params, $suggestion->{$field};
>-                push @query, qq{ AND suggestions.$field = ? };
>-            }
>-        }
>-    }
>-
>-    # filter on date fields
>-    my $dtf = Koha::Database->new->schema->storage->datetime_parser;
>-    foreach my $field (qw( suggesteddate manageddate accepteddate )) {
>-        my $from = $field . "_from";
>-        my $to   = $field . "_to";
>-        my $from_dt;
>-        $from_dt = eval { dt_from_string( $suggestion->{$from} ) } if ( $suggestion->{$from} );
>-        my $to_dt;
>-        $to_dt = eval { dt_from_string( $suggestion->{$to} ) } if ( $suggestion->{$to} );
>-        if ( $from_dt ) {
>-            push @query, qq{ AND suggestions.$field >= ?};
>-            push @sql_params, $dtf->format_date($from_dt);
>-        }
>-        if ( $to_dt ) {
>-            push @query, qq{ AND suggestions.$field <= ?};
>-            push @sql_params, $dtf->format_date($to_dt);
>-        }
>-    }
>-
>-    # By default do not search for archived suggestions
>-    unless ( exists $suggestion->{archived} && $suggestion->{archived} ) {
>-        push @query, q{ AND suggestions.archived = 0 };
>-    }
>-
>-    my $sth = $dbh->prepare("@query");
>-    $sth->execute(@sql_params);
>-    my @results;
>-
>-    # add status as field
>-    while ( my $data = $sth->fetchrow_hashref ) {
>-        $data->{ $data->{STATUS} } = 1;
>-        push( @results, $data );
>-    }
>-
>-    return ( \@results );
>-}
>-
> =head2 GetSuggestion
> 
> \%sth = &GetSuggestion($suggestionid)
>diff --git a/Koha/Suggestions.pm b/Koha/Suggestions.pm
>index 03262bb5f1..5d24dfebea 100644
>--- a/Koha/Suggestions.pm
>+++ b/Koha/Suggestions.pm
>@@ -32,6 +32,27 @@ Koha::Suggestions - Koha Suggestion object set class
> 
> =head1 API
> 
>+=cut
>+
>+sub search_limited {
>+    my ( $self, $params, $attributes ) = @_;
>+
>+    $attributes //= {};
>+    # filter on user branch
>+    if (   C4::Context->preference('IndependentBranches')
>+        && !C4::Context->IsSuperLibrarian() )
>+    {
>+        # If IndependentBranches is set and the logged in user is not superlibrarian
>+        # Then we want to filter by the user's library (i.e. cannot see suggestions from other libraries)
>+        my $userenv = C4::Context->userenv;
>+        if ( $userenv ) {
>+            $params->{branchcode} = { -or => [ $userenv->{branch}, '' ] };
>+        }
>+    }
>+
>+    return $self->search($params, $attributes);
>+}
>+
> =head2 Class Methods
> 
> =cut
>diff --git a/acqui/newordersuggestion.pl b/acqui/newordersuggestion.pl
>index 0ee10365cb..9423e3f0d0 100755
>--- a/acqui/newordersuggestion.pl
>+++ b/acqui/newordersuggestion.pl
>@@ -97,6 +97,7 @@ use C4::Suggestions qw( ConnectSuggestionAndBiblio SearchSuggestion );
> use C4::Budgets;
> 
> use Koha::Acquisition::Booksellers;
>+use Koha::Suggestions;
> 
> my $input = CGI->new;
> 
>@@ -127,23 +128,22 @@ if ( $op eq 'connectDuplicate' ) {
>     ConnectSuggestionAndBiblio( $suggestionid, $duplicateNumber );
> }
> 
>-# getting all suggestions.
>-my $suggestions_loop = SearchSuggestion(
>+my $suggestions = Koha::Suggestions->search_limited(
>     {
>-        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 0946308afb..737c0d3c82 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/newordersuggestion.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/newordersuggestion.tt
>@@ -41,7 +41,7 @@
>             <main>
> 
> <h1>Suggestions</h1>
>-    [% IF ( suggestions_loop ) %]
>+    [% IF suggestions.count %]
>     <a href="#" id="show_only_mine">Show only mine</a> | <a href="#" id="show_all">Show all suggestions</a>
>     <table id="suggestionst">
>         <thead>
>@@ -59,49 +59,45 @@
>         </tr>
>         </thead>
>         <tbody>
>-        [% FOREACH suggestions_loo IN suggestions_loop %]
>+        [% FOREACH suggestion IN suggestions %]
>             <tr>
>-                <td>[% suggestions_loo.managedby | html %]</td>
>+                <td>[% suggestion.managedby | html %]</td>
>                 <td>
>-                    <p>[% suggestions_loo.title | html %] - [% suggestions_loo.author | html %]</p>
>+                    <p>[% suggestion.title | html %] - [% suggestion.author | html %]</p>
>                     <p>
>-                        [% IF ( suggestions_loo.copyrightdate ) %]&copy; [% suggestions_loo.copyrightdate | html %] [% END %]
>-                        [% IF ( suggestions_loo.volumedesc ) %]volume: <em>[% suggestions_loo.volumedesc | html %]</em> [% END %]
>-                        [% IF ( suggestions_loo.isbn ) %]ISBN: <em>[% suggestions_loo.isbn | html %]</em> [% END %]
>-                        [% IF ( suggestions_loo.publishercode ) %]<br />published by: [% suggestions_loo.publishercode | html %] [% END %]
>-                        [% IF ( suggestions_loo.publicationyear ) %] in <em>[% suggestions_loo.publicationyear | html %]</em> [% END %]
>-                        [% IF ( suggestions_loo.place ) %] in <em>[% suggestions_loo.place | html %]</em> [% END %]
>-                        [% IF ( suggestions_loo.note ) %]<p><em>([% suggestions_loo.note | html %])</em></p> [% END %]
>+                        [% IF ( suggestion.copyrightdate ) %]&copy; [% suggestion.copyrightdate | html %] [% END %]
>+                        [% IF ( suggestion.volumedesc ) %]volume: <em>[% suggestion.volumedesc | html %]</em> [% END %]
>+                        [% IF ( suggestion.isbn ) %]ISBN: <em>[% suggestion.isbn | html %]</em> [% END %]
>+                        [% IF ( suggestion.publishercode ) %]<br />published by: [% suggestion.publishercode | html %] [% END %]
>+                        [% IF ( suggestion.publicationyear ) %] in <em>[% suggestion.publicationyear | html %]</em> [% END %]
>+                        [% IF ( suggestion.place ) %] in <em>[% suggestion.place | html %]</em> [% END %]
>+                        [% IF ( suggestion.note ) %]<p><em>([% suggestion.note | html %])</em></p> [% END %]
>                     </p>
>                 </td>
>+                <td>[% INCLUDE 'patron-title.inc' patron => suggestion.suggester %]</td>
>+                <td>[% INCLUDE 'patron-title.inc' patron => suggestion.manager %]</td>
>                 <td>
>-                    [% suggestions_loo.surnamesuggestedby | html %][% IF ( suggestions_loo.firstnamesuggestedby ) %],[% END %] [% suggestions_loo.firstnamesuggestedby | html %]
>+                    [% Branches.GetName(suggestion.branchcode) | html %]
>                 </td>
>                 <td>
>-                    [% suggestions_loo.surnamemanagedby | html %][% IF ( suggestions_loo.firstnamemanagedby ) %],[% END %] [% suggestions_loo.firstnamemanagedby | html %]
>+                    [% suggestion.fund.budget_name | html %]
>                 </td>
>                 <td>
>-                    [% Branches.GetName(suggestions_loo.branchcode) | html %]
>+                    [% suggestion.price | $Price %]
>                 </td>
>                 <td>
>-                    [% suggestions_loo.budget_name | html %]
>-                </td>
>-                <td>
>-                    [% suggestions_loo.price | $Price %]
>-                </td>
>-                <td>
>-                    [% IF (suggestions_loo.quantity > 0) %]
>-                        [% suggestions_loo.quantity | html %]
>+                    [% IF (suggestion.quantity > 0) %]
>+                        [% suggestion.quantity | html %]
>                     [% END %]
>                 </td>
>                 <td>
>-                    [% suggestions_loo.total | $Price %]
>+                    [% suggestion.total | $Price %]
>                 </td>
>                 <td class="actions">
>-                    [% IF ( suggestions_loo.biblionumber ) %]
>-                        <a href="neworderempty.pl?booksellerid=[% booksellerid | uri %]&amp;basketno=[% basketno | uri %]&amp;suggestionid=[% suggestions_loo.suggestionid | uri %]&amp;biblio=[% suggestions_loo.biblionumber | uri %]" class="btn btn-default btn-xs"><i class="fa fa-plus"></i> [% tp('verb', 'Order') | html %]</a>
>+                    [% IF ( suggestion.biblionumber ) %]
>+                        <a href="neworderempty.pl?booksellerid=[% booksellerid | uri %]&amp;basketno=[% basketno | uri %]&amp;suggestionid=[% suggestion.suggestionid | uri %]&amp;biblio=[% suggestion.biblionumber | uri %]" class="btn btn-default btn-xs"><i class="fa fa-plus"></i> [% tp('verb', 'Order') | html %]</a>
>                     [% ELSE %]
>-                        <a href="neworderempty.pl?booksellerid=[% booksellerid | uri %]&amp;basketno=[% basketno | uri %]&amp;suggestionid=[% suggestions_loo.suggestionid | uri %]" class="btn btn-default btn-xs"><i class="fa fa-plus"></i> [% tp('verb', 'Order') | html %]</a>
>+                        <a href="neworderempty.pl?booksellerid=[% booksellerid | uri %]&amp;basketno=[% basketno | uri %]&amp;suggestionid=[% suggestion.suggestionid | uri %]" class="btn btn-default btn-xs"><i class="fa fa-plus"></i> [% tp('verb', 'Order') | html %]</a>
>                     [% END %]
>                 </td>
>             </tr>
>@@ -136,7 +132,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 a0b6dd57fb..fb956e8cad 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
>@@ -42,7 +42,7 @@
>                     <a class="btn btn-default" id="newsuggestion" href="/cgi-bin/koha/suggestion/suggestion.pl?op=add&amp;suggestedby=[% patron.borrowernumber | html %]&amp;redirect=purchase_suggestions&amp;borrowernumber=[% patron.borrowernumber | html %]"><i class="fa fa-plus"></i> New purchase suggestion</a>
>                 </div>
> 
>-                [% IF suggestions %]
>+                [% IF suggestions.count %]
>                   <table id="suggestions">
>                     <thead>
>                         <tr>
>@@ -79,13 +79,7 @@
>                                 </td>
>                                 <td>[% s.note | html %]
>                                 <td>
>-                                    [% IF ( s.surnamemanagedby ) %]
>-                                        [% s.surnamemanagedby | html %]
>-                                        [% IF ( s.firstnamemanagedby ) %],[% END %]
>-                                        [% s.firstnamemanagedby | html %]
>-                                    [% ELSE %]
>-                                        &nbsp;
>-                                    [% END %]
>+                                    [% INCLUDE 'patron-title.inc' patron => s.manager %]
>                                 </td>
>                                 <td data-order="[% s.manageddate | html %]">
>                                     [% s.manageddate | $KohaDates %]
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
>index 27ca89ba9e..ff04d6d753 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
>@@ -707,7 +707,7 @@
>                                     No name
>                                 [% END %]
>                             [% END %]
>-                            ([% suggestion.suggestions_loop.size | html %])</a></li>
>+                            ([% suggestion.suggestions.count| html %])</a></li>
> 
>                         [% END # /FOREACH suggestion %]
>                     </ul> 
>@@ -717,7 +717,7 @@
>                 <div id="[% suggestion.suggestiontype | html %]">
>                     <form class="update_suggestions" name="f" method="post" action="/cgi-bin/koha/suggestion/suggestion.pl#[% suggestion.suggestiontype| uri %]">
> 
>-                        [% IF ( suggestion.suggestions_loop ) %]
>+                        [% IF suggestion.suggestions.count %]
>                             <p>
>                                 <a class="checkall" href="#">Check all</a> | <a class="uncheckall" href="#">Uncheck all</a>
>                             </p>
>@@ -741,113 +741,116 @@
>                                     </tr>
>                                 </thead>
>                                 <tbody>
>-                                    [% FOREACH suggestions_loo IN suggestion.suggestions_loop %]
>+                                    [% FOREACH s IN suggestion.suggestions %]
>                                         <tr>
>                                             <td>
>-                                                <input type="checkbox" name="suggestionid" value="[% suggestions_loo.suggestionid | html %]" />
>+                                                <input type="checkbox" name="suggestionid" value="[% s.suggestionid | html %]" />
>                                             </td>
>                                             <td>
>-                                                <a href="suggestion.pl?suggestionid=[% suggestions_loo.suggestionid | uri %]&amp;op=show" title="suggestion" >
>-                                                    [% suggestions_loo.title | html %][% IF ( suggestions_loo.author ) %], by [% suggestions_loo.author | html %][% END %]
>+                                                <a href="suggestion.pl?suggestionid=[% s.suggestionid | uri %]&amp;op=show" title="suggestion" >
>+                                                    [% s.title | html %][% IF ( s.author ) %], by [% s.author | html %][% END %]
>                                                 </a>
>                                                 <br />
>-                                                [% IF ( suggestions_loo.copyrightdate ) %]
>-                                                    &copy; <span class="suggestion_copyrightdate">[% suggestions_loo.copyrightdate | html %]</span>
>+                                                [% IF ( s.copyrightdate ) %]
>+                                                    &copy; <span class="suggestion_copyrightdate">[% s.copyrightdate | html %]</span>
>                                                 [% END %]
>-                                                [% IF ( suggestions_loo.volumedesc ) %]
>-                                                    ; <span class="suggestion_volume">Volume:<em>[% suggestions_loo.volumedesc | html %]</em></span>
>+                                                [% IF ( s.volumedesc ) %]
>+                                                    ; <span class="suggestion_volume">Volume:<em>[% s.volumedesc | html %]</em></span>
>                                                 [% END %]
>-                                                [% IF ( suggestions_loo.isbn ) %]
>-                                                    ; <span class="suggestion_isbn">ISBN: <em>[% suggestions_loo.isbn | html %]</em></span>
>+                                                [% IF ( s.isbn ) %]
>+                                                    ; <span class="suggestion_isbn">ISBN: <em>[% s.isbn | html %]</em></span>
>                                                 [% END %]
>-                                                [% IF ( suggestions_loo.publishercode ) %]
>-                                                    ; <span class="suggestion_publishercode">Published by [% suggestions_loo.publishercode | html %]</span>
>+                                                [% IF ( s.publishercode ) %]
>+                                                    ; <span class="suggestion_publishercode">Published by [% s.publishercode | html %]</span>
>                                                 [% END %]
>-                                                [% IF ( suggestions_loo.publicationyear ) %]
>-                                                    in <span class="suggestion_publicationyear"><em>[% suggestions_loo.publicationyear | html %]</em></span>
>+                                                [% IF ( s.publicationyear ) %]
>+                                                    in <span class="suggestion_publicationyear"><em>[% s.publicationyear | html %]</em></span>
>                                                 [% END %]
>-                                                [% IF ( suggestions_loo.place ) %]
>-                                                    in <span class="suggestion_place"><em>[% suggestions_loo.place | html %]</em></span>
>+                                                [% IF ( s.place ) %]
>+                                                    in <span class="suggestion_place"><em>[% s.place | html %]</em></span>
>                                                 [% END %]
>-                                                [% IF ( suggestions_loo.collectiontitle ) %]
>-                                                    ; <span class="suggestion_collectiontitle">[% suggestions_loo.collectiontitle | html %]</span>
>+                                                [% IF ( s.collectiontitle ) %]
>+                                                    ; <span class="suggestion_collectiontitle">[% s.collectiontitle | html %]</span>
>                                                 [% END %]
>-                                                [% IF ( suggestions_loo.itemtype ) %]
>-                                                    ; <span class="suggestion_itype">[% AuthorisedValues.GetByCode( 'SUGGEST_FORMAT', suggestions_loo.itemtype, 0 ) | html %]</span>
>+                                                [% IF ( s.itemtype ) %]
>+                                                    ; <span class="suggestion_itype">[% AuthorisedValues.GetByCode( 'SUGGEST_FORMAT', s.itemtype, 0 ) | html %]</span>
>                                                 [% END %]
>                                                 <br />
>-                                                [% IF ( suggestions_loo.note ) %]
>-                                                    <div class="suggestion_note"><i class="fa fa-comment"></i> [% suggestions_loo.note | html %]</div>
>+                                                [% IF ( s.note ) %]
>+                                                    <div class="suggestion_note"><i class="fa fa-comment"></i> [% s.note | html %]</div>
>                                                 [% END %]
>-                                                [% IF suggestions_loo.archived %]
>+                                                [% IF s.archived %]
>                                                     <br /><i class="fa fa-archive"></i> Archived
>                                                 [% END %]
>                                             </td>
>                                             <td>
>-                                                <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% suggestions_loo.suggestedby | uri %]">[% suggestions_loo.surnamesuggestedby | html %][% IF ( suggestions_loo.firstnamesuggestedby ) %], [% suggestions_loo.firstnamesuggestedby | html %][% END %] [% IF (suggestions_loo.cardnumbersuggestedby ) %]([% suggestions_loo.cardnumbersuggestedby | html %])[% END %]</a>
>+                                                [% SET suggester = s.suggester %]
>+                                                <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% suggester.borrowernumber | uri %]">[% suggester.surname | html %][% IF suggester.firstname %], [% suggester.firstname | html %][% END %] [% IF suggester.cardnumber %]([% suggester.cardnumber | html %])[% END %]</a>
>                                             </td>
>-                                            <td data-order="[% suggestions_loo.suggesteddate | html %]">
>-                                                [% IF ( suggestions_loo.suggesteddate ) %][% suggestions_loo.suggesteddate | $KohaDates %][% END %]
>+                                            <td data-order="[% s.suggesteddate | html %]">
>+                                                [% IF ( s.suggesteddate ) %][% s.suggesteddate | $KohaDates %][% END %]
>                                             </td>
>                                             <td>
>-                                                <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% suggestions_loo.managedby | uri %]">[% suggestions_loo.surnamemanagedby | html %][% IF ( suggestions_loo.firstnamemanagedby ) %], [% suggestions_loo.firstnamemanagedby | html %][% END %]</a>
>+                                                [% SET manager = s.manager %]
>+                                                <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% manager.borrowernumber | uri %]">[% manager.surname | html %][% IF manager.firstname %], [% manager.firstname | html %][% END %]</a>
>                                             </td>
>-                                            <td data-order="[% suggestions_loo.manageddate | html %]">
>-                                                [% IF ( suggestions_loo.manageddate ) %][% suggestions_loo.manageddate | $KohaDates %][% END %]
>+                                            <td data-order="[% s.manageddate | html %]">
>+                                                [% IF ( s.manageddate ) %][% s.manageddate | $KohaDates %][% END %]
>                                             </td>
>                                             <td>
>-                                                <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% suggestions_loo.lastmodificationby | uri %]">[% suggestions_loo.surnamelastmodificationby | html %][% IF ( suggestions_loo.firstnamelastmodificationby ) %], [% suggestions_loo.firstnamelastmodificationby | html %][% END %]</a>
>+                                                [% SET last_modifier = s.last_modifier %]
>+                                                <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% last_modifier.borrowernumber | uri %]">[% last_modifier.surname | html %][% IF last_modifier.firstname %], [% last_modifier.firstname | html %][% END %]</a>
>                                             </td>
>-                                            <td data-order="[% suggestions_loo.lastmodificationdate | html %]">
>-                                                [% IF ( suggestions_loo.lastmodificationdate ) %][% suggestions_loo.lastmodificationdate | $KohaDates %][% END %]
>+                                            <td data-order="[% s.lastmodificationdate | html %]">
>+                                                [% IF ( s.lastmodificationdate ) %][% s.lastmodificationdate | $KohaDates %][% END %]
>                                             </td>
>                                             <td>
>-                                                [% suggestions_loo.date | $KohaDates %]
>+                                                [% s.lastmodificationdate | $KohaDates %]
>                                             </td>
>                                             <td>
>-                                                [% Branches.GetName( suggestions_loo.branchcode ) | html %]
>+                                                [% Branches.GetName( s.branchcode ) | html %]
>                                             </td>
>                                             <td>
>-                                                [% suggestions_loo.budget_name | html %]
>+                                                [% s.fund.budget_name | html %]
>                                             </td>
>                                             <td>
>-                                                [% IF ( suggestions_loo.ASKED ) %]
>+                                                [% IF s.STATUS == 'ASKED' %]
>                                                     Pending
>-                                                [% ELSIF ( suggestions_loo.ACCEPTED ) %]
>+                                                [% ELSIF s.STATUS == 'ACCEPTED' %]
>                                                     Accepted
>-                                                [% ELSIF ( suggestions_loo.ORDERED ) %]
>+                                                [% ELSIF s.STATUS == 'ORDERED' %]
>                                                     Ordered
>-                                                [% ELSIF ( suggestions_loo.REJECTED ) %]
>+                                                [% ELSIF s.STATUS == 'REJECTED' %]
>                                                     Rejected
>-                                                [% ELSIF ( suggestions_loo.CHECKED ) %]
>+                                                [% ELSIF s.STATUS == 'CHECKED' %]
>                                                     Checked
>-                                                [% ELSIF ( suggestions_loo.AVAILABLE ) %]
>+                                                [% ELSIF s.STATUS == 'AVAILABLE' %]
>                                                     Available
>-                                                [% ELSIF AuthorisedValues.GetByCode( 'SUGGEST_STATUS', suggestions_loo.STATUS ) %]
>-                                                    [% AuthorisedValues.GetByCode( 'SUGGEST_STATUS', suggestions_loo.STATUS ) | html %]
>+                                                [% ELSIF AuthorisedValues.GetByCode( 'SUGGEST_STATUS', s.STATUS ) %]
>+                                                    [% AuthorisedValues.GetByCode( 'SUGGEST_STATUS', s.STATUS ) | html %]
>                                                 [% ELSE %]
>                                                     Status unknown
>                                                 [% END %]
> 
>-                                                [% IF ( suggestions_loo.reason ) %]
>-                                                    <br />([% suggestions_loo.reason | html %])
>+                                                [% IF ( s.reason ) %]
>+                                                    <br />([% s.reason | html %])
>                                                 [% END %]
>                                             </td>
>                                             <td class="actions">
>                                                 <div class="btn-group dropup">
>-                                                    <a class="btn btn-default btn-xs" role="button" href="suggestion.pl?suggestionid=[% suggestions_loo.suggestionid | html %]&amp;op=edit"><i class="fa fa-pencil"></i> Edit</a><a class="btn btn-default btn-xs dropdown-toggle" id="more_actions_[% suggestions_loo.suggestionid | html %]" role="button" data-toggle="dropdown" href="#"><b class="caret"></b></a>
>-                                                    <ul class="dropdown-menu pull-right" role="menu" aria-labelledby="more_actions_[% suggestions_loo.suggestionid | html %]">
>-                                                        <li><a class="deletesuggestion" href="suggestion.pl?op=delete&amp;suggestionid=[% suggestions_loo.suggestionid | html %]"><i class="fa fa-trash"></i> Delete</a></li>
>-                                                        [% UNLESS suggestions_loo.archived %]
>-                                                            <li><a class="archivesuggestion" href="suggestion.pl?op=archive&amp;suggestionid=[% suggestions_loo.suggestionid | html %]"><i class="fa fa-archive"></i> Archive</a></li>
>+                                                    <a class="btn btn-default btn-xs" role="button" href="suggestion.pl?suggestionid=[% s.suggestionid | html %]&amp;op=edit"><i class="fa fa-pencil"></i> Edit</a><a class="btn btn-default btn-xs dropdown-toggle" id="more_actions_[% s.suggestionid | html %]" role="button" data-toggle="dropdown" href="#"><b class="caret"></b></a>
>+                                                    <ul class="dropdown-menu pull-right" role="menu" aria-labelledby="more_actions_[% s.suggestionid | html %]">
>+                                                        <li><a class="deletesuggestion" href="suggestion.pl?op=delete&amp;suggestionid=[% s.suggestionid | html %]"><i class="fa fa-trash"></i> Delete</a></li>
>+                                                        [% UNLESS s.archived %]
>+                                                            <li><a class="archivesuggestion" href="suggestion.pl?op=archive&amp;suggestionid=[% s.suggestionid | html %]"><i class="fa fa-archive"></i> Archive</a></li>
>                                                         [% ELSE %]
>-                                                            <li><a class="unarchivesuggestion" href="suggestion.pl?op=unarchive&amp;suggestionid=[% suggestions_loo.suggestionid | html %]"><i class="fa fa-archive"></i> Unarchive</a></li>
>+                                                            <li><a class="unarchivesuggestion" href="suggestion.pl?op=unarchive&amp;suggestionid=[% s.suggestionid | html %]"><i class="fa fa-archive"></i> Unarchive</a></li>
>                                                         [% END %]
>                                                     </ul>
>                                                 </div>
>                                             </td>
>                                         </tr>
>-                                    [% END # /FOREACH suggestions_loo %]
>+                                    [% END # /FOREACH s %]
>                                 </tbody>
>                             </table> 
> 
>@@ -1307,7 +1310,7 @@
> 
>                 columns_settings = [% TablesSettings.GetColumns( 'acqui', 'suggestions', 'suggestions', 'json' ) | $raw %]
>                 [% FOREACH suggestion IN suggestions %]
>-                    [% IF ( suggestion.suggestions_loop ) %]
>+                    [% IF suggestion.suggestions.count %]
>                         KohaTable("table_[% loop.count| html %]", {
>                             "sorting": [[ 3, "asc" ]],
>                             "autoWidth": false,
>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 043441d8ce..198a8d8ebc 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt
>@@ -293,7 +293,8 @@
> 
>                             [% IF ( deleted ) %]<div class="alert alert-info">The selected suggestions have been deleted.</div>[% END %]
> 
>-                            [% IF suggestions_loop OR title_filter %]
>+                            [% IF suggestions.count OR title_filter %]
>+                                [% SET can_delete_suggestion = 0 %]
>                                 <form action="/cgi-bin/koha/opac-suggestions.pl" class="form-inline" id="search_suggestions_form" method="get">
>                                     <div class="form-row">
>                                         <label for="title_filter">Search for:</label>
>@@ -320,7 +321,7 @@
>                                     </div>
>                                 </form>
>                             [% END %]
>-                            [% IF suggestions_loop %]
>+                            [% IF suggestions.count %]
>                                 [% SET can_delete_suggestion = 0 %]
>                                 <form action="/cgi-bin/koha/opac-suggestions.pl" method="post" id="delete_suggestions">
>                                     <input type="hidden" name="op" value="delete_confirm" />
>@@ -364,74 +365,73 @@
>                                             </tr>
>                                         </thead>
>                                         <tbody>
>-                                            [% FOREACH suggestions_loo IN suggestions_loop %]
>+                                            [% FOREACH suggestion IN suggestions %]
>                                                 <tr>
>-                                                    [% IF ( loggedinusername ) %]
>+                                                    [% IF logged_in_user %]
>                                                         <td class="selectcol">
>-                                                            [% IF ( suggestions_loo.showcheckbox ) %]
>+                                                            [% IF logged_in_user.borrowernumber == suggestion.suggester.borrowernumber %]
>                                                                 [% SET can_delete_suggestion = 1 %]
>-                                                                <input type="checkbox" class="cb" id="id[% suggestions_loo.suggestionid | html %]" name="delete_field" data-title="[% suggestions_loo.title | html %]" value="[% suggestions_loo.suggestionid | html %]" />
>+                                                                <input type="checkbox" class="cb" id="id[% suggestion.suggestionid | html %]" name="delete_field" data-title="[% suggestion.title | html %]" value="[% suggestion.suggestionid | html %]" />
>                                                             [% END %]
>                                                         </td>
>                                                     [% END %]
>                                                     <td>
>                                                         <p>
>                                                             <label for="id[% suggestions_loo.suggestionid | html %]">
>-                                                                [% IF suggestions_loo.biblionumber %]
>-                                                                    <strong><a href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% suggestions_loo.biblionumber | uri %]">[% suggestions_loo.title | html %]</a></strong>
>+                                                                [% IF suggestion.biblionumber %]
>+                                                                    <strong><a href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% suggestion.biblionumber | uri %]">[% suggestion.title | html %]</a></strong>
>                                                                 [% ELSE %]
>-                                                                    <strong>[% suggestions_loo.title | html %]</strong>
>+                                                                    <strong>[% suggestion.title | html %]</strong>
>                                                                 [% END %]
>                                                             </label>
>                                                         </p>
>-                                                            <p>[% 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 %]
>+                                                            <p>[% 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 %]
>                                                         </p>
>                                                     </td>
>                                                     <td>
>-                                                        [% IF ( suggestions_loo.suggesteddate ) %][% suggestions_loo.suggesteddate |$KohaDates %][% END %]
>+                                                        [% IF ( suggestion.suggesteddate ) %][% suggestion.suggesteddate |$KohaDates %][% END %]
>                                                     </td>
>                                                     <td>
>-                                                        [% IF ( suggestions_loo.note ) %]
>+                                                        [% IF ( suggestion.note ) %]
>                                                             <span class="tdlabel">Note: </span>
>-                                                            [% suggestions_loo.note | html %]
>+                                                            [% suggestion.note | html %]
>                                                         [% END %]
>                                                     </td>
>                                                     [% IF Koha.Preference( 'OPACViewOthersSuggestions' ) == 1 %]
>                                                         <td>
>-                                                            [% IF ( suggestions_loo.branchcodesuggestedby ) %]
>+                                                            [% IF suggestion.suggestedby %]
>                                                                 <span class="tdlabel">Suggested for:</span>
>-                                                                [% suggestions_loo.branchcodesuggestedby | html %]
>+                                                                [% Branches.GetName(suggestion.suggester.branchcode) | html %]
>                                                             [% END %]
>                                                         </td>
>                                                     [% END %]
>                                                     [% IF Koha.Preference( 'OpacSuggestionManagedBy' ) %]
>                                                     <td>
>-                                                        [% IF ( suggestions_loo.surnamemanagedby ) %]
>+                                                        [% IF suggestion.managedby %]
>                                                             <span class="tdlabel">Managed by:</span>
>-                                                            [% suggestions_loo.surnamemanagedby | html %]
>-                                                            [% IF ( suggestions_loo.firstnamemanagedby ) %]    , [% suggestions_loo.firstnamemanagedby | html %]
>-                                                            [% END %]
>+                                                            [% INCLUDE 'patron-title.inc' patron = suggestion.manager %]
>+                                                            [% IF ( suggestion.manageddate ) %] - [% suggestion.manageddate | $KohaDates %][% END %]
>                                                         [% END %]
>                                                     </td>
>                                                     [% END %]
>                                                     <td>
>                                                         <span class="tdlabel">Status:</span>
>-                                                        [% 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 %]
>                                                     </td>
>                                                 </tr>
>-                                            [% END # / FOREACH suggestions_loo %]
>+                                            [% END # / FOREACH suggestions %]
>                                         </tbody>
>                                     </table>
> 
>@@ -470,7 +470,7 @@
>                                         <p><a class="btn btn-link new" href="/cgi-bin/koha/opac-suggestions.pl?op=add"><i class="fa fa-plus" aria-hidden="true"></i> New purchase suggestion</a></p>
>                                     [% END %]
>                                 [% END %]
>-                            [% END # / IF suggestions_loop %]
>+                            [% END # / IF suggestions.count %]
> 
>                         [% END # IF op_else %]
>                     </div> 
>diff --git a/members/deletemem.pl b/members/deletemem.pl
>index 5faf87dc47..31d930cc00 100755
>--- a/members/deletemem.pl
>+++ b/members/deletemem.pl
>@@ -30,10 +30,10 @@ use Try::Tiny qw( catch try );
> use C4::Context;
> use C4::Output qw( output_and_exit_if_error output_and_exit output_html_with_http_headers );
> use C4::Auth qw( get_template_and_user );
>-use C4::Suggestions;
> use Koha::Patrons;
> use Koha::Token;
> use Koha::Patron::Categories;
>+use Koha::Suggestions;
> 
> my $input = CGI->new;
> 
>@@ -99,11 +99,7 @@ my $countholds = $dbh->selectrow_array("SELECT COUNT(*) FROM reserves WHERE borr
> 
> # Add warning if patron has pending suggestions
> $template->param(
>-    pending_suggestions => scalar @{
>-    C4::Suggestions::SearchSuggestion(
>-            { suggestedby => $member, STATUS => 'ASKED' }
>-        )
>-    }
>+    pending_suggestions => Koha::Suggestions->search({ suggestedby => $member, STATUS => 'ASKED' })->count,
> );
> 
> $template->param(
>diff --git a/members/purchase-suggestions.pl b/members/purchase-suggestions.pl
>index 6a9725115e..0bf5103797 100755
>--- a/members/purchase-suggestions.pl
>+++ b/members/purchase-suggestions.pl
>@@ -23,9 +23,8 @@ use CGI qw ( -utf8 );
> use C4::Auth qw( get_template_and_user );
> use C4::Context;
> use C4::Output qw( output_and_exit_if_error output_and_exit output_html_with_http_headers );
>-use C4::Members;
>-use C4::Suggestions qw( SearchSuggestion );
> use Koha::Patrons;
>+use Koha::Suggestions;
> 
> my $input = CGI->new;
> 
>@@ -49,7 +48,7 @@ $template->param(
>     suggestionsview  => 1,
> );
> 
>-my $suggestions = SearchSuggestion( { suggestedby => $borrowernumber } );
>+my $suggestions = Koha::Suggestions->search_limited( { suggestedby => $borrowernumber }, { prefetch => 'managedby' } );
> 
> $template->param( suggestions => $suggestions );
> 
>diff --git a/opac/opac-suggestions.pl b/opac/opac-suggestions.pl
>index 7c3dae9777..d827e1f0d6 100755
>--- a/opac/opac-suggestions.pl
>+++ b/opac/opac-suggestions.pl
>@@ -28,7 +28,6 @@ use C4::Suggestions qw(
>     DelSuggestion
>     MarcRecordFromNewSuggestion
>     NewSuggestion
>-    SearchSuggestion
> );
> use C4::Koha qw( GetAuthorisedValues );
> use C4::Scrubber;
>@@ -99,36 +98,45 @@ 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
>+};
>+$suggestion->{suggestedby} = $borrowernumber;
>+
> 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);
>@@ -154,7 +162,7 @@ if ( $borrowernumber ){
> }
> 
> if ( $op eq "add_confirm" ) {
>-    my $suggestions_loop = &SearchSuggestion($suggestion);
>+    my $suggestions = Koha::Suggestions->search($suggestion);
>     if ( C4::Context->preference("MaxTotalSuggestions") ne '' && $patrons_total_suggestions_count >= C4::Context->preference("MaxTotalSuggestions") )
>     {
>         push @messages, { type => 'error', code => 'total_suggestions' };
>@@ -163,15 +171,15 @@ if ( $op eq "add_confirm" ) {
>     {
>         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 $s (@$suggestions_loop) {
>+        while ( my $suggestion = $suggestions->next ) {
>             push @messages,
>               {
>                 type => 'error',
>                 code => 'already_exists',
>-                id   => $s->{suggestionid}
>+                id   => $suggestion->suggestionid
>               };
>             last;
>         }
>@@ -196,24 +204,23 @@ if ( $op eq "add_confirm" ) {
>         $patrons_pending_suggestions_count++;
>         $patrons_total_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(
>+my $suggestions = Koha::Suggestions->search_limited(
>     {
>-        suggestedby => $suggestion->{suggestedby},
>-        title       => $title_filter,
>+        $suggestion->{suggestedby}
>+        ? ( suggestedby => $suggestion->{suggestedby} )
>+        : (),
>+        $title_filter
>+        ? ( title       => $title_filter )
>+        : (),
>     }
> );
>+
> if ( $op eq "delete_confirm" ) {
>     my @delete_field = $input->multi_param("delete_field");
>     foreach my $delete_field (@delete_field) {
>@@ -224,24 +231,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", "opac");
> 
> my @mandatoryfields;
>@@ -278,7 +267,7 @@ my @unwantedfields;
> 
> $template->param(
>     %$suggestion,
>-    suggestions_loop      => $suggestions_loop,
>+    suggestions           => $suggestions,
>     patron_reason_loop    => $patron_reason_loop,
>     "op_$op"              => 1,
>     $op                   => 1,
>diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl
>index 5f69087ca0..eaae9ac0d1 100755
>--- a/suggestion/suggestion.pl
>+++ b/suggestion/suggestion.pl
>@@ -92,7 +92,7 @@ my $displayby       = $input->param('displayby') || '';
> my $tabcode         = $input->param('tabcode');
> my $save_confirmed  = $input->param('save_confirmed') || 0;
> my $notify          = $input->param('notify');
>-my $filter_archived = $input->param('filter_archived');
>+my $filter_archived = $input->param('filter_archived') || 0;
> 
> my $reasonsloop     = GetAuthorisedValues("SUGGEST");
> 
>@@ -109,6 +109,12 @@ delete $$suggestion_ref{$_} foreach qw( suggestedbyme op displayby tabcode notif
> foreach (keys %$suggestion_ref){
>     delete $$suggestion_ref{$_} if (!$$suggestion_ref{$_} && ($op eq 'else' ));
> }
>+delete $suggestion_only->{branchcode} if $suggestion_only->{branchcode} eq '__ANY__';
>+delete $suggestion_only->{budgetid}   if $suggestion_only->{budgetid}   eq '__ANY__';
>+while ( my ( $k, $v ) = each %$suggestion_only ) {
>+    delete $suggestion_only->{$k} if $v eq '';
>+}
>+
> my ( $template, $borrowernumber, $cookie, $userflags ) = get_template_and_user(
>         {
>             template_name   => "suggestion/suggestion.tt",
>@@ -213,13 +219,12 @@ if ( $op =~ /save/i ) {
>             }
>         } else {
>             ###FIXME:Search here if suggestion already exists.
>-            my $suggestions_loop =
>-                SearchSuggestion( $suggestion_only );
>-            if (@$suggestions_loop>=1){
>+            my $suggestions= Koha::Suggestions->search_limited( $suggestion_only );
>+            if ( $suggestions->count ) {
>                 #some suggestion are answering the request Donot Add
>                 my @messages;
>-                for my $suggestion ( @$suggestions_loop ) {
>-                    push @messages, { type => 'error', code => 'already_exists', id => $suggestion->{suggestionid} };
>+                while ( my $suggestion = $suggestions->next ) {
>+                    push @messages, { type => 'error', code => 'already_exists', id => $suggestion->suggestionid };
>                 }
>                 $template->param( messages => \@messages );
>             }
>@@ -368,20 +373,38 @@ if ($op=~/else/) {
>         next if ( $definedvalue && $$suggestion_ref{$displayby} ne $criteriumvalue ) and ($displayby ne 'branchcode' && $branchfilter ne '__ANY__' );
>         $$suggestion_ref{$displayby} = $criteriumvalue;
> 
>-        my $suggestions = &SearchSuggestion({ %$suggestion_ref, archived => $filter_archived });
>-        foreach my $suggestion (@$suggestions) {
>-            if ($suggestion->{budgetid}){
>-                my $bud = GetBudget( $suggestion->{budgetid} );
>-                $suggestion->{budget_name} = $bud->{budget_name} if $bud;
>+        # filter on date fields
>+        foreach my $field (qw( suggesteddate manageddate accepteddate )) {
>+            my $from = $field . "_from";
>+            my $to   = $field . "_to";
>+            my $from_dt =
>+              $suggestion_ref->{$from}
>+              ? eval { dt_from_string( $suggestion_ref->{$from} ) }
>+              : undef;
>+            my $to_dt =
>+              $suggestion_ref->{$to}
>+              ? eval { dt_from_string( $suggestion_ref->{$to} ) }
>+              : undef;
>+
>+            if ( $from_dt || $to_dt ) {
>+                my $dtf = Koha::Database->new->schema->storage->datetime_parser;
>+                if ( $from_dt && $to_dt ) {
>+                    $suggestion_ref->{$field} = { -between => [ $from_dt, $to_dt ] };
>+                } elsif ( $from_dt ) {
>+                    $suggestion_ref->{$field} = { '>=' => $from_dt };
>+                } elsif ( $to_dt ) {
>+                    $suggestion_ref->{$field} = { '<=' => $to_dt };
>+                }
>             }
>         }
>+        my $suggestions = Koha::Suggestions->search_limited( { %$suggestion_ref, archived => $filter_archived } );
>+
>         push @allsuggestions,{
>                             "suggestiontype"=>$criteriumvalue||"suggest",
>                             "suggestiontypelabel"=>GetCriteriumDesc($criteriumvalue,$displayby)||"",
>-                            "suggestionscount"=>scalar(@$suggestions),             
>-                            'suggestions_loop'=>$suggestions,
>+                            'suggestions'     => $suggestions,
>                             'reasonsloop'     => $reasonsloop,
>-                            } if @$suggestions;
>+                            } if $suggestions->count;
> 
>         delete $$suggestion_ref{$displayby} unless $definedvalue;
>     }
>-- 
>2.32.0

Actions: View | Diff | Splinter Review

Attachments on bug 23991: 95172 | 115203 | 115207 | 116639 | 118454 | 119256 | 127058 | 127100 | 128071 | 128072 | 128073 | 128074 | 128075 | 129511 | 129512 | 129513 | 129523 | 130049 | 131896 | 132026 | 132169 | 132170 | 132171 | 132172 | 132173 | 132174 | 132175 | 133874 | 134922 | 136441 | 136442 | 137201