Commit 15556408 authored by Adam Hegyi's avatar Adam Hegyi

Pass correct Time object to CA DataCollector

This change makes sure that the underlying query works properly if Date
parameters are given. The `created_after` parameter will be casted to
Time to contain the beginning of the day time. The `created_before`
parameter will be casted to Time to contain the end of the day time.
parent 792ff555
...@@ -38,7 +38,7 @@ module CycleAnalyticsParams ...@@ -38,7 +38,7 @@ module CycleAnalyticsParams
end end
def to_utc_time(field) def to_utc_time(field)
date = field.is_a?(Date) ? field : Date.parse(field) date = field.is_a?(Date) || field.is_a?(Time) ? field : Date.parse(field)
date.to_time.utc date.to_time.utc
end end
end end
......
...@@ -234,6 +234,7 @@ export default { ...@@ -234,6 +234,7 @@ export default {
:start-date="startDate" :start-date="startDate"
:end-date="endDate" :end-date="endDate"
:max-date-range="$options.maxDateRange" :max-date-range="$options.maxDateRange"
:include-selected-date="true"
class="js-daterange-picker" class="js-daterange-picker"
@change="setDateRange" @change="setDateRange"
/> />
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
import { GlDaterangePicker, GlSprintf, GlIcon, GlTooltipDirective } from '@gitlab/ui'; import { GlDaterangePicker, GlSprintf, GlIcon, GlTooltipDirective } from '@gitlab/ui';
import { getDayDifference } from '~/lib/utils/datetime_utility'; import { getDayDifference } from '~/lib/utils/datetime_utility';
import { __, sprintf } from '~/locale'; import { __, sprintf } from '~/locale';
import { OFFSET_DATE_BY_ONE } from '../constants';
export default { export default {
components: { components: {
...@@ -45,6 +46,11 @@ export default { ...@@ -45,6 +46,11 @@ export default {
required: false, required: false,
default: 0, default: 0,
}, },
includeSelectedDate: {
type: Boolean,
required: false,
default: false,
},
}, },
data() { data() {
return { return {
...@@ -63,7 +69,8 @@ export default { ...@@ -63,7 +69,8 @@ export default {
}, },
}, },
numberOfDays() { numberOfDays() {
return getDayDifference(this.startDate, this.endDate); const dayDifference = getDayDifference(this.startDate, this.endDate);
return this.includeSelectedDate ? dayDifference + OFFSET_DATE_BY_ONE : dayDifference;
}, },
}, },
}; };
......
...@@ -17,3 +17,5 @@ export const scatterChartLineProps = { ...@@ -17,3 +17,5 @@ export const scatterChartLineProps = {
export const LAST_ACTIVITY_AT = 'last_activity_at'; export const LAST_ACTIVITY_AT = 'last_activity_at';
export const DATE_RANGE_LIMIT = 180; export const DATE_RANGE_LIMIT = 180;
export const OFFSET_DATE_BY_ONE = 1;
---
title: Pass correct Time object to Value Stream Management DataCollector
merge_request: 25885
author:
type: fixed
...@@ -9,12 +9,12 @@ module Gitlab ...@@ -9,12 +9,12 @@ module Gitlab
include ActiveModel::Attributes include ActiveModel::Attributes
MAX_RANGE_DAYS = 180.days.freeze MAX_RANGE_DAYS = 180.days.freeze
DEFAULT_DATE_RANGE = 30.days DEFAULT_DATE_RANGE = 29.days # 30 including Date.today
attr_writer :project_ids attr_writer :project_ids
attribute :created_after, :date attribute :created_after, :datetime
attribute :created_before, :date attribute :created_before, :datetime
attr_accessor :group attr_accessor :group
...@@ -27,12 +27,12 @@ module Gitlab ...@@ -27,12 +27,12 @@ module Gitlab
validate :validate_date_range validate :validate_date_range
def initialize(params = {}, current_user:) def initialize(params = {}, current_user:)
params[:created_before] ||= Date.today.at_end_of_day super(params)
params[:created_after] ||= default_created_after(params[:created_before])
@current_user = current_user self.created_before = (self.created_before || Time.now).at_end_of_day
self.created_after = (created_after || default_created_after).at_beginning_of_day
super(params) @current_user = current_user
end end
def project_ids def project_ids
...@@ -42,8 +42,8 @@ module Gitlab ...@@ -42,8 +42,8 @@ module Gitlab
def to_data_attributes def to_data_attributes
{}.tap do |attrs| {}.tap do |attrs|
attrs[:group] = group_data_attributes if group attrs[:group] = group_data_attributes if group
attrs[:created_after] = created_after.iso8601 attrs[:created_after] = created_after.to_date.iso8601
attrs[:created_before] = created_before.iso8601 attrs[:created_before] = created_before.to_date.iso8601
attrs[:projects] = group_projects(project_ids) if group && project_ids.any? attrs[:projects] = group_projects(project_ids) if group && project_ids.any?
end end
end end
...@@ -90,16 +90,16 @@ module Gitlab ...@@ -90,16 +90,16 @@ module Gitlab
def validate_date_range def validate_date_range
return if created_after.nil? || created_before.nil? return if created_after.nil? || created_before.nil?
if (created_before - created_after).days > MAX_RANGE_DAYS if (created_before - created_after) > MAX_RANGE_DAYS
errors.add(:created_after, s_('CycleAnalytics|The given date range is larger than 180 days')) errors.add(:created_after, s_('CycleAnalytics|The given date range is larger than 180 days'))
end end
end end
def default_created_after(start_date = nil) def default_created_after
if start_date if created_before
(start_date.to_time - DEFAULT_DATE_RANGE).to_datetime (created_before - DEFAULT_DATE_RANGE)
else else
DEFAULT_DATE_RANGE.ago.utc.beginning_of_day DEFAULT_DATE_RANGE.ago
end end
end end
end end
......
...@@ -205,7 +205,6 @@ describe Analytics::CycleAnalytics::StagesController do ...@@ -205,7 +205,6 @@ describe Analytics::CycleAnalytics::StagesController do
end end
include_examples 'cycle analytics data endpoint examples' include_examples 'cycle analytics data endpoint examples'
include_examples 'group permission check on the controller level'
end end
describe 'GET `records`' do describe 'GET `records`' do
......
...@@ -256,9 +256,16 @@ describe 'Group Value Stream Analytics', :js do ...@@ -256,9 +256,16 @@ describe 'Group Value Stream Analytics', :js do
context 'with lots of data', :js do context 'with lots of data', :js do
let!(:issue) { create(:issue, project: project, created_at: 5.days.ago) } let!(:issue) { create(:issue, project: project, created_at: 5.days.ago) }
around do |example|
Timecop.freeze { example.run }
end
before do before do
create_cycle(user, project, issue, mr, milestone, pipeline) create_cycle(user, project, issue, mr, milestone, pipeline)
issue.metrics.update!(first_mentioned_in_commit_at: mr.created_at - 5.hours)
mr.metrics.update!(first_deployed_to_production_at: mr.created_at + 2.hours, merged_at: mr.created_at + 1.hour)
deploy_master(user, project, environment: 'staging') deploy_master(user, project, environment: 'staging')
deploy_master(user, project) deploy_master(user, project)
...@@ -268,10 +275,10 @@ describe 'Group Value Stream Analytics', :js do ...@@ -268,10 +275,10 @@ describe 'Group Value Stream Analytics', :js do
dummy_stages = [ dummy_stages = [
{ title: "Issue", description: "Time before an issue gets scheduled", events_count: 1, median: "5 days" }, { title: "Issue", description: "Time before an issue gets scheduled", events_count: 1, median: "5 days" },
{ title: "Plan", description: "Time before an issue starts implementation", events_count: 0, median: "Not enough data" }, { title: "Plan", description: "Time before an issue starts implementation", events_count: 0, median: "Not enough data" },
{ title: "Code", description: "Time until first merge request", events_count: 0, median: "Not enough data" }, { title: "Code", description: "Time until first merge request", events_count: 1, median: "about 5 hours" },
{ title: "Test", description: "Total test time for all commits/merges", events_count: 0, median: "Not enough data" }, { title: "Test", description: "Total test time for all commits/merges", events_count: 0, median: "Not enough data" },
{ title: "Review", description: "Time between merge request creation and merge/close", events_count: 0, median: "Not enough data" }, { title: "Review", description: "Time between merge request creation and merge/close", events_count: 1, median: "about 1 hour" },
{ title: "Staging", description: "From merge request merge until deploy to production", events_count: 0, median: "Not enough data" }, { title: "Staging", description: "From merge request merge until deploy to production", events_count: 1, median: "about 1 hour" },
{ title: "Total", description: "From issue creation until deploy to production", events_count: 1, median: "5 days" } { title: "Total", description: "From issue creation until deploy to production", events_count: 1, median: "5 days" }
] ]
......
...@@ -68,12 +68,12 @@ describe Gitlab::Analytics::CycleAnalytics::RequestParams do ...@@ -68,12 +68,12 @@ describe Gitlab::Analytics::CycleAnalytics::RequestParams do
end end
end end
it 'casts `created_after` to date' do it 'casts `created_after` to `Time`' do
expect(subject.created_after).to be_a_kind_of(Date) expect(subject.created_after).to be_a_kind_of(Time)
end end
it 'casts `created_before` to date' do it 'casts `created_before` to `Time`' do
expect(subject.created_before).to be_a_kind_of(Date) expect(subject.created_before).to be_a_kind_of(Time)
end end
describe 'optional `project_ids`' do describe 'optional `project_ids`' do
......
...@@ -139,12 +139,12 @@ RSpec.shared_examples 'cycle analytics data endpoint examples' do ...@@ -139,12 +139,12 @@ RSpec.shared_examples 'cycle analytics data endpoint examples' do
end end
end end
context 'when `created_after` is invalid' do context 'when `created_after` is invalid, falls back to default date' do
before do before do
params[:created_after] = 'not-a-date' params[:created_after] = 'not-a-date'
end end
include_examples 'example for invalid parameter' it { expect(subject).to have_gitlab_http_status(:success) }
end end
context 'when `created_before` is invalid' do context 'when `created_before` is invalid' do
......
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