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

Feature/region project viz api #688

Merged
merged 5 commits into from
Apr 29, 2020
Merged

Conversation

thenav56
Copy link
Member

Changes

  • Add API For Regional 3W
  • Add Secondary Tag COVID
  • Update Project Filter
  • Using Integer for Filter/Create/Update for enumfield for Project.

Checklist

Things that should succeed before merging.

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
Copy link
Collaborator

@batpad batpad left a 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 :-)

@batpad
Copy link
Collaborator

batpad commented Apr 29, 2020

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 EXPLAIN ANALYZE and indexes may help provide some obvious speed-ups. Of course, if you feel that's already done and you have a good sense of query performance here, no need to ticket separately.

@batpad
Copy link
Collaborator

batpad commented Apr 29, 2020

@thenav56 - can you ping @GregoryHorvath if this should be merged to staging?

@GergiH GergiH merged commit 4114d1e into develop Apr 29, 2020
@thenav56
Copy link
Member Author

@batpad Sure will look into EXPLAIN ANALYZE to make sure the query is optimized.
And I will make sure to add tests addressing this on the next PR.

@thenav56 thenav56 deleted the feature/region-project-viz-api branch April 29, 2020 07:02
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.

3 participants