From 6e8bd8de6f0c9c763166a3022380d5e61448bab3 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 16 Jul 2025 09:56:13 +0200 Subject: [PATCH] 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 --- C4/Auth.pm | 6 +--- Koha/Patron.pm | 28 +++++++++++++------ circ/circulation.pl | 2 +- .../prog/en/modules/members/holdshistory.tt | 2 +- .../prog/en/modules/members/moremember.tt | 2 +- members/holdshistory.pl | 7 ----- members/moremember.pl | 4 --- members/readingrec.pl | 5 +--- t/db_dependent/Auth.t | 3 +- t/db_dependent/Koha/Patron.t | 26 ++++++++++++++++- 10 files changed, 51 insertions(+), 34 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 2fa9e2bed73..affc7ee43a2 100644 --- a/C4/Auth.pm +++ b/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, diff --git a/Koha/Patron.pm b/Koha/Patron.pm index ecfe7d40bb4..e5713dfe710 100644 --- a/Koha/Patron.pm +++ b/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 diff --git a/circ/circulation.pl b/circ/circulation.pl index 5d432b4f809..52871d12b90 100755 --- a/circ/circulation.pl +++ b/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; } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt index b1a4735c593..602338097bc 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt @@ -45,7 +45,7 @@ [% UNLESS Koha.Preference('IntranetReadingHistoryHolds') %]
Staff members are not allowed to access patron's holds history
- [% ELSIF is_anonymous %] + [% ELSIF patron.is_anonymous %]
This is the anonymous patron, so no holds history is displayed.
[% ELSE %] 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 f1fdb915376..83f4b8893bb 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/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 %]
This is the anonymous patron.
[% END %] diff --git a/members/holdshistory.pl b/members/holdshistory.pl index 9980542f0eb..1a72ec2368d 100755 --- a/members/holdshistory.pl +++ b/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, diff --git a/members/moremember.pl b/members/moremember.pl index 2c7a32b88b2..49b87e97d60 100755 --- a/members/moremember.pl +++ b/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; } diff --git a/members/readingrec.pl b/members/readingrec.pl index 907f3155fa3..b18ef5e0540 100755 --- a/members/readingrec.pl +++ b/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( diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index a2c0a77da7a..502838b91c1 100755 --- a/t/db_dependent/Auth.t +++ b/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; diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index 4f824aa782f..87e0f3c172a 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/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; + + } -- 2.34.1