removes limit on model names, updates README

[LeFnord]

• updates README

[JanStevens]

This will still make assumptions about the grape / entity / class structure, can we just leave it as entity.name or entity.entity_name if defined?
There is no reference in the spec that definitions should have a specific naming convention.

I argue for this point that this will be the most transparent to the developer. For internal documentation it helps to correctly show the entity name (fully).

For example take: AnimalFarm::Internal::CowEntity and AnimalFarm::Internal:GoatEntity both would be rendered as AnimalFarm::Internal

[LeFnord]

ah, yeah I see …

[LeFnord]

it should now be correct …

the assumentions based in both documentations, one is in grape-swagger self, and very often used in the wild and the other one is this, you mentioned from grape-entity

for the first one it would be assumed, that only the class name is the relevant part
for the second only Entit{y|ies} would be removed …
should it then be handled in the same manner as first case?

[JanStevens]

For consistency sake all cases should be handled the same. People can easily overwrite it using the entry name method

[serggl]

Looks good to me

[JanStevens]

Take the example again of animals:

AnimalFarm::Internal::CowEntity and an AnimalFarm::Public::CowEntity, the internal is only used by internal apps and exposes all attributes, the public one only the important attributes. Using this they both end up being CowEntity and depending on the load order the swagger output will show the public or internal

[JanStevens]

Can this elsif just be removed and default to entity.name? less assumptions == happier developer

[serggl]

btw @LeFnord why do we check for end_with? here, but removing all 'Entity' string parts here https://github.com/ruby-grape/grape-swagger/blob/master/lib/grape-swagger/doc_methods/data_type.rb#L49
Which algo is correct?

[JanStevens]

Sorry to be a bitch about this but can it just be like this? :D

def model_name(entity)
  if entity.respond_to?(:entity_name)
    entity.entity_name
  else
    entity.name
  end
end

Benefits:

• No assumptions about your entity namespace/class structure
• Full transparency: As a dev I can easily see which entity/class is being used
• All the special stuff and naming can be done in the following way by the users of grape-swagger (if they want it at all)

class ApplicationEntity < Grape::Entity
  def self.entity_name
    name.to_s.split('::')[-2]
    # OR
    name.demodulize.camelize
  end
end

class AnimalFarmEntity < ApplicationEntity
  ...
end

[LeFnord]

@JanStevens … benefits of this solution are:

• both documented (see above) orginsation structures would be supported
• by a different one, one can set the name self -> 'special stuff'
• no breaking changes

your proposed solution would intruduce one

[JanStevens]

Fair enough, considerable for next major release? Or should I drop it 😀

On Wednesday, 12 October 2016, peter scholz notifications@github.com
wrote:

@JanStevens https://github.com/JanStevens … benefits of this solution
are:

• both documented (see above) orginsation structures would be supported
• by a different one, one can set the name self -> 'special stuff'
• no breaking changes

your proposed solution would intruduce one

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#515 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABeBg4lPh6ZqgUvW0hbWH1HykeM_mDPsks5qzU1igaJpZM4KS2nn
.

~Jan Stevens

[dblock]

I would remove "updates README", it's not relevant, but nice.

[dblock]

I'm going to vote no breaking changes and merge this, but @JanStevens you have a good point, so maybe you want to open a PR on top of this that introduces your proposal and we can talk about that again?

[LeFnord]

@JanStevens … don't drop it … such things should be forgotten, maybe we can use the project feature?
@dblock … the merge was a bit to quick -> what did you think about the project feature for better organizing of ideas/proposals/etc?

[dblock]

I thought this was a good step forward @LeFnord. I'll slow down :)

[johnvuko]

Hi,
in my project I have quite a few models and so I use a lot the namespacing and I find this PR very useful.
I want to have all my entities having their name with the namespace, so instead of defining a method entity_name in each model I would prefer to just inherit from a class, but in the code https://github.com/ruby-grape/grape-swagger/blob/master/lib/grape-swagger/doc_methods/data_type.rb#L44
you did

        def parse_entity_name(model)
          if model.methods(false).include?(:entity_name)
            model.entity_name
          elsif model.to_s.end_with?('::Entity', '::Entities')
            model.to_s.split('::')[-2]
          elsif model.respond_to?(:name)
            model.name.demodulize.camelize
          else
            model.to_s.split('::').last
          end
        end

If you could jsut change model.methods(false).include?(:entity_name) by model.methods.include?(:entity_name) it would be very nice.
I think this is more natural and more logic and it avoid to defining the entity_name everywhere like in my case.

By the way, I think showing the full name is a better behaviour than just the name. Having an internal and external entity seem a little strange, (for what I understand) entities are used for expose data through an API so they are not supposed to be private / internal and it's seems to be a lot more improbable than having two classes with the same name in different namespace.

[LeFnord]

@jonathantribouharet please can you commit your thoughts on this discussion #519