Skip to content

UUID data_type format support #531

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Nov 24, 2016
Merged

Conversation

migmartri
Copy link
Contributor

Hi!

In the Swagger spec, uuid is mentioned next to email as a custom format. So I have added support for it.

In any case, have you guys thought about allowing the format customization via documentation[:format]?, i.e documentation: { type: 'string', format: 'uuid' }? so hardcoding formats is not needed anymore?

Currently the method document_type_and_format seems to to ignore the format for non primitive types.

Thanks!
Miguel

@grape-bot
Copy link

grape-bot commented Nov 22, 2016

<tr>
  <td>:warning:</td>
  <td data-sticky="false">There’re library changes, but not tests. That’s OK as long as you’re refactoring existing code.</td>
</tr>
1 Warning

Generated by 🚫 danger

@LeFnord
Copy link
Member

LeFnord commented Nov 22, 2016

thanks @migmartri for contributing, please add a spec
to your idea, think it has two sides, for one it is very fine to have it,
but the downside would be, one can introduce formats which are not supported by OpenAPI/swagger, so how would you handle it?

Updating changelog
@migmartri
Copy link
Contributor Author

Thanks @LeFnord

In my opinion we should proceed the following way:

  • Keep the scope of this patch to just add the uuid as part of the supported formats since it follows the current approach and uuid is explicitly mentioned in the OpenAPI spec.
  • Discuss via another issue the format override feature.

Please note that I have not added tests for my new type since I could not find any existing spec for the method #primitives or PRIMITIVE_MAPPINGS and I have only extended a constant value. If you still want me to work on extending the current spec/lib/data_type_spec.rb let me know.

Thanks,
Miguel

@LeFnord
Copy link
Member

LeFnord commented Nov 24, 2016

answered on referenced issue

@LeFnord
Copy link
Member

LeFnord commented Nov 24, 2016

@migmartri yeap, sounds good to me, so I will mörge this one, and the more general approach would be a new one

@LeFnord LeFnord merged commit cc30fc8 into ruby-grape:master Nov 24, 2016
@migmartri
Copy link
Contributor Author

@LeFnord thank you. I appreciate it :)

LeFnord pushed a commit to LeFnord/grape-swagger that referenced this pull request Dec 18, 2016
LeFnord pushed a commit to LeFnord/grape-swagger that referenced this pull request Feb 9, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants