refactor import to reuse task_files based on xml_id

[kkoehn]

This enables the reuse of task_file, model_solution and test objects as discussed in #1320

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.16%. Comparing base (0161c22) to head (9467b18).
Report is 28 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1612      +/-   ##
==========================================
+ Coverage   94.14%   94.16%   +0.02%     
==========================================
  Files         123      124       +1     
  Lines        3004     3034      +30     
==========================================
+ Hits         2828     2857      +29     
- Misses        176      177       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[MrSerth]

I've tested the changes locally and think everything works as expected 👍

There is just a comment left regarding the implementation.

[MrSerth]

Very nice, thank you! 🦚

[MrSerth]

Sorry to add another issue to this PR, but I noticed an actual issue when updating changed tasks (through CodeOcean). Consider the following task in the "old" form, updated with the "new" version. (Hint: The bug is also visible through the ZIP upload)

task_845_new.zip
task_845_old.zip

The new task should contain a file for the tests, but after the update it doesn't. The test is present but without any files. Only a second update with the same "new" version "restores" (actually: recreates) the test file.

Reverting back to the old version yields a similar problem, any might optionally display an error.

[kkoehn]

We should also double check that files get deleted, when they are not present anymore

[kkoehn]

@MrSerth I can't seem to get a handle on the problem.
The assignment of the files is correct, but during the save some of them end up with fileable_id nil and get destroyed because of dependent: destroy the fileable_type seems to be set correctly.

Do you have an idea?

[MrSerth]

@MrSerth I can't seem to get a handle on the problem. The assignment of the files is correct, but during the save some of them end up with fileable_id nil and get destroyed because of dependent: destroy the fileable_type seems to be set correctly.

Do you have an idea?

Barely 😵‍💫. I played around with Rails for many hours and was trying many different things, until finding a suitable (not necessarily nice) solution. Your previous tests were super helpful, thanks again for this true test-driven approach!

I am not really satisfied with the approach, and I find overwriting files not ideal. However, I was really unable to find another, better solution. Fixing the "save" action was relatively easy and straight forward, but broke the modified (not saved) version. Fixing both was super hard, and the only solution that worked and did not overwrite much of the internals is the one I present. From my point of view, I find the suggestion okay-ish and would proceed with that base -- all changes are based on the public API and thus should be stable.

See my PR #1657 for details and the in-depth suggestion.

[kkoehn]

@MrSerth I merged your suggestions, thanks again for the help. I think this should be finished now.

[MrSerth]

I think so too, and would consider this PR to be ready. Updating a task with moved files (as mentioned earlier in #1612 (review)) is now working correctly 🥳