Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Controller Generator #1103

Merged
merged 1 commit into from Jun 8, 2011

Conversation

Projects
None yet
4 participants
Contributor

Mab879 commented May 31, 2011

Hello,
I have made a new rails generator that moves the controller in to the users application so they can be customised just like the views. I have also written tests for my additions as well.

Thanks,
Mab879

@josevalim josevalim added a commit that referenced this pull request Jun 8, 2011

@josevalim josevalim Merge pull request #1103 from Mab879/master
Controller Generator
944e1c0

@josevalim josevalim merged commit 944e1c0 into plataformatec:master Jun 8, 2011

Owner

josevalim commented Jun 8, 2011

Merged, thanks!

Owner

josevalim commented Jun 8, 2011

Actually, I am going to revert this. Copying is a bad thing, we should generate controllers using inheritance instead. Would you mind trying out a patch?

@josevalim josevalim added a commit that referenced this pull request Jun 8, 2011

@josevalim josevalim Revert "Merge pull request #1103 from Mab879/master"
This reverts commit 944e1c0, reversing
changes made to 97659a1.
6250fa8
Contributor

Mab879 commented Jun 8, 2011

I wouldn't testing a few things out.

Contributor

Chun-Yang commented Aug 22, 2014

Is anyone on this? I think we need this feature.

Contributor

Chun-Yang commented Aug 22, 2014

I'd like to make a pull request.

Owner

lucasmazza commented Aug 22, 2014

I think this might be a valuable feature - something like rails g devise:controller sessions generating a app/controllers/sessions_controller.rb file. @Chun-Yang feel free to work on it 👍 😄

Contributor

Chun-Yang commented Aug 22, 2014

OK. I am on it. Now I have things to do on the weekends. : )

Owner

josevalim commented Aug 22, 2014

Before there is work on this feature, there are a couple issues we need to discuss. Generating a controller with just inheritance seems to be too much for too little? Also worth remembering that a custom controller requires changing the routes file and I don't think we can do it automatically?

Contributor

Chun-Yang commented Aug 22, 2014

Here are my motivations:

  1. Devise already has a generator for views and it would be more consistent to have controller generator.
  2. New users (like me) would take time to write their custom controllers in the right place with the right class name.
  3. It would be helpful to automatically generate all the methods which we can rewrite in the developer's app, since we then KNOW which methods we can modify by checking the generated controllers.

About modifying routes:

  1. I think we can automatically update the controller by some parsing and regex. I really would like to try.
  2. If we can not modify route.rb, we can give developers some guidance in the console.

What do you think?

Owner

lucasmazza commented Aug 22, 2014

I think that a generator that at least generates a stub for the controller with some getting started docs could be at least useful, but it's true that updating the routes isn't exactly possible, but a console notice sounds good to me (just like the devise:install generator does).

Owner

josevalim commented Aug 22, 2014

Ok, agreed on all counts. 👍 :D

Contributor

Chun-Yang commented Aug 22, 2014

I have a question on how should we deal with custom params. Here are some thoughts:

  1. we can rewrite *_params methods(e.g. sign_up_params, account_update_params) as mentioned in http://www.jacopretorius.net/2014/03/adding-custom-fields-to-your-devise-user-model-in-rails-4.html .
  2. we can use the methods recommended in the devise guide which add a before action:
  before_action :configure_permitted_parameters, if: :devise_controller?

  protected

  def configure_permitted_parameters
    devise_parameter_sanitizer.for(:sign_up) << :username
  end

What do you think?

Owner

lucasmazza commented Aug 22, 2014

I'm not sure if every generated controller should have the configure_permitted_parameters before_filter by default.

Contributor

Chun-Yang commented Aug 22, 2014

How about we comment them out and add some notes on how to use them?

Owner

lucasmazza commented Aug 22, 2014

I might do. Feel free to open a new PR with your work in progress and we can discuss over it as you go instead of discussing it over here :)

Contributor

Chun-Yang commented Aug 22, 2014

OK : )

Contributor

Chun-Yang commented Aug 23, 2014

Hi, guys. I created a primitive PR. You can check it out at #3169

Contributor

Chun-Yang commented Aug 25, 2014

I have already wrote test on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment