Merge pull request #5034 from tgxworld/fix_staged_primary_email

FIX: Staged users are still missing primary email.

app/jobs/onceoff/fix_primary_emails_for_staged_users.rb

@@ -0,0 +1,26 @@
+module Jobs
+  class FixPrimaryEmailsForStagedUsers < Jobs::Onceoff
+    def execute_onceoff
+      User.exec_sql <<~SQL
+      INSERT INTO user_emails (
+        user_id,
+        email,
+        "primary",
+        created_at,
+        updated_at
+      ) SELECT
+        users.id,
+        email_tokens.email,
+        'TRUE',
+        users.created_at,
+        users.updated_at
+      FROM users
+      LEFT JOIN user_emails ON user_emails.user_id = users.id
+      LEFT JOIN email_tokens ON email_tokens.user_id = users.id
+      WHERE staged
+      AND NOT active
+      AND user_emails.id IS NULL
+      SQL
+    end
+  end
+end

app/models/user.rb

@@ -82,10 +82,12 @@ class User < ActiveRecord::Base
   validates :name, user_full_name: true, if: :name_changed?, length: { maximum: 255 }
   validates :ip_address, allowed_ip_address: { on: :create, message: :signup_not_allowed }
   validates :primary_email, presence: true
-  validates_associated :primary_email, if: :should_validate_primary_email?
+  validates_associated :primary_email
 
   after_initialize :add_trust_level
 
+  before_validation :set_should_validate_email
+
   after_create :create_email_token
   after_create :create_user_stat
   after_create :create_user_option
@@ -268,7 +270,7 @@ class User < ActiveRecord::Base
     used_invite.try(:invited_by)
   end
 
-  def should_validate_primary_email?
+  def should_validate_email_address?
     !skip_email_validation && !staged?
   end
 
@@ -938,7 +940,7 @@ class User < ActiveRecord::Base
 
   def email=(email)
     if primary_email
-      self.new_record? ? primary_email.email = email : primary_email.update(email: email)
+      new_record? ? primary_email.email = email : primary_email.update(email: email)
     else
       build_primary_email(email: email)
     end
@@ -1098,6 +1100,14 @@ class User < ActiveRecord::Base
     true
   end
 
+  def set_should_validate_email
+    if self.primary_email
+      self.primary_email.should_validate_email = should_validate_email_address?
+    end
+
+    true
+  end
+
 end
 
 # == Schema Information

app/models/user_email.rb

@@ -3,12 +3,14 @@ require_dependency 'email_validator'
 class UserEmail < ActiveRecord::Base
   belongs_to :user
 
+  attr_accessor :should_validate_email
+
   before_validation :strip_downcase_email
 
   validates :email, presence: true, uniqueness: true
 
   validates :email, email: true, format: { with: EmailValidator.email_regex },
-                    if: :skip_email_validation?
+                    if: :validate_email?
 
   validates :primary, uniqueness: { scope: [:user_id] }
 
@@ -21,8 +23,8 @@ class UserEmail < ActiveRecord::Base
     end
   end
 
-  def skip_email_validation?
-    return true if user && user.skip_email_validation
+  def validate_email?
+    return false unless self.should_validate_email
     email_changed?
   end
 end

db/migrate/20170731030330_fix_primary_emails_for_staged_users.rb

@@ -1,28 +0,0 @@
-class FixPrimaryEmailsForStagedUsers < ActiveRecord::Migration
-  def up
-    execute <<~SQL
-    INSERT INTO user_emails (
-      user_id,
-      email,
-      "primary",
-      created_at,
-      updated_at
-    ) SELECT
-      users.id,
-      email_tokens.email,
-      'TRUE',
-      users.created_at,
-      users.updated_at
-    FROM users
-    LEFT JOIN user_emails ON user_emails.user_id = users.id
-    LEFT JOIN email_tokens ON email_tokens.user_id = users.id
-    WHERE staged
-    AND NOT active
-    AND user_emails.id IS NULL
-    SQL
-  end
-
-  def down
-    raise ActiveRecord::IrreversibleMigration
-  end
-end

spec/controllers/users_controller_spec.rb

@@ -2031,9 +2031,10 @@ describe UsersController do
         user = Fabricate(:inactive_user)
         token = user.email_tokens.first
 
-        xhr :put, :update_activation_email,           username: user.username,
-                                                      password: 'qwerqwer123',
-                                                      email: 'updatedemail@example.com'
+        xhr :put, :update_activation_email,
+          username: user.username,
+          password: 'qwerqwer123',
+          email: 'updatedemail@example.com'
 
         expect(response).to be_success
 

spec/models/user_spec.rb

@@ -28,9 +28,9 @@ describe User do
       end
 
       describe 'when user is staged' do
-        it 'should still validate primary_email' do
+        it 'should still validate presence of primary_email' do
           user.staged = true
-          user.primary_email = nil
+          user.email = nil
 
           expect(user).to_not be_valid
           expect(user.errors.messages).to include(:primary_email)
@@ -623,7 +623,10 @@ describe User do
     it "doesn't validate email address for staged users" do
       SiteSetting.email_domains_whitelist = "foo.com"
       SiteSetting.email_domains_blacklist = "bar.com"
-      expect(Fabricate.build(:user, staged: true, email: "foo@bar.com")).to be_valid
+
+      user = Fabricate.build(:user, staged: true, email: "foo@bar.com")
+
+      expect(user.save).to eq(true)
     end
   end