FEATURE: phase 1 of supporting multiple email addresses

app/controllers/admin/email_controller.rb

@@ -73,7 +73,7 @@ class Admin::EmailController < Admin::AdminController
     params.require(:from)
     params.require(:to)
     # These strings aren't localized; they are sent to an anonymous SMTP user.
-    if !User.exists?(email: Email.downcase(params[:from])) && !SiteSetting.enable_staged_users
+    if !User.with_email(Email.downcase(params[:from])).exists? && !SiteSetting.enable_staged_users
       render json: { reject: true, reason: "Mail from your address is not accepted. Do you have an account here?" }
     elsif Email::Receiver.check_address(Email.downcase(params[:to])).nil?
       render json: { reject: true, reason: "Mail to this address is not accepted. Check the address and try to send again?" }

app/controllers/admin/groups_controller.rb

@@ -29,7 +29,11 @@ class Admin::GroupsController < Admin::AdminController
       users = (params[:users] || []).map {|u| u.downcase}
       valid_emails = {}
       valid_usernames = {}
-      valid_users = User.where("username_lower IN (:users) OR email IN (:users)", users: users).pluck(:id, :username_lower, :email)
+
+      valid_users = User.joins(:user_emails)
+        .where("username_lower IN (:users) OR user_emails.email IN (:users)", users: users)
+        .pluck(:id, :username_lower, :"user_emails.email")
+
       valid_users.map! do |id, username_lower, email|
         valid_emails[email] = valid_usernames[username_lower] = id
         id

app/controllers/finish_installation_controller.rb

@@ -15,7 +15,7 @@ class FinishInstallationController < ApplicationController
       email = params[:email].strip
       raise Discourse::InvalidParameters.new unless @allowed_emails.include?(email)
 
-      return redirect_confirm(email) if User.where(email: email).exists?
+      return redirect_confirm(email) if UserEmail.exists?(email: email)
 
       @user.email = email
       @user.username = params[:username]
@@ -37,7 +37,7 @@ class FinishInstallationController < ApplicationController
 
   def resend_email
     @email = session[:registered_email]
-    @user = User.where(email: @email).first
+    @user = User.find_by_email(@email)
     if @user.present?
       @email_token = @user.email_tokens.unconfirmed.active.first
       if @email_token.present?

app/controllers/groups_controller.rb

@@ -153,7 +153,7 @@ class GroupsController < ApplicationController
       elsif params[:user_ids].present?
         User.find(params[:user_ids].split(","))
       elsif params[:user_emails].present?
