|
Description
Jonathan Druart
2019-12-18 13:03:37 UTC
Created attachment 96418 [details] [review] Bug 24263: Add tests Created attachment 96419 [details] [review] Bug 24263: DB changes Created attachment 96420 [details] [review] Bug 24263: Replace borrowers.relationship with NULL when was empty string When you create a new patron, its relationship field is populated with an empty string when we are expecting NULL. Otherwise the about page will display a warning about a missing/wrong relationship in data (that is not in the syspref) Test plan: 0/ Do not apply the patches from this bug report 1/ Create a new patron (child) 2/ Notice that relationship column in DB is set to an empty string 3/ Go to the about page, notice the invalid warning about relationship values not in the syspref 4/ Apply DB changes and tests 5/ Execute the update DB entry => The value in DB is now NULL when was "" 6/ Execute the tests => It fails 7/ Apply the last patch 8/ Add and edit a patron (child) => relationship is set to NULL in DB 9/ Execute the tests => They pass! Upping severity, it should follow the integration of bug 24215 in stable branches. Created attachment 96531 [details] [review] Bug 24263: Add tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 96532 [details] [review] Bug 24263: DB changes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 96533 [details] [review] Bug 24263: Replace borrowers.relationship with NULL when was empty string When you create a new patron, its relationship field is populated with an empty string when we are expecting NULL. Otherwise the about page will display a warning about a missing/wrong relationship in data (that is not in the syspref) Test plan: 0/ Do not apply the patches from this bug report 1/ Create a new patron (child) 2/ Notice that relationship column in DB is set to an empty string 3/ Go to the about page, notice the invalid warning about relationship values not in the syspref 4/ Apply DB changes and tests 5/ Execute the update DB entry => The value in DB is now NULL when was "" 6/ Execute the tests => It fails 7/ Apply the last patch 8/ Add and edit a patron (child) => relationship is set to NULL in DB 9/ Execute the tests => They pass! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Works as expected, Signing off. Why don't we throw an exception instead? It's following an existing pattern (reset or modify an invalid value in -> store). Throwing an exception here would mean to catch it in all our Koha::Patron->store. That is much more work, especially to fix a major bug that impacts stable branches. Fair enough. We will take care of those at some point. Should this be set to PQA Tomas? The comments read like it should. Created attachment 96989 [details] [review] Bug 24263: Add tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 96990 [details] [review] Bug 24263: DB changes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 96991 [details] [review] Bug 24263: Replace borrowers.relationship with NULL when was empty string When you create a new patron, its relationship field is populated with an empty string when we are expecting NULL. Otherwise the about page will display a warning about a missing/wrong relationship in data (that is not in the syspref) Test plan: 0/ Do not apply the patches from this bug report 1/ Create a new patron (child) 2/ Notice that relationship column in DB is set to an empty string 3/ Go to the about page, notice the invalid warning about relationship values not in the syspref 4/ Apply DB changes and tests 5/ Execute the update DB entry => The value in DB is now NULL when was "" 6/ Execute the tests => It fails 7/ Apply the last patch 8/ Add and edit a patron (child) => relationship is set to NULL in DB 9/ Execute the tests => They pass! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 96992 [details] [review] Bug 24263: (QA follow-up) Atomic update fixes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Nice work everyone! Pushed to master for 20.05 Pushed to 19.11.x branch for 19.11.02 missing dependencies, no backport for 19.05.x |