Add endpoint to provide milestone burnup chart data

Merging events and resource milestone events

Use sorting by id instead of created_at

Remove obsolete methods

Remove events merge

Using distinct issue_ids

Also modify issue retrieval statement to
improve performance

Change changelog title to reflect new scope

Fix visible issues

Store previously assigned milestones

Apply suggestion to ee/app/models/burnup.rb
Apply suggestion to ee/app/models/burnup.rb
Apply suggestion to ee/app/models/burnup.rb
Apply suggestion to ee/app/models/burnup.rb

Make Burnup a BurnupChartService

app/models/resource_milestone_event.rb

@@ -13,9 +13,9 @@ class ResourceMilestoneEvent < ResourceEvent
   validate :exactly_one_issuable
 
   enum action: {
-         add: 1,
-         remove: 2
-       }
+    add: 1,
+    remove: 2
+  }
 
   # state is used for issue and merge request states.
   enum state: Issue.available_states.merge(MergeRequest.available_states)

ee/app/models/ee/milestone.rb

@@ -19,5 +19,9 @@ module EE
 
       resource_parent&.feature_available?(feature_name) && supports_weight?
     end
+
+    def burnup_charts_available?
+      ::Feature.enabled?(:burnup_charts, resource_parent)
+    end
   end
 end

ee/app/services/burnup_chart_service.rb

+# frozen_string_literal: true
+
+class BurnupChartService
+  include Gitlab::Utils::StrongMemoize
+
+  attr_reader :milestone, :start_date, :due_date, :end_date, :user
+
+  def initialize(milestone:, user:)
+    @user = user
+    @milestone = milestone
+
+    assign_dates_by_milestone
+  end
+
+  def execute
+    return [] unless milestone.burnup_charts_available?
+
+    events = []
+    assigned_milestones = {}
+
+    resource_milestone_events.each do |event|
+      handle_added_milestone(event, assigned_milestones)
+
+      events << create_burnup_graph_event_by(event, assigned_milestones)
+
+      handle_removed_milestone(event, assigned_milestones)
+    end
+
+    events
+  end
+
+  private
+
+  def handle_added_milestone(event, assigned_milestones)
+    if event.add?
+      assigned_milestones[event.issue_id] = event.milestone_id
+    end
+  end
+
+  def handle_removed_milestone(event, assigned_milestones)
+    if event.remove?
+      assigned_milestones[event.issue_id] = nil
+    end
+  end
+
+  def create_burnup_graph_event_by(event, assigned_milestones)
+    {
+      created_at: event.created_at,
+      action: event.action,
+      milestone_id: milestone_id_of(event, assigned_milestones),
+      issue_id: event.issue_id
+    }
+  end
+
+  def assign_dates_by_milestone
+    @start_date = milestone.start_date
+    @due_date = milestone.due_date
+    @end_date = if due_date.blank? || due_date > Date.today
+                  Date.today
+                else
+                  due_date
+                end
+  end
+
+  def milestone_id_of(event, assigned_milestones)
+    if event.remove? && event.milestone_id.nil?
+      return assigned_milestones[event.issue_id]
+    end
+
+    event.milestone_id
+  end
+
+  # This is just temporarily disabled until https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29578
+  # is merged.
+  # rubocop: disable CodeReuse/ActiveRecord
+  def resource_milestone_events
+    # Here we use the relevant issues to get *all* milestone events for
+    # them.
+    strong_memoize(:resource_milestone_events) do
+      ResourceMilestoneEvent
+        .where(issue_id: relevant_issue_ids)
+        .where('created_at <= ?', end_time)
+        .order(:created_at)
+    end
+  end
+
+  def relevant_issue_ids
+    # We are using all resource milestone events where the
+    # milestone in question was added to identify the relevant
+    # issues.
+    strong_memoize(:relevant_issue_ids) do
+      ids = ResourceMilestoneEvent
+              .select(:issue_id)
+              .where(milestone_id: milestone.id)
+              .where(action: :add)
+              .distinct
+
+      # We need to perform an additional check whether all these
+      # issues are visible to the given user
+      IssuesFinder.new(user)
+        .execute.select(:id).where(id: ids)
+    end
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+
+  def end_time
+    @end_time ||= @end_date.end_of_day
+  end
+end

ee/changelogs/unreleased/213334-backend-chart-data-on-resource-events-pd.yml

+---
+title: Provide milestone burnup chart data for scope committed graph
+merge_request: 28899
+author:
+type: added

ee/lib/ee/api/group_milestones.rb

@@ -20,6 +20,12 @@ module EE
 
             milestone_burndown_events_for(user_group)
           end
+
+          get ':id/milestones/:milestone_id/burnup_events' do
+            authorize! :read_group, user_group
+
+            milestone_burnup_events_for(user_group)
+          end
         end
       end
     end

