Use full entity name as a default (fixes #779) #786
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I just wanted to resolve this:
```
- model.to_s.split('::').last
+ model.to_s
```
But I met a problem that it receives Class type in here
https://github.com/ruby-grape/grape-swagger/blob/8abddd5f2ce5fe29238990b136625f34371b0be0/lib/grape-swagger/endpoint.rb#L354
But String type in the another place, where the type is generated. So,
splitting and taking the last actually returns the same as `name`
method of the class. That is very strange I think.
Also, if we, for example, use such constructions, we end up with just
`V1` and `V2` fron the `A::B` module as they are parsed first:
```ruby
module A
module B
class V1 < Grape::Entity
...
class V2 < Grape::Entity
...
module A
module C
class V1 < Grape::Entity
...
class V2 < Grape::Entity
...
```
This PR fixes this issue.
* [#786](https://github.com/ruby-grape/grape-swagger/pull/786): Use full entity name as a default - [@mrexox](https://github.com/mrexox)Generated by 🚫 danger |
3 similar comments
LeFnord
requested changes
Apr 18, 2020
|
@LeFnord , added an upgrade entry. Does it look fine? |
LeFnord
approved these changes
Apr 20, 2020
|
will release it tonight |
aka-momo
pushed a commit
to aka-momo/grape-swagger
that referenced
this pull request
Feb 8, 2023
) * Use full entity name as a default (fixes ruby-grape#779) I just wanted to resolve this: ``` - model.to_s.split('::').last + model.to_s ``` But I met a problem that it receives Class type in here https://github.com/ruby-grape/grape-swagger/blob/8abddd5f2ce5fe29238990b136625f34371b0be0/lib/grape-swagger/endpoint.rb#L354 But String type in the another place, where the type is generated. So, splitting and taking the last actually returns the same as `name` method of the class. That is very strange I think. Also, if we, for example, use such constructions, we end up with just `V1` and `V2` fron the `A::B` module as they are parsed first: ```ruby module A module B class V1 < Grape::Entity ... class V2 < Grape::Entity ... module A module C class V1 < Grape::Entity ... class V2 < Grape::Entity ... ``` This PR fixes this issue. * Update CHANGELOG * Fix for old Ruby versions * Skip Representable:: prefix just like Entity:: * Add upgrade entry and move changelog entry from fixes to features scope
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this PR I just wanted to end with this:
But I met a problem that it receives Class type in here
grape-swagger/lib/grape-swagger/endpoint.rb
Line 354 in 8abddd5
But String type in the another place. So, splitting by
::and taking the last word actually returns the same asnamemethod of the class. This is fine, but...For example, if we use such constructions, we end up with just
V1andV2from theA::Bmodule as it is parsed first:This PR fixes this issue.