@@ -, +, @@ --- C4/Members.pm | 33 +++++--- .../prog/en/modules/members/memberentrygen.tt | 13 ++- members/memberentry.pl | 35 ++++----- t/Members/cardnumber.t | 80 ++++++++++++++++++++ t/Members/checkcardnumber.t | 66 ---------------- 5 files changed, 124 insertions(+), 103 deletions(-) create mode 100644 t/Members/cardnumber.t delete mode 100644 t/Members/checkcardnumber.t --- a/C4/Members.pm +++ a/C4/Members.pm @@ -1345,26 +1345,33 @@ sub checkcardnumber { return 1 if $sth->fetchrow_hashref; - if ( my $length = C4::Context->preference('CardnumberLength') ) { + 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 ( - $length =~ m|^\d+$| - and length $cardnumber == $length - ) { - return 0 + if ( $cardnumber_length =~ m|^\d+$| ) { + $min = $max = $cardnumber_length + if $cardnumber_length >= $min + and $cardnumber_length <= $max; } # Else assuming it is a range - else { - my $qr = qr|^\d{$length}$|; - return 0 - if $cardnumber =~ $qr; + elsif ( $cardnumber_length =~ m|(\d*),(\d*)| ) { + $min = $1 if $1 and $min < $1; + $max = $2 if $2 and $max > $2; } - return 1 + } - return 0; + return ( $min, $max ); } - =head2 getzipnamecity (OUEST-PROVENCE) take all info from table city for the fields city and zip --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ a/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. @@ -886,10 +889,12 @@ - [% IF minlength_cardnumber && maxlength_cardnumber %] + [% IF minlength_cardnumber == maxlength_cardnumber %] + + [% ELSIF minlength_cardnumber && maxlength_cardnumber %] [% ELSE %] - + [% END %] [% IF ( mandatorycardnumber ) %]Required[% END %]
  • --- a/members/memberentry.pl +++ a/members/memberentry.pl @@ -294,9 +294,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'}); @@ -751,23 +756,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; --- a/t/Members/cardnumber.t +++ a/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 } ); +} --- a/t/Members/checkcardnumber.t +++ a/t/Members/checkcardnumber.t @@ -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 } ); -} --