Skip to content

Commit ad13690

Browse files
feanilclaude
andcommitted
feat: remove ENABLE_MKTG_SITE from enrollment, courseware, and mobile API
- course_about_url in enrollment emails now uses get_link_for_about_page(), resolving the marketing URL, social sharing URL, or LMS about URL as a fallback - get_link_for_about_page() uses course.marketing_url unconditionally - has_marketing_url() returns bool(self.marketing_url) directly - url_to_enroll() always returns marketing_link('COURSES') - _course_home_redirect_enabled() only checks ENABLE_COURSE_HOME_REDIRECT; default changed from True to False because the flag previously had no effect when ENABLE_MKTG_SITE=False (the old default) — operators who relied on ENABLE_MKTG_SITE=True to activate the course-about redirect must now set ENABLE_COURSE_HOME_REDIRECT=True explicitly - Remove ENABLE_MKTG_SITE patches from courseware, instructor, and mobile API tests; merge redundant test cases where applicable BREAKING CHANGE: ENABLE_COURSE_HOME_REDIRECT now defaults to False. Operators who previously had ENABLE_MKTG_SITE=True and want the course about page to redirect to the learning MFE must explicitly set ENABLE_COURSE_HOME_REDIRECT=True in their configuration. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d009235 commit ad13690

12 files changed

Lines changed: 40 additions & 200 deletions

File tree

common/djangoapps/util/course.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def get_link_for_about_page(course):
5959

6060
if is_social_sharing_enabled and course.social_sharing_url:
6161
course_about_url = course.social_sharing_url
62-
elif settings.FEATURES.get('ENABLE_MKTG_SITE') and getattr(course, 'marketing_url', None):
62+
elif getattr(course, 'marketing_url', None):
6363
course_about_url = course.marketing_url
6464
else:
6565
course_about_url = f'{about_base_url}/courses/{course.id}/about'

common/djangoapps/util/tests/test_course.py

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,12 @@ def setUp(self):
3838
self.course_overview.marketing_url = 'test_marketing_url'
3939
self.course_overview.save()
4040

