Description
Nick Clemens (kidclamp)
2016-12-15 14:17:50 UTC
Created attachment 58203 [details] [review] Bug 17782 - Patron updated_on field should be set to current timestamp when borrower is deleted To test: 01 Find a patron 02 Get the updated_on value from the db in borrowers table 03 Delete the patron 04 Get the update_on value from the db on deletedborrowers table 05 Values from 02 and 04 are the same 06 Apply patch 07 Repeat 01-04 08 Values should now be different Nick, is this ready to be tested? (In reply to Jonathan Druart from comment #2) > Nick, is this ready to be tested? Yes, should be, forgot to change status Hi Nick After applying the patch, the updated_on field in deletedborrowers hasn't been populated after deleting the patron. They appear in the table, but the field is blank. Nick, I did not test, but it does not work that way. If you want DBIx::Class to use the default value you must not pass the key. So: delete $patron_infos->{updated_on}; should work. Created attachment 58890 [details] [review] Bug 17782 - Patron updated_on field should be set to current timestamp when borrower is deleted To test: 01 Find a patron 02 Get the updated_on value from the db in borrowers table 03 Delete the patron 04 Get the update_on value from the db on deletedborrowers table 05 Values from 02 and 04 are the same 06 Apply patch 07 Repeat 01-04 08 Values should now be different I thought it worked the other way, this is better though :-) Updated patch (In reply to Jonathan Druart from comment #5) > Nick, I did not test, but it does not work that way. If you want DBIx::Class > to use the default value you must not pass the key. > So: > delete $patron_infos->{updated_on}; > should work. Created attachment 58913 [details] [review] Bug 17782 - Patron updated_on field should be set to current timestamp when borrower is deleted To test: 01 Find a patron 02 Get the updated_on value from the db in borrowers table 03 Delete the patron 04 Get the update_on value from the db on deletedborrowers table 05 Values from 02 and 04 are the same 06 Apply patch 07 Repeat 01-04 08 Values should now be different Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> Please provide tests (t/db_dependent/Koha/Patrons.t -> move_to_deleted). Note that at one test does not pass on my devbox: # Failed test 'Koha::Patron->move_to_deleted should have correctly moved the patron to the deleted table' # at t/db_dependent/Koha/Patrons.t line 331. # Structures begin differing at: # $got->{updated_on} = '2017-01-13 11:42:27' # $expected->{updated_on} = '2017-01-13 10:42:26' system date is 2017-01-13 11:42:27 I do not understand what's going on actually. This failed the test when I tried to sign off this evening. The timestamp didn't change on the second deletion. Created attachment 59558 [details] [review] Bug 17782 - (QA Followup) Fix tests prove t/db_dependent/Koha/Patrons.t Created attachment 59573 [details] [review] Bug 17782: Try to fix tests Previous code was wrong, $retrieved_patron->{updated_on} is always undef, $retrieved_patron is a Koha::Object Created attachment 59583 [details] [review] Bug 17782 - Allow discard_changes to work for Deletedborrower class Created attachment 59605 [details] [review] Bug 17782 - Patron updated_on field should be set to current timestamp when borrower is deleted To test: 01 Find a patron 02 Get the updated_on value from the db in borrowers table 03 Delete the patron 04 Get the update_on value from the db on deletedborrowers table 05 Values from 02 and 04 are the same 06 Apply patch 07 Repeat 01-04 08 Values should now be different Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 59606 [details] [review] Bug 17782 - (QA Followup) Fix tests prove t/db_dependent/Koha/Patrons.t Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 59607 [details] [review] Bug 17782: Fix tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Let's deal with deletedborrowers.borrowernumber as a PK on another bug report. (In reply to Jonathan Druart from comment #17) > Let's deal with deletedborrowers.borrowernumber as a PK on another bug > report. See bug 18003. Pushed to master for 17.05, thanks Nick, Jonathan! This patch has been pushed to 16.11.x and will be in 16.11.04. Note: updated_on is also present in 3.22.x, so possibly needs backporting all the way down. Created attachment 60047 [details] [review] [3.22.x] Bug 17782: deletedborrowers.updated_on should be set to current timestamp Pushed to 3.22.x for 3.22.17 Pushed to 16.05.x, for 16.05.10 release |