Bug 6521 - Editing a patron fails with blank cardnumber
Summary: Editing a patron fails with blank cardnumber
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: 3.4
Hardware: All All
: PATCH-Sent (DO NOT USE) normal (vote)
Assignee: Robin Sheat
QA Contact: Bugs List
URL:
Keywords:
: 6603 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-06-21 04:09 UTC by Robin Sheat
Modified: 2012-10-25 23:09 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 6521 - allow blank cardnumbers to not trigger "already in use" (3.76 KB, patch)
2011-06-21 04:42 UTC, Robin Sheat
Details | Diff | Splinter Review
follow up (1.17 KB, patch)
2011-07-20 17:26 UTC, Nicole C. Engard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Robin Sheat 2011-06-21 04:09:34 UTC
If it's permissible to have a blank cardnumber, only one person can have one of ''. Editing someone else who has no cardnumber fails with "Cardnumber already in use".

Instead, if the cardnumber is empty, it should save as undef.
Comment 1 Robin Sheat 2011-06-21 04:42:46 UTC
Created attachment 4511 [details] [review]
Bug 6521 - allow blank cardnumbers to not trigger "already in use"

Now a blank cardnumber will get stored as a null in the database, which
is a value that allows duplicates. As such, if cardnumbers aren't
mandatory, then you can actually save a user with it being blank.
Comment 2 Chris Cormack 2011-06-25 14:30:10 UTC
Patch pushed,please test
Comment 3 Ian Walls 2011-07-20 17:10:00 UTC
Problem:  if the library is using autoMemberNum, this patch prevents the cardnumber from being generated upon page load of members/memberentry.pl.  Seems to be this line that's the offender:

@@ -900,6 +900,7 @@ sub fixup_cardnumber ($) {
 
     #     if ($cardnumber !~ /\S/ && $autonumber_members) {
     ($autonumber_members) or return $cardnumber;
+    defined($cardnumber) or return $cardnumber;
     my $checkdigit = C4::Context->preference('checkdigit');
     my $dbh = C4::Context->dbh;
     if ( $checkdigit and $checkdigit eq 'katipo' ) {

If autonumber_members is defined, then cardnumber SHOULDN'T be, because we're auto-generating, and don't yet know what the max value is in the database. That call comes later in the script.

This subroutine is only called in two places:  member/memberentry.pl and tools/import_borrowers.pl.  In both cases, the undefined cardnumber should be passed to the subroutine, and given back as 'undef' if autoMemberNum is off, or the newly calculated cardnumber if ON.

Patch forthcoming.
Comment 4 Nicole C. Engard 2011-07-20 17:26:10 UTC
Follow up signed off.
Comment 5 Nicole C. Engard 2011-07-20 17:26:35 UTC
Created attachment 4681 [details] [review]
follow up
Comment 6 Ian Walls 2011-07-20 17:40:04 UTC
Problem:  if the library is using autoMemberNum, this patch prevents the cardnumber from being generated upon page load of members/memberentry.pl.  Seems to be this line that's the offender:

@@ -900,6 +900,7 @@ sub fixup_cardnumber ($) {
 
     #     if ($cardnumber !~ /\S/ && $autonumber_members) {
     ($autonumber_members) or return $cardnumber;
+    defined($cardnumber) or return $cardnumber;
     my $checkdigit = C4::Context->preference('checkdigit');
     my $dbh = C4::Context->dbh;
     if ( $checkdigit and $checkdigit eq 'katipo' ) {

If autonumber_members is defined, then cardnumber SHOULDN'T be, because we're auto-generating, and don't yet know what the max value is in the database. That call comes later in the script.

This subroutine is only called in two places:  member/memberentry.pl and tools/import_borrowers.pl.  In both cases, the undefined cardnumber should be passed to the subroutine, and given back as 'undef' if autoMemberNum is off, or the newly calculated cardnumber if ON.

Patch forthcoming.
Comment 7 Magnus Enger 2011-07-26 11:54:15 UTC
*** Bug 6603 has been marked as a duplicate of this bug. ***
Comment 8 Chris Cormack 2011-07-31 23:32:27 UTC
Follow up fix easy to test, and already signed off by QA manager, pushed
Comment 9 Katrin Fischer 2011-08-05 21:21:16 UTC
I can save 2 and more patrons without cardnumber and edit them. Tested on current master. Marking fixed.