Skip to content

options[:version] missing, changed to route.version. #438

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

Merged
merged 8 commits into from
May 30, 2016
6 changes: 3 additions & 3 deletions lib/grape-swagger/doc_methods/path_string.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module GrapeSwagger
module DocMethods
class PathString
class << self
def build(path, options = {})
def build(path, route_version, options = {})
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it'd be cleaner if this received the entire route? So def build(route, options = {})?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was my first thought eca701f but the PathString specs would need then some complex route initialization.

Copy link
Member

Choose a reason for hiding this comment

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

You can make the request an OpenStruct in the spec, it will actually look simpler I think. API > spec anyway.

# always removing format
path.sub!(/\(\.\w+?\)$/, '')
path.sub!('(.:format)', '')
Expand All @@ -13,8 +13,8 @@ def build(path, options = {})
# set item from path, this could be used for the definitions object
item = path.gsub(%r{/{(.+?)}}, '').split('/').last.singularize.underscore.camelize || 'Item'

if options[:version] && options[:add_version]
path.sub!('{version}', options[:version].to_s)
if route_version && options[:add_version]
path.sub!('{version}', route_version.to_s)
else
path.sub!('/{version}', '')
end
Expand Down
2 changes: 1 addition & 1 deletion lib/grape-swagger/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def path_item(routes, options)
routes.each do |route|
next if hidden?(route)

@item, path = GrapeSwagger::DocMethods::PathString.build(route.path, options)
@item, path = GrapeSwagger::DocMethods::PathString.build(route.path, route.version, options)
@entity = route.entity || route.options[:success]

verb, method_object = method_object(route, options, path)
Expand Down
3 changes: 2 additions & 1 deletion lib/grape-swagger/grape/route.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# backwards compatibility for Grape < 0.16.0
module Grape
class Route
[:path, :prefix, :entity, :description, :settings, :params, :headers, :http_codes].each do |m|
[:path, :prefix, :entity, :description, :settings, :params, :headers, :http_codes, :version]
.each do |m|
define_method m do
send "route_#{m}"
end
Expand Down
66 changes: 48 additions & 18 deletions spec/lib/path_string_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,59 @@

describe 'operation_id_object' do
describe 'version' do
describe 'defaults: not given, false' do
describe 'defaults: given, true' do
let(:options) { { add_version: true } }
let(:version) { 'v1' }

specify 'The returned path includes version' do
expect(subject.build('/{version}/thing(.json)', version, options)).to eql ['Thing', '/v1/thing']
expect(subject.build('/{version}/thing/foo(.json)', version, options)).to eql ['Foo', '/v1/thing/foo']
expect(subject.build('/{version}/thing(.:format)', version, options)).to eql ['Thing', '/v1/thing']
expect(subject.build('/{version}/thing/foo(.:format)', version, options)).to eql ['Foo', '/v1/thing/foo']
expect(subject.build('/{version}/thing/:id', version, options)).to eql ['Thing', '/v1/thing/{id}']
expect(subject.build('/{version}/thing/foo/:id', version, options)).to eql ['Foo', '/v1/thing/foo/{id}']
end
end

describe 'defaults: not given, both false' do
let(:options) { { add_version: false } }
let(:version) { nil }

specify do
expect(subject.build('/thing(.json)', options)).to eql ['Thing', '/thing']
expect(subject.build('/thing/foo(.json)', options)).to eql ['Foo', '/thing/foo']
expect(subject.build('/thing(.:format)', options)).to eql ['Thing', '/thing']
expect(subject.build('/thing/foo(.:format)', options)).to eql ['Foo', '/thing/foo']
expect(subject.build('/thing/:id', options)).to eql ['Thing', '/thing/{id}']
expect(subject.build('/thing/foo/:id', options)).to eql ['Foo', '/thing/foo/{id}']
specify 'The returned path does not include version' do
expect(subject.build('/thing(.json)', version, options)).to eql ['Thing', '/thing']
expect(subject.build('/thing/foo(.json)', version, options)).to eql ['Foo', '/thing/foo']
expect(subject.build('/thing(.:format)', version, options)).to eql ['Thing', '/thing']
expect(subject.build('/thing/foo(.:format)', version, options)).to eql ['Foo', '/thing/foo']
expect(subject.build('/thing/:id', version, options)).to eql ['Thing', '/thing/{id}']
expect(subject.build('/thing/foo/:id', version, options)).to eql ['Foo', '/thing/foo/{id}']
end
end

describe 'defaults: given, true' do
let(:options) { { version: 'v1', add_version: true } }

specify do
expect(subject.build('/{version}/thing(.json)', options)).to eql ['Thing', '/v1/thing']
expect(subject.build('/{version}/thing/foo(.json)', options)).to eql ['Foo', '/v1/thing/foo']
expect(subject.build('/{version}/thing(.:format)', options)).to eql ['Thing', '/v1/thing']
expect(subject.build('/{version}/thing/foo(.:format)', options)).to eql ['Foo', '/v1/thing/foo']
expect(subject.build('/{version}/thing/:id', options)).to eql ['Thing', '/v1/thing/{id}']
expect(subject.build('/{version}/thing/foo/:id', options)).to eql ['Foo', '/v1/thing/foo/{id}']
describe 'defaults: add_version false' do
let(:options) { { add_version: false } }
let(:version) { 'v1' }

specify 'The returned path does not include version' do
expect(subject.build('/thing(.json)', version, options)).to eql ['Thing', '/thing']
expect(subject.build('/thing/foo(.json)', version, options)).to eql ['Foo', '/thing/foo']
expect(subject.build('/thing(.:format)', version, options)).to eql ['Thing', '/thing']
expect(subject.build('/thing/foo(.:format)', version, options)).to eql ['Foo', '/thing/foo']
expect(subject.build('/thing/:id', version, options)).to eql ['Thing', '/thing/{id}']
expect(subject.build('/thing/foo/:id', version, options)).to eql ['Foo', '/thing/foo/{id}']
end
end

describe 'defaults: root_version nil' do
let(:options) { { add_version: true } }
let(:version) { nil }

specify 'The returned path does not include version' do
expect(subject.build('/thing(.json)', version, options)).to eql ['Thing', '/thing']
expect(subject.build('/thing/foo(.json)', version, options)).to eql ['Foo', '/thing/foo']
expect(subject.build('/thing(.:format)', version, options)).to eql ['Thing', '/thing']
expect(subject.build('/thing/foo(.:format)', version, options)).to eql ['Foo', '/thing/foo']
expect(subject.build('/thing/:id', version, options)).to eql ['Thing', '/thing/{id}']
expect(subject.build('/thing/foo/:id', version, options)).to eql ['Foo', '/thing/foo/{id}']
end
end
end
Expand Down
30 changes: 30 additions & 0 deletions spec/swagger_v2/endpoint_versioned_path_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
require 'spec_helper'

describe 'Grape::Endpoint#path_and_definitions' do
before do
module API
module V1
class Item < Grape::API
version 'v1', using: :path

resource :item do
get '/'
end
end
end

class Root < Grape::API
mount API::V1::Item
add_swagger_documentation add_version: true
end
end

@options = { add_version: true }
@target_routes = API::Root.combined_namespace_routes
end

it 'is returning a versioned path' do
expect(API::V1::Item.endpoints[0]
.path_and_definition_objects(@target_routes, @options)[0].keys[0]).to eql '/v1/item'
end
end