Bug 15632: Koha::Patron::Messages - Remove GetMessages

This subroutine just retrieved the messages given some parameters.
Some job should not have been done in this subroutine.
It was called only 3 times, in circ/circulation.pl and opac-user.pl.
Basically it was used to retrieved the message to displaye for a given
patron ($borrowernumber) at the OPAC (B) or Staff (L).

For the 3 calls, the 2 parameters $borrowernumber and $type
(message_type) were passed, the "%" trick at the beginning of the
subroutine was useless.
Moreover, the date formatting should be done on the TT side, not in
subroutine.
The can_delete flag was set if the branchcode given in parameter was the
same as the one of the message. This has been delegated to the template.
Indeed the can_delete was not valid, since it must depend on the
AllowAllMessageDeletion pref.
The test is now:
  IF message.branchcode == branch OR
  Koha.Preference('AllowAllMessageDeletion'')

There is not specific test plan for this patch, the changes have already
been tested in previous patches.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

diff --git a/C4/Members.pm b/C4/Members.pm
index 693f5b6..a003809 100644 (file)
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -96,8 +96,6 @@ BEGIN {
         &GetExpiryDate
         &GetUpcomingMembershipExpires
 
-        &GetMessages
-
         &IssueSlip
         GetBorrowersWithEmail
 
@@ -2131,48 +2129,6 @@ sub ModPrivacy {
                       privacy        => $privacy );
 }
 
-=head2 GetMessages
-
-  GetMessages( $borrowernumber, $type );
-
-$type is message type, B for borrower, or L for Librarian.
-Empty type returns all messages of any type.
-
-Returns all messages for the given borrowernumber
-
-=cut
-
-sub GetMessages {
-    my ( $borrowernumber, $type, $branchcode ) = @_;
-
-    if ( ! $type ) {
-      $type = '%';
-    }
-
-    my $dbh  = C4::Context->dbh;
-
-    my $query = "SELECT
-                  branches.branchname,
-                  messages.*,
-                  message_date,
-                  messages.branchcode LIKE '$branchcode' AS can_delete
-                  FROM messages, branches
-                  WHERE borrowernumber = ?
-                  AND message_type LIKE ?
-                  AND messages.branchcode = branches.branchcode
-                  ORDER BY message_date DESC";
-    my $sth = $dbh->prepare($query);
-    $sth->execute( $borrowernumber, $type ) ;
-    my @results;
-
-    while ( my $data = $sth->fetchrow_hashref ) {
-        $data->{message_date_formatted} = output_pref( { dt => dt_from_string( $data->{message_date} ), dateonly => 1, dateformat => 'iso' } );
-        push @results, $data;
-    }
-    return \@results;
-
-}
-
 =head2 IssueSlip
 
   IssueSlip($branchcode, $borrowernumber, $quickslip)

diff --git a/circ/circulation.pl b/circ/circulation.pl
index ea07e29..3e433b4 100755 (executable)
--- a/circ/circulation.pl
+++ b/circ/circulation.pl
@@ -46,6 +46,7 @@ use C4::Members::Attributes qw(GetBorrowerAttributes);
 use Koha::Borrower::Debarments qw(GetDebarments IsDebarred);
 use Koha::DateUtils;
 use Koha::Database;
+use Koha::Patron::Messages;
 
 use Date::Calc qw(
   Today
@@ -546,11 +547,23 @@ if ( $borrowernumber && $borrower->{'category_type'} eq 'C') {
     $template->param( 'catcode' =>    $catcodes->[0])  if $cnt == 1;
 }
 
-my $lib_messages_loop = GetMessages( $borrowernumber, 'L', $branch );
-if($lib_messages_loop){ $template->param(flagged => 1 ); }
+my $librarian_messages = Koha::Patron::Messages->search(
+    {
+        borrowernumber => $borrowernumber,
+        message_type => 'L',
+    }
+);
+
+my $patron_messages = Koha::Patron::Messages->search(
+    {
+        borrowernumber => $borrowernumber,
+        message_type => 'B',
+    }
+);
 
-my $bor_messages_loop = GetMessages( $borrowernumber, 'B', $branch );
-if($bor_messages_loop){ $template->param(flagged => 1 ); }
+if( $librarian_messages->count or $patron_messages->count ) {
+    $template->param(flagged => 1)
+}
 
 my $fast_cataloging = 0;
 if (defined getframeworkinfo('FA')) {
@@ -589,9 +602,8 @@ if ($restoreduedatespec || $stickyduedate) {
 }
 
 $template->param(
-    lib_messages_loop => $lib_messages_loop,
-    bor_messages_loop => $bor_messages_loop,
-    all_messages_del  => C4::Context->preference('AllowAllMessageDeletion'),
+    librarian_messages => $librarian_messages,
+    patron_messages   => $patron_messages,
     findborrower      => $findborrower,
     borrower          => $borrower,
     borrowernumber    => $borrowernumber,

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
index 36c44f5..21cc29f 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
@@ -838,35 +838,30 @@ No patron matched <span class="ex">[% message %]</span>
 
     <!-- /If notes -->[% END %]
 
-       <div id="messages" class="circmessage">
-               <h4>Messages:</h4>
-               <ul>
-                       [% FOREACH lib_messages_loo IN lib_messages_loop %]
-                               <li>
-                                       <span class="circ-hlt">
-                                                [% lib_messages_loo.message_date_formatted  | $KohaDates %]
-                                                [% Branches.GetName( lib_messages_loo.branchcode ) %]
-                                               <i>"[% lib_messages_loo.message %]"</i>
-                                       </span>
-                                       [% IF ( lib_messages_loo.can_delete ) %]
-                                               <a href="/cgi-bin/koha/circ/del_message.pl?message_id=[% lib_messages_loo.message_id %]&amp;borrowernumber=[% lib_messages_loo.borrowernumber %]">[Delete]</a>
-                                       [% ELSE %]
-                                               [% IF ( all_messages_del ) %]
-                                                       <a href="/cgi-bin/koha/circ/del_message.pl?message_id=[% lib_messages_loo.message_id %]&amp;borrowernumber=[% lib_messages_loo.borrowernumber %]">[Delete]</a>
-                                               [% END %]
-                                       [% END %]
-                               </li>
-                       [% END %]
-                       [% FOREACH bor_messages_loo IN bor_messages_loop %]
-                            <li><span class="">[% bor_messages_loo.message_date_formatted  | $KohaDates %] [% Branches.GetName( bor_messages_loo.branchcode ) %] <i>"[% bor_messages_loo.message %]"</i></span> [% IF ( bor_messages_loo.can_delete ) %]<a href="/cgi-bin/koha/circ/del_message.pl?message_id=[% bor_messages_loo.message_id %]&amp;borrowernumber=[% bor_messages_loo.borrowernumber %]">[Delete]</a>
-                [% ELSIF ( all_messages_del ) %]
-                    <a href="/cgi-bin/koha/circ/del_message.pl?message_id=[% bor_messages_loo.message_id %]&amp;borrowernumber=[% bor_messages_loo.borrowernumber %]">[Delete]</a>
+    <div id="messages" class="circmessage">
+        <h4>Messages:</h4>
+        <ul>
+            [% FOREACH message IN librarian_messages %]
+                <li>
+                    <span class="circ-hlt">
+                        [% message.message_date | $KohaDates %]
+                        [% Branches.GetName( lib_messages_loo.branchcode ) %]
+                        <i>"[% message.message.raw %]"</i>
+                    </span>
+                    [% IF message.branchcode == branch OR Koha.Preference('AllowAllMessageDeletion') %]
+                        <a href="/cgi-bin/koha/circ/del_message.pl?message_id=[% message.message_id %]&amp;borrowernumber=[% message.borrowernumber %]">[Delete]</a>
+                    [% END %]
+                </li>
+            [% END %]
+            [% FOREACH message IN patron_messages %]
+                <li><span class="">[% message.message_date | $KohaDates %] [% Branches.GetName( message.branchcode )%] <i>"[% message.message.raw %]"</i></span>
+                [% IF message.branchcode == branch OR Koha.Preference('AllowAllMessageDeletion') %]
+                    <a href="/cgi-bin/koha/circ/del_message.pl?message_id=[% message.message_id %]&amp;borrowernumber=[% message.borrowernumber %]">[Delete]</a>
                 [% END %]</li>
-                       [% END %]
+            [% END %]
+        </ul>
+    </div>
 
-               </ul>
-       </div>  
-       
      <!-- /If flagged -->[% END %]
 
        

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
index 3a8b016..d45222e 100644 (file)
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
@@ -1,5 +1,6 @@
 [% USE Koha %]
 [% USE KohaDates %]
+[% USE Branches %]
 
 [% INCLUDE 'doc-head-open.inc' %]
 <title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog &rsaquo; Your library home</title>
@@ -29,17 +30,17 @@
                         <div class="alert alert-info">
                             <h3>Messages for you</h3>
                                 <ul>
-                                    [% FOREACH bor_messages_loo IN bor_messages_loop %]
+                                    [% FOREACH message IN patron_messages %]
                                         <li>
-                                        <strong>[% bor_messages_loo.message %]</strong><br>
-                                        &nbsp;&nbsp;&nbsp;<i>Written on [% bor_messages_loo.message_date | $KohaDates %] by [% bor_messages_loo.branchname %]</i>
+                                        <strong>[% message.message %]</strong><br>
+                                        &nbsp;&nbsp;&nbsp;<i>Written on [% message.message_date | $KohaDates %] by [% Branches.GetName(message.branchcode) %]</i>
                                         </li>
                                     [% END %]
 
                                     [% IF ( opacnote ) %]<li>[% opacnote %]</li>[% END %]
                                 </ul>
                         </div>
-                    [% END # / IF bor_messages %]
+                    [% END %]
                     <h2>Hello, [% INCLUDE 'patron-title.inc' category_type = BORROWER_INFO.category_type firstname = BORROWER_INFO.firstname surname = BORROWER_INFO.surname othernames = BORROWER_INFO.othernames cardnumber = BORROWER_INFO.cardnumber %]
                     </h2>
 

diff --git a/opac/opac-user.pl b/opac/opac-user.pl
index 1d1c422..3a79296 100755 (executable)
--- a/opac/opac-user.pl
+++ b/opac/opac-user.pl
@@ -346,7 +346,7 @@ if (   C4::Context->preference('AllowPatronToSetCheckoutsVisibilityForGuarantor'
 
 $template->param(
     borrower                 => $borr,
-    bor_messages_loop        => GetMessages( $borrowernumber, 'B', 'NONE' ),
+    patron_messages          => $patron_messages,
     patronupdate             => $patronupdate,
     OpacRenewalAllowed       => C4::Context->preference("OpacRenewalAllowed"),
     userview                 => 1,