search

saxifrage / cityasacampus

An open-source platform for connecting and showcasing resources within local learning communities.
http://cityasacampus.org/
5 stars 4 forks source link

User registration #365

Closed dmtroyer closed 8 years ago

dmtroyer commented 8 years ago

Let the user registration begin!

MatthewVita commented 8 years ago

issue in milestone is here: https://github.com/saxifrage/cityasacampus/issues/368

dmtroyer commented 8 years ago

I'm running into an issue with permitting additional user fields at user registration. It seems similar to https://github.com/lynndylanhurley/devise_token_auth/issues/432

dmtroyer commented 8 years ago

When I add one field at a time to the devise_parameter_sanitizer it seems to work

devise_parameter_sanitizer.for(:sign_up) << :name

but when I try to pass it a block it doesn't permit the additional fields

devise_parameter_sanitizer.for(:sign_up) { |u| u.require(:name, :email, :password, :password_confirmation) }

I'm guessing it has something to do with the devise_token_auth controller. Will dig :hammer:

dmtroyer commented 8 years ago

Ok I think I realize what is going on.

The way devise_token_auth calls devise_parameter_sanitizer.for(:sign_up) will never hit the block that is used to register permitted parameters

https://github.com/lynndylanhurley/devise_token_auth/blob/master/app/controllers/devise_token_auth/registrations_controller.rb#L99-L101

def sign_up_params
  params.permit(devise_parameter_sanitizer.for(:sign_up))
end
dmtroyer commented 8 years ago

devise_token_auth should be using the devise_parameter_sanitizer.sanitize(:sign_up) method in RegistrationsController.sign_up_params instead of the for method

dmtroyer commented 8 years ago

but Devise::ParameterSanitizer.resource_name is :api_user instead of :registrations

dmtroyer commented 8 years ago

The issue on devise_token_auth where I go into more detail https://github.com/lynndylanhurley/devise_token_auth/issues/464

chadwhitacre commented 8 years ago

!m @dmtroyer

dmtroyer commented 8 years ago

Gee beez it this is up for review. I could have probably done more to directiveify the Organizer Registration view but I needed to draw the line somewhere and move on.

MatthewVita commented 8 years ago

@dmtroyer the code looks excellent!

Comments:

dmtroyer commented 8 years ago

I just amended your commit correcting a few areas not using string-based injection (please read "A Note on Minification" https://docs.angularjs.org/tutorial/step_05

Ah hah! Thanks for the link, I wondered why we were doing that :wink:

Your paths to the SVGs were incorrect so I fixed them :+1:

Shanks!