Fix API return codes

Created by: Xylakant

The API returns 404 for almost all error states. This PR introduces more appropriate return codes which makes it easier for client developers to distinguish between the specific errors.

Created by: Xylakant

We're done here. All rests are passing, documentation is updated to match the actual behavior. Ready to review and (hopefully) merge.

By Administrator on 2013-02-27T17:38:36 (imported from GitLab project)

By Administrator on 2013-02-27T17:38:36 (imported from GitLab)

Created by: m4tthumphrey

SHAWEEEEET!

By Administrator on 2013-03-01T16:40:24 (imported from GitLab project)

By Administrator on 2013-03-01T16:40:24 (imported from GitLab)

Created by: dzaporozhets

thank you. I will made a review

By Administrator on 2013-03-05T07:35:14 (imported from GitLab project)

By Administrator on 2013-03-05T07:35:14 (imported from GitLab)

Created by: dzaporozhets

???

By Administrator on 2013-03-07T14:12:47 (imported from GitLab project)

By Administrator on 2013-03-07T14:12:47 (imported from GitLab)

Created by: dzaporozhets

maybe return not_found! unless @branch ?

By Administrator on 2013-03-07T14:12:47 (imported from GitLab project)

By Administrator on 2013-03-07T14:12:47 (imported from GitLab)

Created by: dzaporozhets

should we use else here?

if @hook.errors[:url].present?
  error!
else 
  not_found!
end

By Administrator on 2013-03-07T14:12:47 (imported from GitLab project)

By Administrator on 2013-03-07T14:12:47 (imported from GitLab)

Created by: justahero

The idea was to have the delete function to be an idempotent function. When calling it with an invalid hook_id it should return a 200 Ok status code, but the call to ProjectHook.find raises an exception if no hook is found.

By Administrator on 2013-03-07T14:12:47 (imported from GitLab project)

By Administrator on 2013-03-07T14:12:47 (imported from GitLab)

Created by: justahero

The return keyword seems unnecessary since the call to not_found raises an Error.

By Administrator on 2013-03-07T14:12:47 (imported from GitLab project)

By Administrator on 2013-03-07T14:12:47 (imported from GitLab)

Created by: justahero

Same as above, the call error! raises an Error and returns the status code given in parameter. Question is if status code 422 Unprocessable Entity is a good status code in this situation.

By Administrator on 2013-03-07T14:12:47 (imported from GitLab project)

By Administrator on 2013-03-07T14:12:47 (imported from GitLab)

Created by: justahero

First of all thank you for your time, it is much appreciated.

A few words to the changes may give a better insight. Using the error helper functions from the helpers.rb file makes it easy to declare which status code should be returned. Nearly all helper functions use the error! function from Grape that when called raises an Error, causing the API function to return immediately. All other functions based on error! are also suffixed with exclamation mark ! to signal the same behaviour. This is useful when dealing with the 4xx status code range. The most notable change to the API functions is it to check if one or more of the required attributes of an API function is missing (required_attributes!) raising a 400 Bad Request if it is the case.

By Administrator on 2013-03-05T22:49:51 (imported from GitLab project)

By Administrator on 2013-03-05T22:49:51 (imported from GitLab)

Created by: NARKOZ

I think, instead of duplicating info about status codes, it's better to specify them in doc/api/README.md once.

By Administrator on 2013-03-07T14:12:47 (imported from GitLab project)

By Administrator on 2013-03-07T14:12:47 (imported from GitLab)

Created by: dzaporozhets

also can you please make it mergeable?

By Administrator on 2013-03-06T19:17:50 (imported from GitLab project)

By Administrator on 2013-03-06T19:17:50 (imported from GitLab)

Created by: dzaporozhets

ok got it

By Administrator on 2013-03-07T14:12:47 (imported from GitLab project)

By Administrator on 2013-03-07T14:12:47 (imported from GitLab)

Created by: justahero

Merged master into this branch. Infos to return codes are now located in the README.md file as suggested. Tests are all passing.

By Administrator on 2013-03-07T15:42:34 (imported from GitLab project)

By Administrator on 2013-03-07T15:42:34 (imported from GitLab)

Created by: m4tthumphrey

@justahero Have you updated the return codes for the new APIs for deploy keys and system hooks?

By Administrator on 2013-03-07T15:48:38 (imported from GitLab project)

By Administrator on 2013-03-07T15:48:38 (imported from GitLab)

Created by: dzaporozhets

ok lets merge this one :)

By Administrator on 2013-03-07T16:33:27 (imported from GitLab project)

By Administrator on 2013-03-07T16:33:27 (imported from GitLab)

Created by: dzaporozhets

thank you guys!

By Administrator on 2013-03-07T16:33:48 (imported from GitLab project)

By Administrator on 2013-03-07T16:33:48 (imported from GitLab)

Created by: justahero

Wow, thanks :)

@m4tthumphrey I will open a separate PR for this. I prepared everything and once the tests pass I will open it.

By Administrator on 2013-03-07T16:37:12 (imported from GitLab project)

By Administrator on 2013-03-07T16:37:12 (imported from GitLab)

Created by: m4tthumphrey

Ok cool, good job man!

By Administrator on 2013-03-07T16:47:57 (imported from GitLab project)

By Administrator on 2013-03-07T16:47:57 (imported from GitLab)

Created by: vsizov

61 commits. You had to squash.

By Administrator on 2013-03-09T10:27:48 (imported from GitLab project)

By Administrator on 2013-03-09T10:27:48 (imported from GitLab)