Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PLAT-926] Add employment and institution information to user serializer #8546

Merged
merged 23 commits into from
Jul 31, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
510e465
add allow jobs and schools attributes to be updated and validated
Johnetordoff Jul 13, 2018
b6cf748
add tests for v2 updating of jobs and schools attributes
Johnetordoff Jul 13, 2018
9c735bb
add json schema validation and fix tests for that
Johnetordoff Jul 17, 2018
8ae71f9
remove vestigial code
Johnetordoff Jul 17, 2018
130ff07
remove _update_social unnecessary method
Johnetordoff Jul 17, 2018
270702a
fix comments
Johnetordoff Jul 17, 2018
3c6e4c3
add tests for 403s
Johnetordoff Jul 17, 2018
82ecae8
add new line to json files
Johnetordoff Jul 23, 2018
a37c058
use Django builtin validation methods
Johnetordoff Jul 23, 2018
57dabbf
DRY up tests
Johnetordoff Jul 23, 2018
d7338f3
make start year optional
Johnetordoff Jul 23, 2018
699ec81
remove extraneous code
Johnetordoff Jul 23, 2018
198fae1
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf…
Johnetordoff Jul 24, 2018
aed443d
Merge branch 'master' of https://github.com/CenterForOpenScience/osf.…
Johnetordoff Jul 24, 2018
89178ff
Merge branch 'develop' of https://github.com/CenterForOpenScience/osf…
Johnetordoff Jul 24, 2018
39e0c63
unbreak tests after Travis speed up
Johnetordoff Jul 24, 2018
8fd6d66
update schemas and add more tests
Johnetordoff Jul 25, 2018
344f50a
handle startYear with no startMonth
Johnetordoff Jul 26, 2018
811b7b7
add jsonschema validation for ongoing attribute.
Johnetordoff Jul 26, 2018
b7d9324
make the employment/education schema lock tight and remove some custo…
Johnetordoff Jul 27, 2018
72d4a45
add startYear <- endYear dependency
Johnetordoff Jul 30, 2018
53ab147
check for more string types
Johnetordoff Jul 31, 2018
633aa46
move schemas and make utility functions.
Johnetordoff Jul 31, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 76 additions & 4 deletions api/users/serializers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import datetime

from guardian.models import GroupObjectPermission

from django.utils import timezone
Expand All @@ -7,8 +9,8 @@
from api.base.serializers import (
BaseAPISerializer, JSONAPISerializer, JSONAPIRelationshipSerializer,
VersionedDateTimeField, HideIfDisabled, IDField,
Link, LinksField, ListDictField, TypeField, RelationshipField,
WaterbutlerLink, ShowIfCurrentUser, DevOnly
Link, LinksField, ListDictField, TypeField, RelationshipField, JSONAPIListField,
WaterbutlerLink, ShowIfCurrentUser
)
from api.base.utils import absolute_reverse, get_user_auth, waterbutler_api_url_for
from api.files.serializers import QuickFilesSerializer
Expand Down Expand Up @@ -57,8 +59,10 @@ class UserSerializer(JSONAPISerializer):
timezone = HideIfDisabled(ser.CharField(required=False, help_text="User's timezone, e.g. 'Etc/UTC"))
locale = HideIfDisabled(ser.CharField(required=False, help_text="User's locale, e.g. 'en_US'"))
social = ListDictField(required=False)
employment = DevOnly(ser.ListField(child=ser.DictField(), source='jobs', read_only=True))
education = DevOnly(ser.ListField(child=ser.DictField(), source='schools', read_only=True))
employment = JSONAPIListField(required=False, source='jobs')
employment_required_keys = {'department', 'institution', 'ongoing', 'startMonth', 'startYear', 'title'}
education = JSONAPIListField(required=False, source='schools')
education_required_keys = {'department', 'institution', 'ongoing', 'startMonth', 'startYear', 'degree'}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove employment_required_keys and education_required_keys since we're using jsonschema.

can_view_reviews = ShowIfCurrentUser(ser.SerializerMethodField(help_text='Whether the current user has the `view_submissions` permission to ANY reviews provider.'))
accepted_terms_of_service = ShowIfCurrentUser(ser.SerializerMethodField())

Expand Down Expand Up @@ -128,6 +132,67 @@ def profile_image_url(self, user):
size = self.context['request'].query_params.get('profile_image_size')
return user.profile_image_url(size=size)

def _update_social(self, social_dict, user):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was pulled from the first part of the update method? doesn't look like it's being used.

for key, val in social_dict.items():
# currently only profileWebsites are a list, the rest of the social key only has one value
if key == 'profileWebsites':
user.social[key] = val
else:
if len(val) > 1:
raise InvalidModelValueError(
detail='{} only accept a list of one single value'.format(key)
)
user.social[key] = val[0]

def _validate_user_json(self, info_list, required_fields):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend using jsonschema for most of this validation, instead of individually type checking each field. We use this for validating form information for draft registrations. I doubt jsonschema can validate for everything that you want, for example, end date must be greater that start date, but it works well for required fields, field types, etc. You can also define field dependencies (if endMonth is present, endYear must be present, for example).

You would do something like:

  • import jsonschema
  • define your schema, started below:
  • jsonschema.validate(validated_data, schema)

Here's a start on the schema for education data, doesn't have everything:

{
  "type": "array",
  "items": {
    "type": "object",
    "properties": {
      "degree": {
        "type": "string",
      },
      "startYear": {
        "type": "integer",
      },
      "startMonth": {
        "type": "integer"
      },
      "endMonth": {
        "type": "integer"
      },
      "endYear": {
        "type": "integer"
      },
      "ongoing": {
        "type": "boolean"
      },
      "department": {
        "type": "string"
      },
      "institution": {
        "type": "string"
      }
    },
    "required": [
        "startYear",
        "startMonth",
        "ongoing",
        "department",
        "institution",
        "degree"
    ],
    "dependencies": {
        "endMonth": ["endYear"],
        "endYear": ["endMonth"]
     }
  }
}

if type(info_list) != list or not info_list:
raise InvalidModelValueError(detail='The updated information must be in a list of dictionaries.')

for info in info_list:

if type(info) != dict:
raise InvalidModelValueError(detail='The updated information must be in a list of dictionaries.')

fields = set(info.keys())

absent_fields = required_fields.difference(fields)
if absent_fields:
raise InvalidModelValueError(
detail='The updated employment fields must contain keys {}.'.format(', '.join(absent_fields))
)

if type(info['ongoing']) != bool:
raise InvalidModelValueError(detail='The field "ongoing" must be boolean.')

if not info['ongoing']:
required_fields = required_fields.union({'endYear', 'endMonth'})

extra_fields = required_fields.symmetric_difference(fields)
if extra_fields:
raise InvalidModelValueError(
detail='The updated employment fields can only contain keys {}.'.format(', '.join(required_fields))
)

if not info.get('institution'):
raise InvalidModelValueError(detail='The institution field cannot be empty.')

self._validate_dates(info)

def _validate_dates(self, info):
try:
startDate = datetime.date(info['startYear'], info['startMonth'], 1)
except (ValueError, TypeError):
raise InvalidModelValueError(detail='Date values must be valid integers.')

if not info['ongoing']:
try:
endDate = datetime.date(info['endYear'], info['endMonth'], 1)
except (ValueError, TypeError):
raise InvalidModelValueError(detail='Date values must be valid integers.')

if (endDate - startDate).days <= 0:
raise InvalidModelValueError(detail='End date must be greater than or equal to the start date.')

def update(self, instance, validated_data):
assert isinstance(instance, OSFUser), 'instance must be a User'
for attr, value in validated_data.items():
Expand All @@ -142,6 +207,13 @@ def update(self, instance, validated_data):
detail='{} only accept a list of one single value'. format(key)
)
instance.social[key] = val[0]
elif 'schools' == attr:
self._validate_user_json(value, self.education_required_keys)
instance.schools = value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these assignment statements are necessary anymore. The fields will be set by the setattr(instance, attr, value) call below.

elif 'jobs' == attr:
self._validate_user_json(value, self.employment_required_keys)
instance.jobs = value

elif 'accepted_terms_of_service' == attr:
if value and not instance.accepted_terms_of_service:
instance.accepted_terms_of_service = timezone.now()
Expand Down
Loading