From d7b29186943a2d0ba23f4acb3ca3dd2842b13211 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 20 Jan 2016 18:28:55 +0000 Subject: [PATCH] Bug 15632: Koha::Patron::Messages - Remove GetMessages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- C4/Members.pm | 44 ------------------- circ/circulation.pl | 26 ++++++++---- .../prog/en/modules/circ/circulation.tt | 49 ++++++++++------------ .../opac-tmpl/bootstrap/en/modules/opac-user.tt | 9 ++-- opac/opac-user.pl | 2 +- 5 files changed, 47 insertions(+), 83 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 518888e..0ece73f 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -99,8 +99,6 @@ BEGIN { &GetExpiryDate &GetUpcomingMembershipExpires - &GetMessages - &IssueSlip GetBorrowersWithEmail @@ -2188,48 +2186,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 e6ba9a2..83a9d9e 100755 --- 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 @@ -555,11 +556,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')) { @@ -598,9 +611,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 6fd722c..6fbb6ae 100644 --- 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 [% message %] [% END %] -
-

Messages:

-
    - [% FOREACH lib_messages_loo IN lib_messages_loop %] -
  • - - [% lib_messages_loo.message_date_formatted %] - [% lib_messages_loo.branchcode %] - "[% lib_messages_loo.message %]" - - [% IF ( lib_messages_loo.can_delete ) %] - [Delete] - [% ELSE %] - [% IF ( all_messages_del ) %] - [Delete] - [% END %] - [% END %] -
  • - [% END %] - [% FOREACH bor_messages_loo IN bor_messages_loop %] -
  • [% bor_messages_loo.message_date_formatted %] [% bor_messages_loo.branchcode %] "[% bor_messages_loo.message %]" [% IF ( bor_messages_loo.can_delete ) %][Delete] - [% ELSIF ( all_messages_del ) %] - [Delete] +
    +

    Messages:

    +
      + [% FOREACH message IN librarian_messages %] +
    • + + [% message.message_date | $KohaDates %] + [% message.branchcode %] + "[% message.message.raw %]" + + [% IF message.branchcode == branch OR Koha.Preference('AllowAllMessageDeletion') %] + [Delete] + [% END %] +
    • + [% END %] + [% FOREACH message IN patron_messages %] +
    • [% message.message_date | $KohaDates %] [% message.branchcode %] "[% message.message.raw %]" + [% IF message.branchcode == branch OR Koha.Preference('AllowAllMessageDeletion') %] + [Delete] [% END %]
    • - [% END %] + [% END %] +
    +
    -
-
- [% 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 --- 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' %] [% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog › Your library home @@ -29,17 +30,17 @@

Messages for you

    - [% FOREACH bor_messages_loo IN bor_messages_loop %] + [% FOREACH message IN patron_messages %]
  • - [% bor_messages_loo.message %]
    -    Written on [% bor_messages_loo.message_date | $KohaDates %] by [% bor_messages_loo.branchname %] + [% message.message %]
    +    Written on [% message.message_date | $KohaDates %] by [% Branches.GetName(message.branchcode) %]
  • [% END %] [% IF ( opacnote ) %]
  • [% opacnote %]
  • [% END %]
- [% END # / IF bor_messages %] + [% END %]

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 %]

diff --git a/opac/opac-user.pl b/opac/opac-user.pl index 5b629e3..b42697b 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -341,7 +341,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, -- 2.1.4