[ENG-6119] api changes for institution-users updates #10742
Merged
Conversation
This file contains 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
aaxelb
force-pushed
the
eng-6119
branch
5 times, most recently
from
6241ca4
to
46fdfb4
Compare
aaxelb
force-pushed
the
eng-6119
branch
3 times, most recently
from
5328629
to
012fd0b
Compare
aaxelb
changed the title
| operator: str, | ||
| value: str, | ||
| ) -> edsl.Search: | ||
| match operator: # operators from FilterMixin |
There was a problem hiding this comment.
Are we sure there's no [lt]/[lte]/[gt]/[gte] ? Thinking about if admins want custom csv files in certain date ranges they may want to filter by date. I'm not sure it's necessary from this view, not for the user's tab anyway.
There was a problem hiding this comment.
i'm open to adding range filters if/when we have a use for it (would be nice to make this base view a more generally reusable thing), but there's been no ask for it yet and i'm trying to embrace at least the good parts of agile :)
(if we decide to offer an institution-summary history and/or use this to improve the unstable/terrible /_/metrics/reports/ api, then yeah, range queries for sure)
- add skeletons for new institution-user metrics view and serializer - add `toggle_view_by_flag` util to ease feature-flagged views - use that util to toggle new/old views (with new/old serializers) - (nit) use django's `view_fn.view_class` instead of drf's `view_fn.cls`
- set INSTITUTIONAL_DASHBOARD_2024 flag inactive for existing institution-user metrics tests - remove `time.sleep` calls; use index refreshes instead (saves ~70 sec) - unskip tests -- "non-deterministic fails" may be fixed by refreshes?
- add base `ElasticsearchListView`
- use `elasticsearch_dsl.Search` as queryset-analogue for code reuse
- allows filtering (with 'eq' and 'ne' operators) following
`filterable_fields` on the serializer
- allows sorting, following `default_ordering` and `ordering_fields`
on the view
- allows pagination (with `page` and `page[size]` params)
- update "new" institution user metrics view with ElasticsearchListView
- add `MonthlyReport.most_recent_yearmonth`
- rename the `es` pytest marker to `es_metrics` (for clarity)
- update the effect of that marker:
- patch a prefix to each metric class's index and template names
- instead of deleting ALL indexes and index templates, delete only
those with the patched prefix
- update `api_tests.metrics.test_raw_metrics` to clean up after itself
and stop depending on the clobbering
aaxelb
merged commit 9384d76
into
CenterForOpenScience:feature/insti-dash-improv
6 checks passed
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.
Purpose
update api to serve institutional-user metrics based on the monthly report added in #10722
Changes
reusable things added:
toggle_view_by_flagutil to ease feature-flagged viewsElasticsearchListViewelasticsearch_dsl.Searchas queryset-analogue, for code reusefilterable_fieldson the serializer (usingapi.base.filters.FilterMixin)default_orderingandordering_fieldson the viewpageandpage[size]params), using default paginatorapi.base.serializers.YearmonthFieldMonthlyReport.most_recent_yearmonthclassmethodspecific to this project:
INSTITUTIONAL_DASHBOARD_2024Oldprefixtoggle_view_by_flagto toggle new/old views (with new/old serializers)ElasticsearchListView)incidental:
time.sleepcalls; use index refreshes instead (saves ~70 sec)espytest marker toes_metrics(for clarity)those with the patched prefix
api_tests.metrics.test_raw_metricsto clean up after itselfand stop depending on the clobbering
QA Notes
Please make verification statements inspired by your code and what your code touches.
What are the areas of risk?
Any concerns/considerations/questions that development raised?
Documentation
Side Effects
Ticket
ENG-6119