Bug 12159: Fix getting extended patron attributes for circ-menu
authorJosef Moravec <josef.moravec@gmail.com>
Thu, 15 Mar 2018 13:12:55 +0000 (13:12 +0000)
committerNick Clemens <nick@bywatersolutions.com>
Thu, 28 Mar 2019 13:05:22 +0000 (13:05 +0000)
Changes:
- Replace getting preference ExtendedPatronAttributes by Koha.Preference
in templates
- Add Koha::Patron->attributes for getting patrons extended attributes
- Use this method in circ-menu.inc
- Remove getting attributes from members perl scripts

Test plan:
0) Apply the patch
1) Add some patron attributes type - with free text, authorised value,
    limited by libraries...
2) Add some values to this attributes for some patrons
3) Go through as many patron pages as you can and confirm that
attributes are shown at side panel when they shoul and are not shown
when they should not be shown

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Removed Koha/Schema/Result/BorrowerAttribute.pm
[EDIT] Added missing semicolon on L114 in Koha/Patron/Attribute.pm

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

26 files changed:
Koha/Patron.pm
Koha/Patron/Attribute.pm
Koha/Patron/Attributes.pm
circ/circulation.pl
koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc
koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
koha-tmpl/intranet-tmpl/prog/en/modules/reports/borrowers_stats.tt
members/boraccount.pl
members/files.pl
members/housebound.pl
members/mancredit.pl
members/maninvoice.pl
members/member-flags.pl
members/member-password.pl
members/memberentry.pl
members/moremember.pl
members/notices.pl
members/pay.pl
members/paycollect.pl
members/purchase-suggestions.pl
members/readingrec.pl
members/routing-lists.pl
members/statistics.pl
reports/borrowers_stats.pl
tools/viewlog.pl

index eeaf0e4..bc7c811 100644 (file)
@@ -34,6 +34,7 @@ use Koha::DateUtils;
 use Koha::Exceptions::Password;
 use Koha::Holds;
 use Koha::Old::Checkouts;
+use Koha::Patron::Attributes;
 use Koha::Patron::Categories;
 use Koha::Patron::HouseboundProfile;
 use Koha::Patron::HouseboundRole;
@@ -1290,6 +1291,22 @@ sub generate_userid {
 
 }
 
+=head3 attributes
+
+my $attributes = $patron->attributes
+
+Return object of Koha::Patron::Attributes type with all attributes set for this patron
+
+=cut
+
+sub attributes {
+    my ( $self ) = @_;
+    return Koha::Patron::Attributes->search({
+        borrowernumber => $self->borrowernumber,
+        branchcode     => $self->branchcode,
+    });
+}
+
 =head2 Internal methods
 
 =head3 _type
index 906fc6d..09ed98f 100644 (file)
@@ -20,6 +20,7 @@ use Modern::Perl;
 use Koha::Database;
 use Koha::Exceptions::Patron::Attribute;
 use Koha::Patron::Attribute::Types;
+use Koha::AuthorisedValues;
 
 use base qw(Koha::Object);
 
@@ -79,6 +80,57 @@ sub opac_editable {
     return Koha::Patron::Attribute::Types->find( $self->code )->opac_editable;
 }
 
+=head3 display_checkout
+
+    my $attribute = Koha::Patron::Attribute->new({ code => 'a_code', ... });
+    if ( $attribute->display_checkout ) { ... }
+
+=cut
+
+sub display_checkout {
+
+    my $self = shift;
+
+    return $self->type->display_checkout;
+}
+
+=head3 value_description
+
+    my $description = $attribute->value_description
+
+    Return authorised value description or free text based on attribute type settings
+
+=cut
+
+sub value_description {
+
+    my $self = shift;
+
+    if ( $self->type->authorised_value_category ) {
+        my $av = Koha::AuthorisedValues->search({
+            category => $self->type->authorised_value_category,
+            authorised_value => $self->attribute,
+        });
+        return $av->next->lib if $av->count;
+    }
+    return $self->attribute;
+}
+
+=head3 type_description
+
+    my $description = $attribute->type_description
+
+    Return description of attribute type
+
+=cut
+
+sub type_description {
+
+    my $self = shift;
+
+    return $self->type->description;
+}
+
 =head3 type
 
     my $attribute_type = $attribute->type;
