Skip to content

Commit

Permalink
Implemented requested changes
Browse files Browse the repository at this point in the history 
implemented smaller changes that were quested in PR.
- removed comments
- itereated over enums directly
- changed the name in this itereation from con_view to contributor_view (same for gen_view)
- correclty formatted HTML comment
- made things that should be in one line to be in one line
- implemented variables for reused expected answers (on method and instance level) in test_views.py

Co-authored-by: Yannik <[email protected]>
  • Loading branch information
@jooooosef @ybrnr
jooooosef and ybrnr committed Dec 16, 2024
1 parent 49fb0b5 commit 49d1c67
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 57 deletions.
2 changes: 1 addition & 1 deletion evap/results/templates/results_evaluation_detail.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
{% if evaluation.state != evaluation.State.PUBLISHED %}
<div class="alert alert-warning">{% translate 'This is a preview. The results have not been published yet.' %}</div>
{% endif %}
<!--manager is automatically a reviewer (so this are all groups that can see the buttons)-->
{# 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 %}
<div class="alert alert-info d-print-none">
Expand Down
76 changes: 31 additions & 45 deletions evap/results/tests/test_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,9 +485,7 @@ 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(
self,
):
def test_default_view(self):
cache_results(self.evaluation)

page_without_get_parameter = self.app.get(self.url, user=self.manager)
Expand Down Expand Up @@ -756,38 +754,41 @@ class TestResultsTextanswerVisibility(WebTest):
".responsible_contributor_additional_orig_deleted.",
]

contributor_views = [ViewContributorResults.FULL, ViewContributorResults.RATINGS, ViewContributorResults.PERSONAL]
general_views = [ViewGeneralResults.FULL, ViewGeneralResults.RATINGS]
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 helper_test_general(self, user, view_general_results, textanswers_in):
def helper_test_general(self, user, view_general_results, expected_visible_textanswers):

textanswers_not_in = list(set(self.general_textanswers) - set(textanswers_in))
for con_view in self.contributor_views:
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={con_view.value}",
f"/results/semester/1/evaluation/1?view_general_results={view_general_results.value}&view_contributor_results={contributor_view.value}",
user=user,
)

for answer in textanswers_in:
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, textanswers_in):
def helper_test_contributor(self, user, view_contributor_results, expected_visible_textanswers):

textanswers_not_in = list(set(self.contributor_textanswers) - set(textanswers_in))
for gen_view in self.general_views:
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={gen_view.value}",
f"/results/semester/1/evaluation/1?view_contributor_results={view_contributor_results.value}&view_general_results={general_view.value}",
user=user,
)

for answer in textanswers_in:
for answer in expected_visible_textanswers:
self.assertIn(answer, page)
for answer in textanswers_not_in:
self.assertNotIn(answer, page)
Expand All @@ -799,7 +800,7 @@ def test_manager(self):
self.helper_test_general(
user,
ViewGeneralResults.FULL,
[".general_orig_published.", ".general_additional_orig_published.", ".general_changed_published."],
self.standard_general_textanswers,
)
self.helper_test_general(user, ViewGeneralResults.RATINGS, [])

Expand Down Expand Up @@ -844,11 +845,7 @@ def test_responsible(self):
self.helper_test_general(
user,
ViewGeneralResults.FULL,
[
".general_orig_published.",
".general_changed_published.",
".general_additional_orig_published.",
],
self.standard_general_textanswers,
)
self.helper_test_general(user, ViewGeneralResults.RATINGS, [])

Expand All @@ -859,38 +856,30 @@ def test_responsible(self):
def test_responsible_contributor(self):

user = "[email protected]"
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,
[
".general_orig_published.",
".general_changed_published.",
".general_additional_orig_published.",
],
self.standard_general_textanswers,
)
self.helper_test_general(user, ViewGeneralResults.RATINGS, [])

self.helper_test_contributor(
user,
ViewContributorResults.FULL,
[
".responsible_contributor_orig_published.",
".responsible_contributor_changed_published.",
".responsible_contributor_orig_private.",
".responsible_contributor_additional_orig_published.",
],
contributor_textanswers_responsible_contributor_can_see,
)
self.helper_test_contributor(user, ViewContributorResults.RATINGS, [])
self.helper_test_contributor(
user,
ViewContributorResults.PERSONAL,
[
".responsible_contributor_orig_published.",
".responsible_contributor_changed_published.",
".responsible_contributor_orig_private.",
".responsible_contributor_additional_orig_published.",
],
contributor_textanswers_responsible_contributor_can_see,
)

def test_contributor_general_textanswers(self):
Expand All @@ -899,11 +888,7 @@ def test_contributor_general_textanswers(self):
self.helper_test_general(
user,
ViewGeneralResults.FULL,
[
".general_orig_published.",
".general_additional_orig_published.",
".general_changed_published.",
],
self.standard_general_textanswers,
)
self.helper_test_general(user, ViewGeneralResults.RATINGS, [])

Expand All @@ -913,16 +898,17 @@ def test_contributor_general_textanswers(self):

def test_contributor(self):
user = "[email protected]"
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_orig_published.", ".contributor_orig_private."]
user, ViewContributorResults.FULL, contributor_textanswers_contributor_can_see
)
self.helper_test_contributor(user, ViewContributorResults.RATINGS, [])
self.helper_test_contributor(
user, ViewContributorResults.PERSONAL, [".contributor_orig_published.", ".contributor_orig_private."]
user, ViewContributorResults.PERSONAL, contributor_textanswers_contributor_can_see
)


Expand Down
8 changes: 1 addition & 7 deletions evap/results/tools.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
class ViewGeneralResults(Enum):
@property
def do_not_call_in_templates(self):
return False
return True

Check warning on line 43 in evap/results/tools.py

View check run for this annotation

Codecov / codecov/patch

evap/results/tools.py#L43 

Added line #L43 was not covered by tests

FULL = "full"
RATINGS = "ratings"
Expand Down Expand Up @@ -493,7 +493,6 @@ def textanswers_visible_to(contribution):
return TextAnswerVisibility(visible_by_contribution=sorted_contributors, visible_by_delegation_count=num_delegates)


# Filter textanswers based on user selection
def can_textanswer_be_seen_by( # noqa: PLR0911,PLR0912
user: UserProfile,
represented_users: list[UserProfile],
Expand All @@ -507,10 +506,7 @@ def can_textanswer_be_seen_by( # noqa: PLR0911,PLR0912
# 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.contribution.is_general:
if view_general_results == ViewGeneralResults.RATINGS:
return False
if view_general_results == ViewGeneralResults.FULL:
# reviewer can see everything
if user.is_reviewer:
return True

Expand All @@ -527,8 +523,6 @@ def can_textanswer_be_seen_by( # noqa: PLR0911,PLR0912
for user in textanswer.contribution.evaluation.course.responsibles.all()
):
return True
else:
return False
else:
if view_contributor_results == ViewContributorResults.RATINGS:
return False
Expand Down
5 changes: 1 addition & 4 deletions evap/results/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,10 +433,7 @@ def evaluation_detail_parse_get_parameters(request, evaluation):

try:
view_contributor_results = ViewContributorResults(
request.GET.get(
"view_contributor_results",
ViewContributorResults.FULL,
)
request.GET.get("view_contributor_results", ViewContributorResults.FULL)
)
except ValueError as e:
raise BadRequest from e
Expand Down

0 comments on commit 49d1c67

Please sign in to comment.