Merge branch '331291-restore-previous-month-perspective' into 'master'

Restore previous month perspective

See merge request gitlab-org/gitlab!63217

config/gitlab.yml.example

@@ -505,7 +505,7 @@ production: &base
   ee_cron_jobs:
     # Schedule snapshots for all devops adoption segments
     analytics_devops_adoption_create_all_snapshots_worker:
-      cron: 0 4 * * 0
+      cron: 0 0 1 * *
 
     # Snapshot active users statistics
     historical_data_worker:

config/initializers/1_settings.rb

@@ -586,7 +586,7 @@ end
 
 Gitlab.ee do
   Settings.cron_jobs['analytics_devops_adoption_create_all_snapshots_worker'] ||= Settingslogic.new({})
-  Settings.cron_jobs['analytics_devops_adoption_create_all_snapshots_worker']['cron'] ||= '0 4 * * 0'
+  Settings.cron_jobs['analytics_devops_adoption_create_all_snapshots_worker']['cron'] ||= '0 0 1 * *'
   Settings.cron_jobs['analytics_devops_adoption_create_all_snapshots_worker']['job_class'] = 'Analytics::DevopsAdoption::CreateAllSnapshotsWorker'
   Settings.cron_jobs['active_user_count_threshold_worker'] ||= Settingslogic.new({})
   Settings.cron_jobs['active_user_count_threshold_worker']['cron'] ||= '0 12 * * *'

doc/api/graphql/reference/index.md

