diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 36548585f3b..881fb3e9eca 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -113,8 +113,7 @@ class Users::OmniauthCallbacksController < ApplicationController # automatically activate/unstage any account if a provider marked the email valid if @auth_result.email_valid && @auth_result.email == user.email - user.unstage - user.save + user.unstage! if !user.active || !user.email_confirmed? user.update!(password: SecureRandom.hex) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index d3289b9d672..fde828aee6f 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -436,8 +436,16 @@ class UsersController < ApplicationController params[:locale] ||= I18n.locale unless current_user new_user_params = user_params.except(:timezone) - user = User.unstage(new_user_params) - user = User.new(new_user_params) if user.nil? + + user = User.where(staged: true).with_email(new_user_params[:email].strip.downcase).first + + if user + user.active = false + user.unstage! + end + + user ||= User.new + user.attributes = new_user_params # Handle API approval ReviewableUser.set_approved_fields!(user, current_user) if user.approved? diff --git a/app/models/discourse_single_sign_on.rb b/app/models/discourse_single_sign_on.rb index 9d72a9cc981..bc0916078fa 100644 --- a/app/models/discourse_single_sign_on.rb +++ b/app/models/discourse_single_sign_on.rb @@ -74,8 +74,7 @@ class DiscourseSingleSignOn < SingleSignOn end # ensure it's not staged anymore - user.unstage - user.save + user.unstage! change_external_attributes_and_override(sso_record, user) diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index f3eb839cdcb..68a415d274e 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -15,26 +15,27 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f # extracted from User cause it is very specific to invites def self.create_user_from_invite(invite, username, name, password = nil, user_custom_fields = nil, ip_address = nil) + user = User.where(staged: true).with_email(invite.email.strip.downcase).first + user.unstage! if user + + user ||= User.new + if username && UsernameValidator.new(username).valid_format? && User.username_available?(username) available_username = username else available_username = UserNameSuggester.suggest(invite.email) end - available_name = name || available_username - user_params = { + user.attributes = { email: invite.email, username: available_username, - name: available_name, + name: name || available_username, active: false, trust_level: SiteSetting.default_invitee_trust_level, ip_address: ip_address, registration_ip_address: ip_address } - user = User.unstage(user_params) - user = User.new(user_params) if user.nil? - if !SiteSetting.must_approve_users? || (SiteSetting.must_approve_users? && invite.invited_by.staff?) ReviewableUser.set_approved_fields!(user, invite.invited_by) end diff --git a/app/models/user.rb b/app/models/user.rb index 3ddc498bc32..3993d4ea79b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -363,24 +363,19 @@ class User < ActiveRecord::Base user end - def unstage + def unstage! if self.staged - self.staged = false - self.custom_fields[FROM_STAGED] = true - self.notifications.destroy_all + ActiveRecord::Base.transaction do + self.staged = false + self.custom_fields[FROM_STAGED] = true + self.notifications.destroy_all + save! + end + DiscourseEvent.trigger(:user_unstaged, self) end end - def self.unstage(params) - if user = User.where(staged: true).with_email(params[:email].strip.downcase).first - params.each { |k, v| user.public_send("#{k}=", v) } - user.active = false - user.unstage - end - user - end - def self.suggest_name(string) return "" if string.blank? (string[/\A[^@]+/].presence || string[/[^@]+\z/]).tr(".", " ").titleize diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index e56e4cfa23a..ca8ec4d1971 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -167,7 +167,7 @@ class Auth::DefaultCurrentUserProvider impersonate: opts[:impersonate]) cookies[TOKEN_COOKIE] = cookie_hash(@user_token.unhashed_auth_token) - unstage_user(user) + user.unstage! make_developer_admin(user) enable_bootstrap_mode(user) @@ -191,13 +191,6 @@ class Auth::DefaultCurrentUserProvider hash end - def unstage_user(user) - if user.staged - user.unstage - user.save - end - end - def make_developer_admin(user) if user.active? && !user.admin && diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d215a89458e..96e7f277576 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -232,8 +232,7 @@ describe Group do expect(GroupUser.where(user_id: staged.id).count).to eq(0) - staged.unstage - staged.save! + staged.unstage! expect(GroupUser.where(user_id: staged.id).count).to eq(2) end diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index b09245460b7..903f5c10b01 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -1123,7 +1123,7 @@ describe Post do SiteSetting.newuser_max_links = 3 user = Fabricate(:user, staged: true, trust_level: 0) user.created_at = 1.hour.ago - user.unstage + user.unstage! post = Fabricate(:post, raw: raw, user: user) expect(post.has_host_spam?).to eq(true) end @@ -1133,7 +1133,7 @@ describe Post do SiteSetting.newuser_max_links = 3 user = Fabricate(:user, staged: true, trust_level: 0) user.created_at = 2.days.ago - user.unstage + user.unstage! post = Fabricate(:post, raw: raw, user: user) expect(post.has_host_spam?).to eq(false) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 1e3998db7d5..995a532290a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1840,44 +1840,29 @@ describe User do end end - describe "#unstage" do - let!(:staged_user) { Fabricate(:staged, email: 'staged@account.com', active: true, username: 'staged1', name: 'Stage Name') } - let(:params) { { email: 'staged@account.com', active: true, username: 'unstaged1', name: 'Foo Bar' } } + describe "#unstage!" do + let!(:user) { Fabricate(:staged, email: 'staged@account.com', active: true, username: 'staged1', name: 'Stage Name') } it "correctly unstages a user" do - user = User.unstage(params) - - expect(user.id).to eq(staged_user.id) - expect(user.username).to eq('unstaged1') - expect(user.name).to eq('Foo Bar') - expect(user.active).to eq(false) - expect(user.email).to eq('staged@account.com') + user.unstage! expect(user.staged).to eq(false) end - it "returns nil when the user cannot be unstaged" do - Fabricate(:coding_horror) - expect(User.unstage(email: 'jeff@somewhere.com')).to be_nil - expect(User.unstage(email: 'no@account.com')).to be_nil - end - it "removes all previous notifications during unstaging" do - Fabricate(:notification, user: staged_user) - Fabricate(:private_message_notification, user: staged_user) - staged_user.reload + Fabricate(:notification, user: user) + Fabricate(:private_message_notification, user: user) + expect(user.total_unread_notifications).to eq(2) - expect(staged_user.total_unread_notifications).to eq(2) - user = User.unstage(params) + user.unstage! + user.reload expect(user.total_unread_notifications).to eq(0) expect(user.staged).to eq(false) end it "triggers an event" do - unstaged_user = nil - event = DiscourseEvent.track_events { unstaged_user = User.unstage(params) }.first - + event = DiscourseEvent.track_events { user.unstage! }.first expect(event[:event_name]).to eq(:user_unstaged) - expect(event[:params].first).to eq(unstaged_user) + expect(event[:params].first).to eq(user) end end