Bug 16504

Summary: All borrower attribute values for a given code deleted if that attribute has branch limits
Product: Koha Reporter: Jesse Weaver <jweaver>
Component: PatronsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: critical    
Priority: P5 - low CC: brendan, f.demians, gmcharlt, jonathan.druart, julian.maurice, kyle.m.hall, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15163
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 15163    
Bug Blocks:    
Attachments: Bug 16504: Add regression tests
Bug 16504: (follow-up for bug 15163) Do not remove attributes of other patrons
[SIGNED-OFF] Bug 16504: Add regression tests
[SIGNED-OFF] Bug 16504: (follow-up for bug 15163) Do not remove attributes of other patrons
Bug 16504: Add regression tests
Bug 16504: (follow-up for bug 15163) Do not remove attributes of other patrons
Bug 16504: [QA Follow-up] Make koha-qa happy with two newlines

Description Jesse Weaver 2016-05-11 22:13:47 UTC
Bug 15163 introduced a regression for branch-limited patron attributes; when modifying a patron attribute with branch limits, all of the values for that attribute for every patron will be deleted due to a predecence mistake:

            WHERE b_branchcode = ? OR b_branchcode IS NULL
                AND borrowernumber = ?

is parsed as:

            WHERE b_branchcode = ? OR (b_branchcode IS NULL
                AND borrowernumber = ?)
Comment 1 Jonathan Druart 2016-05-13 14:00:43 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-05-13 14:00:47 UTC Comment hidden (obsolete)
Comment 3 Srdjan Jankovic 2016-05-16 06:07:39 UTC Comment hidden (obsolete)
Comment 4 Srdjan Jankovic 2016-05-16 06:08:53 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2016-05-31 12:24:50 UTC
Created attachment 51923 [details] [review]
Bug 16504: Add regression tests

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2016-05-31 12:24:56 UTC
Created attachment 51924 [details] [review]
Bug 16504: (follow-up for bug 15163) Do not remove attributes of other patrons

Simple patch for a silly error, this single line is going to fix a
critical bug.
If a patron attribute is limited to a library, all the values for that attributes
for every patrons will be deleted.

Test plan:
Create a patron attribute limited to a library
Set the the attribute for a patron
Set the the attribute for another patron
=> Without this patch applied, the attribute's value for the first
patron is deleted
=> With this patch applied, the 2 values exist in the DB after the
second edition

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2016-05-31 12:25:00 UTC
Created attachment 51925 [details] [review]
Bug 16504: [QA Follow-up] Make koha-qa happy with two newlines

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2016-05-31 12:27:03 UTC
Thanks for regression test.
Could have been little more explicit but is sufficient.
Comment 9 Jonathan Druart 2016-05-31 13:32:01 UTC
(In reply to Marcel de Rooy from comment #8)
> Thanks for regression test.
> Could have been little more explicit but is sufficient.

What do you mean by "more explicit"?
Comment 10 Marcel de Rooy 2016-05-31 13:41:27 UTC
(In reply to Jonathan Druart from comment #9)
> (In reply to Marcel de Rooy from comment #8)
> > Thanks for regression test.
> > Could have been little more explicit but is sufficient.
> 
> What do you mean by "more explicit"?

It is not directly obvious what happens here.
And that you still have one record of a limited type for the other patron, that would be deleted without this patch.
No big problem
Comment 11 Brendan Gallagher 2016-06-03 06:35:56 UTC
Pushed to Master - Should be in the November 2016 release. Thanks!
Comment 12 Frédéric Demians 2016-06-15 07:33:33 UTC
Pushed in 16.05. Will be in 16.05.01.
Comment 13 Julian Maurice 2016-06-16 07:52:53 UTC
Patches pushed to 3.22.x, will be in 3.22.8