diff --git a/C4/Members.pm b/C4/Members.pm
index 664a6c41f7..9b358051f6 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -1341,25 +1341,32 @@ sub checkcardnumber {
return 1 if $sth->fetchrow_hashref;
- if ( my $length = C4::Context->preference('CardnumberLength') ) {
- # Is integer and length match
- if (
- $length =~ m|^\d+$|
- and length $cardnumber == $length
- ) {
- return 0
- }
- # Else assuming it is a range
- else {
- my $qr = qr|^\d{$length}$|;
- return 0
- if $cardnumber =~ $qr;
- }
- return 1
- }
+ my ( $min_length, $max_length ) = get_cardnumber_length();
+ return 2
+ if length $cardnumber > $max_length
+ or length $cardnumber < $min_length;
+
return 0;
}
+sub get_cardnumber_length {
+ my ( $min, $max ) = ( 1, 16 ); # borrowers.cardnumber is a varchar(16)
+ if ( my $cardnumber_length = C4::Context->preference('CardnumberLength') ) {
+ # Is integer and length match
+ if ( $cardnumber_length =~ m|^\d+$| ) {
+ $min = $max = $cardnumber_length
+ if $cardnumber_length >= $min
+ and $cardnumber_length <= $max;
+ }
+ # Else assuming it is a range
+ elsif ( $cardnumber_length =~ m|(\d*),(\d*)| ) {
+ $min = $1 if $1 and $min < $1;
+ $max = $2 if $2 and $max > $2;
+ }
+
+ }
+ return ( $min, $max );
+}
=head2 getzipnamecity (OUEST-PROVENCE)
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 8bceeb4197..0f28cef071 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
@@ -178,8 +178,11 @@
[% IF ( ERROR_login_exist ) %]
Username/password already exists.
[% END %]
- [% IF ( ERROR_cardnumber ) %]
- Cardnumber already in use or not in a good format.
+ [% IF ERROR_cardnumber_already_exists %]
+ Cardnumber already in use.
+ [% END %]
+ [% IF ERROR_cardnumber_length %]
+ Cardnumber length is incorrect.
[% END %]
[% IF ( ERROR_age_limitations ) %]
Patron's age is incorrect for their category.
@@ -894,10 +897,12 @@
- [% IF minlength_cardnumber && maxlength_cardnumber %]
+ [% IF minlength_cardnumber == maxlength_cardnumber %]
+
+ [% ELSIF minlength_cardnumber && maxlength_cardnumber %]
[% ELSE %]
-
+
[% END %]
[% IF ( mandatorycardnumber ) %]Required[% END %]
diff --git a/members/memberentry.pl b/members/memberentry.pl
index f9bc0c7551..28ab5e0782 100755
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -293,9 +293,14 @@ if ($op eq 'save' || $op eq 'insert'){
# If the cardnumber is blank, treat it as null.
$newdata{'cardnumber'} = undef if $newdata{'cardnumber'} =~ /^\s*$/;
- if (checkcardnumber($newdata{cardnumber},$newdata{borrowernumber})){
- push @errors, 'ERROR_cardnumber';
- }
+ if (my $error_code = checkcardnumber($newdata{cardnumber},$newdata{borrowernumber})){
+ push @errors, $error_code == 1
+ ? 'ERROR_cardnumber_already_exists'
+ : $error_code == 2
+ ? 'ERROR_cardnumber_length'
+ : ()
+ }
+
if ($newdata{dateofbirth} && $dateofbirthmandatory) {
my $age = GetAge($newdata{dateofbirth});
my $borrowercategory=GetBorrowercategory($newdata{'categorycode'});
@@ -711,23 +716,13 @@ if(defined($data{'contacttitle'})){
$template->param("contacttitle_" . $data{'contacttitle'} => "SELECTED");
}
-if ( my $cardnumber_length = C4::Context->preference('CardnumberLength') ) {
- my ( $min, $max );
- # Is integer and length match
- if ( $cardnumber_length =~ m|^\d+$| ) {
- $min = $max = $cardnumber_length;
- }
- # Else assuming it is a range
- elsif ( $cardnumber_length =~ m|(\d+),(\d*)| ) {
- $min = $1;
- $max = $2 || 16; # borrowers.cardnumber is a varchar(16)
- }
- if ( defined $min ) {
- $template->param(
- minlength_cardnumber => $min,
- maxlength_cardnumber => $max
- );
- }
+
+my ( $min, $max ) = C4::Members::get_cardnumber_length();
+if ( defined $min ) {
+ $template->param(
+ minlength_cardnumber => $min,
+ maxlength_cardnumber => $max
+ );
}
output_html_with_http_headers $input, $cookie, $template->output;
diff --git a/t/Members/cardnumber.t b/t/Members/cardnumber.t
new file mode 100644
index 0000000000..8132c93f10
--- /dev/null
+++ b/t/Members/cardnumber.t
@@ -0,0 +1,80 @@
+#!/usr/bin/env perl
+
+use Modern::Perl;
+use Test::More tests => 22;
+
+use Test::MockModule;
+use DBD::Mock;
+
+use_ok('C4::Members');
+
+my $module_context = new Test::MockModule('C4::Context');
+$module_context->mock(
+ '_new_dbh',
+ sub {
+ my $dbh = DBI->connect( 'DBI:Mock:', '', '' )
+ || die "Cannot create handle: $DBI::errstr\n";
+ return $dbh;
+ }
+);
+
+my $dbh = C4::Context->dbh;
+my $rs = [];
+
+my $pref = "10";
+set_pref( $module_context, $pref );
+is_deeply( [ C4::Members::get_cardnumber_length() ], [ 10, 10 ], '10 => min=10 and max=10');
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{123456789} ), 2, "123456789 is shorter than $pref");
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567890123456} ), 2, "1234567890123456 is longer than $pref");
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567890} ), 0, "1234567890 is equal to $pref");
+
+$pref = q|10,10|; # Same as before !
+set_pref( $module_context, $pref );
+is_deeply( [ C4::Members::get_cardnumber_length() ], [ 10, 10 ], '10,10 => min=10 and max=10');
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{123456789} ), 2, "123456789 is shorter than $pref");
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567890123456} ), 2, "1234567890123456 is longer than $pref");
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567890} ), 0, "1234567890 is equal to $pref");
+
+$pref = q|8,10|; # between 8 and 10 chars
+set_pref( $module_context, $pref );
+is_deeply( [ C4::Members::get_cardnumber_length() ], [ 8, 10 ], '8,10 => min=8 and max=10');
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{12345678} ), 0, "12345678 matches $pref");
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567890123456} ), 2, "1234567890123456 is longer than $pref");
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567} ), 2, "1234567 is shorter than $pref");
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567890} ), 0, "1234567890 matches $pref");
+
+$pref = q|8,|; # At least 8 chars
+set_pref( $module_context, $pref );
+is_deeply( [ C4::Members::get_cardnumber_length() ], [ 8, 16 ], '8, => min=8 and max=16');
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567} ), 2, "1234567 is shorter than $pref");
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567890123456} ), 0, "1234567890123456 matches $pref");
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567890} ), 0, "1234567890 matches $pref");
+
+$pref = q|,8|; # max 8 chars
+set_pref( $module_context, $pref );
+is_deeply( [ C4::Members::get_cardnumber_length() ], [ 1, 8 ], ',8 => min=1 and max=8');
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567} ), 0, "1234567 matches $pref");
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567890123456} ), 2, "1234567890123456 is longer than $pref");
+$dbh->{mock_add_resultset} = $rs;
+is( C4::Members::checkcardnumber( q{1234567890} ), 2, "1234567890 is longer than $pref");
+
+
+sub set_pref {
+ my ( $module, $value ) = @_;
+ $module->mock('preference', sub { return $value } );
+}
diff --git a/t/Members/checkcardnumber.t b/t/Members/checkcardnumber.t
deleted file mode 100644
index 94388b9912..0000000000
--- a/t/Members/checkcardnumber.t
+++ /dev/null
@@ -1,66 +0,0 @@
-#!/usr/bin/env perl
-
-use Modern::Perl;
-use Test::More tests =>14;
-
-use Test::MockModule;
-use DBD::Mock;
-
-use_ok('C4::Members');
-
-my $module_context = new Test::MockModule('C4::Context');
-$module_context->mock(
- '_new_dbh',
- sub {
- my $dbh = DBI->connect( 'DBI:Mock:', '', '' )
- || die "Cannot create handle: $DBI::errstr\n";
- return $dbh;
- }
-);
-
-my $dbh = C4::Context->dbh;
-my $rs = [];
-
-my $pref = "10";
-set_pref( $module_context, $pref );
-
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{123456789} ), 1, "123456789 is shorter than $pref");
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{12345678901234567890} ), 1, "12345678901234567890 is longer than $pref");
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{1234567890} ), 0, "1234567890 is equal to $pref");
-
-$pref = q|10,10|; # Same as before !
-set_pref( $module_context, $pref );
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{123456789} ), 1, "123456789 is shorter than $pref");
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{12345678901234567890} ), 1, "12345678901234567890 is longer than $pref");
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{1234567890} ), 0, "1234567890 is equal to $pref");
-
-$pref = q|8,10|; # between 8 and 10 chars
-set_pref( $module_context, $pref );
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{12345678} ), 0, "12345678 matches $pref");
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{12345678901234567890} ), 1, "12345678901234567890 is longer than $pref");
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{1234567} ), 1, "1234567 is shorter than $pref");
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{1234567890} ), 0, "1234567890 matches $pref");
-
-$pref = q|8,|; # At least 8 chars
-set_pref( $module_context, $pref );
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{1234567} ), 1, "1234567 is shorter than $pref");
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{12345678901234567890} ), 0, "12345678901234567890 matches $pref");
-$dbh->{mock_add_resultset} = $rs;
-is( C4::Members::checkcardnumber( q{1234567890} ), 0, "12345678 matches $pref");
-
-sub set_pref {
- my ( $module, $value ) = @_;
- $module->mock('preference', sub { return $value } );
-}