search

pdokoupil / EasyStudy

6 stars 9 forks source link

Deleting an user-study may cause inconsistency (ndbi021) #4

Closed luk27official closed 3 months ago

luk27official commented 3 months ago

This applies to the ndbi021 branch, not sure about main.

In the current implementation, when an user study is deleted from the system, the study is deleted from the DB entirely. https://github.com/pdokoupil/EasyStudy/blob/284e32c242831a01483b5a9e96c331d6b47518ef/server/main.py#L112-L119

I believe that a much better approach would be to have a flag and mark the user studies as deleted with the flag. The problem is that on every creation of a new user study, the nearest bigger non-used primary ID is used for the newly created study.

Suppose the following: I have created 10 user studies, so the last id is 10. Then I create an 11th study which gets the id 11. I let a few users complete the study and when I'm finished, I delete it. Then, I create another user study, and it receives the same id = 11 (as my last saved id in the DB is 10, 11 got deleted). But then, I'm not really able to tell which interactions, participations etc. are relevant for the last study as both of them have the same user study id in the DB.

This means that the only way to somehow deduce the real study would be somehow from the timestamps which seems really tedious, and more importantly, the user might not realize this when evaluating the user studies.

pdokoupil commented 3 months ago

@luk27official Thank you for reporting the issue, this is a good catch. It seems the default behavior of primary keys in SQLite is different than what I expected, and the columns must be explicitly decorated with AUTOINCREMENT (https://www.sqlite.org/autoinc.html) to achieve the desired behavior.

The deletion procedure was somewhat broken overall, as it only removed the study, instead of all corresponding data (participants, messages, and interactions).

The issue should now be resolved in NDBI021 (by https://github.com/pdokoupil/EasyStudy/pull/5)

Where needed, the foreign keys now use ondelete='CASCADE' to ensure all interactions and other user study-related data get removed upon user study deletion. The newly set autoincrement property ensures that we will no longer reuse IDs since the new ID is set to an ID that is one larger than the largest ID that has ever existed in that same table.

luk27official commented 3 months ago

It seems like it is working for me. Thanks :)