Merge branch...

Merge branch '347246-move-product-intelligence-dangerfile-logic-to-tooling-danger-product_intelligence-rb' into 'master'

Move Product Intelligence Dangerfile logic to helper module

See merge request gitlab-org/gitlab!83583

danger/product_intelligence/Dangerfile

 # frozen_string_literal: true
-# rubocop:disable Style/SignalException
 
-CHANGED_FILES_MESSAGE = <<~MSG
-For the following files, a review from the [Data team and Product Intelligence team](https://gitlab.com/groups/gitlab-org/growth/product-intelligence/engineers/-/group_members?with_inherited_permissions=exclude) is recommended
-Please check the ~"product intelligence" [guide](https://docs.gitlab.com/ee/development/usage_ping.html).
-
-For MR review guidelines, see the [Service Ping review guidelines](https://docs.gitlab.com/ee/development/usage_ping/review_guidelines.html) or the [Snowplow review guidelines](https://docs.gitlab.com/ee/development/snowplow/review_guidelines.html).
-
-%<changed_files>s
-
-MSG
-
-# exit if not matching files or if no product intelligence labels
-product_intelligence_paths_to_review = helper.changes_by_category[:product_intelligence]
-labels_to_add = product_intelligence.missing_labels
-
-return if product_intelligence_paths_to_review.empty? || product_intelligence.skip_review?
-
-warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(product_intelligence_paths_to_review)) unless product_intelligence.has_approved_label?
-
-helper.labels_to_add.concat(labels_to_add) unless labels_to_add.empty?
+product_intelligence.check!

spec/tooling/danger/product_intelligence_spec.rb

@@ -20,70 +20,105 @@ RSpec.describe Tooling::Danger::ProductIntelligence do
     allow(fake_helper).to receive(:changed_lines).and_return(changed_lines)
   end
 
-  describe '#missing_labels' do
-    subject { product_intelligence.missing_labels }
+  describe '#check!' do
+    subject { product_intelligence.check! }
 
+    let(:markdown_formatted_list) { 'markdown formatted list' }
+    let(:review_pending_label) { 'product intelligence::review pending' }
+    let(:approved_label) { 'product intelligence::approved' }
     let(:ci_env) { true }
+    let(:previous_label_to_add) { 'label_to_add' }
+    let(:labels_to_add) { [previous_label_to_add] }
+    let(:has_product_intelligence_label) { true }
 
     before do
-      allow(fake_helper).to receive(:mr_has_labels?).and_return(false)
+      allow(fake_helper).to receive(:changes_by_category).and_return(product_intelligence: changed_files, database: ['other_files.yml'])
       allow(fake_helper).to receive(:ci?).and_return(ci_env)
+      allow(fake_helper).to receive(:mr_has_labels?).with('product intelligence').and_return(has_product_intelligence_label)
+      allow(fake_helper).to receive(:markdown_list).with(changed_files).and_return(markdown_formatted_list)
+      allow(fake_helper).to receive(:labels_to_add).and_return(labels_to_add)
     end
 
-    context 'with ci? false' do
-      let(:ci_env) { false }
+    shared_examples "doesn't add new labels" do
+      it "doesn't add new labels" do
+        subject
 
-      it { is_expected.to be_empty }
+        expect(labels_to_add).to match_array [previous_label_to_add]
+      end
     end
 
-    context 'with ci? true' do
-      let(:expected_labels) { ['product intelligence', 'product intelligence::review pending'] }
+    shared_examples "doesn't add new warnings" do
+      it "doesn't add new warnings" do
+        expect(product_intelligence).not_to receive(:warn)
 
-      it { is_expected.to match_array(expected_labels) }
+        subject
+      end
     end
 
-    context 'with product intelligence label' do
-      let(:expected_labels) { ['product intelligence::review pending'] }
-      let(:mr_labels) { [] }
+    shared_examples 'adds new labels' do
+      it 'adds new labels' do
+        subject
+
+        expect(labels_to_add).to match_array [previous_label_to_add, review_pending_label]
+      end
+    end
 
+    context 'with growth experiment label' do
       before do
-        allow(fake_helper).to receive(:mr_has_labels?).with('product intelligence').and_return(true)
-        allow(fake_helper).to receive(:mr_labels).and_return(mr_labels)
+        allow(fake_helper).to receive(:mr_has_labels?).with('growth experiment').and_return(true)
       end
 
-      it { is_expected.to match_array(expected_labels) }
+      include_examples "doesn't add new labels"
+      include_examples "doesn't add new warnings"
+    end
+
+    context 'without growth experiment label' do
+      before do
+        allow(fake_helper).to receive(:mr_has_labels?).with('growth experiment').and_return(false)
+      end
 
-      context 'with product intelligence::review pending' do
-        let(:mr_labels) { ['product intelligence::review pending'] }
+      context 'with approved label' do
+        let(:mr_labels) { [approved_label] }
 
-        it { is_expected.to be_empty }
+        include_examples "doesn't add new labels"
+        include_examples "doesn't add new warnings"
       end
 
-      context 'with product intelligence::approved' do
-        let(:mr_labels) { ['product intelligence::approved'] }
+      context 'without approved label' do
+        include_examples 'adds new labels'
+
+        it 'warns with proper message' do
+          expect(product_intelligence).to receive(:warn).with(%r{#{markdown_formatted_list}})
 
-        it { is_expected.to be_empty }
+          subject
+        end
       end
-    end
-  end
 
-  describe '#skip_review' do
-    subject { product_intelligence.skip_review? }
+      context 'with product intelligence::review pending label' do
+        let(:mr_labels) { ['product intelligence::review pending'] }
 
-    context 'with growth experiment label' do
-      before do
-        allow(fake_helper).to receive(:mr_has_labels?).with('growth experiment').and_return(true)
+        include_examples "doesn't add new labels"
       end
 
-      it { is_expected.to be true }
-    end
+      context 'with product intelligence::approved label' do
+        let(:mr_labels) { ['product intelligence::approved'] }
 
-    context 'without growth experiment label' do
-      before do
-        allow(fake_helper).to receive(:mr_has_labels?).with('growth experiment').and_return(false)
+        include_examples "doesn't add new labels"
       end
 
-      it { is_expected.to be false }
+      context 'with the product intelligence label' do
+        let(:has_product_intelligence_label) { true }
+
+        context 'with ci? false' do
+          let(:ci_env) { false }
+
+          include_examples "doesn't add new labels"
+        end
+
+        context 'with ci? true' do
+          include_examples 'adds new labels'
+        end
+      end
     end
   end
 end

tooling/danger/product_intelligence.rb

@@ -6,12 +6,35 @@ module Tooling
     module ProductIntelligence
       APPROVED_LABEL = 'product intelligence::approved'
       REVIEW_LABEL = 'product intelligence::review pending'
+      CHANGED_FILES_MESSAGE = <<~MSG
+      For the following files, a review from the [Data team and Product Intelligence team](https://gitlab.com/groups/gitlab-org/growth/product-intelligence/engineers/-/group_members?with_inherited_permissions=exclude) is recommended
+      Please check the ~"product intelligence" [guide](https://docs.gitlab.com/ee/development/usage_ping.html).
+
+      For MR review guidelines, see the [Service Ping review guidelines](https://docs.gitlab.com/ee/development/usage_ping/review_guidelines.html) or the [Snowplow review guidelines](https://docs.gitlab.com/ee/development/snowplow/review_guidelines.html).
+
+      %<changed_files>s
+
+      MSG
 
       WORKFLOW_LABELS = [
         APPROVED_LABEL,
         REVIEW_LABEL
       ].freeze
 
+      def check!
+        # exit if not matching files or if no product intelligence labels
+        product_intelligence_paths_to_review = helper.changes_by_category[:product_intelligence]
+        labels_to_add = missing_labels
+
+        return if product_intelligence_paths_to_review.empty? || skip_review?
+
+        warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(product_intelligence_paths_to_review)) unless has_approved_label?
+
+        helper.labels_to_add.concat(labels_to_add) unless labels_to_add.empty?
+      end
+
+      private
+
       def missing_labels
         return [] unless helper.ci?
 
@@ -30,8 +53,6 @@ module Tooling
         helper.mr_has_labels?('growth experiment')
       end
 
-      private
-
       def has_workflow_labels?
         (WORKFLOW_LABELS & helper.mr_labels).any?
       end