Skip to content

Conversation

@virkt25
Copy link
Contributor

@virkt25 virkt25 commented Apr 11, 2018

temporary fix for #788

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

@virkt25 virkt25 requested a review from a team April 11, 2018 19:45
}

// Any item down the sequence chain can override the statusCode if needed
if (request.method === 'POST') {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is too hacky. Not all POST is for create. Can we allow/infer the statusCode from https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#responses-object?

Copy link
Contributor

@raymondfeng raymondfeng Apr 11, 2018

Choose a reason for hiding this comment

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

There are a few potential options for controller methods to express status code:

  1. Direct access to Response object
  2. Decorate the method to customize the default status code
  3. Return a HttpResponse object such as {statusCode: 201, headers: {}, body: result}. It's similar as https://docs.oracle.com/javaee/6/api/javax/ws/rs/core/Response.ResponseBuilder.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have issues to track the stories of providing a way for users to set the statusCode and appropriate response headers.

I personally like option 3 -- but for a POST request I think we should automatically be defaulting to 201 if a different statusCode is not provided.

Copy link
Contributor

@jannyHou jannyHou Apr 16, 2018

Choose a reason for hiding this comment

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

@virkt25 After I did my PoC in #1260, I tend to agree with your workaround here as the quickest fix....or allowing more time, we can do proposal#3.

Miroslav brings an important thing: when a response returns 201, it also includes a header with the URI of created item.

but for a POST request I think we should automatically be defaulting to 201 if a different statusCode is not provided.

I still feel 200 is better, since if default POST to 201, the user need to provide header in result by default as well.

@virkt25
Copy link
Contributor Author

virkt25 commented Apr 13, 2018

I believe @jannyHou is investigating a less "hacky" approach so I'll close this PR for now.

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.

5 participants