From bdc8396e25e6eba6edcf2896daa49bb49695ef8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jarka=20Kadlecov=C3=A1?= Date: Fri, 27 Jul 2018 11:28:17 +0200 Subject: Remove todos when project feature visibility changes --- .../todos/destroy/entity_leave_service_spec.rb | 71 +++++----- .../todos/destroy/private_features_service_spec.rb | 143 +++++++++++++++++++++ 2 files changed, 184 insertions(+), 30 deletions(-) create mode 100644 spec/services/todos/destroy/private_features_service_spec.rb (limited to 'spec/services/todos/destroy') diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb index 52175ed9032..bad408a314e 100644 --- a/spec/services/todos/destroy/entity_leave_service_spec.rb +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -1,38 +1,39 @@ require 'spec_helper' describe Todos::Destroy::EntityLeaveService do - let(:group) { create(:group, :private) } - let(:project) { create(:project, group: group) } - let(:user) { create(:user) } - let(:project_member) { create(:user) } - let(:issue) { create(:issue, :confidential, project: project) } + let(:group) { create(:group, :private) } + let(:project) { create(:project, group: group) } + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:issue) { create(:issue, project: project) } + let(:mr) { create(:merge_request, source_project: project) } - let!(:todo_non_member) { create(:todo, user: user, project: project) } - let!(:todo_conf_issue_non_member) { create(:todo, user: user, target: issue, project: project) } - let!(:todo_conf_issue_member) { create(:todo, user: project_member, target: issue, project: project) } + let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) } + let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_user2) { create(:todo, user: user2, target: issue, project: project) } describe '#execute' do - before do - project.add_developer(project_member) - end - context 'when a user leaves a project' do subject { described_class.new(user.id, project.id, 'Project').execute } context 'when project is private' do - it 'removes todos for a user who is not a member' do + it 'removes todos for the provided user' do expect { subject }.to change { Todo.count }.from(3).to(1) expect(user.todos).to be_empty - expect(project_member.todos).to match_array([todo_conf_issue_member]) + expect(user2.todos).to match_array([todo_issue_user2]) end end context 'when project is not private' do + before do + group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end + context 'when a user is not an author of confidential issue' do before do - group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + issue.update!(confidential: true) end it 'removes only confidential issues todos' do @@ -42,10 +43,7 @@ describe Todos::Destroy::EntityLeaveService do context 'when a user is an author of confidential issue' do before do - issue.update!(author: user) - - group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + issue.update!(author: user, confidential: true) end it 'removes only confidential issues todos' do @@ -55,16 +53,26 @@ describe Todos::Destroy::EntityLeaveService do context 'when a user is an assignee of confidential issue' do before do + issue.update!(confidential: true) issue.assignees << user - - group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) end it 'removes only confidential issues todos' do expect { subject }.not_to change { Todo.count } end end + + context 'feature visibility check' do + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only users issue todos' do + expect { subject }.to change { Todo.count }.from(3).to(2) + end + end + end end end @@ -72,33 +80,36 @@ describe Todos::Destroy::EntityLeaveService do subject { described_class.new(user.id, group.id, 'Group').execute } context 'when group is private' do - it 'removes todos for a user who is not a member' do + it 'removes todos for the user' do expect { subject }.to change { Todo.count }.from(3).to(1) expect(user.todos).to be_empty - expect(project_member.todos).to match_array([todo_conf_issue_member]) + expect(user2.todos).to match_array([todo_issue_user2]) end context 'with nested groups', :nested_groups do let(:subgroup) { create(:group, :private, parent: group) } let(:subproject) { create(:project, group: subgroup) } - let!(:todo_subproject_non_member) { create(:todo, user: user, project: subproject) } - let!(:todo_subproject_member) { create(:todo, user: project_member, project: subproject) } + let!(:todo_subproject_user) { create(:todo, user: user, project: subproject) } + let!(:todo_subproject_user2) { create(:todo, user: user2, project: subproject) } - it 'removes todos for a user who is not a member' do + it 'removes todos for the user including subprojects todos' do expect { subject }.to change { Todo.count }.from(5).to(2) expect(user.todos).to be_empty - expect(project_member.todos) - .to match_array([todo_conf_issue_member, todo_subproject_member]) + expect(user2.todos) + .to match_array([todo_issue_user2, todo_subproject_user2]) end end end context 'when group is not private' do before do + issue.update!(confidential: true) + group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) end it 'removes only confidential issues todos' do diff --git a/spec/services/todos/destroy/private_features_service_spec.rb b/spec/services/todos/destroy/private_features_service_spec.rb new file mode 100644 index 00000000000..be8b5bb3979 --- /dev/null +++ b/spec/services/todos/destroy/private_features_service_spec.rb @@ -0,0 +1,143 @@ +require 'spec_helper' + +describe Todos::Destroy::PrivateFeaturesService do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:another_user) { create(:user) } + let(:project_member) { create(:user) } + let(:issue) { create(:issue, project: project) } + let(:mr) { create(:merge_request, source_project: project) } + + let!(:todo_mr_non_member) { create(:todo, user: user, target: mr, project: project) } + let!(:todo_mr_non_member2) { create(:todo, user: another_user, target: mr, project: project) } + let!(:todo_mr_member) { create(:todo, user: project_member, target: mr, project: project) } + let!(:todo_issue_non_member) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_non_member2) { create(:todo, user: another_user, target: issue, project: project) } + let!(:todo_issue_member) { create(:todo, user: project_member, target: issue, project: project) } + let!(:commit_todo_non_member) { create(:on_commit_todo, user: user, project: project) } + let!(:commit_todo_non_member2) { create(:on_commit_todo, user: another_user, project: project) } + let!(:commit_todo_member) { create(:on_commit_todo, user: project_member, project: project) } + + before do + project.add_developer(project_member) + end + + context 'when user_id is provided' do + subject { described_class.new(project.id, user.id).execute } + + context 'when all feaures have same visibility as the project' do + it 'removes only user issue todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members but the user is a member' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.add_developer(user) + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(8) + end + end + + context 'when mrs, builds and repository are visible only to project members' do + before do + # builds and merge requests cannot have higher visibility than repository + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(repository_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user mr and commit todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs are visible only to project members' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user merge request todo' do + expect { subject }.to change { Todo.count }.from(9).to(8) + end + end + + context 'when mrs and issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user merge request and issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + end + + context 'when user_id is not provided' do + subject { described_class.new(project.id).execute } + + context 'when all feaures have same visibility as the project' do + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs, builds and repository are visible only to project members' do + before do + # builds and merge requests cannot have higher visibility than repository + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(repository_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members mr and commit todos' do + expect { subject }.to change { Todo.count }.from(9).to(5) + end + end + + context 'when mrs are visible only to project members' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members merge request todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs and issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members merge request and issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(5) + end + end + end +end -- cgit v1.2.1