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 multiple version definitions for path versioning #1414

Merged
merged 3 commits into from
Jun 8, 2016

Conversation

304
Copy link
Contributor

@304 304 commented Jun 6, 2016

It is a fix for issue #1400

I created a new spec for this case and found out why it was not working.

Issue ruby-grape#1400

* New spec for this case

* Update CHANGELOG
@@ -28,6 +28,7 @@ module ClassMethods
#
def version(*args, &block)
if args.any?
args.flatten!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem was hidden here.
When the version is defined as an array:

version ['v1', 'v2']

then args is presented as an array inside of an array: [["v1", "v2"]], so flatten! here helps to resolve the issue.

Copy link
Member

Choose a reason for hiding this comment

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

You should not be modifying the parameters passed in, so something like args = args.flatten. However, do investigate why you're getting an array of arrays here, it's possible that there's a better fix upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I've extracted version parameter into local variable to describe the intention in a more explicit way => 82e1f21

In this case the upstream is defined by user. I've checked Grape documentation, but I've not found any examples how to properly define several versions for the endpoint. Looks like it is an undocumented feature that was supporting arrays before routing refactoring.

Should it be like that version ['v1', 'v2'] or like that version 'v1', 'v2' or it is not a correct way to define a version at all?

@dblock
Copy link
Member

dblock commented Jun 8, 2016

This is good, merging.

@dblock dblock merged commit 4d1883f into ruby-grape:master Jun 8, 2016
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.

2 participants