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>
This commit is contained in:
Jonathan Druart 2016-01-20 18:28:55 +00:00 committed by Brendan A Gallagher
parent 74180472ad
commit 2d74d926ce
5 changed files with 47 additions and 83 deletions

View file

@ -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)

View file

@ -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 $bor_messages_loop = GetMessages( $borrowernumber, 'B', $branch );
if($bor_messages_loop){ $template->param(flagged => 1 ); }
my $patron_messages = Koha::Patron::Messages->search(
{
borrowernumber => $borrowernumber,
message_type => 'B',
}
);
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,

View file

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

View file

@ -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>

View file

@ -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,