summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2017-03-31 21:29:51 -0700
committerStan Hu <stanhu@gmail.com>2017-04-02 05:37:05 -0700
commit6a2d022d1d578f8957736de2fb895069c24c072b (patch)
tree206166bcce0e3256b7d8151e5b9172fc01ae4e5a
parent8a71d40e60c799f969af0ed255e931b6c9907634 (diff)
downloadgitlab-ce-sh-fix-destroy-user-race.tar.gz
Delete users asynchronouslysh-fix-destroy-user-race
-rw-r--r--app/controllers/registrations_controller.rb4
-rw-r--r--lib/api/users.rb2
-rw-r--r--spec/controllers/registrations_controller_spec.rb16
-rw-r--r--spec/requests/api/users_spec.rb10
4 files changed, 24 insertions, 8 deletions
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index a49a1f50a81..8109427a45f 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -25,12 +25,12 @@ class RegistrationsController < Devise::RegistrationsController
end
def destroy
- Users::DestroyService.new(current_user).execute(current_user)
+ DeleteUserWorker.perform_async(current_user.id, current_user.id)
respond_to do |format|
format.html do
session.try(:destroy)
- redirect_to new_user_session_path, notice: "Account successfully removed."
+ redirect_to new_user_session_path, notice: "Account scheduled for removal."
end
end
end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index a4201fe6fed..530ca0b5235 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -293,7 +293,7 @@ module API
user = User.find_by(id: params[:id])
not_found!('User') unless user
- ::Users::DestroyService.new(current_user).execute(user)
+ DeleteUserWorker.perform_async(current_user.id, user.id)
end
desc 'Block a user. Available only for admins.'
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index 902911071c4..71dd9ef3eb4 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -68,4 +68,20 @@ describe RegistrationsController do
end
end
end
+
+ describe '#destroy' do
+ let(:user) { create(:user) }
+
+ before do
+ sign_in(user)
+ end
+
+ it 'schedules the user for destruction' do
+ expect(DeleteUserWorker).to receive(:perform_async).with(user.id, user.id)
+
+ post(:destroy)
+
+ expect(response.status).to eq(302)
+ end
+ end
end
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 04e7837fd7a..f793c0db2f3 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -676,7 +676,7 @@ describe API::Users, api: true do
before { admin }
it "deletes user" do
- delete api("/users/#{user.id}", admin)
+ Sidekiq::Testing.inline! { delete api("/users/#{user.id}", admin) }
expect(response).to have_http_status(204)
expect { User.find(user.id) }.to raise_error ActiveRecord::RecordNotFound
@@ -684,23 +684,23 @@ describe API::Users, api: true do
end
it "does not delete for unauthenticated user" do
- delete api("/users/#{user.id}")
+ Sidekiq::Testing.inline! { delete api("/users/#{user.id}") }
expect(response).to have_http_status(401)
end
it "is not available for non admin users" do
- delete api("/users/#{user.id}", user)
+ Sidekiq::Testing.inline! { delete api("/users/#{user.id}", user) }
expect(response).to have_http_status(403)
end
it "returns 404 for non-existing user" do
- delete api("/users/999999", admin)
+ Sidekiq::Testing.inline! { delete api("/users/999999", admin) }
expect(response).to have_http_status(404)
expect(json_response['message']).to eq('404 User Not Found')
end
it "returns a 404 for invalid ID" do
- delete api("/users/ASDF", admin)
+ Sidekiq::Testing.inline! { delete api("/users/ASDF", admin) }
expect(response).to have_http_status(404)
end