Merge branch 'remove-unused-burnup-api-endpoint' into 'master'

Remove unused burnup events API endpoint

See merge request gitlab-org/gitlab!44419

ee/app/services/burnup_chart_service.rb

-# frozen_string_literal: true
-
-class BurnupChartService
-  include Gitlab::Utils::StrongMemoize
-
-  EVENT_TYPE = 'event_type'.freeze
-  CREATED_AT = 'created_at'.freeze
-  MILESTONE_ID = 'value'.freeze
-  WEIGHT = 'value'.freeze
-  ACTION = 'action'.freeze
-  ISSUE_ID = 'issue_id'.freeze
-  STATE = 'value'.freeze
-
-  def initialize(milestone:, user:)
-    @user = user
-    @milestone = milestone
-
-    assign_dates_by_milestone
-  end
-
-  def execute
-    return [] unless milestone.burnup_charts_available?
-
-    return [] unless can_read_milestone?
-
-    events = []
-    assigned_milestones = {}
-
-    resource_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
-
-  attr_reader :milestone, :start_date, :due_date, :end_date, :user
-
-  def can_read_milestone?
-    Ability.allowed?(user, :read_milestone, milestone.parent)
-  end
-
-  def handle_added_milestone(event, assigned_milestones)
-    if add_milestone?(event)
-      assigned_milestones[event[ISSUE_ID]] = event[MILESTONE_ID]
-    end
-  end
-
-  def handle_removed_milestone(event, assigned_milestones)
-    if remove_milestone?(event)
-      assigned_milestones[event[ISSUE_ID]] = nil
-    end
-  end
-
-  def resource_events
-    strong_memoize(:resource_events) do
-      union = Gitlab::SQL::Union.new(all_events).to_sql # rubocop: disable Gitlab/Union
-      ActiveRecord::Base.connection.execute("(#{union}) ORDER BY created_at")
-    end
-  end
-
-  def all_events
-    [milestone_events, weight_events, state_events]
-  end
-
-  def weight_events
-    ResourceWeightEvent.by_issue_ids_and_created_at_earlier_or_equal_to(relevant_issue_ids, end_time)
-      .select('\'weight\' as event_type, created_at, weight as value, null as action, issue_id')
-  end
-
-  def milestone_events
-    ResourceMilestoneEvent.by_issue_ids_and_created_at_earlier_or_equal_to(relevant_issue_ids, end_time)
-      .select('\'milestone\' as event_type, created_at, milestone_id as value, action, issue_id')
-  end
-
-  def state_events
-    ResourceStateEvent.by_issue_ids_and_created_at_earlier_or_equal_to(relevant_issue_ids, end_time)
-      .select('\'state\' as event_type, created_at, state as value, null as action, issue_id')
-  end
-
-  def create_burnup_graph_event_by(event, assigned_milestones)
-    {
-      event_type: event[EVENT_TYPE],
-      created_at: event[CREATED_AT],
-      action: action_of(event),
-      milestone_id: milestone_id_of(event, assigned_milestones),
-      issue_id: event[ISSUE_ID],
-      weight: weight_of(event),
-      state: state_of(event)
-    }
-  end
-
-  def state_of(event)
-    return unless state_event?(event)
-
-    ResourceStateEvent.states.key(event[STATE])
-  end
-
-  def action_of(event)
-    return unless milestone_event?(event)
-
-    ResourceMilestoneEvent.actions.key(event[ACTION])
-  end
-
-  def weight_of(event)
-    return unless event[EVENT_TYPE] == 'weight'
-
-    event[WEIGHT]
-  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)
-    return unless milestone_event?(event)
-
-    if remove_milestone?(event) && event[MILESTONE_ID].nil?
-      return assigned_milestones[event[ISSUE_ID]]
-    end
-
-    event[MILESTONE_ID]
-  end
-
-  def add_milestone?(event)
-    return unless milestone_event?(event)
-
-    event[ACTION] == ResourceMilestoneEvent.actions[:add]
-  end
-
-  def remove_milestone?(event)
-    return unless milestone_event?(event)
-
-    event[ACTION] == ResourceMilestoneEvent.actions[:remove]
-  end
-
-  def milestone_event?(event)
-    event[EVENT_TYPE] == 'milestone'
-  end
-
-  def state_event?(event)
-    event[EVENT_TYPE] == 'state'
-  end
-
-  # rubocop: disable CodeReuse/ActiveRecord
-  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
-      ResourceMilestoneEvent
-            .select(:issue_id)
-            .where(milestone_id: milestone.id)
-            .where(action: :add)
-            .distinct
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-  end
-
-  def end_time
-    @end_time ||= @end_date.end_of_day
-  end
-end

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

