Skip to content

G
gitlab-ce
Commit 2df72783 authored by Brett Walker's avatar Brett Walker
Browse files
Options

Refactor of entity_leave_service 

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