Bszabo/tnl 10136 student course enrollment #31229
Conversation
bszabo
force-pushed
the
bszabo/TNL-10136-student-course-enrollment
branch
from
e718de4 to
eec0c6a
Compare
| from openedx.core.lib.api.test_utils import ApiTestCase | ||
| from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerUserFactory | ||
| from common.djangoapps.student.models import LoginFailures | ||
| from common.djangoapps.util.password_policy_validators import DEFAULT_MAX_PASSWORD_LENGTH | ||
| from openedx_events.tests.utils import OpenEdxEventsTestMixin # lint-amnesty, pylint: disable=wrong-import-order |
There was a problem hiding this comment.
Why do we want this imported at this line in particular?
There was a problem hiding this comment.
I used a PyCharm "Optimize Imports" tool action, which, among other things, arranges imports alphabetically. It would appear that the underscore in "openedx_events" places this import in lower alphabetic order relative to the dot in "openedex.features"
|
Great use of atomic commits here it makes reading the story of this revision much easier. |
|
In side channel slack conversation, Alexander confirmed that none of these changes broke the edx-enterprise build, and that the extraction of the course enrollment from the student model looks largely correct (suitable first approximation). |
No class partitioning
fix imports and mocking to match new home
bszabo
force-pushed
the
bszabo/TNL-10136-student-course-enrollment
branch
from
eec0c6a to
b497a7d
Compare
See details in the related PR Co-authored-by: Omar Al-Ithawi <[email protected]> Co-authored-by: Shadi Naif <[email protected]>
No class partitioning
fix imports and mocking to match new home
All tests now pass. Lots of temporary diagnostic logging statements in place; still need removal feat: TNL-10136 Remove diagnostic print statements Statements were added as part of fixing broken unit tests
|
@bszabo If you could hi light any changes required by the merge, I would be happy to give this a second thumb upon review of those changes. |
|
Hi @bszabo should this PR be closed? |
|
Closing since there hasn't been a response in a while. |
This is an incremental step towards teasing apart a course enrollment model out of the existing student model in
edx-platform. These changes are intended to be non-breaking. Please fail the PR if that objective goes unmet.Model code had previously all resided in a single
student/models.pymodule. Now it resides in two modules, one for student modeling and one for course enrollment modeling.Database tables remain as is.
Importing of student and course enrollment models remain as is.