From a7d78c53967937437a23136ccf792fd1287745f4 Mon Sep 17 00:00:00 2001 From: Jose Ignacio Palma Date: Thu, 18 Jul 2024 20:56:59 -0400 Subject: [PATCH 1/2] feat: adds CourseAboutPageURLRequested and LMSPageURLRequested filters --- .../contentstore/asset_storage_handlers.py | 11 ++- .../contentstore/tests/test_filters.py | 98 +++++++++++++++++++ common/djangoapps/util/course.py | 8 ++ common/djangoapps/util/tests/test_filters.py | 77 +++++++++++++++ requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 8 files changed, 197 insertions(+), 5 deletions(-) create mode 100644 cms/djangoapps/contentstore/tests/test_filters.py create mode 100644 common/djangoapps/util/tests/test_filters.py diff --git a/cms/djangoapps/contentstore/asset_storage_handlers.py b/cms/djangoapps/contentstore/asset_storage_handlers.py index 281258e03a8d..97fcb3125745 100644 --- a/cms/djangoapps/contentstore/asset_storage_handlers.py +++ b/cms/djangoapps/contentstore/asset_storage_handlers.py @@ -25,6 +25,7 @@ from common.djangoapps.util.json_request import JsonResponse from openedx.core.djangoapps.contentserver.caching import del_cached_content from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from openedx_filters.course_authoring.filters import LMSPageURLRequested from xmodule.contentstore.content import StaticContent # lint-amnesty, pylint: disable=wrong-import-order from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.exceptions import NotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -714,7 +715,15 @@ def get_asset_json(display_name, content_type, date, location, thumbnail_locatio Helper method for formatting the asset information to send to client. ''' asset_url = StaticContent.serialize_asset_key_with_slash(location) - external_url = urljoin(configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL), asset_url) + + ## .. filter_implemented_name: LMSPageURLRequested + ## .. filter_type: org.openedx.course_authoring.lms.page.url.requested.v1 + lms_root, _ = LMSPageURLRequested.run_filter( + url=configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL), + org=location.org, + ) + + external_url = urljoin(lms_root, asset_url) portable_url = StaticContent.get_static_path_from_location(location) usage_locations = [] if usage is None else usage return { diff --git a/cms/djangoapps/contentstore/tests/test_filters.py b/cms/djangoapps/contentstore/tests/test_filters.py new file mode 100644 index 000000000000..79a6bf8f8216 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_filters.py @@ -0,0 +1,98 @@ +""" +Unit tests for the asset upload endpoint. +""" +from datetime import datetime +from urllib.parse import urljoin + +from pytz import UTC + +from django.test import override_settings +from cms.djangoapps.contentstore import asset_storage_handlers +from opaque_keys.edx.locator import CourseLocator +from openedx_filters import PipelineStep +from xmodule.contentstore.content import StaticContent +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +class TestPageURLRequestedPipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, url, org): # pylint: disable=arguments-differ + """Pipeline step that modifies lms url creation.""" + url = "https://lms-url-creation" + org = "org" + return { + "url": url, + "org": org, + } + + +class LMSPageURLRequestedFiltersTest(ModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the lms url requested process. + This class guarantees that the following filters are triggered during the microsite render: + - LMSPageURLRequested + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.upload_date = datetime(2013, 6, 1, 10, 30, tzinfo=UTC) + self.content_type = 'image/jpg' + self.course_key = CourseLocator('org', 'class', 'run') + self.location = self.course_key.make_asset_key('asset', 'my_file_name.jpg') + self.thumbnail_location = self.course_key.make_asset_key('thumbnail', 'my_file_name_thumb.jpg') + + self.asset_url = StaticContent.serialize_asset_key_with_slash(self.location) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.course_authoring.lms.page.url.requested.v1": { + "pipeline": [ + "common.djangoapps.util.tests.test_filters.TestPageURLRequestedPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_lms_url_requested_filter_executed(self): + """ + Test whether the lms url requested filter is triggered before the user's + render site process. + Expected result: + - LMSPageURLRequested is triggered and executes TestPageURLRequestedPipelineStep. + - The arguments that the receiver gets are the arguments used by the filter. + """ + output = asset_storage_handlers.get_asset_json( + "my_file", + self.content_type, + self.upload_date, + self.location, + self.thumbnail_location, + True, + self.course_key + ) + + self.assertEqual(output.get('external_url'), urljoin('https://lms-url-creation', self.asset_url)) + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}, LMS_ROOT_URL="https://lms-base") + def test_lms_url_requested_without_filter_configuration(self): + """ + Test usual get link for about page process, without filter's intervention. + Expected result: + - Returns the course sharing url, this can be one of course's social sharing url, marketing url, or + lms course about url. + - The get process ends successfully. + """ + output = asset_storage_handlers.get_asset_json( + "my_file", + self.content_type, + self.upload_date, + self.location, + self.thumbnail_location, + True, + self.course_key + ) + + self.assertEqual(output.get('external_url'), urljoin('https://lms-base', self.asset_url)) diff --git a/common/djangoapps/util/course.py b/common/djangoapps/util/course.py index 721410ff6976..fa82f9c4aaeb 100644 --- a/common/djangoapps/util/course.py +++ b/common/djangoapps/util/course.py @@ -10,6 +10,7 @@ from opaque_keys.edx.keys import CourseKey, UsageKey from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from openedx_filters.learning.filters import CourseAboutPageURLRequested log = logging.getLogger(__name__) @@ -59,6 +60,13 @@ def get_link_for_about_page(course): course_key=str(course.id), ) + ## .. filter_implemented_name: CourseAboutPageURLRequested + ## .. filter_type: org.openedx.learning.course_about.page.url.requested.v1 + course_about_url, _ = CourseAboutPageURLRequested.run_filter( + url=course_about_url, + org=course.id.org, + ) + return course_about_url diff --git a/common/djangoapps/util/tests/test_filters.py b/common/djangoapps/util/tests/test_filters.py new file mode 100644 index 000000000000..55a47744c0d6 --- /dev/null +++ b/common/djangoapps/util/tests/test_filters.py @@ -0,0 +1,77 @@ +""" +Test that various filters are fired for models/views in the student app. +""" +from django.test import override_settings +from common.djangoapps.util import course +from openedx_filters import PipelineStep +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms + + +class TestPageURLRequestedPipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, url, org): # pylint: disable=arguments-differ + """Pipeline step that modifies lms url requested.""" + url = "https://lms-url-creation" + org = "org" + return { + "url": url, + "org": org, + } + + +@skip_unless_lms +class CourseAboutPageURLRequestedFiltersTest(ModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the lms url creation process. + This class guarantees that the following filters are triggered during the microsite render: + - CourseAboutPageURLRequested + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.course = CourseFactory.create() + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course_about.page.url.requested.v1": { + "pipeline": [ + "common.djangoapps.util.tests.test_filters.TestPageURLRequestedPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_course_about_page_url_requested_filter_executed(self): + """ + Test whether the lms url requested filter is triggered before the user's + render site process. + Expected result: + - CourseAboutPageURLRequested is triggered and executes TestPageURLRequestedPipelineStep. + - The arguments that the receiver gets are the arguments used by the filter. + """ + course_about_url = course.get_link_for_about_page(self.course) + + self.assertEqual("https://lms-url-creation", course_about_url) + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}, LMS_ROOT_URL="https://lms-base") + def test_course_about_page_url_requested_without_filter_configuration(self): + """ + Test usual get link for about page process, without filter's intervention. + Expected result: + - Returns the course sharing url, this can be one of course's social sharing url, marketing url, or + lms course about url. + - The get process ends successfully. + """ + course_about_url = course.get_link_for_about_page(self.course) + + expected_course_about = '{about_base_url}/courses/{course_key}/about'.format( + about_base_url='https://lms-base', + course_key=str(self.course.id), + ) + + self.assertEqual(expected_course_about, course_about_url) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 0eca9a532abc..ed00d674846d 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -820,7 +820,7 @@ openedx-events==9.14.1 # edx-name-affirmation # event-tracking # ora2 -openedx-filters==1.10.0 +openedx-filters==1.11.0 # via # -r requirements/edx/kernel.in # lti-consumer-xblock diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index abd99222532b..473e35a048a8 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1368,7 +1368,7 @@ openedx-events==9.14.1 # edx-name-affirmation # event-tracking # ora2 -openedx-filters==1.10.0 +openedx-filters==1.11.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index f8b45c5ddcc1..2a82c7d411d8 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -979,7 +979,7 @@ openedx-events==9.14.1 # edx-name-affirmation # event-tracking # ora2 -openedx-filters==1.10.0 +openedx-filters==1.11.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 7fa8962c4fd0..72605f2f8770 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1030,7 +1030,7 @@ openedx-events==9.14.1 # edx-name-affirmation # event-tracking # ora2 -openedx-filters==1.10.0 +openedx-filters==1.11.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock From 98159aa33e77fb9df7ca1c0d4238f4b0af4b0ece Mon Sep 17 00:00:00 2001 From: Jose Ignacio Palma Date: Tue, 8 Oct 2024 12:25:57 -0400 Subject: [PATCH 2/2] fix: feedback --- cms/djangoapps/contentstore/tests/test_filters.py | 10 +++++----- common/djangoapps/util/tests/test_filters.py | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_filters.py b/cms/djangoapps/contentstore/tests/test_filters.py index 79a6bf8f8216..13bdfa07473e 100644 --- a/cms/djangoapps/contentstore/tests/test_filters.py +++ b/cms/djangoapps/contentstore/tests/test_filters.py @@ -58,8 +58,8 @@ def setUp(self): # pylint: disable=arguments-differ ) def test_lms_url_requested_filter_executed(self): """ - Test whether the lms url requested filter is triggered before the user's - render site process. + Test that filter get new LMS URL for asset URL generation + based on the course organization settings for org. Expected result: - LMSPageURLRequested is triggered and executes TestPageURLRequestedPipelineStep. - The arguments that the receiver gets are the arguments used by the filter. @@ -79,10 +79,10 @@ def test_lms_url_requested_filter_executed(self): @override_settings(OPEN_EDX_FILTERS_CONFIG={}, LMS_ROOT_URL="https://lms-base") def test_lms_url_requested_without_filter_configuration(self): """ - Test usual get link for about page process, without filter's intervention. + Test that filter get new LMS URL for asset URL generation + based on LMS_ROOT_URL settings because OPEN_EDX_FILTERS_CONFIG is not set. Expected result: - - Returns the course sharing url, this can be one of course's social sharing url, marketing url, or - lms course about url. + - Returns the asset URL with domain base LMS_ROOT_URL. - The get process ends successfully. """ output = asset_storage_handlers.get_asset_json( diff --git a/common/djangoapps/util/tests/test_filters.py b/common/djangoapps/util/tests/test_filters.py index 55a47744c0d6..9e8d6d5fdffa 100644 --- a/common/djangoapps/util/tests/test_filters.py +++ b/common/djangoapps/util/tests/test_filters.py @@ -27,7 +27,7 @@ def run_filter(self, url, org): # pylint: disable=arguments-differ @skip_unless_lms class CourseAboutPageURLRequestedFiltersTest(ModuleStoreTestCase): """ - Tests for the Open edX Filters associated with the lms url creation process. + Tests for the Open edX Filters associated with the course about page url requested. This class guarantees that the following filters are triggered during the microsite render: - CourseAboutPageURLRequested """ @@ -48,8 +48,8 @@ def setUp(self): # pylint: disable=arguments-differ ) def test_course_about_page_url_requested_filter_executed(self): """ - Test whether the lms url requested filter is triggered before the user's - render site process. + Test that filter get new course about URL based + on the course organization settings for org. Expected result: - CourseAboutPageURLRequested is triggered and executes TestPageURLRequestedPipelineStep. - The arguments that the receiver gets are the arguments used by the filter. @@ -61,10 +61,10 @@ def test_course_about_page_url_requested_filter_executed(self): @override_settings(OPEN_EDX_FILTERS_CONFIG={}, LMS_ROOT_URL="https://lms-base") def test_course_about_page_url_requested_without_filter_configuration(self): """ - Test usual get link for about page process, without filter's intervention. + Test that filter get new course about URL based + on the LMS_ROOT_URL settings because OPEN_EDX_FILTERS_CONFIG is not set. Expected result: - - Returns the course sharing url, this can be one of course's social sharing url, marketing url, or - lms course about url. + - Returns the course about URL with domain base LMS_ROOT_URL. - The get process ends successfully. """ course_about_url = course.get_link_for_about_page(self.course)