Hide burndown from milestone when all issues were closed prior to GitLab 9.1

app/assets/stylesheets/pages/milestone.scss

@@ -247,7 +247,7 @@
   }
 }
 
-.burndown_docs_link {
+.burndown-docs-link {
   color: inherit;
   text-decoration: underline;
 }

app/helpers/milestones_helper.rb

@@ -115,4 +115,21 @@ module MilestonesHelper
       end
     end
   end
+
+  def data_warning_for(burndown)
+    return unless burndown
+
+    message =
+      if burndown.empty?
+        "The burndown chart can’t be shown, as all milestone issues were closed before the GitLab 9.1 update. "
+      elsif !burndown.accurate?
+        "Some issues can’t be shown in the burndown chart, as they were closed before the GitLab 9.1 update. "
+      end
+
+    if message
+      message += link_to "More information.", help_page_path('workflow/milestones'), class: 'burndown-docs-link'
+
+      content_tag(:div, message.html_safe, id: "data-warning", class: "settings-message prepend-top-20")
+    end
+  end
 end

app/models/burndown.rb

 class Burndown
-  attr_reader :start_date, :due_date, :end_date, :issues_count, :issues_weight, :accurate
+  attr_reader :start_date, :due_date, :end_date, :issues_count, :issues_weight, :accurate, :legacy_data
   alias_method :accurate?, :accurate
+  alias_method :empty?, :legacy_data
 
   def initialize(milestone)
     @milestone = milestone
@@ -8,7 +9,9 @@ class Burndown
     @due_date = @milestone.due_date
     @end_date = @milestone.due_date
     @end_date = Date.today if @end_date.present? && @end_date > Date.today
-    @accurate = issues_with_closed_at.where(closed_at: nil).empty?
+
+    @accurate = milestone_closed_issues.all?(&:closed_at)
+    @legacy_data = milestone_closed_issues.any? && !milestone_closed_issues.any?(&:closed_at)
 
     @issues_count, @issues_weight = milestone.issues.reorder(nil).pluck('COUNT(*), COALESCE(SUM(weight), 0)').first
   end
@@ -54,7 +57,7 @@ class Burndown
     current_date = date.to_date
 
     closed =
-      issues_with_closed_at.select do |issue|
+      milestone_closed_issues.select do |issue|
         (issue.closed_at&.to_date || start_date) == current_date
       end
 
@@ -63,8 +66,8 @@ class Burndown
     [closed, reopened]
   end
 
-  def issues_with_closed_at
-    @issues_with_closed_at ||=
+  def milestone_closed_issues
+    @milestone_closed_issues ||=
       @milestone.issues.select("closed_at, weight, state").
         where("state IN ('reopened', 'closed')").
         order("closed_at ASC")

app/views/shared/milestones/_burndown.html.haml

 - milestone = local_assigns[:milestone]
 - project = local_assigns[:project]
 - burndown = local_assigns[:burndown]
-- can_generate_chart = burndown&.valid?
+- can_generate_chart = burndown&.valid? && !burndown&.empty?
+- warning = data_warning_for(burndown)
 
 - content_for :page_specific_javascripts do
   = page_specific_javascript_bundle_tag('common_d3')
   = page_specific_javascript_bundle_tag('burndown_chart')
 
-- unless burndown.accurate?
-  #no-data-warning.settings-message.prepend-top-20
-    Some issues can't be shown in the burndown chart, as they were closed before GitLab 9.1 update.
-    = link_to "More information.", help_page_path('workflow/milestones'), class: 'burndown_docs_link'
+= warning
 
 - if can_generate_chart
   .burndown-header
@@ -23,7 +21,7 @@
           Issue weight
   .burndown-chart{ data: { start_date: burndown.start_date.strftime("%Y-%m-%d"), due_date: burndown.due_date.strftime("%Y-%m-%d"), chart_data: burndown.to_json } }
 
-- elsif can?(current_user, :admin_milestone, @project) && cookies['hide_burndown_message'].nil?
+- elsif can?(current_user, :admin_milestone, @project) && cookies['hide_burndown_message'].nil? && warning.nil?
   .burndown-hint.content-block.container-fluid
     = icon("times", class: "dismiss-icon")
     .row

doc/workflow/milestones.md

@@ -29,11 +29,11 @@ special options available when filtering by milestone:
 
 ## Burndown (EE-Only)
 
-A burndown chart is available for every project milestone that has a set start date and a set due date. It is located on the project milestone page. It indicates project progress throughout that milestone (for issues that have that milestone assigned to it). In particular, it shows how many issues were or are still open for a given day in the milestone period. Since GitLab only tracks when an issue was last closed (and not its full history) the chart assumes that issue was open on days prior to that date. Reopened issues are considered as open on one day after it was closed. The burndown chart can also be toggled to display the cumulative open issue weight for a given day. When using this feature, make sure your weights have been properly assigned, since an open issue with no weight adds zero to the cumulative value.
+A burndown chart is available for every project milestone that has a set start date and a set due date. It is located on the project milestone page. It indicates project progress throughout that milestone (for issues that have that milestone assigned to it). In particular, it shows how many issues were or are still open for a given day in the milestone period. Since GitLab only tracks when an issue was last closed (and not its full history) the chart assumes that issue was open on days prior to that date. Reopened issues are considered as open on one day after they were closed. The burndown chart can also be toggled to display the cumulative open issue weight for a given day. When using this feature, make sure your weights have been properly assigned, since an open issue with no weight adds zero to the cumulative value.
 
 ![burndown chart](milestones/burndown_chart.png)
 
-Closed or reopened issues prior to GitLab 9.1 version won't have a `closed_at` value, so burndown considers it as closed on the milestone `start_date`. In that case a warning will be displayed.
+Closed or reopened issues prior to GitLab 9.1 version won't have a `closed_at` value, so the burndown chart considers it as closed on the milestone `start_date`. In that case a warning will be displayed.
 
 ![burndown chart warning](milestones/burndown_warning.png)
 

spec/features/milestones/show_spec.rb

@@ -27,14 +27,30 @@ describe 'Milestone show', feature: true do
   context 'burndown' do
     let(:issue_params) { { project: project, assignee: user, author: user, milestone: milestone } }
 
-    context 'when closed issues does not have closed_at value' do
+    context 'when any closed issues do not have closed_at value' do
       it 'shows warning' do
         create(:issue, issue_params)
+        create(:closed_issue, issue_params)
+        create(:closed_issue, issue_params.merge(closed_at: nil))
+
+        visit_milestone
+
+        expect(page).to have_selector('#data-warning', count: 1)
+        expect(page.find('#data-warning').text).to include("Some issues can’t be shown in the burndown chart")
+        expect(page).to have_selector('.burndown-chart')
+      end
+    end
+
+    context 'when all closed issues do not have closed_at value' 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))
 
         visit_milestone
 
