Bugzilla – Attachment 184123 Details for
Bug 40401
Implement Koha::Patron->is_anonymous (was t/db_dependent/Auth.t generates warnings)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40401: Implement Koha::Patron->is_anonymous
Bug-40401-Implement-KohaPatron-isanonymous.patch (text/plain), 9.95 KB, created by
Jonathan Druart
on 2025-07-16 08:06:59 UTC
(
hide
)
Description:
Bug 40401: Implement Koha::Patron->is_anonymous
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2025-07-16 08:06:59 UTC
Size:
9.95 KB
patch
obsolete
>From 6e8bd8de6f0c9c763166a3022380d5e61448bab3 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >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') %] > <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 %] > >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 %] > <div class="alert alert-warning">This is the anonymous patron.</div> > [% 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40401
: 184123