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

Change behavior to skip reconfirmation after creating a record with #save called in callback #4125

Merged
merged 3 commits into from
Jun 13, 2016

Conversation

ArneZsng
Copy link
Contributor

The following commit was breaking stuff on our system for a new email confirmation when the email is changed while the object is still in memory:
1d57169

This is due to skip_reconfirmation! in send_on_create_confirmation_instructions because it sets @bypass_confirmation_postpone to true which lets postpone_email_change? return false.

The case was added to the confirmable test case. Basically, the skipping of the reconfirmation as part of an after_create hook needs to be treated differently from a general bypass. To ensure that this is skipped when save is called in an after_create hook but NOT if the email is changed from an existing email, additional checks were added.

@@ -165,6 +166,12 @@ def skip_reconfirmation!

protected

# To not require reoncfirmation after creating with #save called in a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little typo here reconfirmation

@ulissesalmeida
Copy link
Contributor

Looks good to me. Any consideration @josevalim @lucasmazza

I think it may need a backport to 3.5.x.

@@ -165,6 +166,12 @@ def skip_reconfirmation!

protected

# To not require reconfirmation after creating with #save called in a
# callback call skip_create_confirmation!
def skip_create_confirmation!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ArneZsng @ulissesalmeida I feel like we should have better names to express the checks for dealing with the save + after_create scenarios - skip_create_confirmation sounds like we are skipping the confirmation on a regular create scenario, regardless of the callback issue.

@ArneZsng
Copy link
Contributor Author

@lucasmazza I updated the naming to skip_reconfirmation_in_callback (method and instance var). Would that fit better?

@shaneog shaneog mentioned this pull request May 23, 2016
@ArneZsng
Copy link
Contributor Author

@lucasmazza @ulissesalmeida Looks good to you now?

@ArneZsng
Copy link
Contributor Author

@lucasmazza @ulissesalmeida Anything outstanding from your side?

@ulissesalmeida
Copy link
Contributor

:shipit:

@lucasmazza
Copy link
Contributor

@ArneZsng thanks for the patch, and sorry for the delay 😬

@lucasmazza lucasmazza merged commit ac70284 into heartcombo:master Jun 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants