-
Notifications
You must be signed in to change notification settings - Fork 14
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
Upsert rows in LMSCourse and LMSUser #6576
Conversation
@@ -0,0 +1,286 @@ | |||
"""Create the lms_course and lms_user tables.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be rebased out, here for easier testing.
@@ -243,6 +278,7 @@ def test_upsert_course_sets_canvas_sections_enabled_based_on_legacy_rows( | |||
type_=Any(), | |||
copied_from=None, | |||
) | |||
bulk_upsert.assert_called() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test deals with something that's irrelevant in the current LMSCourse model. Just asserting that we are still calling the upsert.
I don't think we should migrate setting/extra as it exists now on Grouping but rather create columns for the information there.
We can do that one bit at a time once we have the basic structure in place.
b45b7af
to
7541f48
Compare
231dc8d
to
8881f66
Compare
Before doing a migration to backfill LMSUser based on the information on User start making the code aware of both types while writing to the DB.
Before doing a migration to backfill LMSCourse based on the information on Course/Grouping start making the code aware of both types while writing to the DB.
7541f48
to
51acefa
Compare
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
For:
Testing