Merge branch 'dm-internal-user-namespace' into 'master'

Ensure internal users (ghost, support bot) get assigned a namespace

See merge request gitlab-org/gitlab-ce!18194

4 files changed, 12 insertions, 3 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index ba51595e6a3..7b6857a0d34 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -164,12 +164,15 @@ class User < ActiveRecord::Base
 
   before_validation :sanitize_attrs
   before_validation :set_notification_email, if: :email_changed?
+  before_save :set_notification_email, if: :email_changed? # in case validation is skipped
   before_validation :set_public_email, if: :public_email_changed?
+  before_save :set_public_email, if: :public_email_changed? # in case validation is skipped
   before_save :ensure_incoming_email_token
   before_save :ensure_user_rights_and_limits, if: ->(user) { user.new_record? || user.external_changed? }
   before_save :skip_reconfirmation!, if: ->(user) { user.email_changed? && user.read_only_attribute?(:email) }
   before_save :check_for_verified_email, if: ->(user) { user.email_changed? && !user.new_record? }
   before_validation :ensure_namespace_correct
+  before_save :ensure_namespace_correct # in case validation is skipped
   after_validation :set_username_errors
   after_update :username_changed_hook, if: :username_changed?
   after_destroy :post_destroy_hook
@@ -408,7 +411,6 @@ class User < ActiveRecord::Base
       unique_internal(where(ghost: true), 'ghost', email) do |u|
         u.bio = 'This is a "Ghost User", created to hold all issues authored by users that have since been deleted. This user cannot be removed.'
         u.name = 'Ghost User'
-        u.notification_email = email
       end
     end
   end
diff --git a/changelogs/unreleased/dm-internal-user-namespace.yml b/changelogs/unreleased/dm-internal-user-namespace.yml
new file mode 100644
index 00000000000..8517c116795
--- /dev/null
+++ b/changelogs/unreleased/dm-internal-user-namespace.yml
@@ -0,0 +1,5 @@
+---
+title: Ensure internal users (ghost, support bot) get assigned a namespace
+merge_request:
+author:
+type: fixed
diff --git a/spec/migrations/rename_users_with_renamed_namespace_spec.rb b/spec/migrations/rename_users_with_renamed_namespace_spec.rb
index cbc0ebeb44d..e2994103ed7 100644
--- a/spec/migrations/rename_users_with_renamed_namespace_spec.rb
+++ b/spec/migrations/rename_users_with_renamed_namespace_spec.rb
@@ -7,9 +7,9 @@ describe RenameUsersWithRenamedNamespace, :delete do
     other_user1 = create(:user, username: 'api0')
 
     user = create(:user, username: "Users0")
-    user.update_attribute(:username, 'Users')
+    user.update_column(:username, 'Users')
     user1 = create(:user, username: "import0")
-    user1.update_attribute(:username, 'import')
+    user1.update_column(:username, 'import')
 
     described_class.new.up
 
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 4027c420e47..a600987d0bf 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -2071,6 +2071,8 @@ describe User do
 
       expect(ghost).to be_ghost
       expect(ghost).to be_persisted
+      expect(ghost.namespace).not_to be_nil
+      expect(ghost.namespace).to be_persisted
     end
 
     it "does not create a second ghost user if one is already present" do