Skip to content
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

return 201 on successfull invite #147

Closed
wants to merge 1 commit into from
Closed

Conversation

individual-it
Copy link
Member

currently ocis returns 201 and not 200 when the invite is successfull, I think that is correct so adjust the API definition

@rhafer
Copy link
Contributor

rhafer commented Nov 29, 2023

MS says it's a 200: https://learn.microsoft.com/en-us/graph/api/driveitem-invite?view=graph-rest-1.0&tabs=http

I am not really fluent in HTTP, but IIUC https://datatracker.ietf.org/doc/html/rfc7231#section-6.3.1 says a 201 Response should come with Location Header (AFAIK we don't set one). The body seems to be kind of optional. AFAICT the current behavior our implementation matches more what is expected for a 200 status code. (So maybe we should fix the implemenation and return a 200)

I am pretty sure we're doing this inconsistent in a few other places as well. 😢

@micbar @fschade comments?

@individual-it
Copy link
Member Author

@rhafer to change it to 200, here a PR to ocis: owncloud/ocis#7851

@butonic
Copy link
Member

butonic commented Nov 30, 2023

The ms graph api returns a 200: https://learn.microsoft.com/en-us/graph/api/driveitem-invite?view=graph-rest-1.0&tabs=http#http-request-1

AFAICT the /invite endpoint is an action

From the odata spec: https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_CommonResponseStatusCodes

9.1.1 Response Code 200 OK

A request that does not create a resource returns 200 OK if it is completed successfully and the value of the resource is not null. In this case, the response body MUST contain the value of the resource specified in the request URL.
9.1.2 Response Code 201 Created

A Create Entity, Create Media Entity, or Invoke Action request that successfully creates a resource returns 201 Created. In this case, the response body MUST contain the resource created.

Which would mean a 201.

But ... to stay compatdible with the ms graph api I would prefer to return 200.

Oh wait. Technically, the invite does not create a driveItem. It only changes a property on the drive item: permissions only exist as a property of driveItems. So in this case 200 is perfectly correct.

@butonic butonic closed this Nov 30, 2023
@individual-it individual-it deleted the return201OnInvite branch November 30, 2023 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants