Skip to content

Run QuestionAssignment's delete garbage collection in signal#2675

Merged
niklasmohrin merged 2 commits intoe-valuation:mainfrom
niklasmohrin:assignment-post-delete
Mar 16, 2026
Merged

Run QuestionAssignment's delete garbage collection in signal#2675
niklasmohrin merged 2 commits intoe-valuation:mainfrom
niklasmohrin:assignment-post-delete

Conversation

@niklasmohrin
Copy link
Copy Markdown
Member

@niklasmohrin niklasmohrin commented Mar 16, 2026

The model's .delete method is not called when an object is deleted from a queryset or in a cascading delete. Instead, the post_delete signal can be used. I also made sure to wrap the handler in a transaction to rule out this kind of error.

See https://docs.djangoproject.com/en/6.0/topics/db/models/#overriding-model-methods and #2513 (comment)

  • Run QuestionAssignment's delete garbage collection in signal
  • Merge TestQuestionnaire and QuestionnaireTests, and downgrade to TestCase

The model's `.delete` method is not called when an object is deleted
from a queryset or in a cascading delete. Instead, the `post_delete`
signal can be used.

See https://docs.djangoproject.com/en/6.0/topics/db/models/#overriding-model-methods
@niklasmohrin niklasmohrin requested review from Kakadus and richardebeling and removed request for Kakadus March 16, 2026 19:48
Copy link
Copy Markdown
Collaborator

@Kakadus Kakadus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense and reduces complexity in the delete handler.

@richardebeling
Copy link
Copy Markdown
Member

We're also hoping that it may fix #2674 (which has happened again today)

Comment thread evap/evaluation/models.py
@transaction.atomic
def _question_assignment_post_delete(*, instance: QuestionAssignment, **_kwargs) -> None:
"""Garbage-collect unused questions once no questionnaires reference them anymore."""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record: Based on the current django source code, we expect this signal to fire within a transaction that deletes the "parent" object. So, a failure in the handler should also undo the deletion of the object that sent the signal (which is probably good for us)

@niklasmohrin niklasmohrin merged commit 031b579 into e-valuation:main Mar 16, 2026
16 checks passed
@niklasmohrin niklasmohrin deleted the assignment-post-delete branch March 16, 2026 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants