feat: TNL-10136 tease course enrollment from student model #31344
Conversation
bszabo
force-pushed
the
bszabo/TNL-10136-reapply-course-enrollment-changes
branch
from
998fbe5 to
f846ea3
Compare
bszabo
force-pushed
the
bszabo/TNL-10136-reapply-course-enrollment-changes
branch
from
81e3192 to
b897833
Compare
|
@connorhaugh This PR is now ready for your review. It should have no substantive changes from the previous one you reviewed. That one had been approved but ran into dependency conflicts with master. The incremental changes here are to resolve those dependency changes. |
|
Ok @bszabo I will read through again to double-check, but for when we merge this, what kind of smoke tests should I make to make sure we haven't broken the user table? |
|
Good question, @connorhaugh . I haven't actually changed the model (tables are all the same), so in theory if it builds and passes unit tests, it should be OK. But in theory, theory and practice are the same; in practice, they're not. I need to explore a bit to answer your question. |
|
Practice rules, @connorhaugh ! The basic smoke test is to log in to LMS, go to the profile menu, and then save something like the maximum degree you attained. Then you log out, log back in, and see that what you selected is still there. Presently, works in the master branch. Doesn't work on the feature branch. I'll tag you here once I've got it working; there's nothing further for you to do with this PR until then. |
Reapply changes developed in bszabo/TNL-10136-student-course-enrollment branch to current master
inclusion in __init__ docstring
And import cross-references And wrap-around lines
blank lines in __init__ private method that needed to be public importlib and importlib-resources dependencies
Multiple commits squashed. Common theme was to resolve dependencies with master branch
bszabo
force-pushed
the
bszabo/TNL-10136-reapply-course-enrollment-changes
branch
from
b1dc6bd to
6e75002
Compare
Too many blank lines, missing import between student model files
|
@connorhaugh Once again ready for review. I rebased with master yesterday & got the code to where it passes CI checks and successfully runs on devstack. My suggested approach for testing is to run a few manual tests to verify proper end-to-end behavior with representative user model changes and representative course enrollment changes. I further propose a pair programming session where we visit the nature of the changes in the former For the manual student model tests, I recommend logging in to your devstack LMS and making edits to your account profile. Log out, log back in, and confirm that the changes you made are visible. For the manual course enrollment test, I recommend you enroll in a course in your localhost environment, log out, log back in, and confirm that you're still enrolled in that course. |
Remove block of 3 lines duplicated in last rebase exercise
bszabo
deleted the
bszabo/TNL-10136-reapply-course-enrollment-changes
branch
|
@connorhaugh Merge to stage has begun |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
1 similar comment
|
EdX Release Notice: This PR has been deployed to the production environment. |
Reapply changes developed in bszabo/TNL-10136-student-course-enrollment branch to current master
The intent here is to resolve dependency version conflicts only, relative to PR #31229