make the employment/education schema lock tight and remove some custo…

…m error messages
CenterForOpenScience · Jul 27, 2018 · ac63940 · ac63940
1 parent 811b7b7
commit ac63940
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 75 deletions.
diff --git a/api/users/education-schema.json b/api/users/education-schema.json
@@ -25,6 +25,18 @@
 				"minimum": 1900
 			},
 			"ongoing": {
+				"oneOf": [{
+						"enum": [false],
+						"required": ["startYear", "endYear"]
+					},
+					{
+						"enum": [true],
+						"required": ["startYear"],
+						"not": {
+							"required": ["endYear"]
+						}
+					}
+				],
 				"type": "boolean"
 			},
 			"department": {
@@ -41,38 +53,9 @@
 		"additionalProperties": false,
 		"dependencies": {
 			"endMonth": ["endYear"],
-			"startMonth": ["startYear"]
-		},
-		"anyOf": [
-		  {
-			"not": {
-			  "properties": {
-				"ongoing": {
-					"enum": [true]
-				}
-			  },
-			  "required": ["ongoing"]
-			}
-		  },
-		  { "required": ["startYear"]}
-		],
-		"anyOf": [
-		  {
-			"not": {
-			  "properties": {
-				"ongoing": { "enum": [false] }
-			  },
-			  "required": ["ongoing"]
-			}
-		  },
-		  { "required": ["endYear"] }
-		],
-		"not": {
-			"properties": {
-				"ongoing": {"enum": [true]}
-			},
-			"required": ["endYear"],
-			"message": "Ongoing positions cannot have end dates."
+			"startMonth": ["startYear"],
+			"startYear": ["ongoing"],
+			"endYear": ["ongoing"]
 		}
 	}
-}
+}
diff --git a/api/users/employment-schema.json b/api/users/employment-schema.json
@@ -25,6 +25,18 @@
 				"minimum": 1900
 			},
 			"ongoing": {
+				"oneOf": [{
+						"enum": [false],
+						"required": ["startYear", "endYear"]
+					},
+					{
+						"enum": [true],
+						"required": ["startYear"],
+						"not": {
+							"required": ["endYear"]
+						}
+					}
+				],
 				"type": "boolean"
 			},
 			"department": {
@@ -41,38 +53,9 @@
 		"additionalProperties": false,
 		"dependencies": {
 			"endMonth": ["endYear"],
-			"startMonth": ["startYear"]
-		},
-		"anyOf": [
-		  {
-			"not": {
-			  "properties": {
-				"ongoing": {
-					"enum": [true]
-				}
-			  },
-			  "required": ["ongoing"]
-			}
-		  },
-		  { "required": ["startYear"]}
-		],
-		"anyOf": [
-		  {
-			"not": {
-			  "properties": {
-				"ongoing": { "enum": [false] }
-			  },
-			  "required": ["ongoing"]
-			}
-		  },
-		  { "required": ["endYear"] }
-		],
-		"not": {
-			"properties": {
-				"ongoing": {"enum": [true]}
-			},
-			"required": ["endYear"],
-			"message": "Ongoing positions cannot have end dates."
+			"startMonth": ["startYear"],
+			"startYear": ["ongoing"],
+			"endYear": ["ongoing"]
 		}
 	}
-}
+}
diff --git a/api/users/serializers.py b/api/users/serializers.py
@@ -136,8 +136,6 @@ def _validate_user_json(self, value, json_schema):
         try:
             jsonschema.validate(value, from_json(json_schema))
         except jsonschema.ValidationError as e:
-            if type(e.validator_value) == dict and e.validator_value.get('message'):
-                raise InvalidModelValueError(e.validator_value.get('message'))
             if len(e.path) > 1:
                 raise InvalidModelValueError("For '{}' the field value {}".format(e.path[-1], e.message))
             raise InvalidModelValueError(e.message)

diff --git a/api_tests/users/views/test_user_detail.py b/api_tests/users/views/test_user_detail.py
@@ -1017,6 +1017,7 @@ def end_dates_no_start_dates_payload(self, request_payload, request_key):
 
     @pytest.fixture()
     def start_dates_no_end_dates_payload(self, request_payload, request_key):
+        request_payload['data']['attributes'][request_key][0]['ongoing'] = True
         del request_payload['data']['attributes'][request_key][0]['endYear']
         del request_payload['data']['attributes'][request_key][0]['endMonth']
         return request_payload
@@ -1089,12 +1090,12 @@ def test_user_put_profile_validation_empty_string(self, app, user_one, user_one_
         assert res.status_code == 400
         assert res.json['errors'][0]['detail'] == "For 'institution' the field value u'' is too short"
 
-    def test_user_put_profile_validation_ongoing_bool(self, app, user_one, user_one_url, request_payload, request_key):
+    def test_user_put_profile_validation_ongoing_dependency(self, app, user_one, user_one_url, request_payload, request_key):
         # Tests to make sure ongoing is bool
-        request_payload['data']['attributes'][request_key][0]['ongoing'] = '???'
+        del request_payload['data']['attributes'][request_key][0]['ongoing']
         res = app.put_json_api(user_one_url, request_payload, auth=user_one.auth, expect_errors=True)
         assert res.status_code == 400
-        assert res.json['errors'][0]['detail'] == "For 'ongoing' the field value u'???' is not of type u'boolean'"
+        assert res.json['errors'][0]['detail'] == "u'ongoing' is a dependency of u'endYear'"
 
     def test_user_put_profile_date_validate_int(self, app, user_one, user_one_url, request_payload, request_key):
         # Not valid datatypes for dates
@@ -1115,9 +1116,10 @@ def test_user_put_profile_date_validate_positive(self, app, user_one, user_one_u
     def test_user_put_profile_date_validate_ongoing_position(self, app, user_one, user_one_url, request_payload, request_key):
         # endDates for ongoing position
         request_payload['data']['attributes'][request_key][0]['ongoing'] = True
+        del request_payload['data']['attributes'][request_key][0]['endYear']
         res = app.put_json_api(user_one_url, request_payload, auth=user_one.auth, expect_errors=True)
         assert res.status_code == 400
-        assert res.json['errors'][0]['detail'] == 'Ongoing positions cannot have end dates.'
+        assert res.json['errors'][0]['detail'] == "For 'ongoing' the field value True is not valid under any of the given schemas"
 
     def test_user_put_profile_date_validate_end_date(self, app, user_one, user_one_url, request_payload, request_key):
         # End date is greater then start date
@@ -1143,9 +1145,8 @@ def test_user_put_profile_date_validate_start_date_no_end_date_not_ongoing(self,
         res = app.put_json_api(user_one_url, start_dates_no_end_dates_payload, auth=user_one.auth, expect_errors=True)
         user_one.reload()
         assert res.status_code == 400
-        assert res.json['errors'][0]['detail'] == "{{u'startYear': 1991, u'{}': u'', u'startMonth': 9, u'ongoing':" \
-                                                  " False, u'department': u'', u'institution': u'Fake U'}}" \
-                                                  ' is not valid under any of the given schemas'.format('title' if user_attr == 'jobs' else 'degree')
+        assert res.json['errors'][0]['detail'] == "For 'ongoing' the field value True is not valid under any of the given schemas"
+
 
     def test_user_put_profile_date_validate_end_date_no_start_date(self, app, user_one, user_attr, user_one_url, end_dates_no_start_dates_payload, request_key):
         # End dates, but no start dates