-
Notifications
You must be signed in to change notification settings - Fork 37
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
Enrollment data handling fixes #449
Merged
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
johnbaldwin
force-pushed
the
john/enrollment-data-fixes
branch
from
March 11, 2022 19:03
e3faba1
to
c2796c6
Compare
* Problem was that `update_enrollment_data_for_course` returns a list of tuples. Each tuple is `(object, created)`, where `object` is the EnrollmentData record and `created` tells if the record was created * Added logging to the backfill task * Updated tests * Added `get_from_enrollment` method to `EnrollmentDataManager`
johnbaldwin
force-pushed
the
john/enrollment-data-fixes
branch
from
March 14, 2022 21:04
c2796c6
to
ca245db
Compare
New method `figures.course.Course.enrollments_with_student_modules` This method finds `CourseEnrollment` records for a course for enrollments that have `StudentModule` records in the course. This functionality helps simplify finding out of date EnrollmentData records.
Was missing the test case when there were no student modules in the course. We want to make sure our "empty set" handling is correct
What's included here is a generator to yield CourseEnrollment records that need to have their EnrollmentData records updated If we install Figures on a site that's already been running or we miss a day or more in processing, then the daily processing will not pick up on all "stale enrollments", enrollments that Figures EnrollmentData record updated. This is because Figures daily metrics jobs only look at "yesterday" in order to work around the limitations of StudentModule. The real logic is in `figures.enrollment.is_enrollment_data_out_of_date` (Which is still pretty simple)
Now it looks for stale course enrollments and for each stale course enrollment found, updates that enrollment's metrics. Prior to this commit, it was just calling the normal daily enrolment data update function. * Updated unit tests
johnbaldwin
requested review from
OmarIthawi,
melvinsoft,
shadinaif,
estherjsuh and
bryanlandia
as code owners
March 16, 2022 02:53
* Add ability to load course ids from a file * Add test coverage
johnbaldwin
force-pushed
the
john/enrollment-data-fixes
branch
from
March 16, 2022 02:56
cdeb6a2
to
adb67fb
Compare
OmarIthawi
approved these changes
Mar 16, 2022
Thanks @OmarIthawi ! |
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.
This PR has fixes for the new enrollment data processing
Fix backfill_enrollment_data_for_course task.Commit e3faba1
update_enrollment_data_for_course
returns a list of tuples. Each tuple is(object, created)
, whereobject
is the EnrollmentData record andcreated
tells if the record was createdFix class for Python27 Ginkgo. Commit bf83408
object
, since we still need to support py27.Adds enrollments filter method to Figures Course class. Commit bd03842
figures.course.Course.enrollments_with_student_modules
CourseEnrollment
records for a course for enrollments that haveStudentModule
records in the course. This functionality helps simplify finding out of date EnrollmentData records.Add test case to tests/test_course.py . Commit: 2566818
Add stale course enrollment handling to pipeline. Commit ded1349
figures.enrollment.is_enrollment_data_out_of_date
(Which is still pretty simple)Fixed figures.tasks.backfill_enrollment_data_for_course. Commit 3511f2b
Update backfill enrollment management command. Commit cdeb6a2
One last thing to check: The student module GTE check in
figures.enrollment.student_modules_for_enrollment_after_date