Bug 40401: Implement Koha::Patron->is_anonymous

The first motivation of this patch is to remove the following warnings
from t/db_dependent/Auth.t:
Use of uninitialized value $anonymous_patron in string eq at /kohadevbox/koha/C4/Auth.pm line 1497

Actually a correct fix seems to add a new `is_anonymous` method to
Koha::Patron in order to remove the warnings from everywhere else.

Test plan:
Confirm that you cannot log in using the anonymous patron and that the 2
tests are passing
  prove t/db_dependent/Auth.t t/db_dependent/Koha/Patron.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

C4/Auth.pm

@@ -1492,10 +1492,6 @@ sub checkauth {
     my $auth_template_name = ( $type eq 'opac' ) ? 'opac-auth.tt' : 'auth.tt';
     my $template           = C4::Templates::gettemplate( $auth_template_name, $type, $query );
 
-    my $borrowernumber      = $patron and $patron->borrowernumber;
-    my $anonymous_patron    = C4::Context->preference('AnonymousPatron');
-    my $is_anonymous_patron = $patron && ( $patron->borrowernumber eq $anonymous_patron );
-
     $template->param(
         login                                 => 1,
         INPUTS                                => \@inputs,
@@ -1530,7 +1526,7 @@ sub checkauth {
         opac_css_override                     => $ENV{'OPAC_CSS_OVERRIDE'},
         too_many_login_attempts               => ( $patron and $patron->account_locked ),
         password_has_expired                  => ( $patron and $patron->password_expired ),
-        is_anonymous_patron                   => ($is_anonymous_patron),
+        is_anonymous_patron                   => ( $patron and $patron->is_anonymous ),
         password_expiration_date              => ( $patron and $patron->password_expiration_date ),
         date_enrolled                         => ( $patron and $patron->dateenrolled ),
         auth_error                            => $auth_error,

Koha/Patron.pm

@@ -473,9 +473,8 @@ ListOwnershipUponPatronDeletion pref, but entries from the borrower to other lis
 sub delete {
     my ($self) = @_;
 
-    my $anonymous_patron = C4::Context->preference("AnonymousPatron");
     Koha::Exceptions::Patron::FailedDeleteAnonymousPatron->throw()
-        if $anonymous_patron && $self->id eq $anonymous_patron;
+        if $self->is_anonymous;
 
     # Check if patron is protected
     Koha::Exceptions::Patron::FailedDeleteProtectedPatron->throw() if defined $self->protected && $self->protected == 1;
@@ -736,8 +735,7 @@ sub siblings {
 sub merge_with {
     my ( $self, $patron_ids ) = @_;
 
-    my $anonymous_patron = C4::Context->preference("AnonymousPatron");
-    return if $anonymous_patron && $self->id eq $anonymous_patron;
+    return if $self->is_anonymous;
 
     # Do not merge other patrons into a protected patron
     return if $self->protected;
@@ -755,12 +753,12 @@ sub merge_with {
         sub {
             foreach my $patron_id (@patron_ids) {
 
-                next if $anonymous_patron && $patron_id eq $anonymous_patron;
-
                 my $patron = Koha::Patrons->find($patron_id);
 
                 next unless $patron;
 
+                next if $patron->is_anonymous;
+
                 # Do not merge protected patrons into other patrons
                 next if $patron->protected;
 
@@ -3048,11 +3046,9 @@ This method tells if the Koha:Patron object can be deleted. Possible return valu
 sub safe_to_delete {
     my ($self) = @_;
 
-    my $anonymous_patron = C4::Context->preference('AnonymousPatron');
-
     my $error;
 
-    if ( $anonymous_patron && $self->id eq $anonymous_patron ) {
+    if ( $self->is_anonymous ) {
         $error = 'is_anonymous_patron';
     } elsif ( $self->checkouts->count ) {
         $error = 'has_checkouts';
@@ -3405,6 +3401,20 @@ sub is_patron_inside_charge_limits {
     return $patron_charge_limits;
 }
 
+=head3 is_anonymous
+
+my $is_anonymous_patron= $patron->is_anonymous();
+
+Returns true if the patron the anonymous patron (AnonymousPatron)
+
+=cut
+
+sub is_anonymous {
+    my ($self) = @_;
+    my $anonymous_patron = C4::Context->preference('AnonymousPatron');
+    return ( $anonymous_patron && $self->borrowernumber eq $anonymous_patron ) ? 1 : 0;
+}
+
 =head2 Internal methods
 
 =head3 _type

circ/circulation.pl

@@ -626,7 +626,7 @@ if ($patron) {
         $template->param( is_debarred => 1 );
         $noissues = 1;
     }
-    if ( $patron->borrowernumber eq C4::Context->preference("AnonymousPatron") ) {
+    if ( $patron->is_anonymous ) {
         $template->param( is_anonymous => 1 );
         $noissues = 1;
     }

koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt

@@ -45,7 +45,7 @@
 
     [% UNLESS Koha.Preference('IntranetReadingHistoryHolds') %]
         <div class="alert alert-warning">Staff members are not allowed to access patron's holds history</div>
-    [% ELSIF is_anonymous %]
+    [% ELSIF patron.is_anonymous %]
         <div class="alert alert-warning">This is the anonymous patron, so no holds history is displayed.</div>
     [% ELSE %]
 

koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt

@@ -50,7 +50,7 @@
 
     [% INCLUDE 'members-toolbar.inc' %]
 
-    [% IF is_anonymous %]
+    [% IF patron.is_anonymous %]
         <div class="alert alert-warning">This is the anonymous patron.</div>
     [% END %]
 

members/holdshistory.pl

@@ -44,13 +44,6 @@ unless ($patron) {
     exit;
 }
 
-if ( $borrowernumber eq C4::Context->preference('AnonymousPatron') ) {
-
-    # use of 'eq' in the above comparison is intentional -- the
-    # system preference value could be blank
-    $template->param( is_anonymous => 1 );
-}
-
 $template->param(
     holdshistoryview => 1,
     patron           => $patron,

members/moremember.pl

@@ -78,10 +78,6 @@ output_and_exit_if_error(
 
 my $category_type = $patron->category->category_type;
 
-if ( $patron->borrowernumber eq C4::Context->preference("AnonymousPatron") ) {
-    $template->param( is_anonymous => 1 );
-}
-
 for (qw(gonenoaddress lost borrowernotes is_debarred)) {
     $patron->$_ and $template->param( flagged => 1 ) and last;
 }

members/readingrec.pl

@@ -86,10 +86,7 @@ if ( $op eq 'export_barcodes' ) {
 }
 
 # Do not request the old issues of anonymous patron
-if ( $patron->borrowernumber eq C4::Context->preference('AnonymousPatron') ) {
-
-    # use of 'eq' in the above comparison is intentional -- the
-    # system preference value could be blank
+if ( $patron->is_anonymous ) {
     $template->param( is_anonymous => 1 );
 } else {
     $template->param(

t/db_dependent/Auth.t

@@ -7,7 +7,8 @@ use CGI qw ( -utf8 );
 use Test::MockObject;
 use Test::MockModule;
 use List::MoreUtils qw/all any none/;
-use Test::More tests => 23;
+use Test::More tests => 24;
+use Test::NoWarnings;
 use Test::Warn;
 use t::lib::Mocks;
 use t::lib::TestBuilder;

t/db_dependent/Koha/Patron.t

@@ -19,7 +19,8 @@
 
 use Modern::Perl;
 
-use Test::More tests => 40;
+use Test::More tests => 42;
+use Test::NoWarnings;
 use Test::Exception;
 use Test::Warn;
 use Time::Fake;
@@ -3094,3 +3095,26 @@ subtest 'can_place_holds() tests' => sub {
         $schema->storage->txn_rollback;
     };
 };
+
+subtest 'is_anonymous' => sub {
+    plan tests => 3;
+
+    $schema->storage->txn_begin;
+
+    t::lib::Mocks::mock_preference( 'AnonymousPatron', '' );
+
+    my $patron = $builder->build_object( { class => 'Koha::Patrons' } );
+
+    is( $patron->is_anonymous, 0, 'is_anonymous returns 0 if pref is empty' );
+
+    t::lib::Mocks::mock_preference( 'AnonymousPatron', $patron->borrowernumber );
+
+    is( $patron->is_anonymous, 1, q{is_anonymous returns 1 if pref is equal to patron's id} );
+
+    t::lib::Mocks::mock_preference( 'AnonymousPatron', $patron->borrowernumber + 1 );
+
+    is( $patron->is_anonymous, 0, q{is_anonymous returns 0 if pref is not equal to patron's id} );
+
+    $schema->storage->txn_begin;
+
+    }