ee/lib/ee/api/milestone_responses.rb

@@ -17,6 +17,16 @@ module EE
               render_api_error!("Milestone does not support burndown chart", 405)
             end
           end
+
+          def milestone_burnup_events_for(parent)
+            milestone = parent.milestones.find(params[:milestone_id])
+
+            if milestone.supports_burndown_charts?
+              present BurnupChartService.new(milestone: milestone, user: current_user).execute
+            else
+              render_api_error!("Milestone does not support burnup chart", 405)
+            end
+          end
         end
       end
     end

ee/lib/ee/api/project_milestones.rb

@@ -20,6 +20,12 @@ module EE
 
             milestone_burndown_events_for(user_project)
           end
+
+          get ':id/milestones/:milestone_id/burnup_events' do
+            authorize! :read_milestone, user_project
+
+            milestone_burnup_events_for(user_project)
+          end
         end
       end
     end

ee/spec/fixtures/api/schemas/burnup_event.json

+{
+  "type": "object",
+  "additionalProperties": false,
+  "required": [
+    "action",
+    "milestone_id",
+    "created_at",
+    "issue_id"
+  ],
+  "properties": {
+    "milestone_id": {
+      "type": ["integer", "null"]
+    },
+    "issue_id": {
+      "type": "integer"
+    },
+    "action": {
+      "type": "string"
+    },
+    "created_at": {
+      "type": "date"
+    }
+  }
+}

ee/spec/fixtures/api/schemas/burnup_events.json

+{
+  "type": "array",
+  "items": { "$ref": "burnup_event.json" }
+}

ee/spec/requests/api/group_milestones_spec.rb

@@ -27,4 +27,8 @@ describe API::GroupMilestones do
   it_behaves_like 'group and project milestone burndowns', '/groups/:id/milestones/:milestone_id/burndown_events' do
     let(:route) { "/groups/#{group.id}/milestones" }
   end
+
+  it_behaves_like 'group and project milestone burnups', '/groups/:id/milestones/:milestone_id/burnup_events' do
+    let(:route) { "/groups/#{group.id}/milestones" }
+  end
 end

ee/spec/requests/api/project_milestones_spec.rb

@@ -24,4 +24,8 @@ describe API::ProjectMilestones do
   it_behaves_like 'group and project milestone burndowns', '/projects/:id/milestones/:milestone_id/burndown_events' do
     let(:route) { "/projects/#{project.id}/milestones" }
   end
+
+  it_behaves_like 'group and project milestone burnups', '/projects/:id/milestones/:milestone_id/burnup_events' do
+    let(:route) { "/projects/#{project.id}/milestones" }
+  end
 end

ee/spec/services/burnup_chart_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe BurnupChartService do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:other_user) { create(:user) }
+
+  let_it_be(:project) { create(:project, :private) }
+
+  let_it_be(:start_date) { Date.parse('2020-01-04') }
+  let_it_be(:due_date) { Date.parse('2020-01-26') }
+
+  let_it_be(:milestone1) { create(:milestone, :with_dates, title: 'v1.0', project: project, start_date: start_date, due_date: due_date) }
+  let_it_be(:milestone2) { create(:milestone, :with_dates, title: 'v1.1', project: project, start_date: start_date + 1.year, due_date: due_date + 1.year) }
+
+  let_it_be(:issue1) { create(:issue, project: project, milestone: milestone1) }
+  let_it_be(:issue2) { create(:issue, project: project, milestone: milestone1) }
+  let_it_be(:issue3) { create(:issue, project: project, milestone: milestone1) }
+  let_it_be(:other_issue) { create(:issue, project: project) }
+
+  let_it_be(:event1) { create(:resource_milestone_event, issue: issue1, action: :add, milestone: milestone1, created_at: start_date + 1.second) }
+  let_it_be(:event2) { create(:resource_milestone_event, issue: issue2, action: :add, milestone: milestone1, created_at: start_date + 2.seconds) }
+  let_it_be(:event3) { create(:resource_milestone_event, issue: issue3, action: :add, milestone: milestone1, created_at: start_date + 1.day) }
+  let_it_be(:event4) { create(:resource_milestone_event, issue: issue3, action: :remove, milestone: nil, created_at: start_date + 2.days + 1.second) }
+  let_it_be(:event5) { create(:resource_milestone_event, issue: issue3, action: :add, milestone: milestone2, created_at: start_date + 3.days) }
+  let_it_be(:event6) { create(:resource_milestone_event, issue: issue3, action: :remove, milestone: nil, created_at: start_date + 4.days) }
+
+  before do
+    project.add_maintainer(user)
+  end
+
+  describe '#execute' do
+    it 'returns the expected events' do
+      # This event is not within the time frame of the milestone's start and due date
+      # but it should nevertheless be part of the result set since the 'add' events
+      # are important for the graph.
+      create(:resource_milestone_event, issue: issue1, action: :add, milestone: milestone2, created_at: start_date.beginning_of_day - 1.second)
+
+      # These events are ignored
+      create(:resource_milestone_event, issue: other_issue, action: :remove, milestone: milestone2, created_at: start_date.beginning_of_day - 1.second)
+      create(:resource_milestone_event, issue: issue3, action: :remove, milestone: nil, created_at: due_date.end_of_day + 1.second)
+
+      data = described_class.new(milestone: milestone1, user: user).execute
+
+      expected_events = [
+        { action: 'add', issue_id: issue1.id, milestone_id: milestone2.id, created_at: start_date.beginning_of_day - 1.second },
+        { action: 'add', issue_id: issue1.id, milestone_id: milestone1.id, created_at: start_date + 1.second },
+        { action: 'add', issue_id: issue2.id, milestone_id: milestone1.id, created_at: start_date + 2.seconds },
+        { action: 'add', issue_id: issue3.id, milestone_id: milestone1.id, created_at: start_date + 1.day },
+        { action: 'remove', issue_id: issue3.id, milestone_id: milestone1.id, created_at: start_date + 2.days + 1.second },
+        { action: 'add', issue_id: issue3.id, milestone_id: milestone2.id, created_at: start_date + 3.days },
+        { action: 'remove', issue_id: issue3.id, milestone_id: milestone2.id, created_at: start_date + 4.days }
+      ]
+
+      expect(data).to eq(expected_events)
+    end
+
+    it 'excludes issues which should not be visible to the user ' do
+      data = described_class.new(milestone: milestone1, user: other_user).execute
+
+      expect(data).to be_empty
+    end
+
+    context 'when burnup charts are not available' do
+      before do
+        stub_feature_flags(burnup_charts: false)
+      end
+
+      it 'returns an empty array' do
+        data = described_class.new(milestone: milestone1, user: user).execute
+
+        expect(data).to be_empty
+      end
+    end
+  end
+end

