search

openHPI / codeharbor

Exchange of Programming Exercises acrossdiverse Code Assessment Systems through CodeHarbor
BSD 3-Clause "New" or "Revised" License
6 stars 2 forks source link

Refactor deletion of dangling labels #1606

Closed MrSerth closed 2 weeks ago

MrSerth commented 2 months ago

The previous mechanism required newly-assigned TaskLabels to be saved before aiming to destroy dangling labels. However, there is one edge-case previously used in the tests which caused erroneous behavior:

Imagine, there is a dangling label (with no matching TaskLabel). Then, a task is assigned that dangling label through the label_names= mechanism. This will cause the Label.find_or_initialize_by(name:) method to return the dangling label and assign that to the task. However, when this assignment is not saved it (and no TaskLabel record is created), the following call to destroy dangling labels also removed the one just assigned to a task.

This caused errors in tests, when switching from labels to label_names in the TaskController specs to remove the use of unpermitted parameters and transactional fixtures.

The new mechanism will still remove dangling labels, but only those associated with a TaskLabel just removed. Through the transactional setting of destroy hooks, this still ensures there aren't any leftovers.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.78%. Comparing base (a271eed) to head (815c20a). Report is 44 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1606 +/- ## ======================================= Coverage 94.77% 94.78% ======================================= Files 130 130 Lines 3330 3334 +4 ======================================= + Hits 3156 3160 +4 Misses 174 174 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

MrSerth commented 2 months ago

Looks good, I am just wondering if we need Label#to_s? I could not find any usage and it is also not covered by the tests.

I knew you would point that out. It is not used, that's right. I found it helpful to have while debugging, since it replaced the magic object ID with the actual label name when printing. If you like, I can add a simple test for that.

Mathis-Z commented 2 months ago

The test is a bit silly :wink: but at least Codecov is happy now :+1: