diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 424d9ceb71..ba01a9c433 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -89,6 +89,7 @@ from openedx.core.djangoapps.programs.utils import ( ) from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming import helpers as theming_helpers +from openedx.core.djangoapps.user_api import accounts as accounts_settings from openedx.core.djangoapps.user_api.preferences import api as preferences_api from openedx.core.djangoapps.waffle_utils import WaffleFlagNamespace, WaffleFlag from openedx.core.djangolib.markup import HTML @@ -1965,7 +1966,7 @@ def create_account_with_params(request, params): params["email"] = eamap.external_email except ValidationError: pass - if eamap.external_name.strip() != '': + if len(eamap.external_name.strip()) >= accounts_settings.NAME_MIN_LENGTH: params["name"] = eamap.external_name params["password"] = eamap.internal_password log.debug(u'In create_account with external_auth: user = %s, email=%s', params["name"], params["email"]) diff --git a/lms/djangoapps/shoppingcart/views.py b/lms/djangoapps/shoppingcart/views.py index cc094a5301..eccb77e9ae 100644 --- a/lms/djangoapps/shoppingcart/views.py +++ b/lms/djangoapps/shoppingcart/views.py @@ -250,7 +250,9 @@ def remove_item(request): item_id ) else: - item = items[0] + # Reload the item directly to prevent select_subclasses() hackery from interfering with + # deletion of all objects in the model inheritance hierarchy + item = items[0].__class__.objects.get(id=item_id) if item.user == request.user: Order.remove_cart_item_from_order(item, request.user) item.order.update_order_type() @@ -395,6 +397,9 @@ def register_code_redemption(request, registration_code): else: for cart_item in cart_items: if isinstance(cart_item, PaidCourseRegistration) or isinstance(cart_item, CourseRegCodeItem): + # Reload the item directly to prevent select_subclasses() hackery from interfering with + # deletion of all objects in the model inheritance hierarchy + cart_item = cart_item.__class__.objects.get(id=cart_item.id) cart_item.delete() #now redeem the reg code. diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index f88a362953..8bca85b5ca 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -47,6 +47,7 @@ from openedx.core.djangoapps.user_api.accounts.api import activate_account, crea from openedx.core.djangolib.js_utils import dump_js_escaped_json from openedx.core.djangolib.markup import HTML, Text from openedx.core.djangolib.testing.utils import CacheIsolationTestCase +from openedx.tests.util import expected_redirect_url from student.tests.factories import UserFactory from student_account.views import account_settings_context, get_user_orders from third_party_auth.tests.testutil import ThirdPartyAuthTestMixin, simulate_running_pipeline @@ -592,7 +593,7 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html") self.assertRedirects( response, - 'auth/login/google-oauth2/?auth_entry={}&next=%2Fcourses%2Fsomething%2F%3Ftpa_hint%3Doa2-google-oauth2'.format(auth_entry), + expected_redirect_url('auth/login/google-oauth2/?auth_entry={}&next=%2Fcourses%2Fsomething%2F%3Ftpa_hint%3Doa2-google-oauth2'.format(auth_entry)), target_status_code=302 ) @@ -636,7 +637,7 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html") self.assertRedirects( response, - 'auth/login/google-oauth2/?auth_entry={}&next=%2Fcourses%2Fsomething%2F%3Ftpa_hint%3Doa2-google-oauth2'.format(auth_entry), + expected_redirect_url('auth/login/google-oauth2/?auth_entry={}&next=%2Fcourses%2Fsomething%2F%3Ftpa_hint%3Doa2-google-oauth2'.format(auth_entry)), target_status_code=302 ) diff --git a/lms/djangoapps/support/views/refund.py b/lms/djangoapps/support/views/refund.py index dd5e65e4dd..7448f17981 100644 --- a/lms/djangoapps/support/views/refund.py +++ b/lms/djangoapps/support/views/refund.py @@ -113,6 +113,7 @@ class RefundSupportView(FormView): """ extra context data to add to page """ + kwargs = super(RefundSupportView, self).get_context_data(**kwargs) form = getattr(kwargs['form'], 'cleaned_data', {}) if form.get('confirmed') == 'true': kwargs['cert'] = form.get('cert') diff --git a/openedx/core/djangoapps/external_auth/tests/test_shib.py b/openedx/core/djangoapps/external_auth/tests/test_shib.py index 257d544682..342a532dcb 100644 --- a/openedx/core/djangoapps/external_auth/tests/test_shib.py +++ b/openedx/core/djangoapps/external_auth/tests/test_shib.py @@ -8,6 +8,7 @@ import unittest from importlib import import_module from urllib import urlencode +import django import pytest from ddt import ddt, data from django.conf import settings @@ -21,6 +22,8 @@ from openedx.core.djangoapps.external_auth.models import ExternalAuthMap from openedx.core.djangoapps.external_auth.views import ( shib_login, course_specific_login, course_specific_register, _flatten_to_ascii ) +from openedx.core.djangoapps.user_api import accounts as accounts_settings +from openedx.tests.util import expected_redirect_url from mock import patch from nose.plugins.attrib import attr @@ -357,15 +360,19 @@ class ShibSPTest(CacheIsolationTestCase): # check that the created user profile has the right name, either taken from shib or user input profile = UserProfile.objects.get(user=user) - sn_empty = not identity.get('sn') - given_name_empty = not identity.get('givenName') + external_name = self.client.session['ExternalAuthMap'].external_name displayname_empty = not identity.get('displayName') if displayname_empty: - if sn_empty and given_name_empty: + if len(external_name.strip()) < accounts_settings.NAME_MIN_LENGTH: self.assertEqual(profile.name, postvars['name']) else: - self.assertEqual(profile.name, self.client.session['ExternalAuthMap'].external_name) + expected_name = external_name + # TODO: Remove Django 1.11 upgrade shim + # SHIM: form character fields strip leading and trailing whitespace by default in Django 1.9+ + if django.VERSION >= (1, 9): + expected_name = expected_name.strip() + self.assertEqual(profile.name, expected_name) self.assertNotIn(u';', profile.name) else: self.assertEqual(profile.name, self.client.session['ExternalAuthMap'].external_name) @@ -576,7 +583,8 @@ class ShibSPTestModifiedCourseware(ModuleStoreTestCase): response = self.client.get(**request_kwargs) # successful login is a redirect to the URL that handles auto-enrollment self.assertEqual(response.status_code, 302) - self.assertEqual(response['location'], 'http://testserver/account/finish_auth?{}'.format(urlencode(params))) + self.assertEqual(response['location'], + expected_redirect_url('/account/finish_auth?{}'.format(urlencode(params)))) class ShibUtilFnTest(TestCase): diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 7e595ca3c4..431e4ac720 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -116,7 +116,7 @@ class UserAPITestCase(APITestCase): legacy_profile.gender = "f" legacy_profile.bio = "Tired mother of twins" legacy_profile.profile_image_uploaded_at = TEST_PROFILE_IMAGE_UPLOADED_AT - legacy_profile.language_proficiencies.add(LanguageProficiency(code='en')) + legacy_profile.language_proficiencies.create(code='en') legacy_profile.save() def _verify_profile_image_data(self, data, has_profile_image): diff --git a/openedx/tests/util/__init__.py b/openedx/tests/util/__init__.py index b9835638df..b99c3c88a6 100644 --- a/openedx/tests/util/__init__.py +++ b/openedx/tests/util/__init__.py @@ -11,10 +11,19 @@ import django def expected_redirect_url(relative_url, hostname='testserver'): """ Get the expected redirect URL for the current Django version and the - given relative URL. Django 1.8 and earlier redirect to absolute URLs, - later versions redirect to relative ones. + given relative URL: + * Django 1.8 and earlier redirect URLs beginning with a slash to absolute + URLs, later versions redirect to relative ones. + * Django 1.8 and earlier leave URLs without a leading slash alone, later + versions prepend the missing slash. """ if django.VERSION < (1, 9): - return 'http://{}{}'.format(hostname, relative_url) + if relative_url.startswith('/'): + return 'http://{}{}'.format(hostname, relative_url) + else: + return relative_url else: - return relative_url + if relative_url.startswith('/'): + return relative_url + else: + return '/{}'.format(relative_url)