Present Burndown charts for group milestones

app/assets/javascripts/pages/milestones/shared/components/promote_milestone_modal.vue

@@ -39,27 +39,15 @@ export default {
           groupName: this.groupName,
         },
       );
-      const missingFeatureWarn = sprintf(
-        s__(`Milestones|Group milestones are currently %{linkStart} missing features such as burndown charts. %{linkEnd}
-          You will not have these features once you've promoted a project milestone.
-          They will be available in future releases.`),
-        {
-          linkStart: `<a href="https://docs.gitlab.com/ee/user/project/milestones/"
-              target="_blank" rel="noopener noreferrer">`,
-          linkEnd: '</a>',
-        },
-        false,
-      );
       const finalWarning = s__('Milestones|This action cannot be reversed.');
 
       return sprintf(
         s__(
           `Milestones|<p>%{milestonePromotion}</p>
-          <p>%{missingFeatureWarn}</p>%{finalWarning}`,
+          %{finalWarning}`,
         ),
         {
           milestonePromotion,
-          missingFeatureWarn,
           finalWarning,
         },
         false,

app/controllers/projects/milestones_controller.rb

@@ -43,11 +43,6 @@ class Projects::MilestonesController < Projects::ApplicationController
   def show
     @project_namespace = @project.namespace.becomes(Namespace)
 
-    if @project.feature_available?(:burndown_charts, current_user) &&
-        @project.feature_available?(:issue_weights, current_user)
-      @burndown = Burndown.new(@milestone)
-    end
-
     respond_to do |format|
       format.html
     end

app/helpers/gitlab_routing_helper.rb

@@ -83,6 +83,14 @@ module GitlabRoutingHelper
     end
   end
 
+  def edit_milestone_path(entity, *args)
+    if entity.parent.is_a?(Group)
+      edit_group_milestone_path(entity.parent, entity, *args)
+    else
+      edit_project_milestone_path(entity.parent, entity, *args)
+    end
+  end
+
   def toggle_subscription_path(entity, *args)
     if entity.is_a?(Issue)
       toggle_subscription_project_issue_path(entity.project, entity)

app/helpers/milestones_helper.rb

 module MilestonesHelper
+  prepend EE::MilestonesHelper
   include EntityDateHelper
 
   def milestones_filter_path(opts = {})
@@ -194,39 +195,6 @@ module MilestonesHelper
     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 "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
-
-  def can_generate_chart?(burndown)
-    return unless @project.feature_available?(:burndown_charts, current_user) &&
-        @project.feature_available?(:issue_weights, current_user)
-
-    burndown&.valid? && !burndown&.empty?
-  end
-
-  def show_burndown_placeholder?(warning)
-    return false if cookies['hide_burndown_message'].present?
-
-    return false unless @project.feature_available?(:burndown_charts, current_user) &&
-        @project.feature_available?(:issue_weights, current_user)
-
-    warning.nil? && can?(current_user, :admin_milestone, @project)
-  end
-
   def milestone_merge_request_tab_path(milestone)
     if @project
       merge_requests_project_milestone_path(@project, milestone, format: :json)

app/views/projects/milestones/show.html.haml

@@ -69,7 +69,7 @@
           .wiki
             = markdown_field(@milestone, :description)
 
-  = render 'shared/milestones/burndown', milestone: @milestone, project: @project, burndown: @burndown
+  = render 'shared/milestones/burndown', milestone: @milestone, project: @project
 
   - if can?(current_user, :read_issue, @project) && @milestone.total_items_count(current_user).zero?
     .alert.alert-success.prepend-top-default

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

@@ -48,6 +48,8 @@
     - close_msg = group ? 'You may close the milestone now.' : 'Navigate to the project to close the milestone.'
     %span All issues for this milestone are closed. #{close_msg}
 
+= render 'shared/milestones/burndown', milestone: @milestone, project: @project
+
 - if is_dynamic_milestone
   .table-holder
     %table.table

doc/user/project/milestones/burndown_charts.md

 # Burndown Charts **[STARTER]**
 
 >**Notes:**
-- [Introduced][ee-1540] in [GitLab Starter 9.1][ee-9.1].
+- [Introduced][ee-1540] in [GitLab Starter 9.1][ee-9.1] for project milestones.
+- [Introduced][ee-5354] in [GitLab Silver 10.8][ee-10.8] for group milestones.
 - 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.
@@ -42,13 +43,16 @@ it was taken care of closely throughout the whole quarter
 
 ## How it works
 
->**Note:** Burndown charts are only available for project milestones. They will be available for group milestones [in the future](https://gitlab.com/gitlab-org/gitlab-ee/issues/3064).
-
-A Burndown Chart is available for every project milestone that has been attributed a **start
+A Burndown Chart is available for every project or group milestone that has been attributed a **start
 date** and a **due date**.
 
 Find your project's **Burndown Chart** under **Project > Issues > Milestones**,
-and select a milestone from your current ones.
+and select a milestone from your current ones, while for group's, access the **Groups** dashboard,
+select a group, and go through **Issues > Milestones** on the sidebar.
+
+>
+  **Note:** You're able to [promote project][promote-milestone] to group milestones and still
+  see the **Burndown Chart** for them, respecting license limitations.
 
 The chart indicates the project's progress throughout that milestone (for issues assigned to it).
 
@@ -73,3 +77,6 @@ cumulative value.
 
 [ee-1540]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540
 [ee-9.1]: https://about.gitlab.com/2017/04/22/gitlab-9-1-released/#burndown-charts-ees-eep
+[ee-5354]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/5354
+[ee-10.8]: https://about.gitlab.com/2017/04/22/gitlab-10-8-released/#burndown-charts-eep-eeu
+[promote-milestone]: https://docs.gitlab.com/ee/user/project/milestones/#promoting-project-milestones-to-group-milestones

ee/app/assets/javascripts/pages/groups/milestones/show/index.js

+import '~/pages/groups/milestones/show/index';
+import UserCallout from '~/user_callout';
+import initBurndownChart from 'ee/burndown_chart';
+
+document.addEventListener('DOMContentLoaded', () => {
+  new UserCallout(); // eslint-disable-line no-new
+  initBurndownChart();
+});

ee/app/helpers/ee/milestones_helper.rb

+module EE
+  module MilestonesHelper
+    def burndown_chart(milestone)
+      Burndown.new(milestone) if milestone.supports_burndown_charts?
+    end
+
+    def can_generate_chart?(milestone, burndown)
+      return false unless milestone.supports_burndown_charts?
+
+      burndown&.valid? && !burndown&.empty?
+    end
+
+    def show_burndown_placeholder?(milestone, warning)
+      return false if cookies['hide_burndown_message'].present?
+      return false unless milestone.supports_burndown_charts?
+
+      warning.nil? && can_admin_milestone?(milestone)
+    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 "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
+
+    private
+
+    def can_admin_milestone?(milestone)
+      policy_name = milestone.group_milestone? ? :admin_milestones : :admin_milestone
+
+      can?(current_user, policy_name, milestone.parent)
+    end
+  end
+end

ee/app/helpers/license_helper.rb

@@ -96,10 +96,6 @@ module LicenseHelper
     end
   end
 
-  def show_project_feature_promotion?(project_feature, callout_id = nil)
-    !@project.feature_available?(project_feature) && show_promotions? && (callout_id.nil? || show_callout?(callout_id))
-  end
-
   def show_advanced_search_promotion?
     !Gitlab::CurrentSettings.should_check_namespace_plan? && show_promotions? && show_callout?('promote_advanced_search_dismissed') && !License.feature_available?(:elastic_search)
   end

ee/app/models/ee/global_milestone.rb

@@ -3,5 +3,12 @@ module EE
     def supports_weight?
       false
     end
+
+    # Legacy group milestones or dashboard milestones (grouped by title)
+    # can't present Burndown charts since they don't have
+    # proper limits set.
+    def supports_burndown_charts?
+      false
+    end
   end
 end

ee/app/models/ee/milestone.rb

 module EE
   module Milestone
     def supports_weight?
-      project&.feature_available?(:issue_weights)
+      parent&.feature_available?(:issue_weights)
+    end
+
+    def supports_burndown_charts?
+      feature_name = group_milestone? ? :group_burndown_charts : :burndown_charts
+
+      parent&.feature_available?(feature_name) && supports_weight?
     end
   end
 end

ee/app/models/license.rb

@@ -59,6 +59,7 @@ class License < ActiveRecord::Base
     commit_committer_check
     external_authorization_service
     ci_cd_projects
+    group_burndown_charts
   ].freeze
 
   EEU_FEATURES = EEP_FEATURES + %i[

ee/app/views/projects/audit_events/index.html.haml

 - page_title "Audit Events"
+- feature_available = @project.feature_available?(:audit_events)
 
-- if show_project_feature_promotion?(:audit_events)
+- if !feature_available && show_promotions?
   = render 'shared/promotions/promote_audit_events'
 
 %h3.page-title Project Audit Events
 %p.light Events in #{@project.full_path}
 
-- if @project.feature_available?(:audit_events)
+- if feature_available
   = render 'shared/audit_events/event_table', events: @events

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

 - milestone = local_assigns[:milestone]
 - project = local_assigns[:project]
-- burndown = local_assigns[:burndown]
+- burndown = burndown_chart(milestone)
 - warning = data_warning_for(burndown)
 
 = warning
 
-- if can_generate_chart?(burndown)
+- if can_generate_chart?(milestone, burndown)
   .burndown-header
     %h3
       Burndown chart
@@ -16,7 +16,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 show_burndown_placeholder?(warning)
+- elsif show_burndown_placeholder?(milestone, warning)
   .burndown-hint.content-block.container-fluid
     = icon("times", class: "dismiss-icon")
     .row
@@ -29,5 +29,5 @@
           View your milestone's progress as a burndown chart.  Add both a start and a due date to
           this milestone and the chart will appear here, always up-to-date.
 
-        = link_to "Add start and due date", edit_project_milestone_path(project, milestone), class: 'btn'
-= render 'shared/promotions/promote_burndown_charts'
+        = link_to "Add start and due date", edit_milestone_path(milestone), class: 'btn'
+= render 'shared/promotions/promote_burndown_charts', milestone: milestone

ee/app/views/shared/promotions/_promote_burndown_charts.html.haml

-- if show_project_feature_promotion?(:burndown_charts, 'promote_burndown_charts_dismissed')
-  .user-callout.promotion-callout#promote_burndown_charts{ data: { uid: 'promote_burndown_charts_dismissed' } }
+- callout_id = 'promote_burndown_charts_dismissed'
+
+- if !milestone.supports_burndown_charts? && show_promotions? && show_callout?(callout_id)
+  .user-callout.promotion-callout#promote_burndown_charts{ data: { uid: callout_id } }
     .bordered-box.content-block
       %button.btn.btn-default.close.js-close-callout{ type: 'button', 'aria-label' => 'Dismiss burndown charts promotion' }
         = icon('times', class: 'dismiss-icon', 'aria-hidden' => 'true')

ee/changelogs/unreleased/3064-group-burndown-chart.yml

+---
+title: Present Burndown charts for group milestones
+merge_request:
+author:
+type: added

ee/spec/models/burndown_spec.rb

@@ -2,105 +2,155 @@ require 'spec_helper'
 
 describe Burndown do
   let(:start_date) { "2017-03-01" }
-  let(:due_date)   { "2017-03-05" }
-  let(:milestone)  { create(:milestone, start_date: start_date, due_date: due_date) }
-  let(:project)    { milestone.project }
-  let(:user)       { create(:user) }
-
-  let(:issue_params) do
-    {
-      title: FFaker::Lorem.sentence(6),
-      description: FFaker::Lorem.sentence,
-      state: 'opened',
-      milestone: milestone,
-      weight: 2,
-      project_id: project.id
-    }
-  end
+  let(:due_date) { "2017-03-05" }
+  let(:user) { create(:user) }
 
-  around do |example|
-    Timecop.travel(due_date) do
-      example.run
+  shared_examples 'burndown for milestone' do
+    before do
+      scope.add_master(user)
+      build_sample(milestone, issue_params)
     end
-  end
 
-  before do
-    project.add_master(user)
-    build_sample
-  end
+    around do |example|
+      Timecop.travel(due_date) do
+        example.run
+      end
+    end
 
-  subject { described_class.new(milestone).to_json }
+    subject { described_class.new(milestone).to_json }
 
-  it "generates an array with date, issue count and weight" do
-    expect(subject).to eq([
-      ["2017-03-01", 33, 66],
-      ["2017-03-02", 35, 70],
-      ["2017-03-03", 28, 56],
-      ["2017-03-04", 32, 64],
-      ["2017-03-05", 21, 42]
-    ].to_json)
-  end
+    it "generates an array with date, issue count and weight" do
+      expect(subject).to eq([
+        ["2017-03-01", 33, 66],
+        ["2017-03-02", 35, 70],
+        ["2017-03-03", 28, 56],
+        ["2017-03-04", 32, 64],
+        ["2017-03-05", 21, 42]
+      ].to_json)
+    end
 
-  it "returns empty array if milestone start date is nil" do
-    milestone.update(start_date: nil)
+    it "returns empty array if milestone start date is nil" do
+      milestone.update(start_date: nil)
 
-    expect(subject).to eq([].to_json)
-  end
-
-  it "returns empty array if milestone due date is nil" do
-    milestone.update(due_date: nil)
+      expect(subject).to eq([].to_json)
+    end
 
-    expect(subject).to eq([].to_json)
-  end
+    it "returns empty array if milestone due date is nil" do
+      milestone.update(due_date: nil)
 
-  it "it counts until today if milestone due date > Date.today" do
-    Timecop.travel(milestone.due_date - 1.day) do
-      expect(JSON.parse(subject).last[0]).to eq(Time.now.strftime("%Y-%m-%d"))
+      expect(subject).to eq([].to_json)
     end
-  end
 
-  it "sets attribute accurate to true" do
-    burndown = described_class.new(milestone)
+    it "it counts until today if milestone due date > Date.today" do
+      Timecop.travel(milestone.due_date - 1.day) do
+        expect(JSON.parse(subject).last[0]).to eq(Time.now.strftime("%Y-%m-%d"))
+      end
+    end
 
-    expect(burndown).to be_accurate
-  end
+    it "sets attribute accurate to true" do
+      burndown = described_class.new(milestone)
 
-  context "when all closed issues does not have closed events" do
-    before do
-      Event.where(target: milestone.issues, action: Event::CLOSED).destroy_all
+      expect(burndown).to be_accurate
     end
 
-    it "considers closed_at as milestone start date" do
-      expect(subject).to eq([
-        ["2017-03-01", 27, 54],
-        ["2017-03-02", 27, 54],
-        ["2017-03-03", 27, 54],
-        ["2017-03-04", 27, 54],
-        ["2017-03-05", 27, 54]
-      ].to_json)
+    context "when all closed issues does not have closed events" do
+      before do
+        Event.where(target: milestone.issues, action: Event::CLOSED).destroy_all
+      end
+
+      it "considers closed_at as milestone start date" do
+        expect(subject).to eq([
+          ["2017-03-01", 27, 54],
+          ["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
 
-    it "sets attribute empty to true" do
-      burndown = described_class.new(milestone)
+    context "when one or more closed issues does not have a closed event" do
+      before do
+        Event.where(target: milestone.issues.closed.first, action: Event::CLOSED).destroy_all
+      end
+
+      it "sets attribute accurate to false" do
+        burndown = described_class.new(milestone)
 
-      expect(burndown).to be_empty
+        expect(burndown).not_to be_accurate
+      end
     end
   end
 
-  context "when one or more closed issues does not have a closed event" do
-    before do
-      Event.where(target: milestone.issues.closed.first, action: Event::CLOSED).destroy_all
+  describe 'project milestone burndown' do
+    it_behaves_like 'burndown for milestone' do
+      let(:milestone) { create(:milestone, start_date: start_date, due_date: due_date) }
+      let(:project) { milestone.project }
+      let(:issue_params) do
+        {
+          title: FFaker::Lorem.sentence(6),
+          description: FFaker::Lorem.sentence,
+          state: 'opened',
+          milestone: milestone,
+          weight: 2,
+          project_id: project.id
+        }
+      end
+      let(:scope) { project }
     end
+  end
 
-    it "sets attribute accurate to false" do
-      burndown = described_class.new(milestone)
+  describe 'group milestone burndown' do
+    let(:group) { create(:group) }
+    let(:nested_group) { create(:group, parent: group) }
+    let(:group_project) { create(:project, group: group) }
+    let(:nested_group_project) { create(:project, group: nested_group) }
+    let(:group_milestone) { create(:milestone, project: nil, group: group, start_date: start_date, due_date: due_date) }
+    let(:nested_group_milestone) { create(:milestone, group: nested_group, start_date: start_date, due_date: due_date) }
+
+    context 'when nested group milestone', :nested_groups do
+      it_behaves_like 'burndown for milestone' do
+        let(:milestone) { nested_group_milestone }
+        let(:issue_params) do
+          {
+            title: FFaker::Lorem.sentence(6),
+            description: FFaker::Lorem.sentence,
+            state: 'opened',
+            milestone: milestone,
+            weight: 2,
+            project_id: nested_group_project.id
+          }
+        end
+        let(:scope) { group }
+      end
+    end
 
-      expect(burndown).not_to be_accurate
+    context 'when non-nested group milestone' do
+      it_behaves_like 'burndown for milestone' do
+        let(:milestone) { group_milestone }
+        let(:issue_params) do
+          {
+            title: FFaker::Lorem.sentence(6),
+            description: FFaker::Lorem.sentence,
+            state: 'opened',
+            milestone: milestone,
+            weight: 2,
+            project_id: group_project.id
+          }
+        end
+        let(:scope) { group }
+      end
     end
   end
 
   # Creates, closes and reopens issues only for odd days numbers
-  def build_sample
+  def build_sample(milestone, issue_params)
     milestone.start_date.upto(milestone.due_date) do |date|
       day = date.day
       next if day.even?

spec/helpers/gitlab_routing_helper_spec.rb

@@ -89,4 +89,19 @@ describe GitlabRoutingHelper do
       expect(preview_markdown_path(project)).to eq("/#{project.full_path}/preview_markdown")
     end
   end
+
+  describe '#edit_milestone_path' do
+    it 'returns group milestone edit path when given entity parent is a Group' do
+      group = create(:group)
+      milestone = create(:milestone, group: group)
+
+      expect(edit_milestone_path(milestone)).to eq("/groups/#{group.path}/-/milestones/#{milestone.iid}/edit")
+    end
+
+    it 'returns project milestone edit path when given entity parent is not a Group' do
+      milestone = create(:milestone, group: nil)
+
+      expect(edit_milestone_path(milestone)).to eq("/#{milestone.project.full_path}/milestones/#{milestone.iid}/edit")
+    end
+  end
 end