Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22,986 changes: 11,804 additions & 11,182 deletions evap/development/fixtures/test_data.json

Large diffs are not rendered by default.

14 changes: 8 additions & 6 deletions evap/evaluation/management/commands/anonymize.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,25 +232,27 @@ def anonymize_answers(self, lorem_ipsum):

self.stdout.write("Shuffling rating answer counter counts...")

contributions = Contribution.objects.all().prefetch_related("ratinganswercounter_set__question")
contributions = Contribution.objects.all().prefetch_related("ratinganswercounter_set__assignment__question")
try:
self.stdout.ending = ""
progress_bar = ProgressBar(self.stdout, contributions.count())
for contribution_counter, contribution in enumerate(contributions):
progress_bar.update(contribution_counter + 1)

counters_per_question = unordered_groupby(
(counter.question, counter) for counter in contribution.ratinganswercounter_set.all()
counters_per_assignment = unordered_groupby(
(counter.assignment, counter) for counter in contribution.ratinganswercounter_set.all()
)

for question, counters in counters_per_question.items():
for assignment, counters in counters_per_assignment.items():
original_sum = sum(counter.count for counter in counters)

missing_values = set(CHOICES[question.type].values).difference({c.answer for c in counters})
missing_values = set(CHOICES[assignment.question.type].values).difference(
{c.answer for c in counters}
)
missing_values.discard(NO_ANSWER) # don't add NO_ANSWER counter if it didn't exist before
for value in missing_values:
counters.append(
RatingAnswerCounter(question=question, contribution=contribution, answer=value, count=0)
RatingAnswerCounter(assignment=assignment, contribution=contribution, answer=value, count=0)
)

generated_counts = [random.random() for c in counters] # nosec
Expand Down
166 changes: 166 additions & 0 deletions evap/evaluation/migrations/0161_temporary_newquestion_relation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
# Generated by Django 5.2 on 2025-08-12 22:21

import django.db.models.deletion
from django.db import migrations, models


def questions_to_question_assignments(apps, _schema_editor):
Question = apps.get_model("evaluation", "Question")
NewQuestion = apps.get_model("evaluation", "NewQuestion")
QuestionAssignment = apps.get_model("evaluation", "QuestionAssignment")
RatingAnswerCounter = apps.get_model("evaluation", "RatingAnswerCounter")
TextAnswer = apps.get_model("evaluation", "TextAnswer")
assignments = {}
new_questions = {}
for question in Question.objects.all():
new_question = new_questions.setdefault(
(question.text_de, question.text_en, question.allows_additional_textanswers, question.type),
NewQuestion(
# pk=question.pk, # was only active when the test data jsons were regenerated to minimize the diff
Comment thread
richardebeling marked this conversation as resolved.
text_de=question.text_de,
Comment thread
Kakadus marked this conversation as resolved.
text_en=question.text_en,
allows_additional_textanswers=question.allows_additional_textanswers,
type=question.type,
),
)
assignments[question.pk] = QuestionAssignment(
question=new_question, questionnaire_id=question.questionnaire_id, order=question.order
)
Comment on lines +16 to +28
Copy link
Copy Markdown
Collaborator Author

@Kakadus Kakadus Feb 9, 2026

Choose a reason for hiding this comment

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

from ai review:

The migration has a potential issue with question deduplication. It uses a tuple of (text_de, text_en, allows_additional_textanswers, type) as a key to deduplicate questions. However, this approach may incorrectly merge questions that happen to have the same text but are actually different questions in different contexts. For instance, two different questionnaires might have a question "How satisfied are you?" that should remain separate. This could lead to unintended data merging during migration. Consider whether this deduplication is intended or if each question should be preserved separately.

Question deduplication is not bad per se, and the "copy on write" logic mimics current behavior.
What I haven't thought of specifically is questionnaires that contain the same question multiple times. We prohibited this specifically with unique_together = [("question", "questionnaire")], so the check fails then. This may happen when migrating production.
@janno42, can you check this by running

from django.db.models import CharField, Count, F, Value
from django.db.models.functions import Concat

questionnaires_with_duplicates = Questionnaire.objects.annotate(
    total_questions=Count('questions'),
    unique_combinations=Count(
        Concat(
            'questions__text_de',
            Value('|==||==|'),
            'questions__text_en',
            Value('|==||==|'),
            'questions__allows_additional_textanswers',
            Value('|==||==|'),
            'questions__type',
            output_field=CharField()
        ),
        distinct=True
    )
).filter(total_questions__gt=F('unique_combinations'))
print(len(questionnaires_with_duplicates), "questionnaires ask the same question multiple times.")

in production?

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.

I cannot imagine a situation where we intentionally included the same question twice in a questionnaire, but it probably doesn't hurt to double check

NewQuestion.objects.bulk_create(new_questions.values())
QuestionAssignment.objects.bulk_create(assignments.values())

def set_question_assignment(answer):
answer.assignment = assignments[answer.question_id]
return answer

for model in (RatingAnswerCounter, TextAnswer):
model.objects.bulk_update(
map(set_question_assignment, model.objects.all()),
fields=["assignment"],
)


class Migration(migrations.Migration):
dependencies = [
("evaluation", "0160_evaluation_staff_notes"),
]

operations = [
migrations.CreateModel(
name="NewQuestion",
fields=[
("id", models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")),
("text_de", models.CharField(max_length=1024, verbose_name="question text (german)")),
("text_en", models.CharField(max_length=1024, verbose_name="question text (english)")),
(
"allows_additional_textanswers",
models.BooleanField(default=True, verbose_name="allow additional text answers"),
),
(
"type",
models.PositiveSmallIntegerField(
choices=[
("Text", [(0, "Text question")]),
(
"Unipolar Likert",
[(1, "Positive agreement question"), (12, "Negative agreement question")],
),
("Grade", [(2, "Grade question")]),
(
"Bipolar Likert",
[
(6, "Easy-difficult question"),
(7, "Few-many question"),
(8, "Little-much question"),
(9, "Small-large question"),
(10, "Slow-fast question"),
(11, "Short-long question"),
],
),
("Yes-no", [(3, "Positive yes-no question"), (4, "Negative yes-no question")]),
("Layout", [(5, "Heading")]),
],
verbose_name="question type",
),
),
],
options={
"verbose_name": "question",
"verbose_name_plural": "questions",
},
),
migrations.CreateModel(
name="QuestionAssignment",
fields=[
("id", models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")),
("order", models.IntegerField(default=-1, verbose_name="question order")),
(
"question",
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
related_name="assignments",
to="evaluation.newquestion",
),
),
(
"questionnaire",
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
related_name="question_assignments",
to="evaluation.questionnaire",
),
),
],
options={"ordering": ["order"]},
),
migrations.AlterUniqueTogether(
name="questionassignment",
unique_together={("question", "questionnaire")},
),
migrations.AddField(
model_name="newquestion",
name="questionnaires",
field=models.ManyToManyField(
related_name="questions", through="evaluation.QuestionAssignment", to="evaluation.questionnaire"
),
),
migrations.AlterField(
model_name="ratinganswercounter",
name="question",
field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to="evaluation.question", null=True),
),
migrations.AlterField(
model_name="textanswer",
name="question",
field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to="evaluation.question", null=True),
),
migrations.AddField(
model_name="ratinganswercounter",
name="assignment",
field=models.ForeignKey(
on_delete=django.db.models.deletion.PROTECT, to="evaluation.questionassignment", null=True
),
),
migrations.AddField(
model_name="textanswer",
name="assignment",
field=models.ForeignKey(
on_delete=django.db.models.deletion.PROTECT, to="evaluation.questionassignment", null=True
),
),
migrations.AddConstraint(
model_name="newquestion",
constraint=models.CheckConstraint(
condition=models.Q(
models.Q(("type", 0), ("type", 5), _connector="OR", _negated=True),
models.Q(("allows_additional_textanswers", True), _negated=True),
_connector="OR",
),
name="check_evaluation_textanswer_or_heading_question_has_no_additional_textanswers_new",
),
),
migrations.RunPython(
code=questions_to_question_assignments,
reverse_code=migrations.RunPython.noop,
),
]
107 changes: 107 additions & 0 deletions evap/evaluation/migrations/0162_unified_questions_from_tmp_relation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# Generated by Django 5.2 on 2025-10-27 17:17

