Generators: decorators should respect --model-name
#931
Conversation
Alexander-Senko
changed the title
--model-name
Decorator will respect `--model-name` passed to `scaffold_controller` or `controller` generator. That could be of a particular use to solve namespace issues (e.g. when controllers are namespaced and models are not). The specs have been refactored to reduce and DRY the code. Resolves drapergem#919.
Alexander-Senko
force-pushed
the
generators/model_name
branch
from
250eb7b
to
0099f25
Compare
|
|
||
| module Rails | ||
| module Generators | ||
| class ControllerGenerator | ||
| include Rails::Generators::ModelHelpers | ||
|
|
||
| class_option :model_name, type: :string, desc: "ModelName to be used" |
There was a problem hiding this comment.
Hmm. Basically, controller doesn't need to know about model. And, the current implementation depends on controller name for naming. So overring controller name by model_name looks strange to me.
How about using decorator_name?
There was a problem hiding this comment.
scaffold_controller already has that interface and I'd prefer controller to be uniform with it. Rails already has --model-name option and I'd rather use it.
I agree that model dependency for controller is dubious, but we already have a decorator hook for controller that needs to know about a model. That's why controller may also need to.
Another option is to remove decorator hook from controller and add it to model. In my opinion, decorators are bound to models rather than controllers or views. However, I'd keep the existing API for now rather than introducing another braking change.
There was a problem hiding this comment.
I agree that it might be good to hook my models, not controllers. But, it seems that the current behavior is intended #553.
So generating decorators by controller names is the intended behavior(At least, so far). Setting a decorator name by --model-name is inconsistent with that behavior I think.
There was a problem hiding this comment.
So generating decorators by controller names is the intended behavior.
Yes, it is. And it's not being changed.
Models are also generated based on controller names, but we have --model-name to override that when needed.
scaffold_controlleralready has that interface…
So, why shouldn't a decorator respect it as well when it's specified explicitly?
I'm afraid, I'm missing your point… What is your solution for #919?
There was a problem hiding this comment.
Yes, it is. And it's not being changed.
I think this PR changes the behavior. For example, if you run ./bin/rails g scaffold admin/book name:string --model-name book, the result will be change between master branch and this PR.
To be honest, I don't think #919 is a bug. That is the intended behavior of the current generator structure(I can't understand why draper only hooks for controller though).
But of course, I understand the user's expectations. So adding decorator_name(or something like that) to override decorators name is a good solution I think.
Description
Decorator will respect
--model-namepassed toscaffold_controllerorcontrollergenerator.That could be of a particular use to solve namespace issues (e.g. when controllers are namespaced and models are not).
The specs have been refactored to reduce and DRY the code.
Testing
controllerorscaffold_controllergenerator with--model-nameoption.To-Dos
References