`cmsImportContest -u` deletes participations

[lucach]

When there is already a contest with some participations, updating it with cmsImportContest -u deletes its participations.

This only happens when the script is actually doing something (i.e., it does not happen when nothing has changed and the script is basically a NOP).

[lucach]

Update: the problem may be that this no longer holds after introducing Participation.

[nir-lavee]

Hi, any news regarding this issue? My setup relies a lot on cmsImportContest, and submissions disappearing without warning is a critical bug. Using v1.3.

For a quick local workaround, I tried adding ignore=set(["participations"]) to the following:

cms/cmscontrib/ImportContest.py

Line 105 in cf5b096

self._update_object(old_contest, contest)

If I understand correctly, this will prevent the script from touching existing participations (whether I want it to or not), but everything else will be updated as intended. Can anyone confirm that this is the case? It seems to work, and I prefer such behavior over the existing one by far, but I want to be sure that I'm not corrupting the database in some subtle way. Thanks!

(Edit: just to be safe I'll mention @lerks and @stefano-maggiolo in case you missed this or weren't notified).

[stefano-maggiolo]

I'm inclined to think this has always been broken. Did it ever work for you?

I think my investigation was similar to @nir-lavee 's:

1. ImportContest gets contest, tasks and participations from the loader, independently; in particular, the contest object does not have any task of participation.
2. Then it asks _update_object to update the contest; since tasks and participations are empty lists, it will clear them, this includes deleting the participations.
3. Later it readds all participations (that now don't clash because there are none in the DB anymode).

Why in 2 participations are deleted from DB whereas tasks only removed from the contest? I haven't followed on, my guess is that is due to contest_id being nullable in tasks but not null in participations.

My hunch is that the easiest fix should be to add tasks and participations to the contest object before 2, but then I think that in the way the code works now, inserting a participation in the middle would have disastrous effects (like, reassigning a submission to a different user). Need to look into it more.

[stefano-maggiolo]

Ok, quick update: I think the reason tasks and participations were dealt independently was exactly to avoid that problem in the last paragraph of my previous comment. So what seems to be the solution is the one @nir-lavee suggested, to ignore participations. Tasks seem instead to already work, and are readded correctly in ImportContest.

[nir-lavee]

Thanks for investigating!

Did it ever work for you?

No. Well, it did in older versions of course, before participations existed.

Previously I thought the expected behavior of cmsImportContest was to update existing participations as well (so I didn't mind skipping them because it's a rare use case for me). However, looking at the code again in light of what you said, this was never implemented or expected, correct? The _update_object line above would not perform such an update anyway. In other words, this is a fix and not just a workaround, and it is not a regression in any way as far as I can tell.

Would you consider pulling it and backporting it to 1.3?

The only hypothetical issue would be if someone is relying on this behavior to remove participations while preparing contests. After this fix, they will find that participations can only grow with each run of the script. This is a pretty remote possibility, far outweighed by the risk of losing submissions.

[stefano-maggiolo]

I agree with everything you say. And yes, this should be backported. Only note is that I think most of the time the issue in your last paragraph doesn't happen because the script will refuse to create new user/participations for usernames that already exist.

I suspect there might be some other problem with that code, let me look at it a little bit more before implementing this.

[stefano-maggiolo]

Reopening for 1.3 - I will backport only the quick fix (ignore participations).