import django.db.models.deletion
from django.db import migrations, models


def question_assignments_to_questions(apps, _schema_editor):
Question = apps.get_model("evaluation", "Question")
QuestionAssignment = apps.get_model("evaluation", "QuestionAssignment")
RatingAnswerCounter = apps.get_model("evaluation", "RatingAnswerCounter")
TextAnswer = apps.get_model("evaluation", "TextAnswer")
questions = {}
for question_assignment in QuestionAssignment.objects.all().prefetch_related("question"):
new_question = question_assignment.question
questions[question_assignment.pk] = Question(
text_de=new_question.text_de,
text_en=new_question.text_en,
allows_additional_textanswers=new_question.allows_additional_textanswers,
type=new_question.type,
questionnaire=question_assignment.questionnaire,
order=question_assignment.order,
)
Question.objects.bulk_create(questions.values())

def set_question(answer):
answer.question_id = questions[answer.assignment_id].pk
return answer

for model in (RatingAnswerCounter, TextAnswer):
model.objects.bulk_update(
map(set_question, model.objects.all()),
fields=["question"],
)


class Migration(migrations.Migration):
dependencies = [
("evaluation", "0161_temporary_newquestion_relation"),
]

operations = [
migrations.RunPython(
code=migrations.RunPython.noop,
reverse_code=question_assignments_to_questions,
),
migrations.AlterUniqueTogether(
name="ratinganswercounter",
unique_together=set(),
),
migrations.RemoveField(
model_name="ratinganswercounter",
name="question",
),
migrations.RemoveField(
model_name="textanswer",
name="question",
),
migrations.DeleteModel(
name="Question",
),
migrations.RenameModel(old_name="NewQuestion", new_name="Question"),
migrations.RemoveConstraint(
model_name="question",
name="check_evaluation_textanswer_or_heading_question_has_no_additional_textanswers_new",
),
migrations.AlterField(
model_name="questionassignment",
name="questionnaire",
field=models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
related_name="question_assignments",
to="evaluation.questionnaire",
),
),
migrations.AlterField(
model_name="ratinganswercounter",
name="assignment",
field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to="evaluation.questionassignment"),
),
migrations.AlterField(
model_name="textanswer",
name="assignment",
field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to="evaluation.questionassignment"),
),
migrations.AddConstraint(
model_name="question",
constraint=models.CheckConstraint(
condition=models.Q(
models.Q(("type", 0), ("type", 5), _connector="OR", _negated=True),
models.Q(("allows_additional_textanswers", True), _negated=True),
_connector="OR",
),
name="check_evaluation_textanswer_or_heading_question_has_no_additional_textanswers",
),
),
migrations.AddConstraint(
model_name="question",
constraint=models.CheckConstraint(
condition=models.Q(("type__in", [0, 1, 12, 2, 6, 7, 8, 9, 10, 11, 3, 4, 5])),
name="Question_type_choices",
),
),
migrations.AlterUniqueTogether(
name="ratinganswercounter",
unique_together={("assignment", "contribution", "answer")},
),
]
40 changes: 33 additions & 7 deletions evap/evaluation/models.py
Comment thread
Kakadus marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -1208,8 +1208,8 @@ def __str__(self):