ee/spec/support/shared_examples/requests/api/milestone_burnup_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'group and project milestone burnups' do |route_definition|
+  let(:resource_route) { "#{route}/#{milestone.id}/burnup_events" }
+
+  let(:event_time) { milestone.start_date.beginning_of_day }
+
+  let!(:event1) { create(:resource_milestone_event, issue: issue1, action: :add, milestone: milestone, created_at: event_time - 1.hour) }
+  let!(:event2) { create(:resource_milestone_event, issue: issue2, action: :add, milestone: milestone, created_at: event_time + 1.hour) }
+  let!(:event3) { create(:resource_milestone_event, issue: issue1, action: :remove, milestone: nil, created_at: event_time + 2.hours) }
+
+  describe "GET #{route_definition}" do
+    it 'returns burnup events list' do
+      get api(resource_route, user)
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(json_response).to be_an Array
+      expect(json_response).to match_schema('burnup_events', dir: 'ee')
+
+      expected_events = [
+          { 'issue_id' => issue1.id, 'milestone_id' => milestone.id, 'action' => 'add', 'created_at' => '2020-04-16T23:00:00.000Z' },
+          { 'issue_id' => issue2.id, 'milestone_id' => milestone.id, 'action' => 'add', 'created_at' => '2020-04-17T01:00:00.000Z' },
+          { 'issue_id' => issue1.id, 'milestone_id' => milestone.id, 'action' => 'remove', 'created_at' => '2020-04-17T02:00:00.000Z' }
+      ]
+
+      expect(json_response).to eq(expected_events)
+    end
+
+    it 'returns 404 when user is not authorized to read milestone' do
+      outside_user = create(:user)
+
+      get api(resource_route, outside_user)
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+end

spec/models/resource_milestone_event_spec.rb

@@ -52,4 +52,30 @@ describe ResourceMilestoneEvent, type: :model do
       end
     end
   end
+
+  shared_examples 'a milestone action queryable resource event' do |expected_results_for_actions|
+    [Issue, MergeRequest].each do |klass|
+      expected_results_for_actions.each do |action, expected_result|
+        it "is #{expected_result} for action #{action} on #{klass.name.underscore}" do
+          model = create(klass.name.underscore)
+          key = model.class.name.underscore
+          event = build(described_class.name.underscore.to_sym, key => model, action: action)
+
+          expect(event.send(query_method)).to eq(expected_result)
+        end
+      end
+    end
+  end
+
+  describe '#added?' do
+    it_behaves_like 'a milestone action queryable resource event', { add: true, remove: false } do
+      let(:query_method) { :add? }
+    end
+  end
+
+  describe '#removed?' do
+    it_behaves_like 'a milestone action queryable resource event', { add: false, remove: true } do
+      let(:query_method) { :remove? }
+    end
+  end
 end