gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 10861: Add a check on cardnumber length 

Some libraries would like to add a check on the cardnumber length.
This patch adds the ability to restrict the cardnumber to a specific
length (strictly equal to XX, or length > XX or min < length < max).
This restriction is checked on inserting/updating a patron or on importing
patrons.

This patch adds:
- 1 new syspref CardnumberLength. 2 formats: a number or a range
  (xx,yy).

- 1 new unit test file t/Members/checkcardnumber.t for the
C4::Members::checkcardnumber routine.

Test plan:
1/ Fill the pref CardnumberLength with '5,8'
2/ Create a new patron with an invalid cardnumber (123456789)
3/ Check that you cannot save
4/ With Firebug, replace the pattern attribute value (for the cardnumber
input) with ".{5,10}"
5/ You are allowed to save but an error occurred.
6/ Try the same steps for update.
7/ Go to the import borrowers tool.
8/ Play with the import borrowers tool. We must test add/update patrons
and the "record matching" field (cardnumber or a uniq patron attribute)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested adding, updating; importing and ran unit test.
Preliminary QA comments on Bugzilla

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-04-19 09:58:02 +02:00 committed by Galen Charlton
parent fd773f8e79
commit 834abff631
8 changed files with 166 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
C4/Members.pm
View File

@ -1325,25 +1325,40 @@ sub checkuniquemember {
}
sub checkcardnumber {
my ($cardnumber,$borrowernumber) = @_;
my ( $cardnumber, $borrowernumber ) = @_;
# If cardnumber is null, we assume they're allowed.
return 0 if !defined($cardnumber);
return 0 unless defined $cardnumber;
my $dbh = C4::Context->dbh;
my $query = "SELECT * FROM borrowers WHERE cardnumber=?";
$query .= " AND borrowernumber <> ?" if ($borrowernumber);
my $sth = $dbh->prepare($query);
if ($borrowernumber) {
$sth->execute($cardnumber,$borrowernumber);
} else {
$sth->execute($cardnumber);
}
if (my $data= $sth->fetchrow_hashref()){
return 1;
my $sth = $dbh->prepare($query);
$sth->execute(
$cardnumber,
( $borrowernumber ? $borrowernumber : () )
);
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
}
else {
return 0;
}
}
return 0;
}
=head2 getzipnamecity (OUEST-PROVENCE)

10
installer/data/mysql/updatedatabase.pl
View File

@ -8046,6 +8046,16 @@ if (CheckVersion($DBversion)) {
SetVersion($DBversion);
}
$DBversion = "3.15.00.XXX";
if ( CheckVersion($DBversion) ) {
$dbh->do("
INSERT INTO systempreferences (variable,value,options,explanation,type)
VALUES('CardnumberLength', '', '', 'Set a length for card numbers.', 'Free');
");
print "Upgrade to $DBversion done (Bug 10861: Add CardnumberLength syspref)\n";
SetVersion ($DBversion);
}
=head1 FUNCTIONS
=head2 TableExists($table)

4
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
View File

@ -133,3 +133,7 @@ Patrons:
yes: Do
no: "Don't"
- enable the ability to upload and attach arbitrary files to a borrower record.
-
- Card numbers for patrons must be strictly equal to
- pref: CardnumberLength
- characters. The length can be a simple number or a range separated by a coma (xx,yy).

34
koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
View File

@ -178,9 +178,9 @@
[% 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.</li>
[% END %]
[% IF ( ERROR_cardnumber ) %]
<li id="ERROR_cardnumber">Cardnumber already in use or not in a good format.</li>
[% END %]
[% IF ( ERROR_age_limitations ) %]
<li id="ERROR_age_limitations">Patron's age is incorrect for their category.
Ages allowed are [% age_low %]-[% age_high %].</li>
@ -886,17 +886,23 @@
<fieldset class="rows" id="memberentry_library_management">
<legend id="library_management_lgd">Library management</legend><ol>
[% UNLESS nocardnumber %]
<li> [% IF ( mandatorycardnumber ) %]
<label for="cardnumber" class="required">
[% ELSE %]
<label for="cardnumber">
[% END %]
Card number: </label>
<input type="text" id="cardnumber" name="cardnumber" size="20" value="[% cardnumber %]" />
[% IF ( mandatorycardnumber ) %]<span class="required">Required</span>[% END %]</li>
[% END %]
[% UNLESS nobranchcode %]
[% UNLESS nocardnumber %]
<li>
[% IF ( mandatorycardnumber ) %]
<label for="cardnumber" class="required">
[% ELSE %]
<label for="cardnumber">
[% END %]
Card number: </label>
[% IF 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 %]" />
[% END %]
[% IF ( mandatorycardnumber ) %]<span class="required">Required</span>[% END %]
</li>
[% END %]
[% UNLESS nobranchcode %]
<li>
<label for="libraries" class="required">Library:</label>
<select name="branchcode" size="1" id="libraries">

6
koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt
View File

@ -72,6 +72,12 @@
<br /><code>[% missing_critical.lineraw %]</code>
</li>
[% END %]
[% IF ERROR.invalid_cardnumber %]
<li class="line_error">
Cardnumber [% ERROR.cardnumber %] is not a valid cardnumber
[% IF ERROR.borrowernumber %] (for patron with borrowernumber [% ERROR.borrowernumber %])[% END %]
</li>
[% END %]
[% END %]
</ul>
</div>

20
members/memberentry.pl
View File

@ -711,7 +711,25 @@ 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
);
}
}
output_html_with_http_headers $input, $cookie, $template->output;
sub parse_extended_patron_attributes {

66
t/Members/checkcardnumber.t Normal file
View File

@ -0,0 +1,66 @@
#!/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 } );
}

13
tools/import_borrowers.pl
View File

@ -231,7 +231,18 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
}
}
}
if ( C4::Members::checkcardnumber( $borrower{cardnumber}, $borrowernumber ) ) {
push @errors, {
invalid_cardnumber => 1,
borrowernumber => $borrowernumber,
cardnumber => $borrower{cardnumber}
};
$invalid++;
next;
}
if ($borrowernumber) {
# borrower exists
unless ($overwrite_cardnumber) {