[Bug 15479] New: SIPserver rejects renewals for patrons with alphanumeric cardnumbers
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15479 Bug ID: 15479 Summary: SIPserver rejects renewals for patrons with alphanumeric cardnumbers Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: SIP2 Assignee: koha-bugs@lists.koha-community.org Reporter: colin.campbell@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org CC: colin.campbell@ptfs-europe.com If you allow non-numeric characters in patron barcodes they may be stored in either lower or uppercase in the borrowers table which is defined with a _ci (case insensitive) collation and therefore treats both forms as equivalent. However when the sipserver processes an issue transaction (used by most devices for both issue and renew) the test to see if the issue is in fact a renew uses a case sensitive comparison of the incoming cardnumber and that in the database causing the renewal to be rejected as 'Item checked out to another patron' e.g. this client sends cardnumber A17 but the database has stored this as 'a17' Request:11YN20160106 12164220160828 093005AO|AAA17|AB502326000767|AC|AD|BOY|AY1AZED86 Response:120NUN20160106 121642AOCPL|AAA17|AB502326000767|AJProzac nation [region 2] [import]|AH|AFItem checked out to another patron|BLY|AY1AZD57E[0d][0a] Request:11YN20160106 12164220160828 093005AO|AAA17|AB502326000755|AC|AD|AY2AZEEEE Response:120NUN20160106 121642AOCPL|AAA17|AB502326000755|AJAdvanced programming in the UNIX environment /|AH|AFItem checked out to another patron|BLY|AY2AZD100[0d][0a] Patron status reveals cardnumber as stored and that the two items are indeed on loan to this patron: Request:6302420160106 121642 Y AOCPL|AAA17|AC|AD|AY3AZF34D Response:64 02420160106 121642000000000002000000000000AOCPL|AAa17|AEFHT Trial|BLY|CQN|BV2.00|CC5|AU502326000767|AU502326000755|PCST|PIY|AFGreetings from Koha. -- Patron owes 2.00|AY3AZCE4B[0d][0a] Comparison in code should use same type of comparison as the database -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15479 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |colin.campbell@ptfs-europe. |ity.org |com Status|NEW |Needs Signoff --- Comment #1 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Created attachment 46326 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46326&action=edit Proposed Patch Patch uses a case insensitive comparison to get correct result. To test issue a checkout for a borrower whith an item already on loan to them and where the request uses a different case to the db see examples in bug -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15479 --- Comment #2 from Colin Campbell <colin.campbell@ptfs-europe.com> --- With patch applied repeating the requests listed in the bug result in issues being correctly identified and processed as renewals: Request:11YN20160106 12224420160828 093005AO|AAA17|AB502326000767|AC|AD|BOY|AY1AZED87 Response:120NUN20160106 122244AOCPL|AAA17|AB502326000767|AJProzac nation [region 2] [import]|AH|AFNO_MORE_RENEWALS: 1|BLY|BV2.00|BHUSD|BT06|AY1AZD77D[0d][0a] Request:11YN20160106 12224420160828 093005AO|AAA17|AB502326000755|AC|AD|AY2AZEEEF Response:121YNY20160106 122245AOCPL|AAA17|AB502326000755|AJAdvanced programming in the UNIX environment /|AH20160108 235900|AFItem already checked out to you: renewing item.|AY2AZCA9E[0d][0a] -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15479 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #3 from Frédéric Demians <frederic@tamil.fr> --- I get the reasoning, and agree with it. Do you think you could cook a unit test to check this case? A UT which fails without your patch, and succeed with it? And a minor point of detail: why do you use a function rather than a regex in your test. For example: $item->{patron} && $item->{patron} !~ /^$patron_id$/i -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15479 --- Comment #4 from Colin Campbell <colin.campbell@ptfs-europe.com> --- (In reply to Frédéric Demians from comment #3)
I get the reasoning, and agree with it. Do you think you could cook a unit test to check this case? A UT which fails without your patch, and succeed with it?
And a minor point of detail: why do you use a function rather than a regex in your test. For example:
$item->{patron} && $item->{patron} !~ /^$patron_id$/i
Readability it might be to easy for the eye to skip over the regex, and as the comparison is called twice ensure they both make the same test. Supplementary patch tests the sub's returns as well as adding basic tests for ILS.pm inteface -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15479 --- Comment #5 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Created attachment 47105 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=47105&action=edit Add test for routine Add a test for the routine added in bug 15479 Adds a script making basic tests to ILS.pm -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org