Skip to content

G
gitlab-ce
Commit dd4a66b1 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre
Browse files
Options

Merge branch '7145-simplify-date-sourcing-milestones-finder' into 'master' 

Simplify Epic::DateSourceMilestonesFinder

See merge request gitlab-org/gitlab-ee!8988
parents de97f66f 42ff624f
Hide whitespace changes
Inline Side-by-side
Showing with 60 additions and 182 deletions
ee/app/finders/epics/date_sourcing_milestones_finder.rb deleted 100644 → 0
View file @ de97f66f
# frozen_string_literal: true
module Epics
class DateSourcingMilestonesFinder
include Gitlab::Utils::StrongMemoize
FIELDS = [:id, :start_date, :due_date].freeze
ID_INDEX = FIELDS.index(:id)
START_DATE_INDEX = FIELDS.index(:start_date)
DUE_DATE_INDEX = FIELDS.index(:due_date)
def initialize(epic_id)
@epic_id = epic_id
end
# rubocop: disable CodeReuse/ActiveRecord
def execute
strong_memoize(:execute) do
Milestone.joins(issues: :epic_issue).where(epic_issues: { epic_id: epic_id }).joins(
<<~SQL
INNER JOIN (
SELECT MIN(milestones.start_date) AS start_date, MAX(milestones.due_date) AS due_date
FROM milestones
INNER JOIN issues ON issues.milestone_id = milestones.id
INNER JOIN epic_issues ON epic_issues.issue_id = issues.id
WHERE epic_issues.epic_id = #{epic_id}
) inner_results ON (inner_results.start_date = milestones.start_date OR inner_results.due_date = milestones.due_date)
SQL
).pluck(*FIELDS)
end
end
# rubocop: enable CodeReuse/ActiveRecord
def start_date
start_date_sourcing_milestone&.slice(START_DATE_INDEX)
end
def start_date_sourcing_milestone_id
start_date_sourcing_milestone&.slice(ID_INDEX)
end
def due_date
due_date_sourcing_milestone&.slice(DUE_DATE_INDEX)
end
def due_date_sourcing_milestone_id
due_date_sourcing_milestone&.slice(ID_INDEX)
end
private
attr_reader :epic_id
def start_date_sourcing_milestone
@start_date_sourcing_milestone ||= execute
.reject { |row| row[START_DATE_INDEX].nil? }
.min_by { |row| row[START_DATE_INDEX] }
end
def due_date_sourcing_milestone
@due_date_sourcing_milestone ||= execute
.reject { |row| row[DUE_DATE_INDEX].nil? }
.max_by { |row| row[DUE_DATE_INDEX] }
end
end
end
ee/app/models/ee/epic.rb
View file @ dd4a66b1
......@@ -127,36 +127,6 @@ module EE
::Group
end
def update_start_and_due_dates(epics)
groups = epics.includes(:issues).group_by do |epic|
milestone_ids = epic.issues.map(&:milestone_id)
milestone_ids.compact!
milestone_ids.uniq!
milestone_ids
end
groups.each do |milestone_ids, epics|
next if milestone_ids.empty?
results = Epics::DateSourcingMilestonesFinder.new(epics.first.id)
self.where(id: epics.map(&:id)).update_all(
[
%{
start_date = CASE WHEN start_date_is_fixed = true THEN start_date ELSE ? END,
start_date_sourcing_milestone_id = ?,
end_date = CASE WHEN due_date_is_fixed = true THEN end_date ELSE ? END,
due_date_sourcing_milestone_id = ?
},
results.start_date,
results.start_date_sourcing_milestone_id,
results.due_date,
results.due_date_sourcing_milestone_id
]
)
end
end
# Return the deepest relation level for an epic.
# Example 1:
# epic1 - parent: nil
......@@ -195,6 +165,48 @@ module EE
# So we sum 1 to the result to take into account first parent.
deepest_level + 1
end
def update_start_and_due_dates(epics)
# In MySQL, we cannot use a subquery that references the table being updated
epics = epics.pluck(:id) if ::Gitlab::Database.mysql? && epics.is_a?(ActiveRecord::Relation)
self.where(id: epics).update_all(
[
%{
start_date = CASE WHEN start_date_is_fixed = true THEN start_date_fixed ELSE (?) END,
start_date_sourcing_milestone_id = (?),
end_date = CASE WHEN due_date_is_fixed = true THEN due_date_fixed ELSE (?) END,
due_date_sourcing_milestone_id = (?)
},
start_date_milestone_query.select(:start_date),
start_date_milestone_query.select(:id),
due_date_milestone_query.select(:due_date),
due_date_milestone_query.select(:id)
]
)
end
private
def start_date_milestone_query
source_milestones_query
.where.not(start_date: nil)
.order(:start_date, :id)
.limit(1)
end
def due_date_milestone_query
source_milestones_query
.where.not(due_date: nil)
.order(due_date: :desc, id: :desc)
.limit(1)
end
def source_milestones_query
::Milestone
.joins(issues: :epic_issue)
.where('epic_issues.epic_id = epics.id')
end
end
def assignees
......@@ -227,14 +239,7 @@ module EE
alias_attribute(:due_date, :end_date)
def update_start_and_due_dates
results = Epics::DateSourcingMilestonesFinder.new(id)
self.start_date = start_date_is_fixed? ? start_date_fixed : results.start_date
self.start_date_sourcing_milestone_id = results.start_date_sourcing_milestone_id
self.due_date = due_date_is_fixed? ? due_date_fixed : results.due_date
self.due_date_sourcing_milestone_id = results.due_date_sourcing_milestone_id
save if changed?
self.class.update_start_and_due_dates([self])
end
def start_date_from_milestones
......
ee/app/services/epics/update_service.rb
View file @ dd4a66b1
......@@ -16,7 +16,10 @@ module Epics
update(epic)
epic.update_start_and_due_dates if have_epic_dates_changed?(epic)
if have_epic_dates_changed?(epic)
epic.update_start_and_due_dates
epic.reload
end
epic
end
......
ee/spec/finders/epics/date_sourcing_milestones_finder_spec.rb deleted 100644 → 0
View file @ de97f66f
# frozen_string_literal: true
require 'spec_helper'
describe Epics::DateSourcingMilestonesFinder do
describe '#execute' do
it 'returns date and id from milestones' do
epic = create(:epic)
milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10))
milestone2 = create(:milestone, due_date: Date.new(2000, 1, 30))
milestone3 = create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 20))
create(:issue, epic: epic, milestone: milestone1)
create(:issue, epic: epic, milestone: milestone2)
create(:issue, epic: epic, milestone: milestone3)
results = described_class.new(epic.id)
expect(results).to have_attributes(
start_date: milestone1.start_date,
start_date_sourcing_milestone_id: milestone1.id,
due_date: milestone2.due_date,
due_date_sourcing_milestone_id: milestone2.id
)
end
it 'returns date and id from single milestone' do
epic = create(:epic)
milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1), due_date: Date.new(2000, 1, 10))
create(:issue, epic: epic, milestone: milestone1)
results = described_class.new(epic.id)
expect(results).to have_attributes(
start_date: milestone1.start_date,
start_date_sourcing_milestone_id: milestone1.id,
due_date: milestone1.due_date,
due_date_sourcing_milestone_id: milestone1.id
)
end
it 'returns date and id from milestone without date' do
epic = create(:epic)
milestone1 = create(:milestone, start_date: Date.new(2000, 1, 1))
create(:issue, epic: epic, milestone: milestone1)
results = described_class.new(epic.id)
expect(results).to have_attributes(
start_date: milestone1.start_date,
start_date_sourcing_milestone_id: milestone1.id,
due_date: nil,
due_date_sourcing_milestone_id: nil
)
end
it 'handles epics without milestone' do
epic = create(:epic)
results = described_class.new(epic.id)
expect(results).to have_attributes(
start_date: nil,
start_date_sourcing_milestone_id: nil,
due_date: nil,
due_date_sourcing_milestone_id: nil
)
end
end
end
ee/spec/models/epic_spec.rb
View file @ dd4a66b1
......@@ -204,11 +204,16 @@ describe Epic do
end
describe '#update_start_and_due_dates' do
def update_and_reload_subject
subject.update_start_and_due_dates
subject.reload
end
context 'fixed date is set' do
subject { create(:epic, :use_fixed_dates, start_date: nil, end_date: nil) }
it 'updates to fixed date' do
subject.update_start_and_due_dates
update_and_reload_subject
expect(subject.start_date).to eq(subject.start_date_fixed)
expect(subject.due_date).to eq(subject.due_date_fixed)
......@@ -243,7 +248,7 @@ describe Epic do
context 'complete start and due dates' do
it 'updates to milestone dates' do
subject.update_start_and_due_dates
update_and_reload_subject
expect(subject.start_date).to eq(milestone1.start_date)
expect(subject.due_date).to eq(milestone2.due_date)
......@@ -267,7 +272,7 @@ describe Epic do
end
it 'updates to milestone dates' do
subject.update_start_and_due_dates
update_and_reload_subject
expect(subject.start_date).to eq(milestone1.start_date)
expect(subject.due_date).to eq(nil)
......@@ -291,7 +296,7 @@ describe Epic do
end
it 'updates to milestone dates' do
subject.update_start_and_due_dates
update_and_reload_subject
expect(subject.start_date).to eq(nil)
expect(subject.due_date).to eq(nil)
......@@ -305,7 +310,7 @@ describe Epic do
end
it 'updates to milestone dates' do
subject.update_start_and_due_dates
update_and_reload_subject
expect(subject.start_date).to eq(nil)
expect(subject.start_date_sourcing_milestone_id).to eq(nil)
......@@ -322,7 +327,7 @@ describe Epic do
context 'complete start and due dates' do
it 'updates to milestone dates' do
subject.update_start_and_due_dates
update_and_reload_subject
expect(subject.start_date).to eq(milestone1.start_date)
expect(subject.due_date).to eq(milestone1.due_date)
......@@ -339,7 +344,7 @@ describe Epic do
end
it 'updates to milestone dates' do
subject.update_start_and_due_dates
update_and_reload_subject
expect(subject.start_date).to eq(milestone1.start_date)
expect(subject.due_date).to eq(nil)
......@@ -356,7 +361,7 @@ describe Epic do
end
it 'updates to milestone dates' do
subject.update_start_and_due_dates
update_and_reload_subject
expect(subject.start_date).to eq(nil)
expect(subject.due_date).to eq(nil)
......
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