Bug 23991: Move SearchSuggestion to Koha::Suggestions
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Thu, 12 May 2022 09:52:45 +0000 (11:52 +0200)
committerTomas Cohen Arazi <tomascohen@theke.io>
Mon, 27 Jun 2022 15:30:28 +0000 (12:30 -0300)
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>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 23991: Remove SearchSuggestion tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 23991: (QA follow-up) Save some DB queries

This patch makes the suggestion-related pages rely on array size instead
of querying the DB each time they need to. In the case of
suggestion/suggestion.pl it goes from 4 COUNT(*) to 1.

To test, with KTD:
1. Run on the host machine:
    $ docker exec -ti koha_db_1 bash
    $ mysql -ppassword
    > SET GLOBAL general_log_file='/var/log/mysql/mycustom.log';
    > SET GLOBAL log_output = 'FILE';
    > SET GLOBAL general_log = 'ON';
    > \q
    $ tail -f /var/log/mysql/mycustom.log | grep suggestions
2. Visit the different pages changed on this bug
=> SUCCESS: Some queries
3. Apply this patch
4. Repeat 2
=> SUCCESS: Less queries!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 23991: Fix branchcode and budgetid filtering

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 23991: Fix conflict with bug 28941

Well, this patchset fixed the security bug...
Redoing on top of bug 28941

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 23991: (follow-up) Missing semicolon

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 23991: Fix 'all' libraries

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 23991: (follow-up) Add value to filter_archived

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
C4/Suggestions.pm
acqui/newordersuggestion.pl
koha-tmpl/intranet-tmpl/prog/en/modules/acqui/newordersuggestion.tt
koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt
koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-suggestions.tt
members/deletemem.pl
members/purchase-suggestions.pl
opac/opac-suggestions.pl
suggestion/suggestion.pl
t/db_dependent/Suggestions.t

index 8818665..2d30913 100644 (file)
@@ -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)
index 0ee1036..9d35ec4 100755 (executable)
@@ -93,10 +93,11 @@ use Modern::Perl;
 use CGI qw ( -utf8 );
 use C4::Auth qw( get_template_and_user );
 use C4::Output qw( output_html_with_http_headers );
-use C4::Suggestions qw( ConnectSuggestionAndBiblio SearchSuggestion );
+use C4::Suggestions qw( ConnectSuggestionAndBiblio );
 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'] },
+)->as_list ];
 
 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,
 );
 
index 0946308..278984d 100644 (file)
@@ -41,7 +41,7 @@
             <main>
 
 <h1>Suggestions</h1>
-    [% IF ( suggestions_loop ) %]
+    [% IF suggestions.size %]
     <a href="#" id="show_only_mine">Show only mine</a> | <a href="#" id="show_all">Show all suggestions</a>
     <table id="suggestionst">
         <thead>
         </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>
         }));
         $("#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();
index fdfbbb1..24da899 100644 (file)
@@ -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.size %]
                   <table id="suggestions">
                     <thead>
                         <tr>
                                 </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 %]
index 8a3fec4..b4cd1a4 100644 (file)
                                     <span>No name</span>
                                 [% END %]
                             [% END %]