-        User.where(email: params[:user_emails].split(","))
+        User.with_email(params[:user_emails].split(","))
       else
         raise Discourse::InvalidParameters.new(
           'user_ids or usernames or user_emails must be present'

app/controllers/users_controller.rb

@@ -311,7 +311,7 @@ class UsersController < ApplicationController
       return fail_with("login.reserved_username")
     end
 
-    if user = User.find_by(staged: true, email: params[:email].strip.downcase)
+    if user = User.where(staged: true).with_email(params[:email].strip.downcase).first
       user_params.each { |k, v| user.send("#{k}=", v) }
       user.staged = false
     else
@@ -490,7 +490,7 @@ class UsersController < ApplicationController
       RateLimiter.new(nil, "admin-login-hr-#{request.remote_ip}", 6, 1.hour).performed!
       RateLimiter.new(nil, "admin-login-min-#{request.remote_ip}", 3, 1.minute).performed!
 
-      user = User.where(email: params[:email], admin: true).human_users.first
+      user = User.with_email(params[:email]).where(admin: true).human_users.first
       if user
         email_token = user.email_tokens.create(email: user.email)
         Jobs.enqueue(:critical_user_email, type: :admin_login, user_id: user.id, email_token: email_token.token)
@@ -606,6 +606,7 @@ class UsersController < ApplicationController
 
     User.transaction do
       @user.email = params[:email]
+      
       if @user.save
         @user.email_tokens.create(email: @user.email)
         enqueue_activation_email

app/jobs/regular/automatic_group_membership.rb

@@ -13,7 +13,8 @@ module Jobs
 
       domains = group.automatic_membership_email_domains.gsub('.', '\.')
 
-      User.where("email ~* '@(#{domains})$'")
+      User.joins(:user_emails)
+          .where("user_emails.email ~* '@(#{domains})$'")
           .where("users.id NOT IN (SELECT user_id FROM group_users WHERE group_users.group_id = ?)", group_id)
           .activated
           .where(staged: false)

app/models/email_token.rb

@@ -71,7 +71,7 @@ class EmailToken < ActiveRecord::Base
       end
 
       if user
-        return User.find_by(email: Email.downcase(user.email)) if Invite.redeem_from_email(user.email).present?
+        return User.find_by_email(user.email) if Invite.redeem_from_email(user.email).present?
         user
       end
     end

app/models/group.rb

@@ -595,7 +595,7 @@ end
 #  allow_membership_requests          :boolean          default(FALSE), not null
 #  full_name                          :string
 #  default_notification_level         :integer          default(3), not null
-#  visibility_level                   :integer          default(0)
+#  visibility_level                   :integer          default(0), not null
 #
 # Indexes
 #

app/models/invite.rb

@@ -32,8 +32,9 @@ class Invite < ActiveRecord::Base
   def user_doesnt_already_exist
     @email_already_exists = false
     return if email.blank?
-    u = User.find_by("email = ?", Email.downcase(email))
+    user = User.find_by_email(email)
-    if u && u.id != self.user_id
+
+    if user && user.id != self.user_id
       @email_already_exists = true
       errors.add(:email)
     end
@@ -98,11 +99,9 @@ class Invite < ActiveRecord::Base
     group_ids = opts[:group_ids]
     send_email = opts[:send_email].nil? ? true : opts[:send_email]
     custom_message = opts[:custom_message]
-
     lower_email = Email.downcase(email)
-    user = User.find_by(email: lower_email)
 
-    if user
+    if user = User.find_by_email(lower_email)
       extend_permissions(topic, user, invited_by) if topic
       raise UserExists.new I18n.t("invite.user_exists", email: lower_email, username: user.username)
     end

app/models/invite_redeemer.rb

@@ -91,7 +91,7 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f
   end
 
   def get_existing_user
-    User.where(admin: false).find_by(email: invite.email)
+    User.where(admin: false).find_by_email(invite.email)
   end
 
 

app/models/search_log.rb

@@ -55,3 +55,16 @@ class SearchLog < ActiveRecord::Base
     end
   end
 end
+
+# == Schema Information
+#
+# Table name: search_logs
+#
+#  id               :integer          not null, primary key
+#  term             :string           not null
+#  user_id          :integer
+#  ip_address       :inet             not null
+#  clicked_topic_id :integer
+#  search_type      :integer          not null
+#  created_at       :datetime         not null
+#

app/models/topic.rb

@@ -810,7 +810,7 @@ SQL
   end
 
   def grant_permission_to_user(lower_email)
-    user = User.find_by(email: lower_email)
+    user = User.find_by_email(lower_email)
     topic_allowed_users.create!(user_id: user.id)
   end
 

app/models/user.rb

@@ -42,7 +42,11 @@ class User < ActiveRecord::Base
   has_many :email_change_requests, dependent: :destroy
   has_many :directory_items, dependent: :delete_all
   has_many :user_auth_tokens, dependent: :destroy
+  has_many :user_emails, dependent: :destroy
 
+  has_one :primary_email, -> { where(primary: true)  },
+    dependent: :destroy,
+    class_name: 'UserEmail'
 
   has_one :user_option, dependent: :destroy
   has_one :user_avatar, dependent: :destroy
@@ -74,16 +78,13 @@ class User < ActiveRecord::Base
 
   delegate :last_sent_email_address, :to => :email_logs
 
-  before_validation :strip_downcase_email
-
   validates_presence_of :username
   validate :username_validator, if: :username_changed?
-  validates :email, presence: true, uniqueness: true
-  validates :email, format: { with: EmailValidator.email_regex }, if: :email_changed?
-  validates :email, email: true, if: :should_validate_email?
   validate :password_validator
   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, if: :should_validate_primary_email?
+  validates_associated :primary_email, if: :should_validate_primary_email?
 
   after_initialize :add_trust_level
 
@@ -123,6 +124,8 @@ class User < ActiveRecord::Base
   # set to true to optimize creation and save for imports
   attr_accessor :import_mode
 
+  scope :with_email, ->(email) { joins(:user_emails).where(user_emails: { email: email }) }
+
   scope :human_users, -> { where('users.id > 0') }
 
   # excluding fake users like the system user or anonymous users
@@ -232,7 +235,7 @@ class User < ActiveRecord::Base
   end
 
   def self.find_by_email(email)
-    find_by(email: Email.downcase(email))
+    self.with_email(Email.downcase(email)).first
   end
 
   def self.find_by_username(username)
@@ -267,8 +270,8 @@ class User < ActiveRecord::Base
     used_invite.try(:invited_by)
   end
 
-  def should_validate_email?
+  def should_validate_primary_email?
-    return !skip_email_validation && !staged? && email_changed?
+    !skip_email_validation && !staged?
   end
 
   # Approve this user
@@ -935,6 +938,18 @@ class User < ActiveRecord::Base
     end
   end
 
+  def email
+    primary_email.email
+  end
+
+  def email=(email)
+    if primary_email
+      self.new_record? ? primary_email.email = email : primary_email.update(email: email)
+    else
+      build_primary_email(email: email)
+    end
+  end
+
   protected
 
   def badge_grant
@@ -1011,13 +1026,6 @@ class User < ActiveRecord::Base
     self.username_lower = username.downcase
   end
 
-  def strip_downcase_email
-    if self.email
-      self.email = self.email.strip
-      self.email = self.email.downcase
-    end
-  end
-
   def username_validator
     username_format_validator || begin
       lower = username.downcase
@@ -1104,7 +1112,7 @@ end
 #  name                    :string
 #  seen_notification_id    :integer          default(0), not null
 #  last_posted_at          :datetime
-#  email                   :string(513)      not null
+#  email                   :string(513)
 #  password_hash           :string(64)
 #  salt                    :string(32)
 #  active                  :boolean          default(FALSE), not null

app/models/user_email.rb

@@ -0,0 +1,45 @@
+require_dependency 'email_validator'
+
+class UserEmail < ActiveRecord::Base
+  belongs_to :user
+
+  before_validation :strip_downcase_email
+
+  validates :email, presence: true, uniqueness: true
+
+  validates :email, email: true, format: { with: EmailValidator.email_regex },
+    if: :skip_email_validation?
+
+  validates :primary, uniqueness: { scope: [:user_id] }
+
+  private
+
+  def strip_downcase_email
+    if self.email
+      self.email = self.email.strip
+      self.email = self.email.downcase
+    end
+  end
+
+  def skip_email_validation?
+    return true if user && user.skip_email_validation
+    email_changed?
+  end
+end
+
+# == Schema Information
+#
+# Table name: user_emails
+#
+#  id         :integer          not null, primary key
+#  user_id    :integer          not null
+#  email      :string(513)      not null
+#  primary    :boolean          default(FALSE), not null
+#  created_at :datetime
+#  updated_at :datetime
+#
+# Indexes
+#
+#  index_user_emails_on_user_id              (user_id)
+#  index_user_emails_on_user_id_and_primary  (user_id,primary) UNIQUE
+#

db/fixtures/009_users.rb

@@ -6,12 +6,18 @@ if user
   user.save
 end
 
+UserEmail.seed do |ue|
+  ue.id = -1
+  ue.email = "no_email"
+  ue.primary = true
+  ue.user_id = -1
+end
+
 User.seed do |u|
   u.id = -1
   u.name = "system"
   u.username = "system"
   u.username_lower = "system"
-  u.email = "no_email"
   u.password = SecureRandom.hex
   u.active = true
   u.admin = true
@@ -56,12 +62,18 @@ ColumnDropper.drop(
 
 # User for the smoke tests
 if ENV["SMOKE"] == "1"
+  UserEmail.seed do |ue|
+    ue.id = 0
+    ue.email = "smoke_user@discourse.org"
+    ue.primary = true
+    ue.user_id = 0
+  end
+
   smoke_user = User.seed do |u|
     u.id = 0
     u.name = "smoke_user"
     u.username = "smoke_user"
     u.username_lower = "smoke_user"
-    u.email = "smoke_user@discourse.org"
     u.password = "P4ssw0rd"
     u.active = true
     u.approved = true
@@ -77,4 +89,3 @@ if ENV["SMOKE"] == "1"
 
   EmailToken.where(user_id: smoke_user.id).update_all(confirmed: true)
 end
-

db/migrate/20170717084947_create_user_emails.rb

@@ -0,0 +1,42 @@
+require_dependency 'column_dropper'
+
+class CreateUserEmails < ActiveRecord::Migration
+  def up
+    create_table :user_emails do |t|
+      t.integer :user_id, null: false
+      t.string :email, limit: 513, null: false
+      t.boolean :primary, default: false, null: false
+      t.timestamps
+    end
+
+    add_index :user_emails, :user_id
+    add_index :user_emails, [:user_id, :primary], unique: true
+
+    execute "CREATE UNIQUE INDEX index_user_emails_on_email ON user_emails (lower(email));"
+
+    execute <<~SQL
+    INSERT INTO user_emails (
+      id,
+      user_id,
+      email,
+      "primary",
+      created_at,
+      updated_at
+    ) SELECT
+      id,
+      id,
+      email,
+      'TRUE',
+      created_at,
+      updated_at
+    FROM users
+    SQL
+
+    change_column_null :users, :email, true
+    ColumnDropper.mark_readonly(:users, :email)
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/admin_user_index_query.rb

@@ -4,7 +4,7 @@ class AdminUserIndexQuery
 
   def initialize(params = {}, klass = User, trust_levels = TrustLevel.levels)
     @params = params
-    @query = initialize_query_with_order(klass)
+    @query = initialize_query_with_order(klass).joins(:user_emails)
     @trust_levels = trust_levels
   end
 
@@ -52,7 +52,7 @@ class AdminUserIndexQuery
 
     if !custom_order.present?
       if params[:query] == "active"
-        order << "COALESCE(last_seen_at, to_date('1970-01-01', 'YYYY-MM-DD')) DESC"
+        order << "COALESCE(users.last_seen_at, to_date('1970-01-01', 'YYYY-MM-DD')) DESC"
       else
         order << "users.created_at DESC"
       end
@@ -106,7 +106,7 @@ class AdminUserIndexQuery
       if ip = IPAddr.new(params[:filter]) rescue nil
         @query.where('ip_address <<= :ip OR registration_ip_address <<= :ip', ip: ip.to_cidr_s)
       else
-        @query.where('username_lower ILIKE :filter OR email ILIKE :filter', filter: "%#{params[:filter]}%")
+        @query.where('username_lower ILIKE :filter OR user_emails.email ILIKE :filter', filter: "%#{params[:filter]}%")
       end
     end
   end
@@ -119,7 +119,7 @@ class AdminUserIndexQuery
 
   def filter_exclude
     if params[:exclude].present?
-      @query.where('id != ?', params[:exclude])
+      @query.where('users.id != ?', params[:exclude])
     end
   end
 

lib/avatar_lookup.rb

@@ -12,7 +12,7 @@ class AvatarLookup
   private
 
   def self.lookup_columns
-    @lookup_columns ||= %i{id email username uploaded_avatar_id}
+    @lookup_columns ||= %i{id user_emails.email username uploaded_avatar_id}
   end
 
   def users
@@ -22,7 +22,8 @@ class AvatarLookup
   def user_lookup_hash
     # adding tap here is a personal taste thing
     hash = {}
-    User.where(:id => @user_ids)
+    User.joins(:user_emails)
+        .where(id: @user_ids)
         .select(AvatarLookup.lookup_columns)
         .each{ |user| hash[user.id] = user }
     hash

lib/backup_restore/restorer.rb

@@ -400,7 +400,7 @@ module BackupRestore
     end
 
     def notify_user
-      if user = User.find_by(email: @user_info[:email])
+      if user = User.find_by_email(@user_info[:email])
         log "Notifying '#{user.username}' of the end of the restore..."
         status = @success ? :restore_succeeded : :restore_failed
 

lib/column_dropper.rb

@@ -8,7 +8,7 @@ class ColumnDropper
 
     delay ||= Rails.env.production? ? 60 : 0
 
-    sql = <<SQL
+    sql = <<~SQL
     SELECT 1
     FROM INFORMATION_SCHEMA.COLUMNS
     WHERE table_schema = 'public' AND
@@ -32,7 +32,41 @@ SQL
       columns.each do |column|
         # safe cause it is protected on method entry, can not be passed in params
         ActiveRecord::Base.exec_sql("ALTER TABLE #{table} DROP COLUMN IF EXISTS #{column}")
+
+        ActiveRecord::Base.exec_sql <<~SQL
+        DROP FUNCTION IF EXISTS #{readonly_function_name(table, column)};
+        DROP TRIGGER IF EXISTS #{readonly_trigger_name(table, column)} ON #{table};
+        SQL
       end
     end
   end
+
+  def self.mark_readonly(table_name, column_name)
+    ActiveRecord::Base.exec_sql <<-SQL
+    CREATE OR REPLACE FUNCTION #{readonly_function_name(table_name, column_name)} RETURNS trigger AS $rcr$
+      BEGIN
+        RAISE EXCEPTION 'Discourse: #{column_name} in #{table_name} is readonly';
+      END
+    $rcr$ LANGUAGE plpgsql;
+    SQL
+
+    ActiveRecord::Base.exec_sql <<-SQL
+    CREATE TRIGGER #{readonly_trigger_name(table_name, column_name)}
+    BEFORE INSERT OR UPDATE OF #{column_name}
+    ON #{table_name}
+    FOR EACH ROW
+    WHEN (NEW.#{column_name} IS NOT NULL)
+    EXECUTE PROCEDURE #{readonly_function_name(table_name, column_name)};
+    SQL
+  end
+
+  private
+
+  def self.readonly_function_name(table_name, column_name)
+    "raise_#{table_name}_#{column_name}_readonly()"
+  end
+
+  def self.readonly_trigger_name(table_name, column_name)
+    "#{table_name}_#{column_name}_readonly"
+  end
 end

lib/email/receiver.rb

@@ -153,7 +153,7 @@ module Email
       if $redis.setnx(key, "1")
         $redis.expire(key, 25.hours)
 
-        if user = User.find_by(email: email)
+        if user = User.find_by_email(email)
           user.user_stat.bounce_score += score
           user.user_stat.reset_bounce_score_after = SiteSetting.reset_bounce_score_after_days.days.from_now
           user.user_stat.save

lib/email_updater.rb

@@ -81,7 +81,7 @@ class EmailUpdater
           confirm_result = :authorizing_new
         when EmailChangeRequest.states[:authorizing_new]
           change_req.update_column(:change_state, EmailChangeRequest.states[:complete])
-          user.update_column(:email, token.email)
+          user.primary_email.update_column(:email, token.email)
           confirm_result = :complete
         end
       else

lib/import_export/topic_importer.rb

@@ -18,7 +18,7 @@ module ImportExport
 
     def import_users
       @export_data[:users].each do |u|
-        existing = User.where(email: u[:email]).first
+        existing = User.with_email(email: u[:email]).first
         if existing
           if existing.custom_fields["import_id"] != u[:id]
             existing.custom_fields["import_id"] = u[:id]

plugins/discourse-narrative-bot/db/fixtures/001_discobot.rb

@@ -4,12 +4,18 @@ user = User.find_by(id: -2)
 if !user
   suggested_username = UserNameSuggester.suggest(discobot_username)
 
+  UserEmail.seed do |ue|
+    ue.id = -2
+    ue.email = "discobot_email"
+    ue.primary = true
+    ue.user_id = -2
+  end
+
   User.seed do |u|
     u.id = -2
     u.name = discobot_username
     u.username = suggested_username
     u.username_lower = suggested_username.downcase
-    u.email = "discobot_email"
     u.password = SecureRandom.hex
     u.active = true
     u.approved = true

script/bulk_import/base.rb

@@ -93,7 +93,7 @@ class BulkImport::Base
 
     puts "Loading users indexes..."
     @last_user_id = User.unscoped.maximum(:id)
-    @emails = User.unscoped.pluck(:email).to_set
+    @emails = User.unscoped.pluck(:"user_emails.email").to_set
     @usernames_lower = User.unscoped.pluck(:username_lower).to_set
     @mapped_usernames = UserCustomField.joins(:user).where(name: "import_username").pluck("user_custom_fields.value", "users.username").to_h
 

script/import_scripts/base.rb

@@ -324,7 +324,7 @@ class ImportScripts::Base
     rescue => e
       # try based on email
       if e.try(:record).try(:errors).try(:messages).try(:[], :email).present?
-        if existing = User.find_by(email: opts[:email].downcase)
+        if existing = User.find_by_email(opts[:email].downcase)
           existing.custom_fields["import_id"] = import_id
           existing.save!
           u = existing

spec/components/column_dropper_spec.rb

@@ -1,7 +1,7 @@
 require 'rails_helper'
 require 'column_dropper'
 
-describe ColumnDropper do
+RSpec.describe ColumnDropper do
 
   def has_column?(table, column)
     Topic.exec_sql("SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS
@@ -48,5 +48,78 @@ describe ColumnDropper do
     expect(dropped_proc_called).to eq(true)
 
   end
+
+  describe '.mark_readonly' do
+    let(:table_name) { "table_with_readonly_column" }
+
+    before do
+      ActiveRecord::Base.exec_sql <<~SQL
+      CREATE TABLE #{table_name} (topic_id INTEGER, email TEXT);
+
+      INSERT INTO #{table_name} (topic_id, email)
+      VALUES (1, 'something@email.com');
+      SQL
+
+      described_class.mark_readonly(table_name, 'email')
     end
 
+    after do
+      ActiveRecord::Base.exec_sql <<~SQL
+      DROP TABLE IF EXISTS #{table_name};
+      DROP TRIGGER IF EXISTS #{table_name}_email_readonly ON #{table_name};
+      SQL
+    end
+
+    it 'should prevent updates to the readonly column' do
+      expect do
+        ActiveRecord::Base.exec_sql <<~SQL
+        UPDATE #{table_name}
+        SET email = 'testing@email.com'
+        WHERE topic_id = 1;
+        SQL
+      end.to raise_error(
+        PG::RaiseException,
+        /Discourse: email in #{table_name} is readonly/
+      )
+
+      ActiveRecord::Base.exec_sql("ROLLBACK")
+    end
+
+    it 'should allow updates to the other columns' do
+      ActiveRecord::Base.exec_sql <<~SQL
+      UPDATE #{table_name}
+      SET topic_id = 2
+      WHERE topic_id = 1
+      SQL
+
+      expect(
+        ActiveRecord::Base.exec_sql("SELECT * FROM #{table_name};").values
+      ).to include(["2", "something@email.com"])
+    end
+
+    it 'should prevent insertions to the readonly column' do
+      expect do
+        ActiveRecord::Base.exec_sql <<~SQL
+        INSERT INTO #{table_name} (topic_id, email)
+        VALUES (2, 'something@email.com');
+        SQL
+      end.to raise_error(
+        PG::RaiseException,
+        /Discourse: email in table_with_readonly_column is readonly/
+      )
+
+      ActiveRecord::Base.exec_sql("ROLLBACK")
+    end
+
+    it 'should allow insertions to the other columns' do
+      ActiveRecord::Base.exec_sql <<~SQL
+      INSERT INTO #{table_name} (topic_id)
+      VALUES (2);
+      SQL
+
+      expect(
+        ActiveRecord::Base.exec_sql("SELECT * FROM #{table_name} WHERE topic_id = 2;").values
+      ).to include(["2", nil])
+    end
+  end
+end

spec/components/email/receiver_spec.rb

@@ -381,7 +381,7 @@ describe Email::Receiver do
 
     it "invites everyone in the chain but emails configured as 'incoming' (via reply, group or category)" do
       expect { process(:cc) }.to change(Topic, :count)
-      emails = Topic.last.allowed_users.pluck(:email)
+      emails = Topic.last.allowed_users.joins(:user_emails).pluck(:"user_emails.email")
       expect(emails.size).to eq(3)
       expect(emails).to include("someone@else.com", "discourse@bar.com", "wat@bar.com")
     end

spec/controllers/admin/users_controller_spec.rb

@@ -154,6 +154,10 @@ describe Admin::UsersController do
         @another_user = Fabricate(:coding_horror)
       end
 
+      after do
+        $redis.flushall
+      end
+
       it "raises an error when the user doesn't have permission" do
         Guardian.any_instance.expects(:can_grant_admin?).with(@another_user).returns(false)
         xhr :put, :grant_admin, user_id: @another_user.id
@@ -512,7 +516,7 @@ describe Admin::UsersController do
         xhr :post, :invite_admin, name: 'Bill', username: 'bill22', email: 'bill@bill.com'
         expect(response).to be_success
 
-        u = User.find_by(email: 'bill@bill.com')
+        u = User.find_by_email('bill@bill.com')
         expect(u.name).to eq("Bill")
         expect(u.username).to eq("bill22")
         expect(u.admin).to eq(true)
@@ -587,7 +591,7 @@ describe Admin::UsersController do
       xhr :post, :sync_sso, Rack::Utils.parse_query(sso.payload)
       expect(response).to be_success
 
-      user = User.where(email: 'dr@claw.com').first
+      user = User.find_by_email('dr@claw.com')
       expect(user).to be_present
       expect(user.ip_address).to be_blank
     end
@@ -599,7 +603,7 @@ describe Admin::UsersController do
 
       xhr :post, :sync_sso, Rack::Utils.parse_query(sso.payload)
       expect(response.status).to eq(403)
-      expect(JSON.parse(response.body)["message"]).to include("Email can't be blank")
+      expect(JSON.parse(response.body)["message"]).to include("Primary email is invalid")
     end
   end
 end

spec/controllers/users_controller_spec.rb

@@ -822,7 +822,7 @@ describe UsersController do
         it "should succeed without the optional field" do
           xhr :post, :create, create_params
           expect(response).to be_success
-          inserted = User.where(email: @user.email).first
+          inserted = User.find_by_email(@user.email)
           expect(inserted).to be_present
           expect(inserted.custom_fields).to be_present
           expect(inserted.custom_fields["user_field_#{user_field.id}"]).to eq('value1')
@@ -834,7 +834,7 @@ describe UsersController do
           create_params[:user_fields][optional_field.id.to_s] = 'value3'
           xhr :post, :create, create_params.merge(create_params)
           expect(response).to be_success
-          inserted = User.where(email: @user.email).first
+          inserted = User.find_by_email(@user.email)
           expect(inserted).to be_present
           expect(inserted.custom_fields).to be_present
           expect(inserted.custom_fields["user_field_#{user_field.id}"]).to eq('value1')
@@ -846,7 +846,7 @@ describe UsersController do
           create_params[:user_fields][optional_field.id.to_s] = ('x' * 3000)
           xhr :post, :create, create_params.merge(create_params)
           expect(response).to be_success
-          inserted = User.where(email: @user.email).first
+          inserted = User.find_by_email(@user.email)
 
           val = inserted.custom_fields["user_field_#{optional_field.id}"]
           expect(val.length).to eq(UserField.max_length)
@@ -868,7 +868,7 @@ describe UsersController do
         it "should succeed" do
           xhr :post, :create, create_params
           expect(response).to be_success
-          inserted = User.where(email: @user.email).first
+          inserted = User.find_by_email(@user.email)
           expect(inserted).to be_present
           expect(inserted.custom_fields).not_to be_present
           expect(inserted.custom_fields["user_field_#{user_field.id}"]).to be_blank
@@ -883,7 +883,7 @@ describe UsersController do
         xhr :post, :create, email: staged.email, username: "zogstrip", password: "P4ssw0rd$$"
         result = ::JSON.parse(response.body)
         expect(result["success"]).to eq(true)
-        expect(User.find_by(email: staged.email).staged).to eq(false)
+        expect(User.find_by_email(staged.email).staged).to eq(false)
       end
     end
 
@@ -2033,6 +2033,7 @@ describe UsersController do
           password: 'qwerqwer123',
           email: user.email
         }
+
         expect(response).to_not be_success
       end
 

spec/fabricators/user_email_fabricator.rb

@@ -0,0 +1,4 @@
+Fabricator(:user_email) do
+  email { sequence(:email) { |i| "bruce#{i}@wayne.com" } }
+  primary true
+end

spec/integration/groups_spec.rb

@@ -315,6 +315,13 @@ describe "Groups" do
 
           expect(response).to be_success
         end
+
+        it "adds by email" do
+          expect { xhr :put, "/groups/#{group.id}/members", user_emails: [user1.email, user2.email].join(",") }
+            .to change { group.users.count }.by(2)
+
+          expect(response).to be_success
+        end
       end
 
       it "returns 422 if member already exists" do

spec/models/discourse_single_sign_on_spec.rb

@@ -271,7 +271,7 @@ describe DiscourseSingleSignOn do
       user = sso.lookup_or_create_user(ip_address)
       expect(user.active).to eq(true)
 
-      user.update_columns(email: 'xXx@themovie.com')
+      user.primary_email.update_columns(email: 'xXx@themovie.com')
 
       user = sso.lookup_or_create_user(ip_address)
       expect(user.email).to eq(old_email)

spec/models/user_spec.rb

@@ -9,11 +9,10 @@ describe User do
     describe 'emails' do
       let(:user) { Fabricate.build(:user) }
 
-      it { is_expected.to validate_presence_of :email }
-
       describe 'when record has a valid email' do
         it "should be valid" do
           user.email = 'test@gmail.com'
+
           expect(user).to be_valid
         end
       end
@@ -21,7 +20,9 @@ describe User do
       describe 'when record has an invalid email' do
         it 'should not be valid' do
           user.email = 'test@gmailcom'
+
           expect(user).to_not be_valid
+          expect(user.errors.messages).to include(:primary_email)
         end
       end
     end

spec/services/badge_granter_spec.rb

@@ -38,8 +38,10 @@ describe BadgeGranter do
   describe 'preview' do
     it 'can correctly preview' do
       Fabricate(:user, email: 'sam@gmail.com')
-      result = BadgeGranter.preview('select id user_id, null post_id, created_at granted_at from users
+      result = BadgeGranter.preview('select u.id user_id, null post_id, u.created_at granted_at from users u
-                                     where email like \'%gmail.com\'', explain: true)
+                                     join user_emails ue on ue.user_id = u.id AND ue.primary
+                                     where ue.email like \'%gmail.com\'', explain: true)
+
       expect(result[:grant_count]).to eq(1)
       expect(result[:query_plan]).to be_present
     end