Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Account and profile MFE legacy removal - redeployment #31893

Merged
merged 12 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion cms/envs/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
from .common import *

# import settings from LMS for consistent behavior with CMS
from lms.envs.test import ( # pylint: disable=wrong-import-order
from lms.envs.test import ( # pylint: disable=wrong-import-order, disable=unused-import
ACCOUNT_MICROFRONTEND_URL,
BLOCKSTORE_USE_BLOCKSTORE_APP_API,
BLOCKSTORE_API_URL,
COMPREHENSIVE_THEME_DIRS, # unimport:skip
Expand All @@ -37,8 +38,10 @@
LOGIN_ISSUE_SUPPORT_LINK,
MEDIA_ROOT,
MEDIA_URL,
ORDER_HISTORY_MICROFRONTEND_URL,
PLATFORM_DESCRIPTION,
PLATFORM_NAME,
PROFILE_MICROFRONTEND_URL,
REGISTRATION_EXTRA_FIELDS,
GRADES_DOWNLOAD,
SITE_NAME,
Expand Down
3 changes: 2 additions & 1 deletion common/djangoapps/student/tests/test_filters.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
Test that various filters are fired for models/views in the student app.
"""
from django.conf import settings
from django.http import HttpResponse
from django.test import override_settings
from django.urls import reverse
Expand Down Expand Up @@ -421,7 +422,7 @@ def test_dashboard_redirect_account_settings(self):
response = self.client.get(self.dashboard_url)

self.assertEqual(status.HTTP_302_FOUND, response.status_code)
self.assertEqual(reverse("account_settings"), response.url)
self.assertEqual(settings.ACCOUNT_MICROFRONTEND_URL, response.url)

@override_settings(
OPEN_EDX_FILTERS_CONFIG={
Expand Down
2 changes: 1 addition & 1 deletion common/djangoapps/student/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def test_redirect_account_settings(self):
"""
UserProfile.objects.get(user=self.user).delete()
response = self.client.get(self.path)
self.assertRedirects(response, reverse('account_settings'))
self.assertRedirects(response, settings.ACCOUNT_MICROFRONTEND_URL, target_status_code=302)

@patch('common.djangoapps.student.views.dashboard.should_redirect_to_learner_home_mfe')
def test_redirect_to_learner_home(self, mock_should_redirect_to_learner_home_mfe):
Expand Down
6 changes: 3 additions & 3 deletions common/djangoapps/student/views/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ def student_dashboard(request): # lint-amnesty, pylint: disable=too-many-statem
"""
user = request.user
if not UserProfile.objects.filter(user=user).exists():
return redirect(reverse('account_settings'))
return redirect(settings.ACCOUNT_MICROFRONTEND_URL)

if should_redirect_to_learner_home_mfe(user):
return redirect(settings.LEARNER_HOME_MICROFRONTEND_URL)
Expand Down Expand Up @@ -624,7 +624,7 @@ def student_dashboard(request): # lint-amnesty, pylint: disable=too-many-statem
"Go to {link_start}your Account Settings{link_end}.")
).format(
link_start=HTML("<a href='{account_setting_page}'>").format(
account_setting_page=reverse('account_settings'),
account_setting_page=settings.ACCOUNT_MICROFRONTEND_URL,
),
link_end=HTML("</a>")
)
Expand Down Expand Up @@ -897,7 +897,7 @@ def student_dashboard(request): # lint-amnesty, pylint: disable=too-many-statem
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'))
response = HttpResponseRedirect(exc.redirect_to or settings.ACCOUNT_MICROFRONTEND_URL)
except DashboardRenderStarted.RenderCustomResponse as exc:
response = exc.response
else:
Expand Down
8 changes: 5 additions & 3 deletions common/djangoapps/third_party_auth/api/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
Tests for the Third Party Auth REST API
"""

import urllib
from unittest.mock import patch

import ddt
import six
from django.conf import settings
from django.http import QueryDict
from django.test.utils import override_settings
from django.urls import reverse
Expand Down Expand Up @@ -219,7 +220,7 @@ def make_url(self, identifier):
"""
return '?'.join([
reverse('third_party_auth_users_api_v2'),
six.moves.urllib.parse.urlencode(identifier)
urllib.parse.urlencode(identifier)
])


Expand Down Expand Up @@ -377,11 +378,12 @@ def test_get(self):
"""
self.client.login(username=self.user.username, password=PASSWORD)
response = self.client.get(self.url, content_type="application/json")
next_url = urllib.parse.quote(settings.ACCOUNT_MICROFRONTEND_URL, safe="")
assert response.status_code == 200
assert (response.data ==
[{
'accepts_logins': True, 'name': 'Google',
'disconnect_url': '/auth/disconnect/google-oauth2/?',
'connect_url': '/auth/login/google-oauth2/?auth_entry=account_settings&next=%2Faccount%2Fsettings',
'connect_url': f'/auth/login/google-oauth2/?auth_entry=account_settings&next={next_url}',
'connected': False, 'id': 'oa2-google-oauth2'
}])
3 changes: 1 addition & 2 deletions common/djangoapps/third_party_auth/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
from django.db.models import Q
from django.http import Http404
from django.urls import reverse
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
from rest_framework import exceptions, permissions, status, throttling
Expand Down Expand Up @@ -425,7 +424,7 @@ def get(self, request):
state.provider.provider_id,
pipeline.AUTH_ENTRY_ACCOUNT_SETTINGS,
# The url the user should be directed to after the auth process has completed.
redirect_url=reverse('account_settings'),
redirect_url=settings.ACCOUNT_MICROFRONTEND_URL,
),
'accepts_logins': state.provider.accepts_logins,
# If the user is connected, sending a POST request to this url removes the connection
Expand Down
77 changes: 46 additions & 31 deletions common/djangoapps/third_party_auth/tests/specs/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import pytest
from django import test
from django.conf import settings
from django.contrib import auth
from django.contrib import auth, messages
from django.contrib.auth import models as auth_models
from django.contrib.messages.storage import fallback
from django.contrib.sessions.backends import cache
Expand All @@ -28,7 +28,6 @@
from openedx.core.djangoapps.user_authn.views.register import RegistrationView
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
from openedx.core.djangoapps.user_api.accounts.settings_views import account_settings_context
from common.djangoapps.student import models as student_models
from common.djangoapps.student.tests.factories import UserFactory

Expand Down Expand Up @@ -99,6 +98,43 @@ def assert_register_response_in_pipeline_looks_correct(self, response, pipeline_
if prepopulated_form_data in required_fields:
self.assertContains(response, form_field_data[prepopulated_form_data])

def _get_user_providers_state(self, request):
"""
Return provider user states and duplicated providers.
"""
data = {
'auth': {},
}
data['duplicate_provider'] = pipeline.get_duplicate_provider(messages.get_messages(request))
auth_states = pipeline.get_provider_user_states(request.user)
data['auth']['providers'] = [{
'name': state.provider.name,
'connected': state.has_account,
} for state in auth_states if state.provider.display_for_login or state.has_account]
return data

def assert_third_party_accounts_state(self, request, duplicate=False, linked=None):
"""
Asserts the user's third party account in the expected state.

If duplicate is True, we expect data['duplicate_provider'] to contain
the duplicate provider backend name. If linked is passed, we conditionally
check that the provider is included in data['auth']['providers'] and
its connected state is correct.
"""
data = self._get_user_providers_state(request)
if duplicate:
assert data['duplicate_provider'] == self.provider.backend_name
else:
assert data['duplicate_provider'] is None

if linked is not None:
expected_provider = [
provider for provider in data['auth']['providers'] if provider['name'] == self.provider.name
][0]
assert expected_provider is not None
assert expected_provider['connected'] == linked

def assert_register_form_populates_unicode_username_correctly(self, request): # lint-amnesty, pylint: disable=invalid-name
"""
Check the registration form username field behaviour with unicode values.
Expand All @@ -118,27 +154,6 @@ def assert_register_form_populates_unicode_username_correctly(self, request): #
with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_UNICODE_USERNAME': True}):
self._check_registration_form_username(pipeline_kwargs, unicode_username, unicode_username)

# pylint: disable=invalid-name
def assert_account_settings_context_looks_correct(self, context, duplicate=False, linked=None):
"""Asserts the user's account settings page context is in the expected state.

If duplicate is True, we expect context['duplicate_provider'] to contain
the duplicate provider backend name. If linked is passed, we conditionally
check that the provider is included in context['auth']['providers'] and
its connected state is correct.
"""
if duplicate:
assert context['duplicate_provider'] == self.provider.backend_name
else:
assert context['duplicate_provider'] is None

if linked is not None:
expected_provider = [
provider for provider in context['auth']['providers'] if provider['name'] == self.provider.name
][0]
assert expected_provider is not None
assert expected_provider['connected'] == linked

def assert_exception_redirect_looks_correct(self, expected_uri, auth_entry=None):
"""Tests middleware conditional redirection.

Expand Down Expand Up @@ -611,7 +626,7 @@ def test_full_pipeline_succeeds_for_linking_account(self, _mock_segment_track):

# First we expect that we're in the unlinked state, and that there
# really is no association in the backend.
self.assert_account_settings_context_looks_correct(account_settings_context(get_request), linked=False)
self.assert_third_party_accounts_state(get_request, linked=False)
self.assert_social_auth_does_not_exist_for_user(get_request.user, strategy)

# We should be redirected back to the complete page, setting
Expand All @@ -630,7 +645,7 @@ def test_full_pipeline_succeeds_for_linking_account(self, _mock_segment_track):

# Now we expect to be in the linked state, with a backend entry.
self.assert_social_auth_exists_for_user(get_request.user, strategy)
self.assert_account_settings_context_looks_correct(account_settings_context(get_request), linked=True)
self.assert_third_party_accounts_state(get_request, linked=True)

def test_full_pipeline_succeeds_for_unlinking_account(self):
# First, create, the GET request and strategy that store pipeline state,
Expand Down Expand Up @@ -662,7 +677,7 @@ def test_full_pipeline_succeeds_for_unlinking_account(self):
get_request.user = post_request.user

# First we expect that we're in the linked state, with a backend entry.
self.assert_account_settings_context_looks_correct(account_settings_context(get_request), linked=True)
self.assert_third_party_accounts_state(get_request, linked=True)
self.assert_social_auth_exists_for_user(get_request.user, strategy)

# Fire off the disconnect pipeline to unlink.
Expand All @@ -676,7 +691,7 @@ def test_full_pipeline_succeeds_for_unlinking_account(self):
)

# Now we expect to be in the unlinked state, with no backend entry.
self.assert_account_settings_context_looks_correct(account_settings_context(get_request), linked=False)
self.assert_third_party_accounts_state(get_request, linked=False)
self.assert_social_auth_does_not_exist_for_user(user, strategy)

def test_linking_already_associated_account_raises_auth_already_associated(self):
Expand Down Expand Up @@ -734,8 +749,8 @@ def test_already_associated_exception_populates_dashboard_with_error(self):
post_request,
exceptions.AuthAlreadyAssociated(self.provider.backend_name, 'account is already in use.'))

