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

Email reconfirmation #319

Open
edbond opened this issue Jan 25, 2013 · 14 comments
Open

Email reconfirmation #319

edbond opened this issue Jan 25, 2013 · 14 comments
Labels

Comments

@edbond
Copy link

edbond commented Jan 25, 2013

When user changes email in profile, reconfirmation email should be sent.
Devise does have reconfirmation feature.

@atd
Copy link
Contributor

atd commented Jan 28, 2013

A patch is welcomed!

@edbond
Copy link
Author

edbond commented Jan 28, 2013

Here is a quick summary of my monkey-patching:

add email column to User and copy email from Actor (note that unconfirmed_email is already in users):

class AddEmailToUsers < ActiveRecord::Migration
  def up
    add_column :users, :email, :string

    User.reset_column_information

    User.all(include: :actor).each do |u|
      u.email = u.actor.email
      u.unconfirmed_email = nil
      u.skip_confirmation!
      u.skip_reconfirmation!
      u.save!
    end
  end

  def down
    # Copy email to actor
    User.all(include: :actor).each do |u|
      u.actor.update_attributes! email: u.email
    end

    remove_column :users, :email
  end
end

wire profle and user, add to Profile model:

  has_one :user, through: :actor
  accepts_nested_attributes_for :user

change field_for in profile/edit to use user:

  <%= f.fields_for :user do |user_form| %>
      <%= custom_field user_form, :email, label: t('profile.email') %>
  <% end %>

HTH

@atd
Copy link
Contributor

atd commented Jan 30, 2013

Hi @edbond, I cannot see how unconfirmed emails are confirmed again.

Anyway, I believe the right place to make this contribution is Devise itself, don't you think so?

@edbond
Copy link
Author

edbond commented Jan 30, 2013

Devise already have such feature, see http://rubydoc.info/github/plataformatec/devise/master/Devise/Models/Confirmable

  • +reconfirmable+: requires any email changes to be confirmed (exactly the same way as
    initial account confirmation) to be applied. Requires additional unconfirmed_email
    db field to be setup (t.reconfirmable in migrations). Until confirmed new email is
    stored in unconfirmed email column, and copied to email column on successful
    confirmation.

social_stream breaks it.

@atd
Copy link
Contributor

atd commented Jan 30, 2013

Sorry @edbond, I did not notice.

Why replicating the column from actors.email to the users table?

@edbond
Copy link
Author

edbond commented Jan 30, 2013

This was needed for email_changed and devise callbacks work. Maybe it can be achieved without replication.

@atd
Copy link
Contributor

atd commented Jan 30, 2013

It works for me...

atd@seraph:~/dev/social_stream/spec/dummy$ rails c
Loading development environment (Rails 3.2.11)
[1] pry(main)> u = User.first
  User Load (0.2ms)  SELECT "users".* FROM "users" LIMIT 1
  Actor Load (0.6ms)  SELECT "actors".* FROM "actors" WHERE "actors"."id" = 1 LIMIT 1
  ActivityObject Load (0.2ms)  SELECT "activity_objects".* FROM "activity_objects" WHERE "activity_objects"."id" = 1 LIMIT 1
=> #<User id: 1, encrypted_password: "$2a$10$UKK1LcHAoYwf2HVCeXCa3Og2AnbRjaFIZgzTaNrJWc8V...", password_salt: nil, reset_password_token: nil, reset_password_sent_at: nil, remember_created_at: nil, sign_in_count: 0, current_sign_in_at: nil, last_sign_in_at: nil, current_sign_in_ip: nil, last_sign_in_ip: nil, authentication_token: nil, created_at: "2013-01-25 15:01:21", updated_at: "2013-01-25 15:01:21", actor_id: 1, language: nil, connected: false, status: "available", chat_enabled: true>
[2] pry(main)> u.email_changed?
=> false
[3] pry(main)> u.email = "[email protected]"
=> "[email protected]"
[4] pry(main)> u.email_changed?
=> true
[5] pry(main)>

@edbond
Copy link
Author

edbond commented Jan 30, 2013

afaik this call was forwarded to Actor which is not devise model. Try to change email and save, you will not get reconfirmation email.

@atd
Copy link
Contributor

atd commented Jan 30, 2013

Please, provide an application or spec that reproduces it

@atd atd closed this as completed Jan 30, 2013
@edbond
Copy link
Author

edbond commented Jan 30, 2013

Here is a spec which pass after changes:

describe User do
  let(:old_email) { "[email protected]" }
  let(:new_email) { "[email protected]" }

  let(:password) { "Udur9uYo" }

  it "asks to confirm changed email" do
    user = FactoryGirl.build(:user, email: old_email, password: password,
      password_confirmation: password)
    user.skip_confirmation!
    user.save!

    user.valid_password?(password).should be_true

    ActionMailer::Base.deliveries.clear
    user.reload

    user.email = new_email
    user.save!

    ActionMailer::Base.deliveries.should_not be_empty
  end
end

@atd
Copy link
Contributor

atd commented Jan 30, 2013

The spec does not work in current Social Stream's code:

  1) User asks to confirm changed email
     Failure/Error: user = FactoryGirl.build(:user, email: old_email, password: password,
     NameError:
       uninitialized constant FactoryGirl
     # ./base/spec/models/user_email_spec.rb:10:in `block (2 levels) in <top (required)>'

Finished in 0.00169 seconds
1 example, 1 failure

@edbond
Copy link
Author

edbond commented Jan 30, 2013

Nevermind, if someone needs to fix confirmation it will find the code here.

@atd
Copy link
Contributor

atd commented Feb 11, 2013

It seems that the User model is not changed and thus is not updated, so Devise's callback is not triggered.

@atd atd reopened this Feb 11, 2013
@edbond
Copy link
Author

edbond commented Feb 11, 2013

Yes, maybe you can forward email_changed from Actor to User?

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

No branches or pull requests

2 participants