Skip to content

Commit

Permalink
Merge pull request #1776 from niklasmohrin/whodis
Browse files Browse the repository at this point in the history
Change vocabulary around text answer review
  • Loading branch information
niklasmohrin authored Aug 2, 2022
2 parents ba680aa + b6c7672 commit 70e1746
Show file tree
Hide file tree
Showing 17 changed files with 2,096 additions and 2,009 deletions.
3,798 changes: 1,899 additions & 1,899 deletions evap/development/fixtures/test_data.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
("evaluation", "0128_django_40_noop"),
]

operations = [
migrations.RenameField(
model_name="textanswer",
old_name="state",
new_name="review_decision",
),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
from django.db import migrations, models


def forward(apps, _schema_editor):
TextAnswer = apps.get_model("evaluation", "TextAnswer")
TextAnswer.objects.filter(review_decision="HI").update(review_decision="DE")
TextAnswer.objects.filter(review_decision="NR").update(review_decision="UN")


def backward(apps, _schema_editor):
TextAnswer = apps.get_model("evaluation", "TextAnswer")
TextAnswer.objects.filter(review_decision="DE").update(review_decision="HI")
TextAnswer.objects.filter(review_decision="UN").update(review_decision="NR")


class Migration(migrations.Migration):

dependencies = [
("evaluation", "0129_rename_state_textanswer_review_decision"),
]

operations = [
migrations.AlterField(
model_name="textanswer",
name="review_decision",
field=models.CharField(
choices=[("PU", "public"), ("PR", "private"), ("DE", "deleted"), ("UN", "undecided")],
default="UN",
max_length=2,
verbose_name="review decision for the answer",
),
),
migrations.RunPython(forward, reverse_code=backward),
]
60 changes: 30 additions & 30 deletions evap/evaluation/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ def publish(self):
if not self.can_publish_text_results:
self.textanswer_set.delete()
else:
self.textanswer_set.filter(state=TextAnswer.State.HIDDEN).delete()
self.textanswer_set.filter(review_decision=TextAnswer.ReviewDecision.DELETED).delete()
self.textanswer_set.update(original_answer=None)

@transition(field=state, source=State.PUBLISHED, target=State.REVIEWED)
Expand Down Expand Up @@ -864,11 +864,11 @@ def num_textanswers(self):

@property
def unreviewed_textanswer_set(self):
return self.textanswer_set.filter(state=TextAnswer.State.NOT_REVIEWED)
return self.textanswer_set.filter(review_decision=TextAnswer.ReviewDecision.UNDECIDED)

@property
def reviewed_textanswer_set(self):
return self.textanswer_set.exclude(state=TextAnswer.State.NOT_REVIEWED)
return self.textanswer_set.exclude(review_decision=TextAnswer.ReviewDecision.UNDECIDED)

@cached_property
def num_reviewed_textanswers(self):
Expand Down Expand Up @@ -1410,15 +1410,22 @@ class TextAnswer(Answer):
# If the text answer was changed during review, original_answer holds the original text. Otherwise, it's null.
original_answer = models.TextField(verbose_name=_("original answer"), blank=True, null=True)

class State(models.TextChoices):
HIDDEN = "HI", _("hidden")
PUBLISHED = "PU", _("published")
# Reviewers decided that this answer should only be displayed to the contributor the question was about
PRIVATE = "PR", _("private")
NOT_REVIEWED = "NR", _("not reviewed")
class ReviewDecision(models.TextChoices):
"""
When publishing evaluation results, this answer should be ...
"""

PUBLIC = "PU", _("public")
PRIVATE = "PR", _("private") # This answer should only be displayed to the contributor the question was about
DELETED = "DE", _("deleted")

state = models.CharField(
max_length=2, choices=State.choices, verbose_name=_("state of answer"), default=State.NOT_REVIEWED
UNDECIDED = "UN", _("undecided")

review_decision = models.CharField(
max_length=2,
choices=ReviewDecision.choices,
verbose_name=_("review decision for the answer"),
default=ReviewDecision.UNDECIDED,
)

class Meta:
Expand All @@ -1429,37 +1436,30 @@ class Meta:
verbose_name_plural = _("text answers")

@property
def is_hidden(self):
return self.state == self.State.HIDDEN
def will_be_deleted(self):
return self.review_decision == self.ReviewDecision.DELETED

@property
def is_private(self):
return self.state == self.State.PRIVATE
def will_be_private(self):
return self.review_decision == self.ReviewDecision.PRIVATE

@property
def is_published(self):
return self.state == self.State.PUBLISHED
def will_be_public(self):
return self.review_decision == self.ReviewDecision.PUBLIC

# Once evaluation results are published, the review decision is executed
# and thus, an answer _is_ private or _is_ public from that point on.
is_private = will_be_private
is_public = will_be_public

@property
def is_reviewed(self):
return self.state != self.State.NOT_REVIEWED
return self.review_decision != self.ReviewDecision.UNDECIDED

def save(self, *args, **kwargs):
super().save(*args, **kwargs)
assert self.answer != self.original_answer

def publish(self):
self.state = self.State.PUBLISHED

def hide(self):
self.state = self.State.HIDDEN

def make_private(self):
self.state = self.State.PRIVATE

def unreview(self):
self.state = self.State.NOT_REVIEWED


class FaqSection(models.Model):
"""Section in the frequently asked questions"""
Expand Down
2 changes: 1 addition & 1 deletion evap/evaluation/tests/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ def test_send_reminder(self):
baker.make(
TextAnswer,
contribution=evaluation.general_contribution,
state=TextAnswer.State.NOT_REVIEWED,
review_decision=TextAnswer.ReviewDecision.UNDECIDED,
)

management.call_command("send_reminders")
Expand Down
18 changes: 12 additions & 6 deletions evap/evaluation/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ def test_textanswers_do_not_get_deleted_if_they_can_be_published(self):
evaluation.publish()
self.assertEqual(evaluation.textanswer_set.count(), 1)

def test_hidden_textanswers_get_deleted_on_publish(self):
def test_textanswers_to_delete_get_deleted_on_publish(self):
student = baker.make(UserProfile)
student2 = baker.make(UserProfile)
evaluation = baker.make(
Expand All @@ -360,16 +360,22 @@ def test_hidden_textanswers_get_deleted_on_publish(self):
TextAnswer,
question=question,
contribution=evaluation.general_contribution,
answer=iter(["hidden", "published", "private"]),
state=iter([TextAnswer.State.HIDDEN, TextAnswer.State.PUBLISHED, TextAnswer.State.PRIVATE]),
answer=iter(["deleted", "public", "private"]),
review_decision=iter(
[
TextAnswer.ReviewDecision.DELETED,
TextAnswer.ReviewDecision.PUBLIC,
TextAnswer.ReviewDecision.PRIVATE,
]
),
_quantity=3,
_bulk_create=True,
)

self.assertEqual(evaluation.textanswer_set.count(), 3)
evaluation.publish()
self.assertEqual(evaluation.textanswer_set.count(), 2)
self.assertFalse(TextAnswer.objects.filter(answer="hidden").exists())
self.assertFalse(TextAnswer.objects.filter(answer="deleted").exists())

def test_original_textanswers_get_deleted_on_publish(self):
student = baker.make(UserProfile)
Expand All @@ -390,7 +396,7 @@ def test_original_textanswers_get_deleted_on_publish(self):
contribution=evaluation.general_contribution,
answer="published answer",
original_answer="original answer",
state=TextAnswer.State.PUBLISHED,
review_decision=TextAnswer.ReviewDecision.PUBLIC,
)

self.assertEqual(evaluation.textanswer_set.count(), 1)
Expand Down Expand Up @@ -519,7 +525,7 @@ def test_textanswer_review_state(self):
evaluation.TextAnswerReviewState.REVIEW_URGENT, # grades were uploaded
)

