From e427fad52bfb29929efeb2559128b84780fc2049 Mon Sep 17 00:00:00 2001 From: David Joy Date: Fri, 21 Feb 2020 15:41:26 -0500 Subject: [PATCH] Removing redirect to MFE for jump_to links. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will ultmately be put back in as part of TNL-6982, but for now we don’t want this code to be associated with the waffle flag. --- lms/djangoapps/courseware/tests/test_views.py | 51 +------------------ lms/djangoapps/courseware/url_helpers.py | 13 +++-- 2 files changed, 7 insertions(+), 57 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 6464ffaa26..aa88525002 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -4,7 +4,6 @@ Tests courseware views.py """ -import logging import itertools import json import unittest @@ -37,7 +36,6 @@ from xblock.core import XBlock from xblock.fields import Scope, String import lms.djangoapps.courseware.views.views as views -from lms.djangoapps.courseware.toggles import REDIRECT_TO_COURSEWARE_MICROFRONTEND import shoppingcart from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory @@ -70,7 +68,6 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOvervi from openedx.core.djangoapps.crawlers.models import CrawlersConfig from openedx.core.djangoapps.credit.api import set_credit_requirements from openedx.core.djangoapps.credit.models import CreditCourse, CreditProvider -from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES, override_waffle_flag from openedx.core.djangolib.testing.utils import get_mock_request from openedx.core.lib.gating import api as gating_api @@ -106,10 +103,9 @@ QUERY_COUNT_TABLE_BLACKLIST = WAFFLE_TABLES FEATURES_WITH_DISABLE_HONOR_CERTIFICATE = settings.FEATURES.copy() FEATURES_WITH_DISABLE_HONOR_CERTIFICATE['DISABLE_HONOR_CERTIFICATES'] = True -LOGGER = logging.getLogger(__name__) @ddt.ddt -class TestJumpTo(ModuleStoreTestCase, SiteMixin): +class TestJumpTo(ModuleStoreTestCase): """ Check the jumpto link for a course. """ @@ -254,51 +250,6 @@ class TestJumpTo(ModuleStoreTestCase, SiteMixin): self.assertEqual(expected_url, get_redirect_url(course_key, usage_key, request)) - def test_jump_to_with_microfrontend_enabled(self): - course = CourseFactory.create() - chapter = ItemFactory.create(category='chapter', parent_location=course.location) - section = ItemFactory.create(category='sequential', parent_location=chapter.location) - unit = ItemFactory.create(category='vertical', parent_location=section.location) - course_key = CourseKey.from_string(six.text_type(course.id)) - - # unit position is hard coded here for simplicity - is there a simple way to get it from here? - expected = '/courses/{course_id}/courseware/{chapter_id}/{section_id}/1?{activate_block_id}'.format( - course_id=six.text_type(course.id), - chapter_id=chapter.url_name, - section_id=section.url_name, - activate_block_id=urlencode({'activate_block_id': six.text_type(unit.location)}) - ) - jumpto_url = '{0}/{1}/jump_to/{2}'.format( - '/courses', - six.text_type(course.id), - six.text_type(unit.location), - ) - - microfrontend_expected = 'http://learning-mfe/course/{course_key}/{section_id}/{unit_id}'.format( - course_key=course_key, - section_id=section.location, - unit_id=unit.location - ) - """ - http://learning-mfe/course/org.0/course_0/Run_0/i4x://org.0/course_0/sequential/sequential_2/i4x://org.0/course_0/vertical/vertical_3', - http://learning-mfe/course/org.0/course_0/Run_0/i4x://org.0/course_0/sequential/sequential_2/vertical_3 - """ - - with override_waffle_flag(REDIRECT_TO_COURSEWARE_MICROFRONTEND, active=True): - response = self.client.get(jumpto_url) - self.assertRedirects(response, expected, status_code=302, target_status_code=302) - - # Test with waffle flag active and site setting enabled, redirects to microfrontend - site_domain = 'othersite.example.com' - self.set_up_site(site_domain, { - 'SITE_NAME': site_domain, - 'ENABLE_COURSEWARE_MICROFRONTEND': True - }) - - response = self.client.get(jumpto_url) - LOGGER.error(response.url) # None/course/org.0/course_0/Run_0/i4x://org.0/course_0/sequential/sequential_2 - self.assertRedirects(response, microfrontend_expected, fetch_redirect_response=False, status_code=302, target_status_code=302) - @ddt.ddt class IndexQueryTestCase(ModuleStoreTestCase): diff --git a/lms/djangoapps/courseware/url_helpers.py b/lms/djangoapps/courseware/url_helpers.py index 4fbe27ad9c..52bea116a8 100644 --- a/lms/djangoapps/courseware/url_helpers.py +++ b/lms/djangoapps/courseware/url_helpers.py @@ -2,6 +2,7 @@ Module to define url helpers functions """ + import six from django.conf import settings from django.urls import reverse @@ -9,7 +10,6 @@ from six.moves.urllib.parse import urlencode # pylint: disable=import-error from xmodule.modulestore.django import modulestore from xmodule.modulestore.search import navigation_index, path_to_location -from lms.djangoapps.courseware.toggles import should_redirect_to_courseware_microfrontend def get_redirect_url(course_key, usage_key, request=None): @@ -26,10 +26,6 @@ def get_redirect_url(course_key, usage_key, request=None): Redirect url string """ - if should_redirect_to_courseware_microfrontend(course_key): - path = path_to_location(modulestore(), usage_key, request, full_path=True) - return get_microfrontend_redirect_url(course_key, path) - ( course_key, chapter, section, vertical_unused, position, final_target_id @@ -65,7 +61,9 @@ def get_microfrontend_redirect_url(course_key, path=None): a separate API call, so all we need here is the course_key, section, and vertical IDs to format it's URL. - It is also capable of determining our section and vertical if they're not present. Fully specifying it all is preferable, though, as the micro-frontend can save itself some work, resulting in a better user experience. + It is also capable of determining our section and vertical if they're not present. Fully + specifying it all is preferable, though, as the micro-frontend can save itself some work, + resulting in a better user experience. We're building a URL like this: @@ -86,7 +84,8 @@ def get_microfrontend_redirect_url(course_key, path=None): # - chapter # - sequence # - vertical - # We skip course because we already have it from our argument above, and we skip chapter because the micro-frontend URL doesn't include it. + # We skip course because we already have it from our argument above, and we skip chapter + # because the micro-frontend URL doesn't include it. if len(path) > 2: redirect_url += '/{sequence_key}'.format( sequence_key=path[2]