Commit 64c784d2 authored by oswaldoferreira's avatar oswaldoferreira Committed by Oswaldo Ferreira

Use events table to feed Burndown data on closed issues

https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/4855 introduces a
change which nullifies issues.closed_at whenever one reopens a issue.
Which is not currently expected by the burndown charts. Hence we're now
using the events.created_at information.
parent b20c602c
......@@ -82,10 +82,33 @@ class Burndown
def milestone_issues
@milestone_issues ||=
@milestone.issues
.where("state = 'closed' OR (state = 'opened' AND closed_at IS NOT NULL)")
.reorder("closed_at ASC")
.pluck("closed_at, weight, state")
.map {|attrs| Issue.new(*attrs) }
begin
# We make use of `events` table to get the closed_at timestamp.
# `issues.closed_at` can't be used once it's nullified if the issue is
# reopened.
internal_clause =
::Issue
.joins("LEFT OUTER JOIN events e ON issues.id = e.target_id AND e.target_type = 'Issue'")
.where(milestone: @milestone)
.where("state = 'closed' OR (state = 'opened' AND e.action = #{Event::CLOSED})") # rubocop:disable GitlabSecurity/SqlInjection
rel =
if Gitlab::Database.postgresql?
::Issue
.select("*")
.from(internal_clause.select('DISTINCT ON (issues.id) issues.id, issues.state, issues.weight, e.created_at AS closed_at'))
else
::Issue
.select("*")
.from(internal_clause.select('issues.id, issues.state, issues.weight, e.created_at AS closed_at'))
.group('id')
.having('closed_at = MIN(closed_at) OR closed_at IS NULL')
end
rel
.order('closed_at ASC')
.pluck('closed_at, weight, state')
.map { |attrs| Issue.new(*attrs) }
end
end
end
......@@ -13,6 +13,10 @@ describe 'Milestones on EE' do
visit project_milestone_path(project, milestone)
end
def close_issue(issue)
Issues::CloseService.new(issue.project, user, {}).execute(issue)
end
context 'burndown charts' do
let(:milestone) do
create(:milestone,
......@@ -36,11 +40,13 @@ describe 'Milestones on EE' do
end
end
context 'when any closed issues do not have closed_at value' do
context 'when a closed issue do not have closed events' do
it 'shows warning' do
create(:issue, issue_params)
close_issue(create(:issue, issue_params))
close_issue(create(:issue, issue_params))
# Legacy issue: No closed events being created
create(:closed_issue, issue_params)
create(:closed_issue, issue_params.merge(closed_at: nil))
visit_milestone
......@@ -50,10 +56,10 @@ describe 'Milestones on EE' do
end
end
context 'when all closed issues do not have closed_at value' do
context 'when all closed issues do not have closed events' do
it 'shows warning and hides burndown' do
create(:closed_issue, issue_params.merge(closed_at: nil))
create(:closed_issue, issue_params.merge(closed_at: nil))
create(:closed_issue, issue_params)
create(:closed_issue, issue_params)
visit_milestone
......@@ -66,7 +72,7 @@ describe 'Milestones on EE' do
context 'data is accurate' do
it 'does not show warning' do
create(:issue, issue_params)
create(:closed_issue, issue_params)
close_issue(create(:issue, issue_params))
visit_milestone
......
......@@ -65,9 +65,9 @@ describe Burndown do
expect(burndown).to be_accurate
end
context "when all closed issues does not have closed_at" do
context "when all closed issues does not have closed events" do
before do
milestone.issues.update_all(closed_at: nil)
Event.where(target: milestone.issues, action: Event::CLOSED).destroy_all
end
it "considers closed_at as milestone start date" do
......@@ -87,9 +87,9 @@ describe Burndown do
end
end
context "when one or more closed issues does not have closed_at" do
context "when one or more closed issues does not have a closed event" do
before do
milestone.issues.closed.first.update(closed_at: nil)
Event.where(target: milestone.issues.closed.first, action: Event::CLOSED).destroy_all
end
it "sets attribute accurate to false" do
......@@ -112,11 +112,25 @@ describe Burndown do
# Close issues
closed = issues.slice(0..count / 2)
closed.each(&:close)
closed.each { |issue| close_issue(issue) }
# Reopen issues
closed.slice(0..count / 4).each(&:reopen)
reopened_issues = closed.slice(0..count / 4)
reopened_issues.each { |issue| reopen_issue(issue) }
# This generates an issue with multiple closing events
issue_closed_twice = reopened_issues.last
close_issue(issue_closed_twice)
reopen_issue(issue_closed_twice)
end
end
end
def close_issue(issue)
Issues::CloseService.new(issue.project, user, {}).execute(issue)
end
def reopen_issue(issue)
Issues::ReopenService.new(issue.project, user, {}).execute(issue)
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