[BUGFIX] ?exclude query parameter now works on nested fields#919
Open
calumbell wants to merge 3 commits into
Open
[BUGFIX] ?exclude query parameter now works on nested fields#919calumbell wants to merge 3 commits into
?exclude query parameter now works on nested fields#919calumbell wants to merge 3 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes an issue with the
ExcludeFieldsMixinwhere was not able to remove nested fields from API responses.The specific use case that PR supports can be found in the following issue: open5e/open5e#865 - on the
/v2/creaturesendpoint there is no way currently of excluding theattackssub-field of theactionsfield. This can now be achieved by:Not the use of the double-underscore operator mirrors the default DRF method of whitelisting nested fields via the
?fieldsquery parameter.The code is pretty knotty, requires a bit of parsing out. For this PR I have emphasised readability and gone slightly more verbose then I typically would. It think it serves the feature best.
ExcludeFieldsMixinnow takes captures all of the query parameters from a request which end with"__excludes"along with exact matches with"exclude"and uses them as keys in a dict where the value for each is a list of fields to remove from that sub-field. The serializer is then recursively traversed and fields marked for exclusion are pruned.During testing I noticed some weird interactions with the
EagerLoadingMixin, slow response times and an increase in several orders of magnitude to the number of SQL queries made. Changes were made so that it played nicely with theExcludeFieldsMixin. After the fact I thought that these changes had made the mixin difficult to read so I opted to refactor out some of the thorny one-liners into their own methods and generally improve the documentation.Related Issue
Closes #918
How was this tested
pytesttests are passing on local