diff --git a/evap/evaluation/migrations/0164_remove_questionnaire_questionnaire_visibility_choices_and_more.py b/evap/evaluation/migrations/0164_remove_questionnaire_questionnaire_visibility_choices_and_more.py new file mode 100644 index 0000000000..afdff0208f --- /dev/null +++ b/evap/evaluation/migrations/0164_remove_questionnaire_questionnaire_visibility_choices_and_more.py @@ -0,0 +1,31 @@ +# Generated by Django 5.2.11 on 2026-03-02 21:12 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("evaluation", "0163_migrate_cms_links"), + ] + + operations = [ + migrations.RemoveConstraint( + model_name="questionnaire", + name="Questionnaire_visibility_choices", + ), + migrations.AlterField( + model_name="questionnaire", + name="visibility", + field=models.IntegerField( + choices=[(0, "Don't show"), (1, "Managers only"), (2, "Managers and editors"), (3, "Archived")], + default=1, + verbose_name="visibility", + ), + ), + migrations.AddConstraint( + model_name="questionnaire", + constraint=models.CheckConstraint( + condition=models.Q(("visibility__in", [0, 1, 2, 3])), name="Questionnaire_visibility_choices" + ), + ), + ] diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index fc8f4b02fe..ed31ccf366 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -214,6 +214,7 @@ class Visibility(models.IntegerChoices): HIDDEN = 0, _("Don't show") MANAGERS = 1, _("Managers only") EDITORS = 2, _("Managers and editors") + ARCHIVED = 3, _("Archived") visibility = models.IntegerField( choices=Visibility.choices, verbose_name=_("visibility"), default=Visibility.MANAGERS diff --git a/evap/evaluation/tools.py b/evap/evaluation/tools.py index 8ff1cb777b..45ca866bf5 100644 --- a/evap/evaluation/tools.py +++ b/evap/evaluation/tools.py @@ -175,11 +175,19 @@ def get_parameter_from_url_or_session(request: HttpRequest, parameter: str, defa if result_str is None: # if no parameter is given take session value result = request.session.get(parameter, default) else: - result = {"true": True, "false": False}.get(result_str.lower()) # convert parameter to boolean + result = {"true": True, "false": False}.get(result_str.lower(), default) # convert parameter to boolean request.session[parameter] = result # store value for session return result +def get_string_from_url_or_session(request: HttpRequest, parameter: str, default: str | None = None) -> str | None: + result = request.GET.get(parameter, None) + if result is None: + result = request.session.get(parameter, default) + request.session[parameter] = result + return result + + def translate(**kwargs): # pylint is really buggy with this method. # pylint: disable=unused-variable, useless-suppression diff --git a/evap/staff/forms.py b/evap/staff/forms.py index 149e18772d..ee35175f83 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -444,7 +444,7 @@ def __init__(self, *args, requires_decided_main_language=False, **kwargs): super().__init__(*args, **kwargs) self.fields["course"].queryset = Course.objects.filter(semester=semester) - visible_questionnaires = ~Q(visibility=Questionnaire.Visibility.HIDDEN) + visible_questionnaires = Q(visibility__in=[Questionnaire.Visibility.EDITORS, Questionnaire.Visibility.MANAGERS]) if self.instance.pk is not None: visible_questionnaires |= Q(contributions__evaluation=self.instance) diff --git a/evap/staff/templates/staff_questionnaire_index.html b/evap/staff/templates/staff_questionnaire_index.html index b310a14668..a2cce297b9 100644 --- a/evap/staff/templates/staff_questionnaire_index.html +++ b/evap/staff/templates/staff_questionnaire_index.html @@ -10,13 +10,16 @@
-
{% translate 'Hidden questionnaires' %}
+
{% translate 'Show questionnaires' %}
- - {% translate 'Show' %} + + {% translate 'All' %} - - {% translate 'Hide' %} + + {% translate 'Visible' %} + + + {% translate 'Archived' %}
@@ -56,14 +59,47 @@ }); - {% translate 'Top general questionnaires' as headline %} - {% include 'staff_questionnaire_index_list.html' with questionnaires=general_questionnaires_top headline=headline extra_classes='mb-3' type='top' %} - {% translate 'Contributor questionnaires' as headline %} - {% include 'staff_questionnaire_index_list.html' with questionnaires=contributor_questionnaires headline=headline extra_classes='mb-3' type='contributor' %} - {% translate 'Bottom general questionnaires' as headline %} - {% include 'staff_questionnaire_index_list.html' with questionnaires=general_questionnaires_bottom headline=headline extra_classes='mb-3' type='bottom' %} - {% translate 'Dropout questionnaires' as headline %} - {% include 'staff_questionnaire_index_list.html' with questionnaires=dropout_questionnaires headline=headline extra_classes='' type='dropout' %} +
+
+ +
+
+
+ {% include 'staff_questionnaire_index_list.html' with questionnaires=general_questionnaires_top type='top' %} +
+
+ {% include 'staff_questionnaire_index_list.html' with questionnaires=contributor_questionnaires type='contributor' %} +
+
+ {% include 'staff_questionnaire_index_list.html' with questionnaires=general_questionnaires_bottom type='bottom' %} +
+
+ {% include 'staff_questionnaire_index_list.html' with questionnaires=dropout_questionnaires type='dropout' %} +
+
+
+ {% else %}

