Add cummulative approach to devops adoption calculations

We shouldn't recalculate metrics which were already evaluated
as true for given period. It will allow us to schedule
metrics update more frequently without performance overhead

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

@@ -24,6 +24,8 @@ class Analytics::DevopsAdoption::Snapshot < ApplicationRecord
     joins("INNER JOIN (#{inner_select.to_sql}) latest_snapshots ON latest_snapshots.id = analytics_devops_adoption_snapshots.id")
   end
 
+  scope :for_month, -> (month_date) { where(end_time: month_date.end_of_month) }
+
   def start_time
     end_time.beginning_of_month
   end

ee/app/services/analytics/devops_adoption/snapshots/calculate_and_save_service.rb

@@ -12,7 +12,19 @@ module Analytics
         end
 
         def execute
-          CreateService.new(params: SnapshotCalculator.new(segment: segment, range_end: range_end).calculate).execute
+          if snapshot
+            UpdateService.new(snapshot: snapshot, params: calculated_data).execute
+          else
+            CreateService.new(params: calculated_data).execute
+          end
+        end
+
+        def snapshot
+          @snapshot ||= segment.snapshots.for_month(range_end).first
+        end
+
+        def calculated_data
+          @calculated_data ||= SnapshotCalculator.new(segment: segment, range_end: range_end, snapshot: snapshot).calculate
         end
       end
     end

ee/app/services/analytics/devops_adoption/snapshots/create_service.rb

@@ -3,54 +3,9 @@
 module Analytics
   module DevopsAdoption
     module Snapshots
-      class CreateService
-        ALLOWED_ATTRIBUTES = [
-          :segment,
-          :segment_id,
-          :end_time,
-          :recorded_at,
-          :issue_opened,
-          :merge_request_opened,
-          :merge_request_approved,
-          :runner_configured,
-          :pipeline_succeeded,
-          :deploy_succeeded,
-          :security_scan_succeeded
-        ].freeze
-
-        def initialize(snapshot: Analytics::DevopsAdoption::Snapshot.new, params: {})
-          @snapshot = snapshot
-          @params = params
-        end
-
-        def execute
-          success = false
-
-          ActiveRecord::Base.transaction do
-            snapshot.assign_attributes(attributes)
-
-            success = snapshot.save && snapshot.segment.update(last_recorded_at: snapshot.recorded_at)
-
-            raise ActiveRecord::Rollback unless success
-          end
-
-          if success
-            ServiceResponse.success(payload: response_payload)
-          else
-            ServiceResponse.error(message: 'Validation error', payload: response_payload)
-          end
-        end
-
-        private
-
-        attr_reader :snapshot, :params
-
-        def response_payload
-          { snapshot: snapshot }
-        end
-
-        def attributes
-          params.slice(*ALLOWED_ATTRIBUTES)
+      class CreateService < UpdateService
+        def initialize(**args)
+          super(**{ snapshot: Snapshot.new }.merge(args))
         end
       end
     end

ee/app/services/analytics/devops_adoption/snapshots/update_service.rb

+# frozen_string_literal: true
+
+module Analytics
+  module DevopsAdoption
+    module Snapshots
+      class UpdateService
+        ALLOWED_ATTRIBUTES = [
+          :segment,
+          :segment_id,
+          :end_time,
+          :recorded_at,
+          :issue_opened,
+          :merge_request_opened,
+          :merge_request_approved,
+          :runner_configured,
+          :pipeline_succeeded,
+          :deploy_succeeded,
+          :security_scan_succeeded
+        ].freeze
+
+        def initialize(snapshot:, params: {})
+          @snapshot = snapshot
+          @params = params
+        end
+
+        def execute
+          success = false
+
+          ActiveRecord::Base.transaction do
+            snapshot.assign_attributes(attributes)
+
+            success = snapshot.save && snapshot.segment.update(last_recorded_at: snapshot.recorded_at)
+
+            raise ActiveRecord::Rollback unless success
+          end
+
+          if success
+            ServiceResponse.success(payload: response_payload)
+          else
+            ServiceResponse.error(message: 'Validation error', payload: response_payload)
+          end
+        end
+
+        private
+
+        attr_reader :snapshot, :params
+
+        def response_payload
+          { snapshot: snapshot }
+        end
+
+        def attributes
+          params.slice(*ALLOWED_ATTRIBUTES)
+        end
+      end
+    end
+  end
+end

ee/lib/analytics/devops_adoption/snapshot_calculator.rb

@@ -3,21 +3,22 @@
 module Analytics
   module DevopsAdoption
     class SnapshotCalculator
-      attr_reader :segment, :range_end, :range_start
+      attr_reader :segment, :range_end, :range_start, :snapshot
 
       ADOPTION_FLAGS = %i[issue_opened merge_request_opened merge_request_approved runner_configured pipeline_succeeded deploy_succeeded security_scan_succeeded].freeze
 
-      def initialize(segment:, range_end:)
+      def initialize(segment:, range_end:, snapshot: nil)
         @segment = segment
         @range_end = range_end
         @range_start = Snapshot.new(end_time: range_end).start_time
+        @snapshot = snapshot
       end
 
       def calculate
         params = { recorded_at: Time.zone.now, end_time: range_end, segment: segment }
 
         ADOPTION_FLAGS.each do |flag|
