Commit 45a591c4 authored by Jarka Kadlecova's avatar Jarka Kadlecova

Refactor code based on MR comments

parent 2fc79403
class CreateEpicIssuesTable < ActiveRecord::Migration class CreateEpicIssuesTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false DOWNTIME = false
disable_ddl_transaction! disable_ddl_transaction!
......
...@@ -735,8 +735,6 @@ ActiveRecord::Schema.define(version: 20171107144726) do ...@@ -735,8 +735,6 @@ ActiveRecord::Schema.define(version: 20171107144726) do
create_table "epic_issues", force: :cascade do |t| create_table "epic_issues", force: :cascade do |t|
t.integer "epic_id", null: false t.integer "epic_id", null: false
t.integer "issue_id", null: false t.integer "issue_id", null: false
t.datetime_with_timezone "created_at"
t.datetime_with_timezone "updated_at"
end end
add_index "epic_issues", ["epic_id"], name: "index_epic_issues_on_epic_id", using: :btree add_index "epic_issues", ["epic_id"], name: "index_epic_issues_on_epic_id", using: :btree
...@@ -2403,8 +2401,8 @@ ActiveRecord::Schema.define(version: 20171107144726) do ...@@ -2403,8 +2401,8 @@ ActiveRecord::Schema.define(version: 20171107144726) do
add_foreign_key "deploy_keys_projects", "projects", name: "fk_58a901ca7e", on_delete: :cascade add_foreign_key "deploy_keys_projects", "projects", name: "fk_58a901ca7e", on_delete: :cascade
add_foreign_key "deployments", "projects", name: "fk_b9a3851b82", on_delete: :cascade add_foreign_key "deployments", "projects", name: "fk_b9a3851b82", on_delete: :cascade
add_foreign_key "environments", "projects", name: "fk_d1c8c1da6a", on_delete: :cascade add_foreign_key "environments", "projects", name: "fk_d1c8c1da6a", on_delete: :cascade
add_foreign_key "epic_issues", "epics" add_foreign_key "epic_issues", "epics", on_delete: :cascade
add_foreign_key "epic_issues", "issues" add_foreign_key "epic_issues", "issues", on_delete: :cascade
add_foreign_key "epic_metrics", "epics", on_delete: :cascade add_foreign_key "epic_metrics", "epics", on_delete: :cascade
add_foreign_key "epics", "milestones", on_delete: :nullify add_foreign_key "epics", "milestones", on_delete: :nullify
add_foreign_key "epics", "namespaces", column: "group_id", name: "fk_f081aa4489", on_delete: :cascade add_foreign_key "epics", "namespaces", column: "group_id", name: "fk_f081aa4489", on_delete: :cascade
......
...@@ -17,7 +17,7 @@ module EpicIssues ...@@ -17,7 +17,7 @@ module EpicIssues
end end
def linkable_issues(issues) def linkable_issues(issues)
return [] unless can?(current_user, :admin_epic, issuable.group) return [] unless can?(current_user, :admin_epic, issuable.group)
issues.select { |issue| issue.project.group == issuable.group } issues.select { |issue| issue.project.group == issuable.group }
end end
......
...@@ -46,7 +46,7 @@ describe Groups::EpicIssuesController do ...@@ -46,7 +46,7 @@ describe Groups::EpicIssuesController do
post :create, group_id: group, epic_id: epic.to_param, issue_references: reference post :create, group_id: group, epic_id: epic.to_param, issue_references: reference
end end
context 'when user has permissions to create requested associtaion' do context 'when user has permissions to create requested association' do
before do before do
group.add_developer(user) group.add_developer(user)
end end
...@@ -64,7 +64,7 @@ describe Groups::EpicIssuesController do ...@@ -64,7 +64,7 @@ describe Groups::EpicIssuesController do
end end
end end
context 'when user does not have permissions to create requested associtaion' do context 'when user does not have permissions to create requested association' do
it 'returns correct response for the correct issue reference' do it 'returns correct response for the correct issue reference' do
subject subject
...@@ -134,7 +134,7 @@ describe Groups::EpicIssuesController do ...@@ -134,7 +134,7 @@ describe Groups::EpicIssuesController do
end end
end end
context 'when the epic_issue record does not exixst' do context 'when the epic_issue record does not exists' do
it 'returns status 404' do it 'returns status 404' do
delete :destroy, group_id: group, epic_id: epic.to_param, id: 9999 delete :destroy, group_id: group, epic_id: epic.to_param, id: 9999
......
...@@ -40,22 +40,22 @@ describe Epic do ...@@ -40,22 +40,22 @@ describe Epic do
] ]
end end
subject { epic.issues(user) } let(:result) { epic.issues(user) }
it 'returns all issues if a user has access to them' do it 'returns all issues if a user has access to them' do
group.add_developer(user) group.add_developer(user)
expect(subject.count).to eq(2) expect(result.count).to eq(2)
expect(subject.map(&:id)).to match_array([issue.id, other_issue.id]) expect(result.map(&:id)).to match_array([issue.id, other_issue.id])
expect(subject.map(&:epic_issue_id)).to match_array(epic_issues.map(&:id)) expect(result.map(&:epic_issue_id)).to match_array(epic_issues.map(&:id))
end end
it 'does not return issues user can not see' do it 'does not return issues user can not see' do
project.add_developer(user) project.add_developer(user)
expect(subject.count).to eq(1) expect(result.count).to eq(1)
expect(subject.map(&:id)).to match_array([issue.id]) expect(result.map(&:id)).to match_array([issue.id])
expect(subject.map(&:epic_issue_id)).to match_array([epic_issues.first.id]) expect(result.map(&:epic_issue_id)).to match_array([epic_issues.first.id])
end end
end end
end end
...@@ -59,6 +59,22 @@ describe EpicIssues::CreateService do ...@@ -59,6 +59,22 @@ describe EpicIssues::CreateService do
subject { assign_issue([valid_reference]) } subject { assign_issue([valid_reference]) }
include_examples 'returns success' include_examples 'returns success'
it 'does not perofrm N + 1 queries' do
params = { issue_references: [valid_reference] }
control_count = ActiveRecord::QueryRecorder.new { described_class.new(epic, user, params).execute }.count
user = create(:user)
group = create(:group)
project = create(:project, group: group)
issues = create_list(:issue, 5, project: project)
epic = create(:epic, group: group)
group.add_developer(user)
params = { issue_references: issues.map { |i| i.to_reference(full: true) } }
expect { described_class.new(epic, user, params).execute }.not_to exceed_query_limit(control_count)
end
end end
context 'when an issue links is given' do context 'when an issue links is given' do
......
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