-                            ([% suggestion.suggestions_loop.size | html %])
-                            </a>
-                            </li>
+                            ([% suggestion.suggestions.size| html %])
+                            </a></li>
+
                         [% END # /FOREACH suggestion %]
                     </ul> <!-- /.ui-tabs-nav -->
                     <div class="tab-content">
             [% END # /UNLESS notabs %]
 
                 [% FOREACH suggestion IN suggestions %]
-                    <div id="[% suggestion.suggestiontype | html %]" role="tabpanel" class="tab-pane">
+                    <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.size %]
                                 <p>
                                     <a class="checkall" href="#">Check all</a> | <a class="uncheckall" href="#">Uncheck all</a>
                                 </p>
                                         </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>[% AuthorisedValues.GetByCode( 'OPAC_SUG', suggestions_loo.patronreason ) | html %]</td>
+                                                <td>[% AuthorisedValues.GetByCode( 'OPAC_SUG', s.patronreason ) | html %]</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' %]
                                                         <span>Pending</span>
-                                                    [% ELSIF ( suggestions_loo.ACCEPTED ) %]
+                                                    [% ELSIF s.STATUS == 'ACCEPTED' %]
                                                         <span>Accepted</span>
-                                                    [% ELSIF ( suggestions_loo.ORDERED ) %]
+                                                    [% ELSIF s.STATUS == 'ORDERED' %]
                                                         <span>Ordered</span>
-                                                    [% ELSIF ( suggestions_loo.REJECTED ) %]
+                                                    [% ELSIF s.STATUS == 'REJECTED' %]
                                                         <span>Rejected</span>
-                                                    [% ELSIF ( suggestions_loo.CHECKED ) %]
+                                                    [% ELSIF s.STATUS == 'CHECKED' %]
                                                         <span>Checked</span>
-                                                    [% ELSIF ( suggestions_loo.AVAILABLE ) %]
+                                                    [% ELSIF s.STATUS == 'AVAILABLE' %]
                                                         <span>Available</span>
-                                                    [% 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 %]
                                                         <span>Status unknown</span>
                                                     [% 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> <!-- /#table_[% loop.count | html %] -->
 
                                             </fieldset>
                                         </fieldset>
                                     </div> <!-- /.col-sm-2 -->
-                                    <div class="col-sm-2">
-                                        <fieldset>
-                                            <span class="label">Archive selected</span>
-                                            <input type="hidden" name="branchcode" value="[% branchfilter | html %]" />
-                                            <input type="hidden" name="filter_archived" value="[% filter_archived | html %]" />
-                                            <fieldset class="action">
-                                                <button type="submit" class="btn btn-default btn-xs" value="archive">Archive</button>
-                                            </fieldset>
-                                        </fieldset>
-                                    </div> <!-- /.col-sm-2 -->
                                 </div> <!-- /.row -->
 
                             [% ELSE %]
                                         <li>
                                             <label for="archived" style="display: inline;">Include archived:</label>
                                             [% IF filter_archived %]
-                                                <input type="checkbox" id="archived" name="filter_archived" checked="checked" title="Include archived suggestions in the search" />
+                                                <input type="checkbox" id="archived" value="1" name="filter_archived" checked="checked" title="Include archived suggestions in the search" />
                                             [% ELSE %]
-                                                <input type="checkbox" id="archived" name="filter_archived" title="Include archived suggestions in the search" />
+                                                <input type="checkbox" id="archived" value="1" name="filter_archived" title="Include archived suggestions in the search" />
                                             [% END %]
                                         </li>
                                         <li>
                 if( $("#suggestiontabs .tab-pane.active").length < 1 ){
                     $("#suggestiontabs a:first").tab("show");
                 }
+
                 table_settings = [% TablesSettings.GetTableSettings( 'acqui', 'suggestions', 'suggestions', 'json' ) | $raw %]
                 [% FOREACH suggestion IN suggestions %]
-                    [% IF ( suggestion.suggestions_loop ) %]
+                    [% IF suggestion.suggestions.size %]
                         KohaTable("table_[% loop.count| html %]", {
                             "sorting": [[ 3, "asc" ]],
                             "autoWidth": false,
index 0a2a264..cdc7927 100644 (file)
 
                             [% IF ( deleted ) %]<div class="alert alert-info">The selected suggestions have been deleted.</div>[% END %]
 
-                            [% IF suggestions_loop OR title_filter %]
+                            [% IF suggestions.size > 0 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>
                                     </div>
                                 </form>
                             [% END %]
-                            [% IF suggestions_loop %]
+                            [% IF suggestions.size > 0 %]
                                 [% 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" />
                                             </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 ) %]<span>Requested</span>
-                                                        [% ELSIF ( suggestions_loo.CHECKED ) %]<span>Checked by the library</span>
-                                                        [% ELSIF ( suggestions_loo.ACCEPTED ) %]<span>Accepted by the library</span>
-                                                        [% ELSIF ( suggestions_loo.ORDERED ) %]<span>Ordered by the library</span>
-                                                        [% ELSIF ( suggestions_loo.REJECTED ) %]<span>Suggestion declined</span>
-                                                        [% ELSIF ( suggestions_loo.AVAILABLE ) %]<span>Available in the library</span>
-                                                        [% ELSE %] [% AuthorisedValues.GetByCode( 'SUGGEST_STATUS', suggestions_loo.STATUS, 1 ) | html %] [% END %]
-                                                        [% IF ( suggestions_loo.reason ) %]([% suggestions_loo.reason | html %])[% END %]
+                                                        [% IF ( suggestion.STATUS == 'ASKED' ) %]<span>Requested</span>
+                                                        [% ELSIF ( suggestion.STATUS == 'CHECKED' ) %]<span>Checked by the library</span>
+                                                        [% ELSIF ( suggestion.STATUS == 'ACCEPTED' ) %]<span>Accepted by the library</span>
+                                                        [% ELSIF ( suggestion.STATUS == 'ORDERED' ) %]<span>Ordered by the library</span>
+                                                        [% ELSIF ( suggestion.STATUS == 'REJECTED' ) %]<span>Suggestion declined</span>
+                                                        [% ELSIF ( suggestion.STATUS == 'AVAILABLE' ) %]<span>Available in the library</span>
+                                                        [% 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>
 
                                         <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.size %]
 
                         [% END # IF op_else %]
                     </div> <!-- / #usersuggestions -->
index 5faf87d..31d930c 100755 (executable)
@@ -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(
index 6a97251..1739e84 100755 (executable)
@@ -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,10 @@ $template->param(
     suggestionsview  => 1,
 );
 
-my $suggestions = SearchSuggestion( { suggestedby => $borrowernumber } );
+my $suggestions = [
+    Koha::Suggestions->search_limited( { suggestedby => $borrowernumber },
+        { prefetch => 'managedby' } )->as_list
+];
 
 $template->param( suggestions => $suggestions );
 
index fcc9580..b2ab760 100755 (executable)
@@ -28,7 +28,6 @@ use C4::Suggestions qw(
     DelSuggestion
     MarcRecordFromNewSuggestion
     NewSuggestion
-    SearchSuggestion
 );
 use C4::Koha qw( GetAuthorisedValues );
 use C4::Scrubber;
@@ -100,36 +99,43 @@ 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");
     }
 }
 
+$suggestion = {
+    map {
+        my $p = $suggestion->{$_};
+        # Keep parameters that are not an empty string
+        ( defined $p && $p ne '' ? ( $_ => $p ) : () )
+    } keys %$suggestion
+};
+$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);
@@ -155,7 +161,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' };
@@ -164,15 +170,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;
         }
@@ -197,24 +203,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 )
+        : (),
     }
