Merge branch 'grape-router-helpers-bugfix' into 'master'
Fix grape-route-helper route shadowing See merge request gitlab-org/gitlab-ce!16781
This commit is contained in:
commit
51c8d5b2f3
|
@ -1,5 +1,21 @@
|
||||||
if defined?(GrapeRouteHelpers)
|
if defined?(GrapeRouteHelpers)
|
||||||
module GrapeRouteHelpers
|
module GrapeRouteHelpers
|
||||||
|
module AllRoutes
|
||||||
|
# Bringing in PR https://github.com/reprah/grape-route-helpers/pull/21 due to abandonment.
|
||||||
|
#
|
||||||
|
# Without the following fix, when two helper methods are the same, but have different arguments
|
||||||
|
# (for example: api_v1_cats_owners_path(id: 1) vs api_v1_cats_owners_path(id: 1, owner_id: 2))
|
||||||
|
# if the helper method with the least number of arguments is defined first (because the route was defined first)
|
||||||
|
# then it will shadow the longer route.
|
||||||
|
#
|
||||||
|
# The fix is to sort descending by amount of arguments
|
||||||
|
def decorated_routes
|
||||||
|
@decorated_routes ||= all_routes
|
||||||
|
.map { |r| DecoratedRoute.new(r) }
|
||||||
|
.sort_by { |r| -r.dynamic_path_segments.count }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
class DecoratedRoute
|
class DecoratedRoute
|
||||||
# GrapeRouteHelpers gem tries to parse the versions
|
# GrapeRouteHelpers gem tries to parse the versions
|
||||||
# from a string, not supporting Grape `version` array definition.
|
# from a string, not supporting Grape `version` array definition.
|
||||||
|
|
|
@ -0,0 +1,14 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
require_relative '../../config/initializers/grape_route_helpers_fix'
|
||||||
|
|
||||||
|
describe 'route shadowing' do
|
||||||
|
include GrapeRouteHelpers::NamedRouteMatcher
|
||||||
|
|
||||||
|
it 'does not occur' do
|
||||||
|
path = api_v4_projects_merge_requests_path(id: 1)
|
||||||
|
expect(path).to eq('/api/v4/projects/1/merge_requests')
|
||||||
|
|
||||||
|
path = api_v4_projects_merge_requests_path(id: 1, merge_request_iid: 3)
|
||||||
|
expect(path).to eq('/api/v4/projects/1/merge_requests/3')
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in New Issue