[ENG-6125] Implement Required Changes to Institution-Metric Summary API #10757
Conversation
Johnetordoff
force-pushed
the
api-for-summary-for-insti-dashboard
branch
2 times, most recently
from
6994fb4
to
8e220fc
Compare
Johnetordoff
force-pushed
the
api-for-summary-for-insti-dashboard
branch
from
8e220fc
to
bfab32a
Compare
Johnetordoff
force-pushed
the
api-for-summary-for-insti-dashboard
branch
from
138b418
to
03f201a
Compare
Johnetordoff
force-pushed
the
api-for-summary-for-insti-dashboard
branch
from
03f201a
to
53fb3d6
Compare
Johnetordoff
changed the title
api/institutions/views.py
Outdated
| class _NewInstitutionSummaryMetricsList(JSONAPIBaseView, generics.RetrieveAPIView, InstitutionMixin): | ||
| '''list view for institution-summary metrics |
There was a problem hiding this comment.
either keep it an actual list view (which should be easy with ElasticsearchListView, as your copy-paste included get_default_search) or update to stop calling it a list view (both in class-name and docstring)
this sort of inconsistency is actively (tho subtly) hostile to future contributors
api/institutions/serializers.py
Outdated
|
|
||
| Summary contains counts of | ||
| - Total users in the institution | ||
| - Total public project count for the institution | ||
| - Total private project count for the institution | ||
| - Total public registration count for the institution | ||
| - Total private registration count for the institution | ||
| - Total published preprint count for the institution | ||
|
|
There was a problem hiding this comment.
nit: i'd say this kind of info is not helpful in the docstring, since it duplicates info in the fields themselves and is unlikely to be kept in sync as fields are added/removed over time (again, thinking of confusion for future contributors) -- and here it's already out of sync (missing the last four fields)
api/institutions/views.py
Outdated
| ) | ||
|
|
||
|
|
||
| institution_summary_metrics_list_view = toggle_view_by_flag( |
There was a problem hiding this comment.
if you make it a list view, this is fine, but if it's a detail view (as implemented now) it shouldn't be called list_view
Johnetordoff
force-pushed
the
api-for-summary-for-insti-dashboard
branch
2 times, most recently
from
927f8c0
to
e319a15
Compare
Johnetordoff
force-pushed
the
api-for-summary-for-insti-dashboard
branch
from
e319a15
to
ad248ba
Compare
aaxelb
merged commit 44dd395
into
CenterForOpenScience:feature/insti-dash-improv
Purpose
This will hook up the ES back-end to our JSON API allowing fields to be sorted and filtered.
Changes
NewInstitutionSummaryMetricsSerializer_NewInstitutionSummaryMetricsListInstitutionMonthlySummaryReportQA 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
https://openscience.atlassian.net/browse/ENG-6125