-);
+)->as_list ];
+
 if ( $op eq "delete_confirm" ) {
     my @delete_field = $input->multi_param("delete_field");
     foreach my $delete_field (@delete_field) {
@@ -225,24 +230,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;
@@ -279,7 +266,7 @@ my @unwantedfields;
 
 $template->param(
     %$suggestion,
-    suggestions_loop      => $suggestions_loop,
+    suggestions           => $suggestions,
     patron_reason_loop    => $patron_reason_loop,
     "op_$op"              => 1,
     $op                   => 1,
index 6bea518..c383672 100755 (executable)
@@ -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",
@@ -214,13 +220,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 );
             }
@@ -363,31 +368,65 @@ if ($op=~/else/) {
         unshift @criteria_dv, 'ASKED';
     }
 
+    unless ( exists $suggestion_ref->{branchcode} ) {
+        $suggestion_ref->{branchcode} = C4::Context->userenv->{'branch'};
+    }
+
     my @allsuggestions;
     foreach my $criteriumvalue ( @criteria_dv ) {
+        my $search_params = {%$suggestion_ref};
         # By default, display suggestions from current working branch
-        unless ( exists $$suggestion_ref{'branchcode'} ) {
-            $$suggestion_ref{'branchcode'} = C4::Context->userenv->{'branch'};
-        }
         my $definedvalue = defined $$suggestion_ref{$displayby} && $$suggestion_ref{$displayby} ne "";
 
         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;
+        $search_params->{$displayby} = $criteriumvalue;
+
+        # 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 ) {
+                    $search_params->{$field} = { -between => [ $from_dt, $to_dt ] };
+                } elsif ( $from_dt ) {
+                    $search_params->{$field} = { '>=' => $from_dt };
+                } elsif ( $to_dt ) {
+                    $search_params->{$field} = { '<=' => $to_dt };
+                }
             }
         }
-        push @allsuggestions,{
-                            "suggestiontype"=>$criteriumvalue||"suggest",
-                            "suggestiontypelabel"=>GetCriteriumDesc($criteriumvalue,$displayby)||"",
-                            "suggestionscount"=>scalar(@$suggestions),             
-                            'suggestions_loop'=>$suggestions,
-                            'reasonsloop'     => $reasonsloop,
-                            } if @$suggestions;
+        if ( $search_params->{budgetid} && $search_params->{budgetid} eq '__NONE__' ) {
+            $search_params->{budgetid} = [undef, '' ];
+        }
+        for my $f (qw (branchcode budgetid)) {
+            delete $search_params->{$f}
+              if $search_params->{$f} eq '__ANY__'
+              || $search_params->{$f} eq '';
+        }
+
+        my @suggestions =
+          Koha::Suggestions->search_limited(
+            { %$search_params, archived => $filter_archived } )->as_list;
+
+        push @allsuggestions,
+          {
+            "suggestiontype"      => $criteriumvalue || "suggest",
+            "suggestiontypelabel" => GetCriteriumDesc( $criteriumvalue, $displayby ) || "",
+            'suggestions'         => \@suggestions,
+            'reasonsloop'         => $reasonsloop,
+          }
+          if scalar @suggestions > 0;
 
         delete $$suggestion_ref{$displayby} unless $definedvalue;
     }
index c48b2d5..481dfaa 100755 (executable)
@@ -18,7 +18,7 @@
 use Modern::Perl;
 
 use DateTime::Duration;
-use Test::More tests => 107;
+use Test::More tests => 91;
 use Test::Warn;
 
 use t::lib::Mocks;