@@ -2,7 +2,6 @@
 - burndown = burndown_chart(milestone)
 - warning = data_warning_for(burndown)
 - burndown_endpoint = milestone.group_milestone? ? api_v4_groups_milestones_burndown_events_path(id: milestone.group.id, milestone_id: milestone.id) : api_v4_projects_milestones_burndown_events_path(id: milestone.project.id, milestone_id: milestone.timebox_id)
-- burnup_endpoint = milestone.group_milestone? ? api_v4_groups_milestones_burnup_events_path(id: milestone.group.id, milestone_id: milestone.id) : api_v4_projects_milestones_burnup_events_path(id: milestone.project.id, milestone_id: milestone.timebox_id)
 
 = warning
 
@@ -10,7 +9,7 @@
   .burndown-chart.mb-2{ data: { start_date: burndown.start_date.strftime("%Y-%m-%d"),
     due_date: burndown.due_date.strftime("%Y-%m-%d"),
     milestone_id: milestone.to_global_id,
-    burndown_events_path: expose_url(burndown_endpoint), burnup_events_path: expose_url(burnup_endpoint) } }
+    burndown_events_path: expose_url(burndown_endpoint) } }
 
 - elsif show_burndown_placeholder?(milestone, warning)
   .burndown-hint.content-block.container-fluid

ee/lib/ee/api/group_milestones.rb

@@ -20,12 +20,6 @@ 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,16 +17,6 @@ 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_milestone_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,12 +20,6 @@ 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": {
-    "event_type": {
-      "type": { "enum": [ "milestone", "weight", "state" ] }
-    },
-    "weight": {
-      "type": ["integer", "null"]
-    },
-    "milestone_id": {
-      "type": ["integer", "null"]
-    },
-    "state": {
-      "type": ["string", "null"]
-    },
-    "issue_id": {
-      "type": "integer"
-    },
-    "action": {
-      "type": ["string", "null"]
-    },
-    "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,8 +27,4 @@ RSpec.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,8 +24,4 @@ RSpec.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'
-
-RSpec.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(:weight_event1) { create(:resource_weight_event, issue: issue1, weight: 9, created_at: start_date + 2.seconds) }
-  let_it_be(:weight_event2) { create(:resource_weight_event, issue: issue2, weight: 3, created_at: start_date + 3.seconds) }
-  let_it_be(:weight_event3) { create(:resource_weight_event, issue: issue3, weight: 1, created_at: start_date + 2.days) }
-  let_it_be(:weight_event4) { create(:resource_weight_event, issue: issue3, weight: 2, created_at: start_date + 2.days + 23.hours + 59.minutes + 59.seconds) }
-  let_it_be(:weight_event5) { create(:resource_weight_event, issue: issue3, weight: 7, created_at: start_date + 4.days + 1.second) }
-
-  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 + 4.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) }
-
-  let_it_be(:state_event1) { create(:resource_state_event, issue: issue1, state: :opened, created_at: start_date + 5.seconds) }
-  let_it_be(:state_event2) { create(:resource_state_event, issue: issue2, state: :closed, created_at: start_date + 6.seconds) }
-  let_it_be(:state_event3) { create(:resource_state_event, issue: issue3, state: :opened, created_at: start_date + 3.days + 1.second) }
-  let_it_be(:state_event4) { create(:resource_state_event, issue: issue3, state: :closed, created_at: start_date + 3.days + 23.hours + 59.minutes + 58.seconds) }
-  let_it_be(:state_event5) { create(:resource_state_event, issue: issue3, state: :reopened, created_at: start_date + 4.days + 2.seconds) }
-
-  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
-
-      expect(data.size).to eq(17)
-
-      expect(data[0]).to include(event_type: 'milestone', action: 'add', issue_id: issue1.id, milestone_id: milestone2.id, created_at: start_date.beginning_of_day - 1.second)
-      expect(data[1]).to include(event_type: 'milestone', action: 'add', issue_id: issue1.id, milestone_id: milestone1.id, created_at: start_date + 1.second)
-      expect(data[2]).to include(event_type: 'weight', issue_id: issue1.id, weight: 9, created_at: start_date + 2.seconds)
-      expect(data[3]).to include(event_type: 'weight', issue_id: issue2.id, weight: 3, created_at: start_date + 3.seconds)
-      expect(data[4]).to include(event_type: 'milestone', action: 'add', issue_id: issue2.id, milestone_id: milestone1.id, created_at: start_date + 4.seconds)
-      expect(data[5]).to include(event_type: 'state', issue_id: issue1.id, state: 'opened', created_at: start_date + 5.seconds)
-      expect(data[6]).to include(event_type: 'state', issue_id: issue2.id, state: 'closed', created_at: start_date + 6.seconds)
-      expect(data[7]).to include(event_type: 'milestone', action: 'add', issue_id: issue3.id, milestone_id: milestone1.id, created_at: start_date + 1.day)
-      expect(data[8]).to include(event_type: 'weight', issue_id: issue3.id, weight: 1, created_at: start_date + 2.days)
-      expect(data[9]).to include(event_type: 'milestone', action: 'remove', issue_id: issue3.id, milestone_id: milestone1.id, created_at: start_date + 2.days + 1.second)
-      expect(data[10]).to include(event_type: 'weight', issue_id: issue3.id, weight: 2, created_at: start_date + 2.days + 23.hours + 59.minutes + 59.seconds)
-      expect(data[11]).to include(event_type: 'milestone', action: 'add', issue_id: issue3.id, milestone_id: milestone2.id, created_at: start_date + 3.days)
-      expect(data[12]).to include(event_type: 'state', issue_id: issue3.id, state: 'opened', created_at: start_date + 3.days + 1.second)
-      expect(data[13]).to include(event_type: 'state', issue_id: issue3.id, state: 'closed', created_at: start_date + 3.days + 23.hours + 59.minutes + 58.seconds)
-      expect(data[14]).to include(event_type: 'milestone', action: 'remove', issue_id: issue3.id, milestone_id: milestone2.id, created_at: start_date + 4.days)
-      expect(data[15]).to include(event_type: 'weight', issue_id: issue3.id, weight: 7, created_at: start_date + 4.days + 1.second)
-      expect(data[16]).to include(event_type: 'state', issue_id: issue3.id, state: 'reopened', created_at: start_date + 4.days + 2.seconds)
-    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')
-
-      expect(json_response.size).to eq(3)
-
-      expect(json_response.first).to include('issue_id' => issue1.id, 'milestone_id' => milestone.id, 'action' => 'add', 'created_at' => event_time - 1.hour)
-      expect(json_response.second).to include('issue_id' => issue2.id, 'milestone_id' => milestone.id, 'action' => 'add', 'created_at' => event_time + 1.hour)
-      expect(json_response.third).to include('issue_id' => issue1.id, 'milestone_id' => milestone.id, 'action' => 'remove', 'created_at' => event_time + 2.hours)
-    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