Assign contributor questionnaires per course type #2373
Conversation
added the option to specify course types, where speific contributor questionaires should be added to all such courses
aparrently when its used it is not always checked if its not None. Thats why linter dies here.
TestSemesterQuestionnaireAssignment was missing "general" and "contributor"
| ) | ||
|
|
||
| for course_type in course_types: | ||
| self.fields["general-" + course_type.name] = forms.ModelMultipleChoiceField( | ||
| label=_(course_type.name), |
There was a problem hiding this comment.
I don't think translating with _ works here. course_type.name should already be localized correctly. Repeated below.
| queryset=general_questionnaires, | ||
| ) | ||
| self.fields["contributor-" + course_type.name] = forms.ModelMultipleChoiceField( | ||
| label=_(course_type.name), required=False, queryset=contributor_questionnaires |
There was a problem hiding this comment.
The trailing comma in an argument list above forces our code formatter into a "one argument per line" mode, while it's absent here, so we get all arguments in one line. I don't care which mode you use, but please be consistent.
| # self.fields["all-contributors"] = forms.ModelMultipleChoiceField( | ||
| # label=_("All contributors"), required=False, queryset=contributor_questionnaires | ||
| # ) |
| <fieldset> | ||
| {% include 'bootstrap_form.html' with form=form %} | ||
| </fieldset> | ||
| <p>{% translate 'Select the questionnaires which shall be assigned to each of these course types. This will only change evaluations in preparation. If you do select nothing, the currently applied questionnaires for the corresponding course type or contributor will not be changed.' %}</p> |
There was a problem hiding this comment.
- I don't know how proficient users reading this are, but technically, we are not assigning anything to course types. To me, the text here implise that we would somehow store this in the course type, so it would be applied even to evaluations created after this. Can/should we change it to:
which shall be assigned to (contributions of) courses in each of these course types
- "If you do select nothing" should probably be "If you select nothing"?
- The "or contributor" in the final sentence probably also needs to be changed.
@janno42 probably easiest if you give us the new text for this paragraph.
| @@ -908,15 +908,16 @@ class TestSemesterAssignView(WebTestStaffMode): | |||
| @classmethod | |||
| def setUpTestData(cls): | |||
| cls.manager = make_manager() | |||
| cls.semester = baker.make(Semester) | |||
| cls.semester: Semester = baker.make(Semester) | |||
There was a problem hiding this comment.
Why? Type inference should work here without problems. We don't want to duplicate types.
| ) | ||
|
|
||
| for course_type in course_types: | ||
| self.fields["general-" + course_type.name] = forms.ModelMultipleChoiceField( |
There was a problem hiding this comment.
course_type.name is the translated human-readable version of the name and can easily contain spaces. Using - as a delimiter here while appending that name is weird. Using a translated name is weird. Maybe just use the id? (Repeated below)
| form = QuestionnairesAssignForm(request.POST or None, course_types=course_types) | ||
| form = QuestionnairesAssignForm( | ||
| request.POST or None, course_types=course_types | ||
| ) # das erste is inital data, damit bei fehler die alten sachen da sind |
fix #2299