@@ -5033,6 +5033,29 @@ The edge type for [`DevopsAdoptionEnabledNamespace`](#devopsadoptionenablednames
 | <a id="devopsadoptionenablednamespaceedgecursor"></a>`cursor` | [`String!`](#string) | A cursor for use in pagination. |
 | <a id="devopsadoptionenablednamespaceedgenode"></a>`node` | [`DevopsAdoptionEnabledNamespace`](#devopsadoptionenablednamespace) | The item at the end of the edge. |
 
+#### `DevopsAdoptionSnapshotConnection`
+
+The connection type for [`DevopsAdoptionSnapshot`](#devopsadoptionsnapshot).
+
+##### Fields
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="devopsadoptionsnapshotconnectionedges"></a>`edges` | [`[DevopsAdoptionSnapshotEdge]`](#devopsadoptionsnapshotedge) | A list of edges. |
+| <a id="devopsadoptionsnapshotconnectionnodes"></a>`nodes` | [`[DevopsAdoptionSnapshot]`](#devopsadoptionsnapshot) | A list of nodes. |
+| <a id="devopsadoptionsnapshotconnectionpageinfo"></a>`pageInfo` | [`PageInfo!`](#pageinfo) | Information to aid in pagination. |
+
+#### `DevopsAdoptionSnapshotEdge`
+
+The edge type for [`DevopsAdoptionSnapshot`](#devopsadoptionsnapshot).
+
+##### Fields
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="devopsadoptionsnapshotedgecursor"></a>`cursor` | [`String!`](#string) | A cursor for use in pagination. |
+| <a id="devopsadoptionsnapshotedgenode"></a>`node` | [`DevopsAdoptionSnapshot`](#devopsadoptionsnapshot) | The item at the end of the edge. |
+
 #### `DiscussionConnection`
 
 The connection type for [`Discussion`](#discussion).
@@ -8191,9 +8214,28 @@ Enabled namespace for DevopsAdoption.
 | ---- | ---- | ----------- |
 | <a id="devopsadoptionenablednamespacedisplaynamespace"></a>`displayNamespace` | [`Namespace`](#namespace) | Namespace where data should be displayed. |
 | <a id="devopsadoptionenablednamespaceid"></a>`id` | [`ID!`](#id) | ID of the enabled namespace. |
-| <a id="devopsadoptionenablednamespacelatestsnapshot"></a>`latestSnapshot` | [`DevopsAdoptionSnapshot`](#devopsadoptionsnapshot) | The latest adoption metrics for the enabled namespace. |
+| <a id="devopsadoptionenablednamespacelatestsnapshot"></a>`latestSnapshot` | [`DevopsAdoptionSnapshot`](#devopsadoptionsnapshot) | Metrics snapshot for previous month for the enabled namespace. |
 | <a id="devopsadoptionenablednamespacenamespace"></a>`namespace` | [`Namespace`](#namespace) | Namespace which should be calculated. |
 
+#### Fields with arguments
+
+##### `DevopsAdoptionEnabledNamespace.snapshots`
+
+Data snapshots of the namespace.
+
+Returns [`DevopsAdoptionSnapshotConnection`](#devopsadoptionsnapshotconnection).
+
+This field returns a [connection](#connections). It accepts the
+four standard [pagination arguments](#connection-pagination-arguments):
+`before: String`, `after: String`, `first: Int`, `last: Int`.
+
+###### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="devopsadoptionenablednamespacesnapshotsendtimeafter"></a>`endTimeAfter` | [`Time`](#time) | Filter to snapshots with month end after the provided date. |
+| <a id="devopsadoptionenablednamespacesnapshotsendtimebefore"></a>`endTimeBefore` | [`Time`](#time) | Filter to snapshots with month end before the provided date. |
+
 ### `DevopsAdoptionSnapshot`
 
 Snapshot.

ee/app/assets/javascripts/analytics/devops_report/devops_adoption/constants.js

@@ -19,7 +19,7 @@ export const DEVOPS_ADOPTION_ERROR_KEYS = {
 };
 
 export const TABLE_HEADER_TEXT = s__(
-  'DevopsAdoption|Feature adoption is based on usage in the current calendar month. Last updated: %{timestamp}.',
+  'DevopsAdoption|Feature adoption is based on usage in the previous calendar month. Last updated: %{timestamp}.',
 );
 
 export const DEVOPS_ADOPTION_GROUP_LEVEL_LABEL = s__('DevopsAdoption|Add/remove sub-groups');

ee/app/finders/analytics/devops_adoption/snapshots_finder.rb

+# frozen_string_literal: true
+
+module Analytics
+  module DevopsAdoption
+    class SnapshotsFinder
+      attr_reader :params
+
+      def initialize(params:)
+        @params = params
+      end
+
+      def execute
+        scope = Snapshot.by_end_time
+        scope = by_namespace(scope)
+        by_timespan(scope)
+      end
+
+      private
+
+      def by_namespace(scope)
+        scope.for_namespaces(params[:namespace_id])
+      end
+
+      def by_timespan(scope)
+        scope.for_timespan(from: params[:end_time_after], to: params[:end_time_before])
+      end
+    end
+  end
+end

ee/app/graphql/resolvers/analytics/devops_adoption/enabled_namespaces_resolver.rb

@@ -6,6 +6,7 @@ module Resolvers
       class EnabledNamespacesResolver < BaseResolver
         include Gitlab::Graphql::Authorize::AuthorizeResource
         include Gitlab::Allowable
+        include LooksAhead
 
         type Types::Analytics::DevopsAdoption::EnabledNamespaceType, null: true
 
@@ -13,19 +14,25 @@ module Resolvers
                  required: false,
                  description: 'Filter by display namespace.'
 
-        def resolve(display_namespace_id: nil, **)
+        def resolve_with_lookahead(display_namespace_id: nil, **)
           display_namespace_id = GlobalID.parse(display_namespace_id)
           display_namespace = Gitlab::Graphql::Lazy.force(GitlabSchema.find_by_gid(display_namespace_id))
 
           authorize!(display_namespace)
 
-          ::Analytics::DevopsAdoption::EnabledNamespacesFinder.new(current_user, params: {
-            display_namespace: display_namespace
-          }).execute
+          apply_lookahead(finder_class.new(current_user, params: { display_namespace: display_namespace }).execute)
+        end
+
+        def unconditional_includes
+          [:display_namespace]
         end
 
         private
 
+        def finder_class
+          ::Analytics::DevopsAdoption::EnabledNamespacesFinder
+        end
+
         def authorize!(display_namespace)
           display_namespace ? authorize_with_namespace!(display_namespace) : authorize_global!
         end

ee/app/graphql/resolvers/analytics/devops_adoption/snapshots_resolver.rb

+# frozen_string_literal: true
+
+module Resolvers
+  module Analytics
+    module DevopsAdoption
+      class SnapshotsResolver < BaseResolver
+        include Gitlab::Allowable
+
+        type Types::Analytics::DevopsAdoption::SnapshotType.connection_type, null: true
+
+        argument :end_time_before,
+                 ::Types::TimeType,
+                 required: false,
+                 description: 'Filter to snapshots with month end before the provided date.'
+
+        argument :end_time_after,
+                 ::Types::TimeType,
+                 required: false,
+                 description: 'Filter to snapshots with month end after the provided date.'
+
+        def resolve(end_time_after: nil, end_time_before: nil)
+          return [] unless authorize(object)
+
+          params = {
+            end_time_after: end_time_after,
+            end_time_before: end_time_before,
+            namespace_id: object.namespace_id
+          }
+
+          ::Analytics::DevopsAdoption::SnapshotsFinder.new(params: params).execute
+        end
+
+        private
+
+        def authorize(enabled_namespace)
+          if enabled_namespace.display_namespace
+            can?(current_user, :view_group_devops_adoption, enabled_namespace.display_namespace)
+          else
+            can?(current_user, :view_instance_devops_adoption, :global)
+          end
+        end
+      end
+    end
+  end
+end

ee/app/graphql/types/analytics/devops_adoption/enabled_namespace_type.rb

@@ -17,11 +17,15 @@ module Types
         field :display_namespace, Types::NamespaceType, null: true,
               description: 'Namespace where data should be displayed.'
 
+        field :snapshots,
+              description: 'Data snapshots of the namespace.',
+              resolver: Resolvers::Analytics::DevopsAdoption::SnapshotsResolver
+
         field :latest_snapshot, SnapshotType, null: true,
-              description: 'The latest adoption metrics for the enabled namespace.'
+              description: 'Metrics snapshot for previous month for the enabled namespace.'
 
         def latest_snapshot
-          BatchLoader::GraphQL.for(object.namespace_id).batch(key: :devops_adoption_latest_snapshots) do |ids, loader, args|
+          BatchLoader::GraphQL.for(object.namespace_id).batch(key: :devops_adoption_latest_snapshots) do |ids, loader, _args|
             snapshots = ::Analytics::DevopsAdoption::Snapshot
               .latest_snapshot_for_namespace_ids(ids)
               .index_by(&:namespace_id)

ee/app/models/analytics/devops_adoption/snapshot.rb

@@ -25,6 +25,7 @@ class Analytics::DevopsAdoption::Snapshot < ApplicationRecord
   scope :latest_snapshot_for_namespace_ids, -> (ids) do
     inner_select = model
       .default_scoped
+      .finalized
       .distinct
       .select("FIRST_VALUE(id) OVER (PARTITION BY namespace_id ORDER BY end_time DESC) as id")
       .where(namespace_id: ids)
@@ -33,9 +34,13 @@ class Analytics::DevopsAdoption::Snapshot < ApplicationRecord
   end
 
   scope :for_month, -> (month_date) { where(end_time: month_date.end_of_month) }
-  scope :not_finalized, -> { where(arel_table[:recorded_at].lteq(arel_table[:end_time])) }
+  scope :not_finalized, -> { where(arel_table[:recorded_at].lt(arel_table[:end_time])) }
+  scope :finalized, -> { where(arel_table[:recorded_at].gteq(arel_table[:end_time])) }
   scope :by_end_time, -> { order(end_time: :desc) }
 
+  scope :for_timespan, -> (from: nil, to: nil) { where(end_time: from..to) }
+  scope :for_namespaces, -> (ids) { where(namespace: ids) }
+
   def start_time
     end_time.beginning_of_month
   end

ee/app/workers/analytics/devops_adoption/create_snapshot_worker.rb

@@ -3,7 +3,6 @@
 module Analytics
   module DevopsAdoption
     # Updates all pending snapshots for given enabled_namespace (from previous month)
-    # and creates or update snapshot for current month
     class CreateSnapshotWorker
       include ApplicationWorker
 
@@ -27,10 +26,9 @@ module Analytics
       def pending_ranges(enabled_namespace)
         end_times = enabled_namespace.snapshots.not_finalized.pluck(:end_time)
 
-        now = Time.zone.now
-
-        if !now.end_of_month.to_date.in?(end_times.map(&:to_date)) && now.day > 1
-          end_times << now.end_of_month
+        prev_month = Time.current.last_month.end_of_month
+        unless prev_month.to_date.in?(end_times.map(&:to_date)) || enabled_namespace.snapshots.for_month(prev_month).exists?
+          end_times << prev_month
         end
 
         end_times

ee/spec/finders/analytics/devops_adoption/snapshots_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Analytics::DevopsAdoption::SnapshotsFinder do
+  let_it_be(:enabled_namespace) { create(:devops_adoption_enabled_namespace) }
+  let_it_be(:first_end_time) { 1.year.ago.end_of_month }
+  let_it_be(:snapshot1) { create(:devops_adoption_snapshot, namespace_id: enabled_namespace.namespace_id, end_time: first_end_time) }
+  let_it_be(:snapshot2) do
+    create(:devops_adoption_snapshot, namespace_id: enabled_namespace.namespace_id, end_time: 2.months.after(first_end_time).end_of_month)
+  end
+
+  let_it_be(:snapshot3) do
+    create(:devops_adoption_snapshot, namespace_id: enabled_namespace.namespace_id, end_time: 3.months.after(first_end_time).end_of_month)
+  end
+
+  let(:finder) { described_class.new(params: params) }
+
+  let(:params) { { namespace_id: enabled_namespace.namespace_id } }
+
+  describe '#execute' do
+    subject(:snapshots) { finder.execute }
+
+    context 'with timespan provided' do
+      before do
+        params[:end_time_before] = 1.day.before(snapshot3.end_time)
+        params[:end_time_after] = 1.day.after(first_end_time)
+      end
+
+      it 'returns snapshots in given timespan' do
+        expect(snapshots).to match_array([snapshot2])
+      end
+    end
+
+    context 'without timespan provided' do
+      it 'returns all snapshots ordered by end_time' do
+        expect(snapshots).to eq([snapshot3, snapshot2, snapshot1])
+      end
+    end
+  end
+end

ee/spec/frontend/analytics/devops_report/devops_adoption/components/devops_adoption_section_spec.js

@@ -92,7 +92,7 @@ describe('DevopsAdoptionSection', () => {
       createComponent();
 
       const text =
-        'Feature adoption is based on usage in the current calendar month. Last updated: 2020-10-31 23:59.';
+        'Feature adoption is based on usage in the previous calendar month. Last updated: 2020-10-31 23:59.';
       expect(getByText(wrapper.element, text)).not.toBeNull();
     });
 

ee/spec/models/analytics/devops_adoption/snapshot_spec.rb

@@ -10,14 +10,14 @@ RSpec.describe Analytics::DevopsAdoption::Snapshot, type: :model do
   it { is_expected.to validate_presence_of(:end_time) }
 
   describe '.latest_snapshot_for_namespace_ids' do
-    it 'returns the latest snapshot for the given namespace ids based on snapshot end_time' do
+    it 'returns the latest finalized snapshot for the given namespace ids based on snapshot end_time' do
       group1 = create(:group)
-      group1_latest_snapshot = create(:devops_adoption_snapshot, namespace: group1, end_time: 1.week.ago)
-      create(:devops_adoption_snapshot, namespace: group1, end_time: 2.weeks.ago)
+      group1_latest_snapshot = create(:devops_adoption_snapshot, namespace: group1, end_time: 1.week.ago, recorded_at: 1.day.ago)
+      create(:devops_adoption_snapshot, namespace: group1, end_time: 2.weeks.ago, recorded_at: 1.day.ago)
 
       group2 = create(:group)
-      group2_latest_snapshot = create(:devops_adoption_snapshot, namespace: group2, end_time: 1.year.ago)
-      create(:devops_adoption_snapshot, namespace: group2, end_time: 2.years.ago)
+      group2_latest_snapshot = create(:devops_adoption_snapshot, namespace: group2, end_time: 1.year.ago, recorded_at: 1.day.ago)
+      create(:devops_adoption_snapshot, namespace: group2, end_time: 2.years.ago, recorded_at: 1.day.ago)
 
       latest_snapshots = described_class.latest_snapshot_for_namespace_ids([group1.id, group2.id])
 
@@ -46,6 +46,38 @@ RSpec.describe Analytics::DevopsAdoption::Snapshot, type: :model do
     end
   end
 
+  describe '.finalized' do
+    it 'returns all snapshots which were recorded later than snapshot end_time' do
+      create(:devops_adoption_snapshot, recorded_at: 1.day.ago, end_time: Time.zone.now)
+      snapshot1 = create(:devops_adoption_snapshot, recorded_at: 1.day.ago, end_time: 2.days.ago)
+
+      expect(described_class.finalized).to match_array([snapshot1])
+    end
+  end
+
+  describe '.for_timespan' do
+    let_it_be(:first_date) { DateTime.parse('2021-05-10').end_of_month }
+    let_it_be(:snapshot1) { create(:devops_adoption_snapshot, recorded_at: 1.day.ago, end_time: first_date)}
+    let_it_be(:snapshot2) { create(:devops_adoption_snapshot, recorded_at: 1.day.ago, end_time: first_date + 1.month)}
+    let_it_be(:snapshot3) { create(:devops_adoption_snapshot, recorded_at: 1.day.ago, end_time: first_date + 2.months)}
+
+    it 'returns snapshots for given timespan', :aggregate_failures do
+      expect(described_class.for_timespan(to: first_date + 1.week)).to match_array([snapshot1])
+      expect(described_class.for_timespan(from: first_date + 1.week)).to match_array([snapshot2, snapshot3])
+      expect(described_class.for_timespan(from: first_date + 1.week, to: first_date + 40.days)).to match_array([snapshot2])
+    end
+  end
+
+  describe '.for_namespaces' do
+    it 'returns all snapshots with given namespaces' do
+      snapshot1 = create(:devops_adoption_snapshot)
+      snapshot2 = create(:devops_adoption_snapshot)
+      create(:devops_adoption_snapshot)
+
+      expect(described_class.for_namespaces([snapshot1.namespace, snapshot2.namespace])).to match_array([snapshot1, snapshot2])
+    end
+  end
+
   describe '#start_time' do
     subject(:snapshot) { described_class.new(end_time: end_time) }
 

ee/spec/requests/api/graphql/analytics/devops_adoption/enabled_namespaces_spec.rb

@@ -26,6 +26,12 @@ RSpec.describe 'devopsAdoptionEnabledNamespaces' do
         displayNamespace {
           name
         }
+        snapshots {
+          nodes {
+            issueOpened
+            mergeRequestOpened
+          }
+        }
         latestSnapshot {
           issueOpened
           mergeRequestOpened
@@ -46,6 +52,10 @@ RSpec.describe 'devopsAdoptionEnabledNamespaces' do
         'id' => enabled_namespace.to_gid.to_s,
         'namespace' => { 'name' => group.name },
         'displayNamespace' => { 'name' => group.name },
+        'snapshots' => { 'nodes' => [{
+          'mergeRequestOpened' => false,
+          'issueOpened' => true
+        }] },
         'latestSnapshot' => {
           'mergeRequestOpened' => false,
           'issueOpened' => true

ee/spec/workers/analytics/devops_adoption/create_snapshot_worker_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe Analytics::DevopsAdoption::CreateSnapshotWorker do
 
     let(:service_mock) { instance_double('Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService', execute: true) }
 
-    it 'updates metrics for all not finalized snapshots and current month' do
+    it 'updates metrics for all not finalized snapshots and previous month' do
       freeze_time do
         allow_next_instance_of(Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService, enabled_namespace: enabled_namespace, range_end: pending_snapshot.end_time) do |instance|
           expect(instance).to receive(:execute)
@@ -32,7 +32,7 @@ RSpec.describe Analytics::DevopsAdoption::CreateSnapshotWorker do
         allow_next_instance_of(Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService, enabled_namespace: enabled_namespace, range_end: finalized_snapshot.end_time) do |instance|
           expect(instance).not_to receive(:execute)
         end
-        allow_next_instance_of(Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService, enabled_namespace: enabled_namespace, range_end: Time.zone.now.end_of_month) do |instance|
+        allow_next_instance_of(Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService, enabled_namespace: enabled_namespace, range_end: (Time.zone.now - 1.month).end_of_month) do |instance|
           expect(instance).to receive(:execute)
         end
 
@@ -40,9 +40,9 @@ RSpec.describe Analytics::DevopsAdoption::CreateSnapshotWorker do
       end
     end
 
-    context 'when metric for current month already exists' do
-      it 'calls for current month calculation only once' do
-        travel_to(pending_snapshot.recorded_at + 1.day) do
+    context 'when pending metric for previous month already exists' do
+      it 'calls for previous month calculation only once' do
+        travel_to(pending_snapshot.recorded_at + 1.month) do
           allow_next_instance_of(Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService, enabled_namespace: enabled_namespace, range_end: pending_snapshot.end_time) do |instance|
             expect(instance).to receive(:execute).once
           end
@@ -55,18 +55,15 @@ RSpec.describe Analytics::DevopsAdoption::CreateSnapshotWorker do
       end
     end
 
-    context 'when today is first day of the month' do
-      it 'doesnt update metrics for current month' do
-        travel_to((pending_snapshot.recorded_at + 1.month).beginning_of_month) do
-          allow_next_instance_of(Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService, enabled_namespace: enabled_namespace, range_end: pending_snapshot.end_time) do |instance|
-            expect(instance).to receive(:execute)
+    context 'when metric for previous month already finalized' do
+      it 'does not call for previous month calculation' do
+        travel_to(finalized_snapshot.recorded_at + 1.month) do
+          allow_next_instance_of(Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService) do |instance|
+            allow(instance).to receive(:execute).and_call_original
           end
           allow_next_instance_of(Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService, enabled_namespace: enabled_namespace, range_end: finalized_snapshot.end_time) do |instance|
             expect(instance).not_to receive(:execute)
           end
-          allow_next_instance_of(Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService, enabled_namespace: enabled_namespace, range_end: Time.zone.now.end_of_month) do |instance|
-            expect(instance).not_to receive(:execute)
-          end
 
           worker.perform(enabled_namespace.id)
         end

locale/gitlab.pot

@@ -11294,7 +11294,7 @@ msgstr ""
 msgid "DevopsAdoption|DevOps adoption tracks the use of key features across your favorite groups. Add a group to the table to begin."
 msgstr ""
 
-msgid "DevopsAdoption|Feature adoption is based on usage in the current calendar month. Last updated: %{timestamp}."
+msgid "DevopsAdoption|Feature adoption is based on usage in the previous calendar month. Last updated: %{timestamp}."
 msgstr ""
 
 msgid "DevopsAdoption|Filter by name"