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

Response model can have required attributes #520

Closed
wants to merge 0 commits into from
Closed

Response model can have required attributes #520

wants to merge 0 commits into from

Conversation

WojciechKo
Copy link
Contributor

Now you can mark attribute that is required in response model.

@kzaitsev
Copy link
Contributor

Hello, i think that should be a part of https://github.com/ruby-grape/grape-swagger-entity

@WojciechKo
Copy link
Contributor Author

It's not possible because https://github.com/ruby-grape/grape-swagger-entity uses
GrapeSwagger.model_parsers.
With that we can manipulate properties attribute of model definition but we need to add required array to model definition. Like in example:

definitions: {
  Location: {
    type: "object",
    properties: {
      id: { type: "integer" },
      name: { type: "string" }
    },
    required: ["id", "name"],
  },
  ...
}

@kzaitsev
Copy link
Contributor

kzaitsev commented Oct 13, 2016

Ok that sounds reasonable, please fix a tests, write a spec for required attributes, put this pull request to CHAGELOG.md, and add notes to README.md about required attributes

@kzaitsev
Copy link
Contributor

/cc @LeFnord

@WojciechKo
Copy link
Contributor Author

Done.

@LeFnord
Copy link
Member

LeFnord commented Oct 13, 2016

thanks @WojciechKo … please can you add a section in the README to document it

@WojciechKo
Copy link
Contributor Author

@LeFnord I think that's clear enough :)

@LeFnord
Copy link
Member

LeFnord commented Oct 14, 2016

thanks @WojciechKo … it is totally strange … please can you make rubocop happy?

@WojciechKo WojciechKo closed this Oct 14, 2016
@LeFnord
Copy link
Member

LeFnord commented Oct 14, 2016

Why? … hope you want to re-open it

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