Skip to content

Commit

Permalink
[ENG-1825] Fix url formatting and exception handling (#9385)
Browse files Browse the repository at this point in the history
## Purpose

Unfortunately it turns out our URLField were essentially broken by our custom exception handling, so that their actual message wasn't getting passed, just the int 0. This meant there was no good way of telling whether a typical 400 validation error was cause by a URLField or any other field. This PR should fix this.

## Changes

- changes to custom exception handler and add tests

## QA Notes

This is ultimately going to extended by a front-end dev who will write the exact error text message.

## Documentation

Bug fix, no docs.

## Side Effects

None that I know of.

## Ticket

https://openscience.atlassian.net/browse/ENG-1825
  • Loading branch information
Johnetordoff committed May 18, 2020
1 parent 9511666 commit 9521eea
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 1 deletion.
26 changes: 25 additions & 1 deletion api/base/exceptions.py
Expand Up @@ -43,11 +43,35 @@ def dict_error_formatting(errors, context, index=None):
formatted_error_list.extend([{'source': {'pointer': '/data/{}'.format(index) + error_key}, 'detail': reason} for reason in error_description])
elif error_key == 'non_field_errors':
formatted_error_list.extend([{'detail': description for description in error_description}])
elif isinstance(error_description, list):
for error in error_description:
formatted_error_list += format_validators_errors(error, error_key, context, index)
else:
formatted_error_list.extend([{'source': {'pointer': '/data/{}{}/'.format(index, get_resource_object_member(error_key, context)) + error_key}, 'detail': reason} for reason in error_description])
formatted_error_list += format_validators_errors(error_description, error_key, context, index)

return formatted_error_list


def format_validators_errors(error_description, error_key, context, index):
errors = []
if isinstance(error_description, ErrorDetail):
errors.append({
'source': {
'pointer': f'/data/{index}{get_resource_object_member(error_key, context)}/' + error_key,
},
'detail': error_description,
})
else:
for key, value in error_description.items():
errors.append({
'source': {
'pointer': f'/data/{index}{get_resource_object_member(error_key, context)}/' + error_key,
},
'detail': value,
})

return errors

def json_api_exception_handler(exc, context):
"""
Custom exception handler that returns errors object as an array
Expand Down
23 changes: 23 additions & 0 deletions api_tests/preprints/views/test_preprint_detail.py
Expand Up @@ -928,6 +928,17 @@ def test_update_data_links(self, app, user, preprint, url):
assert res.status_code == 400
assert res.json['errors'][0]['detail'] == 'Expected a list of items but got type "str".'

@override_switch(features.SLOAN_DATA_INPUT, active=True)
def test_invalid_data_links(self, app, user, preprint, url):
preprint.has_data_links = 'available'
preprint.save()

update_payload = build_preprint_update_payload(preprint._id, attributes={'data_links': ['thisaintright']})

res = app.patch_json_api(url, update_payload, auth=user.auth, expect_errors=True)
assert res.status_code == 400
assert res.json['errors'][0]['detail'] == ['Enter a valid URL.']

def test_update_has_prereg_links(self, app, user, preprint, url):
update_payload = build_preprint_update_payload(preprint._id, attributes={'has_prereg_links': 'available'})

Expand All @@ -953,6 +964,18 @@ def test_update_has_prereg_links(self, app, user, preprint, url):
assert log.action == PreprintLog.UPDATE_HAS_PREREG_LINKS
assert log.params == {'value': 'available', 'user': user._id, 'preprint': preprint._id}

@override_switch(features.SLOAN_PREREG_INPUT, active=True)
def test_invalid_prereg_links(self, app, user, preprint, url):
preprint.has_prereg_links = 'available'
preprint.save()

update_payload = build_preprint_update_payload(preprint._id, attributes={'prereg_links': ['thisaintright']})

res = app.patch_json_api(url, update_payload, auth=user.auth, expect_errors=True)

assert res.status_code == 400
assert res.json['errors'][0]['detail'] == ['Enter a valid URL.']

def test_update_why_no_prereg(self, app, user, preprint, url):
update_payload = build_preprint_update_payload(preprint._id, attributes={'why_no_prereg': 'My dog ate it.'})

Expand Down

0 comments on commit 9521eea

Please sign in to comment.