Commit 72154b41 authored by Eugenia Grieff's avatar Eugenia Grieff

Review feedback

parent d690225d
...@@ -37,6 +37,10 @@ module EpicIssues ...@@ -37,6 +37,10 @@ module EpicIssues
@linkable_issues ||= begin @linkable_issues ||= begin
return [] unless can?(current_user, :read_epic, issuable.group) return [] unless can?(current_user, :read_epic, issuable.group)
Preloaders::UserMaxAccessLevelInProjectsPreloader
.new(issues.map(&:project).compact, current_user)
.execute
issues.select do |issue| issues.select do |issue|
linkable_issue?(issue) linkable_issue?(issue)
end end
......
...@@ -818,15 +818,6 @@ RSpec.describe Issue do ...@@ -818,15 +818,6 @@ RSpec.describe Issue do
end end
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
context 'when a user can not read epic' do
before do
allow(Ability).to receive(:allowed?).with(user, :read_epic, group).and_return(false)
allow(Ability).to receive(:allowed?).with(user, :admin_issue, project).and_call_original
end
it { is_expected.to be_falsey }
end
end end
end end
......
...@@ -38,33 +38,14 @@ RSpec.describe 'Reposition and move issue within board lists' do ...@@ -38,33 +38,14 @@ RSpec.describe 'Reposition and move issue within board lists' do
context 'when user can admin issue' do context 'when user can admin issue' do
before do before do
project.add_reporter(user) project.add_reporter(user)
group.add_guest(user)
end end
it 'updates issue position and epic' do context 'when user can read epic' do
post_graphql_mutation(mutation(params), current_user: user) before do
group.add_guest(user)
expect(response).to have_gitlab_http_status(:success)
response_issue = graphql_mutation_response(:issue_move_list)['issue']
expect(response_issue['iid']).to eq(issue1.iid.to_s)
expect(response_issue['relativePosition']).to be > existing_issue1.relative_position
expect(response_issue['relativePosition']).to be < existing_issue2.relative_position
expect(response_issue['epic']['id']).to eq(epic.to_global_id.to_s)
expect(response_issue['labels']['nodes'].first['title']).to eq(testing.title)
end
context 'when user sets nil epic' do
let_it_be(:epic_issue) { create(:epic_issue, issue: issue1, epic: epic) }
let(:issue_move_params) do
{
epic_id: nil,
move_before_id: existing_issue2.id,
move_after_id: existing_issue1.id
}
end end
it 'updates issue position and epic is unassigned' do it 'updates issue position and epic' do
post_graphql_mutation(mutation(params), current_user: user) post_graphql_mutation(mutation(params), current_user: user)
expect(response).to have_gitlab_http_status(:success) expect(response).to have_gitlab_http_status(:success)
...@@ -72,7 +53,31 @@ RSpec.describe 'Reposition and move issue within board lists' do ...@@ -72,7 +53,31 @@ RSpec.describe 'Reposition and move issue within board lists' do
expect(response_issue['iid']).to eq(issue1.iid.to_s) expect(response_issue['iid']).to eq(issue1.iid.to_s)
expect(response_issue['relativePosition']).to be > existing_issue1.relative_position expect(response_issue['relativePosition']).to be > existing_issue1.relative_position
expect(response_issue['relativePosition']).to be < existing_issue2.relative_position expect(response_issue['relativePosition']).to be < existing_issue2.relative_position
expect(response_issue['epic']).to be_nil expect(response_issue['epic']['id']).to eq(epic.to_global_id.to_s)
expect(response_issue['labels']['nodes'].first['title']).to eq(testing.title)
end
context 'when user sets nil epic' do
let_it_be(:epic_issue) { create(:epic_issue, issue: issue1, epic: epic) }
let(:issue_move_params) do
{
epic_id: nil,
move_before_id: existing_issue2.id,
move_after_id: existing_issue1.id
}
end
it 'updates issue position and epic is unassigned' do
post_graphql_mutation(mutation(params), current_user: user)
expect(response).to have_gitlab_http_status(:success)
response_issue = graphql_mutation_response(:issue_move_list)['issue']
expect(response_issue['iid']).to eq(issue1.iid.to_s)
expect(response_issue['relativePosition']).to be > existing_issue1.relative_position
expect(response_issue['relativePosition']).to be < existing_issue2.relative_position
expect(response_issue['epic']).to be_nil
end
end end
end end
...@@ -85,6 +90,8 @@ RSpec.describe 'Reposition and move issue within board lists' do ...@@ -85,6 +90,8 @@ RSpec.describe 'Reposition and move issue within board lists' do
response_issue = graphql_mutation_response(:issue_move_list)['issue'] response_issue = graphql_mutation_response(:issue_move_list)['issue']
expect(response_issue['epic']).to be_nil expect(response_issue['epic']).to be_nil
expect(response_issue['relativePosition']).to eq(3)
expect(graphql_mutation_response(:issue_move_list)['errors']).to include('Resource not found')
end end
end end
end end
......
...@@ -130,7 +130,7 @@ RSpec.describe EpicIssues::CreateService do ...@@ -130,7 +130,7 @@ RSpec.describe EpicIssues::CreateService do
include_examples 'returns success' include_examples 'returns success'
it 'does not perform N + 1 queries' do it 'does not perform N + 1 queries', :request_store do
allow(SystemNoteService).to receive(:epic_issue) allow(SystemNoteService).to receive(:epic_issue)
allow(SystemNoteService).to receive(:issue_on_epic) allow(SystemNoteService).to receive(:issue_on_epic)
...@@ -160,10 +160,9 @@ RSpec.describe EpicIssues::CreateService do ...@@ -160,10 +160,9 @@ RSpec.describe EpicIssues::CreateService do
# threshold 24 because 6 queries are generated for each insert # threshold 24 because 6 queries are generated for each insert
# (savepoint, find, exists, relative_position get, insert, release savepoint) # (savepoint, find, exists, relative_position get, insert, release savepoint)
# and we insert 5 issues instead of 1 which we do for control count # and we insert 5 issues instead of 1 which we do for control count
# plus 4 to include additional authorization queries "SELECT MAX("project_authorizations"."access_level")"
expect { described_class.new(epic, user, params).execute } expect { described_class.new(epic, user, params).execute }
.not_to exceed_query_limit(control_count) .not_to exceed_query_limit(control_count)
.with_threshold(33) .with_threshold(29)
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