Description
Eric Phetteplace
2021-10-01 23:03:05 UTC
Small update to this - looks like DOB is not one of the fields that can be bulk updated with /cgi-bin/koha/tools/modborrowers.pl either so that makes the problem more annoying. We can't bulk update a bunch of users and then erase all the DOB data afterwards. I know ByWater could run a SQL query for us but it just adds one extra step. I confirm the bug exists. Will take a look tomorrow. It affect date/date-time attributes. Created attachment 125909 [details] [review] Bug 29157: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 125910 [details] [review] Bug 29157: Date and Datetime attributes cannot be set NULL Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 125994 [details] [review] Bug 29157: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Created attachment 125995 [details] [review] Bug 29157: Date and Datetime attributes cannot be set NULL Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Tested by: 1. Applying regression tests patch 2. Run tests - these should fail: prove t/db_dependent/api/v1/patrons.t 3. Apply second patch 4. Run tests again - they should now pass 5. Sign-off! Hope that is sufficient. (In reply to David Nind from comment #7) > Tested by: > > 1. Applying regression tests patch > 2. Run tests - these should fail: prove t/db_dependent/api/v1/patrons.t > 3. Apply second patch > 4. Run tests again - they should now pass > 5. Sign-off! > > Hope that is sufficient. Thanks, I forgot to add *exactly* that! The unit test is in the wrong place, it should test the attributes_from_api function directly, please add it to t/db_dependent/Koha/Object.t. Otherwise the fix looks good. (In reply to Joonas Kylmälä from comment #9) > The unit test is in the wrong place, it should test the attributes_from_api > function directly, please add it to t/db_dependent/Koha/Object.t. Otherwise > the fix looks good. Fair enough. I will of course leave the tests I added first, so we can track breaking this in another place of the stack. Created attachment 126128 [details] [review] Bug 29157: Regression tests This patch implements regression tests for the filed bug. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Created attachment 126129 [details] [review] Bug 29157: Date and Datetime attributes cannot be set NULL This patch makes Koha::Object->attribute_from_api allow setting attributes the undef value. The original implementation passed the value directly to dt_from_string, which made the attribute be set the current date. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/api/v1/patrons.t => FAIL: Tests fail! Our code is buggy! 3. Apply this patch 4. Repeat 2 => SUCCESS! Fix fixed this thing! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Created attachment 126220 [details] [review] Bug 29157: Regression tests I don't think this needs a third sign-off but I wanted to give it a try. Looks good! Thanks for getting to this so quickly! Created attachment 126221 [details] [review] Bug 29157: Date and Datetime attributes cannot be set NULL Signing off on the other patch. Created attachment 126237 [details] [review] Bug 29157: Regression tests This patch implements regression tests for the filed bug. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Phetteplace <phette23@gmail.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Created attachment 126238 [details] [review] Bug 29157: Date and Datetime attributes cannot be set NULL This patch makes Koha::Object->attribute_from_api allow setting attributes the undef value. The original implementation passed the value directly to dt_from_string, which made the attribute be set the current date. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/api/v1/patrons.t => FAIL: Tests fail! Our code is buggy! 3. Apply this patch 4. Repeat 2 => SUCCESS! Fix fixed this thing! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Eric Phetteplace <phette23@gmail.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Pushed to master for 21.11, thanks to everybody involved! Pushed to 21.05.x for 21.05.05 Pushed to 20.11.x for 20.11.11 Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed. |