diff --git a/common/test/acceptance/tests/lms/test_learner_profile.py b/common/test/acceptance/tests/lms/test_learner_profile.py index f02b1720c3..75a7c8e532 100644 --- a/common/test/acceptance/tests/lms/test_learner_profile.py +++ b/common/test/acceptance/tests/lms/test_learner_profile.py @@ -47,7 +47,7 @@ class LearnerProfileTestMixin(EventsTestMixin): Fill in the public profile fields of a user. """ profile_page.value_for_dropdown_field('language_proficiencies', 'English') - profile_page.value_for_dropdown_field('country', 'United Kingdom') + profile_page.value_for_dropdown_field('country', 'United Arab Emirates') profile_page.value_for_textarea_field('bio', 'Nothing Special') def visit_profile_page(self, username, privacy=None): diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 1838ff2e88..faf1c0a84b 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -338,13 +338,6 @@ def account_settings_context(request): """ user = request.user - country_options = [ - (country_code, _(country_name)) # pylint: disable=translation-of-non-string - for country_code, country_name in sorted( - countries.countries, key=lambda(__, name): unicode(name) - ) - ] - year_of_birth_options = [(unicode(year), unicode(year)) for year in UserProfile.VALID_YEARS] context = { @@ -352,7 +345,7 @@ def account_settings_context(request): 'duplicate_provider': None, 'fields': { 'country': { - 'options': country_options, + 'options': list(countries), }, 'gender': { 'options': [(choice[0], _(choice[1])) for choice in UserProfile.GENDER_CHOICES], # pylint: disable=translation-of-non-string }, 'language': { diff --git a/lms/djangoapps/student_profile/test/test_views.py b/lms/djangoapps/student_profile/test/test_views.py index aa78183108..7ce8174ee9 100644 --- a/lms/djangoapps/student_profile/test/test_views.py +++ b/lms/djangoapps/student_profile/test/test_views.py @@ -4,6 +4,7 @@ from django.conf import settings from django.core.urlresolvers import reverse from django.test import TestCase +from django.test.client import RequestFactory from util.testing import UrlResetMixin from student.tests.factories import UserFactory @@ -25,6 +26,8 @@ class LearnerProfileViewTest(UrlResetMixin, TestCase): 'own_profile', 'country_options', 'language_options', + 'account_settings_data', + 'preferences_data', ] def setUp(self): @@ -36,7 +39,9 @@ class LearnerProfileViewTest(UrlResetMixin, TestCase): """ Verify learner profile page context data. """ - context = learner_profile_context(self.user.username, self.USERNAME, self.user.is_staff) + request = RequestFactory().get('/url') + + context = learner_profile_context(self.user, self.USERNAME, self.user.is_staff, request.build_absolute_uri) self.assertEqual( context['data']['default_public_account_fields'], diff --git a/lms/djangoapps/student_profile/views.py b/lms/djangoapps/student_profile/views.py index 1a38b06d25..78354af3a4 100644 --- a/lms/djangoapps/student_profile/views.py +++ b/lms/djangoapps/student_profile/views.py @@ -6,12 +6,15 @@ from django_countries import countries from django.core.urlresolvers import reverse from django.contrib.auth.decorators import login_required -from django.http import HttpResponse +from django.http import Http404 from django.views.decorators.http import require_http_methods from edxmako.shortcuts import render_to_response +from openedx.core.djangoapps.user_api.accounts.api import get_account_settings +from openedx.core.djangoapps.user_api.accounts.serializers import PROFILE_IMAGE_KEY_PREFIX +from openedx.core.djangoapps.user_api.errors import UserNotFound, UserNotAuthorized +from openedx.core.djangoapps.user_api.preferences.api import get_user_preferences from student.models import User - from microsite_configuration import microsite from django.utils.translation import ugettext as _ @@ -29,8 +32,9 @@ def learner_profile(request, username): Returns: HttpResponse: 200 if the page was sent successfully HttpResponse: 302 if not logged in (redirect to login page) - HttpResponse: 404 if the specified username does not exist HttpResponse: 405 if using an unsupported HTTP method + Raises: + Http404: 404 if the specified user is not authorized or does not exist Example usage: GET /account/profile @@ -38,19 +42,20 @@ def learner_profile(request, username): try: return render_to_response( 'student_profile/learner_profile.html', - learner_profile_context(request.user.username, username, request.user.is_staff) + learner_profile_context(request.user, username, request.user.is_staff, request.build_absolute_uri) ) - except ObjectDoesNotExist: - return HttpResponse(status=404) + except (UserNotAuthorized, UserNotFound, ObjectDoesNotExist): + raise Http404 -def learner_profile_context(logged_in_username, profile_username, user_is_staff): +def learner_profile_context(logged_in_user, profile_username, user_is_staff, build_absolute_uri_func): """Context for the learner profile page. Args: - logged_in_username (str): Username of user logged In user. + logged_in_user (object): Logged In user. profile_username (str): username of user whose profile is requested. user_is_staff (bool): Logged In user has staff access. + build_absolute_uri_func (): Returns: dict @@ -60,12 +65,15 @@ def learner_profile_context(logged_in_username, profile_username, user_is_staff) """ profile_user = User.objects.get(username=profile_username) - country_options = [ - (country_code, _(country_name)) # pylint: disable=translation-of-non-string - for country_code, country_name in sorted( - countries.countries, key=lambda(__, name): unicode(name) - ) - ] + own_profile = (logged_in_user.username == profile_username) + + account_settings_data = get_account_settings(logged_in_user, profile_username) + # Account for possibly relative URLs. + for key, value in account_settings_data['profile_image'].items(): + if key.startswith(PROFILE_IMAGE_KEY_PREFIX): + account_settings_data['profile_image'][key] = build_absolute_uri_func(value) + + preferences_data = get_user_preferences(profile_user, profile_username) context = { 'data': { @@ -74,17 +82,18 @@ def learner_profile_context(logged_in_username, profile_username, user_is_staff) 'default_visibility': settings.ACCOUNT_VISIBILITY_CONFIGURATION['default_visibility'], 'accounts_api_url': reverse("accounts_api", kwargs={'username': profile_username}), 'preferences_api_url': reverse('preferences_api', kwargs={'username': profile_username}), + 'preferences_data': preferences_data, + 'account_settings_data': account_settings_data, 'profile_image_upload_url': reverse('profile_image_upload', kwargs={'username': profile_username}), 'profile_image_remove_url': reverse('profile_image_remove', kwargs={'username': profile_username}), 'profile_image_max_bytes': settings.PROFILE_IMAGE_MAX_BYTES, 'profile_image_min_bytes': settings.PROFILE_IMAGE_MIN_BYTES, 'account_settings_page_url': reverse('account_settings'), - 'has_preferences_access': (logged_in_username == profile_username or user_is_staff), - 'own_profile': (logged_in_username == profile_username), - 'country_options': country_options, + 'has_preferences_access': (logged_in_user.username == profile_username or user_is_staff), + 'own_profile': own_profile, + 'country_options': list(countries), 'language_options': settings.ALL_LANGUAGES, 'platform_name': microsite.get_value('platform_name', settings.PLATFORM_NAME), } } - return context diff --git a/lms/static/js/spec/student_profile/learner_profile_factory_spec.js b/lms/static/js/spec/student_profile/learner_profile_factory_spec.js index 3aa2cc2458..fb88a0d355 100644 --- a/lms/static/js/spec/student_profile/learner_profile_factory_spec.js +++ b/lms/static/js/spec/student_profile/learner_profile_factory_spec.js @@ -27,7 +27,7 @@ define(['backbone', 'jquery', 'underscore', 'js/common_helpers/ajax_helpers', 'j TemplateHelpers.installTemplate('templates/student_profile/learner_profile'); }); - var createProfilePage = function(ownProfile) { + var createProfilePage = function(ownProfile, options) { return new LearnerProfilePage({ 'accounts_api_url': Helpers.USER_ACCOUNTS_API_URL, 'preferences_api_url': Helpers.USER_PREFERENCES_API_URL, @@ -41,55 +41,13 @@ define(['backbone', 'jquery', 'underscore', 'js/common_helpers/ajax_helpers', 'j 'profile_image_upload_url': Helpers.IMAGE_UPLOAD_API_URL, 'profile_image_remove_url': Helpers.IMAGE_REMOVE_API_URL, 'default_visibility': 'all_users', - 'platform_name': 'edX' + 'platform_name': 'edX', + 'account_settings_data': Helpers.createAccountSettingsData(options), + 'preferences_data': Helpers.createUserPreferencesData() }); }; - it("show loading error when UserAccountModel fails to load", function() { - - requests = AjaxHelpers.requests(this); - - var context = createProfilePage(true), - learnerProfileView = context.learnerProfileView; - - var userAccountRequest = requests[0]; - expect(userAccountRequest.method).toBe('GET'); - expect(userAccountRequest.url).toBe(Helpers.USER_ACCOUNTS_API_URL); - - AjaxHelpers.respondWithError(requests, 500); - - Helpers.expectLoadingErrorIsVisible(learnerProfileView, true); - Helpers.expectLoadingIndicatorIsVisible(learnerProfileView, false); - LearnerProfileHelpers.expectProfileSectionsNotToBeRendered(learnerProfileView); - }); - - it("shows loading error when UserPreferencesModel fails to load", function() { - - requests = AjaxHelpers.requests(this); - - var context = createProfilePage(true), - learnerProfileView = context.learnerProfileView; - - var userAccountRequest = requests[0]; - expect(userAccountRequest.method).toBe('GET'); - expect(userAccountRequest.url).toBe(Helpers.USER_ACCOUNTS_API_URL); - - AjaxHelpers.respondWithJson(requests, Helpers.createAccountSettingsData()); - Helpers.expectLoadingIndicatorIsVisible(learnerProfileView, true); - Helpers.expectLoadingErrorIsVisible(learnerProfileView, false); - LearnerProfileHelpers.expectProfileSectionsNotToBeRendered(learnerProfileView); - - var userPreferencesRequest = requests[1]; - expect(userPreferencesRequest.method).toBe('GET'); - expect(userPreferencesRequest.url).toBe(Helpers.USER_PREFERENCES_API_URL); - - AjaxHelpers.respondWithError(requests, 500); - Helpers.expectLoadingIndicatorIsVisible(learnerProfileView, false); - Helpers.expectLoadingErrorIsVisible(learnerProfileView, true); - LearnerProfileHelpers.expectProfileSectionsNotToBeRendered(learnerProfileView); - }); - - it("renders the full profile after models are successfully fetched", function() { + it("renders the full profile after data is successfully fetched", function() { requests = AjaxHelpers.requests(this); @@ -106,33 +64,19 @@ define(['backbone', 'jquery', 'underscore', 'js/common_helpers/ajax_helpers', 'j it("renders the limited profile for undefined 'year_of_birth'", function() { - requests = AjaxHelpers.requests(this); - - var context = createProfilePage(true), + var context = createProfilePage(true, {year_of_birth: '', requires_parental_consent: true}), learnerProfileView = context.learnerProfileView; - AjaxHelpers.respondWithJson(requests, Helpers.createAccountSettingsData({ - year_of_birth: '', - requires_parental_consent: true - })); - AjaxHelpers.respondWithJson(requests, Helpers.createUserPreferencesData()); - LearnerProfileHelpers.expectLimitedProfileSectionsAndFieldsToBeRendered(learnerProfileView); }); it("renders the limited profile for under 13 users", function() { - requests = AjaxHelpers.requests(this); - - var context = createProfilePage(true), - learnerProfileView = context.learnerProfileView; - - AjaxHelpers.respondWithJson(requests, Helpers.createAccountSettingsData({ - year_of_birth: new Date().getFullYear() - 10, - requires_parental_consent: true - })); - AjaxHelpers.respondWithJson(requests, Helpers.createUserPreferencesData()); - + var context = createProfilePage( + true, + {year_of_birth: new Date().getFullYear() - 10, requires_parental_consent: true} + ); + var learnerProfileView = context.learnerProfileView; LearnerProfileHelpers.expectLimitedProfileSectionsAndFieldsToBeRendered(learnerProfileView); }); }); diff --git a/lms/static/js/student_account/models/user_account_model.js b/lms/static/js/student_account/models/user_account_model.js index e847d0ba88..d2e1791460 100644 --- a/lms/static/js/student_account/models/user_account_model.js +++ b/lms/static/js/student_account/models/user_account_model.js @@ -34,8 +34,8 @@ // Currently when a non-staff user A access user B's profile, the only way to tell whether user B's // profile is public is to check if the api has returned fields other than the default public fields // specified in settings.ACCOUNT_VISIBILITY_CONFIGURATION. - var profileIsPublic = _.size(_.difference(_.keys(response), this.get('default_public_account_fields'))) > 0; - this.set({'profile_is_public': profileIsPublic}, { silent: true }); + var responseKeys = _.filter(_.keys(response), function (key) {return key !== 'default_public_account_fields'}); + response.profile_is_public = _.size(_.difference(responseKeys, response.default_public_account_fields)) > 0; return response; }, diff --git a/lms/static/js/student_profile/views/learner_profile_factory.js b/lms/static/js/student_profile/views/learner_profile_factory.js index a66dc3f45e..033472f09e 100644 --- a/lms/static/js/student_profile/views/learner_profile_factory.js +++ b/lms/static/js/student_profile/views/learner_profile_factory.js @@ -15,19 +15,23 @@ return function (options) { var learnerProfileElement = $('.wrapper-profile'); - var defaultVisibility = options.default_visibility; + + var accountSettingsModel = new AccountSettingsModel( + _.extend( + options.account_settings_data, + {'default_public_account_fields': options.default_public_account_fields} + ), + {parse: true} + ); var AccountPreferencesModelWithDefaults = AccountPreferencesModel.extend({ defaults: { - account_privacy: defaultVisibility + account_privacy: options.default_visibility } }); - var accountPreferencesModel = new AccountPreferencesModelWithDefaults(); - accountPreferencesModel.url = options.preferences_api_url; + var accountPreferencesModel = new AccountPreferencesModelWithDefaults(options.preferences_data); - var accountSettingsModel = new AccountSettingsModel({ - 'default_public_account_fields': options.default_public_account_fields - }); accountSettingsModel.url = options.accounts_api_url; + accountPreferencesModel.url = options.preferences_api_url; var editable = options.own_profile ? 'toggle' : 'never'; @@ -122,10 +126,6 @@ sectionTwoFieldViews: sectionTwoFieldViews }); - var showLoadingError = function () { - learnerProfileView.showLoadingError(); - }; - var getProfileVisibility = function() { if (options.has_preferences_access) { return accountPreferencesModel.get('account_privacy'); @@ -146,26 +146,12 @@ learnerProfileView.render(); }; - accountSettingsModel.fetch({ - success: function () { - // Fetch the preferences model if the user has access - if (options.has_preferences_access) { - accountPreferencesModel.fetch({ - success: function() { - if (accountSettingsModel.get('requires_parental_consent')) { - accountPreferencesModel.set('account_privacy', 'private'); - } - showLearnerProfileView(); - }, - error: showLoadingError - }); - } - else { - showLearnerProfileView(); - } - }, - error: showLoadingError - }); + if (options.has_preferences_access) { + if (accountSettingsModel.get('requires_parental_consent')) { + accountPreferencesModel.set('account_privacy', 'private'); + } + } + showLearnerProfileView(); return { accountSettingsModel: accountSettingsModel, diff --git a/lms/static/js/views/fields.js b/lms/static/js/views/fields.js index c0424b4ada..c6aa7639c8 100644 --- a/lms/static/js/views/fields.js +++ b/lms/static/js/views/fields.js @@ -114,7 +114,11 @@ setTimeout(function () { if ((context === view.lastSuccessMessageContext) && (view.getNotificationMessage() === successMessage)) { - view.showHelpMessage(); + if (view.editable === 'toggle') { + view.showCanEditMessage(true); + } else { + view.showHelpMessage(); + } } }, messageRevertDelay); }, diff --git a/lms/static/sass/views/_learner-profile.scss b/lms/static/sass/views/_learner-profile.scss index b929151c98..7c56d30d08 100644 --- a/lms/static/sass/views/_learner-profile.scss +++ b/lms/static/sass/views/_learner-profile.scss @@ -218,7 +218,6 @@ .wrapper-profile-section-two { width: flex-grid(8, 12); - margin-top: ($baseline*1.5); } .profile-section-two-fields { diff --git a/lms/templates/register.html b/lms/templates/register.html index 51f954f46b..d9d0d7e91d 100644 --- a/lms/templates/register.html +++ b/lms/templates/register.html @@ -256,7 +256,7 @@ diff --git a/lms/templates/student_profile/learner_profile.html b/lms/templates/student_profile/learner_profile.html index d1159103c6..3d895ed744 100644 --- a/lms/templates/student_profile/learner_profile.html +++ b/lms/templates/student_profile/learner_profile.html @@ -1,6 +1,7 @@ <%! import json %> <%! from django.core.urlresolvers import reverse %> <%! from django.utils.translation import ugettext as _ %> +<%! from xmodule.modulestore import EdxJSONEncoder %> <%inherit file="/main.html" /> <%namespace name='static' file='/static_content.html'/> @@ -38,7 +39,7 @@