Commit 47e4613f authored by Gabriel Mazetto's avatar Gabriel Mazetto

Code style fixes and some code simplified

parent ec67e9be
...@@ -26,7 +26,7 @@ class Admin::IdentitiesController < Admin::ApplicationController ...@@ -26,7 +26,7 @@ class Admin::IdentitiesController < Admin::ApplicationController
def update def update
if @identity.update_attributes(identity_params) if @identity.update_attributes(identity_params)
RepairLdapBlockedUserService.new(@user, @identity).execute RepairLdapBlockedUserService.new(@user).execute
redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully updated.' redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully updated.'
else else
render :edit render :edit
...@@ -35,7 +35,7 @@ class Admin::IdentitiesController < Admin::ApplicationController ...@@ -35,7 +35,7 @@ class Admin::IdentitiesController < Admin::ApplicationController
def destroy def destroy
if @identity.destroy if @identity.destroy
RepairLdapBlockedUserService.new(@user, @identity).execute RepairLdapBlockedUserService.new(@user).execute
redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully removed.' redirect_to admin_user_identities_path(@user), notice: 'User identity was successfully removed.'
else else
redirect_to admin_user_identities_path(@user), alert: 'Failed to remove user identity.' redirect_to admin_user_identities_path(@user), alert: 'Failed to remove user identity.'
......
class RepairLdapBlockedUserService class RepairLdapBlockedUserService
attr_accessor :user, :identity attr_accessor :user
def initialize(user, identity) def initialize(user)
@user, @identity = user, identity @user = user
end end
def execute def execute
if identity.destroyed? user.block if ldap_hard_blocked?
user.block if identity.is_ldap? && user.ldap_blocked? && !user.ldap_user?
else
user.block if !identity.is_ldap? && user.ldap_blocked? && !user.ldap_user?
end end
private
def ldap_hard_blocked?
user.ldap_blocked? && !user.ldap_user?
end end
end end
...@@ -40,7 +40,9 @@ module Gitlab ...@@ -40,7 +40,9 @@ module Gitlab
user.ldap_block user.ldap_block
false false
else else
user.activate if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked? if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked?
user.activate
end
true true
end end
else else
......
...@@ -3,7 +3,7 @@ require 'spec_helper' ...@@ -3,7 +3,7 @@ require 'spec_helper'
describe RepairLdapBlockedUserService, services: true do describe RepairLdapBlockedUserService, services: true do
let(:user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } let(:user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
let(:identity) { user.ldap_identity } let(:identity) { user.ldap_identity }
subject(:service) { RepairLdapBlockedUserService.new(user, identity) } subject(:service) { RepairLdapBlockedUserService.new(user) }
describe '#execute' do describe '#execute' do
it 'change to normal block after destroying last ldap identity' do it 'change to normal block after destroying last ldap identity' do
......
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