diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 1838ff2e88..2bd0ea3d33 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -3,6 +3,7 @@ import logging import json from ipware.ip import get_ip +import pyuca from django.conf import settings from django.contrib import messages @@ -338,10 +339,13 @@ def account_settings_context(request): """ user = request.user + collator = pyuca.Collator() + sort_key = lambda item: collator.sort_key(unicode(item[1])) + 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) + countries.countries, key=sort_key ) ] 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 8145f2564f..e0890b3012 100644 --- a/lms/djangoapps/student_profile/views.py +++ b/lms/djangoapps/student_profile/views.py @@ -1,21 +1,22 @@ """ Views for a student's profile information. """ +import pyuca + from django.conf import settings from django.core.exceptions import ObjectDoesNotExist 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.preferences.api import get_user_preferences from openedx.core.djangoapps.user_api.accounts.api import get_account_settings -from openedx.core.djangoapps.user_api.errors import UserNotFound, UserNotAuthorized 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 _ @@ -33,8 +34,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 @@ -42,23 +44,20 @@ def learner_profile(request, username): try: return render_to_response( 'student_profile/learner_profile.html', - learner_profile_context(request, request.user.username, username, request.user.is_staff) + learner_profile_context(request.user, username, request.user.is_staff, request.build_absolute_uri) ) - except UserNotAuthorized: - return HttpResponse(status=403) - except UserNotFound: - return HttpResponse(status=404) - except ObjectDoesNotExist: - return HttpResponse(status=404) + except (UserNotAuthorized, UserNotFound, ObjectDoesNotExist): + raise Http404 -def learner_profile_context(request, 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 @@ -68,20 +67,22 @@ def learner_profile_context(request, logged_in_username, profile_username, user_ """ profile_user = User.objects.get(username=profile_username) + collator = pyuca.Collator() + sort_key = lambda item: collator.sort_key(unicode(item[1])) + 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) + countries.countries, key=sort_key ) ] + own_profile = (logged_in_user.username == profile_username) - own_profile = (logged_in_username == profile_username) - - accounts_data = get_account_settings(request.user, profile_username) + account_settings_data = get_account_settings(logged_in_user, profile_username) # Account for possibly relative URLs. - for key, value in accounts_data['profile_image'].items(): + for key, value in account_settings_data['profile_image'].items(): if key.startswith(PROFILE_IMAGE_KEY_PREFIX): - accounts_data['profile_image'][key] = request.build_absolute_uri(value) + account_settings_data['profile_image'][key] = build_absolute_uri_func(value) preferences_data = get_user_preferences(profile_user, profile_username) @@ -93,13 +94,13 @@ def learner_profile_context(request, logged_in_username, profile_username, user_ 'accounts_api_url': reverse("accounts_api", kwargs={'username': profile_username}), 'preferences_api_url': reverse('preferences_api', kwargs={'username': profile_username}), 'preferences_data': preferences_data, - 'accounts_data': accounts_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), + 'has_preferences_access': (logged_in_user.username == profile_username or user_is_staff), 'own_profile': own_profile, 'country_options': country_options, 'language_options': settings.ALL_LANGUAGES, 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 28ba6ab2af..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,18 +15,21 @@ return function (options) { var learnerProfileElement = $('.wrapper-profile'); - var defaultVisibility = options.default_visibility; - var accountPreferencesModel, accountSettingsModel; - if (options.own_profile) { - accountSettingsModel = new AccountSettingsModel({ - 'default_public_account_fields': options.default_public_account_fields - }); - accountPreferencesModel = new AccountPreferencesModel({account_privacy: defaultVisibility}); - } else { - accountSettingsModel = new AccountSettingsModel(options.accounts_data, {parse: true}); - accountPreferencesModel = new AccountPreferencesModel(options.preferences_data); - } + 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: options.default_visibility + } + }); + var accountPreferencesModel = new AccountPreferencesModelWithDefaults(options.preferences_data); + accountSettingsModel.url = options.accounts_api_url; accountPreferencesModel.url = options.preferences_api_url; @@ -123,10 +126,6 @@ sectionTwoFieldViews: sectionTwoFieldViews }); - var showLoadingError = function () { - learnerProfileView.showLoadingError(); - }; - var getProfileVisibility = function() { if (options.has_preferences_access) { return accountPreferencesModel.get('account_privacy'); @@ -147,34 +146,12 @@ learnerProfileView.render(); }; - if (options.own_profile) { - 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 - }); - } else { - if (options.has_preferences_access) { - if (accountSettingsModel.get('requires_parental_consent')) { - accountPreferencesModel.set('account_privacy', 'private'); - } + if (options.has_preferences_access) { + if (accountSettingsModel.get('requires_parental_consent')) { + accountPreferencesModel.set('account_privacy', 'private'); } - showLearnerProfileView(); } + showLearnerProfileView(); return { accountSettingsModel: accountSettingsModel, diff --git a/lms/static/js/views/fields.js b/lms/static/js/views/fields.js index 67e42e296d..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); }, @@ -201,15 +205,13 @@ this.$el.addClass('mode-edit'); }, - startEditing: function (event) { - event.preventDefault(); + startEditing: function () { if (this.editable === 'toggle' && this.mode !== 'edit') { this.showEditMode(true); } }, - finishEditing: function(event) { - event.preventDefault(); + finishEditing: function() { if (this.fieldValue() !== this.modelValue()) { this.saveValue(); } else { diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index dea5b62317..f7c026e143 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -21,6 +21,8 @@ git+https://github.com/mitocw/django-cas.git@60a5b8e5a62e63e0d5d224a87f0b489201a0c695#egg=django-cas -e git+https://github.com/dgrtwo/ParsePy.git@7949b9f754d1445eff8e8f20d0e967b9a6420639#egg=parse_rest git+https://github.com/mfogel/django-settings-context-processor.git@b758c3930862761216267715a70bbefd206ac03a#egg=django-settings-context-processor==0.2 +git+https://github.com/SmileyChris/pyuca.git@555292b39692e2c8a13fbba02a284fb89c9940e4#egg=pyuca==1.0 + # Master pyfs has a bug working with VPC auth. This is a fix. We should switch # back to master when and if this fix is merged back. # fs==0.4.0