-        expect(page).to have_selector('#no-data-warning')
+        expect(page).to have_selector('#data-warning', count: 1)
+        expect(page.find('#data-warning').text).to include("The burndown chart can’t be shown")
+        expect(page).not_to have_selector('.burndown-chart')
       end
     end
 
@@ -45,7 +61,8 @@ describe 'Milestone show', feature: true do
 
         visit_milestone
 
-        expect(page).not_to have_selector('#no-data-warning')
+        expect(page).not_to have_selector('#data-warning')
+        expect(page).to have_selector('.burndown-chart')
       end
     end
   end

spec/models/burndown_spec.rb

@@ -60,10 +60,10 @@ describe Burndown, models: true do
   it "sets attribute accurate to true" do
     burndown = described_class.new(milestone)
 
-    expect(burndown.accurate?).to be_truthy
+    expect(burndown).to be_accurate
   end
 
-  context "when closed and reopened issues does not have closed_at" do
+  context "when all closed and reopened issues does not have closed_at" do
     before do
       milestone.issues.update_all(closed_at: nil)
     end
@@ -78,10 +78,22 @@ describe Burndown, models: true do
       ].to_json)
     end
 
-    it "setsattribute accurate to false" do
+    it "sets attribute empty to true" do
       burndown = described_class.new(milestone)
 
-      expect(burndown.accurate?).to be_falsy
+      expect(burndown).to be_empty
+    end
+  end
+
+  context "when one or more closed or reopened issues does not have closed_at" do
+    before do
+      milestone.issues.closed.first.update(closed_at: nil)
+    end
+
+    it "sets attribute accurate to false" do
+      burndown = described_class.new(milestone)
+
+      expect(burndown).not_to be_accurate
     end
   end