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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from Oct 18, 2023
Merged

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

merged 2 commits into from Oct 18, 2023

Conversation

charn
Copy link
Contributor

@charn charn commented Oct 13, 2023 •
edited

For privacy reasons the author name is not returned from the API if a filter (hearing, section, comment, created_by) hasn't been used.

For admin users the author name is always returned.

@charn charn requested a review from a team October 13, 2023 14:21
@charn charn force-pushed the KER-257-hide-comment-author-name branch from 3e25c4a to 00bd382 Compare October 13, 2023 14:27
nicobav
nicobav approved these changes Oct 16, 2023
View reviewed changes
Copy link
Contributor

@nicobav nicobav left a comment

Choose a reason for hiding this comment

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

Approving since code wise its a ok and does not introduce anything new to api.

The ticket specs allows one be more open minded with the interpretation and I myself started to ponder this. In the specs are mentioned "wide range of audience..." who shouldn't be seeing this field data, however anonymous user can see the field data with inputting query params. I'm wondering should we allow this field only for authenticated users? As with creator seems to be if i'm reading it correct.

@charn
Move created_by filter to the CommentFilterSet
124aab3 
Also make the filter return no results if user is not authenticated
or filter value is something else than created_by=me.

Refs KER-257
@charn charn force-pushed the KER-257-hide-comment-author-name branch 2 times, most recently from 566eb8d to ed0b194 Compare October 17, 2023 10:00
@charn
Copy link
Contributor Author

charn commented Oct 17, 2023

author_name should be available for the UI, for all users. Requiring user to be authenticated is too much as far as I can tell. That's why I haven't hidden the author_name in SectionCommentViewSet. SectionCommentViewSet could be used in the UI in some cases for fetching comments, but that view seems to be missing some filters that exist for CommentViewSet which is the root level comment endpoint. I guess one option would be to add those missing filters to that endpoint.

@charn
Hide author_name on comment endpoint for unfiltered request
d715b8a 
For privacy reasons the author name is not returned from the API
if a filter (hearing, section, comment, created_by) hasn't
been used.

For admin users the author name is always returned.

Refs KER-257
@charn charn force-pushed the KER-257-hide-comment-author-name branch from ed0b194 to d715b8a Compare October 18, 2023 07:07
@sonarcloud
Copy link

sonarcloud bot commented Oct 18, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 3 Security Hotspots
Code Smell A 4 Code Smells

98.8% 98.8% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@charn charn merged commit 65bcc8b into master Oct 18, 2023
2 of 3 checks passed
@charn charn deleted the KER-257-hide-comment-author-name branch October 18, 2023 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicobav nicobav nicobav approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@charn @nicobav