Main language for evaluations #2367
Conversation
| with self.assertRaises(ValueError): | ||
| form["main_language"] = "some_other_wrong_value" | ||
| form.submit(name="operation", value="save") |
There was a problem hiding this comment.
Like the "Invalid value doesn't work"-block in 303, I think this will already fail on setting the form field as django webtest throws when we try to set a value that is not in the choices list. I think testing that server-side validation works is a good idea here, not sure if it can be done with WebTest form framework though. I think in the past we always manually crafted POST requests to achieve this.
| def _migrate(apps, schema_editor): | ||
| Evaluation = apps.get_model("evaluation", "Evaluation") | ||
| for evaluation in Evaluation.objects.filter(state__gte=40): | ||
| evaluation.main_language = "de" |
There was a problem hiding this comment.
Why? I would have expected "undecided" (which is the default value -> no explicit migration code needed?)
(If we keep this, can we use some named variable instead of the magic 40?)
| def editor_approve(self): | ||
| pass | ||
|
|
||
| @transition( | ||
| field=state, | ||
| source=[State.NEW, State.PREPARED, State.EDITOR_APPROVED], | ||
| target=State.APPROVED, | ||
| conditions=[lambda self: self.general_contribution_has_questionnaires], | ||
| conditions=[lambda self: self.general_contribution_has_questionnaires and self.has_valid_main_language], |
There was a problem hiding this comment.
The issue says that staff users should be able to set "undecided". I think this change here would block that workflow?
| @@ -1920,6 +1920,38 @@ def test_evaluation_create(self): | |||
| form.submit() | |||
| self.assertEqual(Evaluation.objects.get().name_de, "lfo9e7bmxp1xi") | |||
|
|
|||
| def _set_valid_form(self, form): | |||
There was a problem hiding this comment.
factoring out the helper is nice -- but we should also use it in test_evaluation_create then :D
| self.assertNotEqual(Evaluation.objects.first().state, self.evaluation.State.APPROVED) | ||
|
|
||
| form["main_language"] = "en" | ||
| form.submit("operation", value="approve") |
There was a problem hiding this comment.
We probably have to add an assertion that the response now doesn't contain the above error anymore (our views return 200 even if there is a business logic error / form validation failed)
| raise SuspiciousOperation("Invalid POST operation") | ||
|
|
||
| evaluation_form = EvaluationForm( | ||
| request.POST or None, instance=evaluation, semester=evaluation.course.semester, operation=operation |
There was a problem hiding this comment.
random consideration: A narrower, more precise interface would be passing require_main_language: bool instead of passing an operation string that implicitly allows to infer that bool
| data-bs-placement="bottom" | ||
| title="{{ language_name }}" | ||
| role="button" | ||
| href="{% url 'student:vote' evaluation.pk %}?language={{ language_code }}" |
There was a problem hiding this comment.
This should probably be a POST-form, not a GET-anchor. (Currently, this wouldn't need the CSRF token)
No description provided.