index 4264e06..1bbd35b 100644 (file)
@@ -31,6 +31,38 @@ Koha::Patron::Attributes - Koha Patron Attributes Object set class
 
 =cut
 
+=head3 search
+
+my $attributes-> Koha::Patron::Attributes->search( $params );
+
+=cut
+
+sub search {
+    my ( $self, $params, $attributes ) = @_;
+
+    my $branchcode = $params->{branchcode};
+    delete( $params->{branchcode} );
+
+    my $or =
+        $branchcode
+        ? {
+            '-or' => [
+                'borrower_attribute_types_branches.b_branchcode' => undef,
+                'borrower_attribute_types_branches.b_branchcode' => $branchcode,
+            ]
+        } : {};
+
+    my $join =
+        $branchcode
+        ? {
+            join => {
+                'borrower_attribute_types' => 'borrower_attribute_types_branches'
+            },
+        } : {};
+    $attributes //= {};
+    return $self->SUPER::search( { %$params, %$or }, { %$attributes, %$join } );
+}
+
 =head3 _type
 
 =cut
index cb18867..72a3846 100755 (executable)
@@ -43,7 +43,6 @@ use C4::Reserves;
 use Koha::Holds;
 use C4::Context;
 use CGI::Session;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use Koha::AuthorisedValues;
 use Koha::CsvProfiles;
 use Koha::Patrons;
@@ -552,13 +551,6 @@ if ( Koha::BiblioFrameworks->find('FA') ) {
     $fast_cataloging = 1 
 }
 
-if (C4::Context->preference('ExtendedPatronAttributes')) {
-    my $attributes = GetBorrowerAttributes($borrowernumber);
-    $template->param(
-        ExtendedPatronAttributes => 1,
-        extendedattributes => $attributes
-    );
-}
 my $view = $batch
     ?'batch_checkout_view'
     : 'circview';
index 39b0922..b225e85 100644 (file)
         [% END %]
     [% END %]
 
-    [% IF ( ExtendedPatronAttributes ) %][% FOREACH extendedattribute IN extendedattributes %]
-        [% IF ( extendedattribute.display_checkout ) %]
-            [% IF ( extendedattribute.value ) %]
-                <li class="patronattribute"><span class="patronattributelabel">[% extendedattribute.description | html %]</span> : [% IF ( extendedattribute.value_description ) %][% extendedattribute.value_description | html %][% ELSE %][% extendedattribute.value | html %][% END %]</li>
+    [% IF Koha.Preference('ExtendedPatronAttributes') %]
+        [% FOREACH extendedattribute IN patron.attributes %]
+            [% IF ( extendedattribute.display_checkout ) %]
+                <li class="patronattribute"><span class="patronattributelabel">[% extendedattribute.type_description | html %]</span> : [% IF ( extendedattribute.value_description ) %][% extendedattribute.value_description | html %][% ELSE %][% extendedattribute.attribute | html %][% END %]</li>
             [% END %]
         [% END %]
-    [% END %][% END %]
+    [% END %]
     <li class="patroncategory">Category: [% patron.category.description | html %] ([% patron.categorycode | html %])</li>
     <li class="patronlibrary">Home library: [% Branches.GetName( patron.branchcode ) | html %]</li>
     <li class="patronborrowernumber">Borrowernumber: [% patron.borrowernumber | html %]</li>
