Description
Katrin Fischer
2015-01-05 15:03:43 UTC
*** Bug 19699 has been marked as a duplicate of this bug. *** Upping severity a bit - keeping potential sensitive data about patrons with no way to delete them aside from SQL is not good. Created attachment 83367 [details] [review] Bug 13515: Add a foreign key constraint to messages.borrowernumber This patch adds a missing foreign key constraint to mesages.borrowernumber. To create it sucessfully the entries from the messages table that are not linked with an existing patron will be removed. Test plan: 0/ Do not apply the patch 1/ Add messages to different patrons 2/ Delete one of the patron's record 3/ Have a look at the messages table and notice that the messages for the deleted patron's record still appear 4/ Apply the patch and execute the update DB entry 5/ Have a look again at the messages table and notice that the messages for the deleted patron's record have been removed 6/ Delete a patron that have messages 7/ Notice that now the messages are deleted when the patron's record is deleted This will need tests. Created attachment 83463 [details] [review] Bug 13515: Add a foreign key constraint to messages.borrowernumber This patch adds a missing foreign key constraint to mesages.borrowernumber. To create it sucessfully the entries from the messages table that are not linked with an existing patron will be removed. Test plan: 0/ Do not apply the patch 1/ Add messages to different patrons 2/ Delete one of the patron's record 3/ Have a look at the messages table and notice that the messages for the deleted patron's record still appear 4/ Apply the patch and execute the update DB entry 5/ Have a look again at the messages table and notice that the messages for the deleted patron's record have been removed 6/ Delete a patron that have messages 7/ Notice that now the messages are deleted when the patron's record is deleted Signed-off-by: Devinim <kohadevinim@devinim.com.tr> Created attachment 83508 [details] [review] Bug 13515: Add unit tests Created attachment 83509 [details] [review] Bug 13515: Add a foreign key constraint to messages.borrowernumber This patch adds a missing foreign key constraint to mesages.borrowernumber. To create it sucessfully the entries from the messages table that are not linked with an existing patron will be removed. Test plan: 0/ Do not apply the patch 1/ Add messages to different patrons 2/ Delete one of the patron's record 3/ Have a look at the messages table and notice that the messages for the deleted patron's record still appear 4/ Apply the patch and execute the update DB entry 5/ Have a look again at the messages table and notice that the messages for the deleted patron's record have been removed 6/ Delete a patron that have messages 7/ Notice that now the messages are deleted when the patron's record is deleted Signed-off-by: Nazlı Çetin <nazli@devinim.com.tr> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 83510 [details] [review] Bug 13515: Add unit tests QA is ok for the patch. I'd like a second QA point of view for the unit tests i wrote. The update doe snot appear to be idempotent - can you check the condition? The tests seem to cover the basic case (In reply to Nick Clemens from comment #10) > The update doe snot appear to be idempotent - can you check the condition? > Yep. Constraint nameis wrong in foreign_key_exists call. Need a follow-up Created attachment 85138 [details] [review] Bug 13515: Fix contrainst name before deleting it Created attachment 85155 [details] [review] Bug 13515: Add a foreign key constraint to messages.borrowernumber This patch adds a missing foreign key constraint to mesages.borrowernumber. To create it sucessfully the entries from the messages table that are not linked with an existing patron will be removed. Test plan: 0/ Do not apply the patch 1/ Add messages to different patrons 2/ Delete one of the patron's record 3/ Have a look at the messages table and notice that the messages for the deleted patron's record still appear 4/ Apply the patch and execute the update DB entry 5/ Have a look again at the messages table and notice that the messages for the deleted patron's record have been removed 6/ Delete a patron that have messages 7/ Notice that now the messages are deleted when the patron's record is deleted Signed-off-by: Nazlı Çetin <nazli@devinim.com.tr> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 85156 [details] [review] Bug 13515: Add unit tests Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 85157 [details] [review] Bug 13515: Fix constraint name before deleting it Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Awesome work all! Pushed to master for 19.05 Pushed to 18.11.x for 18.11.04. Pushed to 18.05.x for 18.05.10 I prefer not to impact 17.11.x database structure. |