diff options
| -rw-r--r-- | app/models/user.rb | 15 | ||||
| -rw-r--r-- | app/services/notification_service.rb | 1 | ||||
| -rw-r--r-- | db/migrate/20170206115204_add_column_ghost_to_users.rb | 15 | ||||
| -rw-r--r-- | db/schema.rb | 1 | ||||
| -rw-r--r-- | spec/factories/users.rb | 4 | ||||
| -rw-r--r-- | spec/models/user_spec.rb | 14 |
6 files changed, 44 insertions, 6 deletions
diff --git a/app/models/user.rb b/app/models/user.rb index e4adcd6cde9..f71d7e90d33 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -124,6 +124,7 @@ class User < ActiveRecord::Base validate :unique_email, if: ->(user) { user.email_changed? } validate :owns_notification_email, if: ->(user) { user.notification_email_changed? } validate :owns_public_email, if: ->(user) { user.public_email_changed? } + validate :ghost_users_must_be_blocked validates :avatar, file_size: { maximum: 200.kilobytes.to_i } before_validation :generate_password, on: :create @@ -185,8 +186,6 @@ class User < ActiveRecord::Base "administrator if you think this is an error." end end - - state :ghost end mount_uploader :avatar, AvatarUploader @@ -347,7 +346,7 @@ class User < ActiveRecord::Base # Return (create if necessary) the ghost user. The ghost user # owns records previously belonging to deleted users. def ghost - ghost_user = User.with_state(:ghost).first + ghost_user = User.find_by_ghost(true) ghost_user || begin @@ -359,7 +358,7 @@ class User < ActiveRecord::Base # Recheck if a ghost user is already present (one might have been) # added between the time we last checked (first line of this method) # and the time we acquired the lock. - ghost_user = User.with_state(:ghost).first + ghost_user = User.find_by_ghost(true) return ghost_user if ghost_user.present? uniquify = Uniquify.new @@ -373,7 +372,7 @@ class User < ActiveRecord::Base User.create( username: username, password: Devise.friendly_token, - email: email, name: "Ghost User", state: :ghost + email: email, name: "Ghost User", state: :blocked, ghost: true ) ensure advisory_lock.unlock @@ -477,6 +476,12 @@ class User < ActiveRecord::Base errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email) end + def ghost_users_must_be_blocked + if ghost? && !blocked? + errors.add(:ghost, 'cannot be enabled for a user who is not blocked.') + end + end + def update_emails_with_primary_email primary_email_record = emails.find_by(email: email) if primary_email_record diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 97db117aa99..3734e3c4253 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -466,7 +466,6 @@ class NotificationService users = users.to_a.compact.uniq users = users.reject(&:blocked?) - users = users.reject(&:ghost?) users.reject do |user| global_notification_setting = user.global_notification_setting diff --git a/db/migrate/20170206115204_add_column_ghost_to_users.rb b/db/migrate/20170206115204_add_column_ghost_to_users.rb new file mode 100644 index 00000000000..57b66ca91a8 --- /dev/null +++ b/db/migrate/20170206115204_add_column_ghost_to_users.rb @@ -0,0 +1,15 @@ +class AddColumnGhostToUsers < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default :users, :ghost, :boolean, default: false, allow_null: false + end + + def down + remove_column :users, :ghost + end +end diff --git a/db/schema.rb b/db/schema.rb index 82bccda580a..c50d6fc4ad6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1282,6 +1282,7 @@ ActiveRecord::Schema.define(version: 20170216141440) do t.string "incoming_email_token" t.boolean "authorized_projects_populated" t.boolean "notified_of_own_activity", default: false, null: false + t.boolean "ghost", default: false, null: false end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 1732b1a0081..58602c54c7a 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -26,6 +26,10 @@ FactoryGirl.define do two_factor_via_otp end + trait :ghost do + ghost true + end + trait :two_factor_via_otp do before(:create) do |user| user.otp_required_for_login = true diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 861d338ef95..9e2b1c9290f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -208,6 +208,20 @@ describe User, models: true do end end end + + describe 'ghost users' do + it 'does not allow a non-blocked ghost user' do + user = build(:user, :ghost, state: :active) + + expect(user).to be_invalid + end + + it 'allows a blocked ghost user' do + user = build(:user, :ghost, state: :blocked) + + expect(user).to be_valid + end + end end describe "scopes" do |