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

Cop Idea: Controller Action Order #540

Closed
mollerhoj opened this issue Sep 10, 2021 · 13 comments · Fixed by #547
Closed

Cop Idea: Controller Action Order #540

mollerhoj opened this issue Sep 10, 2021 · 13 comments · Fixed by #547

Comments

@mollerhoj
Copy link
Contributor

The default order of controller actions in rails scaffolds are:

index show new edit create update destroy

I often see controllers where this order is not followed. For consistency's sake, I'd like to have a cop to enforce this ordering of the public methods in my controllers.

@andyw8
Copy link
Contributor

andyw8 commented Sep 11, 2021

I'd suggest first opening a PR to add this to https://github.com/rubocop/rails-style-guide, and then adding it here if there's consensus.

@mollerhoj
Copy link
Contributor Author

mollerhoj commented Sep 11, 2021

Done: rubocop/rails-style-guide#289

@pirj
Copy link
Member

pirj commented Sep 11, 2021

Even though (https://docs.rubocop.org/rubocop-rails/):

RuboCop Rails [ is ] A RuboCop extension focused on enforcing Rails best practices and coding conventions.

and (https://docs.rubocop.org/rubocop/1.20/index.html#overview):

A long-term goal of RuboCop (and its core extensions) is to cover with cops all the guidelines from the community style guides.

https://docs.rubocop.org/rubocop/1.20/index.html#philosophy

While we still believe that there’s a lot of merit to just sticking to the community style guides, we acknowledge that Ruby is all about diversity and doing things the way that makes you happy. Whatever style preferences you have RuboCop is there for you.

Having such a cop would be nice.

I'd suggest using https://github.com/rubocop/rubocop/blob/master/lib/rubocop/cop/layout/class_structure.rb as a reference and make the cop configurable and flexible to meet other styles.

@dvandersluis
Copy link
Member

+1 for a cop with a configurable order.

@mollerhoj
Copy link
Contributor Author

All I need is a go from a core member that the cop would be merged, and I'll make a PR😊

@andyw8
Copy link
Contributor

andyw8 commented Sep 12, 2021

I'm in favour of it as long as:

  • It's disabled by default
  • The order is configurable
  • There is a default ordering (i..e. when enabled, the cop will work without configuration). I think following the Rails convention would be the least controversial default

@pirj
Copy link
Member

pirj commented Sep 12, 2021

Go!

@koic
Copy link
Member

koic commented Sep 13, 2021

I often see a method called confirm. Such methods that are not included in the default configuration (i.e. result code of scaffold) will need to be ignored.

@mollerhoj
Copy link
Contributor Author

confirm

Huh? Can you elaborate? I've never seen a "confirm" method in a controller?

I'd suggest only ignoring private methods - I hope to influence people to only use the default methods: https://www.youtube.com/watch?v=HctYHe-YjnE

@andyw8
Copy link
Contributor

andyw8 commented Sep 13, 2021

Not just private methods, it should ignore any that aren't part of the standard 7, and should allow non-standard methods to exist before, after, or intermingled with the standard 7.

@andyw8
Copy link
Contributor

andyw8 commented Sep 13, 2021

I hope to influence people to only use the default methods

I think that should be treated as a separate cop from the action ordering.

@dvandersluis
Copy link
Member

Yeah, whether or not a controller "should" have non-restful actions, a large percentage of them do. This cop should not consider them at all.

@mollerhoj
Copy link
Contributor Author

I hope to influence people to only use the default methods

I think that should be treated as a separate cop from the action ordering.

Let's see if I can convince the masters of the style guide rubocop/rails-style-guide#290

@koic koic closed this as completed in #547 Oct 6, 2022
jdufresne added a commit to jdufresne/rubocop-rails that referenced this issue Oct 23, 2022
Commit 0ef065a introduced the
Rails/ActionOrder cop to enforce the order of RESTful action methods in
controllers.

These actions also appear in routes configuration through the `resource`
and `resources` methods (docs linked below). The cop should enforce the
order for these actions as well.

https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Resources.html#method-i-resources

Refs rubocop#540
jdufresne added a commit to jdufresne/rubocop-rails that referenced this issue Oct 23, 2022
Commit 0ef065a introduced the
Rails/ActionOrder cop to enforce the order of RESTful action methods in
controllers.

These actions also appear in routes configuration through the `resource`
and `resources` methods (docs linked below). The cop should enforce the
order for these actions as well.

https://api.rubyonrails.org/classes/ActionDispatch/Routing/Mapper/Resources.html#method-i-resources

Refs rubocop#540
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants