devilry / devilry-django

Devilry project main repository
http://devilry.org
BSD 3-Clause "New" or "Revised" License
51 stars 24 forks source link

Previously uploaded files stops working when assignment group is split #1225

Closed torgeirl closed 2 years ago

torgeirl commented 2 years ago

Splitting an assignment group can cause previously uploaded files to be moved/deleted. This will lead to archive creating failing and server error when clicking the link in the feedback feed.

This seems to be a regression in Devilry as this didn't cause issues before.

Steps to reproduce

Based on Devilry core > Assignment groups it seems the assignment group split triggered the original group to be copied so perhaps the problem is that is the root of the problem.

Workaround It is possible to fix the error manually, but it requires some work:

StianJul commented 2 years ago

I can't reproduce this when simply splitting a group, but when removing one of the students split from the group I get this error (which I guess is what has happened here).

This makes sense since the specific problem here is that when you split a group (where files have been uploaded before the split), and then remove one of those students from the assignment, their copied comments and files are deleted. This has the unfortunate effect of also deleting the files, since all the copied commentfiles shares the same filepath (and we delete the files on that path).

The fix is simply to check if the filepath of the commentfile to be deleted is shared by any other commentfiles, which ensures that the file only finally deleted when the last commentfile with that filepath is deleted. The fix will be out soon.

torgeirl commented 2 years ago

I can't reproduce this when simply splitting a group, but when removing one of the students split from the group I get this error (which I guess is what has happened here).

This only match one of our examples.

In the other example, student A was split from the group where student B had uploaded a delivery. Student A was then added to another group. In both groups student B's comment with the file upload is shown in the feedback feed, but neither link works.

StianJul commented 2 years ago

In the other example, student A was split from the group where student B had uploaded a delivery. Student A was then added to another group. In both groups student B's comment with the file upload is shown in the feedback feed, but neither link works.

I have now tried to reproduce this without the fix:

  1. Student A and B are in a group.
  2. Student B uploads a file
  3. Student A is split from the group.
  4. Student A is added to a group with student C.

Result:

There might be something that I'm missing, but I can't quite see how this can occur. This is basically just moving students between groups within the same assignment, which means that none of the comments are actually deleted (and does not trigger the deletion of the actual file). I can't see that we delete the commentfiles other than when a student is removed from the assignemnt. But, with the new fix this should be solved anyways.

torgeirl commented 2 years ago

In the other example, student A was split from the group where student B had uploaded a delivery. Student A was then added to another group. In both groups student B's comment with the file upload is shown in the feedback feed, but neither link works.

I have now tried to reproduce this without the fix:

(...)

There might be something that I'm missing, but I can't quite see how this can occur.

I have the same problem reproducing the second example, which is why it took us two occurrence to describe the issue.

Either way the issue it both detectable and with our current setup fixable once it happens, so I believe we should fix the clear cut case for now.

StianJul commented 2 years ago

I agree. Release coming up!