Bug 8533 - Non-numeric cardnumbers screw up autoMemberNum
Summary: Non-numeric cardnumbers screw up autoMemberNum
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low minor (vote)
Assignee: Bugs List
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-31 12:10 UTC by Kyle M Hall
Modified: 2013-12-05 19:59 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 8533 - Non-numeric cardnumbers screw up autoMemberNum (1.03 KB, patch)
2012-07-31 12:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8533 - Non-numeric cardnumbers screw up autoMemberNum (1.04 KB, patch)
2012-07-31 15:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8533 - Non-numeric cardnumbers screw up autoMemberNum (1.18 KB, patch)
2012-08-01 12:12 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2012-07-31 12:10:08 UTC
Non-numeric cardnumbers screw up autoMemberNum. Even though autoMemberNum assumes a library is using numeric cardnumbers, it shouldn't go haywire if one ends up in the database. A simple fix is to select only on numeric cardnumbers only when getting the 'max' current cardnumber.
Comment 1 Kyle M Hall 2012-07-31 12:14:44 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2012-07-31 12:20:47 UTC
Test Plan:
1) Enable autoMemberNum
2) Check the value of the automatically generated cardnumber in memberentry.pl
3) Create a borrower with a cardnumber such as 'P13000'
4) Repeat step 2, your next automatic cardnumber should be way out of order.
5) Apply patch
6) Check the value of the automatic cardnumber in memberentry.pl, it should now be a sane value.
Comment 3 Kyle M Hall 2012-07-31 15:57:13 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2012-08-01 12:10:14 UTC
Hi Kyle,

I can't reproduce the issue without your patch.

I am under the impression, your patch is useless:

mysql> SELECT CAST( "P13000" AS SIGNED );
+----------------------------+
| CAST( "P13000" AS SIGNED ) |
+----------------------------+
|                          0 |
+----------------------------+

Indeed, a non-numeric cast as SIGNED is equal to 0

Maybe I missed something ?
Comment 5 Jonathan Druart 2012-08-01 12:11:07 UTC
However, mysql raises a warning with the previous query:
| Warning | 1292 | Truncated incorrect INTEGER value: 'P13000' |

Which it does not exist with your patch.

Then I will sign off your patch :)
Comment 6 Jonathan Druart 2012-08-01 12:12:07 UTC
Created attachment 11249 [details] [review]
Bug 8533 - Non-numeric cardnumbers screw up autoMemberNum

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Removes a warning from mysql:
    Truncated incorrect INTEGER value: 'P13000'
Comment 7 Mason James 2012-08-10 09:01:58 UTC
(In reply to comment #6)
> Created attachment 11249 [details] [review]
> Bug 8533 - Non-numeric cardnumbers screw up autoMemberNum
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
> Removes a warning from mysql:
>     Truncated incorrect INTEGER value: 'P13000'

patch looks good, passing QA - nice regex Kyle! :)


mason@xen1:~/git/head$ koha-qa.pl 
        * 38c7808 Bug 8533 - Non-numeric cardnumbers screw up autoMemberNum
                C4/Members.pm

- perlcritic-progressive tests... OK
- perl -c syntax tests... OK
- xt/tt_valid.t tests... OK
- xt/author/valid-template.t tests... OK
Comment 8 Paul Poulain 2012-08-31 16:24:46 UTC
Patch pushed. Please not that the query will be long to execute on a large borrowers table, as it's not well optimized by the SQL parser. (But that was already the case before your patch)
Comment 9 Chris Cormack 2012-09-01 07:46:45 UTC
Pushed to 3.8.x will be in 3.8.5