From aeb458ccc0237be0b1086f45f2453548b0851655 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 5 Apr 2017 16:43:41 -0300 Subject: [PATCH] Bug 18403: Use patron-title.inc when hidepatronname is used There is already a HidePatronName syspref to hide patron's information on bibliographic record detail pages and the hold list. Test plan: With the HidePatronName enabled, make sure the patron's information are hidden from the catalogue and hold list pages. If the logged in user is not allowed to see the patron's info, no link and no cardnumber will be displayed With he HidePatronName disabled, make sure the patron's information are displayed if the logged in user is allowed to see the patron's info. Technical note: This patch improves the existing patron-title.inc include file to display patron's information. Using it everywhere patron's details are displayed will permit to homogenise the way they are displayed. The file takes now a patron object (what should be, in the future, the only way to use it), that way we can call the new method on it to know if patron's information can be shown by the logged in used. NOTE: I am not sure this syspref makes sense anymore. Should not we remove it? Signed-off-by: Jon McGowan --- Koha/Hold.pm | 1 + catalogue/detail.pl | 13 +--- catalogue/moredetail.pl | 13 +--- .../prog/en/includes/patron-title.inc | 85 ++++++++++++++++------ .../prog/en/modules/catalogue/detail.tt | 22 ++---- .../prog/en/modules/reserve/request.tt | 8 +- reserve/request.pl | 9 +-- 7 files changed, 77 insertions(+), 74 deletions(-) diff --git a/Koha/Hold.pm b/Koha/Hold.pm index ba75a52..3b6b804 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -252,6 +252,7 @@ Returns the related Koha::Patron object for this Hold =cut +# FIXME Should be renamed with ->patron sub borrower { my ($self) = @_; diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 221532f..2af0268 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -253,11 +253,6 @@ foreach my $item (@items) { $itemfields{$_} = 1 if ( $item->{$_} ); } - if (C4::Context->preference('HidePatronName')){ - $item->{'hidepatronname'} = 1; - } - - # checking for holds my $item_object = Koha::Items->find( $item->{itemnumber} ); my $holds = $item_object->current_holds; @@ -265,15 +260,15 @@ foreach my $item (@items) { my $patron = Koha::Patrons->find( $first_hold->borrowernumber ); $item->{backgroundcolor} = 'reserved'; $item->{reservedate} = $first_hold->reservedate; - $item->{ReservedForBorrowernumber} = $first_hold->borrowernumber; - $item->{ReservedForSurname} = $patron->surname; - $item->{ReservedForFirstname} = $patron->firstname; + $item->{ReservedFor} = $patron, $item->{ExpectedAtLibrary} = $first_hold->branchcode; - $item->{Reservedcardnumber} = $patron->cardnumber; # Check waiting status $item->{waitingdate} = $first_hold->waitingdate; } + if ( my $checkout = $item_object->checkout ) { + $item->{CheckedOutFor} = $checkout->patron; + } # Check the transit status my ( $transfertwhen, $transfertfrom, $transfertto ) = GetTransfers($item->{itemnumber}); diff --git a/catalogue/moredetail.pl b/catalogue/moredetail.pl index 85b491a..c6e317e 100755 --- a/catalogue/moredetail.pl +++ b/catalogue/moredetail.pl @@ -61,8 +61,6 @@ if($query->cookie("holdfor")){ ); } -my $hidepatronname = C4::Context->preference("HidePatronName"); - # get variables my $biblionumber=$query->param('biblionumber'); @@ -187,14 +185,10 @@ foreach my $item (@items){ $item->{'issue'}= 0; } - unless ($hidepatronname) { - if ( $item->{'borrowernumber'} ) { - my $curr_borrower = Koha::Patrons->find( $item->{borrowernumber} ); - $item->{borrowerfirstname} = $curr_borrower->firstname; - $item->{borrowersurname} = $curr_borrower->surname; - } + if ( $item->{'borrowernumber'} ) { + my $curr_borrower = Koha::Patrons->find( $item->{borrowernumber} ); + $item->{patron} = $curr_borrower; } - } my $mss = Koha::MarcSubfieldStructures->search({ frameworkcode => $fw, kohafield => 'items.itemlost', authorised_value => { not => undef } }); @@ -225,7 +219,6 @@ $template->param( itemnumber => $itemnumber, z3950_search_params => C4::Search::z3950_search_args(GetBiblioData($biblionumber)), subtitle => $subtitle, - hidepatronname => $hidepatronname, ); $template->param(ONLY_ONE => 1) if ( $itemnumber && $showncount != @items ); $template->{'VARS'}->{'searchid'} = $query->param('searchid'); diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-title.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-title.inc index caf45d6..89dfc81 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-title.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-title.inc @@ -1,27 +1,64 @@ -[%- IF ( borrower.borrowernumber ) %] - [%- IF borrower.category_type == 'I' %] - [%- borrower.surname | html %] [% IF borrower.othernames %] ([% borrower.othernames | html %]) [% END %] - [%- ELSE %] - [%- IF invert_name %] - [%- borrower.surname | html %], [% borrower.firstname | html %] [% IF borrower.othernames %] ([% borrower.othernames | html %]) [% END %] - [%- ELSE %] - [%- borrower.firstname | html %] [% IF borrower.othernames %] ([% borrower.othernames | html %]) [% END %] [% borrower.surname | html %] - [%- END -%] +[%- USE Koha -%] +[%- USE Branches -%] +[%- SET data = {} -%] +[%- IF patron -%] + [%- SET data.category_type = patron.category.category_type -%] + [%- SET data.surname = patron.surname -%] + [%- SET data.othernames = patron.othernames -%] + [%- SET data.surname = patron.surname -%] + [%- SET data.firstname = patron.firstname -%] + [%- SET data.cardnumber = patron.cardnumber -%] + [%- SET data.borrowernumber = patron.borrowernumber -%] +[%- ELSIF ( borrower.borrowernumber ) -%] + [%- SET data.category_type = borrower.category_type -%] + [%- SET data.surname = borrower.surname -%] + [%- SET data.othernames = borrower.othernames -%] + [%- SET data.surname = borrower.surname -%] + [%- SET data.firstname = borrower.firstname -%] + [%- SET data.cardnumber = borrower.cardnumber -%] + [%- SET data.borrowernumber = borrower.borrowernumber -%] +[%- ELSIF ( borrowernumber ) -%] + [%- SET data.category_type = category_type -%] + [%- SET data.surname = surname -%] + [%- SET data.othernames = othernames -%] + [%- SET data.surname = surname -%] + [%- SET data.firstname = firstname -%] + [%- SET data.cardnumber = cardnumber -%] + [%- SET data.borrowernumber = borrowernumber -%] +[%- END -%] +[%- SET display_patron_name = 1 -%] +[%- SET display_cardnumber = 1 -%] +[%- IF hide_patron_infos_if_needed %] [%# Should only be set if patron is set -%] + [%- SET can_see_patron_infos = logged_in_user.can_see_patron_infos( patron ) -%] + [%- UNLESS can_see_patron_infos -%] + [%- SET display_patron_name = 0 -%] + [%- SET display_cardnumber = 0 -%] + [%- ELSIF Koha.Preference('HidePatronName') -%] + [%- SET display_patron_name = 0 -%] + [%- END -%] +[%- END -%] +[%- IF hide_patron_infos_if_needed AND ( display_patron_name OR display_cardnumber ) -%] + [%- IF link_to == 'circulation_reserves' %] + [%- ELSE %] [%- END -%] - [%- IF ( borrower.cardnumber ) -%] - ([% borrower.cardnumber | html %]) - [%- END %] -[%- ELSIF ( borrowernumber ) %] - [%- IF category_type == 'I' %] - [%- surname | html %] [% IF othernames %] ([% othernames | html %]) [% END %] - [%- ELSE %] - [%- IF invert_name %] - [%- surname | html %], [% firstname | html %] [% IF othernames %] ([% othernames | html %]) [% END %] - [%- ELSE %] - [%- firstname | html %] [% IF othernames %] ([% othernames | html %]) [% END %] [% surname | html %] - [%- END %] +[%- END -%] +[%- IF data.category_type == 'I' -%] + [%- UNLESS display_patron_name %][%- data.surname | html %] [% IF data.othernames %] [% END %]([% data.othernames | html %]) [% END -%] +[%- ELSIF display_patron_name -%] + [%- IF invert_name -%] + [%- data.surname | html %], [% data.firstname | html %] [% IF data.othernames %] ([% data.othernames | html %]) [% END -%] + [%- ELSE -%] + [%- data.firstname | html %] [% IF data.othernames %] ([% data.othernames | html %]) [% END %] [% data.surname | html -%] + [%- END -%] + [%- IF display_cardnumber AND data.cardnumber %]([% data.cardnumber | html %])[% END -%] +[%- ELSIF display_cardnumber -%] + [%- IF data.cardnumber -%][%# FIXME Cardnumber should always be defined, right? -%] + [%- data.cardnumber | html -%] [%- END -%] - [%- IF ( cardnumber ) -%] - ([% cardnumber | html %]) - [%- END %] +[%- ELSE -%] + A patron from library [% Branches.GetName( patron.branchcode ) -%] +[%- END -%] + +[%- IF hide_patron_infos_if_needed AND ( display_patron_name OR display_cardnumber ) -%] + [%- END -%] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt index 31f7723..653cd3f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -675,7 +675,7 @@ function verify_images() { [% IF ( item.itemcallnumber ) %] [% item.itemcallnumber %][% END %] - [% IF ( item.datedue ) %] + [% IF item.CheckedOutFor %] [% IF item.onsite_checkout %] Currently in local use [% ELSE %] @@ -683,16 +683,11 @@ function verify_images() { [% END %] [% UNLESS ( item.NOTSAMEBRANCH ) %] [% IF item.onsite_checkout %] - by + by [% ELSE %] - to + to [% END %] - [% IF ( item.hidepatronname ) %] - [% item.cardnumber %] - [% ELSE %] - [% item.firstname %] [% item.surname %] - [% END %] - + [% INCLUDE 'patron-title.inc' patron=item.CheckedOutFor hide_patron_infos_if_needed=1 %] [% END %] : due [% item.datedue %] @@ -742,13 +737,8 @@ function verify_images() { Item-level hold (placed [% item.reservedate | $KohaDates %]) for delivery at [% Branches.GetName( item.ExpectedAtLibrary ) %]. [% END %] [% IF ( canreservefromotherbranches ) %] - Hold for: - [% IF ( item.hidepatronname ) %] - [% item.Reservedcardnumber %] - [% ELSE %] - [% item.ReservedForFirstname _ " " _ item.ReservedForSurname _ " (" _ item.Reservedcardnumber _ ")" %] - [% END %] - + Hold for: + [% INCLUDE 'patron-title.inc' patron=item.ReservedFor hide_patron_infos_if_needed=1 %] [% END %] [% END %] [% UNLESS ( item.itemnotforloan || item.notforloan_per_itemtype || item.onloan || item.itemlost || item.withdrawn || item.damaged || item.transfertwhen || item.reservedate ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index 34a7304..6e4a3f1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -773,13 +773,7 @@ function checkMultiHold() { [% END %] - - [% IF ( reserveloo.hidename ) %] - [% reserveloo.cardnumber (reserveloo.borrowernumber) %] - [% ELSE %] - [% reserveloo.firstname %] [% reserveloo.surname %] - [% END %] - + [% INCLUDE 'patron-title.inc' patron=reserveloo.patron hide_patron_infos_if_needed=1 %] [% reserveloo.notes %] [% reserveloo.date %] diff --git a/reserve/request.pl b/reserve/request.pl index 20b50cb..9bf390d 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -567,19 +567,12 @@ foreach my $biblionumber (@biblionumbers) { } } - # get borrowers reserve info - if ( C4::Context->preference('HidePatronName') ) { - $reserve{'hidename'} = 1; - $reserve{'cardnumber'} = $res->borrower()->cardnumber(); - } $reserve{'expirationdate'} = output_pref( { dt => dt_from_string( $res->expirationdate ), dateonly => 1 } ) unless ( !defined( $res->expirationdate ) || $res->expirationdate eq '0000-00-00' ); $reserve{'date'} = output_pref( { dt => dt_from_string( $res->reservedate ), dateonly => 1 } ); $reserve{'borrowernumber'} = $res->borrowernumber(); $reserve{'biblionumber'} = $res->biblionumber(); - $reserve{'borrowernumber'} = $res->borrowernumber(); - $reserve{'firstname'} = $res->borrower()->firstname(); - $reserve{'surname'} = $res->borrower()->surname(); + $reserve{'patron'} = $res->borrower; $reserve{'notes'} = $res->reservenotes(); $reserve{'waiting_date'} = $res->waitingdate(); $reserve{'waiting_until'} = $res->is_waiting() ? $res->waiting_expires_on() : undef; -- 2.1.4