-
Notifications
You must be signed in to change notification settings - Fork 134
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
Default response format to JSON #2170
Conversation
31bad98
to
fc67b7c
Compare
@@ -160,6 +161,7 @@ def test_get_on_categorized_field(self): | |||
self.assertEqual(response.data['data'][0]['gender'], 'Male') | |||
self.assertEqual(response.data['data'][1]['gender'], 'Female') | |||
|
|||
@skip("https://github.com/onaio/onadata/pull/2170") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DavisRayM Not sure why we are skipping this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skipping it because we default to JSON instead of returning a bad request with these changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just remove or edit that test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can remove it, it's a bit similar to one of the other tests so I wasn't so sure about editing it. Let me remove it and re-request review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool
- Skip `test_return_bad_request_on_non_json_request_with_field_name` test
6268be7
to
b6990f8
Compare
Changes / Features implemented
JSON
if not format is passedSteps taken to verify this change does what is intended
Side effects of implementing this change
The charts endpoint will always return a JSON response if the
format
query param is emptyBefore submitting this PR for review, please make sure you have:
Related to #2151
Main reason for the update is because the browser would still return a
Not supported
error with the changes introduced in #2151; Which wasn't the ideal scenario for an API.... If the format is not specified we should always default to JSON IMO