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 = ?)
Created attachment 51469 [details] [review] Bug 16504: Add regression tests
Created attachment 51470 [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
Created attachment 51508 [details] [review] [SIGNED-OFF] Bug 16504: Add regression tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Created attachment 51509 [details] [review] [SIGNED-OFF] 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>
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>
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>
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>
Thanks for regression test. Could have been little more explicit but is sufficient.
(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"?
(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
Pushed to Master - Should be in the November 2016 release. Thanks!
Pushed in 16.05. Will be in 16.05.01.
Patches pushed to 3.22.x, will be in 3.22.8