Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7k
Test Serializer exclude for declared fields#5599
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
xordoquy commentedNov 15, 2017
rpkilby commentedNov 15, 2017
@xordoquy - agreed, I just whipped it up given how simple of a change it was. I've added another commit that raises an |
carltongibson commentedNov 16, 2017
For me, an assert here is fine. (Even “good” perhaps 🙂) I’m -1 on supporting the usage from#5596, as per discussion there. It’s my birthday 🍰 today so I’ll leave you folks to decide what to do. |
3578ba6 to4e7e728Compare
carltongibson left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
OK, this is fine. I'm sure it will help someone. Let's have it.
virusdefender commentedDec 1, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
It's great! I have the same issue. When you redeclared some field, it won't be filtered. |
Unpinned a number of requirements so they would be upgraded as part of`make upgrade` as recommended by OEP-18. In particular the edx-enterprise-datadjango app was not pinning edx-drf-extensions which resulted in 2 differentversions of DRF in the requirements (one for enterprise-data and one for thisIDA).Upgrading DRF resulting in an assertion failure that was added in DRF 3.7.4```Cannot both declare the field 'created' and include it in the CourseProgramMetadataSerializer'exclude' option. Remove the field or, if inherited from a parent serializer, disable with `created = None`.```encode/django-rest-framework#5599Because the created date was being excluded, it is not needed in the CourseProgramMetadataSerializer andhas been removed.
* Test current behavior of exclude+declared field* Assert declared fields are not present in exclude
I'm not particularly inclined towards the change in behavior, but it was fairly trivial to implement.
Thoughts@xordoquy,@carltongibson?