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

Allow specifying custom tags at the route level #523

Merged
merged 1 commit into from
Oct 22, 2016

Conversation

jordanfbrown
Copy link
Contributor

@jordanfbrown jordanfbrown commented Oct 20, 2016

The current logic to determine a route's set of tags is based solely on the route, which works in most cases, but isn't flexible when you want to override the tags.

The situation that led me here was the following setup:

prefix 'locations/:id'

resource :employees do
  desc 'Retrieve employees for a given location'

  get '/' do
    ...
  end
end

Which grouped the /locations/:id/employees under the locations namespace. This was undesirable, because almost all of the endpoints in our API are prefixed with /locations/:id, which meant that they were all grouped under the locations namespace.

Allowing an additional tags attribute on the desc method solved the issue for me.

desc 'Retrieve employees for a given location', tags: ['employees']

Before:

screenshot 2016-10-20 15 47 02

After:

screenshot 2016-10-20 15 47 12

Is there another way to accomplish this that I missed altogether, or does this seem like a reasonable addition?

@jordanfbrown jordanfbrown force-pushed the custom-route-tags branch 3 times, most recently from edd12e9 to 9340714 Compare October 20, 2016 22:51
@@ -4,7 +4,7 @@

* [#520](https://github.com/ruby-grape/grape-swagger/pull/520): Response model can have required attributes - [@WojciechKo](https://github.com/WojciechKo).
* [#510](https://github.com/ruby-grape/grape-swagger/pull/510): Use 'token_owner' instead of 'oauth_token' on Swagger UI endpoint authorization. - [@texpert](https://github.com/texpert).
* Your contribution here.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please return it after your changelog note

@kzaitsev
Copy link
Contributor

Hello, thank you for your contribution, this is a good feature, can you provide description of this feature in README.md?

@LeFnord
Copy link
Member

LeFnord commented Oct 21, 2016

thanks @jordanfbrowndid the global tag set also respect your changes?

@jordanfbrown … please add a section to teh README (don't forget a TOC entry 😉),
the global tag set is out of scope, should be generated from the route tags, after this PR is mörged

The current logic to determine a route's set of tags is based solely on the route, which works in most cases, but isn't flexible when you want to override the tags.

The situation that led me here was the following setup:

```
prefix 'locations/:id'

resource :employees do
  desc 'Retrieve employees for a given location'

  get '/' do
    ...
  end
end
```

Which grouped the `/locations/:id/employees` under the `locations` namespace. This was undesirable, because almost all of the endpoints in our API are prefixed with `/locations/:id`, which meant that they were all grouped under the `locations` namespace.

Allowing an additional `tags` attribute on the `desc` method solved the issue for me.

```
desc 'Retrieve employees for a given location', tags: ['employees']
```
@jordanfbrown
Copy link
Contributor Author

@Bugagazavr @LeFnord Feedback addressed, thanks!

@LeFnord
Copy link
Member

LeFnord commented Oct 22, 2016

👍

@LeFnord LeFnord merged commit 14364e3 into ruby-grape:master Oct 22, 2016
LeFnord pushed a commit to LeFnord/grape-swagger that referenced this pull request Feb 9, 2019
The current logic to determine a route's set of tags is based solely on the route, which works in most cases, but isn't flexible when you want to override the tags.

The situation that led me here was the following setup:

```
prefix 'locations/:id'

resource :employees do
  desc 'Retrieve employees for a given location'

  get '/' do
    ...
  end
end
```

Which grouped the `/locations/:id/employees` under the `locations` namespace. This was undesirable, because almost all of the endpoints in our API are prefixed with `/locations/:id`, which meant that they were all grouped under the `locations` namespace.

Allowing an additional `tags` attribute on the `desc` method solved the issue for me.

```
desc 'Retrieve employees for a given location', tags: ['employees']
```
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.

3 participants