fix: when redirecting to the MFE, preserve query flags

This can potentially fix analytics based on query params like
utm_campaign and the like. And generally seems like the correct
thing to do.

AA-1128
This commit is contained in:
Michael Terry
2021-12-07 13:48:04 -05:00
parent f1cc288189
commit bf2c2ce3c3
7 changed files with 53 additions and 11 deletions

View File

@@ -18,6 +18,7 @@ from crum import set_current_request
from django.conf import settings
from django.contrib.auth.models import AnonymousUser
from django.http import Http404, HttpResponseBadRequest
from django.http.request import QueryDict
from django.test import RequestFactory, TestCase
from django.test.client import Client
from django.test.utils import override_settings
@@ -498,9 +499,9 @@ class BaseViewsTestCase(ModuleStoreTestCase): # lint-amnesty, pylint: disable=m
'course_id': str(self.course_key),
'chapter': str(self.chapter.location.block_id),
'section': str(self.section2.location.block_id),
}
)
mfe_url = '{}/course/{}/{}'.format(
},
) + '?foo=b$r'
mfe_url = '{}/course/{}/{}?foo=b%24r'.format(
settings.LEARNING_MICROFRONTEND_URL,
self.course_key,
self.section2.location
@@ -3395,10 +3396,12 @@ class AccessUtilsTestCase(ModuleStoreTestCase):
@ddt.ddt
@override_waffle_flag(COURSE_HOME_USE_LEGACY_FRONTEND, active=True)
class DatesTabTestCase(ModuleStoreTestCase):
"""
Ensure that the dates page renders with the correct data for both a verified and audit learner
"""
MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
def setUp(self):
super().setUp()
@@ -3533,6 +3536,15 @@ class DatesTabTestCase(ModuleStoreTestCase):
response = self._get_response(self.course)
self.assertContains(response, 'div class="banner-cta-text"')
@override_waffle_flag(COURSE_HOME_USE_LEGACY_FRONTEND, active=False)
def test_legacy_redirect(self):
"""
Verify that the legacy dates page redirects to the MFE correctly.
"""
response = self.client.get(reverse('dates', args=[str(self.course.id)]) + '?foo=b$r')
assert response.status_code == 302
assert response.get('Location') == f'http://learning-mfe/course/{self.course.id}/dates?foo=b%24r'
class TestShowCoursewareMFE(TestCase):
"""
@@ -3628,6 +3640,11 @@ class TestShowCoursewareMFE(TestCase):
'https://learningmfe.openedx.org'
'/course/course-v1:OpenEdX+MFE+2020'
)
assert make_learning_mfe_courseware_url(course_key, params=QueryDict('foo=b$r')) == (
'https://learningmfe.openedx.org'
'/course/course-v1:OpenEdX+MFE+2020'
'?foo=b%24r'
)
assert make_learning_mfe_courseware_url(course_key, section_key, '') == (
'https://learningmfe.openedx.org'
'/course/course-v1:OpenEdX+MFE+2020'

View File

@@ -207,7 +207,8 @@ class CoursewareIndex(View):
url = make_learning_mfe_courseware_url(
self.course_key,
self.section.location if self.section else None,
unit_key
unit_key,
params=self.request.GET,
)
return url

View File

@@ -1060,8 +1060,9 @@ def dates(request, course_id):
course_key = CourseKey.from_string(course_id)
if not (course_home_legacy_is_active(course_key) or request.user.is_staff):
microfrontend_url = get_learning_mfe_home_url(course_key=course_key, view_name=COURSE_DATES_NAME)
raise Redirect(microfrontend_url)
raise Redirect(get_learning_mfe_home_url(
course_key=course_key, view_name=COURSE_DATES_NAME, params=request.GET,
))
# Enable NR tracing for this view based on course
monitoring_utils.set_custom_attribute('course_id', str(course_key))
@@ -1137,8 +1138,9 @@ def progress(request, course_id, student_id=None):
course_key = CourseKey.from_string(course_id)
if course_home_mfe_progress_tab_is_active(course_key) and not request.user.is_staff:
microfrontend_url = get_learning_mfe_home_url(course_key=course_key, view_name=COURSE_PROGRESS_NAME)
raise Redirect(microfrontend_url)
raise Redirect(get_learning_mfe_home_url(
course_key=course_key, view_name=COURSE_PROGRESS_NAME, params=request.GET,
))
with modulestore().bulk_operations(course_key):
return _progress(request, course_key, student_id)

View File

@@ -227,6 +227,15 @@ class TestCourseHomePage(CourseHomePageTestCase): # lint-amnesty, pylint: disab
response = self.client.get(url)
assert response.status_code == 200
def test_legacy_redirect(self):
"""
Verify that the legacy course home page redirects to the MFE correctly.
"""
url = course_home_url(self.course) + '?foo=b$r'
response = self.client.get(url)
assert response.status_code == 302
assert response.get('Location') == 'http://learning-mfe/course/course-v1:edX+test+Test_Course/home?foo=b%24r'
@ddt.ddt
class TestCourseHomePageAccess(CourseHomePageTestCase):

View File

@@ -7,10 +7,10 @@ because the Studio course outline may need these utilities.
from enum import Enum
from typing import Optional
import six # lint-amnesty, pylint: disable=unused-import
from django.conf import settings
from django.contrib.auth import get_user_model
from django.http import HttpRequest
from django.http.request import QueryDict
from django.urls import reverse
from opaque_keys.edx.keys import CourseKey, UsageKey
from six.moves.urllib.parse import urlencode, urlparse
@@ -148,6 +148,7 @@ def make_learning_mfe_courseware_url(
course_key: CourseKey,
sequence_key: Optional[UsageKey] = None,
unit_key: Optional[UsageKey] = None,
params: Optional[QueryDict] = None,
) -> str:
"""
Return a str with the URL for the specified courseware content in the Learning MFE.
@@ -176,6 +177,7 @@ def make_learning_mfe_courseware_url(
`course_key`, `sequence_key`, and `unit_key` can be either OpaqueKeys or
strings. They're only ever used to concatenate a URL string.
`params` is an optional QueryDict object (e.g. request.GET)
"""
mfe_link = f'{settings.LEARNING_MICROFRONTEND_URL}/course/{course_key}'
@@ -185,11 +187,16 @@ def make_learning_mfe_courseware_url(
if unit_key:
mfe_link += f'/{unit_key}'
if params:
mfe_link += f'?{params.urlencode()}'
return mfe_link
def get_learning_mfe_home_url(
course_key: CourseKey, view_name: Optional[str] = None
course_key: CourseKey,
view_name: Optional[str] = None,
params: Optional[QueryDict] = None,
) -> str:
"""
Given a course run key and view name, return the appropriate course home (MFE) URL.
@@ -200,12 +207,16 @@ def get_learning_mfe_home_url(
`course_key` can be either an OpaqueKey or a string.
`view_name` is an optional string.
`params` is an optional QueryDict object (e.g. request.GET)
"""
mfe_link = f'{settings.LEARNING_MICROFRONTEND_URL}/course/{course_key}'
if view_name:
mfe_link += f'/{view_name}'
if params:
mfe_link += f'?{params.urlencode()}'
return mfe_link

View File

@@ -70,7 +70,7 @@ class CourseHomeView(CourseTabView):
if course_home_legacy_is_active(course.id) or request.user.is_staff:
home_fragment_view = CourseHomeFragmentView()
return home_fragment_view.render_to_fragment(request, course_id=course_id, **kwargs)
microfrontend_url = get_learning_mfe_home_url(course_key=course_id, view_name="home")
microfrontend_url = get_learning_mfe_home_url(course_key=course_id, view_name='home', params=request.GET)
raise Redirect(microfrontend_url)

View File

@@ -45,6 +45,8 @@ def learner_profile(request, username):
"""
if should_redirect_to_profile_microfrontend():
profile_microfrontend_url = f"{settings.PROFILE_MICROFRONTEND_URL}{username}"
if request.GET:
profile_microfrontend_url += f'?{request.GET.urlencode()}'
return redirect(profile_microfrontend_url)
try: