Commit 467ac44e authored by Jan Provaznik's avatar Jan Provaznik

Merge branch '213334-backend-chart-data-on-resource-events-pd' into 'master'

Provide milestone burnup chart data for scope committed graph

See merge request gitlab-org/gitlab!28899
parents de3a2179 7e4dbfcb
......@@ -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)
......
......@@ -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
# 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
---
title: Provide milestone burnup chart data for scope committed graph
merge_request: 28899
author:
type: added
......@@ -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
......
......@@ -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
......
......@@ -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
......
{
"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"
}
}
}
{
"type": "array",
"items": { "$ref": "burnup_event.json" }
}
......@@ -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
......@@ -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
# 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
# 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
......@@ -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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment