Skip to content

Commit

Permalink
feat: support unit preview in learning MFE (#35747)
Browse files Browse the repository at this point in the history
* feat: update preview url to direct to mfe

* fix: use url builder instead of string formatter

* fix: url redirect for never published units

* fix: remove 404 error when  not a preview or staff

* feat: update sequence metadata to allow draft branch
  • Loading branch information
KristinAoki authored Nov 1, 2024
1 parent bd22449 commit e13d66d
Show file tree
Hide file tree
Showing 10 changed files with 301 additions and 265 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def test_home_page_response(self):
"courses": [{
"course_key": course_id,
"display_name": self.course.display_name,
"lms_link": f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}',
"lms_link": f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}',
"number": self.course.number,
"org": self.course.org,
"rerun_link": f'/course_rerun/{course_id}',
Expand All @@ -144,7 +144,7 @@ def test_home_page_response_with_api_v2(self):
OrderedDict([
("course_key", course_id),
("display_name", self.course.display_name),
("lms_link", f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}'),
("lms_link", f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}'),
("number", self.course.number),
("org", self.course.org),
("rerun_link", f'/course_rerun/{course_id}'),
Expand Down
18 changes: 13 additions & 5 deletions cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def test_home_page_response(self):
OrderedDict([
("course_key", course_id),
("display_name", self.course.display_name),
("lms_link", f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}'),
("lms_link", f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}'),
("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.course.id)}'),
("number", self.course.number),
("org", self.course.org),
Expand All @@ -76,7 +76,7 @@ def test_home_page_response(self):
("display_name", self.archived_course.display_name),
(
"lms_link",
f'//{settings.LMS_BASE}/courses/{archived_course_id}/jump_to/{self.archived_course.location}'
f'{settings.LMS_ROOT_URL}/courses/{archived_course_id}/jump_to/{self.archived_course.location}'
),
(
"cms_link",
Expand Down Expand Up @@ -139,7 +139,7 @@ def test_active_only_query_if_passed(self):
self.assertEqual(response.data["results"]["courses"], [OrderedDict([
("course_key", str(self.course.id)),
("display_name", self.course.display_name),
("lms_link", f'//{settings.LMS_BASE}/courses/{str(self.course.id)}/jump_to/{self.course.location}'),
("lms_link", f'{settings.LMS_ROOT_URL}/courses/{str(self.course.id)}/jump_to/{self.course.location}'),
("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.course.id)}'),
("number", self.course.number),
("org", self.course.org),
Expand All @@ -164,7 +164,11 @@ def test_archived_only_query_if_passed(self):
("display_name", self.archived_course.display_name),
(
"lms_link",
f'//{settings.LMS_BASE}/courses/{str(self.archived_course.id)}/jump_to/{self.archived_course.location}',
'{url_root}/courses/{course_id}/jump_to/{location}'.format(
url_root=settings.LMS_ROOT_URL,
course_id=str(self.archived_course.id),
location=self.archived_course.location
),
),
("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.archived_course.id)}'),
("number", self.archived_course.number),
Expand All @@ -190,7 +194,11 @@ def test_search_query_if_passed(self):
("display_name", self.archived_course.display_name),
(
"lms_link",
f'//{settings.LMS_BASE}/courses/{str(self.archived_course.id)}/jump_to/{self.archived_course.location}',
'{url_root}/courses/{course_id}/jump_to/{location}'.format(
url_root=settings.LMS_ROOT_URL,
course_id=str(self.archived_course.id),
location=self.archived_course.location
),
),
("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.archived_course.id)}'),
("number", self.archived_course.number),
Expand Down
27 changes: 13 additions & 14 deletions cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from collections import defaultdict
from contextlib import contextmanager
from datetime import datetime, timezone
from urllib.parse import quote_plus
from urllib.parse import quote_plus, urlencode, urlunparse, urlparse
from uuid import uuid4

from bs4 import BeautifulSoup
Expand Down Expand Up @@ -193,31 +193,30 @@ def get_lms_link_for_item(location, preview=False):
"""
assert isinstance(location, UsageKey)

# checks LMS_BASE value in site configuration for the given course_org_filter(org)
# if not found returns settings.LMS_BASE
# checks LMS_ROOT_URL value in site configuration for the given course_org_filter(org)
# if not found returns settings.LMS_ROOT_URL
lms_base = SiteConfiguration.get_value_for_org(
location.org,
"LMS_BASE",
settings.LMS_BASE
"LMS_ROOT_URL",
settings.LMS_ROOT_URL
)
query_string = ''

if lms_base is None:
return None

if preview:
# checks PREVIEW_LMS_BASE value in site configuration for the given course_org_filter(org)
# if not found returns settings.FEATURES.get('PREVIEW_LMS_BASE')
lms_base = SiteConfiguration.get_value_for_org(
location.org,
"PREVIEW_LMS_BASE",
settings.FEATURES.get('PREVIEW_LMS_BASE')
)
params = {'preview': '1'}
query_string = urlencode(params)

return "//{lms_base}/courses/{course_key}/jump_to/{location}".format(
lms_base=lms_base,
url_parts = list(urlparse(lms_base))
url_parts[2] = '/courses/{course_key}/jump_to/{location}'.format(
course_key=str(location.course_key),
location=str(location),
)
url_parts[4] = query_string

return urlunparse(url_parts)


def get_lms_link_for_certificate_web_view(course_key, mode):
Expand Down
86 changes: 30 additions & 56 deletions lms/djangoapps/courseware/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,38 +132,6 @@ class TestJumpTo(ModuleStoreTestCase):
"""
Check the jumpto link for a course.
"""
@ddt.data(
(True, False), # preview -> Legacy experience
(False, True), # no preview -> MFE experience
)
@ddt.unpack
def test_jump_to_legacy_vs_mfe(self, preview_mode, expect_mfe):
"""
Test that jump_to and jump_to_id correctly choose which courseware frontend to redirect to.
Can be removed when the MFE supports a preview mode.
"""
course = CourseFactory.create()
chapter = BlockFactory.create(category='chapter', parent_location=course.location)
if expect_mfe:
expected_url = f'http://learning-mfe/course/{course.id}/{chapter.location}'
else:
expected_url = f'/courses/{course.id}/courseware/{chapter.url_name}/'

jumpto_url = f'/courses/{course.id}/jump_to/{chapter.location}'
with set_preview_mode(preview_mode):
response = self.client.get(jumpto_url)
assert response.status_code == 302
# Check the response URL, but chop off the querystring; we don't care here.
assert response.url.split('?')[0] == expected_url

jumpto_id_url = f'/courses/{course.id}/jump_to_id/{chapter.url_name}'
with set_preview_mode(preview_mode):
response = self.client.get(jumpto_id_url)
assert response.status_code == 302
# Check the response URL, but chop off the querystring; we don't care here.
assert response.url.split('?')[0] == expected_url

@ddt.data(
(False, ModuleStoreEnum.Type.split),
(True, ModuleStoreEnum.Type.split),
Expand All @@ -174,32 +142,34 @@ def test_jump_to_invalid_location(self, preview_mode, store_type):
with self.store.default_store(store_type):
course = CourseFactory.create()
location = course.id.make_usage_key(None, 'NoSuchPlace')
expected_redirect_url = (
f'/courses/{course.id}/courseware?' + urlencode({'activate_block_id': str(course.location)})

expected_redirect_url = f'http://learning-mfe/course/{course.id}'
jumpto_url = (
f'/courses/{course.id}/jump_to/{location}?preview=1'
) if preview_mode else (
f'http://learning-mfe/course/{course.id}'
f'/courses/{course.id}/jump_to/{location}'
)

# This is fragile, but unfortunately the problem is that within the LMS we
# can't use the reverse calls from the CMS
jumpto_url = f'/courses/{course.id}/jump_to/{location}'
with set_preview_mode(preview_mode):
with set_preview_mode(False):
response = self.client.get(jumpto_url)
assert response.status_code == 302
assert response.url == expected_redirect_url

@set_preview_mode(True)
def test_jump_to_legacy_from_sequence(self):
@set_preview_mode(False)
def test_jump_to_preview_from_sequence(self):
with self.store.default_store(ModuleStoreEnum.Type.split):
course = CourseFactory.create()
chapter = BlockFactory.create(category='chapter', parent_location=course.location)
sequence = BlockFactory.create(category='sequential', parent_location=chapter.location)
activate_block_id = urlencode({'activate_block_id': str(sequence.location)})
jumpto_url = f'/courses/{course.id}/jump_to/{sequence.location}?preview=1'
expected_redirect_url = (
f'/courses/{course.id}/courseware/{chapter.url_name}/{sequence.url_name}/?{activate_block_id}'
f'http://learning-mfe/preview/course/{course.id}/{sequence.location}'
)
jumpto_url = f'/courses/{course.id}/jump_to/{sequence.location}'
response = self.client.get(jumpto_url)
self.assertRedirects(response, expected_redirect_url, status_code=302, target_status_code=302)
assert response.status_code == 302
assert response.url == expected_redirect_url

@set_preview_mode(False)
def test_jump_to_mfe_from_sequence(self):
Expand All @@ -214,8 +184,8 @@ def test_jump_to_mfe_from_sequence(self):
assert response.status_code == 302
assert response.url == expected_redirect_url

@set_preview_mode(True)
def test_jump_to_legacy_from_block(self):
@set_preview_mode(False)
def test_jump_to_preview_from_block(self):
with self.store.default_store(ModuleStoreEnum.Type.split):
course = CourseFactory.create()
chapter = BlockFactory.create(category='chapter', parent_location=course.location)
Expand All @@ -225,21 +195,21 @@ def test_jump_to_legacy_from_block(self):
block1 = BlockFactory.create(category='html', parent_location=vertical1.location)
block2 = BlockFactory.create(category='html', parent_location=vertical2.location)

activate_block_id = urlencode({'activate_block_id': str(block1.location)})
jumpto_url = f'/courses/{course.id}/jump_to/{block1.location}?preview=1'
expected_redirect_url = (
f'/courses/{course.id}/courseware/{chapter.url_name}/{sequence.url_name}/1?{activate_block_id}'
f'http://learning-mfe/preview/course/{course.id}/{sequence.location}/{vertical1.location}'
)
jumpto_url = f'/courses/{course.id}/jump_to/{block1.location}'
response = self.client.get(jumpto_url)
self.assertRedirects(response, expected_redirect_url, status_code=302, target_status_code=302)
assert response.status_code == 302
assert response.url == expected_redirect_url

activate_block_id = urlencode({'activate_block_id': str(block2.location)})
jumpto_url = f'/courses/{course.id}/jump_to/{block2.location}?preview=1'
expected_redirect_url = (
f'/courses/{course.id}/courseware/{chapter.url_name}/{sequence.url_name}/2?{activate_block_id}'
f'http://learning-mfe/preview/course/{course.id}/{sequence.location}/{vertical2.location}'
)
jumpto_url = f'/courses/{course.id}/jump_to/{block2.location}'
response = self.client.get(jumpto_url)
self.assertRedirects(response, expected_redirect_url, status_code=302, target_status_code=302)
assert response.status_code == 302
assert response.url == expected_redirect_url

@set_preview_mode(False)
def test_jump_to_mfe_from_block(self):
Expand Down Expand Up @@ -300,8 +270,12 @@ def test_jump_to_legacy_from_nested_block(self):
def test_jump_to_id_invalid_location(self, preview_mode, store_type):
with self.store.default_store(store_type):
course = CourseFactory.create()
jumpto_url = f'/courses/{course.id}/jump_to/NoSuchPlace'
with set_preview_mode(preview_mode):
jumpto_url = (
f'/courses/{course.id}/jump_to/NoSuchPlace?preview=1'
) if preview_mode else (
f'/courses/{course.id}/jump_to/NoSuchPlace'
)
with set_preview_mode(False):
response = self.client.get(jumpto_url)
assert response.status_code == 404

Expand Down Expand Up @@ -3359,7 +3333,7 @@ def test_learner_redirect(self):
def test_preview_no_redirect(self):
__, __, preview_url = self._get_urls()
with set_preview_mode(True):
# Previews will not redirect to the mfe
# Previews server from PREVIEW_LMS_BASE will not redirect to the mfe
course_staff = UserFactory.create(is_staff=False)
CourseStaffRole(self.course_key).add_users(course_staff)
self.client.login(username=course_staff.username, password=TEST_PASSWORD)
Expand Down
3 changes: 3 additions & 0 deletions lms/djangoapps/courseware/views/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from xmodule.course_block import COURSE_VISIBILITY_PUBLIC
from xmodule.modulestore.django import modulestore
from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW
from xmodule.util.xmodule_django import get_current_request_hostname

from common.djangoapps.edxmako.shortcuts import render_to_response, render_to_string
from common.djangoapps.student.models import CourseEnrollment
Expand Down Expand Up @@ -188,11 +189,13 @@ def microfrontend_url(self):
unit_key = None
except InvalidKeyError:
unit_key = None
is_preview = settings.FEATURES.get('PREVIEW_LMS_BASE') == get_current_request_hostname()
url = make_learning_mfe_courseware_url(
self.course_key,
self.section.location if self.section else None,
unit_key,
params=self.request.GET,
preview=is_preview,
)
return url

Expand Down
Loading

0 comments on commit e13d66d

Please sign in to comment.