Merge branch 'master' into 20827-getting-started-update-new-group-view20827-getting-started-update-new-group-view

5 files changed, 139 insertions, 66 deletions

diff --git a/spec/services/users/activity_service_spec.rb b/spec/services/users/activity_service_spec.rb
new file mode 100644
index 00000000000..8d67ebe3231
--- /dev/null
+++ b/spec/services/users/activity_service_spec.rb
@@ -0,0 +1,48 @@
+require 'spec_helper'
+
+describe Users::ActivityService, services: true do
+  include UserActivitiesHelpers
+
+  let(:user) { create(:user) }
+
+  subject(:service) { described_class.new(user, 'type') }
+
+  describe '#execute', :redis do
+    context 'when last activity is nil' do
+      before do
+        service.execute
+      end
+
+      it 'sets the last activity timestamp for the user' do
+        expect(last_hour_user_ids).to eq([user.id])
+      end
+
+      it 'updates the same user' do
+        service.execute
+
+        expect(last_hour_user_ids).to eq([user.id])
+      end
+
+      it 'updates the timestamp of an existing user' do
+        Timecop.freeze(Date.tomorrow) do
+          expect { service.execute }.to change { user_activity(user) }.to(Time.now.to_i.to_s)
+        end
+      end
+
+      describe 'other user' do
+        it 'updates other user' do
+          other_user = create(:user)
+          described_class.new(other_user, 'type').execute
+
+          expect(last_hour_user_ids).to match_array([user.id, other_user.id])
+        end
+      end
+    end
+  end
+
+  def last_hour_user_ids
+    Gitlab::UserActivities.new.
+      select { |k, v| v >= 1.hour.ago.to_i.to_s }.
+      map { |k, _| k.to_i }
+  end
+end
diff --git a/spec/services/users/build_service_spec.rb b/spec/services/users/build_service_spec.rb
new file mode 100644
index 00000000000..2a6bfc1b3a0
--- /dev/null
+++ b/spec/services/users/build_service_spec.rb
@@ -0,0 +1,55 @@
+require 'spec_helper'
+
+describe Users::BuildService, services: true do
+  describe '#execute' do
+    let(:params) do
+      { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass' }
+    end
+
+    context 'with an admin user' do
+      let(:admin_user) { create(:admin) }
+      let(:service) { described_class.new(admin_user, params) }
+
+      it 'returns a valid user' do
+        expect(service.execute).to be_valid
+      end
+    end
+
+    context 'with non admin user' do
+      let(:user) { create(:user) }
+      let(:service) { described_class.new(user, params) }
+
+      it 'raises AccessDeniedError exception' do
+        expect { service.execute }.to raise_error Gitlab::Access::AccessDeniedError
+      end
+    end
+
+    context 'with nil user' do
+      let(:service) { described_class.new(nil, params) }
+
+      it 'returns a valid user' do
+        expect(service.execute).to be_valid
+      end
+
+      context 'when "send_user_confirmation_email" application setting is true' do
+        before do
+          stub_application_setting(send_user_confirmation_email: true, signup_enabled?: true)
+        end
+
+        it 'does not confirm the user' do
+          expect(service.execute).not_to be_confirmed
+        end
+      end
+
+      context 'when "send_user_confirmation_email" application setting is false' do
+        before do
+          stub_application_setting(send_user_confirmation_email: false, signup_enabled?: true)
+        end
+
+        it 'confirms the user' do
+          expect(service.execute).to be_confirmed
+        end
+      end
+    end
+  end
+end
diff --git a/spec/services/users/create_service_spec.rb b/spec/services/users/create_service_spec.rb
index a111aec2f89..75746278573 100644
--- a/spec/services/users/create_service_spec.rb
+++ b/spec/services/users/create_service_spec.rb
@@ -1,38 +1,6 @@
 require 'spec_helper'
 
 describe Users::CreateService, services: true do
-  describe '#build' do
-    let(:params) do
-      { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass' }
-    end
-
-    context 'with an admin user' do
-      let(:admin_user) { create(:admin) }
-      let(:service) { described_class.new(admin_user, params) }
-
-      it 'returns a valid user' do
-        expect(service.build).to be_valid
-      end
-    end
-
-    context 'with non admin user' do
-      let(:user) { create(:user) }
-      let(:service) { described_class.new(user, params) }
-
-      it 'raises AccessDeniedError exception' do
-        expect { service.build }.to raise_error Gitlab::Access::AccessDeniedError
-      end
-    end
-
-    context 'with nil user' do
-      let(:service) { described_class.new(nil, params) }
-
-      it 'returns a valid user' do
-        expect(service.build).to be_valid
-      end
-    end
-  end
-
   describe '#execute' do
     let(:admin_user) { create(:admin) }
 
@@ -185,40 +153,18 @@ describe Users::CreateService, services: true do
       end
       let(:service) { described_class.new(nil, params) }
 
-      context 'when "send_user_confirmation_email" application setting is true' do
-        before do
-          current_application_settings = double(:current_application_settings, send_user_confirmation_email: true, signup_enabled?: true)
-          allow(service).to receive(:current_application_settings).and_return(current_application_settings)
-        end
-
-        it 'does not confirm the user' do
-          expect(service.execute).not_to be_confirmed
-        end
-      end
-
-      context 'when "send_user_confirmation_email" application setting is false' do
-        before do
-          current_application_settings = double(:current_application_settings, send_user_confirmation_email: false, signup_enabled?: true)
-          allow(service).to receive(:current_application_settings).and_return(current_application_settings)
-        end
-
-        it 'confirms the user' do
-          expect(service.execute).to be_confirmed
-        end
-
-        it 'persists the given attributes' do
-          user = service.execute
-          user.reload
-
-          expect(user).to have_attributes(
-            name: params[:name],
-            username: params[:username],
-            email: params[:email],
-            password: params[:password],
-            created_by_id: nil,
-            admin: false
-          )
-        end
+      it 'persists the given attributes' do
+        user = service.execute
+        user.reload
+
+        expect(user).to have_attributes(
+          name: params[:name],
+          username: params[:username],
+          email: params[:email],
+          password: params[:password],
+          created_by_id: nil,
+          admin: false
+        )
       end
     end
   end
diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb
index 43c18992d1a..4bc30018ebd 100644
--- a/spec/services/users/destroy_service_spec.rb
+++ b/spec/services/users/destroy_service_spec.rb
@@ -152,6 +152,12 @@ describe Users::DestroyService, services: true do
 
         service.execute(user)
       end
+
+      it 'does not run `MigrateToGhostUser` if hard_delete option is given' do
+        expect_any_instance_of(Users::MigrateToGhostUserService).not_to receive(:execute)
+
+        service.execute(user, hard_delete: true)
+      end
     end
   end
 end
diff --git a/spec/services/users/migrate_to_ghost_user_service_spec.rb b/spec/services/users/migrate_to_ghost_user_service_spec.rb
index 8c5b7e41c15..9e1edf1ac30 100644
--- a/spec/services/users/migrate_to_ghost_user_service_spec.rb
+++ b/spec/services/users/migrate_to_ghost_user_service_spec.rb
@@ -60,5 +60,23 @@ describe Users::MigrateToGhostUserService, services: true do
         end
       end
     end
+
+    context "when record migration fails with a rollback exception" do
+      before do
+        expect_any_instance_of(MergeRequest::ActiveRecord_Associations_CollectionProxy)
+          .to receive(:update_all).and_raise(ActiveRecord::Rollback)
+      end
+
+      context "for records that were already migrated" do
+        let!(:issue) { create(:issue, project: project, author: user) }
+        let!(:merge_request) { create(:merge_request, source_project: project, author: user, target_branch: "first") }
+
+        it "reverses the migration" do
+          service.execute
+
+          expect(issue.reload.author).to eq(user)
+        end
+      end
+    end
   end
 end