{% translate 'There are no questionnaires yet.' %} @@ -125,4 +161,23 @@ button.addEventListener("click", () => changeLocked(button)); } + + {% endblock %} diff --git a/evap/staff/templates/staff_questionnaire_index_list.html b/evap/staff/templates/staff_questionnaire_index_list.html index d05d956939..defc93079f 100644 --- a/evap/staff/templates/staff_questionnaire_index_list.html +++ b/evap/staff/templates/staff_questionnaire_index_list.html @@ -1,84 +1,71 @@ -{% if questionnaires %} -

-
- -
-
-
- - - - - - - - - - - - {% for questionnaire in questionnaires %} - - - - - - - - {% endfor %} - -
{% translate 'Questionnaire' %} - {% if type != 'contributor' %} - {% translate 'Locked' %} - {% endif %} - {% translate 'Visibility' %}{% translate 'Actions' %}
- {{ questionnaire.name }} -
- {% blocktranslate count questionnaire.questions.count as count %}{{ count }} question{% plural %}{{ count }} questions{% endblocktranslate %}, - - {% blocktranslate count count=questionnaire.contributions.count %}used {{ count }} time{% plural %}used {{ count }} times{% endblocktranslate %} - - 		- {% if type != 'contributor' %} -
- - -
- {% endif %} - 		-
- - - -
- 		- - - - - {% if questionnaire.can_be_deleted_by_manager %} - - {% translate 'Delete questionnaire' %} - {% translate 'Delete questionnaire' %} - - {% blocktranslate trimmed with questionnaire_name=questionnaire.name %} - Do you really want to delete the questionnaire {{questionnaire_name}}? - {% endblocktranslate %} - - - - - {% else %} - - {% endif %} -
-
-
+
+
+ + + + + + + + + + + + {% for questionnaire in questionnaires %} + + + + + + + + {% endfor %} + +
{% translate 'Questionnaire' %} + {% if type != 'contributor' %} + {% translate 'Locked' %} + {% endif %} + {% translate 'Visibility' %}{% translate 'Actions' %}
+ {{ questionnaire.name }} +
+ {% blocktranslate count questionnaire.contributions.count as count %}used {{ count }} time{% plural %}used {{ count }} times{% endblocktranslate %} + 		+ {% if type != 'contributor' %} +
+ + +
+ {% endif %} + 		+
+ + + + +
+ 		+ + + + + {% if questionnaire.can_be_deleted_by_manager %} + + {% translate 'Delete questionnaire' %} + {% translate 'Delete questionnaire' %} + + {% blocktranslate trimmed with questionnaire_name=questionnaire.name %} + Do you really want to delete the questionnaire {{questionnaire_name}}? + {% endblocktranslate %} + + + + {% else %} + + {% endif %} +
-{% endif %} +
diff --git a/evap/staff/tests/test_live.py b/evap/staff/tests/test_live.py index eb73906e54..f0d49e32ad 100644 --- a/evap/staff/tests/test_live.py +++ b/evap/staff/tests/test_live.py @@ -226,6 +226,27 @@ def predicate(driver): self.wait.until(make_order_is_as_expected(expected_ascending)) +class QuestionnaireLiveTest(LiveServerTest): + def test_questionnaire_selection(self): + top_questionnaire = baker.make(Questionnaire, type=Questionnaire.Type.TOP) + bottom_questionnaire = baker.make(Questionnaire, type=Questionnaire.Type.BOTTOM) + with self.enter_staff_mode(): + self.selenium.get(self.reverse("staff:questionnaire_index")) + + top_element = self.selenium.find_element(By.XPATH, "//*[contains(text(),'" + top_questionnaire.name + "')]") + bottom_element = self.selenium.find_element( + By.XPATH, "//*[contains(text(),'" + bottom_questionnaire.name + "')]" + ) + + self.assertTrue(top_element.is_displayed()) + self.assertFalse(bottom_element.is_displayed()) + + self.selenium.find_element(By.ID, "bottomTab").click() + + self.assertFalse(top_element.is_displayed()) + self.assertTrue(bottom_element.is_displayed()) + + class TextAnswerEditLiveTest(LiveServerTest): def test_edit_textanswer_redirect(self): """Regression test for #1696""" diff --git a/evap/staff/tests/test_views.py b/evap/staff/tests/test_views.py index 0b104d8866..fcb39223c9 100644 --- a/evap/staff/tests/test_views.py +++ b/evap/staff/tests/test_views.py @@ -2410,6 +2410,23 @@ def test_general_contribution_log_entry(self): page, '

