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>
}
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;
- }
- else {
- return 0;
+ 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
}
-}
+ return 0;
+}
=head2 getzipnamecity (OUEST-PROVENCE)
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)
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).
[% 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>
<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">
<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>
$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 {
--- /dev/null
+#!/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 } );
+}
}
}
}
-
+
+ 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) {
projects / koha_fer / commitdiff