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

Controller generator improvements #45627

Merged
merged 2 commits into from
Jul 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions railties/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
* Add `--parent` option to controller generator to specify parent class of job.

Example:

`bin/rails g controller admin/users --parent=admin_controller` generates:

```ruby
class Admin::UsersController < AdminController
# ...
end
```

*Gannon McGibbon*

* In-app custom credentials templates are now supported. When a credentials
file does not exist, `rails credentials:edit` will now try to use
`lib/templates/rails/credentials/credentials.yml.tt` to generate the
Expand Down
16 changes: 12 additions & 4 deletions railties/lib/rails/generators/rails/controller/USAGE
Original file line number Diff line number Diff line change
@@ -1,18 +1,26 @@
Description:
Generates a new controller and its views. Pass the controller name, either
CamelCased or under_scored, and a list of views as arguments.
CamelCased or under_scored, and a list of actions as arguments.

To create a controller within a module, specify the controller name as a
path like 'parent_module/controller_name'.

This generates a controller class in app/controllers and invokes helper,
template engine, assets, and test framework generators.

Example:
`bin/rails generate controller CreditCards open debit credit close`
Examples:
`bin/rails generate controller credit_cards open debit credit close`

CreditCards controller with URLs like /credit_cards/debit.
This generates a `CreditCardsController` with routes like /credit_cards/debit.
Controller: app/controllers/credit_cards_controller.rb
Test: test/controllers/credit_cards_controller_test.rb
Views: app/views/credit_cards/debit.html.erb [...]
Helper: app/helpers/credit_cards_helper.rb

`bin/rails generate controller users index --skip-routes`

This generates a `UsersController` with an index acion and no routes.
Copy link

Choose a reason for hiding this comment

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

acion -> action

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Do you want to open a PR?


`bin/rails generate controller admin/dashboard --parent=admin_controller`

This generates a `Admin::DashboardController` with an `AdminController` parent class.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class ControllerGenerator < NamedBase # :nodoc:
argument :actions, type: :array, default: [], banner: "action action"
class_option :skip_routes, type: :boolean, desc: "Don't add routes to config/routes.rb."
class_option :helper, type: :boolean
class_option :parent, type: :string, desc: "The parent class for the generated controller"

check_class_collision suffix: "Controller"

Expand All @@ -25,6 +26,18 @@ def add_routes
end

private
def parent
options[:parent]
end

def parent_class_name
if parent
parent
else
"ApplicationController"
end
Comment on lines +34 to +38
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if parent
parent
else
"ApplicationController"
end
parent || "ApplicationController"

Copy link
Member

Choose a reason for hiding this comment

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

I think it will be a good idea to enable the relevant rubocop's check - https://docs.rubocop.org/rubocop/1.31/cops_style.html#styleredundantcondition

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree this would be better addressed by enabling the cop. Looks like there's only 3 offenses in all of Rails and I've introduced 2 of them. 😅

Would you like to open a PR to introduce this @fatkodima?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea @fatkodima ! 👍

Copy link
Member

Choose a reason for hiding this comment

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

instead of adding this method, could a default: "ApplicationController" be specified on the class_option?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but I think a more sweeping change could be made to change the other generators too so they look the same, more or less. I'll open another PR to do that. It is likely better to state the default in the class opt so it shows up in the help text.

end

def file_name
@_file_name ||= remove_possible_suffix(super)
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<% module_namespacing do -%>
class <%= class_name %>Controller < ApplicationController
class <%= class_name %>Controller < <%= parent_class_name.classify %>
<% actions.each do |action| -%>
def <%= action %>
end
Expand Down
7 changes: 7 additions & 0 deletions railties/test/generators/controller_generator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,13 @@ def test_does_not_add_routes_when_action_is_not_specified
end
end

def test_controller_parent_param
run_generator ["admin/dashboard", "--parent", "admin_controller"]
assert_file "app/controllers/admin/dashboard_controller.rb" do |controller|
assert_match(/class Admin::DashboardController < AdminController/, controller)
end
end

def test_controller_suffix_is_not_duplicated
run_generator ["account_controller"]

Expand Down