Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assign contributor questionnaires per course type #2373

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jooooosef
Copy link
Collaborator

fix #2299

evap/staff/forms.py Outdated Show resolved Hide resolved
evap/staff/forms.py Outdated Show resolved Hide resolved
evap/staff/forms.py Outdated Show resolved Hide resolved
evap/staff/tests/test_views.py Outdated Show resolved Hide resolved
evap/staff/forms.py Outdated Show resolved Hide resolved
evap/staff/views.py Outdated Show resolved Hide resolved
@jooooosef
Copy link
Collaborator Author

jooooosef commented Jan 20, 2025

Currently there are two tests doing similiar things to test the questionaire assignment.
That would be TestSemesterQuestionnaireAssignment and TestSemesterAssignView. Should I merge those tests into one or should I at least move those two closer to each other?
(one is in line 907 and one in line 3813) @richardebeling

@niklasmohrin
Copy link
Member

@jooooosef If merging the tests makes your life easier for your testing here, feel free to do it, otherwise consider opening a separate PR - or we can show you how to clean up the commits on your branch next week and make it into two commits here :)

@jooooosef
Copy link
Collaborator Author

@niklasmohrin I'll take the 2 commits option 👷

@jooooosef jooooosef changed the title Assign contributor questionnaires per course type 🚸 Assign contributor questionnaires per course type Jan 27, 2025
@jooooosef jooooosef force-pushed the iss2299-new branch 2 times, most recently from ad2fbb8 to d9a9d31 Compare January 27, 2025 18:36
@jooooosef jooooosef changed the title 🚸 Assign contributor questionnaires per course type Assign contributor questionnaires per course type Jan 27, 2025
added the option to specify course types, where specific contributor questionaires should be added to all such courses
one test that tests questionaire assignemnt was 3k lines below the other one
)

for course_type in course_types:
self.fields["general-" + str(course_type.id)] = forms.ModelMultipleChoiceField(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think f-strings are more concise here, below, in the test and the other view

Suggested change
self.fields["general-" + str(course_type.id)] = forms.ModelMultipleChoiceField(
self.fields[f"general-{course_type.id}"] = forms.ModelMultipleChoiceField(

Comment on lines +941 to +942
self.assertEqual(evaluation.general_contribution.questionnaires.count(), 1)
self.assertEqual(evaluation.general_contribution.questionnaires.get(), self.questionnaire_general)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
self.assertEqual(evaluation.general_contribution.questionnaires.count(), 1)
self.assertEqual(evaluation.general_contribution.questionnaires.get(), self.questionnaire_general)
self.assertQuerySetEqual(evaluation.general_contribution.questionnaires, [self.questionnaire_general])

maybe also below?

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.

Assign contributor questionnaires per course type
5 participants