Commit dc2d1359 authored by Etienne Baqué's avatar Etienne Baqué

Fixed code based on review comments

Used existing scope in issuable concern file.
Cleaned up issuable spec file.
parent 0f7a098b
...@@ -578,8 +578,9 @@ class IssuableFinder ...@@ -578,8 +578,9 @@ class IssuableFinder
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def by_release(items) def by_release(items)
if releases? return items unless releases?
items = if filter_by_no_release?
if filter_by_no_release?
items.without_release items.without_release
elsif filter_by_any_release? elsif filter_by_any_release?
items.any_release items.any_release
...@@ -588,9 +589,6 @@ class IssuableFinder ...@@ -588,9 +589,6 @@ class IssuableFinder
end end
end end
items
end
def filter_by_no_milestone? def filter_by_no_milestone?
# Accepts `No Milestone` for compatibility # Accepts `No Milestone` for compatibility
params[:milestone_title].to_s.downcase == FILTER_NONE || params[:milestone_title] == Milestone::None.title params[:milestone_title].to_s.downcase == FILTER_NONE || params[:milestone_title] == Milestone::None.title
......
...@@ -124,18 +124,16 @@ module Issuable ...@@ -124,18 +124,16 @@ module Issuable
scope :order_milestone_due_asc, -> { left_joins_milestones.reorder(Arel.sql('milestones.due_date IS NULL, milestones.id IS NULL, milestones.due_date ASC')) } scope :order_milestone_due_asc, -> { left_joins_milestones.reorder(Arel.sql('milestones.due_date IS NULL, milestones.id IS NULL, milestones.due_date ASC')) }
scope :without_release, -> do scope :without_release, -> do
joins("LEFT OUTER JOIN milestones AS ms ON #{table_name}.milestone_id = ms.id left_joins_milestones
LEFT OUTER JOIN milestone_releases AS ms_rl ON ms.id = ms_rl.milestone_id" .joins("LEFT OUTER JOIN milestone_releases ON milestones.id = milestone_releases.milestone_id")
).where('ms_rl.release_id IS NULL') .where('milestone_releases.release_id IS NULL')
end end
scope :left_joins_milestones_joins_releases, -> do scope :left_joins_milestones_joins_releases, -> do
joins("LEFT OUTER JOIN milestones AS ms left_joins_milestones
JOIN milestone_releases .joins("JOIN milestone_releases ON milestones.id = milestone_releases.milestone_id
JOIN releases ON milestone_releases.release_id = releases.id JOIN releases ON milestone_releases.release_id = releases.id")
ON ms.id = milestone_releases.milestone_id .where('milestone_releases.release_id IS NOT NULL').distinct
ON #{table_name}.milestone_id = ms.id"
).where('milestone_releases.release_id IS NOT NULL').distinct
end end
scope :without_label, -> { joins("LEFT OUTER JOIN label_links ON label_links.target_type = '#{name}' AND label_links.target_id = #{table_name}.id").where(label_links: { id: nil }) } scope :without_label, -> { joins("LEFT OUTER JOIN label_links ON label_links.target_type = '#{name}' AND label_links.target_id = #{table_name}.id").where(label_links: { id: nil }) }
......
...@@ -853,35 +853,6 @@ describe Issuable do ...@@ -853,35 +853,6 @@ describe Issuable do
end end
end end
# Testing the release related scopes with the following setup:
# - release_1
# - milestone_1
# - issue_1
# - issue_2
# - milestone_2
# - issue_3
#
# - release_2
# - milestone_2
# - issue_3
# - milestone_3
# - (no issue)
#
# - release_3
# - milestone_4
# - issue_4
# - milestone_5
# - issue_4
#
# - release_4
# - milestone_3
# - (no issue)
#
# - milestone_6 (no release)
# - issue_5
#
# - issue_6 (no milestone, no release)
describe 'release scopes' do describe 'release scopes' do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
...@@ -890,35 +861,22 @@ describe Issuable do ...@@ -890,35 +861,22 @@ describe Issuable do
let_it_be(:release_3) { create(:release, tag: 'v3.0', project: project) } let_it_be(:release_3) { create(:release, tag: 'v3.0', project: project) }
let_it_be(:release_4) { create(:release, tag: 'v4.0', project: project) } let_it_be(:release_4) { create(:release, tag: 'v4.0', project: project) }
let_it_be(:milestone_1) { create(:milestone, title: 'm1', project: project) } let_it_be(:milestone_1) { create(:milestone, releases: [release_1], title: 'm1', project: project) }
let_it_be(:milestone_2) { create(:milestone, title: 'm2', project: project) } let_it_be(:milestone_2) { create(:milestone, releases: [release_1, release_2], title: 'm2', project: project) }
let_it_be(:milestone_3) { create(:milestone, title: 'm3', project: project) } let_it_be(:milestone_3) { create(:milestone, releases: [release_2, release_4], title: 'm3', project: project) }
let_it_be(:milestone_4) { create(:milestone, title: 'm4', project: project) } let_it_be(:milestone_4) { create(:milestone, releases: [release_3], title: 'm4', project: project) }
let_it_be(:milestone_5) { create(:milestone, title: 'm5', project: project) } let_it_be(:milestone_5) { create(:milestone, releases: [release_3], title: 'm5', project: project) }
let_it_be(:milestone_6) { create(:milestone, title: 'm6', project: project) } let_it_be(:milestone_6) { create(:milestone, title: 'm6', project: project) }
let_it_be(:issue_1) { create(:issue, project: project) } let_it_be(:issue_1) { create(:issue, milestone: milestone_1, project: project) }
let_it_be(:issue_2) { create(:issue, project: project) } let_it_be(:issue_2) { create(:issue, milestone: milestone_1, project: project) }
let_it_be(:issue_3) { create(:issue, project: project) } let_it_be(:issue_3) { create(:issue, milestone: milestone_2, project: project) }
let_it_be(:issue_4) { create(:issue, project: project) } let_it_be(:issue_4) { create(:issue, milestone: milestone_5, project: project) }
let_it_be(:issue_5) { create(:issue, project: project) } let_it_be(:issue_5) { create(:issue, milestone: milestone_6, project: project) }
let_it_be(:issue_6) { create(:issue, project: project) } let_it_be(:issue_6) { create(:issue, project: project) }
let_it_be(:items) { Issue.all } let_it_be(:items) { Issue.all }
before(:all) do
milestone_1.issues = [issue_1, issue_2]
milestone_2.issues = [issue_3]
milestone_4.issues = [issue_4]
milestone_5.issues = [issue_4]
milestone_6.issues = [issue_5]
release_1.milestones = [milestone_1, milestone_2]
release_2.milestones = [milestone_2, milestone_3]
release_3.milestones = [milestone_4, milestone_5]
release_4.milestones = [milestone_3]
end
describe '#without_release' do describe '#without_release' do
it 'returns the issues not tied to any milestone and the ones tied to milestone with no release' do it 'returns the issues not tied to any milestone and the ones tied to milestone with no release' do
expect(items.without_release).to contain_exactly(issue_5, issue_6) expect(items.without_release).to contain_exactly(issue_5, issue_6)
...@@ -954,19 +912,13 @@ describe Issuable do ...@@ -954,19 +912,13 @@ describe Issuable do
end end
end end
context 'when an issue is tied to multiple milestones under the same release' do
it 'returns this issue only once' do
expect(items.with_release('v3.0')).to contain_exactly(issue_4)
end
end
context 'when there is no issue under a specific release' do context 'when there is no issue under a specific release' do
it 'returns no issue' do it 'returns no issue' do
expect(items.with_release('v4.0')).to be_empty expect(items.with_release('v4.0')).to be_empty
end end
end end
context 'when a non-existant release tag is passed in' do context 'when a non-existent release tag is passed in' do
it 'returns no issue' do it 'returns no issue' do
expect(items.with_release('v999.0')).to be_empty expect(items.with_release('v999.0')).to be_empty
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