Merge branch 'email-on-push-select-branches' into 'master'

Add option to configure branches for which emails should be sent on push

See merge request gitlab-org/gitlab!22196

app/models/project_services/emails_on_push_service.rb

 # frozen_string_literal: true
 
 class EmailsOnPushService < Service
+  include NotificationBranchSelection
+
   boolean_accessor :send_from_committer_email
   boolean_accessor :disable_diffs
-  prop_accessor :recipients
+  prop_accessor :recipients, :branches_to_be_notified
   validates :recipients, presence: true, if: :valid_recipients?
 
   def title
@@ -22,9 +24,17 @@ class EmailsOnPushService < Service
     %w(push tag_push)
   end
 
+  def initialize_properties
+    if properties.nil?
+      self.properties = {}
+      self.branches_to_be_notified ||= "all"
+    end
+  end
+
   def execute(push_data)
     return unless supported_events.include?(push_data[:object_kind])
     return if project.emails_disabled?
+    return unless notify_for_ref?(push_data)
 
     EmailsOnPushWorker.perform_async(
       project_id,
@@ -35,6 +45,13 @@ class EmailsOnPushService < Service
     )
   end
 
+  def notify_for_ref?(push_data)
+    return true if push_data[:object_kind] == 'tag_push'
+    return true if push_data.dig(:object_attributes, :tag)
+
+    notify_for_branch?(push_data)
+  end
+
   def send_from_committer_email?
     Gitlab::Utils.to_boolean(self.send_from_committer_email)
   end
@@ -50,6 +67,7 @@ class EmailsOnPushService < Service
         help: s_("EmailsOnPushService|Send notifications from the committer's email address if the domain is part of the domain GitLab is running on (e.g. %{domains}).") % { domains: domains } },
       { type: 'checkbox', name: 'disable_diffs', title: s_("EmailsOnPushService|Disable code diffs"),
         help: s_("EmailsOnPushService|Don't include possibly sensitive code diffs in notification body.") },
+      { type: 'select', name: 'branches_to_be_notified', choices: BRANCH_CHOICES },
       { type: 'textarea', name: 'recipients', placeholder: s_('EmailsOnPushService|Emails separated by whitespace') }
     ]
   end

changelogs/unreleased/email-on-push-select-branches.yml

+---
+title: Add option to configure branches for which to send emails on push
+merge_request: 22196
+author:
+type: added

doc/api/services.md

@@ -373,6 +373,7 @@ Parameters:
 | `send_from_committer_email` | boolean | false | Send from committer |
 | `push_events` | boolean | false | Enable notifications for push events |
 | `tag_push_events` | boolean | false | Enable notifications for tag push events |
+| `branches_to_be_notified` | string | all | Branches to send notifications for. Valid options are "all", "default", "protected", and "default_and_protected". Notifications will be always fired for tag pushes. |
 
 ### Delete Emails on push service
 

lib/api/helpers/services_helpers.rb

@@ -365,6 +365,12 @@ module API
               name: :send_from_committer_email,
               type: Boolean,
               desc: 'Send from committer'
+            },
+            {
+              required: false,
+              name: :branches_to_be_notified,
+              type: String,
+              desc: 'Branches for which notifications are to be sent'
             }
           ],
           'external-wiki' => [

spec/models/project_services/emails_on_push_service_spec.rb

@@ -25,19 +25,75 @@ describe EmailsOnPushService do
     let(:push_data) { { object_kind: 'push' } }
     let(:project)   { create(:project, :repository) }
     let(:service)   { create(:emails_on_push_service, project: project) }
+    let(:recipients) { 'test@gitlab.com' }
 
-    it 'does not send emails when disabled' do
-      expect(project).to receive(:emails_disabled?).and_return(true)
-      expect(EmailsOnPushWorker).not_to receive(:perform_async)
+    before do
+      subject.recipients = recipients
+    end
+
+    shared_examples 'sending email' do |branches_to_be_notified, branch_being_pushed_to|
+      let(:push_data) { { object_kind: 'push', object_attributes: { ref: branch_being_pushed_to } } }
 
-      service.execute(push_data)
+      before do
+        subject.branches_to_be_notified = branches_to_be_notified
+      end
+
+      it 'sends email' do
+        expect(EmailsOnPushWorker).not_to receive(:perform_async)
+
+        service.execute(push_data)
+      end
     end
 
-    it 'does send emails when enabled' do
-      expect(project).to receive(:emails_disabled?).and_return(false)
-      expect(EmailsOnPushWorker).to receive(:perform_async)
+    shared_examples 'not sending email' do |branches_to_be_notified, branch_being_pushed_to|
+      let(:push_data) { { object_kind: 'push', object_attributes: { ref: branch_being_pushed_to } } }
 
-      service.execute(push_data)
+      before do
+        subject.branches_to_be_notified = branches_to_be_notified
+      end
+
+      it 'does not send email' do
+        expect(EmailsOnPushWorker).not_to receive(:perform_async)
+
+        service.execute(push_data)
+      end
+    end
+
+    context 'when emails are disabled on the project' do
+      it 'does not send emails' do
+        expect(project).to receive(:emails_disabled?).and_return(true)
+        expect(EmailsOnPushWorker).not_to receive(:perform_async)
+
+        service.execute(push_data)
+      end
+    end
+
+    context 'when emails are enabled on the project' do
+      before do
+        create(:protected_branch, project: project, name: 'a-protected-branch')
+        expect(project).to receive(:emails_disabled?).and_return(true)
+      end
+
+      using RSpec::Parameterized::TableSyntax
+
+      where(:case_name, :branches_to_be_notified, :branch_being_pushed_to, :expected_action) do
+        'pushing to a random branch and notification configured for all branches'                           | 'all'                   | 'random'             | 'sending email'
+        'pushing to the default branch and notification configured for all branches'                        | 'all'                   | 'master'             | 'sending email'
+        'pushing to a protected branch and notification configured for all branches'                        | 'all'                   | 'a-protected-branch' | 'sending email'
+        'pushing to a random branch and notification configured for default branch only'                    | 'default'               | 'random'             | 'not sending email'
+        'pushing to the default branch and notification configured for default branch only'                 | 'default'               | 'master'             | 'sending email'
+        'pushing to a protected branch and notification configured for default branch only'                 | 'default'               | 'a-protected-branch' | 'not sending email'
+        'pushing to a random branch and notification configured for protected branches only'                | 'protected'             | 'random'             | 'not sending email'
+        'pushing to the default branch and notification configured for protected branches only'             | 'protected'             | 'master'             | 'not sending email'
+        'pushing to a protected branch and notification configured for protected branches only'             | 'protected'             | 'a-protected-branch' | 'sending email'
+        'pushing to a random branch and notification configured for default and protected branches only'    | 'default_and_protected' | 'random'             | 'not sending email'
+        'pushing to the default branch and notification configured for default and protected branches only' | 'default_and_protected' | 'master'             | 'sending email'
+        'pushing to a protected branch and notification configured for default and protected branches only' | 'default_and_protected' | 'a-protected-branch' | 'sending email'
+      end
+
+      with_them do
+        include_examples params[:expected_action], branches_to_be_notified: params[:branches_to_be_notified], branch_being_pushed_to: params[:branch_being_pushed_to]
+      end
     end
   end
 end