@@ -34,7 +34,7 @@ use Koha::Patrons;
 use Koha::Suggestions;
 
 BEGIN {
-    use_ok('C4::Suggestions', qw( NewSuggestion GetSuggestion ModSuggestion GetSuggestionInfo GetSuggestionFromBiblionumber GetSuggestionInfoFromBiblionumber GetSuggestionByStatus ConnectSuggestionAndBiblio SearchSuggestion DelSuggestion MarcRecordFromNewSuggestion GetUnprocessedSuggestions DelSuggestionsOlderThan ));
+    use_ok('C4::Suggestions', qw( NewSuggestion GetSuggestion ModSuggestion GetSuggestionInfo GetSuggestionFromBiblionumber GetSuggestionInfoFromBiblionumber GetSuggestionByStatus ConnectSuggestionAndBiblio DelSuggestion MarcRecordFromNewSuggestion GetUnprocessedSuggestions DelSuggestionsOlderThan ));
 }
 
 my $schema  = Koha::Database->new->schema;
@@ -329,73 +329,6 @@ is( $connect_suggestion_and_biblio, '1', 'ConnectSuggestionAndBiblio returns 1'
 $suggestion = GetSuggestion($my_suggestionid);
 is( $suggestion->{biblionumber}, $biblio_2->biblionumber, 'ConnectSuggestionAndBiblio updates the biblio number correctly' );
 
-my $search_suggestion = SearchSuggestion();
-is( @$search_suggestion, 3, 'SearchSuggestion without arguments returns all suggestions' );
-
-$search_suggestion = SearchSuggestion({
-    title => $mod_suggestion1->{title},
-});
-is( @$search_suggestion, 1, 'SearchSuggestion returns the correct number of suggestions' );
-$search_suggestion = SearchSuggestion({
-    title => 'another title',
-});
-is( @$search_suggestion, 0, 'SearchSuggestion returns the correct number of suggestions' );
-
-$search_suggestion = SearchSuggestion({
-    author => $mod_suggestion1->{author},
-});
-is( @$search_suggestion, 1, 'SearchSuggestion returns the correct number of suggestions' );
-$search_suggestion = SearchSuggestion({
-    author => 'another author',
-});
-is( @$search_suggestion, 0, 'SearchSuggestion returns the correct number of suggestions' );
-
-$search_suggestion = SearchSuggestion({
-    publishercode => $mod_suggestion1->{publishercode},
-});
-is( @$search_suggestion, 1, 'SearchSuggestion returns the correct number of suggestions' );
-$search_suggestion = SearchSuggestion({
-    publishercode => 'another publishercode',
-});
-is( @$search_suggestion, 0, 'SearchSuggestion returns the correct number of suggestions' );
-
-$search_suggestion = SearchSuggestion({
-    STATUS => $mod_suggestion3->{STATUS},
-});
-is( @$search_suggestion, 2, 'SearchSuggestion returns the correct number of suggestions' );
-
-$search_suggestion = SearchSuggestion({
-    STATUS => q||
-});
-is( @$search_suggestion, 0, 'SearchSuggestion should not return all suggestions if we want the suggestions with a STATUS=""' );
-$search_suggestion = SearchSuggestion({
-    STATUS => 'REJECTED',
-});
-is( @$search_suggestion, 0, 'SearchSuggestion returns the correct number of suggestions' );
-
-$search_suggestion = SearchSuggestion({
-    budgetid => '',
-});
-is( @$search_suggestion, 3, 'SearchSuggestion (budgetid = "") returns the correct number of suggestions' );
-$search_suggestion = SearchSuggestion({
-    budgetid => $budget_id,
-});
-is( @$search_suggestion, 2, 'SearchSuggestion (budgetid = $budgetid) returns the correct number of suggestions' );
-$search_suggestion = SearchSuggestion({
-    budgetid => '__NONE__',
-});
-is( @$search_suggestion, 1, 'SearchSuggestion (budgetid = "__NONE__") returns the correct number of suggestions' );
-$search_suggestion = SearchSuggestion({
-    budgetid => '__ANY__',
-});
-is( @$search_suggestion, 3, 'SearchSuggestion (budgetid = "__ANY__") returns the correct number of suggestions' );
-
-$search_suggestion = SearchSuggestion({ budgetid => $budget_id });
-is( @$search_suggestion[0]->{budget_name}, GetBudget($budget_id)->{budget_name}, 'SearchSuggestion returns the correct budget name');
-$search_suggestion = SearchSuggestion({ budgetid => "__NONE__" });
-is( @$search_suggestion[0]->{budget_name}, undef, 'SearchSuggestion returns the correct budget name');
-
-
 my $del_suggestion = {
     title => 'my deleted title',
     STATUS => 'CHECKED',