From 04ca94eaef6a93f94e433a647b015e02965a54cd Mon Sep 17 00:00:00 2001 From: Ahsan Ulhaq Date: Thu, 30 Mar 2017 16:57:56 +0500 Subject: [PATCH] Account settings page should load and create student user profile ECOM-4776 --- common/djangoapps/student/tests/test_email.py | 10 +++ common/djangoapps/student/tests/test_views.py | 10 ++- common/djangoapps/student/views.py | 23 ++++-- lms/djangoapps/courseware/tests/test_i18n.py | 4 ++ .../core/djangoapps/user_api/accounts/api.py | 3 +- .../user_api/accounts/serializers.py | 70 +++++++++++++------ .../accounts/tests/test_serializers.py | 58 +++++++++++++++ .../core/djangoapps/user_api/serializers.py | 8 ++- 8 files changed, 156 insertions(+), 30 deletions(-) create mode 100644 openedx/core/djangoapps/user_api/accounts/tests/test_serializers.py diff --git a/common/djangoapps/student/tests/test_email.py b/common/djangoapps/student/tests/test_email.py index 529e23cbce..8f6ebe1610 100644 --- a/common/djangoapps/student/tests/test_email.py +++ b/common/djangoapps/student/tests/test_email.py @@ -214,6 +214,16 @@ class ReactivationEmailTests(EmailTestMixin, TestCase): self.assertFalse(response_data['success']) + def test_reactivation_for_no_user_profile(self, email_user): + """ + Test that trying to send a reactivation email to a user without + user profile fails without throwing 500 error. + """ + user = UserFactory.build(username='test_user', email='test_user@test.com') + user.save() + response_data = self.reactivation_email(user) + self.assertFalse(response_data['success']) + def test_reactivation_email_success(self, email_user): response_data = self.reactivation_email(self.user) diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index c22c61b64b..3b62712cbf 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -18,7 +18,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from student.cookies import get_user_info_cookie_data from student.helpers import DISABLE_UNENROLL_CERT_STATES -from student.models import CourseEnrollment, LogoutViewConfiguration +from student.models import CourseEnrollment, LogoutViewConfiguration, UserProfile from student.tests.factories import UserFactory, CourseEnrollmentFactory PASSWORD = 'test' @@ -221,3 +221,11 @@ class StudentDashboardTests(TestCase): self.client.get(self.path) actual = self.client.cookies[settings.EDXMKTG_USER_INFO_COOKIE_NAME].value self.assertEqual(actual, expected) + + def test_redirect_account_settings(self): + """ + Verify if user does not have profile he/she is redirected to account_settings. + """ + UserProfile.objects.get(user=self.user).delete() + response = self.client.get(self.path) + self.assertRedirects(response, reverse('account_settings')) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index d806335655..f3ad473453 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -23,7 +23,7 @@ from django.contrib.auth.views import password_reset_confirm from django.contrib import messages from django.core.context_processors import csrf from django.core import mail -from django.core.exceptions import PermissionDenied +from django.core.exceptions import PermissionDenied, ObjectDoesNotExist from django.core.urlresolvers import reverse, NoReverseMatch, reverse_lazy from django.core.validators import validate_email, ValidationError from django.db import IntegrityError, transaction @@ -624,6 +624,8 @@ def dashboard(request): """ user = request.user + if not UserProfile.objects.filter(user=user).exists(): + return redirect(reverse('account_settings')) platform_name = configuration_helpers.get_value("platform_name", settings.PLATFORM_NAME) enable_verified_certificates = configuration_helpers.get_value( @@ -2436,10 +2438,21 @@ def reactivation_email_for_user(user): "error": _('No inactive user with this e-mail exists'), }) # TODO: this should be status code 400 # pylint: disable=fixme - context = { - 'name': user.profile.name, - 'key': reg.activation_key, - } + try: + context = { + 'name': user.profile.name, + 'key': reg.activation_key, + } + except ObjectDoesNotExist: + log.error( + u'Unable to send reactivation email due to unavailable profile for the user "%s"', + user.username, + exc_info=True + ) + return JsonResponse({ + "success": False, + "error": _('Unable to send reactivation email') + }) subject = render_to_string('emails/activation_email_subject.txt', context) subject = ''.join(subject.splitlines()) diff --git a/lms/djangoapps/courseware/tests/test_i18n.py b/lms/djangoapps/courseware/tests/test_i18n.py index 094f7ed3e2..1d52ae06d7 100644 --- a/lms/djangoapps/courseware/tests/test_i18n.py +++ b/lms/djangoapps/courseware/tests/test_i18n.py @@ -15,6 +15,7 @@ from nose.plugins.attrib import attr from openedx.core.djangoapps.dark_lang.models import DarkLangConfig from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.user_api.preferences.api import set_user_preference +from student.models import UserProfile from student.tests.factories import UserFactory @@ -118,6 +119,7 @@ class I18nRegressionTests(BaseI18nTestCase): def test_unreleased_lang_resolution(self): # Regression test; LOC-85 + UserProfile.objects.create(user=self.user) self.release_languages('fa') self.user_login() @@ -134,6 +136,7 @@ class I18nRegressionTests(BaseI18nTestCase): self.assert_tag_has_attr(response.content, "html", "lang", "fa-ir") def test_preview_lang(self): + UserProfile.objects.create(user=self.user) self.user_login() # Regression test; LOC-87 @@ -169,6 +172,7 @@ class I18nLangPrefTests(BaseI18nTestCase): """ def setUp(self): super(I18nLangPrefTests, self).setUp() + UserProfile.objects.create(user=self.user) self.user_login() def test_lang_preference(self): diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 06c37d27ff..fe9754f88f 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -246,10 +246,11 @@ def _get_user_and_profile(username): """ try: existing_user = User.objects.get(username=username) - existing_user_profile = UserProfile.objects.get(user=existing_user) except ObjectDoesNotExist: raise UserNotFound() + existing_user_profile, _ = UserProfile.objects.get_or_create(user=existing_user) + return existing_user, existing_user_profile diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index 35ff9e2c9e..893893ef26 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -1,9 +1,12 @@ """ Django REST Framework serializers for the User API Accounts sub-application """ +import logging + from rest_framework import serializers from django.contrib.auth.models import User from django.conf import settings +from django.core.exceptions import ObjectDoesNotExist from django.core.urlresolvers import reverse from lms.djangoapps.badges.utils import badges_enabled @@ -18,6 +21,7 @@ from .image_helpers import get_profile_image_urls_for_user PROFILE_IMAGE_KEY_PREFIX = 'image_url' +LOGGER = logging.getLogger(__name__) class LanguageProficiencySerializer(serializers.ModelSerializer): @@ -63,7 +67,12 @@ class UserReadOnlySerializer(serializers.Serializer): :param user: User object :return: Dict serialized account """ - profile = user.profile + try: + user_profile = user.profile + except ObjectDoesNotExist: + user_profile = None + LOGGER.warning("user profile for the user [%s] does not exist", user.username) + accomplishments_shared = badges_enabled() data = { @@ -78,32 +87,51 @@ class UserReadOnlySerializer(serializers.Serializer): # https://docs.djangoproject.com/en/1.8/ref/databases/#fractional-seconds-support-for-time-and-datetime-fields "date_joined": user.date_joined.replace(microsecond=0), "is_active": user.is_active, - "bio": AccountLegacyProfileSerializer.convert_empty_to_None(profile.bio), - "country": AccountLegacyProfileSerializer.convert_empty_to_None(profile.country.code), - "profile_image": AccountLegacyProfileSerializer.get_profile_image( - profile, - user, - self.context.get('request') - ), - "language_proficiencies": LanguageProficiencySerializer( - profile.language_proficiencies.all(), - many=True - ).data, - "name": profile.name, - "gender": AccountLegacyProfileSerializer.convert_empty_to_None(profile.gender), - "goals": profile.goals, - "year_of_birth": profile.year_of_birth, - "level_of_education": AccountLegacyProfileSerializer.convert_empty_to_None(profile.level_of_education), - "mailing_address": profile.mailing_address, - "requires_parental_consent": profile.requires_parental_consent(), + "bio": None, + "country": None, + "profile_image": None, + "language_proficiencies": None, + "name": None, + "gender": None, + "goals": None, + "year_of_birth": None, + "level_of_education": None, + "mailing_address": None, + "requires_parental_consent": None, "accomplishments_shared": accomplishments_shared, - "account_privacy": get_profile_visibility(profile, user, self.configuration), + "account_privacy": self.configuration.get('default_visibility') } + if user_profile: + data.update( + { + "bio": AccountLegacyProfileSerializer.convert_empty_to_None(user_profile.bio), + "country": AccountLegacyProfileSerializer.convert_empty_to_None(user_profile.country.code), + "profile_image": AccountLegacyProfileSerializer.get_profile_image( + user_profile, user, self.context.get('request') + ), + "language_proficiencies": LanguageProficiencySerializer( + user_profile.language_proficiencies.all(), many=True + ).data, + "name": user_profile.name, + "gender": AccountLegacyProfileSerializer.convert_empty_to_None(user_profile.gender), + "goals": user_profile.goals, + "year_of_birth": user_profile.year_of_birth, + "level_of_education": AccountLegacyProfileSerializer.convert_empty_to_None( + user_profile.level_of_education + ), + "mailing_address": user_profile.mailing_address, + "requires_parental_consent": user_profile.requires_parental_consent(), + "account_privacy": get_profile_visibility(user_profile, user, self.configuration) + } + ) + if self.custom_fields: fields = self.custom_fields + elif user_profile: + fields = _visible_fields(user_profile, user, self.configuration) else: - fields = _visible_fields(profile, user, self.configuration) + fields = self.configuration.get('public_fields') return self._filter_fields( fields, diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_serializers.py b/openedx/core/djangoapps/user_api/accounts/tests/test_serializers.py new file mode 100644 index 0000000000..6d4ec161e2 --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_serializers.py @@ -0,0 +1,58 @@ +""" +Test cases to cover Accounts-related serializers of the User API application +""" +import logging + +from django.test import TestCase +from django.test.client import RequestFactory +from testfixtures import LogCapture + +from student.models import UserProfile +from student.tests.factories import UserFactory +from openedx.core.djangoapps.user_api.accounts.serializers import UserReadOnlySerializer + + +LOGGER_NAME = "openedx.core.djangoapps.user_api.accounts.serializers" + + +class UserReadOnlySerializerTest(TestCase): + def setUp(self): + super(UserReadOnlySerializerTest, self).setUp() + request_factory = RequestFactory() + self.request = request_factory.get('/api/user/v1/accounts/') + self.user = UserFactory.build(username='test_user', email='test_user@test.com') + self.user.save() + self.config = { + "default_visibility": "public", + + "shareable_fields": [ + 'name', + ], + + "public_fields": [ + 'email', 'name', 'username' + ], + } + + def test_serializer_data(self): + """ + Test serializer return data properly. + """ + UserProfile.objects.create(user=self.user, name='test name') + data = UserReadOnlySerializer(self.user, configuration=self.config, context={'request': self.request}).data + self.assertEqual(data['username'], self.user.username) + self.assertEqual(data['name'], 'test name') + self.assertEqual(data['email'], self.user.email) + + def test_user_no_profile(self): + """ + Test serializer return data properly when user does not have profile. + """ + with LogCapture(LOGGER_NAME, level=logging.DEBUG) as logger: + data = UserReadOnlySerializer(self.user, configuration=self.config, context={'request': self.request}).data + logger.check( + (LOGGER_NAME, 'WARNING', 'user profile for the user [test_user] does not exist') + ) + + self.assertEqual(data['username'], self.user.username) + self.assertEqual(data['name'], None) diff --git a/openedx/core/djangoapps/user_api/serializers.py b/openedx/core/djangoapps/user_api/serializers.py index d744c848fd..f46197cb95 100644 --- a/openedx/core/djangoapps/user_api/serializers.py +++ b/openedx/core/djangoapps/user_api/serializers.py @@ -20,9 +20,13 @@ class UserSerializer(serializers.HyperlinkedModelSerializer): def get_name(self, user): """ - Return the name attribute from the user profile object + Return the name attribute from the user profile object if profile exists else none """ - profile = UserProfile.objects.get(user=user) + try: + profile = UserProfile.objects.get(user=user) + except UserProfile.DoesNotExist: + return None + return profile.name def get_preferences(self, user):