index 99a0e8e..6d4b0e8 100644 (file)
 [% END # IF step_7 %]
 
 [% IF ( step_4 ) %]
-[% IF ( ExtendedPatronAttributes ) %][% UNLESS ( no_patron_attribute_types ) %]
+[% IF Koha.Preference('ExtendedPatronAttributes') %][% UNLESS ( no_patron_attribute_types ) %]
   <fieldset class="rows" id="memberentry_patron_attributes">
     <legend id="patron_attributes_lgd">Additional attributes and identifiers</legend>
     <input type="hidden" name="setting_extended_patron_attributes" value="1" />
index 45277f6..91c3750 100644 (file)
                                 </div> [% #/ div#houseboundroles %]
                             [% END %]
 
-                            [% IF ( ExtendedPatronAttributes ) %]
+                            [% IF Koha.Preference('ExtendedPatronAttributes') %]
                                 [% UNLESS ( no_patron_attribute_types ) %]
                                     <div id="patron-extended-attributes" class="patroninfo-section">
                                         [% IF ( attributes_loop ) %]
index 25c4843..17a0ee2 100644 (file)
@@ -1,4 +1,5 @@
 [% USE Branches %]
+[% USE Koha %]
 [% SET footerjs = 1 %]
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Reports [% IF ( do_it ) %]&rsaquo; Patrons statistics &rsaquo; Results[% ELSE %]&rsaquo; Patrons statistics[% END %]</title>
             [% ELSE %]
                 <input type="hidden" name="Filter" />
                        [% END %]
-            [% IF ( ExtendedPatronAttributes ) %]
+            [% IF Koha.Preference('ExtendedPatronAttributes') %]
                 <tr>
                     <th colspan="4">Patron attributes</th>
                 </tr>
index 74f8e29..9121a8f 100755 (executable)
@@ -29,7 +29,6 @@ use C4::Output;
 use CGI qw ( -utf8 );
 use C4::Members;
 use C4::Accounts;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use Koha::Patrons;
 use Koha::Patron::Categories;
 
@@ -79,14 +78,6 @@ if($total <= 0){
         $totalcredit = 1;
 }
 
-if (C4::Context->preference('ExtendedPatronAttributes')) {
-    my $attributes = GetBorrowerAttributes($borrowernumber);
-    $template->param(
-        ExtendedPatronAttributes => 1,
-        extendedattributes => $attributes
-    );
-}
-
 $template->param(
     patron              => $patron,
     finesview           => 1,
index 46c3694..0bd4f94 100755 (executable)
@@ -24,7 +24,6 @@ use CGI qw ( -utf8 );
 use C4::Auth;
 use C4::Output;
 use C4::Members;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use C4::Debug;
 
 use Koha::DateUtils;
@@ -107,14 +106,6 @@ else {
         $bf->DelFile( id => scalar $cgi->param('file_id') );
     }
 
-    if (C4::Context->preference('ExtendedPatronAttributes')) {
-        my $attributes = GetBorrowerAttributes($borrowernumber);
-        $template->param(
-            ExtendedPatronAttributes => 1,
-            extendedattributes => $attributes
-        );
-    }
-
     $template->param(
         files => Koha::Patron::Files->new( borrowernumber => $borrowernumber )
           ->GetFilesInfo(),
index 574bacd..4b965d8 100755 (executable)
@@ -28,7 +28,6 @@ use Modern::Perl;
 use CGI;
 use C4::Auth;
 use C4::Context;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use C4::Output;
 use DateTime;
 use Koha::DateUtils;
@@ -157,15 +156,6 @@ $method = 'update_or_create' if ( !$houseboundprofile );
 # Ensure template has all patron details.
 $template->param( patron => $patron );
 
-# Load extended patron attributes if necessary (taken from members/files.pl).
-if ( C4::Context->preference('ExtendedPatronAttributes') and $patron ) {
-    my $attributes = GetBorrowerAttributes($patron->borrowernumber);
-    $template->param(
-        ExtendedPatronAttributes => 1,
-        extendedattributes => $attributes
-    );
-}
-
 $template->param(
     housebound_profile => $houseboundprofile,
     visit              => $houseboundvisit,
index 014d822..34d062a 100755 (executable)
@@ -31,7 +31,7 @@ use CGI qw ( -utf8 );
 use C4::Members;
 use C4::Accounts;
 use C4::Items;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
+use Koha::Patrons;
 
 use Koha::Items;
 use Koha::Patrons;
@@ -100,15 +100,6 @@ if ($add){
     print $input->redirect("/cgi-bin/koha/members/boraccount.pl?borrowernumber=$borrowernumber");
 
 } else {
-
-    if (C4::Context->preference('ExtendedPatronAttributes')) {
-        my $attributes = GetBorrowerAttributes($borrowernumber);
-        $template->param(
-            ExtendedPatronAttributes => 1,
-            extendedattributes => $attributes
-        );
-    }
-
     $template->param(
         patron     => $patron,
         finesview  => 1,
index 668136a..76d224b 100755 (executable)
@@ -30,7 +30,6 @@ use CGI qw ( -utf8 );
 use C4::Members;
 use C4::Accounts;
 use C4::Items;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use Koha::Token;
 
 use Koha::Items;
@@ -121,14 +120,6 @@ if ($add){
   }
   $template->param( invoice_types_loop => \@invoice_types );
 
-    if (C4::Context->preference('ExtendedPatronAttributes')) {
-        my $attributes = GetBorrowerAttributes($borrowernumber);
-        $template->param(
-            ExtendedPatronAttributes => 1,
-            extendedattributes => $attributes
-        );
-    }
-
     $template->param(
         csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $input->cookie('CGISESSID') }),
         patron         => $patron,
index 10324dd..9657a92 100755 (executable)
@@ -11,7 +11,6 @@ use C4::Output;
 use C4::Auth qw(:DEFAULT :EditPermissions);
 use C4::Context;
 use C4::Members;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 #use C4::Acquisitions;
 
 use Koha::Patron::Categories;
@@ -182,14 +181,6 @@ if ($input->param('newflags')) {
         push @loop, \%row;
     }
 
-if (C4::Context->preference('ExtendedPatronAttributes')) {
-    my $attributes = GetBorrowerAttributes($bor->{'borrowernumber'});
-    $template->param(
-        ExtendedPatronAttributes => 1,
-        extendedattributes => $attributes
-    );
-}
-
 $template->param(
     patron         => $patron,
     loop           => \@loop,
index 013e7e5..256c725 100755 (executable)
@@ -13,7 +13,6 @@ use C4::Context;
 use C4::Members;
 use C4::Circulation;
 use CGI qw ( -utf8 );
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use Koha::AuthUtils;
 use Koha::Token;
 
@@ -98,14 +97,6 @@ if ( $newpassword and not @errors) {
     };
 }
 
-if ( C4::Context->preference('ExtendedPatronAttributes') ) {
-    my $attributes = GetBorrowerAttributes( $patron_id );
-    $template->param(
-        ExtendedPatronAttributes => 1,
-        extendedattributes       => $attributes
-    );
-}
-
 $template->param(
     patron      => $patron,
     destination => $destination,
index 031a8e1..b7a3920 100755 (executable)
@@ -746,7 +746,6 @@ foreach (qw(dateenrolled dateexpiry dateofbirth)) {
 }
 
 if (C4::Context->preference('ExtendedPatronAttributes')) {
-    $template->param(ExtendedPatronAttributes => 1);
     patron_attributes_form($template, $borrowernumber);
 }
 
index faaa95e..be365a3 100755 (executable)
@@ -267,7 +267,6 @@ if (C4::Context->preference('ExtendedPatronAttributes')) {
     }
 
     $template->param(
-        ExtendedPatronAttributes => 1,
         attributes_loop => \@attributes_loop
     );
 
index ac6748c..3a70683 100755 (executable)
@@ -25,7 +25,6 @@ use C4::Output;
 use CGI qw ( -utf8 );
 use C4::Members;
 use C4::Letters;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use Koha::Patrons;
 use Koha::Patron::Categories;
 
@@ -66,14 +65,6 @@ if ( $op eq 'resend_notice' ) {
 # Getting the messages
 my $queued_messages = C4::Letters::GetQueuedMessages({borrowernumber => $borrowernumber});
 
-if (C4::Context->preference('ExtendedPatronAttributes')) {
-    my $attributes = GetBorrowerAttributes($borrowernumber);
-    $template->param(
-        ExtendedPatronAttributes => 1,
-        extendedattributes => $attributes
-    );
-}
-
 $template->param(
     patron             => $patron,
     QUEUED_MESSAGES    => $queued_messages,
index 9bba93d..b128053 100755 (executable)
@@ -38,7 +38,6 @@ use C4::Accounts;
 use C4::Stats;
 use C4::Koha;
 use C4::Overdues;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use Koha::Patrons;
 
 use Koha::Patron::Categories;
@@ -149,7 +148,6 @@ sub add_accounts_to_template {
     while ( my $account_line = $account_lines->next ) {
         push @accounts, $account_line;
     }
-    borrower_add_additional_fields($patron);
 
     $template->param(
         patron   => $patron,
@@ -228,23 +226,6 @@ sub writeoff_all {
     return;
 }
 
-sub borrower_add_additional_fields {
-    my $patron = shift;
-
-# some borrower info is not returned in the standard call despite being assumed
-# in a number of templates. It should not be the business of this script but in lieu of
-# a revised api here it is ...
-    if (C4::Context->preference('ExtendedPatronAttributes')) {
-        my $extendedattributes = GetBorrowerAttributes($patron->borrowernumber);
-        $template->param(
-            extendedattributes       => $extendedattributes,
-            ExtendedPatronAttributes => 1,
-        );
-    }
-
-    return;
-}
-
 sub payselected {
     my $parameters = shift;
 
index abd96b5..5bd9c0e 100755 (executable)
@@ -25,7 +25,6 @@ use C4::Context;
 use C4::Auth;
 use C4::Output;
 use C4::Members;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use C4::Accounts;
 use C4::Koha;
 
@@ -180,8 +179,6 @@ if ( $total_paid and $total_paid ne '0.00' ) {
     $total_paid = '0.00';    #TODO not right with pay_individual
 }
 
-borrower_add_additional_fields($patron, $template);
-
 $template->param(%$borrower);
 
 if ( $input->param('error_over') ) {
@@ -193,24 +190,8 @@ $template->param(
     borrowernumber => $borrowernumber,    # some templates require global
     patron        => $patron,
     total         => $total_due,
-    ExtendedPatronAttributes => C4::Context->preference('ExtendedPatronAttributes'),
 
     csrf_token => Koha::Token->new->generate_csrf({ session_id => scalar $input->cookie('CGISESSID') }),
 );
 
 output_html_with_http_headers $input, $cookie, $template->output;
-
-sub borrower_add_additional_fields {
-    my ( $patron, $template ) = @_;
-
-# some borrower info is not returned in the standard call despite being assumed
-# in a number of templates. It should not be the business of this script but in lieu of
-# a revised api here it is ...
-
-    if (C4::Context->preference('ExtendedPatronAttributes')) {
-        my $extendedattributes = GetBorrowerAttributes($patron->borrowernumber);
-        $template->param( extendedattributes => $extendedattributes );
-    }
-
-    return;
-}
index 8e4b76a..67c021e 100755 (executable)
@@ -24,7 +24,6 @@ use C4::Auth;
 use C4::Context;
 use C4::Output;
 use C4::Members;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use C4::Suggestions;
 use Koha::Patrons;
 
@@ -52,14 +51,6 @@ $template->param(
     suggestionsview  => 1,
 );
 
-if (C4::Context->preference('ExtendedPatronAttributes')) {
-    my $attributes = GetBorrowerAttributes($borrowernumber);
-    $template->param(
-        ExtendedPatronAttributes => 1,
-        extendedattributes => $attributes
-    );
-}
-
 my $suggestions = SearchSuggestion( { suggestedby => $borrowernumber } );
 
 $template->param( suggestions => $suggestions );
index 4ce115e..7dd71f8 100755 (executable)
@@ -29,7 +29,6 @@ use C4::Output;
 use C4::Members;
 use List::MoreUtils qw/any uniq/;
 use Koha::DateUtils;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 
 use Koha::Patrons;
 use Koha::Patron::Categories;
@@ -96,14 +95,6 @@ if (! $limit){
        $limit = 'full';
 }
 
-if (C4::Context->preference('ExtendedPatronAttributes')) {
-    my $attributes = GetBorrowerAttributes($patron->borrowernumber);
-    $template->param(
-        ExtendedPatronAttributes => 1,
-        extendedattributes => $attributes
-    );
-}
-
 $template->param(
     patron            => $patron,
     readingrecordview => 1,
index 51c6645..4885b14 100755 (executable)
@@ -22,7 +22,6 @@ use CGI qw ( -utf8 );
 use C4::Output;
 use C4::Auth qw/:DEFAULT/;
 use C4::Members;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use C4::Context;
 use C4::Serials;
 use Koha::Patrons;
@@ -58,12 +57,4 @@ $template->param(
     routinglistview   => 1,
 );
 
-if (C4::Context->preference('ExtendedPatronAttributes')) {
-    my $attributes = GetBorrowerAttributes($borrowernumber);
-    $template->param(
-        ExtendedPatronAttributes => 1,
-        extendedattributes => $attributes
-    );
-}
-
 output_html_with_http_headers $query, $cookie, $template->output;
index e63d221..61c42f9 100755 (executable)
@@ -29,7 +29,6 @@ use C4::Auth;
 use C4::Context;
 use C4::Members;
 use C4::Members::Statistics;
-use C4::Members::Attributes qw(GetBorrowerAttributes);
 use C4::Output;
 use Koha::Patrons;
 use Koha::Patron::Categories;
@@ -77,14 +76,6 @@ my $count_total_issues = $total->{count_total_issues_today} || 0;
 my $count_total_issues_returned = $total->{count_total_issues_returned_today} || 0;
 my $count_total_actual_state = ($count_total_precedent_state - $count_total_issues_returned + $count_total_issues);
 
-if (C4::Context->preference('ExtendedPatronAttributes')) {
-    my $attributes = GetBorrowerAttributes($borrowernumber);
-    $template->param(
-        ExtendedPatronAttributes => 1,
-        extendedattributes => $attributes
-    );
-}
-
 $template->param(
     patron             => $patron,
     statisticsview     => 1,
index 4b700c1..73c655c 100755 (executable)
@@ -137,7 +137,6 @@ if ($do_it) {
                CGIsepChoice => $CGIsepChoice,
     );
     if (C4::Context->preference('ExtendedPatronAttributes')) {
-        $template->param(ExtendedPatronAttributes => 1);
         patron_attributes_form($template);
     }
 }
index 381b0f8..f38ca19 100755 (executable)
@@ -74,21 +74,12 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
 
 if ( $src eq 'circ' ) {
 
-    # if we were called from circulation, use the circulation menu and get data to populate it -fbcit
-    use C4::Members::Attributes qw(GetBorrowerAttributes);
     my $borrowernumber = $object;
     my $patron = Koha::Patrons->find( $borrowernumber );
     my $circ_info = 1;
     unless ( $patron ) {
          $circ_info = 0;
     }
-    if ( C4::Context->preference('ExtendedPatronAttributes') ) {
-        my $attributes = GetBorrowerAttributes( $borrowernumber );
-        $template->param(
-            ExtendedPatronAttributes => 1,
-            extendedattributes       => $attributes
-        );
-    }
 
     $template->param(
         patron      => $patron,