-
Notifications
You must be signed in to change notification settings - Fork 29
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
Extend hooks config endpoint #256
Conversation
dadi/lib/index.js
Outdated
|
||
return help.sendBackJSON(404, res, next)(null, {}) | ||
return help.sendBackJSON(400, res, next)(null, {'error': 'Invalid method'}) |
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 think we could return a 405 here, Method Not Allowed
dadi/lib/index.js
Outdated
|
||
return help.sendBackJSON(200, res, next)(null, data) | ||
return help.sendBackJSON(400, res, next)(null, {'error': 'Invalid method'}) |
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 think we could return a 405 here, Method Not Allowed
test/acceptance/app.js
Outdated
@@ -3223,7 +3223,7 @@ describe('Application', function () { | |||
|
|||
it('should return 400 if request method is not supported', function (done) { | |||
request(connectionString) | |||
.put('/api/hooks/xx/config') | |||
.put('/api/hooks') |
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.
405? :)
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.
Looks great, apart from the HTTP response codes! :)
🥇
Totally! There were a few instances of |
This PR extends the hooks config endpoint (
/api/hooks/:hookName/config
) to acceptPOST
,PUT
andDELETE
requests to create, update and delete hooks, respectively. Acceptance tests are included.Close #254
@jimlambie I branched off
feat/media-endpoint
, and so I made the PR with that branch as base. Let me know if that's an issue.Thanks!