From d949dcebb067a6aad547c3a6de9dac49261f0cd3 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 21 Jul 2023 12:11:21 +0200 Subject: [PATCH] Bug 34338: Do not allow leading or trailing whitespaces in cardnumber --- Koha/Policy/Patrons/Cardnumber.pm | 3 +++ installer/onboarding.pl | 3 +++ .../prog/en/includes/onboarding_messages.inc | 2 ++ .../prog/en/modules/members/memberentrygen.tt | 7 +++++-- .../opac-tmpl/bootstrap/en/modules/opac-memberentry.tt | 6 ++++-- members/memberentry.pl | 6 ++++-- opac/opac-memberentry.pl | 2 ++ t/db_dependent/Koha/Policy/Patrons/Cardnumber.t | 8 +++++++- 8 files changed, 30 insertions(+), 7 deletions(-) diff --git a/Koha/Policy/Patrons/Cardnumber.pm b/Koha/Policy/Patrons/Cardnumber.pm index 99cfffcfb17..83157b6a7bd 100644 --- a/Koha/Policy/Patrons/Cardnumber.pm +++ b/Koha/Policy/Patrons/Cardnumber.pm @@ -54,6 +54,9 @@ sub is_valid { return Koha::Result::Boolean->new(1) unless defined $cardnumber; + return Koha::Result::Boolean->new(0)->add_message( { message => "not_trimmed" } ) + if $cardnumber =~ m[^\s|\s$]; + return Koha::Result::Boolean->new(0)->add_message( { message => "already_exists" } ) if Koha::Patrons->search( { diff --git a/installer/onboarding.pl b/installer/onboarding.pl index 457f1fe278f..6913d8d0e56 100755 --- a/installer/onboarding.pl +++ b/installer/onboarding.pl @@ -148,6 +148,9 @@ if ( $step == 3 ) { if ( $message eq 'already_exists' ) { push @messages, { code => 'ERROR_cardnumber_already_exists' }; } + elsif ( $message eq 'not_trimmed' ) { + push @messages, { code => 'ERROR_cardnumber_not_trimmed' }; + } elsif ( $message eq 'invalid_length' ) { push @messages, { code => 'ERROR_cardnumber_length' }; } diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/onboarding_messages.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/onboarding_messages.inc index 135cc437877..9213aa46a21 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/onboarding_messages.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/onboarding_messages.inc @@ -14,6 +14,8 @@
The patron has not been created! Cardnumber or Userid may already exist.
[% CASE 'ERROR_cardnumber_already_exists' %]
Card number already in use.
+ [% CASE 'ERROR_cardnumber_not_trimmed' %] +
Card number contains leading or trailing whitespaces.
[% CASE 'ERROR_cardnumber_length' %]
Card number length is incorrect.
[% CASE 'ERROR_password_mismatch' %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt index 5d174b52889..e8415f927ac 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -133,10 +133,13 @@ legend:hover {
  • Username already exists or could not create unique new one.
  • [% END %] [% IF ERROR_cardnumber_already_exists %] -
  • Cardnumber already in use.
  • +
  • Card number already in use.
  • + [% END %] + [% IF ERROR_cardnumber_not_trimmed %] +
  • Card number contains leading or trailing whitespaces.
  • [% END %] [% IF ERROR_cardnumber_length %] -
  • Cardnumber length is incorrect.
  • +
  • Card number length is incorrect.
  • [% END %] [% IF ( ERROR_age_limitations ) %]
  • Patron's age is incorrect for their category. diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt index 89a9e841e0b..bcafb875234 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt @@ -124,12 +124,14 @@ [% END %] - [% IF cardnumber_wrong_length || cardnumber_already_exists %] + [% IF cardnumber_wrong_length || cardnumber_already_exists || cardnumber_not_trimmed %]
    [% IF cardnumber_wrong_length %] The entered card number is the wrong length. [% ELSIF cardnumber_already_exists %] The entered card number is already in use. + [% ELSIF cardnumber_not_trimmed %] + The entered card number contains leading or trailing whitespaces. [% END %] Please correct and resubmit.
    @@ -233,7 +235,7 @@ - [% IF borrower && !(cardnumber_wrong_length || cardnumber_already_exists) && action == 'edit' %] + [% IF borrower && !(cardnumber_wrong_length || cardnumber_already_exists || cardnumber_not_trimmed) && action == 'edit' %] [% borrower.cardnumber | html %] [% ELSE %] [% IF minlength_cardnumber == maxlength_cardnumber %] diff --git a/members/memberentry.pl b/members/memberentry.pl index fad9b713d6b..066360849ce 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -298,9 +298,11 @@ if ($op eq 'save' || $op eq 'insert'){ for my $m ( @{ $is_valid->messages } ) { my $message = $m->message; if ( $message eq 'already_exists' ) { - $template->param( ERROR_cardnumber_already_exists => 1 ); + push @errors, 'ERROR_cardnumber_already_exists'; + } elsif ( $message eq 'not_trimmed' ) { + push @errors, 'ERROR_cardnumber_not_trimmed'; } elsif ( $message eq 'invalid_length' ) { - $template->param( ERROR_cardnumber_length => 1 ); + push @errors, 'ERROR_cardnumber_length'; } } } diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 9af4a2371d3..31548e7aa02 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -144,6 +144,8 @@ if ( $action eq 'create' ) { my $message = $m->message; if ( $message eq 'already_exists' ) { $template->param( cardnumber_already_exists => 1 ); + } elsif ( $message eq 'not_trimmed' ) { + $template->param( cardnumber_not_trimmed => 1 ); } elsif ( $message eq 'invalid_length' ) { $template->param( cardnumber_wrong_length => 1 ); } diff --git a/t/db_dependent/Koha/Policy/Patrons/Cardnumber.t b/t/db_dependent/Koha/Policy/Patrons/Cardnumber.t index e08ccb3976a..043cf7c1a6b 100755 --- a/t/db_dependent/Koha/Policy/Patrons/Cardnumber.t +++ b/t/db_dependent/Koha/Policy/Patrons/Cardnumber.t @@ -31,7 +31,7 @@ my $builder = t::lib::TestBuilder->new; subtest 'is_valid' => sub { - plan tests => 23; + plan tests => 25; $schema->storage->txn_begin; @@ -61,6 +61,9 @@ subtest 'is_valid' => sub { $is_valid = $policy->is_valid( "123" ); ok( !$is_valid, "Invalid cardnumber length"); + $is_valid = $policy->is_valid( " 123" ); + ok( !$is_valid, "Invalid cardnumber length"); + my $pref = "10"; t::lib::Mocks::mock_preference('CardnumberLength', $pref); ok( !$policy->is_valid( q{123456789} ), "123456789 is shorter than $pref"); @@ -95,6 +98,9 @@ subtest 'is_valid' => sub { ok( $policy->is_valid(undef), "If cardnumber is null, we assume they're allowed") ; ok( !$policy->is_valid(""), "Empty string is not correct" ); + $is_valid = $policy->is_valid( " " ); + ok( !$is_valid, "Spaces are not allowed"); + $schema->storage->txn_rollback; }; -- 2.25.1