Skip to content

Commit

Permalink
feat: fixed tests and pylint errors
Browse files Browse the repository at this point in the history
  • Loading branch information
UvgenGen committed May 2, 2022
1 parent eadf859 commit 94abf32
Show file tree
Hide file tree
Showing 15 changed files with 65 additions and 35 deletions.
3 changes: 3 additions & 0 deletions cms/envs/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

# import settings from LMS for consistent behavior with CMS
from lms.envs.test import ( # pylint: disable=wrong-import-order
ACCOUNT_MICROFRONTEND_URL, # pylint: disable=unused-import
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, # pylint: disable=unused-import
PLATFORM_DESCRIPTION,
PLATFORM_NAME,
PROFILE_MICROFRONTEND_URL, # pylint: disable=unused-import
REGISTRATION_EXTRA_FIELDS,
GRADES_DOWNLOAD,
SITE_NAME,
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 @@ -227,7 +227,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)

def test_course_cert_available_message_after_course_end(self):
course_key = CourseKey.from_string('course-v1:edX+DemoX+Demo_Course')
Expand Down
7 changes: 4 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 @@ -4,10 +4,10 @@


import unittest
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
Expand Down Expand Up @@ -221,7 +221,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 @@ -374,11 +374,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'
}])
1 change: 0 additions & 1 deletion 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
48 changes: 47 additions & 1 deletion 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 Down Expand Up @@ -85,6 +85,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_exception_redirect_looks_correct(self, expected_uri, auth_entry=None):
"""Tests middleware conditional redirection.
Expand Down Expand Up @@ -557,6 +594,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_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 @@ -575,6 +613,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_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 @@ -606,6 +645,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_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 @@ -619,6 +659,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_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 @@ -676,6 +717,9 @@ 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_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):
# First, create, the GET request and strategy that store pipeline state,
Expand Down Expand Up @@ -734,6 +778,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_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 @@ -874,6 +919,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_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 @@ -26,7 +26,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 @@ -162,12 +161,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 @@ -207,7 +204,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 @@ -222,7 +218,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 @@ -250,7 +246,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
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 @@ -716,7 +716,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/discussion/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,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': urljoin(settings.PROFILE_MICROFRONTEND_URL + '/', django_user.username),
'learner_profile_page_url': urljoin(settings.PROFILE_MICROFRONTEND_URL, f'/u/{django_user.username}'),
})
return context

Expand Down
2 changes: 1 addition & 1 deletion lms/envs/devstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing
############## Settings for Microfrontends #########################
LEARNING_MICROFRONTEND_URL = 'http://localhost:2000'
ACCOUNT_MICROFRONTEND_URL = 'http://localhost:1997'
PROFILE_MICROFRONTEND_URL = 'http://localhost:1995/u'
PROFILE_MICROFRONTEND_URL = 'http://localhost:1995'
COMMUNICATIONS_MICROFRONTEND_URL = 'http://localhost:1984'
AUTHN_MICROFRONTEND_URL = 'http://localhost:1999'
AUTHN_MICROFRONTEND_DOMAIN = 'localhost:1999'
Expand Down
2 changes: 1 addition & 1 deletion lms/envs/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@
PDF_RECEIPT_TERMS_AND_CONDITIONS = 'add your own terms and conditions'
PDF_RECEIPT_TAX_ID_LABEL = 'Tax ID'

PROFILE_MICROFRONTEND_URL = "http://profile-mfe/abc/"
PROFILE_MICROFRONTEND_URL = "http://profile-mfe"
ORDER_HISTORY_MICROFRONTEND_URL = "http://order-history-mfe/"
ACCOUNT_MICROFRONTEND_URL = "http://account-mfe"
AUTHN_MICROFRONTEND_URL = "http://authn-mfe"
Expand Down
2 changes: 1 addition & 1 deletion lms/templates/header/user_dropdown.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
<div class="mobile-nav-item dropdown-item dropdown-nav-item"><a href="${settings.ENTERPRISE_LEARNER_PORTAL_BASE_URL}/${enterprise_customer_portal.get('slug')}" role="menuitem">${_("Dashboard")}</a></div>
% endif

<div class="mobile-nav-item dropdown-item dropdown-nav-item"><a href="${urljoin(settings.PROFILE_MICROFRONTEND_URL + '/', user.username)}" role="menuitem">${_("Profile")}</a></div>
<div class="mobile-nav-item dropdown-item dropdown-nav-item"><a href="${urljoin(settings.PROFILE_MICROFRONTEND_URL, f'/u/{user.username}')}" role="menuitem">${_("Profile")}</a></div>
<div class="mobile-nav-item dropdown-item dropdown-nav-item"><a href="${settings.ACCOUNT_MICROFRONTEND_URL}" role="menuitem">${_("Account")}</a></div>
% if should_show_order_history:
<div class="mobile-nav-item dropdown-item dropdown-nav-item"><a href="${settings.ORDER_HISTORY_MICROFRONTEND_URL}" role="menuitem">${_("Order History")}</a></div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,6 @@ def test_footer(self):
# This string comes from header.html of test-theme
self.assertContains(resp, "This is a footer for test-theme.")

@with_comprehensive_theme("edx.org")
def test_account_settings_hide_nav(self):
"""
Test that theme header doesn't show marketing site links for Account Settings page.
"""
self._login()

account_settings_url = reverse('account_settings')
response = self.client.get(account_settings_url)

# Verify that the header navigation links are hidden for the edx.org version
self.assertNotContains(response, "How it Works")
self.assertNotContains(response, "Find courses")
self.assertNotContains(response, "Schools & Partners")

@with_comprehensive_theme("test-theme")
def test_logo_image(self):
"""
Expand Down
2 changes: 1 addition & 1 deletion openedx/core/djangoapps/user_authn/cookies.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def _get_user_info_cookie_data(request, user):
# (most likely just hiding the links).
try:
header_urls['account_settings'] = settings.ACCOUNT_MICROFRONTEND_URL
header_urls['learner_profile'] = urljoin(settings.PROFILE_MICROFRONTEND_URL + '/', user.username)
header_urls['learner_profile'] = urljoin(settings.PROFILE_MICROFRONTEND_URL, f'/u/{user.username}')
except NoReverseMatch:
pass

Expand Down
2 changes: 1 addition & 1 deletion openedx/core/djangoapps/user_authn/tests/test_cookies.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def _get_expected_header_urls(self):
'logout': reverse('logout'),
'resume_block': retrieve_last_sitewide_block_completed(self.user),
'account_settings': settings.ACCOUNT_MICROFRONTEND_URL,
'learner_profile': urljoin(settings.PROFILE_MICROFRONTEND_URL + '/', self.user.username),
'learner_profile': urljoin(settings.PROFILE_MICROFRONTEND_URL, f'/u/{self.user.username}'),
}

expected_header_urls = self._convert_to_absolute_uris(self.request, expected_header_urls)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ def test_login_user_info_cookie(self):

# Check that the URLs are absolute
for url in user_info["header_urls"].values():
assert 'http://testserver/' in url
assert 'http://' in url

def test_logout_deletes_mktg_cookies(self):
response, _ = self._login_response(self.user_email, self.password)
Expand Down

0 comments on commit 94abf32

Please sign in to comment.