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

New endpoint for enrollment metrics - provide data captured in LCGM #233

Merged
merged 7 commits into from
Jul 15, 2020

Conversation

johnbaldwin
Copy link
Contributor

@johnbaldwin johnbaldwin commented Jul 1, 2020

Added new endpoint to retrieve enrollment metrics.

The new base endpoint is /figures/api/enrollment-metrics/which provides an API view for LearnerCourseGradeMetrics model. The LearnerCourseGradeMetrics model is where we capture learner progress and completion data. These data originate from CourseGrade and populated during the daily pipeline jobs with calls to CourseGradeFactory().read(...)

  • Updated LearnerCourseGradeMetricsManager model manager to retrieve "course completed" data.

  • Added broad query parameter filtering capability to this endpoint. All fields should be filterable except for site. Also added two list filters user_ids= and course_ids= which take comma delimited lists of user ids and course ids respectively. Added two filters only_completed and exclude_completed that take a True. Yes, Capitalize the first letter, this is an artifact of django-filter. We can improve this in the future, for now, just want to have the functionality in so we can start working with it in the front end

  • Added experimental raw SQL queries to the LCGM model manager. These are not called by the endpoint and here to test retrieving the most recent LCGM record for a set of users

Fixes

  • removed "LIMIT 1" query from LCGM model manager most_recent_for_learner_course method
  • Added pylint: disable=E1101 to stop pylint failures in recognizing that filter is a method of the queryset (PyLint thinks the object is a list at these places).

We can retrieve the core "completed" metrics data now, but we're likely going to want to rework and/or improve the endpoint after we cleanup LCGM and add indexes to the course_id and date_for fields. We may also want to convert LCGM completed proper to a table column so we can index on that

Tests are mostly there, but need more coverage. First I wanted to get this out to review

@codecov-commenter
Copy link

codecov-commenter commented Jul 2, 2020

Codecov Report

Merging #233 into master will increase coverage by 0.14%.
The diff coverage is 94.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #233      +/-   ##
==========================================
+ Coverage   91.36%   91.50%   +0.14%     
==========================================
  Files          40       40              
  Lines        1968     2061      +93     
==========================================
+ Hits         1798     1886      +88     
- Misses        170      175       +5     
Impacted Files Coverage Δ
figures/models.py 95.36% <85.18%> (-1.67%) ⬇️
figures/filters.py 98.07% <96.42%> (-0.61%) ⬇️
figures/serializers.py 94.59% <100.00%> (+0.21%) ⬆️
figures/urls.py 100.00% <100.00%> (ø)
figures/views.py 92.39% <100.00%> (+0.65%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5e41d5...a569593. Read the comment docs.

@johnbaldwin johnbaldwin changed the title Model changes - Adds getting completed stats from LearnerCourseGradeMetrics New endpoint for enrollment metrics - provide data captured in LCGM Jul 2, 2020
…completed"

- Added "completed" property to LearnerCourseGradeMerics
- Added model manager methods to retrieve completed records and
identities
- Added experimental query to LCGM to retrieve most recent records for a
course
- Updates model and manager unit tests
- Fixed pylint errors
@johnbaldwin johnbaldwin force-pushed the john/enrollment-completed branch from f4d507a to 21eec6b Compare July 6, 2020 03:18
This endpoint retrieves data from Figures LearnerCourseGradeMetrics
model
- Added new viewset for endpoint `/figures/api/enrollment-metrics/`
- Added serializers and filters to support the new endpoint
- Added view tests
Quick hack to populate LCGM model for local development
@johnbaldwin johnbaldwin force-pushed the john/enrollment-completed branch from 21eec6b to 12eca82 Compare July 6, 2020 04:03
@johnbaldwin johnbaldwin marked this pull request as ready for review July 6, 2020 04:04
@johnbaldwin
Copy link
Contributor Author

johnbaldwin commented Jul 13, 2020

I'm working on improving coverage. Looks like the big spots are the new view, filter methods, and a couple of spots in the model update. There was an incomplete view test for EnrollmentMetricsViewSet

@johnbaldwin johnbaldwin force-pushed the john/enrollment-completed branch from c533043 to d611dd8 Compare July 15, 2020 02:51
@johnbaldwin
Copy link
Contributor Author

Coverage dropped into unhappy territory after merging from master. Working on boosting it above the waterline

This makes the function useable by checking that the response data is
not paginated as well as is paginated
Added test coverage for when viewset does not pagingate. This can be a
case where the existing class paginagor is removed or set to None for
the class being inherited.

Instead of doing this, we could have just assumed that the results are
always paginated in the EnrollmentMetricsViewSet completed and
comletede_ids actions. However, the example I learned from did the conditional
check for paginated results.
So this is a bit of cheap defensive programming to allow the viewset
method to paginate or not and have test coverage for this
@johnbaldwin
Copy link
Contributor Author

Added test coverage for cases where paginated results not retrieved for the "completed" and "completed_ids" actions

@johnbaldwin johnbaldwin merged commit 04b6de7 into master Jul 15, 2020
@johnbaldwin johnbaldwin deleted the john/enrollment-completed branch July 15, 2020 16:52
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