-
Notifications
You must be signed in to change notification settings - Fork 524
Generators: decorators should respect --model-name
#931
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
base: master
Are you sure you want to change the base?
Generators: decorators should respect --model-name
#931
Conversation
--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.
250eb7b to
0099f25
Compare
| class ControllerGenerator | ||
| include Rails::Generators::ModelHelpers | ||
|
|
||
| class_option :model_name, type: :string, desc: "ModelName to be used" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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