Merge branch 'fix-vsm-created-at-filter-condition' into 'master'

Pass correct Time object to VSM DataCollector

See merge request gitlab-org/gitlab!25885

app/controllers/concerns/cycle_analytics_params.rb

@@ -38,7 +38,7 @@ module CycleAnalyticsParams
   end
 
   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
   end
 end

ee/app/assets/javascripts/analytics/cycle_analytics/components/base.vue

@@ -234,6 +234,7 @@ export default {
             :start-date="startDate"
             :end-date="endDate"
             :max-date-range="$options.maxDateRange"
+            :include-selected-date="true"
             class="js-daterange-picker"
             @change="setDateRange"
           />

ee/app/assets/javascripts/analytics/shared/components/daterange.vue

@@ -2,6 +2,7 @@
 import { GlDaterangePicker, GlSprintf, GlIcon, GlTooltipDirective } from '@gitlab/ui';
 import { getDayDifference } from '~/lib/utils/datetime_utility';
 import { __, sprintf } from '~/locale';
+import { OFFSET_DATE_BY_ONE } from '../constants';
 
 export default {
   components: {
@@ -45,6 +46,11 @@ export default {
       required: false,
       default: 0,
     },
+    includeSelectedDate: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   data() {
     return {
@@ -63,7 +69,8 @@ export default {
       },
     },
     numberOfDays() {
-      return getDayDifference(this.startDate, this.endDate);
+      const dayDifference = getDayDifference(this.startDate, this.endDate);
+      return this.includeSelectedDate ? dayDifference + OFFSET_DATE_BY_ONE : dayDifference;
     },
   },
 };

ee/app/assets/javascripts/analytics/shared/constants.js

@@ -17,3 +17,5 @@ export const scatterChartLineProps = {
 export const LAST_ACTIVITY_AT = 'last_activity_at';
 
 export const DATE_RANGE_LIMIT = 180;
+
+export const OFFSET_DATE_BY_ONE = 1;

ee/changelogs/unreleased/fix-vsm-created-at-filter-condition.yml

+---
+title: Pass correct Time object to Value Stream Management DataCollector
+merge_request: 25885
+author:
+type: fixed

ee/lib/gitlab/analytics/cycle_analytics/request_params.rb

@@ -9,12 +9,12 @@ module Gitlab
         include ActiveModel::Attributes
 
         MAX_RANGE_DAYS = 180.days.freeze
-        DEFAULT_DATE_RANGE = 30.days
+        DEFAULT_DATE_RANGE = 29.days # 30 including Date.today
 
         attr_writer :project_ids
 
-        attribute :created_after, :date
-        attribute :created_before, :date
+        attribute :created_after, :datetime
+        attribute :created_before, :datetime
 
         attr_accessor :group
 
@@ -27,12 +27,12 @@ module Gitlab
         validate :validate_date_range
 
         def initialize(params = {}, current_user:)
-          params[:created_before] ||= Date.today.at_end_of_day
-          params[:created_after] ||= default_created_after(params[:created_before])
+          super(params)
 
-          @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
 
         def project_ids
@@ -42,8 +42,8 @@ module Gitlab
         def to_data_attributes
           {}.tap do |attrs|
             attrs[:group] = group_data_attributes if group
-            attrs[:created_after] = created_after.iso8601
-            attrs[:created_before] = created_before.iso8601
+            attrs[:created_after] = created_after.to_date.iso8601
+            attrs[:created_before] = created_before.to_date.iso8601
             attrs[:projects] = group_projects(project_ids) if group && project_ids.any?
           end
         end
@@ -90,16 +90,16 @@ module Gitlab
         def validate_date_range
           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'))
           end
         end
 
-        def default_created_after(start_date = nil)
-          if start_date
-            (start_date.to_time - DEFAULT_DATE_RANGE).to_datetime
+        def default_created_after
+          if created_before
+            (created_before - DEFAULT_DATE_RANGE)
           else
-            DEFAULT_DATE_RANGE.ago.utc.beginning_of_day
+            DEFAULT_DATE_RANGE.ago
           end
         end
       end

ee/spec/controllers/analytics/cycle_analytics/stages_controller_spec.rb

@@ -205,7 +205,6 @@ describe Analytics::CycleAnalytics::StagesController do
       end
 
       include_examples 'cycle analytics data endpoint examples'
-      include_examples 'group permission check on the controller level'
     end
 
     describe 'GET `records`' do

ee/spec/features/analytics/cycle_analytics/cycle_analytics_spec.rb

@@ -256,9 +256,16 @@ describe 'Group Value Stream Analytics', :js do
   context 'with lots of data', :js do
     let!(:issue) { create(:issue, project: project, created_at: 5.days.ago) }
 
+    around do |example|
+      Timecop.freeze { example.run }
+    end
+
     before do
       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)
 
@@ -268,10 +275,10 @@ describe 'Group Value Stream Analytics', :js do
     dummy_stages = [
       { 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: "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: "Review", description: "Time between merge request creation and merge/close", events_count: 0, median: "Not enough data" },
-      { title: "Staging", description: "From merge request merge until deploy to production", 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: 1, median: "about 1 hour" },
       { title: "Total", description: "From issue creation until deploy to production", events_count: 1, median: "5 days" }
     ]
 

ee/spec/lib/gitlab/analytics/cycle_analytics/request_params_spec.rb

@@ -68,12 +68,12 @@ describe Gitlab::Analytics::CycleAnalytics::RequestParams do
     end
   end
 
-  it 'casts `created_after` to date' do
-    expect(subject.created_after).to be_a_kind_of(Date)
+  it 'casts `created_after` to `Time`' do
+    expect(subject.created_after).to be_a_kind_of(Time)
   end
 
-  it 'casts `created_before` to date' do
-    expect(subject.created_before).to be_a_kind_of(Date)
+  it 'casts `created_before` to `Time`' do
+    expect(subject.created_before).to be_a_kind_of(Time)
   end
 
   describe 'optional `project_ids`' do

ee/spec/support/shared_examples/controllers/analytics/cycle_analytics/shared_stage_shared_examples.rb

@@ -139,12 +139,12 @@ RSpec.shared_examples 'cycle analytics data endpoint examples' do
     end
   end
 
-  context 'when `created_after` is invalid' do
+  context 'when `created_after` is invalid, falls back to default date' do
     before do
       params[:created_after] = 'not-a-date'
     end
 
-    include_examples 'example for invalid parameter'
+    it { expect(subject).to have_gitlab_http_status(:success) }
   end
 
   context 'when `created_before` is invalid' do