Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 10861: (follow-up) various refactoring

Browse source 
This patch refactors the previous code and moves the logic from the pl
to a new routine.

Same test plan as previous patch.

/!\ new unit test filename.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 10861: Reintroduced the cardnumber length check (client side)

Previous patches has removed the pattern attribute of the input, it was
not needed. This patch reintroduces it. It will only work for new
browser version.

Moreover, it manages with the ',XX' format (see UT).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Squashed the last two follow-ups. The pattern test did not work fully for me
in Firefox 26 (very recent). But I see the message when I clear the field.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This commit is contained in:
Jonathan Druart 2013-12-11 19:06:15 +01:00 committed by Galen Charlton
parent c898f23f13
commit 3d938ffc82
5 changed files with 127 additions and 106 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
C4/Members.pm
View file

@ -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)

13
koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
View file

@ -178,8 +178,11 @@
[% IF ( ERROR_login_exist ) %]
<li id="ERROR_login_exist">Username/password already exists.</li>
[% END %]
[% IF ( ERROR_cardnumber ) %]
<li id="ERROR_cardnumber">Cardnumber already in use or not in a good format.</li>
[% IF ERROR_cardnumber_already_exists %]
<li id="ERROR_cardnumber">Cardnumber already in use.</li>
[% END %]
[% IF ERROR_cardnumber_length %]
<li id="ERROR_cardnumber">Cardnumber length is incorrect.</li>
[% END %]
[% IF ( ERROR_age_limitations ) %]
<li id="ERROR_age_limitations">Patron's age is incorrect for their category.
@ -894,10 +897,12 @@
<label for="cardnumber">
[% END %]
Card number: </label>
[% IF minlength_cardnumber && maxlength_cardnumber %]
[% IF minlength_cardnumber == maxlength_cardnumber %]
<input type="text" id="cardnumber" name="cardnumber" size="20" value="[% cardnumber %]" pattern=".{[% minlength_cardnumber %]}" title="Exactly [% minlength_cardnumber %] characters" />
[% ELSIF minlength_cardnumber && maxlength_cardnumber %]
<input type="text" id="cardnumber" name="cardnumber" size="20" value="[% cardnumber %]" pattern=".{[% minlength_cardnumber %],[% maxlength_cardnumber %]}" title="between [% minlength_cardnumber %] and [% maxlength_cardnumber %] characters" />
[% ELSE %]
<input type="text" id="cardnumber" name="cardnumber" size="20" value="[% cardnumber %]" />
<input type="text" id="cardnumber" name="cardnumber" size="20" value="[% cardnumber %]" />
[% END %]
[% IF ( mandatorycardnumber ) %]<span class="required">Required</span>[% END %]
</li>

35
members/memberentry.pl
View file

@ -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;

80
t/Members/cardnumber.t Normal file
View file

@ -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 } );
}

66
t/Members/checkcardnumber.t
View file

@ -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 } );
}