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

Devise ignores (some) validations on signup #4673

Closed
EleanorRagone opened this issue Oct 16, 2017 · 2 comments
Closed

Devise ignores (some) validations on signup #4673

EleanorRagone opened this issue Oct 16, 2017 · 2 comments

Comments

@EleanorRagone
Copy link

tl;dr: During the signup process, I'm guessing only if trackable is turned on, Devise will call a save(validate: false) on the model, causing it to save despite any failed validations, due to: (https://github.com/plataformatec/devise/blob/88724e10adaf9ffd1d8dbfbaadda2b9d40de756a/lib/devise/models/trackable.rb#L33)[https://github.com/plataformatec/devise/blob/88724e10adaf9ffd1d8dbfbaadda2b9d40de756a/lib/devise/models/trackable.rb#L33]

While trying to figure out what change I had made that broke things, I discovered my application was saving with failed validations during the signup process. In my specific case, I have an after_create callback that calls create on an associated model. However, that ends up being invalid, and so does not save (which led my tests to fail). Yet if I tried it through the Devise signup process, everything went through and that associated model saved anyway. I eventually tracked this down to the above referenced line within devise, causing my associated model to save despite being invalid.

FWIW, this doesn't actually affect how my application ends up performing, as I need that model created despite being invalid, so it actually works in my favor (I have an after-signup profile creation process users through, so that's why the validation exists in the first place). I don't know if this is necessarily a bug you want fixed, or just something you want documented.

AshleyFoster added a commit to AshleyFoster/devise that referenced this issue Oct 17, 2017
This commit fixes issue
heartcombo#4673

This removes `validate: false` from saving a record when `Trackable` is
in use.
tegon pushed a commit that referenced this issue Nov 28, 2017
* Fix missing validations on Signup

This commit fixes issue
#4673

This removes `validate: false` from saving a record when `Trackable` is
in use.

* Add test case

* Add mongoid model
@tegon
Copy link
Member

tegon commented Nov 28, 2017

Fixed in #4674

@tegon tegon closed this as completed Nov 28, 2017
tegon added a commit that referenced this issue Mar 5, 2018
In some cases, invalid records could be created during the signup
process because we were calling `save(validate: false)` inside the
`update_tracked_fields!` method. See
#4673 for more
information.
This was fixed on #4674 by
calling `save` directly, but it caused some trouble and confusion since
it changed Devise's behavior significantly.
We talked about on #4790
and it doesn't even make sense to call `save` on a object that isn't
persisted yet, so I've added a guard clause to the
`update_tracked_fields!` method.
tegon added a commit that referenced this issue Mar 5, 2018
In some cases, invalid records could be created during the signup
process because we were calling `save(validate: false)` inside the
`update_tracked_fields!` method. See
#4673 for more
information.
This was fixed on #4674 by
calling `save` directly, but it caused some trouble and confusion since
it changed Devise's behavior significantly.
We talked about on #4790
and it doesn't even make sense to call `save` on an object that isn't
persisted yet, so I've added a guard clause to the
`update_tracked_fields!` method.

Fixes #4790
tegon added a commit that referenced this issue Mar 14, 2018
In some cases, invalid records could be created during the signup
process because we were calling `save(validate: false)` inside the
`update_tracked_fields!` method. See
#4673 for more
information.
This was fixed on #4674 by
calling `save` directly, but it caused some trouble and confusion since
it changed Devise's behavior significantly.
We talked about on #4790
and it doesn't even make sense to call `save` on an object that isn't
persisted yet, so I've added a guard clause to the
`update_tracked_fields!` method.

Fixes #4790
@carlosantoniodasilva
Copy link
Member

Just linking a comment on the final related fix here: #4796 (comment).

I'm not sure I can see how Devise would be trying to save an invalid record that wasn't already saved before.

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

No branches or pull requests

3 participants