self.assert_account_settings_context_looks_correct(
account_settings_context(post_request), duplicate=True, linked=True)
self.assert_third_party_accounts_state(
post_request, duplicate=True, linked=True)

@mock.patch('common.djangoapps.third_party_auth.pipeline.segment.track')
def test_full_pipeline_succeeds_for_signing_in_to_existing_active_account(self, _mock_segment_track):
Expand Down Expand Up @@ -795,7 +810,7 @@ def test_full_pipeline_succeeds_for_signing_in_to_existing_active_account(self,
self.assert_redirect_after_pipeline_completes(
self.do_complete(strategy, get_request, partial_pipeline_token, partial_data, user)
)
self.assert_account_settings_context_looks_correct(account_settings_context(get_request))
self.assert_third_party_accounts_state(get_request)

def test_signin_fails_if_account_not_active(self):
_, strategy = self.get_request_and_strategy(
Expand Down Expand Up @@ -937,7 +952,7 @@ def test_full_pipeline_succeeds_registering_new_account(self):
)
# Now the user has been redirected to the dashboard. Their third party account should now be linked.
self.assert_social_auth_exists_for_user(created_user, strategy)
self.assert_account_settings_context_looks_correct(account_settings_context(request), linked=True)
self.assert_third_party_accounts_state(request, linked=True)

def test_new_account_registration_assigns_distinct_username_on_collision(self):
original_username = self.get_username()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
from common.djangoapps.third_party_auth.saml import log as saml_log
from common.djangoapps.third_party_auth.tasks import fetch_saml_metadata
from common.djangoapps.third_party_auth.tests import testutil, utils
from openedx.core.djangoapps.user_api.accounts.settings_views import account_settings_context
from openedx.core.djangoapps.user_authn.views.login import login_user
from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerFactory

Expand Down Expand Up @@ -239,12 +238,10 @@ class TestShibIntegrationTest(SamlIntegrationTestUtilities, IntegrationTestMixin
}

@patch('openedx.features.enterprise_support.api.enterprise_customer_for_request')
@patch('openedx.core.djangoapps.user_api.accounts.settings_views.enterprise_customer_for_request')
@patch('openedx.features.enterprise_support.utils.third_party_auth.provider.Registry.get')
def test_full_pipeline_succeeds_for_unlinking_testshib_account(
self,
mock_auth_provider,
mock_enterprise_customer_for_request_settings_view,
mock_enterprise_customer_for_request,
):

Expand Down Expand Up @@ -284,7 +281,6 @@ def test_full_pipeline_succeeds_for_unlinking_testshib_account(
}
mock_auth_provider.return_value.backend_name = 'tpa-saml'
mock_enterprise_customer_for_request.return_value = enterprise_customer_data
mock_enterprise_customer_for_request_settings_view.return_value = enterprise_customer_data

# Instrument the pipeline to get to the dashboard with the full expected state.
self.client.get(
Expand All @@ -299,7 +295,7 @@ def test_full_pipeline_succeeds_for_unlinking_testshib_account(
request=request)

# First we expect that we're in the linked state, with a backend entry.
self.assert_account_settings_context_looks_correct(account_settings_context(request), linked=True)
self.assert_third_party_accounts_state(request, linked=True)
self.assert_social_auth_exists_for_user(request.user, strategy)

FEATURES_WITH_ENTERPRISE_ENABLED = settings.FEATURES.copy()
Expand Down Expand Up @@ -327,7 +323,7 @@ def test_full_pipeline_succeeds_for_unlinking_testshib_account(
)
)
# Now we expect to be in the unlinked state, with no backend entry.
self.assert_account_settings_context_looks_correct(account_settings_context(request), linked=False)
self.assert_third_party_accounts_state(request, linked=False)
self.assert_social_auth_does_not_exist_for_user(user, strategy)
assert EnterpriseCustomerUser.objects\
.filter(enterprise_customer=enterprise_customer, user_id=user.id).count() == 0
Expand Down
4 changes: 0 additions & 4 deletions conf/locale/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,6 @@ segment:
djangojs-partial.po:
djangojs-studio.po:
- cms/*
djangojs-account-settings-view.po:
- lms/static/js/student_account/views/account_settings_view.js
# Segregating student account settings view strings, so that beta language message
# can be translated for wide set of partially supported languages.
mako.po:
mako-studio.po:
- cms/*
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/bulk_email/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def _get_course_email_context(course):
'course_url': course_url,
'course_image_url': image_url,
'course_end_date': course_end_date,
'account_settings_url': '{}{}'.format(lms_root_url, reverse('account_settings')),
'account_settings_url': settings.ACCOUNT_MICROFRONTEND_URL,
'email_settings_url': '{}{}'.format(lms_root_url, reverse('dashboard')),
'logo_url': get_logo_url_for_email(),
'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/bulk_email/tests/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,7 @@ def verify_email_context(self, email_context, scheme):
assert email_context['course_image_url'] == \
f'{scheme}://edx.org/asset-v1:{course_id_fragment}+type@asset+block@images_course_image.jpg'
assert email_context['email_settings_url'] == f'{scheme}://edx.org/dashboard'
assert email_context['account_settings_url'] == f'{scheme}://edx.org/account/settings'
assert email_context['account_settings_url'] == settings.ACCOUNT_MICROFRONTEND_URL

@override_settings(LMS_ROOT_URL="http://edx.org")
def test_insecure_email_context(self):
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/views/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2143,7 +2143,7 @@ def financial_assistance_form(request, course_id=None):
'header_text': _get_fa_header(FINANCIAL_ASSISTANCE_HEADER),
'student_faq_url': marketing_link('FAQ'),
'dashboard_url': reverse('dashboard'),
'account_settings_url': reverse('account_settings'),
'account_settings_url': settings.ACCOUNT_MICROFRONTEND_URL,
'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
'user_details': {
'email': user.email,
Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/discussion/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import logging
from functools import wraps
from urllib.parse import urljoin

from django.conf import settings
from django.contrib.auth import get_user_model
Expand Down Expand Up @@ -629,7 +630,7 @@ def create_user_profile_context(request, course_key, user_id):
'page': query_params['page'],
'num_pages': query_params['num_pages'],
'sort_preference': user.default_sort_key,
'learner_profile_page_url': reverse('learner_profile', kwargs={'username': django_user.username}),
'learner_profile_page_url': urljoin(settings.PROFILE_MICROFRONTEND_URL, f'/u/{django_user.username}'),
})
return context

Expand Down
1 change: 0 additions & 1 deletion lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -3237,7 +3237,6 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
'openedx.features.course_bookmarks',
'openedx.features.course_experience',
'openedx.features.enterprise_support.apps.EnterpriseSupportConfig',
'openedx.features.learner_profile',
'openedx.features.course_duration_limits',
'openedx.features.content_type_gating',
'openedx.features.discounts',
Expand Down
Loading
Loading