-
Notifications
You must be signed in to change notification settings - Fork 651
Separate permissions logic for comments #854
Conversation
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.
@joehoyle This comment looks misleading. We should presume we can view or get, not create.
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.
@rachelbaker I'm not sure the best way to handle this, as post_id is a required param, this will never return true however, this is also used for the OPTIONS pre flight check, so if someone does OPTIONS /wp/comments do we say they have permission to POST there or not? (as opposed to OPTIONS /wp/comments?post_id=123
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.
@joehoyle I think I understand where you are coming from. $post_id is a required parameter for creating a comment, but not for returning a collection of comments.
The issue is what to do in the following use-case:
Jane wants to sent a POST request to create a comment.
Jane first sends an OPTIONS request to wp/comments
Jane expects to receive a response to her request that includes if she CAN create a comment.
Is that correct?
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.
@rachelbaker doh you are totally right, I misread where you had put this comment. Yes, the use case you mentioned is correct, though the user should probably send a OPTIONS to wp/comments/?p=123 to check for that specific comment. In which case we should probably return false for POST wp/comments as I presume someone can't POST there.
Separate permissions logic for comments
|
Merged #854 |
As we can now specify the permissions logic separatly to the callbacks themselves, I tried this for comments. This has the benefit of more modular code, but also the client can now send HEAD requests to the resource to find out what method they are allowed to perform on the resource.
I removed the specific errors for each permission, but I can put them back in if we require. I also updated the tests to expect a 403 rather than a 401, as that's what we decided for failed cap
checks I believe (feel free to correct me here, I was going off what I interpretted from http://stackoverflow.com/questions/3297048/403-forbidden-vs-401-unauthorized-http-responses)