The Contribution "General Contribution" was created.

', html=True ) + def test_hidden_and_archived_not_shown(self): + hidden_questionnaire = baker.make(Questionnaire, visibility=Questionnaire.Visibility.HIDDEN) + archived_questionnaire = baker.make(Questionnaire, visibility=Questionnaire.Visibility.ARCHIVED) + selected_hidden_questionnaire = baker.make(Questionnaire, visibility=Questionnaire.Visibility.HIDDEN) + selected_archived_questionnaire = baker.make(Questionnaire, visibility=Questionnaire.Visibility.ARCHIVED) + + self.evaluation.general_contribution.questionnaires.set( + [selected_hidden_questionnaire, selected_archived_questionnaire] + ) + + page = self.app.get(self.url, user=self.manager) + + self.assertIn(selected_hidden_questionnaire.name, page) + self.assertIn(selected_archived_questionnaire.name, page) + self.assertNotIn(archived_questionnaire.name, page) + self.assertNotIn(hidden_questionnaire.name, page) + class TestEvaluationDeleteView(WebTestStaffMode): csrf_checks = False @@ -3162,12 +3179,12 @@ def setUpTestData(cls): cls.manager = make_manager() cls.name_de_orig = "kurzer name" cls.name_en_orig = "short name" - questionnaire = baker.make(Questionnaire, name_de=cls.name_de_orig, name_en=cls.name_en_orig) - cls.url = f"/staff/questionnaire/{questionnaire.pk}/new_version" + cls.questionnaire = baker.make(Questionnaire, name_de=cls.name_de_orig, name_en=cls.name_en_orig) + cls.url = f"/staff/questionnaire/{cls.questionnaire.pk}/new_version" - baker.make(QuestionAssignment, questionnaire=questionnaire) + baker.make(QuestionAssignment, questionnaire=cls.questionnaire) - def test_changes_old_title(self): + def test_changes_old_title_and_visibility(self): page = self.app.get(url=self.url, user=self.manager) form = page.forms["questionnaire-form"] @@ -3180,6 +3197,10 @@ def test_changes_old_title(self): self.assertTrue(Questionnaire.objects.filter(name_de=self.name_de_orig, name_en=self.name_en_orig).exists()) self.assertTrue(Questionnaire.objects.filter(name_de=new_name_de, name_en=new_name_en).exists()) + self.assertEqual( + Questionnaire.objects.get(pk=self.questionnaire.pk).visibility, Questionnaire.Visibility.ARCHIVED + ) + def test_no_second_update(self): # First save. page = self.app.get(url=self.url, user=self.manager) @@ -3245,17 +3266,15 @@ class TestQuestionnaireIndexView(WebTestStaffMode): @classmethod def setUpTestData(cls): cls.manager = make_manager() - cls.contributor_questionnaire = baker.make(Questionnaire, type=Questionnaire.Type.CONTRIBUTOR) - cls.top_questionnaire = baker.make(Questionnaire, type=Questionnaire.Type.TOP) - cls.bottom_questionnaire = baker.make(Questionnaire, type=Questionnaire.Type.BOTTOM) + cls.ordered_questionnaire = baker.make(Questionnaire, type=Questionnaire.Type.TOP, order=5) + cls.normal_questionnaire = baker.make(Questionnaire, type=Questionnaire.Type.TOP) def test_ordering(self): content = self.app.get(self.url, user=self.manager).body.decode() - top_index = content.index(self.top_questionnaire.name) - contributor_index = content.index(self.contributor_questionnaire.name) - bottom_index = content.index(self.bottom_questionnaire.name) + ordered_index = content.index(self.ordered_questionnaire.name) + normal_index = content.index(self.normal_questionnaire.name) - self.assertTrue(top_index < contributor_index < bottom_index) + self.assertTrue(normal_index < ordered_index) class TestQuestionnaireEditView(WebTestStaffModeWith200Check): diff --git a/evap/staff/views.py b/evap/staff/views.py index 480f7317d9..dc52871d4c 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -70,6 +70,7 @@ StrOrPromise, get_object_from_dict_pk_entry_or_logged_40x, get_parameter_from_url_or_session, + get_string_from_url_or_session, sort_formset, temporary_receiver, ) @@ -1821,29 +1822,33 @@ def evaluation_preview(request, evaluation_id): @manager_required def questionnaire_index(request): - filter_questionnaires = get_parameter_from_url_or_session(request, "filter_questionnaires") - - prefetch_list = ("questions", "contributions__evaluation") - general_questionnaires = Questionnaire.objects.general_questionnaires().prefetch_related(*prefetch_list) - contributor_questionnaires = Questionnaire.objects.contributor_questionnaires().prefetch_related(*prefetch_list) - dropout_questionnaires = Questionnaire.objects.dropout_questionnaires().prefetch_related(*prefetch_list) - - if filter_questionnaires: - general_questionnaires = general_questionnaires.exclude(visibility=Questionnaire.Visibility.HIDDEN) - contributor_questionnaires = contributor_questionnaires.exclude(visibility=Questionnaire.Visibility.HIDDEN) + filters = ["all", "visible", "archived"] + filter_questionnaires = get_string_from_url_or_session(request, "filter_questionnaires", filters[0]) + if filter_questionnaires not in filters: + raise SuspiciousOperation - general_questionnaires_top = [ - questionnaire for questionnaire in general_questionnaires if questionnaire.is_above_contributors - ] - general_questionnaires_bottom = [ - questionnaire for questionnaire in general_questionnaires if questionnaire.is_below_contributors - ] + match filter_questionnaires: + case "all": + questionnaires_filter = ~Q(visibility=Questionnaire.Visibility.ARCHIVED) + case "visible": + questionnaires_filter = ~Q( + visibility__in=[Questionnaire.Visibility.ARCHIVED, Questionnaire.Visibility.HIDDEN] + ) + case "archived": + questionnaires_filter = Q(visibility=Questionnaire.Visibility.ARCHIVED) + + questionnaires = ( + Questionnaire.objects.all() + .filter(questionnaires_filter) + .order_by("order", "pk") + .prefetch_related("questions", "contributions__evaluation") + ) template_data = { - "general_questionnaires_top": general_questionnaires_top, - "general_questionnaires_bottom": general_questionnaires_bottom, - "contributor_questionnaires": contributor_questionnaires, - "dropout_questionnaires": dropout_questionnaires, + "general_questionnaires_top": list(questionnaires.filter(type=Questionnaire.Type.TOP)), + "general_questionnaires_bottom": list(questionnaires.filter(type=Questionnaire.Type.BOTTOM)), + "contributor_questionnaires": list(questionnaires.filter(type=Questionnaire.Type.CONTRIBUTOR)), + "dropout_questionnaires": list(questionnaires.filter(type=Questionnaire.Type.DROPOUT)), "filter_questionnaires": filter_questionnaires, } return render(request, "staff_questionnaire_index.html", template_data) @@ -2072,7 +2077,7 @@ def questionnaire_new_version(request, questionnaire_id): # Change old name before checking Form. old_questionnaire.name_de = new_name_de old_questionnaire.name_en = new_name_en - old_questionnaire.visibility = Questionnaire.Visibility.HIDDEN + old_questionnaire.visibility = Questionnaire.Visibility.ARCHIVED old_questionnaire.save() if not form.is_valid() or not formset.is_valid(): diff --git a/evap/static/scss/_adjustments.scss b/evap/static/scss/_adjustments.scss index fba140f17a..e468fd6053 100644 --- a/evap/static/scss/_adjustments.scss +++ b/evap/static/scss/_adjustments.scss @@ -173,3 +173,11 @@ $alert-colors: ( input[type="search"]::-webkit-search-cancel-button { -webkit-appearance: none; } + +// Fix tabs in card headers +.card .card-header .nav-tabs .nav-item .nav-link { + border: none; + &:hover:not(.active) { + background-color: $light-gray; + } +}