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

Fix request body and response representation for ref POST operations #237

Merged

Conversation

millicentachieng
Copy link
Member

@millicentachieng millicentachieng commented Jun 22, 2022

Fixes #228

  • Create reference for ref POST operations in requestBodies
  • Update schema for ref POST operations to include odata.id
  • Update response status code to 204 from 201

POST Operation
image

Referenced Request Body
image

Referenced Schema
image

@millicentachieng millicentachieng changed the title Fix/request body and response representation for ref operations Fix request body and response representation for ref operations Jun 23, 2022
@millicentachieng millicentachieng marked this pull request as ready for review June 23, 2022 18:39
@millicentachieng millicentachieng changed the title Fix request body and response representation for ref operations Fix request body and response representation for ref POST operations Jun 23, 2022
@peombwa peombwa requested a review from baywet June 23, 2022 20:56
peombwa
peombwa previously approved these changes Jun 23, 2022
@irvinesunday
Copy link
Contributor

I think we can also do the same for the RequestBody for ref PUT operations?
cc: @baywet

@baywet
Copy link
Member

baywet commented Jun 26, 2022

I know that patch uses the odata bind style on the parent entity. I'm not sure whether put uses odata bind or is straight on the collection like post. Can you recall?

@irvinesunday
Copy link
Contributor

irvinesunday commented Jun 27, 2022

I think we can also do the same for the RequestBody for ref PUT operations? cc: @baywet

(Edited the previous post)

This is the current schema in the RequestBody for ref PUT operations:

image

We can refactor it out into a reusable component as well and reference it as:
image

@baywet
Copy link
Member

baywet commented Jun 27, 2022

@irvinesunday correct, thanks for catching that, they are essentially the same since the odata type is nullable. To be specific, inserting a new reference in a collection (e.g. a new group member) via POST or updating a single reference (e.g. manager) via PUT takes the same payload. (and both reply 204)

@millicentachieng millicentachieng force-pushed the fix/requestBodyAndResponseRepresentationForRefOperations branch from 59114ec to 2c3c51b Compare June 29, 2022 08:38
@baywet
Copy link
Member

baywet commented Jun 29, 2022

@millicentachieng I believe @irvinesunday was arguing to refactor things so both PUT and POST in the cases we outlined use the same schema component, or did I misunderstand the point?

@millicentachieng
Copy link
Member Author

@baywet PUT and POST reference different schemas.
image

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes. Re-reading Irvine's comment I realized he was advocating for a naming convention, not reusing the same schema for PUT and POST. A couple of minor things left.

Co-authored-by: Vincent Biret <vibiret@microsoft.com>
@millicentachieng millicentachieng requested a review from zengin as a code owner July 5, 2022 19:56
baywet
baywet previously approved these changes Jul 6, 2022
@irvinesunday irvinesunday merged commit f7f2689 into master Jul 7, 2022
@irvinesunday irvinesunday deleted the fix/requestBodyAndResponseRepresentationForRefOperations branch July 7, 2022 14:45
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.

Inacurrate request body and response representation for ref POST operations
4 participants