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

(Bugfix) Fixes only and exclude nested props #174

Merged
merged 3 commits into from
Aug 2, 2023

Conversation

regiscamimura
Copy link
Contributor

@regiscamimura regiscamimura commented Aug 2, 2023

What does this PR do?

Marshmallow pass only and exclude props through a normalizer that removes nested properties (defined in API and using the dot notation, f.e. profile.user.id). When we call the serializer, only and exclude are already changed, so I added a raw_only and raw_exclude to pass forward and Marshmallow can do its magic from there.

What Worf gif best describes this PR or how it makes you feel?

Checklist

  • This PR increases test coverage

@regiscamimura regiscamimura force-pushed the bug/only-and-exclude-cleanup branch 3 times, most recently from 1ec0a63 to c689975 Compare August 2, 2023 20:53
Copy link

@jciskey jciskey left a comment

Choose a reason for hiding this comment

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

In general, it looks good. One minor quibble about one of the tests, but nothing that absolutely must be changed.

response = client.get(f"/profiles/trimmed/{profile.pk}/")
result = response.json()
assert response.status_code == 200, result
assert "username" not in result
Copy link

Choose a reason for hiding this comment

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

Not a blocking change, but I'd argue that since this is testing the only keyword, we should be checking to ensure that the only key in the response is the id key, not just that a particular other key isn't present.

@regiscamimura regiscamimura merged commit 812246f into master Aug 2, 2023
8 checks passed
@regiscamimura regiscamimura deleted the bug/only-and-exclude-cleanup branch August 2, 2023 21:22
@github-actions github-actions bot mentioned this pull request Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants