Send email to current email address on request to update email

[sonalkr132]

This would ensure unintended email address update don't go unnoticed.

[colby-swandale]

is there some missing context around this test? Is this testing that an email cannot be updated again unless the first email update is verified?

[sonalkr132]

so, when user updates his email address we don't update email column until the user clicks on confirmation link sent to the new "unconfirmed_email. I have updated this to should "not update the current email" do`. Let me know if you have a better idea.

[colby-swandale]

The idea is great, but it seems like this test is depending on state being set by the test above. We want to avoid that as much as possible and set the correct state inside the current test.

We should also wrap a context block of when the user has updated but yet verified the email or something to help improve readability.

[sonalkr132]

this test is depending on state being set by the test above

I disagree. should not update email only depends on setup and put :update action, not any other tests. Please explain why do feel this way and we can fix it.
IMHO, testing big picture things like how is this related to the user not clicking on confirmation mail after updating email goes in integration tests (which we have added).

I have wrapped this in context "yet to verify the updated email" do since you suggested this improved reliability.

[colby-swandale]

Ok I think I understand what's happening now, the user email is not verified by default in the User model. Can we add email_confirmed: false into the create stub so just make it more easier to understand?

👍 for everything else

[sonalkr132]

the user email is not verified by default in the User model.

I think you are talking about initial state of user creation[1]. This test is for context "updating email" (equivalent integration test would be updating email from profile edit form). When the user updates his email, we set unconfirmed_email column using this hook and not update email column right away. I am adding a test for that. The existing email (one before update) stays confirmed (email_confirmed does not change).

[1] email_confirmed is not relevant for this test but just so you know although the user model has email_confirmed set to false by default, in factory we use email_confirmed: true.

[colby-swandale]

Ok, please just ignore me and merge this, I'm not following but thanks for your efforts.

[sonalkr132]

Allow me to explain one more time. Following are two processes related to email confirmation:

- user signs up
    - set `email`, confirmation_token, expiry and email_confirmed (value false)
    - when the user clicks on confirmation URL update `email_confirmed` to true
- user updates email from the profile edit page
    - set `unconfirmed_email`, confirmation_token, expiry 
    - when the user clicks on confirmation URL update `email` with `unconfirmed_email`

Two processes are not the same (or using same columns) as in sign up we don't allow users to sign in unless email_confirmed is true. In the email update process, users can sign in using old email (which was already confirmed) until they verify their new email.
Feel free to message on slack if you have any questions or clarification.