From 2d74d926ce54c95f3a638709935f56a90fa7752f 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=utf8 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 Signed-off-by: Brendan A Gallagher --- C4/Members.pm | 44 ----------------- circ/circulation.pl | 26 +++++++--- .../prog/en/modules/circ/circulation.tt | 49 +++++++++---------- .../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 693f5b65a2..a00380965e 100644 --- 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 ea07e29532..3e433b4f19 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 @@ -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 36c44f59c5..21cc29ffe5 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 | $KohaDates %] - [% Branches.GetName( 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 | $KohaDates %] [% Branches.GetName( 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 %] + [% Branches.GetName( lib_messages_loo.branchcode ) %] + "[% message.message.raw %]" + + [% IF message.branchcode == branch OR Koha.Preference('AllowAllMessageDeletion') %] + [Delete] + [% END %] +
    • + [% END %] + [% FOREACH message IN patron_messages %] +
    • [% message.message_date | $KohaDates %] [% Branches.GetName( 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 3a8b01651f..d45222e0c9 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 1d1c4220d7..3a792969e9 100755 --- 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, -- 2.39.5