Bug 20348

Summary: SIP2 patron identification fails to use userid
Product: Koha Reporter: Jose Martin <jose.i.martin.cuesta>
Component: SIP2Assignee: Jose Martin <jose.i.martin.cuesta>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: normal    
Priority: P5 - low CC: colin.campbell, fridolin.somers, jonathan.druart, josef.moravec, kohapatch, magnus, nick
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 17829    
Bug Blocks:    
Attachments: Replaces "or" with "||" in variable assignment
Patch with signoff
Bug 20348: SIP2 patron identification fails to use userid

Description Jose Martin 2018-03-07 08:19:46 UTC
The following code in C4/SIP/ILS/Patron.pm:

    $kp = Koha::Patrons->find( { cardnumber => $patron_id } )
        or Koha::Patrons->find( { userid => $patron_id } );

should be (using || instead of or):

    $kp = Koha::Patrons->find( { cardnumber => $patron_id } )
        || Koha::Patrons->find( { userid => $patron_id } );

It will never try to match a patron by userid.
This is a bug according to Perl documentation here: https://perldoc.perl.org/perlop.html#Logical-or-and-Exclusive-Or
Comment 1 Jose Martin 2018-03-07 12:27:10 UTC
Created attachment 72502 [details] [review]
Replaces "or" with "||" in variable assignment
Comment 2 Colin Campbell 2018-03-08 15:04:26 UTC
Created attachment 72539 [details] [review]
Patch with signoff

Signed off - added CR to commit message to avoid auto wrapping
added a line to clarify what was being fixed
Comment 3 Colin Campbell 2018-03-08 15:10:26 UTC
Confirmed that lookup by userid was not being executed and that reverting the or to || does correct that and restores the code to a working lookup.

People should beware of prettifying the code by replacing operators with operators that are not equivalent. Specially as in this case when the buggy change is hidden in a large commit which is not explicit about making the change
Comment 4 Josef Moravec 2018-03-09 08:02:52 UTC
Created attachment 72556 [details] [review]
Bug 20348: SIP2 patron identification fails to use userid

Replaces "or" with "||" in variable assignment

second attempt to retrieve borrower was not being executed due
to changed precedence

Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 5 Jonathan Druart 2018-03-19 16:29:31 UTC
Caused by
  commit 2b90ea2cb0e5e976de7ddef0151ae83d8ac578e6
    Bug 17829: Move GetMember to Koha::Patron
Comment 6 Jonathan Druart 2018-03-19 18:29:27 UTC
Patch pushed to master for 18.05

Thanks Jose for your first patch in!
Comment 7 Magnus Enger 2018-03-23 10:31:03 UTC
Hooray for Jose! This also needs to go into the stable branch, please.
Comment 8 Magnus Enger 2018-03-23 10:31:47 UTC
*** Bug 10372 has been marked as a duplicate of this bug. ***
Comment 9 Nick Clemens 2018-03-23 12:53:13 UTC
Awesome work all! Pushed to stable for 17.11.04
Comment 10 Fridolin Somers 2018-03-27 11:45:22 UTC
Not in 17.05.x since Bug 17829 not in it