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

Fixes paths that starts with mount path followed by dash #261

Merged
merged 1 commit into from
Jun 22, 2015

Conversation

lasseebert
Copy link
Contributor

This is an addition to https://github.com/tim-vandecasteele/grape-swagger/pull/260

There is still a bug in which too many endpoints are hidden when documentation path is hidden.
Changed regex to only disallow

  • /mount_path
  • /mount_path/*
  • /mount_path(.:format)

Specifically, I still had a bug if an endpoint has "#{mount_point}-" in it. See the spec.

@lasseebert lasseebert force-pushed the path_clash_with_hidden_docs branch from 8d02845 to 3d8165c Compare June 22, 2015 08:25
@@ -31,7 +31,8 @@ def add_swagger_documentation(options = {})
next if resource.empty?
resource.downcase!
@target_class.combined_routes[resource] ||= []
next if documentation_class.hide_documentation_path && route.route_path.match(/#{documentation_class.mount_path}(\W|$)/)
next if documentation_class.hide_documentation_path && route.route_path.match(/#{documentation_class.mount_path}($|\/|\(\.)/)
p route.route_path
Copy link
Member

Choose a reason for hiding this comment

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

Stray p.

@lasseebert lasseebert force-pushed the path_clash_with_hidden_docs branch from 3d8165c to 0ccb330 Compare June 22, 2015 18:43
@lasseebert
Copy link
Contributor Author

Thanks for the review :)
Both issues fixed and squashed.

dblock added a commit that referenced this pull request Jun 22, 2015
Fixes paths that starts with mount path followed by dash
@dblock dblock merged commit dd284f4 into ruby-grape:master Jun 22, 2015
@dblock
Copy link
Member

dblock commented Jun 22, 2015

Merged, thanks!

@lasseebert
Copy link
Contributor Author

Thanks :)

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