diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 8239e2806ef..438ab7c8e9c 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -9,11 +9,13 @@ on: jobs: run-tests: - runs-on: [ self-hosted ] + runs-on: [ edx-platform-runner ] strategy: matrix: python-version: ['3.8'] - django-version: ["3.2"] + django-version: + - "pinned" + #- "4.0" shard_name: [ "lms-1", "lms-2", @@ -35,9 +37,11 @@ jobs: - name: sync directory owner run: sudo chown runner:runner -R .* - uses: actions/checkout@v2 - - name: start mongodb service + - name: start mongod server for tests run: | - sudo /etc/init.d/mongodb start + sudo mkdir -p /data/db + sudo chmod -R a+rw /data/db + mongod & - name: set top-level module name run: | @@ -55,7 +59,15 @@ jobs: run: | sudo pip install -r requirements/pip.txt sudo pip install -r requirements/edx/testing.txt - sudo pip install "django~=${{ matrix.django-version }}.0" + if [[ "${{ matrix.django-version }}" == "pinned" ]]; then + sudo pip install -r requirements/edx/django.txt + else + sudo pip install "django~=${{ matrix.django-version }}.0" + fi + + - name: list installed package versions + run: | + sudo pip freeze - name: get unit tests for shard run: | diff --git a/.github/workflows/verify-gha-unit-tests-count.yml b/.github/workflows/verify-gha-unit-tests-count.yml index 38df28180fb..4f9aafbdde2 100644 --- a/.github/workflows/verify-gha-unit-tests-count.yml +++ b/.github/workflows/verify-gha-unit-tests-count.yml @@ -8,7 +8,7 @@ on: jobs: collect-and-verify: - runs-on: [ self-hosted ] + runs-on: [ edx-platform-runner ] steps: - name: sync directory owner run: sudo chown runner:runner -R .* diff --git a/cms/djangoapps/contentstore/courseware_index.py b/cms/djangoapps/contentstore/courseware_index.py index 5d9b627ec02..1878e04b3b0 100644 --- a/cms/djangoapps/contentstore/courseware_index.py +++ b/cms/djangoapps/contentstore/courseware_index.py @@ -587,6 +587,8 @@ class CourseAboutSearchIndexer(CoursewareSearchIndexer): AboutInfo("org", AboutInfo.PROPERTY, AboutInfo.FROM_COURSE_PROPERTY), AboutInfo("modes", AboutInfo.PROPERTY, AboutInfo.FROM_COURSE_MODE), AboutInfo("language", AboutInfo.PROPERTY, AboutInfo.FROM_COURSE_PROPERTY), + AboutInfo("invitation_only", AboutInfo.PROPERTY, AboutInfo.FROM_COURSE_PROPERTY), + AboutInfo("catalog_visibility", AboutInfo.PROPERTY, AboutInfo.FROM_COURSE_PROPERTY), ] @classmethod diff --git a/cms/djangoapps/contentstore/management/commands/export_content_library.py b/cms/djangoapps/contentstore/management/commands/export_content_library.py index c07526b35fd..5a53a17ec1d 100644 --- a/cms/djangoapps/contentstore/management/commands/export_content_library.py +++ b/cms/djangoapps/contentstore/management/commands/export_content_library.py @@ -61,6 +61,6 @@ def handle(self, *args, **options): filename = prefix + suffix target = os.path.join(dest_path, filename) tarball.file.seek(0) - with open(target, 'w') as f: + with open(target, 'wb') as f: shutil.copyfileobj(tarball.file, f) print(f'Library "{library.location.library_key}" exported to "{target}"') diff --git a/cms/djangoapps/contentstore/management/commands/import_content_library.py b/cms/djangoapps/contentstore/management/commands/import_content_library.py index 9d33523cdb5..9accd84a86d 100644 --- a/cms/djangoapps/contentstore/management/commands/import_content_library.py +++ b/cms/djangoapps/contentstore/management/commands/import_content_library.py @@ -43,13 +43,13 @@ def handle(self, *args, **options): username = options['owner_username'] data_root = Path(settings.GITHUB_REPO_ROOT) - subdir = base64.urlsafe_b64encode(os.path.basename(archive_path)) + subdir = base64.urlsafe_b64encode(os.path.basename(archive_path).encode('utf-8')).decode('utf-8') course_dir = data_root / subdir # Extract library archive tar_file = tarfile.open(archive_path) # lint-amnesty, pylint: disable=consider-using-with try: - safetar_extractall(tar_file, course_dir.encode('utf-8')) + safetar_extractall(tar_file, course_dir) except SuspiciousOperation as exc: raise CommandError(f'\n=== Course import {archive_path}: Unsafe tar file - {exc.args[0]}\n') # lint-amnesty, pylint: disable=raise-missing-from finally: diff --git a/cms/envs/common.py b/cms/envs/common.py index 974e99370a2..13c1375a70f 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -472,6 +472,18 @@ # .. toggle_target_removal_date: 2021-10-01 # .. toggle_tickets: 'https://openedx.atlassian.net/browse/MICROBA-1405' 'ENABLE_V2_CERT_DISPLAY_SETTINGS': False, + + # .. toggle_name: FEATURES['DISABLE_UNENROLLMENT'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Set to True to disable self-unenrollments via REST API. + # This also hides the "Unenroll" button on the Learner Dashboard. + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2021-10-11 + # .. toggle_warnings: For consistency in user experience, keep the value in sync with the setting of the same name + # in the LMS and CMS. + # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/429' + 'DISABLE_UNENROLLMENT': False, } ENABLE_JASMINE = False diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index a7c08277821..7d8f1431f80 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -261,3 +261,7 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing # Don't form the return redirect URL with HTTPS on devstack SOCIAL_AUTH_REDIRECT_IS_HTTPS = False + +#################### Network configuration #################### +# Devstack is directly exposed to the caller +CLOSEST_CLIENT_IP_FROM_HEADERS = [] diff --git a/cms/envs/production.py b/cms/envs/production.py index 0f9740cc4f9..58b4ab7fca6 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -74,6 +74,7 @@ def get_env_setting(setting): 'CELERY_QUEUES', 'MKTG_URL_LINK_MAP', 'MKTG_URL_OVERRIDES', + 'REST_FRAMEWORK', ] for key in KEYS_WITH_MERGED_VALUES: if key in __config_copy__: @@ -100,8 +101,8 @@ def get_env_setting(setting): BROKER_POOL_LIMIT = 0 BROKER_CONNECTION_TIMEOUT = 1 -# For the Result Store, use the django cache named 'celery' -CELERY_RESULT_BACKEND = 'django-cache' +# Allow env to configure celery result backend with default set to django-cache +CELERY_RESULT_BACKEND = ENV_TOKENS.get('CELERY_RESULT_BACKEND', 'django-cache') # When the broker is behind an ELB, use a heartbeat to refresh the # connection and to detect if it has been dropped. @@ -602,3 +603,6 @@ def get_env_setting(setting): SHOW_ACCOUNT_ACTIVATION_CTA = ENV_TOKENS.get('SHOW_ACCOUNT_ACTIVATION_CTA', SHOW_ACCOUNT_ACTIVATION_CTA) LANGUAGE_COOKIE_NAME = ENV_TOKENS.get('LANGUAGE_COOKIE', None) or ENV_TOKENS.get('LANGUAGE_COOKIE_NAME') + +############## DRF overrides ############## +REST_FRAMEWORK.update(ENV_TOKENS.get('REST_FRAMEWORK', {})) diff --git a/cms/envs/test.py b/cms/envs/test.py index 772be14a259..751af0ff4b3 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -337,3 +337,7 @@ ############### Settings for proctoring ############### PROCTORING_USER_OBFUSCATION_KEY = 'test_key' + +#################### Network configuration #################### +# Tests are not behind any proxies +CLOSEST_CLIENT_IP_FROM_HEADERS = [] diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index e47ccb21fe1..7e2bd1490e7 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -66,6 +66,7 @@ COURSE_ENROLLMENT_CREATED, COURSE_UNENROLLMENT_COMPLETED, ) +from openedx_filters.learning.filters import CourseEnrollmentStarted, CourseUnenrollmentStarted import openedx.core.djangoapps.django_comment_common.comment_client as cc from common.djangoapps.course_modes.models import CourseMode, get_cosmetic_verified_display_price from common.djangoapps.student.emails import send_proctoring_requirements_email @@ -1113,6 +1114,14 @@ class AlreadyEnrolledError(CourseEnrollmentException): pass +class EnrollmentNotAllowed(CourseEnrollmentException): + pass + + +class UnenrollmentNotAllowed(CourseEnrollmentException): + pass + + class CourseEnrollmentManager(models.Manager): """ Custom manager for CourseEnrollment with Table-level filter methods. @@ -1623,6 +1632,13 @@ def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=Fal Also emits relevant events for analytics purposes. """ + try: + user, course_key, mode = CourseEnrollmentStarted.run_filter( + user=user, course_key=course_key, mode=mode, + ) + except CourseEnrollmentStarted.PreventEnrollment as exc: + raise EnrollmentNotAllowed(str(exc)) from exc + if mode is None: mode = _default_course_mode(str(course_key)) # All the server-side checks for whether a user is allowed to enroll. @@ -1751,6 +1767,14 @@ def unenroll(cls, user, course_id, skip_refund=False): try: record = cls.objects.get(user=user, course_id=course_id) + + try: + # .. filter_implemented_name: CourseUnenrollmentStarted + # .. filter_type: org.openedx.learning.course.unenrollment.started.v1 + record = CourseUnenrollmentStarted.run_filter(enrollment=record) + except CourseUnenrollmentStarted.PreventUnenrollment as exc: + raise UnenrollmentNotAllowed(str(exc)) from exc + record.update_enrollment(is_active=False, skip_refund=skip_refund) except cls.DoesNotExist: diff --git a/common/djangoapps/student/tests/test_enrollment.py b/common/djangoapps/student/tests/test_enrollment.py index 5165ca57d2e..e22d1c9e973 100644 --- a/common/djangoapps/student/tests/test_enrollment.py +++ b/common/djangoapps/student/tests/test_enrollment.py @@ -358,6 +358,22 @@ def test_with_invalid_course_id(self): resp = self._change_enrollment('unenroll', course_id="edx/") assert resp.status_code == 400 + @patch.dict(settings.FEATURES, {'DISABLE_UNENROLLMENT': True}) + def test_unenroll_when_unenrollment_disabled(self): + """ + Tests that a user cannot unenroll when unenrollment has been disabled. + """ + # Enroll the student in the course + CourseEnrollment.enroll(self.user, self.course.id, mode="honor") + + # Attempt to unenroll + resp = self._change_enrollment('unenroll') + assert resp.status_code == 400 + + # Verify that user is still enrolled + is_enrolled = CourseEnrollment.is_enrolled(self.user, self.course.id) + assert is_enrolled + def test_enrollment_limit(self): """ Assert that in a course with max student limit set to 1, we can enroll staff and instructor along with diff --git a/common/djangoapps/student/tests/test_filters.py b/common/djangoapps/student/tests/test_filters.py new file mode 100644 index 00000000000..4f2da590856 --- /dev/null +++ b/common/djangoapps/student/tests/test_filters.py @@ -0,0 +1,465 @@ +""" +Test that various filters are fired for models/views in the student app. +""" +from django.http import HttpResponse +from django.test import override_settings +from django.urls import reverse +from openedx_filters import PipelineStep +from openedx_filters.learning.filters import DashboardRenderStarted, CourseEnrollmentStarted, CourseUnenrollmentStarted +from rest_framework import status +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from common.djangoapps.student.models import CourseEnrollment, EnrollmentNotAllowed, UnenrollmentNotAllowed +from common.djangoapps.student.tests.factories import UserFactory, UserProfileFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms + + +class TestEnrollmentPipelineStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, user, course_key, mode): # pylint: disable=arguments-differ, unused-argument + """Pipeline steps that changes mode to honor.""" + if mode == "no-id-professional": + raise CourseEnrollmentStarted.PreventEnrollment() + return {"mode": "honor"} + + +class TestUnenrollmentPipelineStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, enrollment): # pylint: disable=arguments-differ + """Pipeline steps that modifies user's profile before unenrolling.""" + if enrollment.mode == "no-id-professional": + raise CourseUnenrollmentStarted.PreventUnenrollment( + "You can't un-enroll from this site." + ) + + enrollment.user.profile.set_meta({"unenrolled_from": str(enrollment.course_id)}) + enrollment.user.profile.save() + return {} + + +class TestDashboardRenderPipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, template_name): # pylint: disable=arguments-differ + """Pipeline step that modifies dashboard data.""" + context["course_enrollments"] = [] + return { + "context": context, + "template_name": template_name, + } + + +class TestRenderInvalidDashboard(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, template_name): # pylint: disable=arguments-differ + """Pipeline step that stops the dashboard render process.""" + raise DashboardRenderStarted.RenderInvalidDashboard( + "You can't render this sites dashboard.", + dashboard_template="static_templates/server-error.html" + ) + + +class TestRedirectDashboardPageStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, template_name): # pylint: disable=arguments-differ + """Pipeline step that redirects before the dashboard is rendered.""" + raise DashboardRenderStarted.RedirectToPage( + "You can't see this site's dashboard, redirecting to the correct location.", + redirect_to="https://custom-dashboard.com", + ) + + +class TestRedirectToAccSettingsPage(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, template_name): # pylint: disable=arguments-differ + """Pipeline step that redirects to account settings before the dashboard is rendered.""" + raise DashboardRenderStarted.RedirectToPage( + "You can't see this site's dashboard, redirecting to the correct location.", + ) + + +class TestRenderCustomResponse(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, template_name): # pylint: disable=arguments-differ, unused-argument + """Pipeline step that changes dashboard view response before the dashboard is rendered.""" + response = HttpResponse("This is a custom response.") + raise DashboardRenderStarted.RenderCustomResponse( + "You can't see this site's dashboard.", + response=response, + ) + + +@skip_unless_lms +class EnrollmentFiltersTest(ModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the enrollment process through the enroll method. + + This class guarantees that the following filters are triggered during the user's enrollment: + + - CourseEnrollmentStarted + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.course = CourseFactory.create() + self.user = UserFactory.create( + username="test", + email="test@example.com", + password="password", + ) + self.user_profile = UserProfileFactory.create(user=self.user, name="Test Example") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course.enrollment.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestEnrollmentPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_enrollment_filter_executed(self): + """ + Test whether the student enrollment filter is triggered before the user's + enrollment process. + + Expected result: + - CourseEnrollmentStarted is triggered and executes TestEnrollmentPipelineStep. + - The arguments that the receiver gets are the arguments used by the filter + with the enrollment mode changed. + """ + enrollment = CourseEnrollment.enroll(self.user, self.course.id, mode='audit') + + self.assertEqual('honor', enrollment.mode) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course.enrollment.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestEnrollmentPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_enrollment_filter_prevent_enroll(self): + """ + Test prevent the user's enrollment through a pipeline step. + + Expected result: + - CourseEnrollmentStarted is triggered and executes TestEnrollmentPipelineStep. + - The user can't enroll. + """ + with self.assertRaises(EnrollmentNotAllowed): + CourseEnrollment.enroll(self.user, self.course.id, mode='no-id-professional') + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_enrollment_without_filter_configuration(self): + """ + Test usual enrollment process, without filter's intervention. + + Expected result: + - CourseEnrollmentStarted does not have any effect on the enrollment process. + - The enrollment process ends successfully. + """ + enrollment = CourseEnrollment.enroll(self.user, self.course.id, mode='audit') + + self.assertEqual('audit', enrollment.mode) + self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id)) + + +@skip_unless_lms +class UnenrollmentFiltersTest(ModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the unenrollment process through the unenroll method. + + This class guarantees that the following filters are triggered during the user's unenrollment: + + - CourseUnenrollmentStarted + """ + + USERNAME = "test" + EMAIL = "test@example.com" + PASSWORD = "password" + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.course = CourseFactory.create() + self.user = UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD) + self.client.login(username=self.USERNAME, password=self.PASSWORD) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course.unenrollment.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestUnenrollmentPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_unenrollment_filter_executed(self): + """ + Test whether the student unenrollment filter is triggered before the user's + unenrollment process. + + Expected result: + - CourseUnenrollmentStarted is triggered and executes TestUnenrollmentPipelineStep. + - The user's profile has unenrolled_from in its meta field. + """ + CourseEnrollment.enroll(self.user, self.course.id, mode="audit") + + CourseEnrollment.unenroll(self.user, self.course.id) + + self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course.unenrollment.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestUnenrollmentPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_unenrollment_filter_prevent_unenroll(self): + """ + Test prevent the user's unenrollment through a pipeline step. + + Expected result: + - CourseUnenrollmentStarted is triggered and executes TestUnenrollmentPipelineStep. + - The user can't unenroll. + """ + CourseEnrollment.enroll(self.user, self.course.id, mode="no-id-professional") + + with self.assertRaises(UnenrollmentNotAllowed): + CourseEnrollment.unenroll(self.user, self.course.id) + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_unenrollment_without_filter_configuration(self): + """ + Test usual unenrollment process without filter's intervention. + + Expected result: + - CourseUnenrollmentStarted does not have any effect on the unenrollment process. + - The unenrollment process ends successfully. + """ + CourseEnrollment.enroll(self.user, self.course.id, mode="audit") + + CourseEnrollment.unenroll(self.user, self.course.id) + + self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course.unenrollment.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestUnenrollmentPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_unenrollment_blocked_by_filter(self): + """ + Test cannot unenroll using change_enrollment view course when UnenrollmentNotAllowed is + raised by unenroll method. + + Expected result: + - CourseUnenrollmentStarted does not have any effect on the unenrollment process. + - The unenrollment process ends successfully. + """ + CourseEnrollment.enroll(self.user, self.course.id, mode="no-id-professional") + params = { + "enrollment_action": "unenroll", + "course_id": str(self.course.id) + } + + response = self.client.post(reverse("change_enrollment"), params) + + self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code) + self.assertEqual("You can't un-enroll from this site.", response.content.decode("utf-8")) + + +@skip_unless_lms +class StudentDashboardFiltersTest(ModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the dashboard rendering process. + + This class guarantees that the following filters are triggered during the students dashboard rendering: + - DashboardRenderStarted + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.user = UserFactory() + self.client.login(username=self.user.username, password="test") + self.dashboard_url = reverse("dashboard") + self.first_course = CourseFactory.create( + org="test1", course="course1", display_name="run1", + ) + self.second_course = CourseFactory.create( + org="test2", course="course2", display_name="run1", + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.dashboard.render.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestDashboardRenderPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_dashboard_render_filter_executed(self): + """ + Test whether the student dashboard filter is triggered before the user's + dashboard rendering process. + + Expected result: + - DashboardRenderStarted is triggered and executes TestDashboardRenderPipelineStep. + - The dashboard is rendered using the filtered enrollments list. + """ + CourseEnrollment.enroll(self.user, self.first_course.id) + CourseEnrollment.enroll(self.user, self.second_course.id) + + response = self.client.get(self.dashboard_url) + + self.assertNotContains(response, self.first_course.id) + self.assertNotContains(response, self.second_course.id) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.dashboard.render.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestRenderInvalidDashboard", + ], + "fail_silently": False, + }, + }, + PLATFORM_NAME="My site", + ) + def test_dashboard_render_invalid(self): + """ + Test rendering an invalid template after catching PreventDashboardRender exception. + + Expected result: + - DashboardRenderStarted is triggered and executes TestRenderInvalidDashboard. + - The server error template is rendered instead of the usual dashboard. + """ + response = self.client.get(self.dashboard_url) + + self.assertContains(response, "There has been a 500 error on the My site servers") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.dashboard.render.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestRedirectDashboardPageStep", + ], + "fail_silently": False, + }, + }, + ) + def test_dashboard_redirect(self): + """ + Test redirecting to a new page after catching RedirectDashboardPage exception. + + Expected result: + - DashboardRenderStarted is triggered and executes TestRedirectDashboardPageStep. + - The view response is a redirection. + - The redirection url is the custom dashboard specified in the filter. + """ + response = self.client.get(self.dashboard_url) + + self.assertEqual(status.HTTP_302_FOUND, response.status_code) + self.assertEqual("https://custom-dashboard.com", response.url) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.dashboard.render.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestRedirectToAccSettingsPage", + ], + "fail_silently": False, + }, + }, + ) + def test_dashboard_redirect_account_settings(self): + """ + Test redirecting to the account settings page after catching RedirectDashboardPage exception. + + Expected result: + - DashboardRenderStarted is triggered and executes TestRedirectToAccSettingsPage. + - The view response is a redirection. + - The redirection url is the account settings (as the default when not specifying one). + """ + response = self.client.get(self.dashboard_url) + + self.assertEqual(status.HTTP_302_FOUND, response.status_code) + self.assertEqual(reverse("account_settings"), response.url) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.dashboard.render.started.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestRenderCustomResponse", + ], + "fail_silently": False, + }, + }, + ) + def test_dashboard_custom_response(self): + """ + Test returning a custom response after catching RenderCustomResponse exception. + + Expected result: + - DashboardRenderStarted is triggered and executes TestRenderCustomResponse. + - The view response contains the custom response text. + """ + response = self.client.get(self.dashboard_url) + + self.assertEqual("This is a custom response.", response.content.decode("utf-8")) + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_dashboard_render_without_filter_config(self): + """ + Test whether the student dashboard filter is triggered before the user's + dashboard rendering process without any modification in the app flow. + + Expected result: + - DashboardRenderStarted executes a noop (empty pipeline). + - The view response is HTTP_200_OK. + - There's no modification in the dashboard. + """ + CourseEnrollment.enroll(self.user, self.first_course.id) + CourseEnrollment.enroll(self.user, self.second_course.id) + + response = self.client.get(self.dashboard_url) + + self.assertContains(response, self.first_course.id) + self.assertContains(response, self.second_course.id) diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index e78b34c7eaf..1b6c69bd1ba 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -10,6 +10,7 @@ from django.conf import settings from django.contrib import messages from django.contrib.auth.decorators import login_required +from django.http import HttpResponseRedirect from django.shortcuts import redirect from django.urls import reverse from django.utils.translation import gettext as _ @@ -18,6 +19,7 @@ from edx_django_utils.plugins import get_plugins_view_context from edx_toggles.toggles import LegacyWaffleFlag, LegacyWaffleFlagNamespace from opaque_keys.edx.keys import CourseKey +from openedx_filters.learning.filters import DashboardRenderStarted from pytz import UTC from lms.djangoapps.bulk_email.api import is_bulk_email_feature_enabled @@ -514,6 +516,10 @@ def student_dashboard(request): # lint-amnesty, pylint: disable=too-many-statem empty_dashboard_message = configuration_helpers.get_value( 'EMPTY_DASHBOARD_MESSAGE', None ) + disable_unenrollment = configuration_helpers.get_value( + 'DISABLE_UNENROLLMENT', + settings.FEATURES.get('DISABLE_UNENROLLMENT') + ) disable_course_limit = request and 'course_limit' in request.GET course_limit = get_dashboard_course_limit() if not disable_course_limit else None @@ -797,6 +803,7 @@ def student_dashboard(request): # lint-amnesty, pylint: disable=too-many-statem # TODO START: clean up as part of REVEM-199 (START) 'course_info': get_dashboard_course_info(user, course_enrollments), # TODO START: clean up as part of REVEM-199 (END) + 'disable_unenrollment': disable_unenrollment, } # Include enterprise learner portal metadata and messaging @@ -833,7 +840,22 @@ def student_dashboard(request): # lint-amnesty, pylint: disable=too-many-statem 'resume_button_urls': resume_button_urls }) - response = render_to_response('dashboard.html', context) + dashboard_template = 'dashboard.html' + try: + # .. filter_implemented_name: DashboardRenderStarted + # .. filter_type: org.openedx.learning.dashboard.render.started.v1 + context, dashboard_template = DashboardRenderStarted.run_filter( + context=context, template_name=dashboard_template, + ) + except DashboardRenderStarted.RenderInvalidDashboard as exc: + response = render_to_response(exc.dashboard_template, exc.template_context) + except DashboardRenderStarted.RedirectToPage as exc: + response = HttpResponseRedirect(exc.redirect_to or reverse('account_settings')) + except DashboardRenderStarted.RenderCustomResponse as exc: + response = exc.response + else: + response = render_to_response(dashboard_template, context) + if show_account_activation_popup: response.delete_cookie( settings.SHOW_ACTIVATE_CTA_POPUP_COOKIE_NAME, diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 9c1768a2f32..74a6d346d1b 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -18,7 +18,8 @@ from django.db import transaction from django.db.models.signals import post_save from django.dispatch import Signal, receiver # lint-amnesty, pylint: disable=unused-import -from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden +from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseRedirect + from django.shortcuts import redirect from django.template.context_processors import csrf from django.urls import reverse @@ -33,6 +34,8 @@ # Note that this lives in LMS, so this dependency should be refactored. from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey +from openedx_filters.exceptions import OpenEdxFilterException +from openedx_filters.tooling import OpenEdxPublicFilter from pytz import UTC from common.djangoapps.track import views as track_views @@ -64,6 +67,7 @@ PendingSecondaryEmailChange, Registration, RegistrationCookieConfiguration, + UnenrollmentNotAllowed, UserAttribute, UserProfile, UserSignupSource, @@ -108,6 +112,79 @@ def csrf_token(context): ' name="csrfmiddlewaretoken" value="{}" />').format(Text(token))) +class HomepageRenderStarted(OpenEdxPublicFilter): + """ + Custom class used to create homepage render filters and its custom methods. + """ + + filter_type = "org.openedx.learning.homepage.render.started.v1" + + class RedirectToPage(OpenEdxFilterException): + """ + Custom class used to stop the homepage rendering process. + """ + + def __init__(self, message, redirect_to=""): + """ + Override init that defines specific arguments used in the homepage render process. + + Arguments: + message: error message for the exception. + redirect_to: URL to redirect to. + """ + super().__init__(message, redirect_to=redirect_to) + + class RenderInvalidHomepage(OpenEdxFilterException): + """ + Custom class used to stop the homepage render process. + """ + + def __init__(self, message, index_template="", template_context=None): + """ + Override init that defines specific arguments used in the index render process. + + Arguments: + message: error message for the exception. + index_template: template path rendered instead. + template_context: context used to the new index_template. + """ + super().__init__( + message, + index_template=index_template, + template_context=template_context, + ) + + class RenderCustomResponse(OpenEdxFilterException): + """ + Custom class used to stop the homepage rendering process. + """ + + def __init__(self, message, response=None): + """ + Override init that defines specific arguments used in the homepage render process. + + Arguments: + message: error message for the exception. + response: custom response which will be returned by the homepage view. + """ + super().__init__( + message, + response=response, + ) + + @classmethod + def run_filter(cls, context, template_name): + """ + Execute a filter with the signature specified. + + Arguments: + context (dict): context dictionary for homepage template. + template_name (str): template name to be rendered by the homepage. + """ + data = super().run_pipeline(context=context, template_name=template_name) + return data.get("context"), data.get("template_name") + + # NOTE: This view is not linked to directly--it is called from # branding/views.py:index(), which is cached for anonymous users. # This means that it should always return the same thing for anon @@ -162,7 +239,23 @@ def index(request, extra_context=None, user=AnonymousUser()): # Add marketable programs to the context. context['programs_list'] = get_programs_with_type(request.site, include_hidden=False) - return render_to_response('index.html', context) + index_template = 'index.html' + try: + # .. filter_implemented_name: HomepageRenderStarted + # .. filter_type: org.openedx.learning.homepage.render.started.v1 + context, index_template = HomepageRenderStarted.run_filter( + context=context, template_name=index_template, + ) + except HomepageRenderStarted.RenderInvalidHomepage as exc: + response = render_to_response(exc.index_template, exc.template_context) # pylint: disable=no-member + except HomepageRenderStarted.RedirectToPage as exc: + response = HttpResponseRedirect(exc.redirect_to or reverse('account_settings')) + except HomepageRenderStarted.RenderCustomResponse as exc: + response = exc.response # pylint: disable=no-member + else: + response = render_to_response(index_template, context) + + return response def compose_activation_email(user, user_registration=None, route_enabled=False, profile_name='', redirect_url=None): @@ -397,6 +490,12 @@ def change_enrollment(request, check_access=True): # Otherwise, there is only one mode available (the default) return HttpResponse() elif action == "unenroll": + if configuration_helpers.get_value( + "DISABLE_UNENROLLMENT", + settings.FEATURES.get("DISABLE_UNENROLLMENT") + ): + return HttpResponseBadRequest(_("Unenrollment is currently disabled")) + enrollment = CourseEnrollment.get_enrollment(user, course_id) if not enrollment: return HttpResponseBadRequest(_("You are not enrolled in this course")) @@ -405,7 +504,11 @@ def change_enrollment(request, check_access=True): if certificate_info.get('status') in DISABLE_UNENROLL_CERT_STATES: return HttpResponseBadRequest(_("Your certificate prevents you from unenrolling from this course")) - CourseEnrollment.unenroll(user, course_id) + try: + CourseEnrollment.unenroll(user, course_id) + except UnenrollmentNotAllowed as exc: + return HttpResponseBadRequest(str(exc)) + REFUND_ORDER.send(sender=None, course_enrollment=enrollment) return HttpResponse() else: diff --git a/common/djangoapps/third_party_auth/saml_configuration/views.py b/common/djangoapps/third_party_auth/saml_configuration/views.py index 093717b2962..aa051aac7f9 100644 --- a/common/djangoapps/third_party_auth/saml_configuration/views.py +++ b/common/djangoapps/third_party_auth/saml_configuration/views.py @@ -16,7 +16,7 @@ class SAMLConfigurationMixin: serializer_class = SAMLConfigurationSerializer -class SAMLConfigurationViewSet(SAMLConfigurationMixin, viewsets.ModelViewSet): +class SAMLConfigurationViewSet(SAMLConfigurationMixin, viewsets.ReadOnlyModelViewSet): """ A View to handle SAMLConfiguration GETs diff --git a/common/djangoapps/third_party_auth/tests/test_views.py b/common/djangoapps/third_party_auth/tests/test_views.py index 3508a9d941b..10c64579ebb 100644 --- a/common/djangoapps/third_party_auth/tests/test_views.py +++ b/common/djangoapps/third_party_auth/tests/test_views.py @@ -4,17 +4,23 @@ import unittest +from unittest.mock import patch import ddt import pytest from django.conf import settings +from django.test import TestCase, override_settings +from django.test.client import RequestFactory from django.urls import reverse from lxml import etree from onelogin.saml2.errors import OneLogin_Saml2_Error +from common.djangoapps.student.models import Registration +from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.third_party_auth import pipeline # Define some XML namespaces: from common.djangoapps.third_party_auth.tasks import SAML_XML_NS +from common.djangoapps.third_party_auth.views import inactive_user_view from .testutil import AUTH_FEATURE_ENABLED, AUTH_FEATURES_KEY, SAMLTestCase @@ -196,3 +202,55 @@ def get_idp_redirect_url(provider_slug, next_destination=None): idp_redirect_url=reverse('idp_redirect', kwargs={'provider_slug': provider_slug}), next_destination=next_destination, ) + + +@unittest.skipUnless(AUTH_FEATURE_ENABLED, AUTH_FEATURES_KEY + ' not enabled') +class InactiveUserViewTests(TestCase): + """Test inactive user view """ + @patch('common.djangoapps.third_party_auth.views.redirect') + @override_settings(LOGIN_REDIRECT_WHITELIST=['courses.edx.org']) + def test_inactive_user_view_allows_valid_redirect(self, mock_redirect): + inactive_user = UserFactory(is_active=False) + Registration().register(inactive_user) + request = RequestFactory().get(settings.SOCIAL_AUTH_INACTIVE_USER_URL, {'next': 'https://courses.edx.org'}) + request.user = inactive_user + with patch('common.djangoapps.edxmako.request_context.get_current_request', return_value=request): + with patch('common.djangoapps.third_party_auth.pipeline.running', return_value=False): + inactive_user_view(request) + mock_redirect.assert_called_with('https://courses.edx.org') + + @patch('common.djangoapps.third_party_auth.views.redirect') + def test_inactive_user_view_prevents_invalid_redirect(self, mock_redirect): + inactive_user = UserFactory(is_active=False) + Registration().register(inactive_user) + request = RequestFactory().get(settings.SOCIAL_AUTH_INACTIVE_USER_URL, {'next': 'https://evil.com'}) + request.user = inactive_user + with patch('common.djangoapps.edxmako.request_context.get_current_request', return_value=request): + with patch('common.djangoapps.third_party_auth.pipeline.running', return_value=False): + inactive_user_view(request) + mock_redirect.assert_called_with('dashboard') + + @patch('common.djangoapps.third_party_auth.views.redirect') + def test_inactive_user_view_redirects_back_to_host(self, mock_redirect): + inactive_user = UserFactory(is_active=False) + Registration().register(inactive_user) + request = RequestFactory().get(settings.SOCIAL_AUTH_INACTIVE_USER_URL, {'next': 'https://myedxhost.com'}, + HTTP_HOST='myedxhost.com') + request.user = inactive_user + with patch('common.djangoapps.edxmako.request_context.get_current_request', return_value=request): + with patch('common.djangoapps.third_party_auth.pipeline.running', return_value=False): + inactive_user_view(request) + mock_redirect.assert_called_with('https://myedxhost.com') + + @patch('common.djangoapps.third_party_auth.views.redirect') + @override_settings(LOGIN_REDIRECT_WHITELIST=['courses.edx.org']) + def test_inactive_user_view_does_not_redirect_https_to_http(self, mock_redirect): + inactive_user = UserFactory(is_active=False) + Registration().register(inactive_user) + request = RequestFactory().get(settings.SOCIAL_AUTH_INACTIVE_USER_URL, {'next': 'http://courses.edx.org'}, + secure=True) + request.user = inactive_user + with patch('common.djangoapps.edxmako.request_context.get_current_request', return_value=request): + with patch('common.djangoapps.third_party_auth.pipeline.running', return_value=False): + inactive_user_view(request) + mock_redirect.assert_called_with('dashboard') diff --git a/common/djangoapps/third_party_auth/views.py b/common/djangoapps/third_party_auth/views.py index 3f35a48a326..c6a406c5301 100644 --- a/common/djangoapps/third_party_auth/views.py +++ b/common/djangoapps/third_party_auth/views.py @@ -14,7 +14,7 @@ from social_django.views import complete from common.djangoapps import third_party_auth -from common.djangoapps.student.helpers import get_next_url_for_login_page +from common.djangoapps.student.helpers import get_next_url_for_login_page, is_safe_login_or_logout_redirect from common.djangoapps.student.models import UserProfile from common.djangoapps.student.views import compose_and_send_activation_email from common.djangoapps.third_party_auth import pipeline, provider @@ -54,7 +54,15 @@ def inactive_user_view(request): if not activated: compose_and_send_activation_email(user, profile) - return redirect(request.GET.get('next', 'dashboard')) + request_params = request.GET + redirect_to = request_params.get('next') + + if redirect_to and is_safe_login_or_logout_redirect(redirect_to=redirect_to, request_host=request.get_host(), + dot_client_id=request_params.get('client_id'), + require_https=request.is_secure()): + return redirect(redirect_to) + + return redirect('dashboard') def saml_metadata_view(request): diff --git a/lms/djangoapps/certificates/apis/v0/tests/test_views.py b/lms/djangoapps/certificates/apis/v0/tests/test_views.py index 731aec06b11..9a56168f9ac 100644 --- a/lms/djangoapps/certificates/apis/v0/tests/test_views.py +++ b/lms/djangoapps/certificates/apis/v0/tests/test_views.py @@ -21,6 +21,7 @@ from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.user_api.tests.factories import UserPreferenceFactory from openedx.core.djangoapps.user_authn.tests.utils import JWT_AUTH_TYPES, AuthAndScopesTestMixin, AuthType +from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -308,7 +309,7 @@ def test_no_certificate(self): def test_query_counts(self): # Test student with no certificates student_no_cert = UserFactory.create(password=self.user_password) - with self.assertNumQueries(18): + with self.assertNumQueries(17, table_blacklist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, @@ -318,7 +319,7 @@ def test_query_counts(self): assert len(resp.data) == 0 # Test student with 1 certificate - with self.assertNumQueries(12): + with self.assertNumQueries(12, table_blacklist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, @@ -358,7 +359,7 @@ def test_query_counts(self): download_url='www.google.com', grade="0.88", ) - with self.assertNumQueries(12): + with self.assertNumQueries(12, table_blacklist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index 59ffd4729e4..669c870a8c6 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -7,6 +7,7 @@ """ import logging +from openedx_filters.learning.filters import CertificateCreationRequested from common.djangoapps.course_modes import api as modes_api from common.djangoapps.course_modes.models import CourseMode @@ -28,7 +29,15 @@ log = logging.getLogger(__name__) -def generate_certificate_task(user, course_key, generation_mode=None): +class GeneratedCertificateException(Exception): + pass + + +class CertificateGenerationNotAllowed(GeneratedCertificateException): + pass + + +def generate_certificate_task(user, course_key, generation_mode=None, _delay_seconds=CERTIFICATE_DELAY_SECONDS): """ Create a task to generate a certificate for this user in this course run, if the user is eligible and a certificate can be generated. @@ -52,8 +61,19 @@ def generate_allowlist_certificate_task(user, course_key, generation_mode=None): enrollment_mode = _get_enrollment_mode(user, course_key) course_grade = _get_course_grade(user, course_key) if _can_generate_allowlist_certificate(user, course_key, enrollment_mode): - return _generate_certificate_task(user=user, course_key=course_key, enrollment_mode=enrollment_mode, - course_grade=course_grade, generation_mode=generation_mode) + try: + return _generate_certificate_task( + user=user, course_key=course_key, enrollment_mode=enrollment_mode, course_grade=course_grade, + generation_mode=generation_mode, + ) + except CertificateGenerationNotAllowed: + # Catch exception to contain error message in console. + log.error( + "Certificate generation not allowed for user %s in course %s", + user.id, + course_key, + ) + return False status = _set_allowlist_cert_status(user, course_key, enrollment_mode, course_grade) if status is not None: @@ -88,6 +108,20 @@ def _generate_certificate_task(user, course_key, enrollment_mode, course_grade, course_grade_val = _get_grade_value(course_grade) + try: + # .. filter_implemented_name: CertificateCreationRequested + # .. filter_type: org.openedx.learning.certificate.creation.requested.v1 + user, course_key, enrollment_mode, status, course_grade, generation_mode = CertificateCreationRequested.run_filter( # pylint: disable=line-too-long + user=user, + course_key=course_key, + mode=enrollment_mode, + status=status, + grade=course_grade, + generation_mode=generation_mode, + ) + except CertificateCreationRequested.PreventCertificateCreation as exc: + raise CertificateGenerationNotAllowed(str(exc)) from exc + kwargs = { 'student': str(user.id), 'course_key': str(course_key), diff --git a/lms/djangoapps/certificates/management/commands/cert_generation.py b/lms/djangoapps/certificates/management/commands/cert_generation.py index 122e16a6754..e80d2c413d4 100644 --- a/lms/djangoapps/certificates/management/commands/cert_generation.py +++ b/lms/djangoapps/certificates/management/commands/cert_generation.py @@ -9,6 +9,7 @@ from django.core.management.base import BaseCommand, CommandError from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey +from lms.djangoapps.certificates.generation_handler import CertificateGenerationNotAllowed from lms.djangoapps.certificates.generation_handler import generate_certificate_task from lms.djangoapps.certificates.models import CertificateGenerationCommandConfiguration @@ -96,4 +97,11 @@ def handle(self, *args, **options): user=user.id, course=course_key )) - generate_certificate_task(user, course_key) + try: + generate_certificate_task(user, course_key) + except CertificateGenerationNotAllowed as e: + log.exception( + "Certificate generation not allowed for user %s in course %s", + user.id, + course_key, + ) diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 983446275a6..ceb9b5804f1 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -465,8 +465,12 @@ def save(self, *args, **kwargs): # pylint: disable=signature-differs Credentials IDA. """ super().save(*args, **kwargs) + if self._meta.proxy: + sending_class = self._meta.proxy_for_model + else: + sending_class = self.__class__ COURSE_CERT_CHANGED.send_robust( - sender=self.__class__, + sender=sending_class, user=self.user, course_key=self.course_id, mode=self.mode, @@ -498,7 +502,7 @@ def save(self, *args, **kwargs): # pylint: disable=signature-differs if CertificateStatuses.is_passing_status(self.status): COURSE_CERT_AWARDED.send_robust( - sender=self.__class__, + sender=sending_class, user=self.user, course_key=self.course_id, mode=self.mode, diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index 7fdb77b9826..ecabad637be 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -11,6 +11,7 @@ from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.signals import ENROLLMENT_TRACK_UPDATED from lms.djangoapps.certificates.generation_handler import ( + CertificateGenerationNotAllowed, generate_allowlist_certificate_task, generate_certificate_task, is_on_certificate_allowlist @@ -79,7 +80,15 @@ def listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: disa return log.info(f'Attempt will be made to generate a course certificate for {user.id} : {course_id} as a passing grade ' f'was received.') - return generate_certificate_task(user, course_id) + try: + return generate_certificate_task(user, course_id) + except CertificateGenerationNotAllowed: + log.exception( + "Certificate generation not allowed for user %s in course %s", + str(user), + course_id, + ) + return False @receiver(COURSE_GRADE_NOW_FAILED, dispatch_uid="new_failing_learner") @@ -117,7 +126,14 @@ def _listen_for_id_verification_status_changed(sender, user, **kwargs): # pylin for enrollment in user_enrollments: log.info(f'Attempt will be made to generate a course certificate for {user.id} : {enrollment.course_id}. Id ' f'verification status is {expected_verification_status}') - generate_certificate_task(user, enrollment.course_id) + try: + generate_certificate_task(user, enrollment.course_id) + except CertificateGenerationNotAllowed: + log.exception( + "Certificate generation not allowed for user %s in course %s", + str(user), + enrollment.course_id, + ) @receiver(ENROLLMENT_TRACK_UPDATED) @@ -131,4 +147,12 @@ def _listen_for_enrollment_mode_change(sender, user, course_key, mode, **kwargs) if modes_api.is_eligible_for_certificate(mode): log.info(f'Attempt will be made to generate a course certificate for {user.id} : {course_key} since the ' f'enrollment mode is now {mode}.') - generate_certificate_task(user, course_key) + try: + return generate_certificate_task(user, course_key) + except CertificateGenerationNotAllowed: + log.exception( + "Certificate generation not allowed for user %s in course %s", + str(user), + course_key, + ) + return False diff --git a/lms/djangoapps/certificates/tests/test_filters.py b/lms/djangoapps/certificates/tests/test_filters.py new file mode 100644 index 00000000000..7527e3bc925 --- /dev/null +++ b/lms/djangoapps/certificates/tests/test_filters.py @@ -0,0 +1,379 @@ +""" +Test that various filters are fired for models in the certificates app. +""" +from unittest import mock + +from django.core.management import call_command +from django.test import override_settings +from django.urls import reverse +from openedx_filters import PipelineStep +from openedx_filters.learning.filters import CertificateCreationRequested +from rest_framework import status as status_code +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.roles import SupportStaffRole +from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory +from lms.djangoapps.certificates.generation_handler import ( + CertificateGenerationNotAllowed, + generate_allowlist_certificate_task, + generate_certificate_task +) +from lms.djangoapps.certificates.models import GeneratedCertificate +from lms.djangoapps.certificates.signals import ( + _listen_for_enrollment_mode_change, + _listen_for_id_verification_status_changed, + listen_for_passing_grade +) +from lms.djangoapps.certificates.tests.factories import CertificateAllowlistFactory +from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms + + +class TestCertificatePipelineStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, user, course_key, mode, status, grade, generation_mode): # pylint: disable=arguments-differ, unused-argument + """Pipeline steps that changes certificate mode from honor to no-id-professional.""" + if mode == 'honor': + return { + 'mode': 'no-id-professional', + } + return {} + + +class TestStopCertificateGenerationStep(PipelineStep): + """ + Utility function used when getting steps for pipeline. + """ + + def run_filter(self, user, course_key, mode, status, grade, generation_mode): # pylint: disable=arguments-differ, unused-argument + """Pipeline step that stops the certificate generation process.""" + raise CertificateCreationRequested.PreventCertificateCreation( + "You can't generate a certificate from this site." + ) + + +@mock.patch( + 'lms.djangoapps.certificates.generation_handler.has_html_certificates_enabled', mock.Mock(return_value=True), +) +@mock.patch('lms.djangoapps.certificates.generation_handler._is_passing_grade', mock.Mock(return_value=True)) +@skip_unless_lms +class CertificateFiltersTest(SharedModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the certificate generation process. + + This class guarantees that the following filters are triggered during the user's certificate generation: + + - CertificateCreationRequested + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.course_run = CourseFactory() + self.user = UserFactory.create( + username="somestudent", + first_name="Student", + last_name="Person", + email="robot@robot.org", + is_active=True, + password="password", + ) + self.grade = CourseGradeFactory().read(self.user, self.course_run) + self.enrollment = CourseEnrollmentFactory( + user=self.user, + course_id=self.course_run.id, + is_active=True, + mode=CourseMode.HONOR, + ) + self.client.login(username=self.user.username, password="password") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestCertificatePipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_certificate_creation_filter_executed(self): + """ + Test whether the student certificate filter is triggered before the user's + certificate creation process. + + Expected result: + - CertificateCreationRequested is triggered and executes TestCertificatePipelineStep. + - The certificate generates with no-id-professional mode instead of honor mode. + """ + cert_gen_task_created = generate_certificate_task( + self.user, self.course_run.id, generation_mode=CourseMode.HONOR, + ) + + certificate = GeneratedCertificate.objects.get( + user=self.user, + course_id=self.course_run.id, + ) + + self.assertTrue(cert_gen_task_created) + self.assertEqual(CourseMode.NO_ID_PROFESSIONAL_MODE, certificate.mode) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + def test_certificate_creation_filter_prevent_generation(self): + """ + Test prevent the user's certificate generation through a pipeline step. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + with self.assertRaises(CertificateGenerationNotAllowed): + generate_certificate_task( + self.user, self.course_run.id, generation_mode=CourseMode.HONOR, + ) + + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_certificate_generation_without_filter_configuration(self): + """ + Test usual certificate process, without filter's intervention. + + Expected result: + - CertificateCreationRequested does not have any effect on the certificate generation process. + - The certificate generation process ends successfully. + """ + cert_gen_task_created = generate_certificate_task( + self.user, self.course_run.id, generation_mode=CourseMode.HONOR, + ) + + certificate = GeneratedCertificate.objects.get( + user=self.user, + course_id=self.course_run.id, + ) + + self.assertTrue(cert_gen_task_created) + self.assertEqual(CourseMode.HONOR, certificate.mode) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + def test_generate_allowlist_certificate_fail(self): + """ + Test stop certificate process by raising a filter exception when the user is in the + allow list. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + CertificateAllowlistFactory.create(course_id=self.course_run.id, user=self.user) + + certificate_generated = generate_allowlist_certificate_task(self.user, self.course_run.id) + + self.assertFalse(certificate_generated) + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + def test_generate_certificate_command(self): + """ + Test stop certificate process through the Django command by raising a filter exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + with self.assertLogs(level="ERROR"): + call_command("cert_generation", "--u", self.user.id, "--c", self.course_run.id) + + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + @mock.patch("lms.djangoapps.certificates.api.auto_certificate_generation_enabled", mock.Mock(return_value=True)) + def test_listen_for_passing_grade(self): + """ + Test stop automatic certificate generation process by raising a filters exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + signal_result = listen_for_passing_grade(None, self.user, self.course_run.id) + + self.assertFalse(signal_result) + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + @mock.patch( + 'lms.djangoapps.verify_student.services.IDVerificationService.user_status', + mock.Mock(return_value={"status": "approved"}) + ) + @mock.patch("lms.djangoapps.certificates.api.auto_certificate_generation_enabled", mock.Mock(return_value=True)) + def test_listen_for_id_verification_status_changed(self): + """ + Test stop certificate generation process after the verification status changed by raising a filters exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + _listen_for_id_verification_status_changed(None, self.user) + + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + def test_listen_for_enrollment_mode_change(self): + """ + Test stop automatic certificate generation process by raising a filters exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The certificate is not generated. + """ + signal_result = _listen_for_enrollment_mode_change(None, self.user, self.course_run.id, CourseMode.HONOR) + + self.assertFalse(signal_result) + self.assertFalse( + GeneratedCertificate.objects.filter( + user=self.user, course_id=self.course_run.id, mode=CourseMode.HONOR, + ) + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + @mock.patch( + "lms.djangoapps.certificates.generation_handler._can_generate_regular_certificate", + mock.Mock(return_value=True), + ) + def test_generate_cert_support_view(self): + """ + Test stop automatic certificate generation process by raising a filters exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The view returns HTTP_400_BAD_REQUEST. + """ + SupportStaffRole().add_users(self.user) + url = reverse( + "certificates:regenerate_certificate_for_user", + ) + body = { + "course_key": str(self.course_run.id), + "username": self.user.username, + } + + response = self.client.post(url, body) + + self.assertEqual(status_code.HTTP_400_BAD_REQUEST, response.status_code) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.creation.requested.v1": { + "pipeline": [ + "lms.djangoapps.certificates.tests.test_filters.TestStopCertificateGenerationStep", + ], + "fail_silently": False, + }, + }, + ) + def test_generate_cert_progress_view(self): + """ + Test stop certificate generation from the progress view by raising a filters exception. + + Expected result: + - CertificateCreationRequested is triggered and executes TestStopCertificateGenerationStep. + - The view returns HTTP_400_BAD_REQUEST. + """ + url = reverse("generate_user_cert", kwargs={"course_id": str(self.course_run.id)}) + + response = self.client.post(url) + + self.assertContains( + response, + "You can't generate a certificate from this site.", + status_code=status_code.HTTP_400_BAD_REQUEST, + ) diff --git a/lms/djangoapps/certificates/views/support.py b/lms/djangoapps/certificates/views/support.py index 67a75f3fa5c..7aadf309195 100644 --- a/lms/djangoapps/certificates/views/support.py +++ b/lms/djangoapps/certificates/views/support.py @@ -22,6 +22,7 @@ from common.djangoapps.student.models import CourseEnrollment, User from common.djangoapps.util.json_request import JsonResponse from lms.djangoapps.certificates.api import generate_certificate_task, get_certificates_for_user +from lms.djangoapps.certificates.generation_handler import CertificateGenerationNotAllowed from lms.djangoapps.certificates.permissions import GENERATE_ALL_CERTIFICATES, VIEW_ALL_CERTIFICATES from lms.djangoapps.instructor_task.api import generate_certificates_for_students from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none @@ -202,6 +203,13 @@ def regenerate_certificate_for_user(request): # Attempt to regenerate certificates try: generate_certificate_task(user, course_key) + except CertificateGenerationNotAllowed as e: + log.exception( + "Certificate generation not allowed for user %s in course %s", + str(user), + course_key, + ) + return HttpResponseBadRequest(str(e)) except: # pylint: disable=bare-except # We are pessimistic about the kinds of errors that might get thrown by the # certificates API. This may be overkill, but we're logging everything so we can diff --git a/lms/djangoapps/certificates/views/tests/__init__.py b/lms/djangoapps/certificates/views/tests/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lms/djangoapps/certificates/views/tests/test_filters.py b/lms/djangoapps/certificates/views/tests/test_filters.py new file mode 100644 index 00000000000..e9b4a825cf0 --- /dev/null +++ b/lms/djangoapps/certificates/views/tests/test_filters.py @@ -0,0 +1,269 @@ +""" +Test that various filters are fired for views in the certificates app. +""" +from django.conf import settings +from django.http import HttpResponse +from django.test import override_settings +from openedx_filters import PipelineStep +from openedx_filters.learning.filters import CertificateRenderStarted +from rest_framework import status +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase + +from lms.djangoapps.certificates.models import CertificateTemplate +from lms.djangoapps.certificates.tests.test_webview_views import CommonCertificatesTestCase +from lms.djangoapps.certificates.utils import get_certificate_url +from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration +from openedx.core.djangolib.testing.utils import skip_unless_lms + +FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() +FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True + + +class TestStopCertificateRenderStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, custom_template): # pylint: disable=arguments-differ + """Pipeline step that stops the certificate render process.""" + raise CertificateRenderStarted.RenderAlternativeInvalidCertificate( + "You can't generate a certificate from this site.", + ) + + +class TestRedirectToPageStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, custom_template): # pylint: disable=arguments-differ + """Pipeline step that redirects to another page before rendering the certificate.""" + raise CertificateRenderStarted.RedirectToPage( + "You can't generate a certificate from this site, redirecting to the correct location.", + redirect_to="https://certificate.pdf", + ) + + +class TestRenderCustomResponse(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, custom_template): # pylint: disable=arguments-differ, unused-argument + """Pipeline step that returns a custom response when rendering the certificate.""" + response = HttpResponse("Here's the text of the web page.") + raise CertificateRenderStarted.RenderCustomResponse( + "You can't generate a certificate from this site.", + response=response, + ) + + +class TestCertificateRenderPipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, custom_template): # pylint: disable=arguments-differ, unused-argument + """ + Pipeline step that gets or creates a new custom template to render instead + of the original. + """ + custom_template = self._create_custom_template(mode='honor') + return {"custom_template": custom_template} + + def _create_custom_template(self, org_id=None, mode=None, course_key=None, language=None): + """ + Creates a custom certificate template entry in DB. + """ + template_html = """ + <%namespace name='static' file='static_content.html'/> + + + lang: ${LANGUAGE_CODE} + course name: ${accomplishment_copy_course_name} + mode: ${course_mode} + ${accomplishment_copy_course_description} + ${twitter_url} + + + + """ + template = CertificateTemplate( + name='custom template', + template=template_html, + organization_id=org_id, + course_key=course_key, + mode=mode, + is_active=True, + language=language + ) + template.save() + return template + + +@skip_unless_lms +class CertificateFiltersTest(CommonCertificatesTestCase, SharedModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the certificate rendering process. + + This class guarantees that the following filters are triggered during the user's certificate rendering: + + - CertificateRenderStarted + """ + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.render.started.v1": { + "pipeline": [ + "lms.djangoapps.certificates.views.tests.test_filters.TestCertificateRenderPipelineStep", + ], + "fail_silently": False, + }, + }, + FEATURES=FEATURES_WITH_CERTS_ENABLED, + ) + def test_certificate_render_filter_executed(self): + """ + Test whether the student certificate render filter is triggered before the user's + certificate rendering process. + + Expected result: + - CertificateRenderStarted is triggered and executes TestCertificateRenderPipelineStep. + - The certificate renders using the custom template. + """ + test_url = get_certificate_url( + user_id=self.user.id, + course_id=str(self.course.id), + uuid=self.cert.verify_uuid + ) + self._add_course_certificates(count=1, signatory_count=1, is_active=True) + + response = self.client.get(test_url) + + self.assertContains( + response, + '', + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.render.started.v1": { + "pipeline": [ + "lms.djangoapps.certificates.views.tests.test_filters.TestStopCertificateRenderStep", + ], + "fail_silently": False, + }, + }, + FEATURES=FEATURES_WITH_CERTS_ENABLED, + ) + def test_certificate_render_invalid(self): + """ + Test rendering an invalid template after catching RenderAlternativeInvalidCertificate exception. + + Expected result: + - CertificateRenderStarted is triggered and executes TestStopCertificateRenderStep. + - The invalid certificate template is rendered. + """ + test_url = get_certificate_url( + user_id=self.user.id, + course_id=str(self.course.id), + uuid=self.cert.verify_uuid + ) + self._add_course_certificates(count=1, signatory_count=1, is_active=True) + + response = self.client.get(test_url) + + self.assertContains(response, "Invalid Certificate") + self.assertContains(response, "Cannot Find Certificate") + self.assertContains(response, "We cannot find a certificate with this URL or ID number.") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.render.started.v1": { + "pipeline": [ + "lms.djangoapps.certificates.views.tests.test_filters.TestRedirectToPageStep", + ], + "fail_silently": False, + }, + }, + FEATURES=FEATURES_WITH_CERTS_ENABLED, + ) + def test_certificate_redirect(self): + """ + Test redirecting to a new page after catching RedirectToPage exception. + + Expected result: + - CertificateRenderStarted is triggered and executes TestRedirectToPageStep. + - The webview response is a redirection. + """ + test_url = get_certificate_url( + user_id=self.user.id, + course_id=str(self.course.id), + uuid=self.cert.verify_uuid + ) + self._add_course_certificates(count=1, signatory_count=1, is_active=True) + + response = self.client.get(test_url) + + self.assertEqual(status.HTTP_302_FOUND, response.status_code) + self.assertEqual("https://certificate.pdf", response.url) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.certificate.render.started.v1": { + "pipeline": [ + "lms.djangoapps.certificates.views.tests.test_filters.TestRenderCustomResponse", + ], + "fail_silently": False, + }, + }, + FEATURES=FEATURES_WITH_CERTS_ENABLED, + ) + def test_certificate_render_custom_response(self): + """ + Test rendering an invalid template after catching RenderCustomResponse exception. + + Expected result: + - CertificateRenderStarted is triggered and executes TestRenderCustomResponse. + - The custom response is found in the certificate. + """ + test_url = get_certificate_url( + user_id=self.user.id, + course_id=str(self.course.id), + uuid=self.cert.verify_uuid + ) + self._add_course_certificates(count=1, signatory_count=1, is_active=True) + + response = self.client.get(test_url) + + self.assertContains(response, "Here's the text of the web page.") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={}, + FEATURES=FEATURES_WITH_CERTS_ENABLED, + ) + @with_site_configuration( + configuration={ + 'platform_name': 'My Platform Site', + }, + ) + def test_certificate_render_without_filter_config(self): + """ + Test whether the student certificate filter is triggered before the user's + certificate rendering without affecting its execution flow. + + Expected result: + - CertificateRenderStarted executes a noop (empty pipeline). + - The webview response is HTTP_200_OK. + """ + test_url = get_certificate_url( + user_id=self.user.id, + course_id=str(self.course.id), + uuid=self.cert.verify_uuid + ) + self._add_course_certificates(count=1, signatory_count=1, is_active=True) + + response = self.client.get(test_url) + + self.assertEqual(status.HTTP_200_OK, response.status_code) + self.assertContains(response, "My Platform Site") diff --git a/lms/djangoapps/certificates/views/webview.py b/lms/djangoapps/certificates/views/webview.py index 285a65caecd..dd48c9fba23 100644 --- a/lms/djangoapps/certificates/views/webview.py +++ b/lms/djangoapps/certificates/views/webview.py @@ -11,13 +11,14 @@ import pytz from django.conf import settings from django.contrib.auth.decorators import login_required -from django.http import Http404, HttpResponse +from django.http import Http404, HttpResponse, HttpResponseRedirect from django.template import RequestContext from django.utils import translation from django.utils.encoding import smart_str from eventtracking import tracker from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey +from openedx_filters.learning.filters import CertificateRenderStarted from organizations import api as organizations_api from edx_django_utils.plugins import pluggable_override @@ -504,8 +505,8 @@ def render_cert_by_uuid(request, certificate_uuid): template_path="certificates/server-error.html", test_func=lambda request: request.GET.get('preview', None) ) -@pluggable_override('OVERRIDE_RENDER_CERTIFICATE_VIEW') -def render_html_view(request, course_id, certificate=None): +@pluggable_override('OVERRIDE_RENDER_CERTIFICATE_VIEW') # pylint: disable=too-many-statements +def render_html_view(request, course_id, certificate=None): # pylint: disable=too-many-statements """ This public view generates an HTML representation of the specified user and course If a certificate is not available, we display a "Sorry!" screen instead @@ -636,8 +637,30 @@ def render_html_view(request, course_id, certificate=None): # Track certificate view events _track_certificate_events(request, course, user, user_certificate) + try: + # .. filter_implemented_name: CertificateRenderStarted + # .. filter_type: org.openedx.learning.certificate.render.started.v1 + context, custom_template = CertificateRenderStarted.run_filter( + context=context, + custom_template=custom_template, + ) + except CertificateRenderStarted.RenderAlternativeInvalidCertificate as exc: + response = _render_invalid_certificate( + request, + course_id, + platform_name, + configuration, + cert_path=exc.template_name or INVALID_CERTIFICATE_TEMPLATE_PATH, + ) + except CertificateRenderStarted.RedirectToPage as exc: + response = HttpResponseRedirect(exc.redirect_to) + except CertificateRenderStarted.RenderCustomResponse as exc: + response = exc.response + else: + response = _render_valid_certificate(request, context, custom_template) + # Render the certificate - return _render_valid_certificate(request, context, custom_template) + return response def _get_catalog_data_for_course(course_key): diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index e2cb2521c20..5057de88f68 100644 --- a/lms/djangoapps/course_api/tests/test_views.py +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -23,6 +23,7 @@ from common.djangoapps.student.auth import add_users from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole from common.djangoapps.student.tests.factories import AdminFactory +from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from openedx.core.lib.api.view_utils import LazySequence from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.course_duration_limits.models import CourseDurationLimitConfig @@ -417,14 +418,14 @@ def test_too_many_courses(self): self.setup_user(self.audit_user) # These query counts were found empirically - query_counts = [54, 46, 46, 46, 46, 46, 46, 46, 46, 46, 16] + query_counts = [50, 46, 46, 46, 46, 46, 46, 46, 46, 46, 16] ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])]) self.clear_caches() for page in range(1, 12): RequestCache.clear_all_namespaces() - with self.assertNumQueries(query_counts[page - 1]): + with self.assertNumQueries(query_counts[page - 1], table_blacklist=WAFFLE_TABLES): response = self.verify_response(params={'page': page, 'page_size': 30}) assert 'results' in response.data diff --git a/lms/djangoapps/courseware/tests/test_comprehensive_theming.py b/lms/djangoapps/courseware/tests/test_comprehensive_theming.py index 43eb9663a44..0e3bcf6b16f 100644 --- a/lms/djangoapps/courseware/tests/test_comprehensive_theming.py +++ b/lms/djangoapps/courseware/tests/test_comprehensive_theming.py @@ -8,6 +8,8 @@ from common.djangoapps import edxmako from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_theme +from openedx.core.djangoapps.theming.helpers import get_themes +from openedx.core.djangoapps.theming.helpers_dirs import get_theme_dirs from openedx.core.lib.tempdir import create_symlink, delete_symlink, mkdtemp_clean @@ -20,6 +22,10 @@ def setUp(self): # Clear the internal staticfiles caches, to get test isolation. staticfiles.finders.get_finder.cache_clear() + # Clear cache on get_theme methods. + get_themes.cache_clear() + get_theme_dirs.cache_clear() + @with_comprehensive_theme('red-theme') def test_red_footer(self): """ diff --git a/lms/djangoapps/courseware/tests/test_filters.py b/lms/djangoapps/courseware/tests/test_filters.py new file mode 100644 index 00000000000..0159b12234a --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_filters.py @@ -0,0 +1,253 @@ +""" +Test that various filters are fired for courseware views. +""" +from django.http import HttpResponse +from django.test import override_settings +from django.urls import reverse +from openedx_filters import PipelineStep +from openedx_filters.learning.filters import CourseAboutRenderStarted +from rest_framework import status +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 TestRenderInvalidCourseAbout(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, template_name): # pylint: disable=arguments-differ + """ + Pipeline step that stops the course about render process. + + When raising PreventCourseAboutRender, this filter overrides the course about + template name so the view renders a module-error instead. + """ + raise CourseAboutRenderStarted.RenderInvalidCourseAbout( + "You can't access the courses about page.", + course_about_template="module-error.html", + template_context=context, + ) + + +class TestRedirectToPage(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, template_name): # pylint: disable=arguments-differ, unused-argument + """ + Pipeline step that redirects to the course survey. + + When raising RedirectToPage, this filter uses a redirect_to field handled by + the course about view that redirects to that URL. + """ + course_key = str(context.get("course").id) + raise CourseAboutRenderStarted.RedirectToPage( + "You can't access this courses about page, redirecting to the correct location.", + redirect_to=f"courses/{course_key}/survey", + ) + + +class TestRedirectToDefaultPage(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, template_name): # pylint: disable=arguments-differ, unused-argument + """ + Pipeline step that redirects to the default page when redirect_to is not specified. + + When raising RedirectToPage, this filter uses a redirect_to field handled by + the course about view that redirects to that URL. + """ + _course_key = str(context.get("course").id) + raise CourseAboutRenderStarted.RedirectToPage( + "You can't access this courses about page, redirecting to the correct location.", + ) + + +class TestRenderCustomResponse(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, template_name): # pylint: disable=arguments-differ, unused-argument + """ + Pipeline step that redirects to the course survey. + + When raising RenderCustomResponse, this filter uses a redirect_to field handled by + the course about view that redirects to that URL. + """ + response = HttpResponse("Here's the text of the web page.") + + raise CourseAboutRenderStarted.RenderCustomResponse( + "You can't access this courses home page.", + response=response, + ) + + +class TestCourseAboutRender(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, template_name): # pylint: disable=arguments-differ + """Pipeline that gives staff view to the current user.""" + context["staff_access"] = True + context["studio_url"] = "http://madeup-studio.com" + return { + "context": context, template_name: template_name, + } + + +@skip_unless_lms +class CourseAboutFiltersTest(ModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the course about rendering process. + + This class guarantees that the following filters are triggered during the course about rendering: + + - CourseAboutRenderStarted + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.course = CourseFactory.create() + self.course_about_url = reverse( + "about_course", + kwargs={ + "course_id": self.course.id, + } + ) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course_about.render.started.v1": { + "pipeline": [ + "lms.djangoapps.courseware.tests.test_filters.TestCourseAboutRender", + ], + "fail_silently": False, + }, + }, + ) + def test_course_about_render_filter_executed(self): + """ + Test whether the course about filter is triggered before the course about view + renders. + + Expected result: + - CourseAboutRenderStarted is triggered and executes TestCourseAboutRender. + - The course about renders with View About Page in studio. + """ + response = self.client.get(self.course_about_url) + + self.assertContains(response, "View About Page in studio", status_code=200) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course_about.render.started.v1": { + "pipeline": [ + "lms.djangoapps.courseware.tests.test_filters.TestRenderInvalidCourseAbout", + ], + "fail_silently": False, + }, + }, + PLATFORM_NAME="My site", + ) + def test_course_about_render_alternative(self): + """ + Test rendering an error template after catching PreventCourseAboutRender exception. + + Expected result: + - CourseAboutRenderStarted is triggered and executes TestRenderInvalidCourseAbout. + - The module-error template is rendered instead of the usual course about. + """ + response = self.client.get(self.course_about_url) + + self.assertContains(response, "There has been an error on the My site servers") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course_about.render.started.v1": { + "pipeline": [ + "lms.djangoapps.courseware.tests.test_filters.TestRedirectToPage", + ], + "fail_silently": False, + }, + }, + ) + def test_course_about_redirect(self): + """ + Test redirecting to a new page after catching RedirectCourseAboutPage exception. + + Expected result: + - CourseAboutRenderStarted is triggered and executes TestRedirectToPage. + - The view response is a redirection. + """ + response = self.client.get(self.course_about_url) + + self.assertEqual(status.HTTP_302_FOUND, response.status_code) + self.assertEqual(f"courses/{self.course.id}/survey", response.url) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course_about.render.started.v1": { + "pipeline": [ + "lms.djangoapps.courseware.tests.test_filters.TestRedirectToDefaultPage", + ], + "fail_silently": False, + }, + }, + ) + def test_course_about_redirect_default(self): + """ + Test redirecting to the default page after catching RedirectCourseAboutPage exception. + + Expected result: + - CourseAboutRenderStarted is triggered and executes TestRedirectToPage. + - The view response is a redirection. + """ + response = self.client.get(self.course_about_url) + + self.assertEqual(status.HTTP_302_FOUND, response.status_code) + self.assertEqual(f"{reverse('dashboard')}", response.url) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.course_about.render.started.v1": { + "pipeline": [ + "lms.djangoapps.courseware.tests.test_filters.TestRenderCustomResponse", + ], + "fail_silently": False, + }, + }, + ) + def test_course_about_custom_response(self): + """ + Test redirecting to a new page after catching RenderCustomResponse exception. + + Expected result: + - CourseAboutRenderStarted is triggered and executes TestRenderCustomResponse. + - The view response is a redirection. + """ + response = self.client.get(self.course_about_url) + + self.assertContains(response, "Here's the text of the web page.") + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_course_about_render_without_filter_config(self): + """ + Test whether the course about filter is triggered before the course about + render without affecting its execution flow. + + Expected result: + - CourseAboutRenderStarted executes a noop (empty pipeline). Without any + modification comparing it with the effects of TestCourseAboutRender. + - The view response is HTTP_200_OK. + """ + response = self.client.get(self.course_about_url) + + self.assertNotContains(response, "View About Page in studio", status_code=200) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 22774c9dfe6..4700c328aba 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -17,7 +17,7 @@ from django.core.exceptions import PermissionDenied from django.db import transaction from django.db.models import Q, prefetch_related_objects -from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden +from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseRedirect from django.shortcuts import redirect from django.template.context_processors import csrf from django.urls import reverse @@ -38,6 +38,9 @@ from markupsafe import escape from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey +from openedx_filters.learning.filters import CourseAboutRenderStarted +from openedx_filters.exceptions import OpenEdxFilterException +from openedx_filters.tooling import OpenEdxPublicFilter from pytz import UTC from requests.exceptions import ConnectionError, Timeout # pylint: disable=redefined-builtin from rest_framework import status @@ -53,6 +56,7 @@ from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException from lms.djangoapps.certificates import api as certs_api from lms.djangoapps.certificates.data import CertificateStatuses +from lms.djangoapps.certificates.generation_handler import CertificateGenerationNotAllowed from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.course_goals.models import UserActivity from lms.djangoapps.course_home_api.toggles import ( @@ -264,6 +268,79 @@ def user_groups(user): return group_names +class CatalogRenderStarted(OpenEdxPublicFilter): + """ + Custom class used to create catalog render filters and its custom methods. + """ + + filter_type = "org.openedx.learning.catalog.render.started.v1" + + class RedirectToPage(OpenEdxFilterException): + """ + Custom class used to stop the catalog rendering process. + """ + + def __init__(self, message, redirect_to=""): + """ + Override init that defines specific arguments used in the catalog render process. + + Arguments: + message: error message for the exception. + redirect_to: URL to redirect to. + """ + super().__init__(message, redirect_to=redirect_to) + + class RenderInvalidCatalog(OpenEdxFilterException): + """ + Custom class used to stop the catalog render process. + """ + + def __init__(self, message, courses_template="", template_context=None): + """ + Override init that defines specific arguments used in the index render process. + + Arguments: + message: error message for the exception. + courses_template: template path rendered instead. + template_context: context used to the new courses_template. + """ + super().__init__( + message, + courses_template=courses_template, + template_context=template_context, + ) + + class RenderCustomResponse(OpenEdxFilterException): + """ + Custom class used to stop the catalog rendering process. + """ + + def __init__(self, message, response=None): + """ + Override init that defines specific arguments used in the catalog render process. + + Arguments: + message: error message for the exception. + response: custom response which will be returned by the catalog view. + """ + super().__init__( + message, + response=response, + ) + + @classmethod + def run_filter(cls, context, template_name): + """ + Execute a filter with the signature specified. + + Arguments: + context (dict): context dictionary for catalog template. + template_name (str): template name to be rendered by the catalog. + """ + data = super().run_pipeline(context=context, template_name=template_name) + return data.get("context"), data.get("template_name") + + @ensure_csrf_cookie @cache_if_anonymous() def courses(request): @@ -284,14 +361,28 @@ def courses(request): # Add marketable programs to the context. programs_list = get_programs_with_type(request.site, include_hidden=False) - return render_to_response( - "courseware/courses.html", - { - 'courses': courses_list, - 'course_discovery_meanings': course_discovery_meanings, - 'programs_list': programs_list, - } - ) + courses_template = "courseware/courses.html" + context = { + 'courses': courses_list, + 'course_discovery_meanings': course_discovery_meanings, + 'programs_list': programs_list, + } + try: + # .. filter_implemented_name: CatalogRenderStarted + # .. filter_type: org.openedx.learning.catalog.render.started.v1 + context, courses_template = CatalogRenderStarted.run_filter( + context=context, template_name=courses_template, + ) + except CatalogRenderStarted.RenderInvalidCatalog as exc: + response = render_to_response(exc.courses_template, exc.template_context) # pylint: disable=no-member + except CatalogRenderStarted.RedirectToPage as exc: + response = HttpResponseRedirect(exc.redirect_to or reverse('account_settings')) + except CatalogRenderStarted.RenderCustomResponse as exc: + response = exc.response # pylint: disable=no-member + else: + response = render_to_response(courses_template, context) + + return response class PerUserVideoMetadataThrottle(UserRateThrottle): @@ -907,7 +998,7 @@ def post(self, request, course_id): @ensure_csrf_cookie @ensure_valid_course_key @cache_if_anonymous() -def course_about(request, course_id): +def course_about(request, course_id): # pylint: disable=too-many-statements """ Display the course's about page. """ @@ -1019,7 +1110,23 @@ def course_about(request, course_id): 'allow_anonymous': allow_anonymous, } - return render_to_response('courseware/course_about.html', context) + course_about_template = 'courseware/course_about.html' + try: + # .. filter_implemented_name: CourseAboutRenderStarted + # .. filter_type: org.openedx.learning.course_about.render.started.v1 + context, course_about_template = CourseAboutRenderStarted.run_filter( + context=context, template_name=course_about_template, + ) + except CourseAboutRenderStarted.RenderInvalidCourseAbout as exc: + response = render_to_response(exc.course_about_template, exc.template_context) + except CourseAboutRenderStarted.RedirectToPage as exc: + raise CourseAccessRedirect(exc.redirect_to or reverse('dashboard')) from exc + except CourseAboutRenderStarted.RenderCustomResponse as exc: + response = exc.response or render_to_response(course_about_template, context) + else: + response = render_to_response(course_about_template, context) + + return response @ensure_csrf_cookie @@ -1621,7 +1728,16 @@ def generate_user_cert(request, course_id): return HttpResponseBadRequest(_("Course is not valid")) log.info(f'Attempt will be made to generate a course certificate for {student.id} : {course_key}.') - certs_api.generate_certificate_task(student, course_key, 'self') + + try: + certs_api.generate_certificate_task(student, course_key, 'self') + except CertificateGenerationNotAllowed as e: + log.exception( + "Certificate generation not allowed for user %s in course %s", + str(student), + course_key, + ) + return HttpResponseBadRequest(str(e)) if not is_course_passed(student, course): log.info("User %s has not passed the course: %s", student.username, course_id) diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 14089d74ea5..a3f605bbd16 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -152,7 +152,7 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_ if previous_score is not None: prev_raw_earned, prev_raw_possible = (previous_score.grade, previous_score.max_grade) - if not is_score_higher_or_equal(prev_raw_earned, prev_raw_possible, raw_earned, raw_possible): + if not is_score_higher_or_equal(prev_raw_earned, prev_raw_possible, raw_earned, raw_possible, True): update_score = False log.warning( "Grades: Rescore is not higher than previous: " diff --git a/lms/envs/common.py b/lms/envs/common.py index 958d06ccbfd..40facc565b7 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -817,7 +817,7 @@ # .. toggle_tickets: https://openedx.atlassian.net/browse/OSPR-1880 'ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA': False, - # .. toggle_name: FEATURES['ENABLE_CHANGE_USER_PASSWORD_ADMIN'] + # .. toggle_name: FEATURES['ENABLE_PASSWORD_RESET_FAILURE_EMAIL'] # .. toggle_implementation: DjangoSetting # .. toggle_default: False # .. toggle_description: Whether to send an email for failed password reset attempts or not. This happens when a @@ -948,6 +948,18 @@ # .. toggle_target_removal_date: 2021-10-01 # .. toggle_tickets: 'https://openedx.atlassian.net/browse/MICROBA-1405' 'ENABLE_V2_CERT_DISPLAY_SETTINGS': False, + + # .. toggle_name: FEATURES['DISABLE_UNENROLLMENT'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Set to True to disable self-unenrollments via REST API. + # This also hides the "Unenroll" button on the Learner Dashboard. + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2021-10-11 + # .. toggle_warnings: For consistency in user experience, keep the value in sync with the setting of the same name + # in the LMS and CMS. + # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/429' + 'DISABLE_UNENROLLMENT': False, } # Specifies extra XBlock fields that should available when requested via the Course Blocks API @@ -1430,6 +1442,7 @@ def _make_mako_template_dirs(settings): GOOGLE_SITE_VERIFICATION_ID = '' GOOGLE_ANALYTICS_LINKEDIN = 'GOOGLE_ANALYTICS_LINKEDIN_DUMMY' GOOGLE_ANALYTICS_TRACKING_ID = None +GOOGLE_ANALYTICS_4_ID = None ######################## BRANCH.IO ########################### BRANCH_IO_KEY = '' @@ -3982,6 +3995,21 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring SEARCH_FILTER_GENERATOR = "lms.lib.courseware_search.lms_filter_generator.LmsSearchFilterGenerator" # Override to skip enrollment start date filtering in course search SEARCH_SKIP_ENROLLMENT_START_DATE_FILTERING = False +# .. toggle_name: SEARCH_SKIP_INVITATION_ONLY_FILTERING +# .. toggle_implementation: DjangoSetting +# .. toggle_default: True +# .. toggle_description: If enabled, invitation-only courses will appear in search results. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2021-08-27 +SEARCH_SKIP_INVITATION_ONLY_FILTERING = True +# .. toggle_name: SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING +# .. toggle_implementation: DjangoSetting +# .. toggle_default: True +# .. toggle_description: If enabled, courses with a catalog_visibility set to "none" will still +# appear in search results. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2021-08-27 +SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING = True # The configuration visibility of account fields. ACCOUNT_VISIBILITY_CONFIGURATION = { diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 4df21b4ca60..97e907c1ac0 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -442,3 +442,7 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing #################### Webpack Configuration Settings ############################## WEBPACK_LOADER['DEFAULT']['TIMEOUT'] = 5 + +#################### Network configuration #################### +# Devstack is directly exposed to the caller +CLOSEST_CLIENT_IP_FROM_HEADERS = [] diff --git a/lms/envs/production.py b/lms/envs/production.py index 75a225ca30c..6c7e0815a4a 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -82,6 +82,7 @@ def get_env_setting(setting): 'CELERY_QUEUES', 'MKTG_URL_LINK_MAP', 'MKTG_URL_OVERRIDES', + 'REST_FRAMEWORK', ] for key in KEYS_WITH_MERGED_VALUES: if key in __config_copy__: @@ -108,8 +109,8 @@ def get_env_setting(setting): BROKER_POOL_LIMIT = 0 BROKER_CONNECTION_TIMEOUT = 1 -# For the Result Store, use the django cache named 'celery' -CELERY_RESULT_BACKEND = 'django-cache' +# Allow env to configure celery result backend with default set to django-cache +CELERY_RESULT_BACKEND = ENV_TOKENS.get('CELERY_RESULT_BACKEND', 'django-cache') # When the broker is behind an ELB, use a heartbeat to refresh the # connection and to detect if it has been dropped. @@ -686,6 +687,7 @@ def get_env_setting(setting): GOOGLE_ANALYTICS_TRACKING_ID = AUTH_TOKENS.get('GOOGLE_ANALYTICS_TRACKING_ID') GOOGLE_ANALYTICS_LINKEDIN = AUTH_TOKENS.get('GOOGLE_ANALYTICS_LINKEDIN') GOOGLE_SITE_VERIFICATION_ID = ENV_TOKENS.get('GOOGLE_SITE_VERIFICATION_ID') +GOOGLE_ANALYTICS_4_ID = AUTH_TOKENS.get('GOOGLE_ANALYTICS_4_ID') ##### BRANCH.IO KEY ##### BRANCH_IO_KEY = AUTH_TOKENS.get('BRANCH_IO_KEY') @@ -728,6 +730,15 @@ def get_env_setting(setting): SEARCH_ENGINE = "search.elastic.ElasticSearchEngine" SEARCH_FILTER_GENERATOR = ENV_TOKENS.get('SEARCH_FILTER_GENERATOR', SEARCH_FILTER_GENERATOR) +SEARCH_SKIP_INVITATION_ONLY_FILTERING = ENV_TOKENS.get( + 'SEARCH_SKIP_INVITATION_ONLY_FILTERING', + SEARCH_SKIP_INVITATION_ONLY_FILTERING, +) +SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING = ENV_TOKENS.get( + 'SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING', + SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING, +) + # TODO: Once we have successfully upgraded to ES7, switch this back to ELASTIC_SEARCH_CONFIG. ELASTIC_SEARCH_CONFIG = ENV_TOKENS.get('ELASTIC_SEARCH_CONFIG_ES7', [{}]) @@ -1053,3 +1064,6 @@ def get_env_setting(setting): CHROME_DISABLE_SUBFRAME_DIALOG_SUPPRESSION_TOKEN = ENV_TOKENS.get( 'CHROME_DISABLE_SUBFRAME_DIALOG_SUPPRESSION_TOKEN', CHROME_DISABLE_SUBFRAME_DIALOG_SUPPRESSION_TOKEN ) + +############## DRF overrides ############## +REST_FRAMEWORK.update(ENV_TOKENS.get('REST_FRAMEWORK', {})) diff --git a/lms/envs/test.py b/lms/envs/test.py index 8b788d1ef78..69b290fc53e 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -599,3 +599,7 @@ RESET_PASSWORD_API_RATELIMIT = '2/m' CORS_ORIGIN_WHITELIST = ['https://sandbox.edx.org'] + +#################### Network configuration #################### +# Tests are not behind any proxies +CLOSEST_CLIENT_IP_FROM_HEADERS = [] diff --git a/lms/lib/courseware_search/lms_filter_generator.py b/lms/lib/courseware_search/lms_filter_generator.py index 14b539b4291..c4e5ab7ac73 100644 --- a/lms/lib/courseware_search/lms_filter_generator.py +++ b/lms/lib/courseware_search/lms_filter_generator.py @@ -2,6 +2,7 @@ This file contains implementation override of SearchFilterGenerator which will allow * Filter by all courses in which the user is enrolled in """ +from django.conf import settings from search.filter_generator import SearchFilterGenerator from openedx.core.djangoapps.course_groups.partition_scheme import CohortPartitionScheme @@ -52,4 +53,9 @@ def exclude_dictionary(self, **kwargs): if org_filter_out_set: exclude_dictionary['org'] = list(org_filter_out_set) + if not getattr(settings, "SEARCH_SKIP_INVITATION_ONLY_FILTERING", True): + exclude_dictionary['invitation_only'] = True + if not getattr(settings, "SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING", True): + exclude_dictionary['catalog_visibility'] = 'none' + return exclude_dictionary diff --git a/lms/static/js/groups/views/cohort_editor.js b/lms/static/js/groups/views/cohort_editor.js index feb45132817..9da4a0c92a6 100644 --- a/lms/static/js/groups/views/cohort_editor.js +++ b/lms/static/js/groups/views/cohort_editor.js @@ -260,16 +260,17 @@ // Show error messages. this.undelegateViewEvents(this.errorNotifications); - numErrors = modifiedUsers.unknown.length + modifiedUsers.invalid.length; + numErrors = modifiedUsers.unknown.length + modifiedUsers.invalid.length + modifiedUsers.not_allowed.length; if (numErrors > 0) { - createErrorDetails = function(unknownUsers, invalidEmails, showAllErrors) { + createErrorDetails = function(unknownUsers, invalidEmails, notAllowed, showAllErrors) { var unknownErrorsShown = showAllErrors ? unknownUsers.length : Math.min(errorLimit, unknownUsers.length); var invalidErrorsShown = showAllErrors ? invalidEmails.length : Math.min(errorLimit - unknownUsers.length, invalidEmails.length); + var notAllowedErrorsShown = showAllErrors ? notAllowed.length : + Math.min(errorLimit - notAllowed.length, notAllowed.length); details = []; - for (i = 0; i < unknownErrorsShown; i++) { details.push(interpolate_text(gettext('Unknown username: {user}'), {user: unknownUsers[i]})); @@ -278,6 +279,10 @@ details.push(interpolate_text(gettext('Invalid email address: {email}'), {email: invalidEmails[i]})); } + for (i = 0; i < notAllowedErrorsShown; i++) { + details.push(interpolate_text(gettext('Cohort assignment not allowed: {email_or_username}'), + {email_or_username: notAllowed[i]})); + } return details; }; @@ -286,12 +291,12 @@ '{numErrors} learners could not be added to this cohort:', numErrors), {numErrors: numErrors} ); - details = createErrorDetails(modifiedUsers.unknown, modifiedUsers.invalid, false); + details = createErrorDetails(modifiedUsers.unknown, modifiedUsers.invalid, modifiedUsers.not_allowed, false); errorActionCallback = function(view) { view.model.set('actionText', null); view.model.set('details', - createErrorDetails(modifiedUsers.unknown, modifiedUsers.invalid, true)); + createErrorDetails(modifiedUsers.unknown, modifiedUsers.invalid, modifiedUsers.not_allowed, true)); view.render(); }; diff --git a/lms/static/js/learner_dashboard/views/unenroll_view.js b/lms/static/js/learner_dashboard/views/unenroll_view.js index c4b0f2751d0..40d6affa9d5 100644 --- a/lms/static/js/learner_dashboard/views/unenroll_view.js +++ b/lms/static/js/learner_dashboard/views/unenroll_view.js @@ -75,6 +75,11 @@ class UnenrollView extends Backbone.View { this.switchToSlideOne(); this.$('.reasons_survey:first .submit_reasons').click(this.switchToSlideTwo.bind(this)); } + } else if (xhr.status === 400) { + $('#unenroll_error').text( + xhr.responseText, + ).stop() + .css('display', 'block'); } else if (xhr.status === 403) { location.href = `${this.urls.signInUser}?course_id=${ encodeURIComponent($('#unenroll_course_id').val())}&enrollment_action=unenroll`; diff --git a/lms/static/js/spec/groups/views/cohorts_spec.js b/lms/static/js/spec/groups/views/cohorts_spec.js index fa76052daa2..79acf25fb74 100644 --- a/lms/static/js/spec/groups/views/cohorts_spec.js +++ b/lms/static/js/spec/groups/views/cohorts_spec.js @@ -14,6 +14,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers var catLoversInitialCount = 123, dogLoversInitialCount = 456, unknownUserMessage, + notAllowedUserMessage, invalidEmailMessage, createMockCohort, createMockCohorts, createMockContentGroups, createMockCohortSettingsJson, createMockVerifiedTrackCohortsJson, flushVerifiedTrackCohortRequests, createCohortsView, cohortsView, requests, respondToRefresh, verifyMessage, verifyNoMessage, @@ -257,6 +258,10 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers return 'Invalid email address: ' + name; }; + notAllowedUserMessage = function(email) { + return 'Cohort assignment not allowed: ' + email; + }; + beforeEach(function() { setFixtures('