Team API include correct info when expanding users TNL-2975
Create read-only serializer to handle User/UserProfile data together. Refactor GET accounts to use the new read-only joint serializer Fix tests to check for new expanded user info Update tests in teams/tests/test_views.py to test for both public and private profiles on expands user Update serializer to utilize the request object for creating absolute uris
This commit is contained in:
@@ -40,8 +40,9 @@ class LearnerProfileViewTest(UrlResetMixin, TestCase):
|
||||
Verify learner profile page context data.
|
||||
"""
|
||||
request = RequestFactory().get('/url')
|
||||
request.user = self.user
|
||||
|
||||
context = learner_profile_context(self.user, self.USERNAME, self.user.is_staff, request.build_absolute_uri)
|
||||
context = learner_profile_context(request, self.USERNAME, self.user.is_staff)
|
||||
|
||||
self.assertEqual(
|
||||
context['data']['default_public_account_fields'],
|
||||
|
||||
@@ -40,13 +40,13 @@ def learner_profile(request, username):
|
||||
try:
|
||||
return render_to_response(
|
||||
'student_profile/learner_profile.html',
|
||||
learner_profile_context(request.user, username, request.user.is_staff, request.build_absolute_uri)
|
||||
learner_profile_context(request, username, request.user.is_staff)
|
||||
)
|
||||
except (UserNotAuthorized, UserNotFound, ObjectDoesNotExist):
|
||||
raise Http404
|
||||
|
||||
|
||||
def learner_profile_context(logged_in_user, profile_username, user_is_staff, build_absolute_uri_func):
|
||||
def learner_profile_context(request, profile_username, user_is_staff):
|
||||
"""Context for the learner profile page.
|
||||
|
||||
Args:
|
||||
@@ -62,14 +62,11 @@ def learner_profile_context(logged_in_user, profile_username, user_is_staff, bui
|
||||
ObjectDoesNotExist: the specified profile_username does not exist.
|
||||
"""
|
||||
profile_user = User.objects.get(username=profile_username)
|
||||
logged_in_user = request.user
|
||||
|
||||
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)
|
||||
account_settings_data = get_account_settings(request, profile_username)
|
||||
|
||||
preferences_data = get_user_preferences(profile_user, profile_username)
|
||||
|
||||
|
||||
@@ -0,0 +1,12 @@
|
||||
"""
|
||||
Defines common methods shared by Teams classes
|
||||
"""
|
||||
|
||||
from django.conf import settings
|
||||
|
||||
|
||||
def is_feature_enabled(course):
|
||||
"""
|
||||
Returns True if the teams feature is enabled.
|
||||
"""
|
||||
return settings.FEATURES.get('ENABLE_TEAMS', False) and course.teams_enabled
|
||||
|
||||
@@ -1,10 +1,9 @@
|
||||
"""
|
||||
Definition of the course team feature.
|
||||
"""
|
||||
|
||||
from django.utils.translation import ugettext_noop
|
||||
from courseware.tabs import EnrolledTab
|
||||
from .views import is_feature_enabled
|
||||
from teams import is_feature_enabled
|
||||
|
||||
|
||||
class TeamsTab(EnrolledTab):
|
||||
|
||||
@@ -1,13 +1,14 @@
|
||||
"""Defines serializers used by the Team API."""
|
||||
|
||||
from copy import deepcopy
|
||||
from django.contrib.auth.models import User
|
||||
from django.db.models import Count
|
||||
from django.conf import settings
|
||||
|
||||
from rest_framework import serializers
|
||||
|
||||
from openedx.core.lib.api.serializers import CollapsedReferenceSerializer, PaginationSerializer
|
||||
from openedx.core.lib.api.fields import ExpandableField
|
||||
from openedx.core.djangoapps.user_api.serializers import UserSerializer
|
||||
from openedx.core.djangoapps.user_api.accounts.serializers import UserReadOnlySerializer
|
||||
|
||||
from .models import CourseTeam, CourseTeamMembership
|
||||
|
||||
@@ -17,6 +18,10 @@ class UserMembershipSerializer(serializers.ModelSerializer):
|
||||
|
||||
Used for listing team members.
|
||||
"""
|
||||
profile_configuration = deepcopy(settings.ACCOUNT_VISIBILITY_CONFIGURATION)
|
||||
profile_configuration['shareable_fields'].append('url')
|
||||
profile_configuration['public_fields'].append('url')
|
||||
|
||||
user = ExpandableField(
|
||||
collapsed_serializer=CollapsedReferenceSerializer(
|
||||
model_class=User,
|
||||
@@ -24,7 +29,7 @@ class UserMembershipSerializer(serializers.ModelSerializer):
|
||||
view_name='accounts_api',
|
||||
read_only=True,
|
||||
),
|
||||
expanded_serializer=UserSerializer(),
|
||||
expanded_serializer=UserReadOnlySerializer(configuration=profile_configuration),
|
||||
)
|
||||
|
||||
class Meta(object):
|
||||
@@ -87,6 +92,10 @@ class CourseTeamCreationSerializer(serializers.ModelSerializer):
|
||||
|
||||
class MembershipSerializer(serializers.ModelSerializer):
|
||||
"""Serializes CourseTeamMemberships with information about both teams and users."""
|
||||
profile_configuration = deepcopy(settings.ACCOUNT_VISIBILITY_CONFIGURATION)
|
||||
profile_configuration['shareable_fields'].append('url')
|
||||
profile_configuration['public_fields'].append('url')
|
||||
|
||||
user = ExpandableField(
|
||||
collapsed_serializer=CollapsedReferenceSerializer(
|
||||
model_class=User,
|
||||
@@ -94,7 +103,7 @@ class MembershipSerializer(serializers.ModelSerializer):
|
||||
view_name='accounts_api',
|
||||
read_only=True,
|
||||
),
|
||||
expanded_serializer=UserSerializer(read_only=True)
|
||||
expanded_serializer=UserReadOnlySerializer(configuration=profile_configuration)
|
||||
)
|
||||
team = ExpandableField(
|
||||
collapsed_serializer=CollapsedReferenceSerializer(
|
||||
|
||||
@@ -110,7 +110,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
|
||||
@classmethod
|
||||
def setUpClass(cls):
|
||||
super(TeamAPITestCase, cls).setUpClass()
|
||||
teams_configuration = {
|
||||
teams_configuration_1 = {
|
||||
'topics':
|
||||
[
|
||||
{
|
||||
@@ -124,9 +124,30 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
|
||||
org='TestX',
|
||||
course='TS101',
|
||||
display_name='Test Course',
|
||||
teams_configuration=teams_configuration
|
||||
teams_configuration=teams_configuration_1
|
||||
)
|
||||
|
||||
teams_configuration_2 = {
|
||||
'topics':
|
||||
[
|
||||
{
|
||||
'id': 'topic_5',
|
||||
'name': 'Other Interests',
|
||||
'description': 'Description for topic 5.'
|
||||
},
|
||||
{
|
||||
'id': 'topic_6',
|
||||
'name': 'Public Profiles',
|
||||
'description': 'Description for topic 6.'
|
||||
},
|
||||
]
|
||||
}
|
||||
cls.test_course_2 = CourseFactory.create(
|
||||
org='MIT',
|
||||
course='6.002x',
|
||||
display_name='Circuits',
|
||||
teams_configuration=teams_configuration_2
|
||||
)
|
||||
cls.test_course_2 = CourseFactory.create(org='MIT', course='6.002x', display_name='Circuits')
|
||||
|
||||
def setUp(self):
|
||||
super(TeamAPITestCase, self).setUp()
|
||||
@@ -152,6 +173,15 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
|
||||
username='student_enrolled_both_courses_other_team'
|
||||
)
|
||||
|
||||
# Make this student have a public profile
|
||||
self.create_and_enroll_student(
|
||||
courses=[self.test_course_2],
|
||||
username='student_enrolled_public_profile'
|
||||
)
|
||||
profile = self.users['student_enrolled_public_profile'].profile
|
||||
profile.year_of_birth = 1970
|
||||
profile.save()
|
||||
|
||||
# 'solar team' is intentionally lower case to test case insensitivity in name ordering
|
||||
self.test_team_1 = CourseTeamFactory.create(
|
||||
name=u'sólar team',
|
||||
@@ -162,11 +192,13 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
|
||||
self.test_team_3 = CourseTeamFactory.create(name='Nuclear Team', course_id=self.test_course_1.id)
|
||||
self.test_team_4 = CourseTeamFactory.create(name='Coal Team', course_id=self.test_course_1.id, is_active=False)
|
||||
self.test_team_5 = CourseTeamFactory.create(name='Another Team', course_id=self.test_course_2.id)
|
||||
self.test_team_6 = CourseTeamFactory.create(
|
||||
name='Public Profile Team',
|
||||
course_id=self.test_course_2.id,
|
||||
topic_id='topic_6'
|
||||
)
|
||||
|
||||
for user, course in [
|
||||
('staff', self.test_course_1),
|
||||
('course_staff', self.test_course_1),
|
||||
]:
|
||||
for user, course in [('staff', self.test_course_1), ('course_staff', self.test_course_1)]:
|
||||
CourseEnrollment.enroll(
|
||||
self.users[user], course.id, check_access=True
|
||||
)
|
||||
@@ -174,6 +206,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
|
||||
self.test_team_1.add_user(self.users['student_enrolled'])
|
||||
self.test_team_3.add_user(self.users['student_enrolled_both_courses_other_team'])
|
||||
self.test_team_5.add_user(self.users['student_enrolled_both_courses_other_team'])
|
||||
self.test_team_6.add_user(self.users['student_enrolled_public_profile'])
|
||||
|
||||
def create_and_enroll_student(self, courses=None, username=None):
|
||||
""" Creates a new student and enrolls that student in the course.
|
||||
@@ -305,11 +338,18 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
|
||||
**kwargs
|
||||
)
|
||||
|
||||
def verify_expanded_user(self, user):
|
||||
def verify_expanded_public_user(self, user):
|
||||
"""Verifies that fields exist on the returned user json indicating that it is expanded."""
|
||||
for field in ['id', 'url', 'email', 'name', 'username', 'preferences']:
|
||||
for field in ['username', 'url', 'bio', 'country', 'profile_image', 'time_zone', 'language_proficiencies']:
|
||||
self.assertIn(field, user)
|
||||
|
||||
def verify_expanded_private_user(self, user):
|
||||
"""Verifies that fields exist on the returned user json indicating that it is expanded."""
|
||||
for field in ['username', 'url', 'profile_image']:
|
||||
self.assertIn(field, user)
|
||||
for field in ['bio', 'country', 'time_zone', 'language_proficiencies']:
|
||||
self.assertNotIn(field, user)
|
||||
|
||||
def verify_expanded_team(self, team):
|
||||
"""Verifies that fields exist on the returned team json indicating that it is expanded."""
|
||||
for field in ['id', 'name', 'is_active', 'course_id', 'topic_id', 'date_created', 'description']:
|
||||
@@ -347,7 +387,12 @@ class TestListTeamsAPI(TeamAPITestCase):
|
||||
self.verify_names({'course_id': 'no_such_course'}, 400)
|
||||
|
||||
def test_filter_course_id(self):
|
||||
self.verify_names({'course_id': self.test_course_2.id}, 200, ['Another Team'], user='staff')
|
||||
self.verify_names(
|
||||
{'course_id': self.test_course_2.id},
|
||||
200,
|
||||
['Another Team', 'Public Profile Team'],
|
||||
user='staff'
|
||||
)
|
||||
|
||||
def test_filter_topic_id(self):
|
||||
self.verify_names({'course_id': self.test_course_1.id, 'topic_id': 'topic_0'}, 200, [u'sólar team'])
|
||||
@@ -386,9 +431,22 @@ class TestListTeamsAPI(TeamAPITestCase):
|
||||
self.assertIsNone(result['next'])
|
||||
self.assertIsNotNone(result['previous'])
|
||||
|
||||
def test_expand_user(self):
|
||||
def test_expand_private_user(self):
|
||||
# Use the default user which is already private because to year_of_birth is set
|
||||
result = self.get_teams_list(200, {'expand': 'user', 'topic_id': 'topic_0'})
|
||||
self.verify_expanded_user(result['results'][0]['membership'][0]['user'])
|
||||
self.verify_expanded_private_user(result['results'][0]['membership'][0]['user'])
|
||||
|
||||
def test_expand_public_user(self):
|
||||
result = self.get_teams_list(
|
||||
200,
|
||||
{
|
||||
'expand': 'user',
|
||||
'topic_id': 'topic_6',
|
||||
'course_id': self.test_course_2.id
|
||||
},
|
||||
user='student_enrolled_public_profile'
|
||||
)
|
||||
self.verify_expanded_public_user(result['results'][0]['membership'][0]['user'])
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@@ -515,9 +573,19 @@ class TestDetailTeamAPI(TeamAPITestCase):
|
||||
def test_does_not_exist(self):
|
||||
self.get_team_detail('no_such_team', 404)
|
||||
|
||||
def test_expand_user(self):
|
||||
def test_expand_private_user(self):
|
||||
# Use the default user which is already private because to year_of_birth is set
|
||||
result = self.get_team_detail(self.test_team_1.team_id, 200, {'expand': 'user'})
|
||||
self.verify_expanded_user(result['membership'][0]['user'])
|
||||
self.verify_expanded_private_user(result['membership'][0]['user'])
|
||||
|
||||
def test_expand_public_user(self):
|
||||
result = self.get_team_detail(
|
||||
self.test_team_6.team_id,
|
||||
200,
|
||||
{'expand': 'user'},
|
||||
user='student_enrolled_public_profile'
|
||||
)
|
||||
self.verify_expanded_public_user(result['membership'][0]['user'])
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@@ -715,9 +783,18 @@ class TestListMembershipAPI(TeamAPITestCase):
|
||||
def test_bad_team_id(self):
|
||||
self.get_membership_list(404, {'team_id': 'no_such_team'})
|
||||
|
||||
def test_expand_user(self):
|
||||
def test_expand_private_user(self):
|
||||
# Use the default user which is already private because to year_of_birth is set
|
||||
result = self.get_membership_list(200, {'team_id': self.test_team_1.team_id, 'expand': 'user'})
|
||||
self.verify_expanded_user(result['results'][0]['user'])
|
||||
self.verify_expanded_private_user(result['results'][0]['user'])
|
||||
|
||||
def test_expand_public_user(self):
|
||||
result = self.get_membership_list(
|
||||
200,
|
||||
{'team_id': self.test_team_6.team_id, 'expand': 'user'},
|
||||
user='student_enrolled_public_profile'
|
||||
)
|
||||
self.verify_expanded_public_user(result['results'][0]['user'])
|
||||
|
||||
def test_expand_team(self):
|
||||
result = self.get_membership_list(200, {'team_id': self.test_team_1.team_id, 'expand': 'team'})
|
||||
@@ -842,14 +919,25 @@ class TestDetailMembershipAPI(TeamAPITestCase):
|
||||
404
|
||||
)
|
||||
|
||||
def test_expand_user(self):
|
||||
def test_expand_private_user(self):
|
||||
# Use the default user which is already private because to year_of_birth is set
|
||||
result = self.get_membership_detail(
|
||||
self.test_team_1.team_id,
|
||||
self.users['student_enrolled'].username,
|
||||
200,
|
||||
{'expand': 'user'}
|
||||
)
|
||||
self.verify_expanded_user(result['user'])
|
||||
self.verify_expanded_private_user(result['user'])
|
||||
|
||||
def test_expand_public_user(self):
|
||||
result = self.get_membership_detail(
|
||||
self.test_team_6.team_id,
|
||||
self.users['student_enrolled_public_profile'].username,
|
||||
200,
|
||||
{'expand': 'user'},
|
||||
user='student_enrolled_public_profile'
|
||||
)
|
||||
self.verify_expanded_public_user(result['user'])
|
||||
|
||||
def test_expand_team(self):
|
||||
result = self.get_membership_detail(
|
||||
|
||||
@@ -1,13 +1,11 @@
|
||||
"""HTTP endpoints for the Teams API."""
|
||||
|
||||
from django.shortcuts import render_to_response
|
||||
from courseware.courses import get_course_with_access, has_access
|
||||
from django.http import Http404
|
||||
from django.conf import settings
|
||||
from django.core.paginator import Paginator
|
||||
from django.views.generic.base import View
|
||||
import newrelic.agent
|
||||
|
||||
from rest_framework.generics import GenericAPIView
|
||||
from rest_framework.response import Response
|
||||
from rest_framework.reverse import reverse
|
||||
@@ -18,16 +16,11 @@ from rest_framework.authentication import (
|
||||
)
|
||||
from rest_framework import status
|
||||
from rest_framework import permissions
|
||||
|
||||
from django.db.models import Count
|
||||
from django.contrib.auth.models import User
|
||||
from django_countries import countries
|
||||
from django.utils.translation import ugettext as _
|
||||
from django.utils.translation import ugettext_noop
|
||||
|
||||
from student.models import CourseEnrollment, CourseAccessRole
|
||||
from student.roles import CourseStaffRole
|
||||
|
||||
from openedx.core.lib.api.parsers import MergePatchParser
|
||||
from openedx.core.lib.api.permissions import IsStaffOrReadOnly
|
||||
from openedx.core.lib.api.view_utils import (
|
||||
@@ -37,14 +30,15 @@ from openedx.core.lib.api.view_utils import (
|
||||
ExpandableFieldViewMixin
|
||||
)
|
||||
from openedx.core.lib.api.serializers import PaginationSerializer
|
||||
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
from opaque_keys import InvalidKeyError
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
|
||||
from courseware.courses import get_course_with_access, has_access
|
||||
from student.models import CourseEnrollment, CourseAccessRole
|
||||
from student.roles import CourseStaffRole
|
||||
from django_comment_client.utils import has_discussion_privileges
|
||||
|
||||
from teams import is_feature_enabled
|
||||
from .models import CourseTeam, CourseTeamMembership
|
||||
from .serializers import (
|
||||
CourseTeamSerializer,
|
||||
@@ -58,7 +52,6 @@ from .serializers import (
|
||||
from .errors import AlreadyOnTeamInCourse, NotEnrolledInCourseForTeam
|
||||
|
||||
|
||||
# Constants
|
||||
TEAM_MEMBERSHIPS_PER_PAGE = 2
|
||||
TOPICS_PER_PAGE = 12
|
||||
|
||||
@@ -120,13 +113,6 @@ class TeamsDashboardView(View):
|
||||
return render_to_response("teams/teams.html", context)
|
||||
|
||||
|
||||
def is_feature_enabled(course):
|
||||
"""
|
||||
Returns True if the teams feature is enabled.
|
||||
"""
|
||||
return settings.FEATURES.get('ENABLE_TEAMS', False) and course.teams_enabled
|
||||
|
||||
|
||||
def has_team_api_access(user, course_key, access_username=None):
|
||||
"""Returns True if the user has access to the Team API for the course
|
||||
given by `course_key`. The user must either be enrolled in the course,
|
||||
|
||||
@@ -1482,7 +1482,9 @@ class TestSubmitPhotosForVerification(TestCase):
|
||||
AssertionError
|
||||
|
||||
"""
|
||||
account_settings = get_account_settings(self.user)
|
||||
request = RequestFactory().get('/url')
|
||||
request.user = self.user
|
||||
account_settings = get_account_settings(request)
|
||||
self.assertEqual(account_settings['name'], full_name)
|
||||
|
||||
def _get_post_data(self):
|
||||
|
||||
@@ -2517,6 +2517,25 @@ ACCOUNT_VISIBILITY_CONFIGURATION = {
|
||||
'username',
|
||||
'profile_image',
|
||||
],
|
||||
|
||||
# The list of account fields that are visible only to staff and users viewing their own profiles
|
||||
"admin_fields": [
|
||||
"username",
|
||||
"email",
|
||||
"date_joined",
|
||||
"is_active",
|
||||
"bio",
|
||||
"country",
|
||||
"profile_image",
|
||||
"language_proficiencies",
|
||||
"name",
|
||||
"gender",
|
||||
"goals",
|
||||
"year_of_birth",
|
||||
"level_of_education",
|
||||
"mailing_address",
|
||||
"requires_parental_consent",
|
||||
]
|
||||
}
|
||||
|
||||
# E-Commerce API Configuration
|
||||
|
||||
@@ -22,84 +22,67 @@ from ..helpers import intercept_errors
|
||||
from ..models import UserPreference
|
||||
|
||||
from . import (
|
||||
ACCOUNT_VISIBILITY_PREF_KEY, ALL_USERS_VISIBILITY, PRIVATE_VISIBILITY,
|
||||
ACCOUNT_VISIBILITY_PREF_KEY, PRIVATE_VISIBILITY,
|
||||
EMAIL_MIN_LENGTH, EMAIL_MAX_LENGTH, PASSWORD_MIN_LENGTH, PASSWORD_MAX_LENGTH,
|
||||
USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH
|
||||
)
|
||||
from .serializers import AccountLegacyProfileSerializer, AccountUserSerializer
|
||||
from .serializers import (
|
||||
AccountLegacyProfileSerializer, AccountUserSerializer,
|
||||
UserReadOnlySerializer
|
||||
)
|
||||
|
||||
|
||||
@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError])
|
||||
def get_account_settings(requesting_user, username=None, configuration=None, view=None):
|
||||
def get_account_settings(request, username=None, configuration=None, view=None):
|
||||
"""Returns account information for a user serialized as JSON.
|
||||
|
||||
Note:
|
||||
If `requesting_user.username` != `username`, this method will return differing amounts of information
|
||||
based on who `requesting_user` is and the privacy settings of the user associated with `username`.
|
||||
If `request.user.username` != `username`, this method will return differing amounts of information
|
||||
based on who `request.user` is and the privacy settings of the user associated with `username`.
|
||||
|
||||
Args:
|
||||
requesting_user (User): The user requesting the account information. Only the user with username
|
||||
`username` or users with "is_staff" privileges can get full account information.
|
||||
Other users will get the account fields that the user has elected to share.
|
||||
request (Request): The request object with account information about the requesting user.
|
||||
Only the user with username `username` or users with "is_staff" privileges can get full
|
||||
account information. Other users will get the account fields that the user has elected to share.
|
||||
username (str): Optional username for the desired account information. If not specified,
|
||||
`requesting_user.username` is assumed.
|
||||
`request.user.username` is assumed.
|
||||
configuration (dict): an optional configuration specifying which fields in the account
|
||||
can be shared, and the default visibility settings. If not present, the setting value with
|
||||
key ACCOUNT_VISIBILITY_CONFIGURATION is used.
|
||||
view (str): An optional string allowing "is_staff" users and users requesting their own
|
||||
account information to get just the fields that are shared with everyone. If view is
|
||||
"shared", only shared account information will be returned, regardless of `requesting_user`.
|
||||
"shared", only shared account information will be returned, regardless of `request.user`.
|
||||
|
||||
Returns:
|
||||
A dict containing account fields.
|
||||
|
||||
Raises:
|
||||
UserNotFound: no user with username `username` exists (or `requesting_user.username` if
|
||||
UserNotFound: no user with username `username` exists (or `request.user.username` if
|
||||
`username` is not specified)
|
||||
UserAPIInternalError: the operation failed due to an unexpected error.
|
||||
"""
|
||||
requesting_user = request.user
|
||||
|
||||
if username is None:
|
||||
username = requesting_user.username
|
||||
|
||||
try:
|
||||
existing_user = User.objects.get(username=username)
|
||||
except ObjectDoesNotExist:
|
||||
raise UserNotFound()
|
||||
|
||||
has_full_access = requesting_user.username == username or requesting_user.is_staff
|
||||
return_all_fields = has_full_access and view != 'shared'
|
||||
|
||||
existing_user, existing_user_profile = _get_user_and_profile(username)
|
||||
|
||||
user_serializer = AccountUserSerializer(existing_user)
|
||||
legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile)
|
||||
|
||||
account_settings = dict(user_serializer.data, **legacy_profile_serializer.data)
|
||||
|
||||
if return_all_fields:
|
||||
return account_settings
|
||||
|
||||
if not configuration:
|
||||
configuration = settings.ACCOUNT_VISIBILITY_CONFIGURATION
|
||||
|
||||
visible_settings = {}
|
||||
|
||||
profile_visibility = _get_profile_visibility(existing_user_profile, configuration)
|
||||
if profile_visibility == ALL_USERS_VISIBILITY:
|
||||
field_names = configuration.get('shareable_fields')
|
||||
if has_full_access and view != 'shared':
|
||||
admin_fields = settings.ACCOUNT_VISIBILITY_CONFIGURATION.get('admin_fields')
|
||||
else:
|
||||
field_names = configuration.get('public_fields')
|
||||
admin_fields = None
|
||||
|
||||
for field_name in field_names:
|
||||
visible_settings[field_name] = account_settings.get(field_name, None)
|
||||
|
||||
return visible_settings
|
||||
|
||||
|
||||
def _get_profile_visibility(user_profile, configuration):
|
||||
"""Returns the visibility level for the specified user profile."""
|
||||
if user_profile.requires_parental_consent():
|
||||
return PRIVATE_VISIBILITY
|
||||
|
||||
# Calling UserPreference directly because the requesting user may be different from existing_user
|
||||
# (and does not have to be is_staff).
|
||||
profile_privacy = UserPreference.get_value(user_profile.user, ACCOUNT_VISIBILITY_PREF_KEY)
|
||||
return profile_privacy if profile_privacy else configuration.get('default_visibility')
|
||||
return UserReadOnlySerializer(
|
||||
existing_user,
|
||||
configuration=configuration,
|
||||
custom_fields=admin_fields,
|
||||
context={'request': request}
|
||||
).data
|
||||
|
||||
|
||||
@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError])
|
||||
|
||||
@@ -72,7 +72,7 @@ def get_profile_image_names(username):
|
||||
return {size: _get_profile_image_filename(name, size) for size in _PROFILE_IMAGE_SIZES}
|
||||
|
||||
|
||||
def get_profile_image_urls_for_user(user):
|
||||
def get_profile_image_urls_for_user(user, request=None):
|
||||
"""
|
||||
Return a dict {size:url} for each profile image for a given user.
|
||||
Notes:
|
||||
@@ -93,13 +93,19 @@ def get_profile_image_urls_for_user(user):
|
||||
|
||||
"""
|
||||
if user.profile.has_profile_image:
|
||||
return _get_profile_image_urls(
|
||||
urls = _get_profile_image_urls(
|
||||
_make_profile_image_name(user.username),
|
||||
get_profile_image_storage(),
|
||||
version=user.profile.profile_image_uploaded_at.strftime("%s"),
|
||||
)
|
||||
else:
|
||||
return _get_default_profile_image_urls()
|
||||
urls = _get_default_profile_image_urls()
|
||||
|
||||
if request:
|
||||
for key, value in urls.items():
|
||||
urls[key] = request.build_absolute_uri(value)
|
||||
|
||||
return urls
|
||||
|
||||
|
||||
def _get_default_profile_image_urls():
|
||||
|
||||
@@ -1,10 +1,16 @@
|
||||
from rest_framework import serializers
|
||||
from django.contrib.auth.models import User
|
||||
from django.conf import settings
|
||||
from django.core.urlresolvers import reverse
|
||||
from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH
|
||||
from openedx.core.djangoapps.user_api.serializers import ReadOnlyFieldsSerializerMixin
|
||||
|
||||
from student.models import UserProfile, LanguageProficiency
|
||||
from ..models import UserPreference
|
||||
from .image_helpers import get_profile_image_urls_for_user
|
||||
from . import (
|
||||
ACCOUNT_VISIBILITY_PREF_KEY, ALL_USERS_VISIBILITY, PRIVATE_VISIBILITY,
|
||||
)
|
||||
|
||||
|
||||
PROFILE_IMAGE_KEY_PREFIX = 'image_url'
|
||||
@@ -32,6 +38,104 @@ class LanguageProficiencySerializer(serializers.ModelSerializer):
|
||||
return None
|
||||
|
||||
|
||||
class UserReadOnlySerializer(serializers.Serializer):
|
||||
"""
|
||||
Class that serializes the User model and UserProfile model together.
|
||||
"""
|
||||
def __init__(self, *args, **kwargs):
|
||||
# Don't pass the 'configuration' arg up to the superclass
|
||||
self.configuration = kwargs.pop('configuration', None)
|
||||
if not self.configuration:
|
||||
self.configuration = settings.ACCOUNT_VISIBILITY_CONFIGURATION
|
||||
|
||||
# Don't pass the 'custom_fields' arg up to the superclass
|
||||
self.custom_fields = kwargs.pop('custom_fields', None)
|
||||
|
||||
super(UserReadOnlySerializer, self).__init__(*args, **kwargs)
|
||||
|
||||
def to_native(self, user):
|
||||
"""
|
||||
Overwrite to_native to handle custom logic since we are serializing two models as one here
|
||||
:param user: User object
|
||||
:return: Dict serialized account
|
||||
"""
|
||||
profile = user.profile
|
||||
|
||||
data = {
|
||||
"username": user.username,
|
||||
"url": self.context.get('request').build_absolute_uri(
|
||||
reverse('accounts_api', kwargs={'username': user.username})
|
||||
),
|
||||
"email": user.email,
|
||||
"date_joined": user.date_joined,
|
||||
"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')
|
||||
),
|
||||
"time_zone": None,
|
||||
"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(),
|
||||
}
|
||||
|
||||
return self._filter_fields(
|
||||
self._visible_fields(profile, user),
|
||||
data
|
||||
)
|
||||
|
||||
def _visible_fields(self, user_profile, user):
|
||||
"""
|
||||
Return what fields should be visible based on user settings
|
||||
|
||||
:param user_profile: User profile object
|
||||
:param user: User object
|
||||
:return: whitelist List of fields to be shown
|
||||
"""
|
||||
|
||||
if self.custom_fields:
|
||||
return self.custom_fields
|
||||
|
||||
profile_visibility = self._get_profile_visibility(user_profile, user)
|
||||
|
||||
if profile_visibility == ALL_USERS_VISIBILITY:
|
||||
return self.configuration.get('shareable_fields')
|
||||
else:
|
||||
return self.configuration.get('public_fields')
|
||||
|
||||
def _get_profile_visibility(self, user_profile, user):
|
||||
"""Returns the visibility level for the specified user profile."""
|
||||
if user_profile.requires_parental_consent():
|
||||
return PRIVATE_VISIBILITY
|
||||
|
||||
# Calling UserPreference directly because the requesting user may be different from existing_user
|
||||
# (and does not have to be is_staff).
|
||||
profile_privacy = UserPreference.get_value(user, ACCOUNT_VISIBILITY_PREF_KEY)
|
||||
return profile_privacy if profile_privacy else self.configuration.get('default_visibility')
|
||||
|
||||
def _filter_fields(self, field_whitelist, serialized_account):
|
||||
"""
|
||||
Filter serialized account Dict to only include whitelisted keys
|
||||
"""
|
||||
visible_serialized_account = {}
|
||||
|
||||
for field_name in field_whitelist:
|
||||
visible_serialized_account[field_name] = serialized_account.get(field_name, None)
|
||||
|
||||
return visible_serialized_account
|
||||
|
||||
|
||||
class AccountUserSerializer(serializers.HyperlinkedModelSerializer, ReadOnlyFieldsSerializerMixin):
|
||||
"""
|
||||
Class that serializes the portion of User model needed for account information.
|
||||
@@ -47,7 +151,7 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
|
||||
"""
|
||||
Class that serializes the portion of UserProfile model needed for account information.
|
||||
"""
|
||||
profile_image = serializers.SerializerMethodField("get_profile_image")
|
||||
profile_image = serializers.SerializerMethodField("_get_profile_image")
|
||||
requires_parental_consent = serializers.SerializerMethodField("get_requires_parental_consent")
|
||||
language_proficiencies = LanguageProficiencySerializer(many=True, allow_add_remove=True, required=False)
|
||||
|
||||
@@ -102,10 +206,11 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
|
||||
""" Helper method to convert empty string to None (other values pass through). """
|
||||
return None if value == "" else value
|
||||
|
||||
def get_profile_image(self, user_profile):
|
||||
@staticmethod
|
||||
def get_profile_image(user_profile, user, request=None):
|
||||
""" Returns metadata about a user's profile image. """
|
||||
data = {'has_image': user_profile.has_profile_image}
|
||||
urls = get_profile_image_urls_for_user(user_profile.user)
|
||||
urls = get_profile_image_urls_for_user(user, request)
|
||||
data.update({
|
||||
'{image_key_prefix}_{size}'.format(image_key_prefix=PROFILE_IMAGE_KEY_PREFIX, size=size_display_name): url
|
||||
for size_display_name, url in urls.items()
|
||||
@@ -115,3 +220,13 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
|
||||
def get_requires_parental_consent(self, user_profile):
|
||||
""" Returns a boolean representing whether the user requires parental controls. """
|
||||
return user_profile.requires_parental_consent()
|
||||
|
||||
def _get_profile_image(self, user_profile):
|
||||
"""
|
||||
Returns metadata about a user's profile image
|
||||
|
||||
This protected method delegates to the static 'get_profile_image' method
|
||||
because 'serializers.SerializerMethodField("_get_profile_image")' will
|
||||
call the method with a single argument, the user_profile object.
|
||||
"""
|
||||
return AccountLegacyProfileSerializer.get_profile_image(user_profile, user_profile.user)
|
||||
|
||||
@@ -15,6 +15,7 @@ from student.tests.factories import UserFactory
|
||||
from django.conf import settings
|
||||
from django.contrib.auth.models import User
|
||||
from django.core import mail
|
||||
from django.test.client import RequestFactory
|
||||
from student.models import PendingEmailChange
|
||||
from student.tests.tests import UserSettingsEventTestMixin
|
||||
from ...errors import (
|
||||
@@ -43,21 +44,24 @@ class TestAccountApi(UserSettingsEventTestMixin, TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(TestAccountApi, self).setUp()
|
||||
self.request_factory = RequestFactory()
|
||||
self.table = "student_languageproficiency"
|
||||
self.user = UserFactory.create(password=self.password)
|
||||
self.default_request = self.request_factory.get("/api/user/v1/accounts/")
|
||||
self.default_request.user = self.user
|
||||
self.different_user = UserFactory.create(password=self.password)
|
||||
self.staff_user = UserFactory(is_staff=True, password=self.password)
|
||||
self.reset_tracker()
|
||||
|
||||
def test_get_username_provided(self):
|
||||
"""Test the difference in behavior when a username is supplied to get_account_settings."""
|
||||
account_settings = get_account_settings(self.user)
|
||||
account_settings = get_account_settings(self.default_request)
|
||||
self.assertEqual(self.user.username, account_settings["username"])
|
||||
|
||||
account_settings = get_account_settings(self.user, username=self.user.username)
|
||||
account_settings = get_account_settings(self.default_request, username=self.user.username)
|
||||
self.assertEqual(self.user.username, account_settings["username"])
|
||||
|
||||
account_settings = get_account_settings(self.user, username=self.different_user.username)
|
||||
account_settings = get_account_settings(self.default_request, username=self.different_user.username)
|
||||
self.assertEqual(self.different_user.username, account_settings["username"])
|
||||
|
||||
def test_get_configuration_provided(self):
|
||||
@@ -75,29 +79,35 @@ class TestAccountApi(UserSettingsEventTestMixin, TestCase):
|
||||
}
|
||||
|
||||
# With default configuration settings, email is not shared with other (non-staff) users.
|
||||
account_settings = get_account_settings(self.user, self.different_user.username)
|
||||
account_settings = get_account_settings(self.default_request, self.different_user.username)
|
||||
self.assertFalse("email" in account_settings)
|
||||
|
||||
account_settings = get_account_settings(self.user, self.different_user.username, configuration=config)
|
||||
account_settings = get_account_settings(
|
||||
self.default_request,
|
||||
self.different_user.username,
|
||||
configuration=config
|
||||
)
|
||||
self.assertEqual(self.different_user.email, account_settings["email"])
|
||||
|
||||
def test_get_user_not_found(self):
|
||||
"""Test that UserNotFound is thrown if there is no user with username."""
|
||||
with self.assertRaises(UserNotFound):
|
||||
get_account_settings(self.user, username="does_not_exist")
|
||||
get_account_settings(self.default_request, username="does_not_exist")
|
||||
|
||||
self.user.username = "does_not_exist"
|
||||
request = self.request_factory.get("/api/user/v1/accounts/")
|
||||
request.user = self.user
|
||||
with self.assertRaises(UserNotFound):
|
||||
get_account_settings(self.user)
|
||||
get_account_settings(request)
|
||||
|
||||
def test_update_username_provided(self):
|
||||
"""Test the difference in behavior when a username is supplied to update_account_settings."""
|
||||
update_account_settings(self.user, {"name": "Mickey Mouse"})
|
||||
account_settings = get_account_settings(self.user)
|
||||
account_settings = get_account_settings(self.default_request)
|
||||
self.assertEqual("Mickey Mouse", account_settings["name"])
|
||||
|
||||
update_account_settings(self.user, {"name": "Donald Duck"}, username=self.user.username)
|
||||
account_settings = get_account_settings(self.user)
|
||||
account_settings = get_account_settings(self.default_request)
|
||||
self.assertEqual("Donald Duck", account_settings["name"])
|
||||
|
||||
with self.assertRaises(UserNotAuthorized):
|
||||
@@ -171,7 +181,7 @@ class TestAccountApi(UserSettingsEventTestMixin, TestCase):
|
||||
self.assertIn("Error thrown from do_email_change_request", context_manager.exception.developer_message)
|
||||
|
||||
# Verify that the name change happened, even though the attempt to send the email failed.
|
||||
account_settings = get_account_settings(self.user)
|
||||
account_settings = get_account_settings(self.default_request)
|
||||
self.assertEqual("Mickey Mouse", account_settings["name"])
|
||||
|
||||
@patch('openedx.core.djangoapps.user_api.accounts.serializers.AccountUserSerializer.save')
|
||||
@@ -229,7 +239,9 @@ class AccountSettingsOnCreationTest(TestCase):
|
||||
|
||||
# Retrieve the account settings
|
||||
user = User.objects.get(username=self.USERNAME)
|
||||
account_settings = get_account_settings(user)
|
||||
request = RequestFactory().get("/api/user/v1/accounts/")
|
||||
request.user = user
|
||||
account_settings = get_account_settings(request)
|
||||
|
||||
# Expect a date joined field but remove it to simplify the following comparison
|
||||
self.assertIsNotNone(account_settings['date_joined'])
|
||||
@@ -250,8 +262,8 @@ class AccountSettingsOnCreationTest(TestCase):
|
||||
'bio': None,
|
||||
'profile_image': {
|
||||
'has_image': False,
|
||||
'image_url_full': '/static/default_50.png',
|
||||
'image_url_small': '/static/default_10.png',
|
||||
'image_url_full': request.build_absolute_uri('/static/default_50.png'),
|
||||
'image_url_small': request.build_absolute_uri('/static/default_10.png'),
|
||||
},
|
||||
'requires_parental_consent': True,
|
||||
'language_proficiencies': [],
|
||||
@@ -303,18 +315,22 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase):
|
||||
u'a' * (PASSWORD_MAX_LENGTH + 1)
|
||||
]
|
||||
|
||||
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
|
||||
def test_activate_account(self):
|
||||
# Create the account, which is initially inactive
|
||||
activation_key = create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
|
||||
user = User.objects.get(username=self.USERNAME)
|
||||
account = get_account_settings(user)
|
||||
|
||||
request = RequestFactory().get("/api/user/v1/accounts/")
|
||||
request.user = user
|
||||
account = get_account_settings(request)
|
||||
self.assertEqual(self.USERNAME, account["username"])
|
||||
self.assertEqual(self.EMAIL, account["email"])
|
||||
self.assertFalse(account["is_active"])
|
||||
|
||||
# Activate the account and verify that it is now active
|
||||
activate_account(activation_key)
|
||||
account = get_account_settings(user)
|
||||
account = get_account_settings(request)
|
||||
self.assertTrue(account['is_active'])
|
||||
|
||||
def test_create_account_duplicate_username(self):
|
||||
|
||||
@@ -321,11 +321,12 @@ class TestAccountAPI(UserAPITestCase):
|
||||
legacy_profile.country = ""
|
||||
legacy_profile.level_of_education = ""
|
||||
legacy_profile.gender = ""
|
||||
legacy_profile.bio = ""
|
||||
legacy_profile.save()
|
||||
|
||||
self.client.login(username=self.user.username, password=self.test_password)
|
||||
response = self.send_get(self.client)
|
||||
for empty_field in ("level_of_education", "gender", "country"):
|
||||
for empty_field in ("level_of_education", "gender", "country", "bio"):
|
||||
self.assertIsNone(response.data[empty_field])
|
||||
|
||||
@ddt.data(
|
||||
|
||||
@@ -146,11 +146,7 @@ class AccountView(APIView):
|
||||
GET /api/user/v1/accounts/{username}/
|
||||
"""
|
||||
try:
|
||||
account_settings = get_account_settings(request.user, username, view=request.QUERY_PARAMS.get('view'))
|
||||
# Account for possibly relative URLs.
|
||||
for key, value in account_settings['profile_image'].items():
|
||||
if key.startswith(PROFILE_IMAGE_KEY_PREFIX):
|
||||
account_settings['profile_image'][key] = request.build_absolute_uri(value)
|
||||
account_settings = get_account_settings(request, username, view=request.QUERY_PARAMS.get('view'))
|
||||
except UserNotFound:
|
||||
return Response(status=status.HTTP_403_FORBIDDEN if request.user.is_staff else status.HTTP_404_NOT_FOUND)
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@ from django.contrib.auth.models import User
|
||||
from django.test import TestCase
|
||||
from django.test.testcases import TransactionTestCase
|
||||
from django.test.utils import override_settings
|
||||
from django.test.client import RequestFactory
|
||||
|
||||
from social.apps.django_app.default.models import UserSocialAuth
|
||||
|
||||
@@ -1269,7 +1270,9 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, ApiTestCase):
|
||||
self.assertIn(settings.EDXMKTG_USER_INFO_COOKIE_NAME, self.client.cookies)
|
||||
|
||||
user = User.objects.get(username=self.USERNAME)
|
||||
account_settings = get_account_settings(user)
|
||||
request = RequestFactory().get('/url')
|
||||
request.user = user
|
||||
account_settings = get_account_settings(request)
|
||||
|
||||
self.assertEqual(self.USERNAME, account_settings["username"])
|
||||
self.assertEqual(self.EMAIL, account_settings["email"])
|
||||
@@ -1307,7 +1310,10 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, ApiTestCase):
|
||||
|
||||
# Verify the user's account
|
||||
user = User.objects.get(username=self.USERNAME)
|
||||
account_settings = get_account_settings(user)
|
||||
request = RequestFactory().get('/url')
|
||||
request.user = user
|
||||
account_settings = get_account_settings(request)
|
||||
|
||||
self.assertEqual(account_settings["level_of_education"], self.EDUCATION)
|
||||
self.assertEqual(account_settings["mailing_address"], self.ADDRESS)
|
||||
self.assertEqual(account_settings["year_of_birth"], int(self.YEAR_OF_BIRTH))
|
||||
|
||||
Reference in New Issue
Block a user