Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cms/djangoapps/contentstore/tests/test_course_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,9 @@ def test_disable_advanced_settings_feature(self, disable_advanced_settings):
"""
advanced_settings_link_html = f"<a href=\"{self.course_setting_url}\">Advanced Settings</a>".encode('utf-8')

with override_settings(FEATURES={'DISABLE_ADVANCED_SETTINGS': disable_advanced_settings}):
with override_settings(FEATURES={
'DISABLE_ADVANCED_SETTINGS': disable_advanced_settings,
}):
for handler in (
'import_handler',
'export_handler',
Expand Down
2 changes: 2 additions & 0 deletions cms/envs/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,3 +373,5 @@
SOFTWARE_SECURE_VERIFICATION_ROUTING_KEY = "edx.lms.core.default"
STATIC_ROOT_BASE = "/edx/var/edxapp/staticfiles"
STATIC_URL_BASE = "/static/"

CATALOG_MICROFRONTEND_URL = "http://catalog-mfe"
26 changes: 15 additions & 11 deletions common/djangoapps/util/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from django.conf import settings
from opaque_keys.edx.keys import CourseKey, UsageKey

from lms.djangoapps.branding.toggles import use_catalog_mfe
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx_filters.learning.filters import CourseAboutPageURLRequested

Expand Down Expand Up @@ -50,22 +51,25 @@ def get_link_for_about_page(course):
'SOCIAL_SHARING_SETTINGS',
getattr(settings, 'SOCIAL_SHARING_SETTINGS', {})
).get('CUSTOM_COURSE_URLS')

if use_catalog_mfe():
about_base_url = settings.CATALOG_MICROFRONTEND_URL
else:
about_base_url = configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL)

if is_social_sharing_enabled and course.social_sharing_url:
course_about_url = course.social_sharing_url
elif settings.FEATURES.get('ENABLE_MKTG_SITE') and getattr(course, 'marketing_url', None):
course_about_url = course.marketing_url
else:
course_about_url = '{about_base_url}/courses/{course_key}/about'.format(
about_base_url=configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL),
course_key=str(course.id),
)

## .. filter_implemented_name: CourseAboutPageURLRequested
## .. filter_type: org.openedx.learning.course_about.page.url.requested.v1
course_about_url, _ = CourseAboutPageURLRequested.run_filter(
url=course_about_url,
org=course.id.org,
)
course_about_url = f'{about_base_url}/courses/{course.id}/about'

## .. filter_implemented_name: CourseAboutPageURLRequested
## .. filter_type: org.openedx.learning.course_about.page.url.requested.v1
course_about_url, _ = CourseAboutPageURLRequested.run_filter(
url=course_about_url,
org=course.id.org,
)

return course_about_url

Expand Down
35 changes: 29 additions & 6 deletions common/djangoapps/util/tests/test_course.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import ddt
from django.conf import settings
from django.test import override_settings

from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from common.djangoapps.util.course import get_link_for_about_page
Expand Down Expand Up @@ -51,17 +52,18 @@ def get_course_sharing_link(self, enable_social_sharing, enable_mktg_site, use_o
"""
mock_settings = {
'FEATURES': {
'ENABLE_MKTG_SITE': enable_mktg_site
'ENABLE_MKTG_SITE': enable_mktg_site,
},
'SOCIAL_SHARING_SETTINGS': {
'CUSTOM_COURSE_URLS': enable_social_sharing
},
}
}

with mock.patch.multiple('django.conf.settings', **mock_settings):
course_sharing_link = get_link_for_about_page(
self.course_overview if use_overview else self.course
)
with override_settings(ENABLE_CATALOG_MICROFRONTEND=False):
with mock.patch.multiple('django.conf.settings', **mock_settings):
course_sharing_link = get_link_for_about_page(
self.course_overview if use_overview else self.course
)

return course_sharing_link

Expand Down Expand Up @@ -126,3 +128,24 @@ def test_sharing_link_with_course_block(self, enable_social_sharing, expected_co
use_overview=False,
)
assert actual_course_sharing_link == expected_course_sharing_link

@ddt.data(
(
True,
f'{settings.CATALOG_MICROFRONTEND_URL}/courses/course-v1:test_org+test_number+test_run/about'
),
(
False,
f'{settings.LMS_ROOT_URL}/courses/course-v1:test_org+test_number+test_run/about'
)
)
@ddt.unpack
def test_sharing_link_with_new_course_about_page(
self, catalog_mfe_enabled, expected_course_sharing_link
):
"""
Verify the method gives correct course sharing url when new course about page is used.
"""
with override_settings(ENABLE_CATALOG_MICROFRONTEND=catalog_mfe_enabled):
actual_course_sharing_link = get_link_for_about_page(self.course_overview)
assert actual_course_sharing_link == expected_course_sharing_link
2 changes: 1 addition & 1 deletion lms/djangoapps/branding/test_toggles.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ def test_use_catalog_mfe(self, enabled):
"""
Test the use_catalog_mfe toggle.
"""
with override_settings(FEATURES={'ENABLE_CATALOG_MICROFRONTEND': enabled}):
with override_settings(ENABLE_CATALOG_MICROFRONTEND=enabled):
assert use_catalog_mfe() == enabled
4 changes: 2 additions & 2 deletions lms/djangoapps/branding/toggles.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

def use_catalog_mfe():
"""
Determine if Catalog MFE is enabled, replacing student_dashboard
Returns a boolean = true if the Catalog MFE is enabled.
"""
return configuration_helpers.get_value(
'ENABLE_CATALOG_MICROFRONTEND', settings.FEATURES['ENABLE_CATALOG_MICROFRONTEND']
'ENABLE_CATALOG_MICROFRONTEND', getattr(settings, 'ENABLE_CATALOG_MICROFRONTEND', False)
)
Comment on lines +11 to 15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are now saying that this returns a boolean, which means in should never return None which can currently happen if the setting is not set. Either set a default for this value here or in the common.py settings file (I would prefer in the common.py settings file).

Also note, we have stopped using the FEATURES dictionary as a part of the platform settings simplification work. You should not use it in your new code and update anything that you're touching to drop its usage as it makes sense. You don't need to fix everything but do the minimal necessary to not add new code that uses this deprecated setting.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ENABLE_CATALOG_MICROFRONTEND is already in common.py. I changed this line to look up the key directly in settings, and adjusted the many tests that looked up this value from FEATURES.

13 changes: 13 additions & 0 deletions lms/djangoapps/learner_home/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
ENTERPRISE_ENABLED = "ENABLE_ENTERPRISE_INTEGRATION"


@ddt.ddt
class TestGetPlatformSettings(TestCase):
"""Tests for get_platform_settings"""

Expand Down Expand Up @@ -88,6 +89,18 @@ def test_happy_path(self, mock_marketing_link):
},
)

@ddt.data(
(True, f'{settings.CATALOG_MICROFRONTEND_URL}/courses'),
(False, '/courses'),
)
@ddt.unpack
def test_link_with_new_catalog_page(self, catalog_mfe_enabled, expected_catalog_link):
"""
Test that the catalog link is constructed correctly based on the MFE flags.
"""
with override_settings(ENABLE_CATALOG_MICROFRONTEND=catalog_mfe_enabled):
assert get_platform_settings()["courseSearchUrl"] == expected_catalog_link


@ddt.ddt
class TestGetUserAccountConfirmationInfo(SharedModuleStoreTestCase):
Expand Down
7 changes: 6 additions & 1 deletion lms/djangoapps/learner_home/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
from common.djangoapps.util.milestones_helpers import (
get_pre_requisite_courses_not_completed,
)
from lms.djangoapps.branding import toggles
from lms.djangoapps.bulk_email.models import Optout
from lms.djangoapps.bulk_email.models_api import is_bulk_email_feature_enabled
from lms.djangoapps.commerce.utils import EcommerceService
Expand Down Expand Up @@ -71,10 +72,14 @@
def get_platform_settings():
"""Get settings used for platform level connections: emails, url routes, etc."""

course_search_url = marketing_link("COURSES")
if toggles.use_catalog_mfe():
course_search_url = f"{settings.CATALOG_MICROFRONTEND_URL}/courses"

return {
"supportEmail": settings.DEFAULT_FEEDBACK_EMAIL,
"billingEmail": settings.PAYMENT_SUPPORT_EMAIL,
"courseSearchUrl": marketing_link("COURSES"),
"courseSearchUrl": course_search_url,
}


Expand Down
4 changes: 4 additions & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -3217,6 +3217,10 @@
# .. setting_default: None
# .. setting_description: Base URL of the exams dashboard micro-frontend for instructors.
EXAMS_DASHBOARD_MICROFRONTEND_URL = None
# .. setting_name: CATALOG_MICROFRONTEND_URL
# .. setting_default: None
# .. setting_description: Base URL of the micro-frontend-based course catalog page.
CATALOG_MICROFRONTEND_URL = None

# .. setting_name: DISCUSSION_SPAM_URLS
# .. setting_default: []
Expand Down
1 change: 1 addition & 0 deletions lms/envs/devstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing
AUTHN_MICROFRONTEND_URL = 'http://localhost:1999'
AUTHN_MICROFRONTEND_DOMAIN = 'localhost:1999'
EXAMS_DASHBOARD_MICROFRONTEND_URL = 'http://localhost:2020'
CATALOG_MICROFRONTEND_URL = 'http://localhost:1998/catalog'

################### FRONTEND APPLICATION DISCUSSIONS ###################
DISCUSSIONS_MICROFRONTEND_URL = 'http://localhost:2002'
Expand Down
1 change: 1 addition & 0 deletions lms/envs/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@
LEARNER_HOME_MICROFRONTEND_URL = "http://learner-home-mfe"
ORA_GRADING_MICROFRONTEND_URL = "http://ora-grading-mfe"
ORA_MICROFRONTEND_URL = "http://ora-mfe"
CATALOG_MICROFRONTEND_URL = "http://catalog-mfe"

########################## limiting dashboard courses ######################

Expand Down
Loading