diff options
author | Rémy Coutable <remy@rymai.me> | 2018-02-16 15:10:22 +0100 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2018-02-27 16:02:22 +0100 |
commit | 1c88d92b3fe174a56080575a14d6b473f17f7d8f (patch) | |
tree | 7150c48b368e82d70f6bfc228c3d836c7be7c3ac /app/services/members | |
parent | e82f629be4b9c91e2611095cd4296e487ed137ef (diff) | |
download | gitlab-ce-1c88d92b3fe174a56080575a14d6b473f17f7d8f.tar.gz |
Improve Member servicesrc/reduce-delta-with-ce-in-controllers-ce
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'app/services/members')
-rw-r--r-- | app/services/members/approve_access_request_service.rb | 15 | ||||
-rw-r--r-- | app/services/members/authorized_destroy_service.rb | 59 | ||||
-rw-r--r-- | app/services/members/base_service.rb | 8 | ||||
-rw-r--r-- | app/services/members/create_service.rb | 4 | ||||
-rw-r--r-- | app/services/members/destroy_service.rb | 52 | ||||
-rw-r--r-- | app/services/members/request_access_service.rb | 4 | ||||
-rw-r--r-- | app/services/members/update_service.rb | 3 |
7 files changed, 60 insertions, 85 deletions
diff --git a/app/services/members/approve_access_request_service.rb b/app/services/members/approve_access_request_service.rb index 19431ac76dc..6be08b590bc 100644 --- a/app/services/members/approve_access_request_service.rb +++ b/app/services/members/approve_access_request_service.rb @@ -1,25 +1,20 @@ module Members class ApproveAccessRequestService < Members::BaseService - # opts - A hash of options - # :ldap - The call is from a LDAP sync: current_user can be nil in that case - def execute(access_requester, opts = {}) - raise Gitlab::Access::AccessDeniedError unless can_update_access_requester?(access_requester, opts[:ldap]) + def execute(access_requester, skip_authorization: false, skip_log_audit_event: false) + raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_update_access_requester?(access_requester) access_requester.access_level = params[:access_level] if params[:access_level] access_requester.accept_request - after_execute(member: access_requester, **opts) + after_execute(member: access_requester, skip_log_audit_event: skip_log_audit_event) access_requester end private - def can_update_access_requester?(access_requester, ldap) - access_requester && ( - ldap || - can?(current_user, update_member_permission(access_requester), access_requester) - ) + def can_update_access_requester?(access_requester) + can?(current_user, update_member_permission(access_requester), access_requester) end end end diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb deleted file mode 100644 index 793a951d63b..00000000000 --- a/app/services/members/authorized_destroy_service.rb +++ /dev/null @@ -1,59 +0,0 @@ -module Members - class AuthorizedDestroyService < BaseService - def initialize(current_user = nil) - @current_user = current_user - end - - def execute(member) - return false if member.is_a?(GroupMember) && member.source.last_owner?(member.user) - - Member.transaction do - unassign_issues_and_merge_requests(member) unless member.invite? - member.notification_setting&.destroy - - member.destroy - end - - if member.request? && member.user != current_user - notification_service.decline_access_request(member) - end - - member - end - - private - - def unassign_issues_and_merge_requests(member) - if member.is_a?(GroupMember) - issues = Issue.unscoped.select(1) - .joins(:project) - .where('issues.id = issue_assignees.issue_id AND projects.namespace_id = ?', member.source_id) - - # DELETE FROM issue_assignees WHERE user_id = X AND EXISTS (...) - IssueAssignee.unscoped - .where('user_id = :user_id AND EXISTS (:sub)', user_id: member.user_id, sub: issues) - .delete_all - - MergeRequestsFinder.new(current_user, group_id: member.source_id, assignee_id: member.user_id) - .execute - .update_all(assignee_id: nil) - else - project = member.source - - # SELECT 1 FROM issues WHERE issues.id = issue_assignees.issue_id AND issues.project_id = X - issues = Issue.unscoped.select(1) - .where('issues.id = issue_assignees.issue_id') - .where(project_id: project.id) - - # DELETE FROM issue_assignees WHERE user_id = X AND EXISTS (...) - IssueAssignee.unscoped - .where('user_id = :user_id AND EXISTS (:sub)', user_id: member.user_id, sub: issues) - .delete_all - - project.merge_requests.opened.assigned_to(member.user).update_all(assignee_id: nil) - end - - member.user.invalidate_cache_counts - end - end -end diff --git a/app/services/members/base_service.rb b/app/services/members/base_service.rb index 5c52468aaca..74556fb20cf 100644 --- a/app/services/members/base_service.rb +++ b/app/services/members/base_service.rb @@ -1,17 +1,13 @@ module Members class BaseService < ::BaseService - attr_accessor :source - - # source - The source object that respond to `#members` (e.g. project or group) # current_user - The user that performs the action # params - A hash of parameters - def initialize(source, current_user, params = {}) - @source = source + def initialize(current_user = nil, params = {}) @current_user = current_user @params = params end - def after_execute(**args) + def after_execute(args) # overriden in EE::Members modules end diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index 61802ba09ce..bc6a9405aac 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -2,7 +2,7 @@ module Members class CreateService < Members::BaseService DEFAULT_LIMIT = 100 - def execute + def execute(source) return error('No users specified.') if params[:user_ids].blank? user_ids = params[:user_ids].split(',').uniq @@ -17,7 +17,7 @@ module Members current_user: current_user ) - members.compact.each { |member| after_execute(member: member) } + members.each { |member| after_execute(member: member) } success end diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index 477ab127b2d..b141bfd5fbc 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -1,9 +1,20 @@ module Members class DestroyService < Members::BaseService - def execute(member) - raise Gitlab::Access::AccessDeniedError unless can_destroy_member?(member) + def execute(member, skip_authorization: false) + raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_destroy_member?(member) - AuthorizedDestroyService.new(current_user).execute(member) + return member if member.is_a?(GroupMember) && member.source.last_owner?(member.user) + + Member.transaction do + unassign_issues_and_merge_requests(member) unless member.invite? + member.notification_setting&.destroy + + member.destroy + end + + if member.request? && member.user != current_user + notification_service.decline_access_request(member) + end after_execute(member: member) @@ -13,7 +24,7 @@ module Members private def can_destroy_member?(member) - member && can?(current_user, destroy_member_permission(member), member) + can?(current_user, destroy_member_permission(member), member) end def destroy_member_permission(member) @@ -26,5 +37,38 @@ module Members raise "Unknown member type: #{member}!" end end + + def unassign_issues_and_merge_requests(member) + if member.is_a?(GroupMember) + issues = Issue.unscoped.select(1) + .joins(:project) + .where('issues.id = issue_assignees.issue_id AND projects.namespace_id = ?', member.source_id) + + # DELETE FROM issue_assignees WHERE user_id = X AND EXISTS (...) + IssueAssignee.unscoped + .where('user_id = :user_id AND EXISTS (:sub)', user_id: member.user_id, sub: issues) + .delete_all + + MergeRequestsFinder.new(current_user, group_id: member.source_id, assignee_id: member.user_id) + .execute + .update_all(assignee_id: nil) + else + project = member.source + + # SELECT 1 FROM issues WHERE issues.id = issue_assignees.issue_id AND issues.project_id = X + issues = Issue.unscoped.select(1) + .where('issues.id = issue_assignees.issue_id') + .where(project_id: project.id) + + # DELETE FROM issue_assignees WHERE user_id = X AND EXISTS (...) + IssueAssignee.unscoped + .where('user_id = :user_id AND EXISTS (:sub)', user_id: member.user_id, sub: issues) + .delete_all + + project.merge_requests.opened.assigned_to(member.user).update_all(assignee_id: nil) + end + + member.user.invalidate_cache_counts + end end end diff --git a/app/services/members/request_access_service.rb b/app/services/members/request_access_service.rb index e30722efc13..24293b30005 100644 --- a/app/services/members/request_access_service.rb +++ b/app/services/members/request_access_service.rb @@ -1,6 +1,6 @@ module Members class RequestAccessService < Members::BaseService - def execute + def execute(source) raise Gitlab::Access::AccessDeniedError unless can_request_access?(source) source.members.create( @@ -12,7 +12,7 @@ module Members private def can_request_access?(source) - source && can?(current_user, :request_access, source) + can?(current_user, :request_access, source) end end end diff --git a/app/services/members/update_service.rb b/app/services/members/update_service.rb index ebea9f96069..48b3d59f7bd 100644 --- a/app/services/members/update_service.rb +++ b/app/services/members/update_service.rb @@ -2,8 +2,7 @@ module Members class UpdateService < Members::BaseService # returns the updated member def execute(member, permission: :update) - permission_target = permission == :override ? source : member - raise Gitlab::Access::AccessDeniedError unless can?(current_user, action_member_permission(permission, member), permission_target) + raise Gitlab::Access::AccessDeniedError unless can?(current_user, action_member_permission(permission, member), member) old_access_level = member.human_access |