diff --git a/api/users/education-schema.json b/api/users/education-schema.json new file mode 100644 index 000000000000..a1a1ddeeed2e --- /dev/null +++ b/api/users/education-schema.json @@ -0,0 +1,52 @@ +{ + "type": "array", + "items": { + "type": "object", + "properties": { + "degree": { + "type": "string" + }, + "startYear": { + "type": "integer", + "minimum": 1900 + }, + "startMonth": { + "type": "integer", + "minimum": 1, + "maximum": 12 + }, + "endMonth": { + "type": "integer", + "minimum": 1, + "maximum": 12 + }, + "endYear": { + "type": "integer", + "minimum": 1900 + }, + "ongoing": { + "type": "boolean" + }, + "department": { + "type": "string" + }, + "institution": { + "type": "string", + "minLength": 1 + } + }, + "required": [ + "startYear", + "startMonth", + "ongoing", + "department", + "institution", + "degree" + ], + "additionalProperties": false, + "dependencies": { + "endMonth": ["endYear"], + "endYear": ["endMonth"] + } + } +} \ No newline at end of file diff --git a/api/users/employment-schema.json b/api/users/employment-schema.json new file mode 100644 index 000000000000..5b47116ed2e6 --- /dev/null +++ b/api/users/employment-schema.json @@ -0,0 +1,52 @@ +{ + "type": "array", + "items": { + "type": "object", + "properties": { + "title": { + "type": "string" + }, + "startYear": { + "type": "integer", + "minimum": 1900 + }, + "startMonth": { + "type": "integer", + "minimum": 1, + "maximum": 12 + }, + "endMonth": { + "type": "integer", + "minimum": 1, + "maximum": 12 + }, + "endYear": { + "type": "integer", + "minimum": 1900 + }, + "ongoing": { + "type": "boolean" + }, + "department": { + "type": "string" + }, + "institution": { + "type": "string", + "minLength": 1 + } + }, + "required": [ + "startYear", + "startMonth", + "ongoing", + "department", + "institution", + "title" + ], + "additionalProperties": false, + "dependencies": { + "endMonth": ["endYear"], + "endYear": ["endMonth"] + } + } +} \ No newline at end of file diff --git a/api/users/schemas.py b/api/users/schemas.py new file mode 100644 index 000000000000..3e9cb336f865 --- /dev/null +++ b/api/users/schemas.py @@ -0,0 +1,8 @@ +import os +import json + +here = os.path.split(os.path.abspath(__file__))[0] + +def from_json(fname): + with open(os.path.join(here, fname)) as f: + return json.load(f) diff --git a/api/users/serializers.py b/api/users/serializers.py index 7e995cc963ac..3842a2fce3e3 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -144,54 +144,33 @@ def _update_social(self, social_dict, user): ) user.social[key] = val[0] - def _validate_user_json(self, info_list, required_fields): - 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)) - ) + def _validate_user_json(self, value, json_schema): + from api.users.schemas import from_json + import jsonschema + try: + jsonschema.validate(value, from_json(json_schema)) + except jsonschema.ValidationError as e: + if len(e.path) > 1: + raise InvalidModelValueError("For '{}' the field value {}".format(e.path[-1], e.message)) - if not info.get('institution'): - raise InvalidModelValueError(detail='The institution field cannot be empty.') + raise InvalidModelValueError(e.message) + except jsonschema.SchemaError as e: + raise InvalidModelValueError(e.message) - self._validate_dates(info) + self._validate_dates(value) 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.') + for history in info: + + startDate = datetime.date(history['startYear'], history['startMonth'], 1) - 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 not history['ongoing']: + endDate = datetime.date(history['endYear'], history['endMonth'], 1) - if (endDate - startDate).days <= 0: - raise InvalidModelValueError(detail='End date must be greater than or equal to the start date.') + if (endDate - startDate).days <= 0: + raise InvalidModelValueError(detail='End date must be greater than or equal to the start date.') + elif history.get('endYear') or history.get('endMonth'): + raise InvalidModelValueError(detail='Ongoing positions cannot have end dates.') def update(self, instance, validated_data): assert isinstance(instance, OSFUser), 'instance must be a User' @@ -208,10 +187,10 @@ def update(self, instance, validated_data): ) instance.social[key] = val[0] elif 'schools' == attr: - self._validate_user_json(value, self.education_required_keys) + self._validate_user_json(value, 'education-schema.json') instance.schools = value elif 'jobs' == attr: - self._validate_user_json(value, self.employment_required_keys) + self._validate_user_json(value, 'employment-schema.json') instance.jobs = value elif 'accepted_terms_of_service' == attr: diff --git a/api_tests/users/views/test_user_detail.py b/api_tests/users/views/test_user_detail.py index 37d2362b656d..e4887e69ddc8 100644 --- a/api_tests/users/views/test_user_detail.py +++ b/api_tests/users/views/test_user_detail.py @@ -913,20 +913,6 @@ def test_user_put_schools_validation(self, app, user_one, url_user_one): assert res.status_code == 400 assert res.json['errors'][0]['detail'] == 'Expected a list of items but got type "dict".' - # Tests to make sure structure is lists of dicts - res = app.put_json_api(url_user_one, { - 'data': { - 'id': user_one._id, - 'type': 'users', - 'attributes': { - 'full_name': user_one.fullname, - 'education': [] - }, - } - }, auth=user_one.auth, expect_errors=True) - assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'The updated information must be in a list of dictionaries.' - # Tests to make sure structure is lists of dicts consisting of proper fields res = app.put_json_api(url_user_one, { 'data': { @@ -938,9 +924,9 @@ def test_user_put_schools_validation(self, app, user_one, url_user_one): }, } }, auth=user_one.auth, expect_errors=True) + print res.json assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'The updated employment fields must contain keys degree, startYear,' \ - ' startMonth, ongoing, department, institution.' + assert res.json['errors'][0]['detail'] == "u'startYear' is a required property" # Tests to make sure institution is not empty string res = app.put_json_api(url_user_one, { @@ -962,7 +948,7 @@ def test_user_put_schools_validation(self, app, user_one, url_user_one): } }, auth=user_one.auth, expect_errors=True) assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'The institution field cannot be empty.' + assert res.json['errors'][0]['detail'] == "For 'institution' the field value u'' is too short" # Tests to make sure ongoing is bool res = app.put_json_api(url_user_one, { @@ -982,7 +968,7 @@ def test_user_put_schools_validation(self, app, user_one, url_user_one): } }, auth=user_one.auth, expect_errors=True) assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'The field "ongoing" must be boolean.' + assert res.json['errors'][0]['detail'] == "For 'ongoing' the field value u'???' is not of type u'boolean'" def test_user_put_schools_date_validation(self, app, user_one, url_user_one): @@ -1004,7 +990,7 @@ def test_user_put_schools_date_validation(self, app, user_one, url_user_one): } }, auth=user_one.auth, expect_errors=True) assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'Date values must be valid integers.' + assert res.json['errors'][0]['detail'] == "For 'startYear' the field value u'string' is not of type u'integer'" # Not valid values for dates res = app.put_json_api(url_user_one, { @@ -1024,7 +1010,7 @@ def test_user_put_schools_date_validation(self, app, user_one, url_user_one): } }, auth=user_one.auth, expect_errors=True) assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'Date values must be valid integers.' + assert res.json['errors'][0]['detail'] == "For 'startYear' the field value -2 is less than the minimum of 1900" # endDates for ongoing position res = app.put_json_api(url_user_one, { @@ -1046,8 +1032,7 @@ def test_user_put_schools_date_validation(self, app, user_one, url_user_one): } }, auth=user_one.auth, expect_errors=True) assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'The updated employment fields can only contain keys degree, ' \ - 'startYear, startMonth, ongoing, department, institution.' + assert res.json['errors'][0]['detail'] == 'Ongoing positions cannot have end dates.' # End date is greater then start date res = app.put_json_api(url_user_one, { @@ -1142,20 +1127,6 @@ def test_user_put_jobs_validation(self, app, user_one, url_user_one): assert res.status_code == 400 assert res.json['errors'][0]['detail'] == 'Expected a list of items but got type "dict".' - # Tests to make sure structure is lists of dicts - res = app.put_json_api(url_user_one, { - 'data': { - 'id': user_one._id, - 'type': 'users', - 'attributes': { - 'full_name': user_one.fullname, - 'employment': [] - }, - } - }, auth=user_one.auth, expect_errors=True) - assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'The updated information must be in a list of dictionaries.' - # Tests to make sure structure is lists of dicts consisting of proper fields res = app.put_json_api(url_user_one, { 'data': { @@ -1168,8 +1139,7 @@ def test_user_put_jobs_validation(self, app, user_one, url_user_one): } }, auth=user_one.auth, expect_errors=True) assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'The updated employment fields must contain keys startYear, title,' \ - ' startMonth, ongoing, department, institution.' + assert res.json['errors'][0]['detail'] == "u'startYear' is a required property" # Tests to make sure institution is not empty string res = app.put_json_api(url_user_one, { @@ -1191,7 +1161,7 @@ def test_user_put_jobs_validation(self, app, user_one, url_user_one): } }, auth=user_one.auth, expect_errors=True) assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'The institution field cannot be empty.' + assert res.json['errors'][0]['detail'] == "For 'institution' the field value u'' is too short" # Tests to make sure ongoing is bool res = app.put_json_api(url_user_one, { @@ -1211,7 +1181,7 @@ def test_user_put_jobs_validation(self, app, user_one, url_user_one): } }, auth=user_one.auth, expect_errors=True) assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'The field "ongoing" must be boolean.' + assert res.json['errors'][0]['detail'] == "For 'ongoing' the field value u'???' is not of type u'boolean'" def test_user_put_jobs_date_validation(self, app, user_one, url_user_one): @@ -1233,7 +1203,7 @@ def test_user_put_jobs_date_validation(self, app, user_one, url_user_one): } }, auth=user_one.auth, expect_errors=True) assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'Date values must be valid integers.' + assert res.json['errors'][0]['detail'] == "For 'startYear' the field value u'string' is not of type u'integer'" # Not valid values for dates res = app.put_json_api(url_user_one, { @@ -1253,7 +1223,7 @@ def test_user_put_jobs_date_validation(self, app, user_one, url_user_one): } }, auth=user_one.auth, expect_errors=True) assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'Date values must be valid integers.' + assert res.json['errors'][0]['detail'] == "For 'startYear' the field value -2 is less than the minimum of 1900" # endDates for ongoing position res = app.put_json_api(url_user_one, { @@ -1275,8 +1245,7 @@ def test_user_put_jobs_date_validation(self, app, user_one, url_user_one): } }, auth=user_one.auth, expect_errors=True) assert res.status_code == 400 - assert res.json['errors'][0]['detail'] == 'The updated employment fields can only contain keys startYear,' \ - ' title, startMonth, ongoing, department, institution.' + assert res.json['errors'][0]['detail'] == 'Ongoing positions cannot have end dates.' # End date is greater then start date res = app.put_json_api(url_user_one, {