Description
Jesse Weaver
2016-05-11 22:13:47 UTC
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 |