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

KER-257 | Hide author_name on comment endpoint for unfiltered request #452

Merged
merged 2 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
103 changes: 81 additions & 22 deletions democracy/tests/integrationtest/test_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ def test_56_add_comment_to_section_without_authentication_with_reply_to(
def test_56_add_comment_to_section_requiring_strong_auth_without_authentication(
api_client, strong_auth_hearing, get_comments_url_and_data
):

section = strong_auth_hearing.sections.first()
url, data = get_comments_url_and_data(strong_auth_hearing, section)
response = api_client.post(url, data=data)
Expand All @@ -133,7 +132,6 @@ def test_56_add_comment_to_section_requiring_strong_auth_without_authentication(
def test_56_add_comment_to_section_requiring_strong_auth_with_weak_auth(
john_doe_api_client, strong_auth_hearing, get_comments_url_and_data
):

section = strong_auth_hearing.sections.first()
url, data = get_comments_url_and_data(strong_auth_hearing, section)
response = john_doe_api_client.post(url, data=data)
Expand All @@ -145,7 +143,6 @@ def test_56_add_comment_to_section_requiring_strong_auth_with_weak_auth(
def test_56_add_comment_to_section_requiring_strong_auth_with_strong_auth(
stark_doe_api_client, strong_auth_hearing, get_comments_url_and_data
):

section = strong_auth_hearing.sections.first()
url, data = get_comments_url_and_data(strong_auth_hearing, section)
response = stark_doe_api_client.post(url, data=data)
Expand All @@ -157,7 +154,6 @@ def test_56_add_comment_to_section_requiring_strong_auth_with_strong_auth(
def test_56_add_comment_to_section_requiring_strong_auth_with_organization(
john_smith_api_client, strong_auth_hearing, get_comments_url_and_data
):

section = strong_auth_hearing.sections.first()
url, data = get_comments_url_and_data(strong_auth_hearing, section)
response = john_smith_api_client.post(url, data=data)
Expand Down Expand Up @@ -241,7 +237,7 @@ def test_56_add_comment_to_section(john_doe_api_client, default_hearing, get_com
assert len(new_comment_list) == len(old_comment_list) + 1
new_comment = [c for c in new_comment_list if c["id"] == data["id"]][0]
assert_common_keys_equal(new_comment, comment_data)
assert new_comment["is_registered"] == True
assert new_comment["is_registered"] is True
assert new_comment["author_name"] is None


Expand Down Expand Up @@ -284,7 +280,7 @@ def test_56_add_comment_to_comment(john_doe_api_client, default_hearing, get_com
# Comment data now contains section id too
comment_data["section"] = section.pk
assert_common_keys_equal(new_comment, comment_data)
assert new_comment["is_registered"] == True
assert new_comment["is_registered"] is True
assert new_comment["author_name"] is None


Expand Down Expand Up @@ -456,14 +452,13 @@ def test_56_add_comment_with_label_to_section(john_doe_api_client, default_heari
assert len(new_comment_list) == len(old_comment_list) + 1
new_comment = [c for c in new_comment_list if c["id"] == data["id"]][0]
assert_common_keys_equal(new_comment, comment_data)
assert new_comment["is_registered"] == True
assert new_comment["is_registered"] is True
assert new_comment["label"] == {"id": 1, "label": {default_lang_code: "The Label"}}
assert new_comment["author_name"] is None


@pytest.mark.django_db
def test_add_empty_comment_with_label(john_doe_api_client, default_hearing, get_comments_url_and_data):

label_one = Label(id=1, label="The Label")
label_one.save()
section = default_hearing.sections.first()
Expand Down Expand Up @@ -494,7 +489,7 @@ def test_add_empty_comment_with_label(john_doe_api_client, default_hearing, get_
assert len(new_comment_list) == len(old_comment_list) + 1
new_comment = [c for c in new_comment_list if c["id"] == data["id"]][0]
assert_common_keys_equal(new_comment, comment_data)
assert new_comment["is_registered"] == True
assert new_comment["is_registered"] is True
assert new_comment["label"] == {"id": 1, "label": {default_lang_code: "The Label"}}
assert new_comment["author_name"] is None
assert new_comment["content"] == ""
Expand Down Expand Up @@ -533,13 +528,12 @@ def test_56_add_empty_comment_with_geojson(
assert len(new_comment_list) == len(old_comment_list) + 1
new_comment = [c for c in new_comment_list if c["id"] == data["id"]][0]
assert_common_keys_equal(new_comment, comment_data)
assert new_comment["is_registered"] == True
assert new_comment["is_registered"] is True
assert new_comment["author_name"] is None


@pytest.mark.django_db
def test_56_add_comment_with_inexistant_label(john_doe_api_client, default_hearing, get_comments_url_and_data):

section = default_hearing.sections.first()
url, data = get_comments_url_and_data(default_hearing, section)
old_comment_list = get_data_from_response(john_doe_api_client.get(url))
Expand All @@ -557,7 +551,6 @@ def test_56_add_comment_with_inexistant_label(john_doe_api_client, default_heari

@pytest.mark.django_db
def test_56_add_comment_with_no_label_id(john_doe_api_client, default_hearing, get_comments_url_and_data):

label_one = Label(id=1, label="The Label")
label_one.save()
section = default_hearing.sections.first()
Expand All @@ -579,7 +572,6 @@ def test_56_add_comment_with_no_label_id(john_doe_api_client, default_hearing, g

@pytest.mark.django_db
def test_56_add_comment_with_images_to_section(john_doe_api_client, default_hearing, get_comments_url_and_data):

section = default_hearing.sections.first()
url, data = get_comments_url_and_data(default_hearing, section)
old_comment_list = get_data_from_response(john_doe_api_client.get(url))
Expand Down Expand Up @@ -617,15 +609,14 @@ def test_56_add_comment_with_images_to_section(john_doe_api_client, default_hear
new_comment = [c for c in new_comment_list if c["id"] == data["id"]][0]
new_comment["images"][0].pop("id")
assert_common_keys_equal(new_comment, response_data)
assert new_comment["is_registered"] == True
assert new_comment["is_registered"] is True
assert new_comment["author_name"] is None


@pytest.mark.django_db
def test_56_add_comment_with_empty_image_field_to_section(
john_doe_api_client, default_hearing, get_comments_url_and_data
):

section = default_hearing.sections.first()
url, data = get_comments_url_and_data(default_hearing, section)
old_comment_list = get_data_from_response(john_doe_api_client.get(url))
Expand All @@ -638,17 +629,16 @@ def test_56_add_comment_with_empty_image_field_to_section(
# allow null images field
comment_data = get_comment_data(section=section.pk, images=None)
response = john_doe_api_client.post(url, data=comment_data, format="json")
data = get_data_from_response(response, status_code=201)
get_data_from_response(response, status_code=201)

# allow empty images array
comment_data = get_comment_data(section=section.pk, images=[])
response = john_doe_api_client.post(url, data=comment_data, format="json")
data = get_data_from_response(response, status_code=201)
get_data_from_response(response, status_code=201)


@pytest.mark.django_db
def test_56_add_comment_with_invalid_content_as_images(john_doe_api_client, default_hearing, get_comments_url_and_data):

section = default_hearing.sections.first()
url, data = get_comments_url_and_data(default_hearing, section)

Expand All @@ -669,7 +659,6 @@ def test_56_add_comment_with_invalid_content_as_images(john_doe_api_client, defa

@pytest.mark.django_db
def test_56_add_comment_with_image_too_big(john_doe_api_client, default_hearing, get_comments_url_and_data):

section = default_hearing.sections.first()
url, data = get_comments_url_and_data(default_hearing, section)

Expand Down Expand Up @@ -955,7 +944,7 @@ def test_comment_delete_versioning(john_doe_api_client, default_hearing, lookup_
comment = SectionComment.objects.deleted().get(pk=comment_id)
versions = Version.objects.get_for_object(comment)
assert len(versions) == 2
assert versions[0].field_dict["deleted"] == True
assert versions[0].field_dict["deleted"] is True


@pytest.mark.django_db
Expand Down Expand Up @@ -1437,6 +1426,16 @@ def test_get_comments_created_by_user(john_doe_api_client, jane_doe_api_client,
assert data["results"][0]["content"] == "Jane created this comment"


@pytest.mark.parametrize("query_params,expected", [({"created_by": "me"}, 0), (None, 9)])
@pytest.mark.django_db
def test_get_comments_created_by_user__unauthenticated(query_params, expected, api_client, default_hearing):
"""Unauthenticated users get no results with created_by filter."""
url = reverse("comment-list")
response = api_client.get(url, query_params)

assert len(response.data["results"]) == expected


@pytest.mark.django_db
def test_deleted_comments_data_returned(john_doe_api_client, default_hearing):
"""Deleted comment is returned, with contents censored"""
Expand Down Expand Up @@ -1547,7 +1546,6 @@ def test_get_section_comment_edit_delete_rights(

@pytest.mark.django_db
def test_delete_comment_comment(john_doe_api_client, hearing_with_comments_on_comments):

section = hearing_with_comments_on_comments.sections.first()
comment = section.comments.filter(comment=None).first()
comment_to_delete = comment.comments.first()
Expand All @@ -1565,11 +1563,72 @@ def test_delete_comment_comment(john_doe_api_client, hearing_with_comments_on_co
assert len(data["results"][0]["comments"]) == 2


@pytest.mark.parametrize(
"user_type,query_filter,expect_author_name",
[
("admin", None, True),
("regular", None, False),
("anonymous", "section", True),
("anonymous", "hearing", True),
("anonymous", "comment", True),
("anonymous", None, False),
],
)
@pytest.mark.django_db
def test_comment_author_name_hidden_from_unfiltered_requests(
user_type,
query_filter,
expect_author_name,
hearing_without_comments,
api_client,
john_doe_api_client,
steve_staff_api_client,
):
url = reverse("comment-list")
if user_type == "admin":
client = steve_staff_api_client
elif user_type == "regular":
client = john_doe_api_client
else:
client = api_client
section = hearing_without_comments.sections.first()
comment = SectionCommentFactory(section=section, author_name="Name")
SectionCommentFactory(section=section, comment=comment, author_name="Name")
possible_params = {
"hearing": section.hearing_id,
"section": section.id,
"comment": comment.id,
}
query_params = {query_filter: possible_params[query_filter]} if query_filter else None

response = client.get(url, query_params)

expected_value = "Name" if expect_author_name else None
assert response.data["results"][0]["author_name"] == expected_value


@pytest.mark.django_db
def test_comment_author_name_hidden_from_unfiltered_requests__created_by(
hearing_without_comments, api_client, john_doe_api_client
):
"""Regular authenticated user can see author_name with created_by filter."""
url = reverse("comment-list")
client = john_doe_api_client
user = john_doe_api_client.user
section = hearing_without_comments.sections.first()
comment = SectionCommentFactory(section=section, created_by=user, author_name="Name")
SectionCommentFactory(section=section, comment=comment, author_name="Name")

response = client.get(url, {"created_by": "me"})

assert response.data["results"][0]["author_name"] == "Name"


@pytest.mark.django_db
def test_section_comment_num_queries(django_assert_num_queries, john_doe_api_client, hearing_with_comments_on_comments):
url = reverse("comment-list")

with django_assert_num_queries(9):
with django_assert_num_queries(10):
response = john_doe_api_client.get(url)
get_data_from_response(response, 200)

Expand Down
42 changes: 38 additions & 4 deletions democracy/views/section_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,12 @@ def get_hearing_data(self, obj):

return False

def to_representation(self, instance):
ret = super().to_representation(instance)
if self.context.get("remove_author_name"):
ret["author_name"] = None
return ret


class RootSectionCommentCreateUpdateSerializer(SectionCommentCreateUpdateSerializer):
"""
Expand All @@ -444,6 +450,7 @@ class CommentFilterSet(django_filters.rest_framework.FilterSet):
created_at__gt = django_filters.rest_framework.IsoDateTimeFilter(field_name="created_at", lookup_expr="gt")
comment = django_filters.ModelChoiceFilter(queryset=SectionComment.objects.everything())
section = django_filters.CharFilter(field_name="section__id")
created_by = django_filters.CharFilter(method="filter_created_by")

class Meta:
model = SectionComment
Expand All @@ -458,6 +465,12 @@ class Meta:
"pinned",
]

def filter_created_by(self, queryset, name, value: str):
if value.lower() == "me" and not self.request.user.is_anonymous:
return queryset.filter(created_by_id=self.request.user.id)

return queryset.none()


# root level SectionComment endpoint
class CommentViewSet(SectionCommentViewSet):
Expand All @@ -466,17 +479,38 @@ class CommentViewSet(SectionCommentViewSet):
pagination_class = DefaultLimitPagination
filterset_class = CommentFilterSet

@property
def _is_filtered(self):
"""Is the queryset filtered enough to show author_names."""
acceptable_filters = ["section", "hearing", "comment", "created_by"]
filterset = self.filterset_class(data=self.request.query_params, request=self.request)
if filterset.is_valid():
for af in acceptable_filters:
if filterset.form.cleaned_data[af]:
return True
return False

def get_serializer_context(self):
"""Author name will be removed for privacy reasons if fetching unfiltered comments."""
context = super().get_serializer_context()

if (
self.action == "list"
and not self._is_filtered
and not bool(
hasattr(self.request.user, "get_default_organization") and self.request.user.get_default_organization()
)
):
context["remove_author_name"] = True
return context

def get_queryset(self):
"""Returns all root-level comments, including deleted ones"""

queryset = self.model.objects.everything()
queryset = self.apply_select_and_prefetch(queryset)

queryset = filter_by_hearing_visible(queryset, self.request, "section__hearing")
created_by = self.request.query_params.get("created_by", None)
if created_by is not None and not self.request.user.is_anonymous:
if created_by.lower() == "me":
queryset = queryset.filter(created_by_id=self.request.user.id)

user = self._get_user_from_request_or_context()
if user.is_authenticated and user.is_superuser:
Expand Down