textanswer.state = TextAnswer.State.PUBLISHED
textanswer.review_decision = TextAnswer.ReviewDecision.PUBLIC
textanswer.save()
del evaluation.num_reviewed_textanswers # reset cached_property cache

Expand Down
30 changes: 15 additions & 15 deletions evap/results/fixtures/minimal_test_data_results.json
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@
"question": 3,
"original_answer": null,
"contribution": 1,
"state": "PU"
"review_decision": "PU"
}
},
{
Expand All @@ -139,7 +139,7 @@
"question": 3,
"original_answer": null,
"contribution": 1,
"state": "HI"
"review_decision": "HI"
}
},
{
Expand All @@ -150,7 +150,7 @@
"question": 3,
"original_answer": ".general_orig_published_changed.",
"contribution": 1,
"state": "PU"
"review_decision": "PU"
}
},
{
Expand All @@ -161,7 +161,7 @@
"question": 1,
"original_answer": null,
"contribution": 3,
"state": "PU"
"review_decision": "PU"
}
},
{
Expand All @@ -172,7 +172,7 @@
"question": 1,
"original_answer": null,
"contribution": 3,
"state": "PR"
"review_decision": "PR"
}
},
{
Expand All @@ -183,7 +183,7 @@
"question": 1,
"original_answer": null,
"contribution": 6,
"state": "PU"
"review_decision": "PU"
}
},
{
Expand All @@ -194,7 +194,7 @@
"question": 1,
"original_answer": null,
"contribution": 6,
"state": "HI"
"review_decision": "HI"
}
},
{
Expand All @@ -205,7 +205,7 @@
"question": 1,
"original_answer": ".responsible_contributor_orig_published_changed.",
"contribution": 6,
"state": "PU"
"review_decision": "PU"
}
},
{
Expand All @@ -216,7 +216,7 @@
"question": 1,
"original_answer": null,
"contribution": 6,
"state": "PR"
"review_decision": "PR"
}
},
{
Expand All @@ -227,7 +227,7 @@
"question": 1,
"original_answer": null,
"contribution": 6,
"state": "NR"
"review_decision": "NR"
}
},
{
Expand All @@ -238,7 +238,7 @@
"question": 6,
"original_answer": null,
"contribution": 1,
"state": "PU"
"review_decision": "PU"
}
},
{
Expand All @@ -249,7 +249,7 @@
"question": 2,
"original_answer": null,
"contribution": 6,
"state": "PU"
"review_decision": "PU"
}
},
{
Expand All @@ -260,7 +260,7 @@
"question": 2,
"original_answer": null,
"contribution": 6,
"state": "HI"
"review_decision": "HI"
}
},
{
Expand All @@ -271,7 +271,7 @@
"question": 4,
"original_answer": null,
"contribution": 1,
"state": "PU"
"review_decision": "PU"
}
},
{
Expand All @@ -282,7 +282,7 @@
"question": 4,
"original_answer": null,
"contribution": 1,
"state": "HI"
"review_decision": "HI"
}
},
{
Expand Down
2 changes: 1 addition & 1 deletion evap/results/tests/test_exporters.py
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ def test_text_answer_export(self):
question=iter(questions[idx] for idx in [0, 1, 2, 2, 0]),
contribution__evaluation=evaluation,
contribution__questionnaires=iter(questions[idx].questionnaire for idx in [0, 1, 2, 2, 0]),
state=TextAnswer.State.PUBLISHED,
review_decision=TextAnswer.ReviewDecision.PUBLIC,
_quantity=5,
)

Expand Down
Loading

0 comments on commit 70e1746

Please sign in to comment.