-
Notifications
You must be signed in to change notification settings - Fork 5
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
Feature/region project viz api #688
Conversation
Add EnumSerializer mixin to Project (Requires changes in go-frontend)
- Using enum value (instead of label) for Filter/POST/PUT for Project - Fix zero project query bug - Add project_country and reporting_ns multiple filter by ID
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.
@thenav56 this looks really good to me.
Some of the queries there, of course, are pretty wild. Can we ticket out writing tests for some of the more complex queries and aggregations there? Some basic tests for some of the more complex aggregations will probably really, really help, and hopefully we are able to get to them.
But this looks really good to me, learned a few new things about the ORM :-)
Also @thenav56 - not sure how much you've already done, but it would be great to ensure we've done some inspecting and tuning of the queries generated here. Again, if you think there is likely scope to improve query performance, add indexes, etc. please create a ticket - I don't think it should hold us back, but it seems likely that spending some time with |
@thenav56 - can you ping @GregoryHorvath if this should be merged to staging? |
@batpad Sure will look into |
Changes
Checklist
Things that should succeed before merging.