From e8679056e47798146470c2a35aeb99bfdc2228bb Mon Sep 17 00:00:00 2001 From: Justin Hynes Date: Mon, 30 Jan 2023 16:12:25 -0500 Subject: [PATCH] fix: remove learner record MFE URL building logic [APER-2240] The monolith no longer needs to understand how to build URLs to the Learner Record MFE. The Credentials IDA has logic to determine if (and how) a request should be redirected to the MFE so we can remove these changes. Another PR later will remove the unused settings. --- .../credentials/tests/test_utils.py | 67 +------------------ openedx/core/djangoapps/credentials/utils.py | 29 +++----- 2 files changed, 11 insertions(+), 85 deletions(-) diff --git a/openedx/core/djangoapps/credentials/tests/test_utils.py b/openedx/core/djangoapps/credentials/tests/test_utils.py index 49a26e2ac8..4ab15091ff 100644 --- a/openedx/core/djangoapps/credentials/tests/test_utils.py +++ b/openedx/core/djangoapps/credentials/tests/test_utils.py @@ -2,10 +2,6 @@ import uuid from unittest import mock -from django.test import override_settings -from edx_toggles.toggles.testutils import override_waffle_switch - -from openedx.core.djangoapps.credentials.config import USE_LEARNER_RECORD_MFE from openedx.core.djangoapps.credentials.models import CredentialsApiConfig from openedx.core.djangoapps.credentials.tests import factories from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin @@ -93,69 +89,12 @@ class TestGetCredentials(CredentialsApiConfigMixin, CacheIsolationTestCase): } assert kwargs['querystring'] == querystring - @override_settings(LEARNER_RECORD_MICROFRONTEND_URL=None) - @override_settings(CREDENTIALS_PUBLIC_SERVICE_URL="http://foo") def test_get_credentials_records_url(self): """ - A test that verifies the functionality of the `get_credentials_records_url`.  + A test that verifies the functionality of the `get_credentials_records_url`. """ result = get_credentials_records_url() - assert result == "http://foo/records/" + assert result == "https://credentials.example.com/records/" result = get_credentials_records_url("abcdefgh-ijkl-mnop-qrst-uvwxyz123456") - assert result == "http://foo/records/programs/abcdefghijklmnopqrstuvwxyz123456/" - - @override_settings(LEARNER_RECORD_MICROFRONTEND_URL="http://blah") - @override_settings(CREDENTIALS_PUBLIC_SERVICE_URL="http://foo") - @override_waffle_switch(USE_LEARNER_RECORD_MFE, False) - def test_get_credentials_records_mfe_url_waffle_disabled(self): - """ - A test that verifies the results of the `get_credentials_records_url` function when the - LEARNER_RECORD_MICROFRONTEND_URL setting exists but the USE_LEARNER_RECORD_MFE waffle flag is disabled. - """ - result = get_credentials_records_url() - assert result == "http://foo/records/" - - result = get_credentials_records_url("abcdefgh-ijkl-mnop-qrst-uvwxyz123456") - assert result == "http://foo/records/programs/abcdefghijklmnopqrstuvwxyz123456/" - - @override_settings(LEARNER_RECORD_MICROFRONTEND_URL="http://blah") - @override_settings(CREDENTIALS_PUBLIC_SERVICE_URL="http://foo") - @override_waffle_switch(USE_LEARNER_RECORD_MFE, True) - def test_get_credentials_records_mfe_url_waffle_enabled(self): - """ - A test that verifies the results of the `get_credentials_records_url` function when the - LEARNER_RECORD_MICROFRONTEND_URL setting exists but the USE_LEARNER_RECORD_MFE waffle flag is enabled. - """ - result = get_credentials_records_url() - assert result == "http://blah/" - - result = get_credentials_records_url("abcdefgh-ijkl-mnop-qrst-uvwxyz123456") - assert result == "http://blah/abcdefghijklmnopqrstuvwxyz123456/" - - @override_settings(CREDENTIALS_PUBLIC_SERVICE_URL=None) - @override_settings(LEARNER_RECORD_MICROFRONTEND_URL=None) - def test_get_credentials_records_url_expect_none(self): - """ - A test that verifieis the results of the `get_credentials_records_url` function when the system is configured - to use neither the Credentials IDA or the Learner Record MFE. - """ - result = get_credentials_records_url() - assert result is None - - result = get_credentials_records_url("abcdefgh-ijkl-mnop-qrst-uvwxyz123456") - assert result is None - - @override_settings(LEARNER_RECORD_MICROFRONTEND_URL="http://blah") - @override_settings(CREDENTIALS_PUBLIC_SERVICE_URL=None) - @override_waffle_switch(USE_LEARNER_RECORD_MFE, True) - def test_get_credentials_records_url_only_mfe_configured(self): - """ - A test that verifieis the results of the `get_credentials_records_url` function when the system is configured - to use only the Learner Record MFE. - """ - result = get_credentials_records_url() - assert result == "http://blah/" - - result = get_credentials_records_url("abcdefgh-ijkl-mnop-qrst-uvwxyz123456") - assert result == "http://blah/abcdefghijklmnopqrstuvwxyz123456/" + assert result == "https://credentials.example.com/records/programs/abcdefghijklmnopqrstuvwxyz123456" diff --git a/openedx/core/djangoapps/credentials/utils.py b/openedx/core/djangoapps/credentials/utils.py index 752d41bc4b..107555e04d 100644 --- a/openedx/core/djangoapps/credentials/utils.py +++ b/openedx/core/djangoapps/credentials/utils.py @@ -1,10 +1,8 @@ """Helper functions for working with Credentials.""" import requests from edx_rest_api_client.auth import SuppliedJwtAuth +from urllib.parse import urljoin -from django.conf import settings - -from openedx.core.djangoapps.credentials.config import USE_LEARNER_RECORD_MFE from openedx.core.djangoapps.credentials.models import CredentialsApiConfig from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user from openedx.core.lib.edx_api_utils import get_api_data @@ -18,27 +16,16 @@ def get_credentials_records_url(program_uuid=None): Arguments: program_uuid (str): Optional program uuid to link for a program records URL """ - base_url = settings.CREDENTIALS_PUBLIC_SERVICE_URL - learner_record_mfe_base_url = settings.LEARNER_RECORD_MICROFRONTEND_URL - use_learner_record_mfe = USE_LEARNER_RECORD_MFE.is_enabled() and learner_record_mfe_base_url - - if not base_url and not use_learner_record_mfe: + base_url = CredentialsApiConfig.current().public_records_url + if base_url is None: return None - # If we have a program uuid we build a link to the appropriate Program Record page in Credentials (or the Learner - # Record MFE) if program_uuid: - # Credentials expects the UUID without dashes so we strip them here - stripped_program_uuid = program_uuid.replace('-', '') - if use_learner_record_mfe: - return f"{learner_record_mfe_base_url}/{stripped_program_uuid}/" - return f"{base_url}/records/programs/{stripped_program_uuid}/" - else: - # Otherwise, build a link to the appropriate Learner Record index page - if use_learner_record_mfe: - return f"{learner_record_mfe_base_url}/" - else: - return f"{base_url}/records/" + # Credentials expects Program UUIDs without dashes so we remove them here + stripped_program_uuid = program_uuid.replace("-", "") + return urljoin(base_url, f"programs/{stripped_program_uuid}") + + return base_url def get_credentials_api_client(user):