From f8f401a00a13b95ed8e433b1fef732605c27db40 Mon Sep 17 00:00:00 2001 From: Marlon Keating Date: Fri, 31 Jul 2026 19:45:38 +0000 Subject: [PATCH] feat: Add plugin hook for augmenting activation email context A new OpenEdX filter AccountActivationEmailComposed is installed (in place of custom enterprise logic) to allow plugins more flexibility in customizing the account activation email. ENT-11816 --- .../student/tests/test_activate_account.py | 35 +++++++-- .../djangoapps/student/tests/test_filters.py | 76 ++++++++++++++++++- common/djangoapps/student/views/management.py | 33 +++++--- .../djangoapps/user_authn/tests/test_tasks.py | 1 - 4 files changed, 128 insertions(+), 17 deletions(-) diff --git a/common/djangoapps/student/tests/test_activate_account.py b/common/djangoapps/student/tests/test_activate_account.py index e767800155aa..7dea3503b37e 100644 --- a/common/djangoapps/student/tests/test_activate_account.py +++ b/common/djangoapps/student/tests/test_activate_account.py @@ -15,7 +15,6 @@ from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangolib.testing.utils import skip_unless_lms -from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerUserFactory FEATURES_WITH_AUTHN_MFE_ENABLED = settings.FEATURES.copy() FEATURES_WITH_AUTHN_MFE_ENABLED['ENABLE_AUTHN_MICROFRONTEND'] = True @@ -180,15 +179,14 @@ def test_email_confirmation_notification_on_logistration(self): self.assertContains(response, 'Your email could not be confirmed') @override_settings(LOGIN_REDIRECT_WHITELIST=['localhost:1991']) - @override_settings(FEATURES={**FEATURES_WITH_AUTHN_MFE_ENABLED, 'ENABLE_ENTERPRISE_INTEGRATION': True}) + @override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED) def test_authenticated_account_activation_with_valid_next_url(self): """ Verify that an activation link with a valid next URL will redirect - the activated enterprise user to that next URL, even if the AuthN - MFE is active and redirects to it are enabled. + the activated user to that next URL, even if the AuthN MFE is active + and redirects to it are enabled. """ self._assert_user_active_state(expected_active_state=False) - EnterpriseCustomerUserFactory(user_id=self.user.id) # Make sure the user is authenticated before activation. self.login() @@ -208,6 +206,33 @@ def test_authenticated_account_activation_with_valid_next_url(self): self.assertRedirects(response, redirect_url, target_status_code=404) self._assert_user_active_state(expected_active_state=True) + @override_settings(LOGIN_REDIRECT_WHITELIST=['localhost:1991']) + def test_unauthenticated_user_redirects_to_login_with_valid_next_url(self): + """ + Verify that when the AuthN MFE is disabled, an unauthenticated user activating + with a valid next URL is sent to the legacy login page with that URL preserved + as the `next` parameter, rather than having it dropped in favour of the + dashboard. The legacy login page renders the activation message itself. + """ + self._assert_user_active_state(expected_active_state=False) + + redirect_url = 'http://localhost:1991/pied-piper/learn' + base_activation_url = reverse('activate', args=[self.registration.activation_key]) + activation_url = '{base}?{params}'.format( + base=base_activation_url, + params=urlencode({'next': redirect_url}), + ) + + # HTTP_ACCEPT is needed so the safe redirect checks pass. + response = self.client.get(activation_url, HTTP_ACCEPT='*/*') + + expected_destination = '{login_url}?{params}'.format( + login_url=reverse('signin_user'), + params=urlencode({'next': redirect_url}), + ) + assert response.url == expected_destination + self._assert_user_active_state(expected_active_state=True) + @override_settings(LOGIN_REDIRECT_WHITELIST=['localhost:9876']) def test_account_activation_invalid_next_url_redirects_dashboard(self): """ diff --git a/common/djangoapps/student/tests/test_filters.py b/common/djangoapps/student/tests/test_filters.py index bf79ed7ae402..eac96cd724d8 100644 --- a/common/djangoapps/student/tests/test_filters.py +++ b/common/djangoapps/student/tests/test_filters.py @@ -6,13 +6,19 @@ 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 openedx_filters.learning.filters import CourseEnrollmentStarted, CourseUnenrollmentStarted, DashboardRenderStarted 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.models import ( + CourseEnrollment, + EnrollmentNotAllowed, + Registration, + UnenrollmentNotAllowed +) from common.djangoapps.student.tests.factories import UserFactory, UserProfileFactory +from common.djangoapps.student.views.management import compose_activation_email from openedx.core.djangolib.testing.utils import skip_unless_lms @@ -111,6 +117,17 @@ def run_filter(self, context, template_name): # pylint: disable=arguments-diffe ) +class TestActivationEmailComposedPipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, user, message_context): # pylint: disable=arguments-differ + """Pipeline step that stamps a marker onto the activation email context.""" + message_context["is_enterprise_learner"] = True + return {"user": user, "message_context": message_context} + + @skip_unless_lms class EnrollmentFiltersTest(ModuleStoreTestCase): """ @@ -464,3 +481,58 @@ def test_dashboard_render_without_filter_config(self): self.assertContains(response, self.first_course.id) self.assertContains(response, self.second_course.id) + + +@skip_unless_lms +class AccountActivationEmailFiltersTest(ModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with composing the account activation email. + + This class guarantees that the following filter is triggered when the activation email + is composed: + - AccountActivationEmailComposed + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.user = UserFactory() + self.registration = Registration() + self.registration.register(self.user) + self.registration.save() + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.account.activation.email.compose.v1": { + "pipeline": [ + "common.djangoapps.student.tests.test_filters.TestActivationEmailComposedPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_activation_email_composed_filter_executed(self): + """ + Test whether the activation email composed filter is triggered before the + activation email message context is finalized. + + Expected result: + - AccountActivationEmailComposed is triggered and executes + TestActivationEmailComposedPipelineStep. + - The composed message's context contains the pipeline step's modification. + """ + message = compose_activation_email(self.user, self.registration) + + assert message.context["is_enterprise_learner"] is True + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_activation_email_composed_without_filter_config(self): + """ + Test that compose_activation_email succeeds with no pipeline steps configured. + + Expected result: + - AccountActivationEmailComposed executes a noop (empty pipeline). + - No 'is_enterprise_learner' key is injected into the message context. + """ + message = compose_activation_email(self.user, self.registration) + + assert "is_enterprise_learner" not in message.context diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 412781f73cce..71e912fbbf12 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -36,6 +36,7 @@ # 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.learning.filters import AccountActivationEmailComposed from rest_framework.decorators import api_view, authentication_classes, permission_classes from rest_framework.permissions import IsAuthenticated @@ -98,7 +99,6 @@ from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser from openedx.features.course_experience.url_helpers import make_learning_mfe_courseware_url from openedx.features.discounts.applicability import FIRST_PURCHASE_DISCOUNT_OVERRIDE_FLAG -from openedx.features.enterprise_support.utils import is_enterprise_learner from common.djangoapps.util.db import outer_atomic from common.djangoapps.util.json_request import JsonResponse from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -222,7 +222,6 @@ def compose_activation_email( message_context = generate_activation_email_context(user, user_registration) message_context.update({ 'confirm_activation_link': _get_activation_confirmation_link(message_context['key'], redirect_url), - 'is_enterprise_learner': is_enterprise_learner(user), 'is_first_purchase_discount_overridden': FIRST_PURCHASE_DISCOUNT_OVERRIDE_FLAG.is_enabled(), 'route_enabled': route_enabled, 'routed_user': user.username, @@ -231,6 +230,11 @@ def compose_activation_email( 'registration_flow': registration_flow, 'show_auto_generated_username': show_auto_generated_username(user.username), }) + # .. filter_implemented_name: AccountActivationEmailComposed + # .. filter_type: org.openedx.learning.account.activation.email.compose.v1 + __, message_context = AccountActivationEmailComposed.run_filter( + user=user, message_context=message_context, + ) if route_enabled: dest_addr = settings.FEATURES['REROUTE_ACTIVATION_EMAIL'] @@ -678,7 +682,7 @@ def activate_account(request, key): if request.GET.get('next'): redirect_to, root_login_url = get_next_url_for_login_page(request, include_host=True) - # Don't automatically redirect authenticated users to the redirect_url + # Don't automatically redirect to the redirect_url # if the `next` value is either: # 1. "/dashboard" or # 2. "https://{LMS_ROOT_URL}/dashboard" (which we might provide as a value from the AuthN MFE) @@ -688,14 +692,25 @@ def activate_account(request, key): ): redirect_url = get_redirect_url_with_host(root_login_url, redirect_to) - if should_redirect_to_authn_microfrontend() and not request.user.is_authenticated: - params = {'account_activation_status': activation_message_type} + # Visitors who are not signed in have to authenticate before they can use their + # destination, so hand it to the login page rather than dropping it. Both login + # surfaces report the activation outcome to the user: the AuthN MFE reads the + # `account_activation_status` parameter, and the legacy login page renders the + # messages queued above. + if not request.user.is_authenticated: + if should_redirect_to_authn_microfrontend(): + params = {'account_activation_status': activation_message_type} + if redirect_url: + params['next'] = redirect_url + url_path = '/login?{}'.format(urllib.parse.urlencode(params)) + return redirect(settings.AUTHN_MICROFRONTEND_URL + url_path) if redirect_url: - params['next'] = redirect_url - url_path = '/login?{}'.format(urllib.parse.urlencode(params)) - return redirect(settings.AUTHN_MICROFRONTEND_URL + url_path) + redirect_url = '{login_url}?{params}'.format( + login_url=reverse('signin_user'), + params=urllib.parse.urlencode({'next': redirect_url}), + ) - response = redirect(redirect_url) if redirect_url and is_enterprise_learner(request.user) else redirect('dashboard') + response = redirect(redirect_url or 'dashboard') if show_account_activation_popup: response.delete_cookie( settings.SHOW_ACTIVATE_CTA_POPUP_COOKIE_NAME, diff --git a/openedx/core/djangoapps/user_authn/tests/test_tasks.py b/openedx/core/djangoapps/user_authn/tests/test_tasks.py index 5103343a0879..60bb56133fc4 100644 --- a/openedx/core/djangoapps/user_authn/tests/test_tasks.py +++ b/openedx/core/djangoapps/user_authn/tests/test_tasks.py @@ -44,7 +44,6 @@ def test_ComposeEmail(self): assert self.msg.context['routed_user_email'] == self.student.email assert self.msg.context['routed_profile_name'] == '' assert self.msg.context['registration_flow'] is False - assert self.msg.context['is_enterprise_learner'] is False assert self.msg.context['is_first_purchase_discount_overridden'] is False @mock.patch('time.sleep', mock.Mock(return_value=None))