Skip to content

G
gitlab-ce
Commit c9ab107a authored by Michael Kozono's avatar Michael Kozono
Browse files
Options

Merge branch 'bw-followup-entity-leave-service' into 'master' 

Minor refactor to entity_leave_service.rb

See merge request gitlab-org/gitlab!44530
parents 9216e271 2df72783
Hide whitespace changes
Inline Side-by-side
Showing with 106 additions and 134 deletions
app/models/issue.rb
View file @ c9ab107a
...@@ -90,6 +90,7 @@ class Issue < ApplicationRecord ...@@ -90,6 +90,7 @@ class Issue < ApplicationRecord
alias_method :issuing_parent, :project alias_method :issuing_parent, :project
scope :in_projects, ->(project_ids) { where(project_id: project_ids) } scope :in_projects, ->(project_ids) { where(project_id: project_ids) }
scope :not_in_projects, ->(project_ids) { where.not(project_id: project_ids) }
scope :with_due_date, -> { where.not(due_date: nil) } scope :with_due_date, -> { where.not(due_date: nil) }
scope :without_due_date, -> { where(due_date: nil) } scope :without_due_date, -> { where(due_date: nil) }
......
app/models/issue_assignee.rb
View file @ c9ab107a
...@@ -8,6 +8,7 @@ class IssueAssignee < ApplicationRecord ...@@ -8,6 +8,7 @@ class IssueAssignee < ApplicationRecord
scope :in_projects, ->(project_ids) { joins(:issue).where("issues.project_id in (?)", project_ids) } scope :in_projects, ->(project_ids) { joins(:issue).where("issues.project_id in (?)", project_ids) }
scope :on_issues, ->(issue_ids) { where(issue_id: issue_ids) } scope :on_issues, ->(issue_ids) { where(issue_id: issue_ids) }
scope :for_assignee, ->(user) { where(assignee: user) }
end end
IssueAssignee.prepend_if_ee('EE::IssueAssignee') IssueAssignee.prepend_if_ee('EE::IssueAssignee')
app/services/todos/destroy/entity_leave_service.rb
View file @ c9ab107a
...@@ -7,16 +7,14 @@ module Todos ...@@ -7,16 +7,14 @@ module Todos
attr_reader :user, :entity attr_reader :user, :entity
# rubocop: disable CodeReuse/ActiveRecord
def initialize(user_id, entity_id, entity_type) def initialize(user_id, entity_id, entity_type)
unless %w(Group Project).include?(entity_type) unless %w(Group Project).include?(entity_type)
raise ArgumentError.new("#{entity_type} is not an entity user can leave") raise ArgumentError.new("#{entity_type} is not an entity user can leave")
end end
@user = User.find_by(id: user_id) @user = UserFinder.new(user_id).find_by_id
@entity = entity_type.constantize.find_by(id: entity_id) @entity = entity_type.constantize.find_by(id: entity_id) # rubocop: disable CodeReuse/ActiveRecord
end end
# rubocop: enable CodeReuse/ActiveRecord
def execute def execute
return unless entity && user return unless entity && user
...@@ -42,34 +40,37 @@ module Todos ...@@ -42,34 +40,37 @@ module Todos
end end
end end
# rubocop: disable CodeReuse/ActiveRecord
def remove_confidential_issue_todos def remove_confidential_issue_todos
Todo.where( Todo
target_id: confidential_issues.select(:id), target_type: Issue.name, user_id: user.id .for_target(confidential_issues.select(:id))
).delete_all .for_type(Issue.name)
.for_user(user)
.delete_all
end end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def remove_project_todos def remove_project_todos
# Issues are viewable by guests (even in private projects), so remove those todos # Issues are viewable by guests (even in private projects), so remove those todos
# from projects without guest access # from projects without guest access
Todo.where(project_id: non_authorized_guest_projects, user_id: user.id) Todo
.for_project(non_authorized_guest_projects)
.for_user(user)
.delete_all .delete_all
# MRs require reporter access, so remove those todos that are not authorized # MRs require reporter access, so remove those todos that are not authorized
Todo.where(project_id: non_authorized_reporter_projects, target_type: MergeRequest.name, user_id: user.id) Todo
.for_project(non_authorized_reporter_projects)
.for_type(MergeRequest.name)
.for_user(user)
.delete_all .delete_all
end end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def remove_group_todos def remove_group_todos
Todo.where(group_id: non_authorized_groups, user_id: user.id).delete_all Todo
.for_group(non_authorized_groups)
.for_user(user)
.delete_all
end end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def projects def projects
condition = case entity condition = case entity
when Project when Project
...@@ -78,55 +79,40 @@ module Todos ...@@ -78,55 +79,40 @@ module Todos
{ namespace_id: non_authorized_reporter_groups } { namespace_id: non_authorized_reporter_groups }
end end
Project.where(condition) Project.where(condition) # rubocop: disable CodeReuse/ActiveRecord
end end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def authorized_reporter_projects def authorized_reporter_projects
user.authorized_projects(Gitlab::Access::REPORTER).select(:id) user.authorized_projects(Gitlab::Access::REPORTER).select(:id)
end end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def authorized_guest_projects def authorized_guest_projects
user.authorized_projects(Gitlab::Access::GUEST).select(:id) user.authorized_projects(Gitlab::Access::GUEST).select(:id)
end end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def non_authorized_reporter_projects def non_authorized_reporter_projects
projects.where('id NOT IN (?)', authorized_reporter_projects) projects.id_not_in(authorized_reporter_projects)
end end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def non_authorized_guest_projects def non_authorized_guest_projects
projects.where('id NOT IN (?)', authorized_guest_projects) projects.id_not_in(authorized_guest_projects)
end end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def authorized_reporter_groups def authorized_reporter_groups
GroupsFinder.new(user, min_access_level: Gitlab::Access::REPORTER).execute.select(:id) GroupsFinder.new(user, min_access_level: Gitlab::Access::REPORTER).execute.select(:id)
end end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def non_authorized_groups def non_authorized_groups
return [] unless entity.is_a?(Namespace) return [] unless entity.is_a?(Namespace)
entity.self_and_descendants.select(:id) entity.self_and_descendants.select(:id)
.where('id NOT IN (?)', GroupsFinder.new(user).execute.select(:id)) .id_not_in(GroupsFinder.new(user).execute.select(:id))
end end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def non_authorized_reporter_groups def non_authorized_reporter_groups
entity.self_and_descendants.select(:id) entity.self_and_descendants.select(:id)
.where('id NOT IN (?)', authorized_reporter_groups) .id_not_in(authorized_reporter_groups)
end end
# rubocop: enable CodeReuse/ActiveRecord
def user_has_reporter_access? def user_has_reporter_access?
return unless entity.is_a?(Namespace) return unless entity.is_a?(Namespace)
...@@ -134,16 +120,16 @@ module Todos ...@@ -134,16 +120,16 @@ module Todos
entity.member?(User.find(user.id), Gitlab::Access::REPORTER) entity.member?(User.find(user.id), Gitlab::Access::REPORTER)
end end
# rubocop: disable CodeReuse/ActiveRecord
def confidential_issues def confidential_issues
assigned_ids = IssueAssignee.select(:issue_id).where(user_id: user.id) assigned_ids = IssueAssignee.select(:issue_id).for_assignee(user)
Issue.where(project_id: projects, confidential: true) Issue
.where('project_id NOT IN(?)', authorized_reporter_projects) .in_projects(projects)
.where('author_id != ?', user.id) .confidential_only
.where('id NOT IN (?)', assigned_ids) .not_in_projects(authorized_reporter_projects)
.not_authored_by(user)
.id_not_in(assigned_ids)
end end
# rubocop: enable CodeReuse/ActiveRecord
end end
end end
end end
spec/services/todos/destroy/entity_leave_service_spec.rb
View file @ c9ab107a
...@@ -19,20 +19,14 @@ RSpec.describe Todos::Destroy::EntityLeaveService do ...@@ -19,20 +19,14 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
let!(:todo_issue_c_user) { create(:todo, user: user, target: issue_c, project: project) } let!(:todo_issue_c_user) { create(:todo, user: user, target: issue_c, project: project) }
let!(:todo_issue_c_user2) { create(:todo, user: user2, target: issue_c, project: project) } let!(:todo_issue_c_user2) { create(:todo, user: user2, target: issue_c, project: project) }
shared_examples 'using different access permissions' do |access_table| shared_examples 'using different access permissions' do
using RSpec::Parameterized::TableSyntax before do
set_access(project, user, project_access) if project_access
where(:group_access, :project_access, :c_todos, :mr_todos, :method, &access_table) set_access(group, user, group_access) if group_access
end
with_them do
before do
set_access(project, user, project_access) if project_access
set_access(group, user, group_access) if group_access
end
it "#{params[:method].to_s.humanize(capitalize: false)}" do it "#{params[:method].to_s.humanize(capitalize: false)}" do
send(method) send(method_name)
end
end end
end end
...@@ -84,22 +78,20 @@ RSpec.describe Todos::Destroy::EntityLeaveService do ...@@ -84,22 +78,20 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
end end
context 'access permissions' do context 'access permissions' do
# rubocop:disable RSpec/LeakyConstantDeclaration where(:group_access, :project_access, :method_name) do
PRIVATE_PROJECT_PRIVATE_GROUP_ACCESS_TABLE = [
lambda do |_| [nil, :reporter, :does_not_remove_any_todos],
[ [nil, :guest, :removes_confidential_issues_and_merge_request_todos],
# :group_access, :project_access, :c_todos, :mr_todos, :method [:reporter, nil, :does_not_remove_any_todos],
[nil, :reporter, :keep, :keep, :does_not_remove_any_todos], [:guest, nil, :removes_confidential_issues_and_merge_request_todos],
[nil, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], [:guest, :reporter, :does_not_remove_any_todos],
[:reporter, nil, :keep, :keep, :does_not_remove_any_todos], [:guest, :guest, :removes_confidential_issues_and_merge_request_todos]
[:guest, nil, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], ]
[:guest, :reporter, :keep, :keep, :does_not_remove_any_todos], end
[:guest, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos]
]
end
# rubocop:enable RSpec/LeakyConstantDeclaration
it_behaves_like 'using different access permissions', PRIVATE_PROJECT_PRIVATE_GROUP_ACCESS_TABLE with_them do
it_behaves_like 'using different access permissions'
end
end end
end end
...@@ -117,22 +109,20 @@ RSpec.describe Todos::Destroy::EntityLeaveService do ...@@ -117,22 +109,20 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
end end
context 'access permissions' do context 'access permissions' do
# rubocop:disable RSpec/LeakyConstantDeclaration where(:group_access, :project_access, :method_name) do
PRIVATE_PROJECT_INTERNAL_GROUP_ACCESS_TABLE = [
lambda do |_| [nil, :reporter, :does_not_remove_any_todos],
[ [nil, :guest, :removes_confidential_issues_and_merge_request_todos],
# :group_access, :project_access, :c_todos, :mr_todos, :method [:reporter, nil, :does_not_remove_any_todos],
[nil, :reporter, :keep, :keep, :does_not_remove_any_todos], [:guest, nil, :removes_confidential_issues_and_merge_request_todos],
[nil, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], [:guest, :reporter, :does_not_remove_any_todos],
[:reporter, nil, :keep, :keep, :does_not_remove_any_todos], [:guest, :guest, :removes_confidential_issues_and_merge_request_todos]
[:guest, nil, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], ]
[:guest, :reporter, :keep, :keep, :does_not_remove_any_todos], end
[:guest, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos]
]
end
# rubocop:enable RSpec/LeakyConstantDeclaration
it_behaves_like 'using different access permissions', PRIVATE_PROJECT_INTERNAL_GROUP_ACCESS_TABLE with_them do
it_behaves_like 'using different access permissions'
end
end end
end end
...@@ -172,22 +162,20 @@ RSpec.describe Todos::Destroy::EntityLeaveService do ...@@ -172,22 +162,20 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
end end
context 'access permissions' do context 'access permissions' do
# rubocop:disable RSpec/LeakyConstantDeclaration where(:group_access, :project_access, :method_name) do
INTERNAL_PROJECT_INTERNAL_GROUP_ACCESS_TABLE = [
lambda do |_| [nil, :reporter, :does_not_remove_any_todos],
[ [nil, :guest, :removes_only_confidential_issues_todos],
# :group_access, :project_access, :c_todos, :mr_todos, :method [:reporter, nil, :does_not_remove_any_todos],
[nil, :reporter, :keep, :keep, :does_not_remove_any_todos], [:guest, nil, :removes_only_confidential_issues_todos],
[nil, :guest, :delete, :keep, :removes_only_confidential_issues_todos], [:guest, :reporter, :does_not_remove_any_todos],
[:reporter, nil, :keep, :keep, :does_not_remove_any_todos], [:guest, :guest, :removes_only_confidential_issues_todos]
[:guest, nil, :delete, :keep, :removes_only_confidential_issues_todos], ]
[:guest, :reporter, :keep, :keep, :does_not_remove_any_todos], end
[:guest, :guest, :delete, :keep, :removes_only_confidential_issues_todos]
] with_them do
end it_behaves_like 'using different access permissions'
# rubocop:enable RSpec/LeakyConstantDeclaration end
it_behaves_like 'using different access permissions', INTERNAL_PROJECT_INTERNAL_GROUP_ACCESS_TABLE
end end
end end
...@@ -219,22 +207,20 @@ RSpec.describe Todos::Destroy::EntityLeaveService do ...@@ -219,22 +207,20 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
end end
context 'access permissions' do context 'access permissions' do
# rubocop:disable RSpec/LeakyConstantDeclaration where(:group_access, :project_access, :method_name) do
PRIVATE_GROUP_PRIVATE_PROJECT_ACCESS_TABLE = [
lambda do |_| [nil, :reporter, :does_not_remove_any_todos],
[ [nil, :guest, :removes_confidential_issues_and_merge_request_todos],
# :group_access, :project_access, :c_todos, :mr_todos, :method [:reporter, nil, :does_not_remove_any_todos],
[nil, :reporter, :keep, :keep, :does_not_remove_any_todos], [:guest, nil, :removes_confidential_issues_and_merge_request_todos],
[nil, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], [:guest, :reporter, :does_not_remove_any_todos],
[:reporter, nil, :keep, :keep, :does_not_remove_any_todos], [:guest, :guest, :removes_confidential_issues_and_merge_request_todos]
[:guest, nil, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], ]
[:guest, :reporter, :keep, :keep, :does_not_remove_any_todos], end
[:guest, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos]
]
end
# rubocop:enable RSpec/LeakyConstantDeclaration
it_behaves_like 'using different access permissions', PRIVATE_GROUP_PRIVATE_PROJECT_ACCESS_TABLE with_them do
it_behaves_like 'using different access permissions'
end
end end
context 'with nested groups' do context 'with nested groups' do
...@@ -320,23 +306,21 @@ RSpec.describe Todos::Destroy::EntityLeaveService do ...@@ -320,23 +306,21 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
end end
context 'access permissions' do context 'access permissions' do
# rubocop:disable RSpec/LeakyConstantDeclaration where(:group_access, :project_access, :method_name) do
INTERNAL_GROUP_INTERNAL_PROJECT_ACCESS_TABLE = [
lambda do |_| [nil, nil, :removes_only_confidential_issues_todos],
[ [nil, :reporter, :does_not_remove_any_todos],
# :group_access, :project_access, :c_todos, :mr_todos, :method [nil, :guest, :removes_only_confidential_issues_todos],
[nil, nil, :delete, :keep, :removes_only_confidential_issues_todos], [:reporter, nil, :does_not_remove_any_todos],
[nil, :reporter, :keep, :keep, :does_not_remove_any_todos], [:guest, nil, :removes_only_confidential_issues_todos],
[nil, :guest, :delete, :keep, :removes_only_confidential_issues_todos], [:guest, :reporter, :does_not_remove_any_todos],
[:reporter, nil, :keep, :keep, :does_not_remove_any_todos], [:guest, :guest, :removes_only_confidential_issues_todos]
[:guest, nil, :delete, :keep, :removes_only_confidential_issues_todos], ]
[:guest, :reporter, :keep, :keep, :does_not_remove_any_todos], end
[:guest, :guest, :delete, :keep, :removes_only_confidential_issues_todos]
]
end
# rubocop:enable RSpec/LeakyConstantDeclaration
it_behaves_like 'using different access permissions', INTERNAL_GROUP_INTERNAL_PROJECT_ACCESS_TABLE with_them do
it_behaves_like 'using different access permissions'
end
end end
end end
end end
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7