def remove_answers_to_questionnaires(self, questionnaires):
assert set(Answer.__subclasses__()) == {TextAnswer, RatingAnswerCounter}
TextAnswer.objects.filter(contribution=self, question__questionnaire__in=questionnaires).delete()
RatingAnswerCounter.objects.filter(contribution=self, question__questionnaire__in=questionnaires).delete()
TextAnswer.objects.filter(contribution=self, assignment__questionnaire__in=questionnaires).delete()
RatingAnswerCounter.objects.filter(contribution=self, assignment__questionnaire__in=questionnaires).delete()


class QuestionType:
Expand Down Expand Up @@ -1262,8 +1262,9 @@ class Question(models.Model):
(_("Layout"), ((QuestionType.HEADING, _("Heading")),)),
)

order = models.IntegerField(verbose_name=_("question order"), default=-1)
questionnaire = models.ForeignKey(Questionnaire, models.CASCADE, related_name="questions")
questionnaires = models.ManyToManyField(
Questionnaire, through="evaluation.QuestionAssignment", related_name="questions"
)
text_de = models.CharField(max_length=1024, verbose_name=_("question text (german)"))
text_en = models.CharField(max_length=1024, verbose_name=_("question text (english)"))
text = translate(en="text_en", de="text_de")
Expand All @@ -1273,7 +1274,6 @@ class Question(models.Model):

