diff --git a/evap/results/fixtures/minimal_test_data_results.json b/evap/results/fixtures/minimal_test_data_results.json index 76d7e577e..b049f2db8 100644 --- a/evap/results/fixtures/minimal_test_data_results.json +++ b/evap/results/fixtures/minimal_test_data_results.json @@ -109,17 +109,6 @@ "type": 2 } }, -{ - "pk": 6, - "model": "evaluation.question", - "fields": { - "questionnaire": 2, - "text_en": "can you see me?", - "text_de": "can you see me?", - "allows_additional_textanswers": false, - "type": 0 - } -}, { "pk": "00000000-0000-0000-0000-000000000001", "model": "evaluation.textanswer", @@ -135,7 +124,7 @@ "pk": "00000000-0000-0000-0000-000000000002", "model": "evaluation.textanswer", "fields": { - "answer": ".general_orig_hidden.", + "answer": ".general_orig_deleted.", "question": 3, "original_answer": null, "contribution": 1, @@ -190,7 +179,7 @@ "pk": "00000000-0000-0000-0000-000000000012", "model": "evaluation.textanswer", "fields": { - "answer": ".responsible_contributor_orig_hidden.", + "answer": ".responsible_contributor_orig_deleted.", "question": 1, "original_answer": null, "contribution": 6, @@ -223,24 +212,13 @@ "pk": "00000000-0000-0000-0000-000000000015", "model": "evaluation.textanswer", "fields": { - "answer": ".responsible_contributor_orig_notreviewed.", + "answer": ".responsible_contributor_orig_unreviewed.", "question": 1, "original_answer": null, "contribution": 6, "review_decision": "UN" } }, -{ - "pk": "00000000-0000-0000-0000-000000000016", - "model": "evaluation.textanswer", - "fields": { - "answer": ".proxy_user_visibility_info.", - "question": 6, - "original_answer": null, - "contribution": 1, - "review_decision": "PU" - } -}, { "pk": "00000000-0000-0000-0000-000000000017", "model": "evaluation.textanswer", @@ -256,7 +234,7 @@ "pk": "00000000-0000-0000-0000-000000000018", "model": "evaluation.textanswer", "fields": { - "answer": ".responsible_contributor_additional_orig_hidden.", + "answer": ".responsible_contributor_additional_orig_deleted.", "question": 2, "original_answer": null, "contribution": 6, @@ -278,7 +256,7 @@ "pk": "00000000-0000-0000-0000-000000000020", "model": "evaluation.textanswer", "fields": { - "answer": ".general_additional_orig_hidden.", + "answer": ".general_additional_orig_deleted.", "question": 4, "original_answer": null, "contribution": 1, diff --git a/evap/results/templates/results_evaluation_detail.html b/evap/results/templates/results_evaluation_detail.html index 9bbe685f6..472825ae2 100644 --- a/evap/results/templates/results_evaluation_detail.html +++ b/evap/results/templates/results_evaluation_detail.html @@ -21,7 +21,7 @@ {% if evaluation.state != evaluation.State.PUBLISHED %}
{% translate 'This is a preview. The results have not been published yet.' %}
{% endif %} - + {# manager is automatically a reviewer (so this are all groups that can see the buttons) #} {% if is_reviewer or is_responsible_or_contributor_or_delegate %} {% if evaluation.course.is_private %}
@@ -32,68 +32,74 @@

{{ evaluation.full_name }} ({{ evaluation.course.semester.name }})

-
-
-
{% translate 'View' %}
-
- {% if user.is_staff and view == 'export' or is_contributor %} - - {% translate 'Export' context 'view mode' %} - - - {% endif %} - - {% translate 'Full' %} - - {% if not evaluation.can_publish_rating_results %} - - {% else %} - - {% translate 'Participant' %} - {% else %} - {% translate 'Shows results available for everyone logged in.' %}" - > - {% translate 'Public' %} - {% endif %} - - {% endif %} +
+
+
+
{% translate 'General results' %}
+
+ + {% translate 'Full' %} + + + {% translate 'Ratings' %} + +
+
+
+
+ +
+
+
+
{% translate 'Contributor results' %}
+
+ + {% translate 'Full' %} + + + {% translate 'Ratings' %} + + + {% translate 'Personal' %} + +
+
@@ -106,8 +112,8 @@

{{ evaluation.full_name }} ({{ evaluation.course.semester.name }})

{% translate 'Overview' %}
{% if can_export_text_answers %} - - {% translate 'Export text answers' %} + + {% translate 'Export visible text answers' %} {% endif %} {% if evaluation.course.grade_documents.count == 1 and can_download_grades %} diff --git a/evap/results/tests/test_tools.py b/evap/results/tests/test_tools.py index 0891d85c1..35ef02e36 100644 --- a/evap/results/tests/test_tools.py +++ b/evap/results/tests/test_tools.py @@ -18,6 +18,8 @@ ) from evap.evaluation.tests.tools import TestCase, make_rating_answer_counters from evap.results.tools import ( + ViewContributorResults, + ViewGeneralResults, cache_results, calculate_average_course_distribution, calculate_average_distribution, @@ -562,8 +564,12 @@ def test_contributors_and_delegate_count_in_textanswer_visibility_info(self): for user in UserProfile.objects.all(): represented_users = [user] + list(user.represented_users.all()) for i, textanswer in enumerate(textanswers): - if can_textanswer_be_seen_by(user, represented_users, textanswer, "full"): - if can_textanswer_be_seen_by(user, [user], textanswer, "full"): + if can_textanswer_be_seen_by( + user, represented_users, textanswer, ViewGeneralResults.FULL, ViewContributorResults.FULL + ): + if can_textanswer_be_seen_by( + user, [user], textanswer, ViewGeneralResults.FULL, ViewContributorResults.FULL + ): users_seeing_contribution[i][0].add(user) else: users_seeing_contribution[i][1].add(user) diff --git a/evap/results/tests/test_views.py b/evap/results/tests/test_views.py index 0abf6590c..ac80f2b84 100644 --- a/evap/results/tests/test_views.py +++ b/evap/results/tests/test_views.py @@ -31,7 +31,7 @@ render_pages, ) from evap.results.exporters import TextAnswerExporter -from evap.results.tools import cache_results +from evap.results.tools import ViewContributorResults, ViewGeneralResults, cache_results from evap.results.views import get_evaluations_with_prefetched_data, update_template_cache from evap.staff.tests.utils import WebTestStaffMode, helper_exit_staff_mode, run_in_staff_mode @@ -485,23 +485,38 @@ def test_heading_question_filtering(self): self.assertNotIn(heading_question_2.text, page) @override_settings(VOTER_COUNT_NEEDED_FOR_PUBLISHING_RATING_RESULTS=0) - def test_default_view_is_public(self): + def test_default_view(self): cache_results(self.evaluation) page_without_get_parameter = self.app.get(self.url, user=self.manager) - self.assertEqual(page_without_get_parameter.context["view"], "public") + self.assertEqual(page_without_get_parameter.context["view_general_results"], ViewGeneralResults.FULL) + self.assertEqual(page_without_get_parameter.context["view_contributor_results"], ViewContributorResults.FULL) - page_with_get_parameter = self.app.get(self.url + "?view=public", user=self.manager) - self.assertEqual(page_with_get_parameter.context["view"], "public") - - page_with_random_get_parameter = self.app.get(self.url + "?view=asdf", user=self.manager) - self.assertEqual(page_with_random_get_parameter.context["view"], "public") + page_with_ratings_general_get_parameter = self.app.get( + self.url + "?view_general_results=ratings", user=self.manager + ) + self.assertEqual( + page_with_ratings_general_get_parameter.context["view_general_results"], ViewGeneralResults.RATINGS + ) + self.assertEqual( + page_with_ratings_general_get_parameter.context["view_contributor_results"], ViewContributorResults.FULL + ) - page_with_full_get_parameter = self.app.get(f"{self.url}?view=full", user=self.manager) - self.assertEqual(page_with_full_get_parameter.context["view"], "full") + page_with_ratings_general_get_parameter = self.app.get( + self.url + "?view_contributor_results=ratings", user=self.manager + ) + self.assertEqual( + page_with_ratings_general_get_parameter.context["view_general_results"], ViewGeneralResults.FULL + ) + self.assertEqual( + page_with_ratings_general_get_parameter.context["view_contributor_results"], ViewContributorResults.RATINGS + ) - page_with_export_get_parameter = self.app.get(f"{self.url}?view=export", user=self.manager) - self.assertEqual(page_with_export_get_parameter.context["view"], "export") + self.app.get( # raises bad request + self.url + "?view_general_results=josefwarhier&view_contributor_results=yannikwarhier", + user=self.manager, + status=400, + ) def test_wrong_state(self): helper_exit_staff_mode(self) @@ -584,24 +599,6 @@ def test_invalid_contributor_id(self): self.app.get(self.url + "?contributor_id=asd", user=self.manager, status=400) self.app.get(self.url + "?contributor_id=1234", user=self.manager, status=404) - def test_evaluation_sorting(self): - names = ["EvaluationB", "EvaluationA", "EvaluationC"] - additional_evaluations = baker.make( - Evaluation, - state=Evaluation.State.PUBLISHED, - course=self.evaluation.course, - _quantity=3, - _bulk_create=True, - name_en=iter(names), - name_de=iter(names), - ) - - for evaluation in (self.evaluation, *additional_evaluations): - cache_results(evaluation) - - body = self.app.get(self.url, user=self.manager).body.decode() - self.assertTrue(body.find("EvaluationA") < body.find("EvaluationB") < body.find("EvaluationC")) - class TestResultsSemesterEvaluationDetailViewFewVoters(WebTest): @classmethod @@ -732,216 +729,183 @@ def test_private_evaluation(self): self.app.get(url, user=student_external, status=200) -class TestResultsTextanswerVisibilityForManager(WebTestStaffMode): +class TestResultsTextanswerVisibility(WebTest): + fixtures = ["minimal_test_data_results"] + general_textanswers = [ + ".general_orig_published.", + ".general_orig_deleted.", + ".general_changed_published.", + ".general_orig_published_changed.", + ".general_additional_orig_published.", + ".general_additional_orig_deleted.", + ] + + contributor_textanswers = [ + ".contributor_orig_published.", + ".contributor_orig_private.", + ".responsible_contributor_orig_published.", + ".responsible_contributor_orig_deleted.", + ".responsible_contributor_changed_published.", + ".responsible_contributor_orig_published_changed.", + ".responsible_contributor_orig_private.", + ".responsible_contributor_orig_unreviewed.", + ".responsible_contributor_additional_orig_published.", + ".responsible_contributor_additional_orig_deleted.", + ] + + standard_general_textanswers = [ + ".general_orig_published.", + ".general_changed_published.", + ".general_additional_orig_published.", + ] @classmethod def setUpTestData(cls): cls.manager = make_manager() cache_results(Evaluation.objects.get(id=1)) - def test_textanswer_visibility_for_manager_before_publish(self): - evaluation = Evaluation.objects.get(id=1) - voter_count = evaluation._voter_count - participant_count = evaluation._participant_count - evaluation._voter_count = 0 # set these to 0 to make unpublishing work - evaluation._participant_count = 0 - evaluation.unpublish() - evaluation._voter_count = voter_count # reset to original values - evaluation._participant_count = participant_count - evaluation.save() - - page = self.app.get("/results/semester/1/evaluation/1?view=full", user=self.manager) - self.assertIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertIn(".general_additional_orig_published.", page) - self.assertNotIn(".general_additional_orig_hidden.", page) - self.assertIn(".general_changed_published.", page) - self.assertIn(".contributor_orig_published.", page) - self.assertIn(".contributor_orig_private.", page) - self.assertIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertIn(".responsible_contributor_changed_published.", page) - self.assertIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - self.assertIn(".responsible_contributor_additional_orig_published.", page) - self.assertNotIn(".responsible_contributor_additional_orig_hidden.", page) - - def test_textanswer_visibility_for_manager(self): - page = self.app.get("/results/semester/1/evaluation/1?view=full", user=self.manager) - self.assertIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertIn(".general_additional_orig_published.", page) - self.assertNotIn(".general_additional_orig_hidden.", page) - self.assertIn(".general_changed_published.", page) - self.assertIn(".contributor_orig_published.", page) - self.assertIn(".contributor_orig_private.", page) - self.assertIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertIn(".responsible_contributor_changed_published.", page) - self.assertIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - self.assertIn(".responsible_contributor_additional_orig_published.", page) - self.assertNotIn(".responsible_contributor_additional_orig_hidden.", page) + def helper_test_general(self, user, view_general_results, expected_visible_textanswers): + + textanswers_not_in = list(set(self.general_textanswers) - set(expected_visible_textanswers)) + for contributor_view in ViewContributorResults: + page = self.app.get( + f"/results/semester/1/evaluation/1?view_general_results={view_general_results.value}&view_contributor_results={contributor_view.value}", + user=user, + ) + + for answer in expected_visible_textanswers: + self.assertIn(answer, page) + for answer in textanswers_not_in: + self.assertNotIn(answer, page) + def helper_test_contributor(self, user, view_contributor_results, expected_visible_textanswers): -class TestResultsTextanswerVisibility(WebTest): - fixtures = ["minimal_test_data_results"] + textanswers_not_in = list(set(self.contributor_textanswers) - set(expected_visible_textanswers)) + for general_view in ViewGeneralResults: + page = self.app.get( + f"/results/semester/1/evaluation/1?view_contributor_results={view_contributor_results.value}&view_general_results={general_view.value}", + user=user, + ) - @classmethod - def setUpTestData(cls): - cache_results(Evaluation.objects.get(id=1)) + for answer in expected_visible_textanswers: + self.assertIn(answer, page) + for answer in textanswers_not_in: + self.assertNotIn(answer, page) + + def test_manager(self): + with run_in_staff_mode(self): # in staff mode, the manager can see everything + user = "manager@institution.example.com" + + self.helper_test_general( + user, + ViewGeneralResults.FULL, + self.standard_general_textanswers, + ) + self.helper_test_general(user, ViewGeneralResults.RATINGS, []) + + self.helper_test_contributor( + user, + ViewContributorResults.FULL, + [ + ".contributor_orig_published.", + ".contributor_orig_private.", + ".responsible_contributor_orig_published.", + ".responsible_contributor_changed_published.", + ".responsible_contributor_orig_private.", + ".responsible_contributor_additional_orig_published.", + ], + ) + self.helper_test_contributor(user, ViewContributorResults.RATINGS, []) + self.helper_test_contributor(user, ViewContributorResults.PERSONAL, []) + + user = "manager@institution.example.com" + + self.helper_test_general(user, ViewGeneralResults.FULL, []) + self.helper_test_general(user, ViewGeneralResults.RATINGS, []) + + self.helper_test_contributor(user, ViewContributorResults.FULL, []) + self.helper_test_contributor(user, ViewContributorResults.RATINGS, []) + self.helper_test_contributor(user, ViewContributorResults.PERSONAL, []) + + def test_student(self): + user = "student@institution.example.com" + + self.helper_test_general(user, ViewGeneralResults.FULL, []) + self.helper_test_general(user, ViewGeneralResults.RATINGS, []) + + self.helper_test_contributor(user, ViewContributorResults.FULL, []) + self.helper_test_contributor(user, ViewContributorResults.RATINGS, []) + self.helper_test_contributor(user, ViewContributorResults.PERSONAL, []) - def test_textanswer_visibility_for_responsible(self): - page = self.app.get("/results/semester/1/evaluation/1", user="responsible@institution.example.com") - self.assertIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertIn(".general_additional_orig_published.", page) - self.assertNotIn(".general_additional_orig_hidden.", page) - self.assertIn(".general_changed_published.", page) - self.assertNotIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - self.assertNotIn(".responsible_contributor_additional_orig_published.", page) - self.assertNotIn(".responsible_contributor_additional_orig_hidden.", page) - - def test_textanswer_visibility_for_responsible_contributor(self): - page = self.app.get("/results/semester/1/evaluation/1", user="responsible_contributor@institution.example.com") - self.assertIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertIn(".general_additional_orig_published.", page) - self.assertNotIn(".general_additional_orig_hidden.", page) - self.assertIn(".general_changed_published.", page) - self.assertNotIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertIn(".responsible_contributor_changed_published.", page) - self.assertIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - self.assertIn(".responsible_contributor_additional_orig_published.", page) - self.assertNotIn(".responsible_contributor_additional_orig_hidden.", page) - - def test_textanswer_visibility_for_delegate_for_responsible(self): - page = self.app.get("/results/semester/1/evaluation/1", user="delegate_for_responsible@institution.example.com") - self.assertIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertIn(".general_additional_orig_published.", page) - self.assertNotIn(".general_additional_orig_hidden.", page) - self.assertIn(".general_changed_published.", page) - self.assertNotIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - self.assertNotIn(".responsible_contributor_additional_orig_published.", page) - self.assertNotIn(".responsible_contributor_additional_orig_hidden.", page) - - def test_textanswer_visibility_for_contributor(self): - page = self.app.get("/results/semester/1/evaluation/1", user="contributor@institution.example.com") - self.assertNotIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertNotIn(".general_additional_orig_published.", page) - self.assertNotIn(".general_additional_orig_hidden.", page) - self.assertNotIn(".general_changed_published.", page) - self.assertIn(".contributor_orig_published.", page) - self.assertIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - self.assertNotIn(".responsible_contributor_additional_orig_published.", page) - self.assertNotIn(".responsible_contributor_additional_orig_hidden.", page) - - def test_textanswer_visibility_for_delegate_for_contributor(self): - page = self.app.get("/results/semester/1/evaluation/1", user="delegate_for_contributor@institution.example.com") - self.assertNotIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertNotIn(".general_additional_orig_published.", page) - self.assertNotIn(".general_additional_orig_hidden.", page) - self.assertNotIn(".general_changed_published.", page) - self.assertIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - self.assertNotIn(".responsible_contributor_additional_orig_published.", page) - self.assertNotIn(".responsible_contributor_additional_orig_hidden.", page) - - def test_textanswer_visibility_for_contributor_general_textanswers(self): - page = self.app.get( - "/results/semester/1/evaluation/1", user="contributor_general_textanswers@institution.example.com" + def test_responsible(self): + + user = "responsible@institution.example.com" + + self.helper_test_general( + user, + ViewGeneralResults.FULL, + self.standard_general_textanswers, + ) + self.helper_test_general(user, ViewGeneralResults.RATINGS, []) + + self.helper_test_contributor(user, ViewContributorResults.FULL, []) + self.helper_test_contributor(user, ViewContributorResults.RATINGS, []) + self.helper_test_contributor(user, ViewContributorResults.PERSONAL, []) + + def test_responsible_contributor(self): + + user = "responsible_contributor@institution.example.com" + contributor_textanswers_responsible_contributor_can_see = [ + ".responsible_contributor_orig_published.", + ".responsible_contributor_changed_published.", + ".responsible_contributor_orig_private.", + ".responsible_contributor_additional_orig_published.", + ] + + self.helper_test_general( + user, + ViewGeneralResults.FULL, + self.standard_general_textanswers, ) - self.assertIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertIn(".general_additional_orig_published.", page) - self.assertNotIn(".general_additional_orig_hidden.", page) - self.assertIn(".general_changed_published.", page) - self.assertNotIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - self.assertNotIn(".responsible_contributor_additional_orig_published.", page) - self.assertNotIn(".responsible_contributor_additional_orig_hidden.", page) - - def test_textanswer_visibility_for_student(self): - page = self.app.get("/results/semester/1/evaluation/1", user="student@institution.example.com") - self.assertNotIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertNotIn(".general_additional_orig_published.", page) - self.assertNotIn(".general_additional_orig_hidden.", page) - self.assertNotIn(".general_changed_published.", page) - self.assertNotIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - self.assertNotIn(".responsible_contributor_additional_orig_published.", page) - self.assertNotIn(".responsible_contributor_additional_orig_hidden.", page) - - def test_textanswer_visibility_for_student_external(self): - # the external user does not participate in or contribute to the evaluation and therefore can't see the results - self.app.get("/results/semester/1/evaluation/1", user="student_external@example.com", status=403) - - def test_textanswer_visibility_info_is_shown(self): - page = self.app.get("/results/semester/1/evaluation/1", user="contributor@institution.example.com") - self.assertRegex(page.body.decode(), r"can be seen by:
\s*contributor user") - - def test_textanswer_visibility_info_for_proxy_user(self): - page = self.app.get("/results/semester/1/evaluation/1", user="responsible@institution.example.com") - self.assertIn("responsible_contributor user (1 person)", page) + self.helper_test_general(user, ViewGeneralResults.RATINGS, []) + + self.helper_test_contributor( + user, + ViewContributorResults.FULL, + contributor_textanswers_responsible_contributor_can_see, + ) + self.helper_test_contributor(user, ViewContributorResults.RATINGS, []) + self.helper_test_contributor( + user, + ViewContributorResults.PERSONAL, + contributor_textanswers_responsible_contributor_can_see, + ) + + def test_contributor_general_textanswers(self): + user = "contributor_general_textanswers@institution.example.com" + + self.helper_test_general( + user, + ViewGeneralResults.FULL, + self.standard_general_textanswers, + ) + self.helper_test_general(user, ViewGeneralResults.RATINGS, []) + + self.helper_test_contributor(user, ViewContributorResults.FULL, []) + self.helper_test_contributor(user, ViewContributorResults.RATINGS, []) + self.helper_test_contributor(user, ViewContributorResults.PERSONAL, []) + + def test_contributor(self): + user = "contributor@institution.example.com" + contributor_textanswers_contributor_can_see = [".contributor_orig_published.", ".contributor_orig_private."] + + self.helper_test_general(user, ViewGeneralResults.FULL, []) + self.helper_test_general(user, ViewGeneralResults.RATINGS, []) + + self.helper_test_contributor(user, ViewContributorResults.FULL, contributor_textanswers_contributor_can_see) + self.helper_test_contributor(user, ViewContributorResults.RATINGS, []) + self.helper_test_contributor(user, ViewContributorResults.PERSONAL, contributor_textanswers_contributor_can_see) class TestResultsOtherContributorsListOnExportView(WebTest): @@ -950,7 +914,7 @@ def setUpTestData(cls): cls.responsible = baker.make(UserProfile, email="responsible@institution.example.com") evaluation = baker.make(Evaluation, state=Evaluation.State.PUBLISHED) - cls.url = f"/results/semester/{evaluation.course.semester.id}/evaluation/{evaluation.id}?view=export" + cls.url = f"/results/semester/{evaluation.course.semester.id}/evaluation/{evaluation.id}?view_contributor_results=personal" questionnaire = baker.make(Questionnaire) baker.make(Question, questionnaire=questionnaire, type=QuestionType.POSITIVE_LIKERT) @@ -988,143 +952,6 @@ def test_contributor_list(self): self.assertIn(f"
  • {self.other_contributor_2.full_name}
    • ", page) -class TestResultsTextanswerVisibilityForExportView(WebTest): - fixtures = ["minimal_test_data_results"] - - @classmethod - def setUpTestData(cls): - cls.manager = make_manager() - cache_results(Evaluation.objects.get(id=1)) - - def test_textanswer_visibility_for_responsible(self): - page = self.app.get("/results/semester/1/evaluation/1?view=export", user="responsible@institution.example.com") - - self.assertIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertIn(".general_changed_published.", page) - self.assertNotIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - - def test_textanswer_visibility_for_responsible_contributor(self): - page = self.app.get( - "/results/semester/1/evaluation/1?view=export", user="responsible_contributor@institution.example.com" - ) - - self.assertIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertIn(".general_changed_published.", page) - self.assertNotIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - - def test_textanswer_visibility_for_contributor(self): - page = self.app.get("/results/semester/1/evaluation/1?view=export", user="contributor@institution.example.com") - - self.assertNotIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertNotIn(".general_changed_published.", page) - self.assertIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - - def test_textanswer_visibility_for_contributor_general_textanswers(self): - page = self.app.get( - "/results/semester/1/evaluation/1?view=export", - user="contributor_general_textanswers@institution.example.com", - ) - - self.assertIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertIn(".general_changed_published.", page) - self.assertNotIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - - def test_textanswer_visibility_for_student(self): - page = self.app.get("/results/semester/1/evaluation/1?view=export", user="student@institution.example.com") - - self.assertNotIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertNotIn(".general_changed_published.", page) - self.assertNotIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - - def test_textanswer_visibility_for_manager(self): - with run_in_staff_mode(self): - contributor_id = UserProfile.objects.get(email="responsible@institution.example.com").id - page = self.app.get( - f"/results/semester/1/evaluation/1?view=export&contributor_id={contributor_id}", - user="manager@institution.example.com", - ) - - self.assertIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertIn(".general_changed_published.", page) - self.assertNotIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - - def test_textanswer_visibility_for_manager_contributor(self): - manager_group = Group.objects.get(name="Manager") - contributor = UserProfile.objects.get(email="contributor@institution.example.com") - contributor.groups.add(manager_group) - page = self.app.get( - f"/results/semester/1/evaluation/1?view=export&contributor_id={contributor.id}", - user="contributor@institution.example.com", - ) - - self.assertNotIn(".general_orig_published.", page) - self.assertNotIn(".general_orig_hidden.", page) - self.assertNotIn(".general_orig_published_changed.", page) - self.assertNotIn(".general_changed_published.", page) - self.assertIn(".contributor_orig_published.", page) - self.assertNotIn(".contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_published.", page) - self.assertNotIn(".responsible_contributor_orig_hidden.", page) - self.assertNotIn(".responsible_contributor_orig_published_changed.", page) - self.assertNotIn(".responsible_contributor_changed_published.", page) - self.assertNotIn(".responsible_contributor_orig_private.", page) - self.assertNotIn(".responsible_contributor_orig_notreviewed.", page) - - class TestArchivedResults(WebTest): @classmethod def setUpTestData(cls): diff --git a/evap/results/tools.py b/evap/results/tools.py index e4f7abf43..fb22bf9d0 100644 --- a/evap/results/tools.py +++ b/evap/results/tools.py @@ -1,6 +1,7 @@ from collections import OrderedDict, defaultdict from collections.abc import Iterable from copy import copy +from enum import Enum from math import ceil, modf from typing import TypeGuard, cast @@ -36,6 +37,25 @@ } +class ViewGeneralResults(Enum): + @property + def do_not_call_in_templates(self): + return True + + FULL = "full" + RATINGS = "ratings" + + +class ViewContributorResults(Enum): + @property + def do_not_call_in_templates(self): + return True + + FULL = "full" + RATINGS = "ratings" + PERSONAL = "personal" + + class TextAnswerVisibility: def __init__(self, visible_by_contribution: Iterable[UserProfile], visible_by_delegation_count: int): self.visible_by_contribution = [discard_cached_related_objects(copy(user)) for user in visible_by_contribution] @@ -473,49 +493,48 @@ def textanswers_visible_to(contribution): return TextAnswerVisibility(visible_by_contribution=sorted_contributors, visible_by_delegation_count=num_delegates) -def can_textanswer_be_seen_by( # noqa: PLR0911 +def can_textanswer_be_seen_by( # noqa: PLR0911,PLR0912 user: UserProfile, represented_users: list[UserProfile], textanswer: TextAnswer, - view: str, + view_general_results: ViewGeneralResults, + view_contributor_results: ViewContributorResults, ) -> bool: assert textanswer.review_decision in [TextAnswer.ReviewDecision.PRIVATE, TextAnswer.ReviewDecision.PUBLIC] contributor = textanswer.contribution.contributor - if view == "public": - return False - - if view == "export": - if textanswer.is_private: - return False - if not textanswer.contribution.is_general and contributor != user: - return False - elif user.is_reviewer: - return True - - if textanswer.is_private: - return contributor == user - # NOTE: when changing this behavior, make sure all changes are also reflected in results.tools.textanswers_visible_to # and in results.tests.test_tools.TestTextAnswerVisibilityInfo - if textanswer.is_public: - # users can see textanswers if the contributor is one of their represented users (which includes the user itself) - if contributor in represented_users: - return True - # users can see text answers from general contributions if one of their represented users has text answer - # visibility GENERAL_TEXTANSWERS for the evaluation - if ( - textanswer.contribution.is_general - and textanswer.contribution.evaluation.contributions.filter( + if textanswer.contribution.is_general: + if view_general_results == ViewGeneralResults.FULL: + if user.is_reviewer: + return True + + # user can see general textanswer if represented user can see it + if textanswer.contribution.evaluation.contributions.filter( contributor__in=represented_users, textanswer_visibility=Contribution.TextAnswerVisibility.GENERAL_TEXTANSWERS, - ).exists() - ): - return True - # the people responsible for a course can see all general text answers for all its evaluations - if textanswer.contribution.is_general and any( - user in represented_users for user in textanswer.contribution.evaluation.course.responsibles.all() - ): + ).exists(): + return True + + # the people responsible for a course can see all general text answers for all its evaluations + if any( + user in represented_users # includes self + for user in textanswer.contribution.evaluation.course.responsibles.all() + ): + return True + else: + if view_contributor_results == ViewContributorResults.RATINGS: + return False + if user.is_reviewer: return True + if ( + view_contributor_results == ViewContributorResults.PERSONAL or textanswer.is_private + ): # private textanswers should only be seen by the contributor and reviewer + return contributor == user + if view_contributor_results == ViewContributorResults.FULL: + # users can see textanswers if the contributor is one of their represented users (which includes the user itself) + if contributor in represented_users: + return True return False diff --git a/evap/results/views.py b/evap/results/views.py index 9d4adb605..c67792b9f 100644 --- a/evap/results/views.py +++ b/evap/results/views.py @@ -11,7 +11,7 @@ from django.utils import translation from evap.evaluation.auth import internal_required -from evap.evaluation.models import Course, CourseType, Evaluation, Program, Semester, UserProfile +from evap.evaluation.models import Contribution, Course, CourseType, Evaluation, Program, Semester, UserProfile from evap.evaluation.tools import AttachmentResponse from evap.results.exporters import TextAnswerExporter from evap.results.tools import ( @@ -19,6 +19,8 @@ HeadingResult, RatingResult, TextResult, + ViewContributorResults, + ViewGeneralResults, annotate_distributions_and_grades, can_textanswer_be_seen_by, get_evaluations_with_course_result_attributes, @@ -168,23 +170,31 @@ def evaluation_detail(request, semester_id, evaluation_id): semester = get_object_or_404(Semester, id=semester_id) evaluation = get_object_or_404(semester.evaluations, id=evaluation_id, course__semester=semester) - view, view_as_user, represented_users, contributor_id = evaluation_detail_parse_get_parameters(request, evaluation) + ( + view_general_results, + view_contributor_results, + view_as_user, + represented_users, + contributor_id, + ) = evaluation_detail_parse_get_parameters(request, evaluation) evaluation_result = get_results(evaluation) - remove_textanswers_that_the_user_must_not_see(evaluation_result, view_as_user, represented_users, view) + remove_textanswers_that_the_user_must_not_see( + evaluation_result, view_as_user, represented_users, view_general_results, view_contributor_results + ) exclude_empty_headings(evaluation_result) remove_empty_questionnaire_and_contribution_results(evaluation_result) add_warnings(evaluation, evaluation_result) top_results, bottom_results, contributor_results = split_evaluation_result_into_top_bottom_and_contributor( - evaluation_result, view_as_user, view + evaluation_result, view_as_user, view_contributor_results ) course_evaluations = get_evaluations_of_course(evaluation.course, request) course_evaluations.sort(key=lambda evaluation: evaluation.name) contributors_with_omitted_results = [] - if view == "export": + if view_contributor_results == ViewContributorResults.PERSONAL: contributors_with_omitted_results = [ contribution_result.contributor for contribution_result in evaluation_result.contribution_results @@ -199,6 +209,30 @@ def evaluation_detail(request, semester_id, evaluation_id): is_responsible_or_contributor_or_delegate = evaluation.is_user_responsible_or_contributor_or_delegate(view_as_user) + general_textanswers = False + contributor_textanswers = False + contributor_personal = False + + if view_as_user.is_reviewer: + general_textanswers = True + contributor_textanswers = True + + if evaluation.is_user_contributor(view_as_user): + contributor_personal = True + for user in represented_users: + if user in evaluation.course.responsibles.all(): + general_textanswers = True + if evaluation.is_user_contributor(user): + if Contribution.objects.filter( + contributor=user, + evaluation=evaluation, + textanswer_visibility=Contribution.TextAnswerVisibility.GENERAL_TEXTANSWERS, + ).exists(): + general_textanswers = True + contributor_textanswers = True + else: + contributor_textanswers = True + template_data = { "evaluation": evaluation, "course": evaluation.course, @@ -211,30 +245,46 @@ def evaluation_detail(request, semester_id, evaluation_id): "is_responsible_or_contributor_or_delegate": is_responsible_or_contributor_or_delegate, "can_download_grades": view_as_user.can_download_grades, "can_export_text_answers": ( - view in ("export", "full") and (view_as_user.is_reviewer or is_responsible_or_contributor_or_delegate) + ( + view_general_results == ViewGeneralResults.FULL + or view_contributor_results in (ViewContributorResults.PERSONAL, ViewContributorResults.FULL) + ) + and (view_as_user.is_reviewer or is_responsible_or_contributor_or_delegate) ), - "view": view, + "view_contributor_results": view_contributor_results, + "view_general_results": view_general_results, "view_as_user": view_as_user, "contributors_with_omitted_results": contributors_with_omitted_results, "contributor_id": contributor_id, + "general_textanswers": general_textanswers, + "contributor_textanswers": contributor_textanswers, + "contributor_personal": contributor_personal, + "ViewContributorResults": ViewContributorResults, + "ViewGeneralResults": ViewGeneralResults, } return render(request, "results_evaluation_detail.html", template_data) -def remove_textanswers_that_the_user_must_not_see(evaluation_result, user, represented_users, view): +def remove_textanswers_that_the_user_must_not_see( + evaluation_result, user, represented_users, view_general_results, view_contributor_results +): for questionnaire_result in evaluation_result.questionnaire_results: for question_result in questionnaire_result.question_results: if isinstance(question_result, TextResult): question_result.answers = [ answer for answer in question_result.answers - if can_textanswer_be_seen_by(user, represented_users, answer, view) + if can_textanswer_be_seen_by( + user, represented_users, answer, view_general_results, view_contributor_results + ) ] if isinstance(question_result, RatingResult) and question_result.additional_text_result: question_result.additional_text_result.answers = [ answer for answer in question_result.additional_text_result.answers - if can_textanswer_be_seen_by(user, represented_users, answer, view) + if can_textanswer_be_seen_by( + user, represented_users, answer, view_general_results, view_contributor_results + ) ] # remove empty TextResults cleaned_results = [] @@ -290,7 +340,7 @@ def remove_empty_questionnaire_and_contribution_results(evaluation_result): ] -def split_evaluation_result_into_top_bottom_and_contributor(evaluation_result, view_as_user, view): +def split_evaluation_result_into_top_bottom_and_contributor(evaluation_result, view_as_user, view_contributor_results): top_results = [] bottom_results = [] contributor_results = [] @@ -302,7 +352,11 @@ def split_evaluation_result_into_top_bottom_and_contributor(evaluation_result, v bottom_results.append(questionnaire_result) else: top_results.append(questionnaire_result) - elif view != "export" or view_as_user.id == contribution_result.contributor.id: + + elif ( + view_contributor_results != ViewContributorResults.PERSONAL + or view_as_user.id == contribution_result.contributor.id + ): contributor_results.append(contribution_result) if not contributor_results: @@ -377,9 +431,22 @@ def evaluation_detail_parse_get_parameters(request, evaluation): if not evaluation.can_results_page_be_seen_by(request.user): raise PermissionDenied - view = request.GET.get("view", "public" if request.user.is_reviewer else "full") - if view not in ["public", "full", "export"]: - view = "public" + try: + view_contributor_results = ViewContributorResults( + request.GET.get("view_contributor_results", ViewContributorResults.FULL) + ) + except ValueError as e: + raise BadRequest from e + + try: + view_general_results = ViewGeneralResults( + request.GET.get( + "view_general_results", + ViewGeneralResults.FULL, + ) + ) + except ValueError as e: + raise BadRequest from e view_as_user = request.user try: @@ -387,28 +454,31 @@ def evaluation_detail_parse_get_parameters(request, evaluation): except ValueError as e: raise BadRequest from e - if view == "export" and request.user.is_staff: + if view_contributor_results == ViewContributorResults.PERSONAL and request.user.is_staff: view_as_user = contributor contributor_id = contributor.pk if contributor != request.user else None represented_users = [view_as_user] - if view != "export": - represented_users += list(view_as_user.represented_users.all()) - # redirect to non-public view if there is none because the results have not been published - if not evaluation.can_publish_rating_results and view == "public": - view = "full" + represented_users += list(view_as_user.represented_users.all()) - return view, view_as_user, represented_users, contributor_id + return view_general_results, view_contributor_results, view_as_user, represented_users, contributor_id def extract_evaluation_answer_data(request, evaluation): # TextAnswerExporter wants a dict from Question to tuple of contributor_name and string list (of the answers) - - view, view_as_user, represented_users, contributor_id = evaluation_detail_parse_get_parameters(request, evaluation) + ( + view_general_results, + view_contributor_results, + view_as_user, + represented_users, + contributor_id, + ) = evaluation_detail_parse_get_parameters(request, evaluation) evaluation_result = get_results(evaluation) filter_text_answers(evaluation_result) - remove_textanswers_that_the_user_must_not_see(evaluation_result, view_as_user, represented_users, view) + remove_textanswers_that_the_user_must_not_see( + evaluation_result, view_as_user, represented_users, view_general_results, view_contributor_results + ) results = TextAnswerExporter.InputData(evaluation_result.contribution_results) diff --git a/evap/staff/templates/staff_user_form.html b/evap/staff/templates/staff_user_form.html index 2256147a9..63c8501b8 100644 --- a/evap/staff/templates/staff_user_form.html +++ b/evap/staff/templates/staff_user_form.html @@ -101,7 +101,7 @@
    {% translate 'Export evaluation results' %}
    {% for evaluation in semester_evaluations.list %}
  • {% if evaluation|can_results_page_be_seen_by:form.instance %} - + {{ evaluation.full_name }} {% else %}