[DO NOT MERGE] feat: enable marketing site feature in CMS and LMS configurations #176
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enables the marketing site feature in both CMS and LMS configurations by setting ENABLE_MKTG_SITE to true and adding comprehensive marketing URL mappings.
- Changes the default value of
ENABLE_MKTG_SITEfrom false to true in all configuration files - Adds marketing URL mappings for common pages (about, contact, FAQ, courses, etc.)
- Updates both Python and YAML configuration files to maintain consistency
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| py_configuration_files/lms.py | Changes default ENABLE_MARKETING_SITE environment variable from False to True |
| configuration_files/lms.yml | Enables MKTG_SITE feature and adds marketing URL mappings with localhost:18000 root |
| configuration_files/cms.yml | Enables MKTG_SITE feature and adds marketing URL mappings with localhost:18010 root |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
vgulati-apphelix
changed the title
configuration_files/cms.yml
Outdated
| ENABLE_GRADE_DOWNLOADS: true | ||
| ENABLE_LTI_PROVIDER: false | ||
| ENABLE_MKTG_SITE: false | ||
| ENABLE_MKTG_SITE: true |
There was a problem hiding this comment.
I thought the FEATURES dict was going away and all of these were duplicated to top-level?
There was a problem hiding this comment.
i have fixed them and make them to their correct place
| MKTG_URLS: {} | ||
| MKTG_URL_LINK_MAP: {} | ||
| MKTG_URLS: | ||
| ROOT: http://localhost:18010 |
There was a problem hiding this comment.
I know this was asked elsewhere, but ROOT: '' does not work?
There was a problem hiding this comment.
ROOT: '' (empty string) does not work as it requires a non-empty value for ROOT in MKTG_URLS. If you set it to an empty string, you’ll get errors. That’s why we use ROOT: http://localhost:18010 for devstack
Uses the same LMS instance for both learning and marketing pages ROOT: http://localhost:18000 points marketing URLs back to the LMS itself
Keep the configurations different as they are designed for different use cases. The devstack configuration allows you to test marketing site integration during development, while the sandbox configuration provides a simpler setup for basic testing.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| - host: edx.devstack.elasticsearch | ||
| port: 9200 | ||
| use_ssl: false | ||
| ENABLE_MKTG_SITE: true |
There was a problem hiding this comment.
Remove trailing whitespace after 'true'.
| ENABLE_MKTG_SITE: true | |
| ENABLE_MKTG_SITE: true |
Copilot uses AI. Check for mistakes.
| - host: edx.devstack.elasticsearch | ||
| port: 9200 | ||
| use_ssl: false | ||
| ENABLE_MKTG_SITE: true |
There was a problem hiding this comment.
Remove trailing whitespace after 'true'.
| ENABLE_MKTG_SITE: true | |
| ENABLE_MKTG_SITE: true |
Copilot uses AI. Check for mistakes.
| MKTG_URLS: {} | ||
| MKTG_URL_LINK_MAP: {} | ||
| MKTG_URLS: | ||
| ROOT: http://localhost:18000 |
There was a problem hiding this comment.
The ROOT URL uses port 18000, but this appears to be a hardcoded localhost development URL. Consider using an environment variable or configuration parameter to make this configurable across different environments.
| ROOT: http://localhost:18000 | |
| ROOT: ${MKTG_ROOT_URL:http://localhost:18000} |
Copilot uses AI. Check for mistakes.
| MKTG_URLS: {} | ||
| MKTG_URL_LINK_MAP: {} | ||
| MKTG_URLS: | ||
| ROOT: http://localhost:18010 |
There was a problem hiding this comment.
The ROOT URL uses port 18010, but this appears to be a hardcoded localhost development URL. Consider using an environment variable or configuration parameter to make this configurable across different environments.
| ROOT: http://localhost:18010 | |
| # The ROOT URL for marketing pages. Set via the MKTG_URLS_ROOT environment variable, defaults to localhost:18010 for development. | |
| ROOT: ${MKTG_URLS_ROOT:http://localhost:18010} |
Copilot uses AI. Check for mistakes.
3 participants
Description
Adding MKTG_URLS Links In Configuration Files
Enabling ENABLE_MKTG_SITE: true
Related Ticket
BOMS 215