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

User registration #365

Merged
merged 1 commit into from
Dec 21, 2015
Merged

User registration #365

merged 1 commit into from
Dec 21, 2015

Conversation

dmtroyer
Copy link
Contributor

@dmtroyer dmtroyer commented Dec 5, 2015

Let the user registration begin!

@dmtroyer dmtroyer self-assigned this Dec 5, 2015
@MatthewVita
Copy link
Contributor

issue in milestone is here: #368

@dmtroyer dmtroyer mentioned this pull request Dec 10, 2015
@dmtroyer
Copy link
Contributor Author

I'm running into an issue with permitting additional user fields at user registration. It seems similar to lynndylanhurley/devise_token_auth#432

@dmtroyer
Copy link
Contributor Author

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 🔨

@dmtroyer
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@dmtroyer
Copy link
Contributor Author

The issue on devise_token_auth where I go into more detail lynndylanhurley/devise_token_auth#464

@chadwhitacre
Copy link
Contributor

!m @dmtroyer

@dmtroyer
Copy link
Contributor Author

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.

@dmtroyer dmtroyer assigned MatthewVita and unassigned dmtroyer Dec 18, 2015
@dmtroyer dmtroyer added this to the Admin Dashboard Epic milestone Dec 18, 2015
@MatthewVita
Copy link
Contributor

@dmtroyer the code looks excellent!

Comments:

  • 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
  • Your paths to the SVGs were incorrect so I fixed them 👍

MatthewVita added a commit that referenced this pull request Dec 21, 2015
@MatthewVita MatthewVita merged commit 8c6e588 into master Dec 21, 2015
@dmtroyer dmtroyer deleted the user-registration branch December 21, 2015 02:54
@dmtroyer
Copy link
Contributor Author

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 😉

Your paths to the SVGs were incorrect so I fixed them 👍

Shanks!

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

Successfully merging this pull request may close these issues.

3 participants