41-
def get_course_sharing_link(self, enable_social_sharing, enable_mktg_site, use_overview=True):
41+
def get_course_sharing_link(self, enable_social_sharing, use_overview=True):
4242
"""
4343
Get course sharing link.
4444
4545
Arguments:
4646
enable_social_sharing(Boolean): To indicate whether social sharing is enabled.
47-
enable_mktg_site(Boolean): A feature flag to decide activation of marketing site.
4847
4948
Keyword Arguments:
5049
use_overview: indicates whether course overview or course block should get
@@ -53,9 +52,6 @@ def get_course_sharing_link(self, enable_social_sharing, enable_mktg_site, use_o
5352
Returns course sharing url.
5453
"""
5554
mock_settings = {
56-
'FEATURES': {
57-
'ENABLE_MKTG_SITE': enable_mktg_site,
58-
},
5955
'SOCIAL_SHARING_SETTINGS': {
6056
'CUSTOM_COURSE_URLS': enable_social_sharing
6157
}
@@ -70,19 +66,17 @@ def get_course_sharing_link(self, enable_social_sharing, enable_mktg_site, use_o
7066
return course_sharing_link
7167

7268
@ddt.data(
73-
(True, True, 'test_social_sharing_url'),
74-
(False, True, 'test_marketing_url'),
75-
(True, False, 'test_social_sharing_url'),
76-
(False, False, f'{settings.LMS_ROOT_URL}/courses/course-v1:test_org+test_number+test_run/about'),
69+
(True, 'test_social_sharing_url'),
70+
(False, 'test_marketing_url'),
7771
)
7872
@ddt.unpack
79-
def test_sharing_link_with_settings(self, enable_social_sharing, enable_mktg_site, expected_course_sharing_link):
73+
def test_sharing_link_with_settings(self, enable_social_sharing, expected_course_sharing_link):
8074
"""
81-
Verify the method gives correct course sharing url on settings manipulations.
75+
Verify the method gives correct course sharing url: social sharing URL takes priority
76+
over marketing URL, which takes priority over the LMS about URL.
8277
"""
8378
actual_course_sharing_link = self.get_course_sharing_link(
8479
enable_social_sharing=enable_social_sharing,
85-
enable_mktg_site=enable_mktg_site,
8680
)
8781
assert actual_course_sharing_link == expected_course_sharing_link
8882

@@ -107,7 +101,6 @@ def test_sharing_link_with_course_overview_attrs(self, overview_attrs, expected_
107101

108102
actual_course_sharing_link = self.get_course_sharing_link(
109103
enable_social_sharing=True,
110-
enable_mktg_site=True,
111104
)
112105
assert actual_course_sharing_link == expected_course_sharing_link
113106

@@ -122,11 +115,10 @@ def test_sharing_link_with_course_overview_attrs(self, overview_attrs, expected_
122115
def test_sharing_link_with_course_block(self, enable_social_sharing, expected_course_sharing_link):
123116
"""
124117
Verify the method gives correct course sharing url on passing
125-
course block as a parameter.
118+
course block as a parameter (course blocks have no marketing_url attribute).
126119
"""
127120
actual_course_sharing_link = self.get_course_sharing_link(
128121
enable_social_sharing=enable_social_sharing,
129-
enable_mktg_site=True,
130122
use_overview=False,
131123
)
132124
assert actual_course_sharing_link == expected_course_sharing_link
@@ -146,8 +138,11 @@ def test_sharing_link_with_new_course_about_page(
146138
self, catalog_mfe_enabled, expected_course_sharing_link
147139
):
148140
"""
149-
Verify the method gives correct course sharing url when new course about page is used.
141+
Verify the correct about URL base is used when neither social sharing URL nor
142+
marketing URL is set (catalog MFE URL vs LMS root URL).
150143
"""
144+
self.course_overview.marketing_url = None
145+
self.course_overview.save()
151146
with override_settings(ENABLE_CATALOG_MICROFRONTEND=catalog_mfe_enabled):
152147
actual_course_sharing_link = get_link_for_about_page(self.course_overview)
153148
assert actual_course_sharing_link == expected_course_sharing_link

lms/djangoapps/courseware/tests/test_about.py

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -115,15 +115,13 @@ def test_visible_about_page_settings(self):
115115
resp = self.client.get(url)
116116
self.assertRedirects(resp, reverse('dashboard'), fetch_redirect_response=False)
117117

118-
@patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True})
118+
@patch.dict(settings.FEATURES, {'ENABLE_COURSE_HOME_REDIRECT': True})
119119
def test_logged_in_marketing(self):
120120
self.setup_user()
121121
url = reverse('about_course', args=[str(self.course.id)])
122122
resp = self.client.get(url)
123123
self.assertRedirects(resp, course_home_url(self.course.id), fetch_redirect_response=False)
124124

125-
@patch.dict(settings.FEATURES, {'ENABLE_COURSE_HOME_REDIRECT': False})
126-
@patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True})
127125
def test_logged_in_marketing_without_course_home_redirect(self):
128126
"""
129127
Verify user is not redirected to course home page when
@@ -135,19 +133,6 @@ def test_logged_in_marketing_without_course_home_redirect(self):
135133
# should not be redirected
136134
self.assertContains(resp, "OOGIE BLOOGIE")
137135

138-
@patch.dict(settings.FEATURES, {'ENABLE_COURSE_HOME_REDIRECT': True})
139-
@patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': False})
140-
def test_logged_in_marketing_without_mktg_site(self):
141-
"""
142-
Verify user is not redirected to course home page when
143-
ENABLE_MKTG_SITE is set to False
144-
"""
145-
self.setup_user()
146-
url = reverse('about_course', args=[str(self.course.id)])
147-
resp = self.client.get(url)
148-
# should not be redirected
149-
self.assertContains(resp, "OOGIE BLOOGIE")
150-
151136
@patch.dict(settings.FEATURES, {'ENABLE_PREREQUISITE_COURSES': True})
152137
def test_pre_requisite_course(self):
153138
pre_requisite_course = CourseFactory.create(org='edX', course='900', display_name='pre requisite course')

lms/djangoapps/courseware/views/views.py

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -545,14 +545,11 @@ def get(self, request, course_id, tab_type, **kwargs): # pylint: disable=argume
545545
return CourseTabView.handle_exceptions(request, course_key, course, exception)
546546

547547
@staticmethod
548-
def url_to_enroll(course_key):
548+
def url_to_enroll(course_key): # pylint: disable=unused-argument
549549
"""
550550
Returns the URL to use to enroll in the specified course.
551551
"""
552-
url_to_enroll = reverse('about_course', args=[str(course_key)])
553-
if settings.FEATURES.get('ENABLE_MKTG_SITE'):
554-
url_to_enroll = marketing_link('COURSES')
555-
return url_to_enroll
552+
return marketing_link('COURSES')
556553

557554
@staticmethod
558555
def register_user_access_warning_messages(request, course):
@@ -1214,16 +1211,11 @@ def credit_course_requirements(course_key, student):
12141211

12151212
def _course_home_redirect_enabled():
12161213
"""
1217-
Return True value if user needs to be redirected to course home based on value of
1218-
`ENABLE_MKTG_SITE` and `ENABLE_COURSE_HOME_REDIRECT feature` flags
1219-
Returns: boolean True or False
1214+
Return True if users should be redirected from the course about page to course home.
12201215
"""
1221-
if configuration_helpers.get_value(
1222-
'ENABLE_MKTG_SITE', settings.FEATURES.get('ENABLE_MKTG_SITE', False)
1223-
) and configuration_helpers.get_value(
1224-
'ENABLE_COURSE_HOME_REDIRECT', settings.FEATURES.get('ENABLE_COURSE_HOME_REDIRECT', True)
1225-
):
1226-
return True
1216+
return bool(configuration_helpers.get_value(
1217+
'ENABLE_COURSE_HOME_REDIRECT', settings.FEATURES.get('ENABLE_COURSE_HOME_REDIRECT', False)
1218+
))
12271219

12281220

12291221
@login_required

lms/djangoapps/instructor/enrollment.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
get_event_transaction_id,
3636
set_event_transaction_type,
3737
)
38+
from common.djangoapps.util.course import get_link_for_about_page
3839
from lms.djangoapps.branding.api import get_logo_url_for_email
3940
from lms.djangoapps.courseware.models import StudentModule
4041
from lms.djangoapps.grades.api import constants as grades_constants
@@ -494,14 +495,7 @@ def get_email_params(course, auto_enroll, secure=True, course_key=None, display_
494495
path=reverse('course_root', kwargs={'course_id': course_key})
495496
)
496497

497-
# We can't get the url to the course's About page if the marketing site is enabled.
498-
course_about_url = None
499-
if not settings.FEATURES.get('ENABLE_MKTG_SITE', False):
500-
course_about_url = '{proto}://{site}{path}'.format(
501-
proto=protocol,
502-
site=stripped_site_name,
503-
path=reverse('about_course', kwargs={'course_id': course_key})
504-
)
498+
course_about_url = get_link_for_about_page(course)
505499

506500
is_shib_course = uses_shib(course)
507501

lms/djangoapps/instructor/tests/test_api.py

Lines changed: 3 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,50 +1316,7 @@ def test_enroll_with_email_not_registered(self, protocol):
13161316

13171317
assert 'Once you have registered and activated your account,' in body
13181318

1319-
assert '{proto}://{site}{about_path}'.format( # noqa: UP032
1320-
proto=protocol,
1321-
site=self.site_name,
1322-
about_path=self.about_path
1323-
) in body
1324-
1325-
assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body
1326-
1327-
@ddt.data('http', 'https')
1328-
@patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True})
1329-
def test_enroll_email_not_registered_mktgsite(self, protocol):
1330-
url = reverse('students_update_enrollment', kwargs={'course_id': str(self.course.id)})
1331-
params = {'identifiers': self.notregistered_email, 'action': 'enroll', 'email_students': True}
1332-
environ = {'wsgi.url_scheme': protocol}
1333-
response = self.client.post(url, params, **environ)
1334-
1335-
manual_enrollments = ManualEnrollmentAudit.objects.all()
1336-
assert manual_enrollments.count() == 1
1337-
assert manual_enrollments[0].state_transition == UNENROLLED_TO_ALLOWEDTOENROLL
1338-
assert response.status_code == 200
1339-
1340-
text_body = mail.outbox[0].body
1341-
html_body = mail.outbox[0].alternatives[0][0]
1342-
1343-
assert text_body.startswith('Dear student,')
1344-
assert 'To finish your registration, please visit' in text_body
1345-
assert 'Please finish your registration and fill' in html_body
1346-
1347-
for body in [text_body, html_body]:
1348-
assert 'You have been invited to join {display_name} at edx.org by a member of the course staff.'.format( # noqa: UP032 # pylint: disable=line-too-long
1349-
display_name=self.course.display_name
1350-
) in body
1351-
1352-
assert '{proto}://{site}/register'.format( # noqa: UP032
1353-
proto=protocol,
1354-
site=self.site_name
1355-
) in body
1356-
1357-
assert ('fill out the registration form making sure to use '
1358-
'robot-not-an-email-yet@robot.org in the Email field') in body
1359-
1360-
assert 'You can then enroll in {display_name}.'.format( # noqa: UP032
1361-
display_name=self.course.display_name
1362-
) in body
1319+
assert f'{settings.LMS_ROOT_URL}{self.about_path}' in body
13631320

13641321
assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body
13651322

@@ -1587,11 +1544,7 @@ def test_enroll_with_email_not_registered_with_shib(self, protocol, mock_uses_sh
15871544

15881545
text_body = mail.outbox[0].body
15891546
html_body = mail.outbox[0].alternatives[0][0]
1590-
course_url = '{proto}://{site}{about_path}'.format( # noqa: UP032
1591-
proto=protocol,
1592-
site=self.site_name,
1593-
about_path=self.about_path,
1594-
)
1547+
course_url = f'{settings.LMS_ROOT_URL}{self.about_path}'
15951548
assert text_body.startswith('Dear student,')
15961549
assert 'To access this course visit {course_url} and register for this course.'.format( # noqa: UP032
15971550
course_url=course_url,
@@ -1606,31 +1559,6 @@ def test_enroll_with_email_not_registered_with_shib(self, protocol, mock_uses_sh
16061559

16071560
assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body
16081561

1609-
@patch('lms.djangoapps.instructor.enrollment.uses_shib')
1610-
@patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True})
1611-
def test_enroll_email_not_registered_shib_mktgsite(self, mock_uses_shib):
1612-
# Try with marketing site enabled and shib on
1613-
mock_uses_shib.return_value = True
1614-
1615-
url = reverse('students_update_enrollment', kwargs={'course_id': str(self.course.id)})
1616-
# Try with marketing site enabled
1617-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
1618-
response = self.client.post(url, {'identifiers': self.notregistered_email, 'action': 'enroll',
1619-
'email_students': True})
1620-
1621-
assert response.status_code == 200
1622-
1623-
text_body = mail.outbox[0].body
1624-
html_body = mail.outbox[0].alternatives[0][0]
1625-
assert text_body.startswith('Dear student,')
1626-
1627-
for body in [text_body, html_body]:
1628-
assert 'You have been invited to join {display_name} at edx.org by a member of the course staff.'.format( # noqa: UP032 # pylint: disable=line-too-long
1629-
display_name=self.course.display_name,
1630-
) in body
1631-
1632-
assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body
1633-
16341562
@ddt.data('http', 'https')
16351563
@patch('lms.djangoapps.instructor.enrollment.uses_shib')
16361564
def test_enroll_with_email_not_registered_with_shib_autoenroll(self, protocol, mock_uses_shib):
@@ -2270,11 +2198,7 @@ def test_add_notenrolled_with_email(self, protocol):
22702198
assert 'by a member of the course staff.' in body
22712199
assert 'enroll in this course and begin the beta test' in body
22722200

2273-
assert '{proto}://{site}{about_path}'.format( # noqa: UP032
2274-
proto=protocol,
2275-
site=self.site_name,
2276-
about_path=self.about_path,
2277-
) in body
2201+
assert f'{settings.LMS_ROOT_URL}{self.about_path}' in body
22782202

22792203
assert 'This email was automatically sent from edx.org to {student_email}'.format( # noqa: UP032
22802204
student_email=self.notenrolled_student.email,
@@ -2332,31 +2256,6 @@ def test_add_notenrolled_with_email_autoenroll(self, protocol):
23322256
student_email=self.notenrolled_student.email,
23332257
) in body
23342258

2335-
@patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True})
2336-
def test_add_notenrolled_email_mktgsite(self):
2337-
# Try with marketing site enabled
2338-
url = reverse('bulk_beta_modify_access', kwargs={'course_id': str(self.course.id)})
2339-
response = self.client.post(url, {'identifiers': self.notenrolled_student.email, 'action': 'add', 'email_students': True}) # pylint: disable=line-too-long
2340-
2341-
assert response.status_code == 200
2342-
2343-
text_body = mail.outbox[0].body
2344-
html_body = mail.outbox[0].alternatives[0][0]
2345-
student_name = self.notenrolled_student.profile.name
2346-
assert text_body.startswith(f'Dear {student_name}')
2347-
2348-
for body in [text_body, html_body]:
2349-
assert 'You have been invited to be a beta tester for {display_name} at edx.org'.format( # noqa: UP032
2350-
display_name=self.course.display_name,
2351-
) in body
2352-
2353-
assert 'by a member of the course staff.' in body
2354-
assert 'Visit edx.org' in body
2355-
assert 'enroll in this course and begin the beta test' in body
2356-
assert 'This email was automatically sent from edx.org to {student_email}'.format( # noqa: UP032
2357-
student_email=self.notenrolled_student.email,
2358-
) in body
2359-
23602259
def test_enroll_with_email_not_registered(self):
23612260
# User doesn't exist
23622261
url = reverse('bulk_beta_modify_access', kwargs={'course_id': str(self.course.id)})

lms/djangoapps/instructor/tests/test_enrollment.py

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -887,6 +887,8 @@ class TestGetEmailParamsCCX(SharedModuleStoreTestCase):
887887
def setUpClass(cls):
888888
super().setUpClass()
889889
cls.course = CourseFactory.create()
890+
# get_link_for_about_page uses course.id (parent course), not the CCX key
891+
cls.course_about_url = f'{settings.LMS_ROOT_URL}/courses/{cls.course.id}/about'
890892

891893
@override_settings(CUSTOM_COURSES_EDX=True)
892894
def setUp(self):
@@ -903,7 +905,6 @@ def setUp(self):
903905
site,
904906
self.course_key
905907
)
906-
self.course_about_url = self.course_url + 'about'
907908
self.registration_url = f'https://{site}/register'
908909

909910
@override_settings(CUSTOM_COURSES_EDX=True)
@@ -940,13 +941,11 @@ def setUpClass(cls):
940941
site,
941942
str(cls.course.id)
942943
)
943-
cls.course_about_url = cls.course_url + 'about'
944944
cls.registration_url = f'https://{site}/register'
945945
cls.logo_url = get_logo_url_for_email()
946+
cls.course_about_url = f'{settings.LMS_ROOT_URL}/courses/{cls.course.id}/about'
946947

947948
def test_normal_params(self):
948-
# For a normal site, what do we expect to get for the URLs?
949-
# Also make sure `auto_enroll` is properly passed through.
950949
result = get_email_params(self.course, False)
951950

952951
assert result['auto_enroll'] is False
@@ -955,18 +954,11 @@ def test_normal_params(self):
955954
assert result['course_url'] == self.course_url
956955
assert result['logo_url'] == self.logo_url
957956

958-
def test_marketing_params(self):
959-
# For a site with a marketing front end, what do we expect to get for the URLs?
960-
# Also make sure `auto_enroll` is properly passed through.
961-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
962-
result = get_email_params(self.course, True)
957+
def test_auto_enroll_params(self):
958+
result = get_email_params(self.course, True)
963959

964960
assert result['auto_enroll'] is True
965-
# We should *not* get a course about url (LMS doesn't know what the marketing site URLs are)
966-
assert result['course_about_url'] is None
967-
assert result['registration_url'] == self.registration_url
968-
assert result['course_url'] == self.course_url
969-
assert result['logo_url'] == self.logo_url
961+
assert result['course_about_url'] == self.course_about_url
970962

971963
@patch('lms.djangoapps.instructor.enrollment.get_logo_url_for_email', return_value='https://www.logo.png')
972964
def test_logo_url_params(self, mock_get_logo_url_for_email):

0 commit comments

Comments
 (0)