@inject_choices_constraint(locals())
class Meta:
ordering = ["order"]
verbose_name = _("question")
verbose_name_plural = _("questions")
constraints = [
Expand Down Expand Up @@ -1364,6 +1364,28 @@ def can_have_textanswers(self):
return self.is_text_question or self.is_rating_question and self.allows_additional_textanswers


class QuestionAssignment(models.Model):
question = models.ForeignKey(Question, on_delete=models.CASCADE, related_name="assignments")
questionnaire = models.ForeignKey(Questionnaire, on_delete=models.CASCADE, related_name="question_assignments")
order = models.IntegerField(verbose_name=_("question order"), default=-1)

class Meta:
ordering = ["order"]
unique_together = [("question", "questionnaire")]

def delete(self, using=None, keep_parents=False) -> tuple[int, dict[str, int]]:
Comment thread
richardebeling marked this conversation as resolved.
assert not self.question.answer_class.objects.filter(assignment=self).exists(), (
"cannot delete question with answers"
)
count = 0
meta: dict[str, int] = {}

if not self.question.questionnaires.exclude(pk=self.questionnaire.pk).exists():
Comment thread
Kakadus marked this conversation as resolved.
count, meta = self.question.delete(using=using, keep_parents=False) # garbage-collect unused questions
Comment thread
richardebeling marked this conversation as resolved.
self_count, self_meta = super().delete(using=using, keep_parents=keep_parents)
return count + self_count, meta | self_meta


@dataclass
class Choices:
css_class: str
Expand Down Expand Up @@ -1564,14 +1586,18 @@ class Answer(models.Model):
# we use UUIDs to hide insertion order. See https://github.com/e-valuation/EvaP/wiki/Data-Economy
id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)

question = models.ForeignKey(Question, models.PROTECT)
assignment = models.ForeignKey(QuestionAssignment, models.PROTECT)
Comment thread
niklasmohrin marked this conversation as resolved.
contribution = models.ForeignKey(Contribution, models.PROTECT, related_name="%(class)s_set")

class Meta:
abstract = True
verbose_name = _("answer")
verbose_name_plural = _("answers")

@property
def question(self) -> Question:
return self.assignment.question


class RatingAnswerCounter(Answer):
"""
Expand All @@ -1586,7 +1612,7 @@ class RatingAnswerCounter(Answer):
count = models.IntegerField(verbose_name=_("count"), default=0)

class Meta:
unique_together = [["question", "contribution", "answer"]]
unique_together = [["assignment", "contribution", "answer"]]
verbose_name = _("rating answer")
verbose_name_plural = _("rating answers")

Expand Down
Loading