Merge branch 'issue_2151' into 'master'

Add warning when burndown data is not accurate + docs

Closes #2151

See merge request !1650

app/assets/stylesheets/pages/milestone.scss

@@ -247,6 +247,11 @@
   }
 }
 
+.burndown-docs-link {
+  color: inherit;
+  text-decoration: underline;
+}
+
 .burndown-header {
   margin: 24px 0 12px;
 

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 issues assigned to this milestone were closed on an older GitLab version before data was recorded. "
+      elsif !burndown.accurate?
+        "Some issues can’t be shown in the burndown chart, as they were closed on an older GitLab version before data was recorded. "
+      end
+
+    if message
+      message += link_to "Find out about burndown charts.", help_page_path('user/project/milestones/index', anchor: 'burndown-charts'), 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_accessor :start_date, :due_date, :end_date, :issues_count, :issues_weight
+  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,6 +10,9 @@ class Burndown
     @end_date = @milestone.due_date
     @end_date = Date.today if @end_date.present? && @end_date > Date.today
 
+    @accurate = milestone_closed_issues.all?(&:closed_at)
+    @legacy_data = milestone_closed_issues.any? && milestone_closed_issues.none?(&:closed_at)
+
     @issues_count, @issues_weight = milestone.issues.reorder(nil).pluck('COUNT(*), COALESCE(SUM(weight), 0)').first
   end
 
@@ -51,16 +56,20 @@ class Burndown
   def closed_and_reopened_issues_by(date)
     current_date = date.to_date
 
-    closed   = issues_with_closed_at.select { |issue| issue.closed_at.to_date == current_date }
+    closed =
+      milestone_closed_issues.select do |issue|
+        (issue.closed_at&.to_date || start_date) == current_date
+      end
+
     reopened = closed.select { |issue| issue.state == 'reopened' }
 
     [closed, reopened]
   end
 
-  def issues_with_closed_at
-    @issues_with_closed_at ||=
-      @milestone.issues.select('closed_at, weight, state').
-        where('closed_at IS NOT NULL').
-        order('closed_at ASC')
+  def milestone_closed_issues
+    @milestone_closed_issues ||=
+      @milestone.issues.select("closed_at, weight, state").
+        where("state IN ('reopened', 'closed')").
+        order("closed_at ASC")
   end
 end

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')
 
+= warning
+
 - if can_generate_chart
   .burndown-header
     %h3
@@ -18,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

changelogs/unreleased-ee/issue_2151.yml

+---
+title: Add warning when burndown data is not accurate
+merge_request:
+author:

doc/analytics/README.md

 # Analytics
 
 - [Contribution Analytics](contribution_analytics.md)
+- (EE) [Burndown charts](../user/project/milestones/index.md#burndown-charts)

doc/intro/README.md

@@ -13,7 +13,7 @@ Create issues, labels, milestones, cast your vote, and review issues.
 
 - [Create a new issue](../gitlab-basics/create-issue.md)
 - [Assign labels to issues](../user/project/labels.md)
-- [Use milestones as an overview of your project's tracker](../workflow/milestones.md)
+- [Use milestones as an overview of your project's tracker](../user/project/milestones/index.md)
 - [Use voting to express your like/dislike to issues and merge requests](../workflow/award_emoji.md)
 
 ## Collaborate

doc/university/glossary/README.md

@@ -333,7 +333,7 @@ A [platform](https://www.meteor.com) for building javascript apps.
 
 ### Milestones
 
-Allow you to [organize issues](https://docs.gitlab.com/ce/workflow/milestones.html) and merge requests in GitLab into a cohesive group, optionally setting a due date. A common use is keeping track of an upcoming software version. Milestones are created per-project.
+Allow you to [organize issues](https://docs.gitlab.com/ce/user/project/milestones/) and merge requests in GitLab into a cohesive group, optionally setting a due date. A common use is keeping track of an upcoming software version. Milestones are created per-project.
 
 ### Mirror Repositories
 

doc/user/project/milestones/index.md

+# Milestones
+
+Milestones allow you to organize issues and merge requests into a cohesive group, optionally setting a due date.
+A common use is keeping track of an upcoming software version. Milestones are created per-project.
+
+You can find the milestones page under your project's **Issues ➔ Milestones**.
+
+## Creating a milestone
+
+To create a new milestone, simply click the **New milestone** button when in the
+milestones page. A milestone can have a title, a description and start/due dates.
+Once you fill in all the details, hit the **Create milestone** button.
+
+>**Note:**
+The start/due dates are required if you intend to use [Burndown charts](#burndown-charts).
+
+![Creating a milestone](img/milestone_create.png)
+
+## Groups and milestones
+
+You can create a milestone for several projects in the same group simultaneously.
+On the group's **Issues ➔ Milestones** page, you will be able to see the status
+of that milestone across all of the selected projects. To create a new milestone
+for selected projects in the group, click the **New milestone** button. The
+form is the same as when creating a milestone for a specific project with the
+addition of the selection of the projects you want to inherit this milestone.
+
+![Creating a group milestone](img/milestone_group_create.png)
+
+## Special milestone filters
+
+In addition to the milestones that exist in the project or group, there are some
+special options available when filtering by milestone:
+
+* **No Milestone** - only show issues or merge requests without a milestone.
+* **Upcoming** - show issues or merge request that belong to the next open
+  milestone with a due date, by project. (For example: if project A has
+  milestone v1 due in three days, and project B has milestone v2 due in a week,
+  then this will show issues or merge requests from milestone v1 in project A
+  and milestone v2 in project B.)
+* **Started** - show issues or merge requests from any milestone with a start
+  date less than today. Note that this can return results from several
+  milestones in the same project.
+
+## Burndown charts
+
+>**Notes:**
+- [Introduced][ee-1540] in GitLab Enterprise Edition 9.1 and is available for
+  [Enterprise Edition Starter][ee] users.
+- Closed or reopened issues prior to GitLab 9.1 won't have a `closed_at`
+  value, so the burndown chart considers them as closed on the milestone
+  `start_date`. In that case, a warning will be displayed.
+
+A burndown chart is available for every project milestone that has a set start
+date and a set due date and is located on the project's milestone page.
+
+It indicates the project's 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](img/burndown_chart.png)
+
+[ee-1540]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540
+[ee]: https://about.gitlab.com/gitlab-ee

doc/workflow/README.md

@@ -30,7 +30,7 @@
 - [Time tracking](time_tracking.md)
 - [Web Editor](../user/project/repository/web_editor.md)
 - [Releases](releases.md)
-- [Milestones](milestones.md)
+- [Milestones](../user/project/milestones/index.md)
 - [Merge Requests](../user/project/merge_requests/index.md)
   - [Authorization for merge requests](../user/project/merge_requests/authorization_for_merge_requests.md)
   - [Cherry-pick changes](../user/project/merge_requests/cherry_pick_changes.md)

doc/workflow/milestones.md

-# Milestones
-
-Milestones allow you to organize issues and merge requests into a cohesive group, optionally setting a due date.
-A common use is keeping track of an upcoming software version. Milestones are created per-project.
-
-![milestone form](milestones/form.png)
-
-## Groups and milestones
-
-You can create a milestone for several projects in the same group simultaneously.
-On the group's milestones page, you will be able to see the status of that milestone across all of the selected projects.
-
-![group milestone form](milestones/group_form.png)
-
-## Special milestone filters
-
-In addition to the milestones that exist in the project or group, there are some
-special options available when filtering by milestone:
-
-* **No Milestone** - only show issues or merge requests without a milestone.
-* **Upcoming** - show issues or merge request that belong to the next open
-  milestone with a due date, by project. (For example: if project A has
-  milestone v1 due in three days, and project B has milestone v2 due in a week,
-  then this will show issues or merge requests from milestone v1 in project A
-  and milestone v2 in project B.)
-* **Started** - show issues or merge requests from any milestone with a start
-  date less than today. Note that this can return results from several
-  milestones in the same project.
+This document was moved to [another location](../user/project/milestones/index.md).

spec/factories/issues.rb

@@ -10,6 +10,7 @@ FactoryGirl.define do
 
     trait :closed do
       state :closed
+      closed_at Time.now
     end
 
     trait :reopened do

spec/features/milestones/show_spec.rb

@@ -3,12 +3,12 @@ require 'rails_helper'
 describe 'Milestone show', feature: true do
   let(:user) { create(:user) }
   let(:project) { create(:empty_project) }
-  let(:milestone) { create(:milestone, project: project) }
+  let(:milestone) { create(:milestone, project: project, start_date: Date.today, due_date: 7.days.from_now) }
   let(:labels) { create_list(:label, 2, project: project) }
   let(:issue_params) { { project: project, assignee: user, author: user, milestone: milestone, labels: labels } }
 
   before do
-    project.add_user(user, :developer) 
+    project.add_user(user, :developer)
     login_as(user)
   end
 
@@ -23,4 +23,47 @@ describe 'Milestone show', feature: true do
 
     expect { visit_milestone }.not_to exceed_query_limit(control_count)
   end
+
+  context 'burndown' do
+    let(:issue_params) { { project: project, assignee: user, author: user, milestone: milestone } }
+
+    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('#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
+
+    context 'data is accurate' do
+      it 'does not show warning' do
+        create(:issue, issue_params)
+        create(:closed_issue, issue_params)
+
+        visit_milestone
+
+        expect(page).not_to have_selector('#data-warning')
+        expect(page).to have_selector('.burndown-chart')
+      end
+    end
+  end
 end

spec/models/burndown_spec.rb

@@ -57,6 +57,46 @@ describe Burndown, models: true do
     expect(JSON.parse(subject).last[0]).to eq(Time.now.strftime("%Y-%m-%d"))
   end
 
+  it "sets attribute accurate to true" do
+    burndown = described_class.new(milestone)
+
+    expect(burndown).to be_accurate
+  end
+
+  context "when all closed and reopened issues does not have closed_at" do
+    before do
+      milestone.issues.update_all(closed_at: nil)
+    end
+
+    it "considers closed_at as milestone start date" do
+      expect(subject).to eq([
+        ["2017-03-01", 15, 30],
+        ["2017-03-02", 27, 54],
+        ["2017-03-03", 27, 54],
+        ["2017-03-04", 27, 54],
+        ["2017-03-05", 27, 54]
+      ].to_json)
+    end
+
+    it "sets attribute empty to true" do
+      burndown = described_class.new(milestone)
+
+      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
+
   # Creates, closes and reopens issues only for odd days numbers
   def build_sample
     milestone.start_date.upto(milestone.due_date) do |date|