-          params[flag] = send(flag) # rubocop:disable GitlabSecurity/PublicSend
+          params[flag] = snapshot&.public_send(flag) || send(flag) # rubocop:disable GitlabSecurity/PublicSend
         end
 
         params

ee/spec/lib/analytics/devops_adoption/snapshot_calculator_spec.rb

@@ -134,4 +134,20 @@ RSpec.describe Analytics::DevopsAdoption::SnapshotCalculator do
 
     it { is_expected.to eq false }
   end
+
+  context 'when snapshot already exists' do
+    let_it_be(:snapshot) { create :devops_adoption_snapshot, segment: segment, issue_opened: true, merge_request_opened: false }
+
+    subject(:data) { described_class.new(segment: segment, range_end: range_end, snapshot: snapshot).calculate }
+
+    let!(:fresh_merge_request) { create(:merge_request, source_project: project2, created_at: 3.weeks.ago(range_end)) }
+
+    it 'calculates metrics which are not true yet' do
+      expect(data[:merge_request_opened]).to eq true
+    end
+
+    it "doesn't change metrics which are true already" do
+      expect(data[:issue_opened]).to eq true
+    end
+  end
 end

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

@@ -25,6 +25,18 @@ RSpec.describe Analytics::DevopsAdoption::Snapshot, type: :model do
     end
   end
 
+  describe '.for_month' do
+    it 'returns all snapshots where end_time equals given datetime end of month' do
+      end_of_month = Time.zone.now.end_of_month
+      snapshot1 = create(:devops_adoption_snapshot, end_time: end_of_month)
+      create(:devops_adoption_snapshot, end_time: end_of_month - 1.year)
+
+      expect(described_class.for_month(end_of_month)).to match_array([snapshot1])
+      expect(described_class.for_month(end_of_month - 10.days)).to match_array([snapshot1])
+      expect(described_class.for_month(end_of_month.beginning_of_month)).to match_array([snapshot1])
+    end
+  end
+
   describe '#start_time' do
     subject(:segment) { described_class.new(end_time: end_time) }
 

ee/spec/services/analytics/devops_adoption/snapshots/calculate_and_save_service_spec.rb

@@ -3,19 +3,36 @@
 require 'spec_helper'
 
 RSpec.describe Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService do
-  let(:segment_mock) { instance_double('Analytics::DevopsAdoption::Segment') }
+  let_it_be(:segment) { create :devops_adoption_segment }
 
-  subject { described_class.new(segment: segment_mock, range_end: Time.zone.now.end_of_month) }
+  subject { described_class.new(segment: segment, range_end: range_end) }
 
-  it 'creates a snapshot with whatever snapshot calculator returns' do
-    allow_next_instance_of(Analytics::DevopsAdoption::SnapshotCalculator) do |calc|
+  let(:range_end) { Time.zone.now.end_of_month }
+  let(:snapshot) { nil }
+
+  before do
+    allow_next_instance_of(Analytics::DevopsAdoption::SnapshotCalculator, segment: segment, range_end: range_end, snapshot: snapshot) do |calc|
       allow(calc).to receive(:calculate).and_return('calculated_data')
     end
+  end
 
+  it 'creates a snapshot with whatever snapshot calculator returns' do
     expect_next_instance_of(Analytics::DevopsAdoption::Snapshots::CreateService, params: 'calculated_data') do |service|
       expect(service).to receive(:execute).and_return('create_service_response')
     end
 
     expect(subject.execute).to eq('create_service_response')
   end
+
+  context 'when a snapshot for given range already exists' do
+    let(:snapshot) { create :devops_adoption_snapshot, end_time: range_end, segment: segment }
+
+    it 'updates the snapshot with whatever snapshot calculator returns' do
+      expect_next_instance_of(Analytics::DevopsAdoption::Snapshots::UpdateService, snapshot: snapshot, params: 'calculated_data') do |service|
+        expect(service).to receive(:execute).and_return('update_service_response')
+      end
+
+      expect(subject.execute).to eq('update_service_response')
+    end
+  end
 end

ee/spec/services/analytics/devops_adoption/snapshots/update_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Analytics::DevopsAdoption::Snapshots::UpdateService do
+  subject(:service_response) { described_class.new(snapshot: snapshot, params: params).execute }
+
+  let(:snapshot) { create(:devops_adoption_snapshot, segment: segment) }
+  let(:segment) { create(:devops_adoption_segment, last_recorded_at: 1.year.ago) }
+
+  let(:params) do
+    params = Analytics::DevopsAdoption::SnapshotCalculator::ADOPTION_FLAGS.each_with_object({}) do |attribute, result|
+      result[attribute] = rand(2).odd?
+    end
+    params[:recorded_at] = Time.zone.now
+    params[:segment] = segment
+    params
+  end
+
+  it 'updates the snapshot & updates segment last recorded at date' do
+    expect(subject).to be_success
+    expect(snapshot).to have_attributes(params)
+    expect(snapshot.segment.reload.last_recorded_at).to be_like_time(snapshot.recorded_at)
+  end
+
+  context 'when params are invalid' do
+    let(:params) { super().merge(recorded_at: nil) }
+
+    it 'does not update the snapshot' do
+      expect(subject).to be_error
+      expect(subject.message).to eq('Validation error')
+      expect(snapshot.reload.recorded_at).not_to be_nil
+      expect(snapshot.segment.reload.last_recorded_at).not_to eq nil
+    end
+  end
+end