Merge branch...

Merge branch '209277-introduce-a-feature-flag-for-resolve-notifications-for-when-pipelines-are-fixed' into 'master'

Introduce a feature flag for fixed pipeline notifications

Closes #209277

See merge request gitlab-org/gitlab!26682

app/models/ci/pipeline.rb

@@ -236,7 +236,11 @@ module Ci
 
       after_transition any => [:success, :failed] do |pipeline|
         pipeline.run_after_commit do
-          PipelineUpdateCiRefStatusWorker.perform_async(pipeline.id)
+          if Feature.enabled?(:ci_pipeline_fixed_notifications)
+            PipelineUpdateCiRefStatusWorker.perform_async(pipeline.id)
+          else
+            PipelineNotificationWorker.perform_async(pipeline.id)
+          end
         end
       end
 

app/views/shared/notifications/_custom_notifications.html.haml

@@ -23,6 +23,7 @@
                   #{ paragraph.html_safe }
               .col-lg-8
                 - notification_setting.email_events.each_with_index do |event, index|
+                  - next if event == :fixed_pipeline && Feature.disabled?(:ci_pipeline_fixed_notifications)
                   - field_id = "#{notifications_menu_identifier("modal", notification_setting)}_notification_setting[#{event}]"
                   .form-group
                     .form-check{ class: ("prepend-top-0" if index == 0) }

changelogs/unreleased/209277-introduce-a-feature-flag-for-resolve-notifications-for-when-pipeli.yml

+---
+title: Introduce a feature flag for Notifications for when pipelines are fixed
+merge_request: 26682
+author: Jacopo Beschi @jacopo-beschi
+type: changed

doc/user/profile/notifications.md

@@ -181,7 +181,7 @@ To minimize the number of notifications that do not require any action, from [Gi
 | Remove milestone merge request | Subscribers, participants mentioned, and Custom notification level with this event selected |
 | New comment            | The above, plus anyone mentioned by `@username` in the comment, with notification level "Mention" or higher |
 | Failed pipeline        | The author of the pipeline |
-| Fixed pipeline    | The author of the pipeline |
+| Fixed pipeline    | The author of the pipeline. Disabled by default. To activate it you must [enable the `ci_pipeline_fixed_notifications` feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-in-development). |
 | Successful pipeline    | The author of the pipeline, if they have the custom notification setting for successful pipelines set. If the pipeline failed previously, a `Fixed pipeline` message will be sent for the first successful pipeline after the failure, then a `Successful pipeline` message for any further successful pipelines. |
 | New epic **(ULTIMATE)** |        |
 | Close epic **(ULTIMATE)** |      |

spec/features/projects/show/user_manages_notifications_spec.rb

@@ -7,7 +7,6 @@ describe 'Projects > Show > User manages notifications', :js do
 
   before do
     sign_in(project.owner)
-    visit project_path(project)
   end
 
   def click_notifications_button
@@ -15,6 +14,7 @@ describe 'Projects > Show > User manages notifications', :js do
   end
 
   it 'changes the notification setting' do
+    visit project_path(project)
     click_notifications_button
     click_link 'On mention'
 
@@ -26,6 +26,7 @@ describe 'Projects > Show > User manages notifications', :js do
   end
 
   it 'changes the notification setting to disabled' do
+    visit project_path(project)
     click_notifications_button
     click_link 'Disabled'
 
@@ -50,11 +51,13 @@ describe 'Projects > Show > User manages notifications', :js do
         :reassign_merge_request,
         :merge_merge_request,
         :failed_pipeline,
+        :fixed_pipeline,
         :success_pipeline
       ]
     end
 
     it 'shows notification settings checkbox' do
+      visit project_path(project)
       click_notifications_button
       page.find('a[data-notification-level="custom"]').click
 
@@ -64,12 +67,27 @@ describe 'Projects > Show > User manages notifications', :js do
         end
       end
     end
+
+    context 'when ci_pipeline_fixed_notifications is disabled' do
+      before do
+        stub_feature_flags(ci_pipeline_fixed_notifications: false)
+      end
+
+      it 'hides fixed_pipeline checkbox' do
+        visit project_path(project)
+        click_notifications_button
+        page.find('a[data-notification-level="custom"]').click
+
+        expect(page).not_to have_selector("input[name='notification_setting[fixed_pipeline]']")
+      end
+    end
   end
 
   context 'when project emails are disabled' do
     let(:project) { create(:project, :public, :repository, emails_disabled: true) }
 
     it 'is disabled' do
+      visit project_path(project)
       expect(page).to have_selector('.notifications-btn.disabled', visible: true)
     end
   end

spec/models/ci/pipeline_spec.rb

@@ -2509,27 +2509,53 @@ describe Ci::Pipeline, :mailer do
       end
     end
 
-    context 'with success pipeline' do
-      before do
-        perform_enqueued_jobs do
+    shared_examples 'enqueues the notification worker' do
+      it 'enqueues PipelineUpdateCiRefStatusWorker' do
+        expect(PipelineUpdateCiRefStatusWorker).to receive(:perform_async).with(pipeline.id)
+        expect(PipelineNotificationWorker).not_to receive(:perform_async).with(pipeline.id)
+
+        pipeline.succeed
+      end
+
+      context 'when ci_pipeline_fixed_notifications is disabled' do
+        before do
+          stub_feature_flags(ci_pipeline_fixed_notifications: false)
+        end
+
+        it 'enqueues PipelineNotificationWorker' do
+          expect(PipelineUpdateCiRefStatusWorker).not_to receive(:perform_async).with(pipeline.id)
+          expect(PipelineNotificationWorker).to receive(:perform_async).with(pipeline.id)
+
           pipeline.succeed
         end
       end
+    end
 
-      it_behaves_like 'sending a notification'
+    context 'with success pipeline' do
+      it_behaves_like 'sending a notification' do
+        before do
+          perform_enqueued_jobs do
+            pipeline.succeed
+          end
+        end
+      end
+
+      it_behaves_like 'enqueues the notification worker'
     end
 
     context 'with failed pipeline' do
-      before do
-        perform_enqueued_jobs do
-          create(:ci_build, :failed, pipeline: pipeline)
-          create(:generic_commit_status, :failed, pipeline: pipeline)
+      it_behaves_like 'sending a notification' do
+        before do
+          perform_enqueued_jobs do
+            create(:ci_build, :failed, pipeline: pipeline)
+            create(:generic_commit_status, :failed, pipeline: pipeline)
 
-          pipeline.drop
+            pipeline.drop
+          end
         end
       end
 
-      it_behaves_like 'sending a notification'
+      it_behaves_like 'enqueues the notification worker'
     end
 
     context 'with skipped pipeline' do