From eb7e9fad53f45130dd374a30f108a8073e7e3859 Mon Sep 17 00:00:00 2001 From: Josef Moravec Date: Fri, 2 Nov 2018 17:40:45 +0000 Subject: [PATCH] Bug 21757: Clenup of moremember.pl and its templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch: - removes unused templates - use objects as much as possible - remove many template params Test plan: 1) Apply the patch 2) Play with patron detail page and try to broke it anyhow ;) - messaging preferences - enhanced attributes - guarantors and guarantees - fines - messages - checkouts - overdues - use different date formats, price formats and address formats - ... there is many thinks you could try with this one page ;) Signed-off-by: Séverine QUEUNE Signed-off-by: Séverine QUEUNE --- .../prog/en/includes/blocked-fines.inc | 4 +- .../prog/en/includes/checkouts-table-footer.inc | 10 +- .../prog/en/modules/members/moremember-brief.tt | 90 ----------- .../prog/en/modules/members/moremember-receipt.tt | 78 --------- .../prog/en/modules/members/moremember.tt | 37 ++--- members/moremember.pl | 174 ++------------------- 6 files changed, 42 insertions(+), 351 deletions(-) delete mode 100644 koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-brief.tt delete mode 100644 koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-receipt.tt diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/blocked-fines.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/blocked-fines.inc index 80b0514..4bba7f7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/blocked-fines.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/blocked-fines.inc @@ -7,7 +7,7 @@ [% IF !Koha.Preference('AllowFineOverride') && NoIssuesCharge && fines > NoIssuesCharge %] Checkouts are BLOCKED because fine balance is OVER THE LIMIT. [% END %] - Make payment - Pay all fines + Make payment + Pay all fines [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/checkouts-table-footer.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/checkouts-table-footer.inc index 40d2195..d605ad0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/checkouts-table-footer.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/checkouts-table-footer.inc @@ -1,11 +1,11 @@ - Totals: - [% totaldue | html %] - [% finetotal | html %] - [% totalprice | html %] + Totals: + + +
-

+

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-brief.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-brief.tt deleted file mode 100644 index dab14e5..0000000 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-brief.tt +++ /dev/null @@ -1,90 +0,0 @@ -[% USE Koha %] -[% USE KohaDates %] -[% SET footerjs = 1 %] -[% INCLUDE 'doc-head-open.inc' %] -Koha › Check duplicate patron -[% INCLUDE 'doc-head-close.inc' %] - - - -
-
- -
-

[% UNLESS ( I ) %] - [% title | html %] [% firstname | html %] [% END %] [% surname | html %] ([% cardnumber | html %])

-
-
-
-
-
- - [% UNLESS ( I ) %][% IF ( othernames ) %]“[% othernames | html %]”[% END %] -
-
    - [% IF Koha.Preference( 'AddressFormat' ) %] - [% INCLUDE "member-display-address-style-${ Koha.Preference( 'AddressFormat' ) }.inc" %] - [% ELSE %] - [% INCLUDE 'member-display-address-style-us.inc' %] - [% END %] -
-
-
-
    - [% IF ( phone ) %]
  1. Primary phone: [% phone | html %]
  2. [% END %] - [% IF ( phonepro ) %]
  3. Secondary phone: [% phonepro | html %]
  4. [% END %] - [% IF ( mobile ) %]
  5. Other phone: [% mobile | html %]
  6. [% END %] - [% IF ( fax ) %]
  7. Fax: [% fax | html %]
  8. [% END %] - [% IF ( email ) %][% END %] - [% IF ( emailpro ) %][% END %] - [% UNLESS ( I ) %] - [% IF ( inititals ) %]
  9. Initials: [% initials | html %]
  10. [% END %] - [% IF ( dateofbirth ) %]
  11. Date of birth:[% dateofbirth | $KohaDates %]
  12. [% END %] - [% IF ( sex ) %]
  13. Gender:[% IF ( sex == 'F' ) %]Female[% ELSIF ( sex == 'M' ) %]Male[% ELSE %][% sex | html %][% END %]
  14. [% END %][% END %] - [% END %] - [% IF ( isguarantee ) %] - [% IF ( guaranteeloop ) %] -
  15. Guarantees:
  16. - [% END %] - [% ELSE %] - [% IF ( guarantor.borrowernumber ) %] -
  17. Guarantor:[% guarantor.surname | html %], [% guarantor.firstname | html %]
  18. - [% END %] - [% END %] -
-
-
-
- -
-
-

Library use

-
-
    -
  1. Card number: [% cardnumber | html %]
  2. -
  3. Borrowernumber: [% borrowernumber | html %]
  4. -
  5. Category: [% categoryname | html %] ([% categorycode | html %])
  6. -
  7. Registration date: [% dateenrolled | $KohaDates %]
  8. -
  9. Expiration date: - [% IF ( was_renewed ) %] - [% dateexpiry | $KohaDates %] - [% ELSE %] - [% dateexpiry | $KohaDates %] - [% END %] -
  10. -
  11. Library: [% branchname | html %]
  12. - - [% IF ( sort1 ) %]
  13. Sort field 1:[% lib1 | html %]
  14. [% END %] - [% IF ( sort2 ) %]
  15. Sort field 2:[% lib2 | html %]
  16. [% END %] -
-
-
-
-
-
-
-
-
-
- -[% INCLUDE 'intranet-bottom.inc' popup_window=1 %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-receipt.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-receipt.tt deleted file mode 100644 index c645c55..0000000 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-receipt.tt +++ /dev/null @@ -1,78 +0,0 @@ -[% USE raw %] -[% USE Asset %] -[% USE Koha %] -[% SET footerjs = 1 %] -[% INCLUDE 'doc-head-open.inc' %] -Print Receipt for [% cardnumber | html %] -[% INCLUDE 'doc-head-close.inc' %] - - -[% Asset.css("css/print.css") | $raw %] - - - - -
- -

[% LibraryName | html %]

-[% IF ( branchname ) %][% branchname | html %]
[% END %] -Checked out to [% firstname | html %] [% surname | html %]
-([% cardnumber | html %])
- -[% todaysdate | html %]
- -[% IF ( quickslip ) %] -

Checked out today

-[% FOREACH issueloo IN issueloop %] -[% IF ( issueloo.red ) %][% ELSE %] -[% IF ( issueloo.today ) %] -

[% issueloo.title | html %]
-Barcode: [% issueloo.barcode | html %]
-Date due: [% issueloo.date_due | html %]

- [% END %] - [% END %] - [% END %] - -[% ELSE %] -

Checked out

-[% FOREACH issueloo IN issueloop %] -[% IF ( issueloo.red ) %][% ELSE %] -

[% issueloo.title | html %]
-Barcode: [% issueloo.barcode | html %]
-Date due: [% issueloo.date_due | html %]

- [% END %] - [% END %] - -[% END %] - -[% IF ( quickslip ) %] -[% ELSE %] -[% IF ( overdues_exist ) %] -

Overdues

- [% FOREACH issueloo IN issueloop %] - [% IF ( issueloo.red ) %] -

[% issueloo.title | html %]
-Barcode: [% issueloo.barcode | html %]
-Date due: [% issueloo.date_due | html %]

-[% END %] -[% END %] -[% END %] -[% END %] - -[% IF ( koha_news_count ) %] -

News

- - [% FOREACH koha_new IN koha_news %] -
[% koha_new.title | html %]
-

[% koha_new.content | $raw %]

-

Posted on [% koha_new.newdate | html %] - -


- [% END %] -[% END %] - -[% MACRO jsinclude BLOCK %] - [% INCLUDE 'slip-print.inc' #printThenClose %] -[% END %] - -[% INCLUDE 'intranet-bottom.inc' %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt index d64be13..f34a763 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -5,6 +5,7 @@ [% USE KohaDates %] [% USE AuthorisedValues %] [% USE ColumnsSettings %] +[% USE Price %] [% SET footerjs = 1 %] [% INCLUDE 'doc-head-open.inc' %] Koha › Patrons › @@ -95,7 +96,7 @@ [% END %] <i>"[% patron_message.message | html %]"</i> </span> - [% IF patron_message.branchcode == branchcode OR Koha.Preference('AllowAllMessageDeletion') %] + [% IF patron_message.branchcode == patron.branchcode OR Koha.Preference('AllowAllMessageDeletion') %] <a class="btn btn-link btn-sm" href="/cgi-bin/koha/circ/del_message.pl?message_id=[% patron_message.message_id | html %]&borrowernumber=[% patron_message.borrowernumber | html %]&from=moremember" onclick="return confirm(MSG_CONFIRM_DELETE_MESSAGE);"><i class="fa fa-trash"></i> Delete</a> [% END %] </li> @@ -108,18 +109,18 @@ [% IF ( flagged ) %] <div id="circmessages" class="circmessage attention"> <ul> - [% IF ( userdebarred ) %] + [% IF ( patron.is_debarred ) %] <li class="blocker">Patron's account is restricted [% IF ( userdebarreddate ) %] - until [% userdebarreddate | html %] + until [% userdebarreddate | $KohaDates %] [% END %] - [% IF ( debarredcomment ) %] + [% IF ( patron.debarredcomment ) %] with the explanation: <i> - [% IF debarredcomment.search('OVERDUES_PROCESS') %] - Restriction added by overdues process [% debarredcomment.remove('OVERDUES_PROCESS ') | html_line_break %] + [% IF patron.debarredcomment.search('OVERDUES_PROCESS') %] + Restriction added by overdues process [% patron.debarredcomment.remove('OVERDUES_PROCESS ') | html_line_break %] [% ELSE %] - [% debarredcomment | html_line_break %] + [% patron.debarredcomment | html_line_break %] [% END %] </i> [% END %] @@ -129,7 +130,7 @@ [% IF ( patron.gonenoaddress ) %] <li class="blocker">Patron's address is in doubt.</li> [% END %] - [% IF ( lost ) %] + [% IF ( patron.lost ) %] <li class="blocker">Patron's card has been reported lost.</li> [% END %] [% IF ( patron.is_expired ) %] @@ -232,7 +233,7 @@ [% IF ( patron.dateofbirth ) %] <li> <span class="label">Date of birth:</span> - [% patron.dateofbirth | $KohaDates %] ([% age | html %] years) + [% patron.dateofbirth | $KohaDates %] ([% patron.get_age | html %] years) </li> [% END %] [% IF ( patron.sex ) %] @@ -393,7 +394,7 @@ <a class="btn btn-default btn-xs" href="memberentry.pl?op=modify&borrowernumber=[% patron.borrowernumber | html %]&step=5"><i class="fa fa-pencil"></i> Edit</a> </div> [% INCLUDE 'messaging-preference-form.inc' %] - [% IF ( SMSSendDriver ) %] + [% IF Koha.Preference('SMSSendDriver') %] <div class="rows"> <ol> <li> @@ -460,15 +461,15 @@ </li> <li id="patron-branchname"> <span class="label">Library: </span> - [% branchname | html %] + [% Branches.GetName( patron.branchcode ) | html %] </li> - [% IF ( OPACPrivacy ) %] + [% IF Koha.Preference( 'OPACPrivacy') %] <li id="patron-privacypref"> <span class="label">Privacy Pref:</span> - [% IF ( privacy0 ) %]Forever[% END %] - [% IF ( privacy1 ) %]Default[% END %] - [% IF ( privacy2 ) %]Never[% END %] + [% IF ( patron.privacy == 0 ) %]Forever[% END %] + [% IF ( patron.privacy == 1 ) %]Default[% END %] + [% IF ( patron.privacy == 2 ) %]Never[% END %] </li> [% END %] @@ -672,7 +673,7 @@ </li> [% IF relatives_issues_count %] <li> - <a href="#relatives-issues" id="relatives-issues-tab">Relatives' checkouts</a> + <a href="#relatives-issues" id="relatives-issues-tab">[% relatives_issues_count | html %] Relatives' checkouts</a> </li> [% END %] <li> @@ -732,8 +733,8 @@ [% END %] <div id="finesandcharges"> - [% IF ( totaldue_raw ) %] - <p>Total due: [% totaldue | html %]</p> + [% IF ( fines ) %] + <p>Total due: [% fines | $Price %]</p> [% ELSE %] <p>No outstanding charges</p> [% END %] diff --git a/members/moremember.pl b/members/moremember.pl index be365a3..d68e6ef 100755 --- a/members/moremember.pl +++ b/members/moremember.pl @@ -22,98 +22,48 @@ =head1 moremember.pl - script to do a borrower enquiry/bring up borrower details etc - Displays all the details about a borrower - written 20/12/99 by chris@katipo.co.nz - last modified 21/1/2000 by chris@katipo.co.nz - modified 31/1/2001 by chris@katipo.co.nz - to not allow items on request to be renewed - - needs html removed and to use the C4::Output more, but its tricky + script to do a borrower enquiry/bring up patron details etc + Displays all the details about a patron =cut use Modern::Perl; use CGI qw ( -utf8 ); -use HTML::Entities; use C4::Context; use C4::Auth; use C4::Output; -use C4::Members; -use C4::Members::Attributes; use C4::Members::AttributeTypes; -use C4::Reserves; -use C4::Circulation; -use C4::Koha; -use C4::Letters; -use C4::Biblio; use C4::Form::MessagingPreferences; use List::MoreUtils qw/uniq/; use C4::Members::Attributes qw(GetBorrowerAttributes); -use Koha::Account::Lines; -use Koha::AuthorisedValues; -use Koha::CsvProfiles; use Koha::Patron::Debarments qw(GetDebarments); use Koha::Patron::Messages; -#use Smart::Comments; -#use Data::Dumper; -use DateTime; use Koha::DateUtils; -use Koha::Database; +use Koha::CsvProfiles; use Koha::Patrons; -use Koha::Patron::Categories; use Koha::Token; +use Koha::Checkouts; use vars qw($debug); BEGIN { - $debug = $ENV{DEBUG} || 0; + $debug = $ENV{DEBUG} || 0; } -my $dbh = C4::Context->dbh; - my $input = CGI->new; $debug or $debug = $input->param('debug') || 0; -my $print = $input->param('print'); - -my $template_name; -my $quickslip = 0; - -my $flagsrequired; -if (defined $print and $print eq "page") { - $template_name = "members/moremember-print.tt"; - # circ staff who process checkouts but can't edit - # patrons still need to be able to access print view - $flagsrequired = { circulate => "circulate_remaining_permissions" }; -} elsif (defined $print and $print eq "slip") { - $template_name = "members/moremember-receipt.tt"; - # circ staff who process checkouts but can't edit - # patrons still need to be able to print receipts - $flagsrequired = { circulate => "circulate_remaining_permissions" }; -} elsif (defined $print and $print eq "qslip") { - $template_name = "members/moremember-receipt.tt"; - $quickslip = 1; - $flagsrequired = { circulate => "circulate_remaining_permissions" }; -} elsif (defined $print and $print eq "brief") { - $template_name = "members/moremember-brief.tt"; - $flagsrequired = { borrowers => 'edit_borrowers' }; -} else { - $template_name = "members/moremember.tt"; - $flagsrequired = { borrowers => 'edit_borrowers' }; -} my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { - template_name => $template_name, + template_name => "members/moremember.tt", query => $input, type => "intranet", authnotrequired => 0, - flagsrequired => $flagsrequired, + flagsrequired => { borrowers => 'edit_borrowers' }, debug => 1, } ); my $borrowernumber = $input->param('borrowernumber'); -$borrowernumber = HTML::Entities::encode($borrowernumber); my $error = $input->param('error'); $template->param( error => $error ) if ( $error ); @@ -121,46 +71,21 @@ my $patron = Koha::Patrons->find( $borrowernumber ); my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); -my $issues = $patron->checkouts; -my $balance = $patron->account->balance; -$template->param( - issuecount => $issues->count, - fines => $balance, -); - my $category_type = $patron->category->category_type; -my $data = $patron->unblessed; - -$debug and printf STDERR "dates (enrolled,expiry,birthdate) raw: (%s, %s, %s)\n", map {$data->{$_}} qw(dateenrolled dateexpiry dateofbirth); -foreach (qw(dateenrolled dateexpiry dateofbirth)) { # FIXME This should be removed - my $userdate = $data->{$_}; - unless ($userdate) { - $debug and warn sprintf "Empty \$data{%12s}", $_; - $data->{$_} = ''; - next; - } - $data->{$_} = dt_from_string( $userdate ); -} -for (qw(gonenoaddress lost borrowernotes)) { - $data->{$_} and $template->param(flagged => 1) and last; +for (qw(gonenoaddress lost borrowernotes is_debarred)) { + $patron->$_ and $template->param(flagged => 1) and last; } if ( $patron->is_debarred ) { $template->param( - userdebarred => 1, # FIXME Template should use patron->is_debarred - flagged => 1, debarments => scalar GetDebarments({ borrowernumber => $borrowernumber }), ); - my $debar = $data->{'debarred'}; - if ( $debar ne "9999-12-31" ) { - $template->param( 'userdebarreddate' => output_pref( { dt => dt_from_string( $debar ), dateonly => 1 } ) ); - $template->param( 'debarredcomment' => $data->{debarredcomment} ); + if ( $patron->debarred ne "9999-12-31" ) { + $template->param( 'userdebarreddate' => $patron->debarred ); } } -$data->{ "sex_".$data->{'sex'}."_p" } = 1 if defined $data->{sex}; - my @relatives; if ( my $guarantor = $patron->guarantor ) { $template->param( guarantor => $guarantor ); @@ -180,71 +105,13 @@ if ( my $guarantor = $patron->guarantor ) { } my $relatives_issues_count = - Koha::Database->new()->schema()->resultset('Issue') - ->count( { borrowernumber => \@relatives } ); - -my %bor; -$bor{'borrowernumber'} = $borrowernumber; - -# Converts the branchcode to the branch name -my $samebranch; -if ( C4::Context->preference("IndependentBranches") ) { - if ( C4::Context->IsSuperLibrarian() ) { - $samebranch = 1; - } - else { - my $userenv = C4::Context->userenv; - $samebranch = ( $data->{'branchcode'} eq $userenv->{branch} ); - } -} -else { - $samebranch = 1; -} -my $library = Koha::Libraries->find( $data->{branchcode})->unblessed; -@{$data}{keys %$library} = values %$library; # merge in all branch columns # FIXME This is really ugly, we should pass the library instead - -# If printing a page, send the account informations to the template -if (defined $print and $print eq "page") { - my $accts = Koha::Account::Lines->search( - { borrowernumber => $patron->borrowernumber, amountoutstanding => { '>' => 0 } }, - { order_by => { -desc => 'accountlines_id' } } - ); - $template->param( accounts => $accts ); -} - -# Show OPAC privacy preference is system preference is set -if ( C4::Context->preference('OPACPrivacy') ) { # FIXME Should be moved the the template - $template->param( OPACPrivacy => 1); - $template->param( "privacy".$data->{'privacy'} => 1); -} - -my $today = DateTime->now( time_zone => C4::Context->tz); -$today->truncate(to => 'day'); -my $overdues_exist = 0; -my $totalprice = 0; - -# Calculate and display patron's age -if ( $data->{dateofbirth} ) { - $template->param( age => Koha::Patron->new({ dateofbirth => $data->{dateofbirth} })->get_age ); -} - -### ############################################################################### -# BUILD HTML -# show all reserves of this borrower, and the position of the reservation .... -if ($borrowernumber) { - $template->param( - holds_count => Koha::Database->new()->schema()->resultset('Reserve') - ->count( { borrowernumber => $borrowernumber } ) ); -} + Koha::Checkouts->count({ borrowernumber => \@relatives }); # Generate CSRF token for upload and delete image buttons $template->param( csrf_token => Koha::Token->new->generate_csrf({ session_id => $input->cookie('CGISESSID'),}), ); - -$template->param(%$data); # FIXME This should be removed and used $patron instead, but too many things are processed above - if (C4::Context->preference('ExtendedPatronAttributes')) { my $attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); my @classes = uniq( map {$_->{class}} @$attributes ); @@ -279,8 +146,6 @@ if (C4::Context->preference('ExtendedPatronAttributes')) { if (C4::Context->preference('EnhancedMessagingPreferences')) { C4::Form::MessagingPreferences::set_form_values({ borrowernumber => $borrowernumber }, $template); $template->param(messaging_form_inactive => 1); - $template->param(SMSSendDriver => C4::Context->preference("SMSSendDriver")); - $template->param(TalkingTechItivaPhone => C4::Context->preference("TalkingTechItivaPhoneNotification")); } if ( C4::Context->preference("ExportCircHistory") ) { @@ -289,7 +154,7 @@ if ( C4::Context->preference("ExportCircHistory") ) { my $patron_messages = Koha::Patron::Messages->search( { - 'me.borrowernumber' => $borrowernumber, + 'me.borrowernumber' => $patron->borrowernumber, }, { join => 'manager', @@ -314,23 +179,16 @@ if ( $patron->is_expired || $patron->is_going_to_expire ) { ); } -my $total = $patron->account->balance; $template->param( patron => $patron, + issuecount => $patron->checkouts->count, + holds_count => $patron->holds->count, + fines => $patron->account->balance, translated_language => $translated_language, detailview => 1, was_renewed => scalar $input->param('was_renewed') ? 1 : 0, - todaysdate => output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }), - totalprice => sprintf("%.2f", $totalprice), - totaldue => sprintf("%.2f", $total), - totaldue_raw => $total, - overdues_exist => $overdues_exist, - StaffMember => $category_type eq 'S', $category_type => 1, # [% IF ( I ) %] = institutional/organisation - samebranch => $samebranch, - quickslip => $quickslip, housebound_role => scalar $patron->housebound_role, - PatronsPerPage => C4::Context->preference("PatronsPerPage") || 20, relatives_issues_count => $relatives_issues_count, relatives_borrowernumbers => \@relatives, ); -- 2.7.4