Merge branch...

Merge branch '323153-move-product-intelligence-danger-bot-logic-into-a-class-helper-2' into 'master'

Resolve "Move Product Intelligence Danger bot logic into a class/helper"

See merge request gitlab-org/gitlab!61539

danger/plugins/product_intelligence.rb

+# frozen_string_literal: true
+
+require_relative '../../tooling/danger/product_intelligence'
+
+module Danger
+  class ProductIntelligence < ::Danger::Plugin
+    # Put the helper code somewhere it can be tested
+    include Tooling::Danger::ProductIntelligence
+  end
+end

danger/product_intelligence/Dangerfile

@@ -18,82 +18,16 @@ UPDATE_DICTIONARY_MESSAGE = <<~MSG
   ```
 MSG
 
-all_changed_files = helper.all_changed_files
+# exit if not matching files
+matching_changed_files = product_intelligence.matching_changed_files
+return unless matching_changed_files.any?
 
-tracking_files = [
-  'lib/gitlab/tracking.rb',
-  'spec/lib/gitlab/tracking_spec.rb',
-  'app/helpers/tracking_helper.rb',
-  'spec/helpers/tracking_helper_spec.rb',
-  'app/assets/javascripts/tracking.js',
-  'spec/frontend/tracking_spec.js',
-  'generator_templates/usage_metric_definition/metric_definition.yml',
-  'lib/generators/gitlab/usage_metric_definition_generator.rb',
-  'lib/generators/gitlab/usage_metric_definition/redis_hll_generator.rb',
-  'spec/lib/generators/gitlab/usage_metric_definition_generator_spec.rb',
-  'spec/lib/generators/gitlab/usage_metric_definition/redis_hll_generator_spec.rb',
-  'config/metrics/schema.json'
-]
+warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(matching_changed_files))
+fail format(UPDATE_DICTIONARY_MESSAGE) if product_intelligence.need_dictionary_changes?
 
-tracking_changed_files   = all_changed_files & tracking_files
-usage_data_changed_files = all_changed_files.grep(%r{(usage_data)})
-dictionary_changed_file  = all_changed_files.grep(%r{(doc/development/usage_ping/dictionary.md)})
-metrics_changed_files    = all_changed_files.grep(%r{((ee/)?config/metrics/.*\.yml)})
+labels = product_intelligence.missing_labels
+return unless labels.any?
 
-def matching_files?(file, extension:, pattern:)
-  return unless file.end_with?(extension)
-
-  helper.changed_lines(file).grep(pattern).any?
-end
-
-js_patterns = Regexp.union(
-  'Tracking.event',
-  /\btrack\(/,
-  'data-track-event',
-  'data-track-action'
-)
-
-dictionary_pattern = Regexp.union(
-  'key_path:',
-  'description:',
-  'product_section:',
-  'product_stage:',
-  'product_group:',
-  'status:',
-  'tier:'
-)
-
-snowplow_changed_files = all_changed_files.select do |file|
-  matching_files?(file, extension: '.rb', pattern: %r{Gitlab::Tracking\.event}) ||
-    matching_files?(file, extension: '.js', pattern: js_patterns) ||
-    matching_files?(file, extension: '.vue', pattern: js_patterns) ||
-    matching_files?(file, extension: '.haml', pattern: %r{data: \{ track})
-end
-
-required_dictionary_update_changed_files = metrics_changed_files.select do |file|
-  matching_files?(file, extension: '.yml', pattern: dictionary_pattern)
-end
-
-matching_changed_files = usage_data_changed_files +
-                         tracking_changed_files +
-                         metrics_changed_files +
-                         dictionary_changed_file +
-                         snowplow_changed_files
-
-if matching_changed_files.any?
-  warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(matching_changed_files))
-
-  fail format(UPDATE_DICTIONARY_MESSAGE) if required_dictionary_update_changed_files.any? && dictionary_changed_file.empty?
-
-  return unless helper.ci?
-
-  labels = []
-  labels << 'product intelligence' unless helper.mr_has_labels?('product intelligence')
-  labels << 'product intelligence::review pending' unless helper.mr_has_labels?(['product intelligence::approved', 'product intelligence::review pending'])
-
-  if labels.any?
-    gitlab.api.update_merge_request(gitlab.mr_json['project_id'],
+gitlab.api.update_merge_request(gitlab.mr_json['project_id'],
                                     gitlab.mr_json['iid'],
                                     add_labels: labels)
-  end
-end

spec/tooling/danger/product_intelligence_spec.rb

+# frozen_string_literal: true
+
+require 'gitlab-dangerfiles'
+require 'gitlab/dangerfiles/spec_helper'
+
+require_relative '../../../tooling/danger/product_intelligence'
+require_relative '../../../tooling/danger/project_helper'
+
+RSpec.describe Tooling::Danger::ProductIntelligence do
+  include_context "with dangerfile"
+
+  subject(:product_intelligence) { fake_danger.new(helper: fake_helper) }
+
+  let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
+  let(:changed_files) { ['metrics/counts_7d/test_metric.yml', 'doc/development/usage_ping/dictionary.md'] }
+  let(:changed_lines) { ['+tier: ee'] }
+
+  before do
+    allow(fake_helper).to receive(:all_changed_files).and_return(changed_files)
+    allow(fake_helper).to receive(:changed_lines).and_return(changed_lines)
+  end
+
+  describe '#need_dictionary_changes?' do
+    subject { product_intelligence.need_dictionary_changes? }
+
+    context 'when changed files do not contain dictionary changes' do
+      let(:changed_files) { ['config/metrics/counts_7d/test_metric.yml'] }
+
+      it { is_expected.to be true }
+    end
+
+    context 'when changed files already contains dictionary changes' do
+      let(:changed_files) { ['doc/development/usage_ping/dictionary.md'] }
+
+      it { is_expected.to be false }
+    end
+  end
+
+  describe '#missing_labels' do
+    subject { product_intelligence.missing_labels }
+
+    let(:ci_env) { true }
+
+    before do
+      allow(fake_helper).to receive(:mr_has_labels?).and_return(false)
+      allow(fake_helper).to receive(:ci?).and_return(ci_env)
+    end
+
+    context 'with ci? false' do
+      let(:ci_env) { false }
+
+      it { is_expected.to be_empty }
+    end
+
+    context 'with ci? true' do
+      let(:expected_labels) { ['product intelligence', 'product intelligence::review pending'] }
+
+      it { is_expected.to match_array(expected_labels) }
+    end
+
+    context 'with product intelligence label' do
+      let(:expected_labels) { ['product intelligence::review pending'] }
+
+      before do
+        allow(fake_helper).to receive(:mr_has_labels?).with('product intelligence').and_return(true)
+      end
+
+      it { is_expected.to match_array(expected_labels) }
+    end
+
+    context 'with product intelligence::review pending' do
+      before do
+        allow(fake_helper).to receive(:mr_has_labels?).and_return(true)
+      end
+
+      it { is_expected.to be_empty }
+    end
+  end
+
+  describe '#matching_changed_files' do
+    subject { product_intelligence.matching_changed_files }
+
+    let(:changed_files) do
+      [
+        'dashboard/todos_controller.rb',
+        'components/welcome.vue',
+        'admin/groups/_form.html.haml'
+      ]
+    end
+
+    context 'with snowplow files changed' do
+      context 'when vue file changed' do
+        let(:changed_lines) { ['+data-track-event'] }
+
+        it { is_expected.to match_array(['components/welcome.vue']) }
+      end
+
+      context 'when haml file changed' do
+        let(:changed_lines) { ['+ data: { track_label:'] }
+
+        it { is_expected.to match_array(['admin/groups/_form.html.haml']) }
+      end
+
+      context 'when ruby file changed' do
+        let(:changed_lines) { ['+ Gitlab::Tracking.event'] }
+        let(:changed_files) { ['dashboard/todos_controller.rb', 'admin/groups/_form.html.haml'] }
+
+        it { is_expected.to match_array(['dashboard/todos_controller.rb']) }
+      end
+    end
+
+    context 'with dictionary file not changed' do
+      it { is_expected.to be_empty }
+    end
+
+    context 'with metrics files changed' do
+      let(:changed_files) { ['config/metrics/counts_7d/test_metric.yml', 'ee/config/metrics/counts_7d/ee_metric.yml'] }
+
+      it { is_expected.to match_array(changed_files) }
+    end
+
+    context 'with metrics files not changed' do
+      it { is_expected.to be_empty }
+    end
+
+    context 'with tracking files changed' do
+      let(:changed_files) do
+        [
+          'lib/gitlab/tracking.rb',
+          'spec/lib/gitlab/tracking_spec.rb',
+          'app/helpers/tracking_helper.rb'
+        ]
+      end
+
+      it { is_expected.to match_array(changed_files) }
+    end
+
+    context 'with usage_data files changed' do
+      let(:changed_files) do
+        [
+          'doc/api/usage_data.md',
+          'ee/lib/ee/gitlab/usage_data.rb',
+          'spec/lib/gitlab/usage_data_spec.rb'
+        ]
+      end
+
+      it { is_expected.to match_array(changed_files) }
+    end
+  end
+end

tooling/danger/product_intelligence.rb

+# frozen_string_literal: true
+# rubocop:disable Style/SignalException
+
+module Tooling
+  module Danger
+    module ProductIntelligence
+      WORKFLOW_LABELS = [
+        'product intelligence::approved',
+        'product intelligence::review pending'
+      ].freeze
+
+      TRACKING_FILES = [
+        'lib/gitlab/tracking.rb',
+        'spec/lib/gitlab/tracking_spec.rb',
+        'app/helpers/tracking_helper.rb',
+        'spec/helpers/tracking_helper_spec.rb',
+        'app/assets/javascripts/tracking.js',
+        'spec/frontend/tracking_spec.js',
+        'generator_templates/usage_metric_definition/metric_definition.yml',
+        'lib/generators/gitlab/usage_metric_definition_generator.rb',
+        'lib/generators/gitlab/usage_metric_definition/redis_hll_generator.rb',
+        'spec/lib/generators/gitlab/usage_metric_definition_generator_spec.rb',
+        'spec/lib/generators/gitlab/usage_metric_definition/redis_hll_generator_spec.rb',
+        'config/metrics/schema.json'
+      ].freeze
+
+      def missing_labels
+        return [] unless helper.ci?
+
+        labels = []
+        labels << 'product intelligence' unless helper.mr_has_labels?('product intelligence')
+        labels << 'product intelligence::review pending' unless helper.mr_has_labels?(WORKFLOW_LABELS)
+
+        labels
+      end
+
+      def matching_changed_files
+        tracking_changed_files = all_changed_files & TRACKING_FILES
+        usage_data_changed_files = all_changed_files.grep(%r{(usage_data)})
+
+        usage_data_changed_files + tracking_changed_files + metrics_changed_files + dictionary_changed_file + snowplow_changed_files
+      end
+
+      def need_dictionary_changes?
+        required_dictionary_update_changed_files.any? && dictionary_changed_file.empty?
+      end
+
+      private
+
+      def all_changed_files
+        helper.all_changed_files
+      end
+
+      def dictionary_changed_file
+        all_changed_files.grep(%r{(doc/development/usage_ping/dictionary\.md)})
+      end
+
+      def metrics_changed_files
+        all_changed_files.grep(%r{((ee/)?config/metrics/.*\.yml)})
+      end
+
+      def matching_files?(file, extension:, pattern:)
+        return unless file.end_with?(extension)
+
+        helper.changed_lines(file).grep(pattern).any?
+      end
+
+      def snowplow_changed_files
+        js_patterns = Regexp.union(
+          'Tracking.event',
+          /\btrack\(/,
+          'data-track-event',
+          'data-track-action'
+        )
+        all_changed_files.select do |file|
+          matching_files?(file, extension: '.rb', pattern: %r{Gitlab::Tracking\.(event|enabled\?|snowplow_options)$}) ||
+            matching_files?(file, extension: '.js', pattern: js_patterns) ||
+            matching_files?(file, extension: '.vue', pattern: js_patterns) ||
+            matching_files?(file, extension: '.haml', pattern: %r{data: \{ track})
+        end
+      end
+
+      def required_dictionary_update_changed_files
+        dictionary_pattern = Regexp.union(
+          'key_path:',
+          'description:',
+          'product_section:',
+          'product_stage:',
+          'product_group:',
+          'status:',
+          'tier:'
+        )
+
+        metrics_changed_files.select do |file|
+          matching_files?(file, extension: '.yml', pattern: dictionary_pattern)
+        end
+      end
+    end
+  end
+end