Commit 4ea4ec48 authored by Serena Fang's avatar Serena Fang

Check for valid state in service

Also fix spec names
parent b4b91b2c
......@@ -8,6 +8,10 @@ module Users
user.ban
end
def valid_state?(user)
user.active?
end
def action
:ban
end
......
......@@ -8,7 +8,7 @@ module Users
def execute(user)
return permission_error unless allowed?
return state_error(user) unless valid_state(user)
return state_error(user) unless valid_state?(user)
if update_user(user)
log_event(user)
......@@ -23,15 +23,6 @@ module Users
attr_reader :current_user
def valid_state(user)
case action
when :ban
user.active?
when :unban
user.banned?
end
end
def state_error(user)
error(_("You cannot %{action} %{state} users." % { action: action.to_s, state: user.state }), :forbidden)
end
......
......@@ -8,6 +8,10 @@ module Users
user.unban
end
def valid_state?(user)
user.banned?
end
def action
:unban
end
......
......@@ -3018,10 +3018,10 @@ RSpec.describe API::Users do
end
end
context 'with a non existant user' do
context 'with a non existent user' do
let(:user_id) { non_existing_record_id }
it 'does not ban non existant users' do
it 'does not ban non existent users' do
ban_user
expect(response).to have_gitlab_http_status(:not_found)
......@@ -3055,7 +3055,7 @@ RSpec.describe API::Users do
context 'with a banned user' do
let(:user_id) { banned_user.id }
it 'bans an active user' do
it 'activates a banned user' do
unban_user
expect(response).to have_gitlab_http_status(:created)
......@@ -3066,7 +3066,7 @@ RSpec.describe API::Users do
context 'with an ldap_blocked user' do
let(:user_id) { ldap_blocked_user.id }
it 'does not ban ldap_blocked users' do
it 'does not unban ldap_blocked users' do
unban_user
expect(response).to have_gitlab_http_status(:forbidden)
......@@ -3078,7 +3078,7 @@ RSpec.describe API::Users do
context 'with a deactivated user' do
let(:user_id) { deactivated_user.id }
it 'does not ban deactivated users' do
it 'does not unban deactivated users' do
unban_user
expect(response).to have_gitlab_http_status(:forbidden)
......@@ -3090,7 +3090,7 @@ RSpec.describe API::Users do
context 'with an active user' do
let(:user_id) { user.id }
it 'does not ban active users' do
it 'does not unban active users' do
unban_user
expect(response).to have_gitlab_http_status(:forbidden)
......@@ -3099,10 +3099,10 @@ RSpec.describe API::Users do
end
end
context 'with a non existant user' do
context 'with a non existent user' do
let(:user_id) { non_existing_record_id }
it 'does not ban non existant users' do
it 'does not unban non existent users' do
unban_user
expect(response).to have_gitlab_http_status(:not_found)
......@@ -3113,7 +3113,7 @@ RSpec.describe API::Users do
context 'with an invalid id user' do
let(:user_id) { 'ASDF' }
it 'does not ban invalid id users' do
it 'does not unban invalid id users' do
unban_user
expect(response).to have_gitlab_http_status(:not_found)
......
......@@ -50,7 +50,7 @@ RSpec.describe Users::BanService do
response = ban_user
expect(response[:status]).to eq(:error)
expect(response[:message]).to match('You cannot ban a blocked user.')
expect(response[:message]).to match('You cannot ban blocked users.')
end
it_behaves_like 'does not modify the BannedUser record or user state'
......
......@@ -50,7 +50,7 @@ RSpec.describe Users::UnbanService do
response = unban_user
expect(response[:status]).to eq(:error)
expect(response[:message]).to match('You cannot unban a active user.')
expect(response[:message]).to match('You cannot unban active users.')
end
it_behaves_like 'does not modify the BannedUser record or user state'
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment