search

samvera / questioning_authority

Question your authorities
Other
54 stars 30 forks source link

Return a 422 (instead of 404) when check_query_param can't find a :q param #162

Closed afred closed 5 years ago

afred commented 6 years ago

Expected behavior: Hitting an endpoint when missing the :q param returns a 404, which could be any number of problems, instead of the very specific problem of "you are missing the :q parm".

Actual behavior: Hitting and endpoint when missing the :q param returns a 404.

I think this is just a matter of changing these lines to not return :not_found, but something else... https://github.com/samvera/questioning_authority/blob/a30fc63304dfcfb38029b9dfb432790e2acd8e2b/app/controllers/qa/terms_controller.rb#L56-L58

This would also apply to check_vocab_param.

elrayle commented 5 years ago

REF: https://www.bennadel.com/blog/2434-http-status-codes-for-invalid-data-400-vs-422.htm

Need to replace :not_found with :unprocessable_entity

elrayle commented 5 years ago

Looking at this more, it is not returning 404 when q is missing. It is returning 400 bad_request.

elrayle commented 5 years ago

I thought this was a reference to the linked data module. Turns out the 404 was in the non-linked data terms_controller class.

@afred To be consistent with the linked_data_terms_controller, which uses 400 bad_request, I have updated the terms_controller to also use 400 bad_request when a required parameter is missing. Unless there is a strong argument for using 422 over 400, I prefer to stay with 400 bad_request. This is to maintain backward compatibility for the linked_data module.

See PR #211

elrayle commented 5 years ago

Fixed by PR #211