-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fix: course outline not found issue for ccx courses #36128
base: master
Are you sure you want to change the base?
Conversation
Thanks for the pull request, @Anas12091101! This repository is currently maintained by @openedx/wg-maintenance-edx-platform. Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.
|
5713d56
to
3321319
Compare
4fdfece
to
810b157
Compare
810b157
to
e25fca8
Compare
Is all this code behind the feature flag, |
MODULESTORE_FIELD_OVERRIDE_PROVIDERS += ( | ||
'lms.djangoapps.ccx.overrides.CustomCoursesForEdxOverrideProvider', | ||
) | ||
COURSE_IMPORT_EXPORT_STORAGE = DEFAULT_FILE_STORAGE |
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.
Why is this needed?
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.
We included cms.djangoapps.contentstore.apps.ContentstoreConfig
in the INSTALLED_APPS
of the LMS to ensure the app is properly loaded and our course publish signals are triggered properly. The app relies on the COURSE_IMPORT_EXPORT_STORAGE
setting, which must be defined in the LMS configuration.
Failure to define this setting results in an error, as demonstrated in this related build log.
'cms.djangoapps.contentstore.apps.ContentstoreConfig', | ||
'openedx.core.djangoapps.content.search', | ||
'openedx.core.djangoapps.content_staging', | ||
] |
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.
I'd like to understand this better.
First of all, you're only adding 3 new apps here, right?
- ContentstoreConfig
- search
- content-staging
How is it that these aren't running already? Why does the CCX feature need them turned on?
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.
The problem is that a CCX course is created from LMS. All the above apps are already defined in CMS. The course publishing signals are not triggered in LMS until we define cms.djangoapps.contentstore.apps.ContentstoreConfig
in our INSTALLED_APPS
in LMS. The last 2 apps (openedx.core.djangoapps.content.search
and openedx.core.djangoapps.content_staging
) are required to be defined as they are being used in the code.
Is it feasible to add some tests for this? |
Yes
I am not sure, are suggesting adding tests for the feature flag? |
Description
CCX courses are created from LMS and the course publish signals are in CMS. This PR adds
cms.djangoapps.contentstore.apps.ContentstoreConfig
inINSTALLED_APPS
for CCX feature in LMS production settings. Without this, the course publish signals are not triggered at the time of CCX creation resulting in aCourseOutlineData.DoesNotExist
error for the CCX courses.Useful information to include:
changes.
Supporting information
https://github.com/mitodl/hq/issues/3880
Testing instructions
To test this locally, follow the steps below
private.py
in LMS, add these settings to enable CCX:private.py
in CMS, add this setting:In Studio, create a course and go to advanced settings. Toggle the "Enable CCX" setting there.
Next, go to the course's instructor dashboard, open the membership tab, and add a CCX coach.
Log in as the CCX coach, open the course you created, go to the CCX coach tab, and create a CCX. (You can find all the steps to create a CCX in the docs).
Save your changes in the Schedule Tab, then check the course outline of the CCX course.
CourseOutlineData.DoesNotExist
error in your LMS logs.To fix this, update the
INSTALLED_APPS
line in LMS settings to:Go back to the Schedule Tab, make any changes, and save again.
Finally, revisit the course outline page of the CCX course.
Deadline
"None"
Other information
Include anything else that will help reviewers and consumers understand the change.