Merge branch...

Merge branch '50020-allow-email-notifications-to-be-disabled-for-all-users-of-a-group' into 'master'

Allow email notifications to be disabled for all users of a group

See merge request gitlab-org/gitlab-ce!30755

app/controllers/groups_controller.rb

@@ -176,6 +176,7 @@ class GroupsController < Groups::ApplicationController
     [
       :avatar,
       :description,
+      :emails_disabled,
       :lfs_enabled,
       :name,
       :path,

app/controllers/projects_controller.rb

@@ -361,6 +361,7 @@ class ProjectsController < Projects::ApplicationController
       :container_registry_enabled,
       :default_branch,
       :description,
+      :emails_disabled,
       :external_authorization_classification_label,
       :import_url,
       :issues_tracker,

app/models/namespace.rb

@@ -172,6 +172,13 @@ class Namespace < ApplicationRecord
     end
   end
 
+  # any ancestor can disable emails for all descendants
+  def emails_disabled?
+    strong_memoize(:emails_disabled) do
+      Feature.enabled?(:emails_disabled, self, default_enabled: true) && self_and_ancestors.where(emails_disabled: true).exists?
+    end
+  end
+
   def lfs_enabled?
     # User namespace will always default to the global setting
     Gitlab.config.lfs.enabled

app/models/notification_recipient.rb

@@ -4,6 +4,7 @@ class NotificationRecipient
   include Gitlab::Utils::StrongMemoize
 
   attr_reader :user, :type, :reason
+
   def initialize(user, type, **opts)
     unless NotificationSetting.levels.key?(type) || type == :subscription
       raise ArgumentError, "invalid type: #{type.inspect}"
@@ -30,6 +31,7 @@ class NotificationRecipient
 
   def notifiable?
     return false unless has_access?
+    return false if emails_disabled?
     return false if own_activity?
 
     # even users with :disabled notifications receive manual subscriptions
@@ -109,6 +111,12 @@ class NotificationRecipient
 
   private
 
+  # They are disabled if the project or group has disallowed it.
+  # No need to check the group if there is already a project
+  def emails_disabled?
+    @project ? @project.emails_disabled? : @group&.emails_disabled?
+  end
+
   def read_ability
     return if @skip_read_ability
     return @read_ability if instance_variable_defined?(:@read_ability)

app/models/project.rb

@@ -631,6 +631,13 @@ class Project < ApplicationRecord
 
   alias_method :ancestors, :ancestors_upto
 
+  def emails_disabled?
+    strong_memoize(:emails_disabled) do
+      # disabling in the namespace overrides the project setting
+      Feature.enabled?(:emails_disabled, self, default_enabled: true) && (super || namespace.emails_disabled?)
+    end
+  end
+
   def lfs_enabled?
     return namespace.lfs_enabled? if self[:lfs_enabled].nil?
 

app/models/project_services/emails_on_push_service.rb

@@ -24,6 +24,7 @@ class EmailsOnPushService < Service
 
   def execute(push_data)
     return unless supported_events.include?(push_data[:object_kind])
+    return if project.emails_disabled?
 
     EmailsOnPushWorker.perform_async(
       project_id,

app/policies/group_policy.rb

@@ -92,6 +92,7 @@ class GroupPolicy < BasePolicy
     enable :change_visibility_level
 
     enable :set_note_created_at
+    enable :set_emails_disabled
   end
 
   rule { can?(:read_nested_project_resources) }.policy do

app/policies/project_policy.rb

@@ -162,6 +162,7 @@ class ProjectPolicy < BasePolicy
     enable :set_issue_created_at
     enable :set_issue_updated_at
     enable :set_note_created_at
+    enable :set_emails_disabled
   end
 
   rule { can?(:guest_access) }.policy do

app/services/groups/update_service.rb

@@ -46,6 +46,11 @@ module Groups
       params.delete(:parent_id)
     end
 
+    # overridden in EE
+    def remove_unallowed_params
+      params.delete(:emails_disabled) unless can?(current_user, :set_emails_disabled, group)
+    end
+
     def valid_share_with_group_lock_change?
       return true unless changing_share_with_group_lock?
       return true if can?(current_user, :change_share_with_group_lock, group)

app/services/notification_service.rb

@@ -321,6 +321,9 @@ class NotificationService
   end
 
   def decline_project_invite(project_member)
+    # Must always send, regardless of project/namespace configuration since it's a
+    # response to the user's action.
+
     mailer.member_invite_declined_email(
       project_member.real_source_type,
       project_member.project.id,
@@ -351,8 +354,8 @@ class NotificationService
   end
 
   def decline_group_invite(group_member)
-    # always send this one, since it's a response to the user's own
-    # action
+    # Must always send, regardless of project/namespace configuration since it's a
+    # response to the user's action.
 
     mailer.member_invite_declined_email(
       group_member.real_source_type,
@@ -410,6 +413,10 @@ class NotificationService
   end
 
   def pipeline_finished(pipeline, recipients = nil)
+    # Must always check project configuration since recipients could be a list of emails
+    # from the PipelinesEmailService integration.
+    return if pipeline.project.emails_disabled?
+
     email_template = "pipeline_#{pipeline.status}_email"
 
     return unless mailer.respond_to?(email_template)
@@ -428,6 +435,8 @@ class NotificationService
   end
 
   def autodevops_disabled(pipeline, recipients)
+    return if pipeline.project.emails_disabled?
+
     recipients.each do |recipient|
       mailer.autodevops_disabled_email(pipeline, recipient).deliver_later
     end
@@ -472,10 +481,14 @@ class NotificationService
   end
 
   def repository_cleanup_success(project, user)
+    return if project.emails_disabled?
+
     mailer.send(:repository_cleanup_success_email, project, user).deliver_later
   end
 
   def repository_cleanup_failure(project, user, error)
+    return if project.emails_disabled?
+
     mailer.send(:repository_cleanup_failure_email, project, user, error).deliver_later
   end
 

app/services/projects/update_service.rb

@@ -9,6 +9,7 @@ module Projects
 
     # rubocop: disable CodeReuse/ActiveRecord
     def execute
+      remove_unallowed_params
       validate!
 
       ensure_wiki_exists if enabling_wiki?
@@ -54,6 +55,10 @@ module Projects
       end
     end
 
+    def remove_unallowed_params
+      params.delete(:emails_disabled) unless can?(current_user, :set_emails_disabled, project)
+    end
+
     def after_update
       todos_features_changes = %w(
         issues_access_level

changelogs/unreleased/50020-allow-email-notifications-to-be-disabled-for-all-users-of-a-group.yml

+---
+title: Allow email notifications to be disabled for all members of a group or project
+merge_request: 30755
+author: Dustin Spicuzza
+type: added

db/migrate/20190715215532_add_project_emails_disabled.rb

+# frozen_string_literal: true
+
+class AddProjectEmailsDisabled < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column :projects, :emails_disabled, :boolean
+  end
+end

db/migrate/20190715215549_add_group_emails_disabled.rb

+# frozen_string_literal: true
+
+class AddGroupEmailsDisabled < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column :namespaces, :emails_disabled, :boolean
+  end
+end

db/schema.rb

@@ -2175,6 +2175,7 @@ ActiveRecord::Schema.define(version: 2019_08_12_070645) do
     t.boolean "membership_lock", default: false
     t.integer "last_ci_minutes_usage_notification_level"
     t.integer "subgroup_creation_level", default: 1
+    t.boolean "emails_disabled"
     t.index ["created_at"], name: "index_namespaces_on_created_at"
     t.index ["custom_project_templates_group_id", "type"], name: "index_namespaces_on_custom_project_templates_group_id_and_type", where: "(custom_project_templates_group_id IS NOT NULL)"
     t.index ["file_template_project_id"], name: "index_namespaces_on_file_template_project_id"
@@ -2745,6 +2746,7 @@ ActiveRecord::Schema.define(version: 2019_08_12_070645) do
     t.boolean "reset_approvals_on_push", default: true
     t.boolean "service_desk_enabled", default: true
     t.integer "approvals_before_merge", default: 0, null: false
+    t.boolean "emails_disabled"
     t.index ["archived", "pending_delete", "merge_requests_require_code_owner_approval"], name: "projects_requiring_code_owner_approval", where: "((pending_delete = false) AND (archived = false) AND (merge_requests_require_code_owner_approval = true))"
     t.index ["created_at"], name: "index_projects_on_created_at"
     t.index ["creator_id"], name: "index_projects_on_creator_id"

lib/gitlab/import_export/import_export.yml

@@ -137,6 +137,7 @@ excluded_attributes:
     - :packages_enabled
     - :mirror_last_update_at
     - :mirror_last_successful_update_at
+    - :emails_disabled
   namespaces:
     - :runners_token
     - :runners_token_encrypted

spec/factories/services.rb

@@ -16,6 +16,19 @@ FactoryBot.define do
     )
   end
 
+  factory :emails_on_push_service do
+    project
+    type 'EmailsOnPushService'
+    active true
+    push_events true
+    tag_push_events true
+    properties(
+      recipients: 'test@example.com',
+      disable_diffs: true,
+      send_from_committer_email: true
+    )
+  end
+
   factory :mock_deployment_service do
     project
     type 'MockDeploymentService'

spec/models/namespace_spec.rb

@@ -853,4 +853,64 @@ describe Namespace do
       it { is_expected.to be_falsy }
     end
   end
+
+  describe '#emails_disabled?' do
+    context 'when not a subgroup' do
+      it 'returns false' do
+        group = create(:group, emails_disabled: false)
+
+        expect(group.emails_disabled?).to be_falsey
+      end
+
+      it 'returns true' do
+        group = create(:group, emails_disabled: true)
+
+        expect(group.emails_disabled?).to be_truthy
+      end
+    end
+
+    context 'when a subgroup' do
+      let(:grandparent) { create(:group) }
+      let(:parent)      { create(:group, parent: grandparent) }
+      let(:group)       { create(:group, parent: parent) }
+
+      it 'returns false' do
+        expect(group.emails_disabled?).to be_falsey
+      end
+
+      context 'when ancestor emails are disabled' do
+        it 'returns true' do
+          grandparent.update_attribute(:emails_disabled, true)
+
+          expect(group.emails_disabled?).to be_truthy
+        end
+      end
+    end
+
+    context 'when :emails_disabled feature flag is off' do
+      before do
+        stub_feature_flags(emails_disabled: false)
+      end
+
+      context 'when not a subgroup' do
+        it 'returns false' do
+          group = create(:group, emails_disabled: true)
+
+          expect(group.emails_disabled?).to be_falsey
+        end
+      end
+
+      context 'when a subgroup and ancestor emails are disabled' do
+        let(:grandparent) { create(:group) }
+        let(:parent)      { create(:group, parent: grandparent) }
+        let(:group)       { create(:group, parent: parent) }
+
+        it 'returns false' do
+          grandparent.update_attribute(:emails_disabled, true)
+
+          expect(group.emails_disabled?).to be_falsey
+        end
+      end
+    end
+  end
 end

spec/models/notification_recipient_spec.rb

@@ -9,6 +9,38 @@ describe NotificationRecipient do
 
   subject(:recipient) { described_class.new(user, :watch, target: target, project: project) }
 
+  describe '#notifiable?' do
+    let(:recipient) { described_class.new(user, :mention, target: target, project: project) }
+
+    context 'when emails are disabled' do
+      it 'returns false if group disabled' do
+        expect(project.namespace).to receive(:emails_disabled?).and_return(true)
+        expect(recipient).to receive(:emails_disabled?).and_call_original
+        expect(recipient.notifiable?).to eq false
+      end
+
+      it 'returns false if project disabled' do
+        expect(project).to receive(:emails_disabled?).and_return(true)
+        expect(recipient).to receive(:emails_disabled?).and_call_original
+        expect(recipient.notifiable?).to eq false
+      end
+    end
+
+    context 'when emails are enabled' do
+      it 'returns true if group enabled' do
+        expect(project.namespace).to receive(:emails_disabled?).and_return(false)
+        expect(recipient).to receive(:emails_disabled?).and_call_original
+        expect(recipient.notifiable?).to eq true
+      end
+
+      it 'returns true if project enabled' do
+        expect(project).to receive(:emails_disabled?).and_return(false)
+        expect(recipient).to receive(:emails_disabled?).and_call_original
+        expect(recipient.notifiable?).to eq true
+      end
+    end
+  end
+
   describe '#has_access?' do
     before do
       allow(user).to receive(:can?).and_call_original

spec/models/project_services/emails_on_push_service_spec.rb

@@ -20,4 +20,24 @@ describe EmailsOnPushService do
       it { is_expected.not_to validate_presence_of(:recipients) }
     end
   end
+
+  context 'project emails' do
+    let(:push_data) { { object_kind: 'push' } }
+    let(:project)   { create(:project, :repository) }
+    let(:service)   { create(:emails_on_push_service, project: project) }
+
+    it 'does not send emails when disabled' do
+      expect(project).to receive(:emails_disabled?).and_return(true)
+      expect(EmailsOnPushWorker).not_to receive(:perform_async)
+
+      service.execute(push_data)
+    end
+
+    it 'does send emails when enabled' do
+      expect(project).to receive(:emails_disabled?).and_return(false)
+      expect(EmailsOnPushWorker).to receive(:perform_async)
+
+      service.execute(push_data)
+    end
+  end
 end

spec/models/project_spec.rb

@@ -2315,6 +2315,57 @@ describe Project do
     end
   end
 
+  describe '#emails_disabled?' do
+    let(:project) { create(:project, emails_disabled: false) }
+
+    context 'emails disabled in group' do
+      it 'returns true' do
+        allow(project.namespace).to receive(:emails_disabled?) { true }
+
+        expect(project.emails_disabled?).to be_truthy
+      end
+    end
+
+    context 'emails enabled in group' do
+      before do
+        allow(project.namespace).to receive(:emails_disabled?) { false }
+      end
+
+      it 'returns false' do
+        expect(project.emails_disabled?).to be_falsey
+      end
+
+      it 'returns true' do
+        project.update_attribute(:emails_disabled, true)
+
+        expect(project.emails_disabled?).to be_truthy
+      end
+    end
+
+    context 'when :emails_disabled feature flag is off' do
+      before do
+        stub_feature_flags(emails_disabled: false)
+      end
+
+      context 'emails disabled in group' do
+        it 'returns false' do
+          allow(project.namespace).to receive(:emails_disabled?) { true }
+
+          expect(project.emails_disabled?).to be_falsey
+        end
+      end
+
+      context 'emails enabled in group' do
+        it 'returns false' do
+          allow(project.namespace).to receive(:emails_disabled?) { false }
+          project.update_attribute(:emails_disabled, true)
+
+          expect(project.emails_disabled?).to be_falsey
+        end
+      end
+    end
+  end
+
   describe '#lfs_enabled?' do
     let(:project) { create(:project) }
 

spec/services/groups/update_service_spec.rb

@@ -86,6 +86,7 @@ describe Groups::UpdateService do
 
   context "unauthorized visibility_level validation" do
     let!(:service) { described_class.new(internal_group, user, visibility_level: 99) }
+
     before do
       internal_group.add_user(user, Gitlab::Access::MAINTAINER)
     end
@@ -96,6 +97,20 @@ describe Groups::UpdateService do
     end
   end
 
+  context 'when updating #emails_disabled' do
+    let(:service) { described_class.new(internal_group, user, emails_disabled: true) }
+
+    it 'updates the attribute' do
+      internal_group.add_user(user, Gitlab::Access::OWNER)
+
+      expect { service.execute }.to change { internal_group.emails_disabled }.to(true)
+    end
+
+    it 'does not update when not group owner' do
+      expect { service.execute }.not_to change { internal_group.emails_disabled }
+    end
+  end
+
   context 'rename group' do
     let!(:service) { described_class.new(internal_group, user, path: SecureRandom.hex) }
 

spec/services/notification_service_spec.rb

@@ -240,11 +240,14 @@ describe NotificationService, :mailer do
       end
 
       describe '#new_note' do
-        it do
+        context do
+          before do
             add_users(project)
             add_user_subscriptions(issue)
             reset_delivered_emails!
+          end
 
+          it do
             expect(SentNotification).to receive(:record).with(issue, any_args).exactly(10).times
 
             notification.new_note(note)
@@ -270,10 +273,6 @@ describe NotificationService, :mailer do
           end
 
           it "emails the note author if they've opted into notifications about their activity" do
-          add_users(project)
-          add_user_subscriptions(issue)
-          reset_delivered_emails!
-
             note.author.notified_of_own_activity = true
 
             notification.new_note(note)
@@ -281,6 +280,12 @@ describe NotificationService, :mailer do
             should_email(note.author)
           end
 
+          it_behaves_like 'project emails are disabled' do
+            let(:notification_target)  { note }
+            let(:notification_trigger) { notification.new_note(note) }
+          end
+        end
+
         it 'filters out "mentioned in" notes' do
           mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author)
 
@@ -337,6 +342,11 @@ describe NotificationService, :mailer do
 
         it_behaves_like 'new note notifications'
 
+        it_behaves_like 'project emails are disabled' do
+          let(:notification_target)  { note }
+          let(:notification_trigger) { notification.new_note(note) }
+        end
+
         context 'which is a subgroup' do
           let!(:parent) { create(:group) }
           let!(:group) { create(:group, parent: parent) }
@@ -472,6 +482,11 @@ describe NotificationService, :mailer do
           expect(Notify).not_to receive(:note_issue_email)
           notification.new_note(mentioned_note)
         end
+
+        it_behaves_like 'project emails are disabled' do
+          let(:notification_target)  { note }
+          let(:notification_trigger) { notification.new_note(note) }
+        end
       end
     end
 
@@ -619,6 +634,11 @@ describe NotificationService, :mailer do
           notification.new_note(note)
           should_not_email(@u_committer)
         end
+
+        it_behaves_like 'project emails are disabled' do
+          let(:notification_target)  { note }
+          let(:notification_trigger) { notification.new_note(note) }
+        end
       end
     end
 
@@ -645,6 +665,11 @@ describe NotificationService, :mailer do
             .to contain_exactly(*merge_request.assignees.pluck(:id), merge_request.author.id, @u_watcher.id)
           expect(SentNotification.last.in_reply_to_discussion_id).to eq(note.discussion_id)
         end
+
+        it_behaves_like 'project emails are disabled' do
+          let(:notification_target)  { note }
+          let(:notification_trigger) { notification.new_note(note) }
+        end
       end
     end
   end
@@ -819,6 +844,11 @@ describe NotificationService, :mailer do
         should_email(user_4)
       end
 
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { issue }
+        let(:notification_trigger) { notification.new_issue(issue, @u_disabled) }
+      end
+
       context 'confidential issues' do
         let(:author) { create(:user) }
         let(:assignee) { create(:user) }
@@ -861,6 +891,11 @@ describe NotificationService, :mailer do
       let(:mentionable) { issue }
 
       include_examples 'notifications for new mentions'
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { issue }
+        let(:notification_trigger) { send_notifications(@u_watcher, @u_participant_mentioned, @u_custom_global, @u_mentioned) }
+      end
     end
 
     describe '#reassigned_issue' do
@@ -969,6 +1004,11 @@ describe NotificationService, :mailer do
         let(:issuable) { issue }
         let(:notification_trigger) { notification.reassigned_issue(issue, @u_disabled, [assignee]) }
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { issue }
+        let(:notification_trigger) { notification.reassigned_issue(issue, @u_disabled, [assignee]) }
+      end
     end
 
     describe '#relabeled_issue' do
@@ -1028,6 +1068,11 @@ describe NotificationService, :mailer do
         should_email(subscriber_to_both)
       end
 
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { issue }
+        let(:notification_trigger) { notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled) }
+      end
+
       context 'confidential issues' do
         let(:author) { create(:user) }
         let(:assignee) { create(:user) }
@@ -1065,15 +1110,22 @@ describe NotificationService, :mailer do
     end
 
     describe '#removed_milestone_issue' do
-      it_behaves_like 'altered milestone notification on issue' do
+      context do
         let(:milestone) { create(:milestone, project: project, issues: [issue]) }
         let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } }
 
+        it_behaves_like 'altered milestone notification on issue' do
           before do
             notification.removed_milestone_issue(issue, issue.author)
           end
         end
 
+        it_behaves_like 'project emails are disabled' do
+          let(:notification_target)  { issue }
+          let(:notification_trigger) { notification.removed_milestone_issue(issue, issue.author) }
+        end
+      end
+
       context 'confidential issues' do
         let(:author) { create(:user) }
         let(:assignee) { create(:user) }
@@ -1110,15 +1162,22 @@ describe NotificationService, :mailer do
     end
 
     describe '#changed_milestone_issue' do
-      it_behaves_like 'altered milestone notification on issue' do
+      context do
         let(:new_milestone) { create(:milestone, project: project, issues: [issue]) }
         let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } }
 
+        it_behaves_like 'altered milestone notification on issue' do
           before do
             notification.changed_milestone_issue(issue, new_milestone, issue.author)
           end
         end
 
+        it_behaves_like 'project emails are disabled' do
+          let(:notification_target)  { issue }
+          let(:notification_trigger) { notification.changed_milestone_issue(issue, new_milestone, issue.author) }
+        end
+      end
+
       context 'confidential issues' do
         let(:author) { create(:user) }
         let(:assignee) { create(:user) }
@@ -1183,6 +1242,11 @@ describe NotificationService, :mailer do
         let(:issuable) { issue }
         let(:notification_trigger) { notification.close_issue(issue, @u_disabled) }
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { issue }
+        let(:notification_trigger) { notification.close_issue(issue, @u_disabled) }
+      end
     end
 
     describe '#reopen_issue' do
@@ -1214,6 +1278,11 @@ describe NotificationService, :mailer do
         let(:issuable) { issue }
         let(:notification_trigger) { notification.reopen_issue(issue, @u_disabled) }
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { issue }
+        let(:notification_trigger) { notification.reopen_issue(issue, @u_disabled) }
+      end
     end
 
     describe '#issue_moved' do
@@ -1240,6 +1309,11 @@ describe NotificationService, :mailer do
         let(:issuable) { issue }
         let(:notification_trigger) { notification.issue_moved(issue, new_issue, @u_disabled) }
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { issue }
+        let(:notification_trigger) { notification.issue_moved(issue, new_issue, @u_disabled) }
+      end
     end
 
     describe '#issue_due' do
@@ -1280,6 +1354,11 @@ describe NotificationService, :mailer do
         let(:issuable) { issue }
         let(:notification_trigger) { notification.issue_due(issue) }
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { issue }
+        let(:notification_trigger) { notification.issue_due(issue) }
+      end
     end
   end
 
@@ -1374,6 +1453,11 @@ describe NotificationService, :mailer do
         should_email(user_4)
       end
 
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.new_merge_request(merge_request, @u_disabled) }
+      end
+
       context 'participating' do
         it_should_behave_like 'participating by assignee notification' do
           let(:participant) { create(:user, username: 'user-participant')}
@@ -1406,6 +1490,11 @@ describe NotificationService, :mailer do
       let(:mentionable) { merge_request }
 
       include_examples 'notifications for new mentions'
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { send_notifications(@u_watcher, @u_participant_mentioned, @u_custom_global, @u_mentioned) }
+      end
     end
 
     describe '#reassigned_merge_request' do
@@ -1449,6 +1538,11 @@ describe NotificationService, :mailer do
         let(:issuable) { merge_request }
         let(:notification_trigger) { notification.reassigned_merge_request(merge_request, current_user, [assignee]) }
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.reassigned_merge_request(merge_request, current_user, [assignee]) }
+      end
     end
 
     describe '#push_to_merge_request' do
@@ -1479,6 +1573,11 @@ describe NotificationService, :mailer do
         let(:issuable) { merge_request }
         let(:notification_trigger) { notification.push_to_merge_request(merge_request, @u_disabled) }
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.push_to_merge_request(merge_request, @u_disabled) }
+      end
     end
 
     describe '#relabel_merge_request' do
@@ -1512,28 +1611,43 @@ describe NotificationService, :mailer do
         should_not_email(@u_participating)
         should_not_email(@u_lazy_participant)
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.relabeled_merge_request(merge_request, [group_label_2, label_2], @u_disabled) }
+      end
     end
 
     describe '#removed_milestone_merge_request' do
-      it_behaves_like 'altered milestone notification on merge request' do
       let(:milestone) { create(:milestone, project: project, merge_requests: [merge_request]) }
       let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } }
 
+      it_behaves_like 'altered milestone notification on merge request' do
         before do
           notification.removed_milestone_merge_request(merge_request, merge_request.author)
         end
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.removed_milestone_merge_request(merge_request, merge_request.author) }
+      end
     end
 
     describe '#changed_milestone_merge_request' do
-      it_behaves_like 'altered milestone notification on merge request' do
       let(:new_milestone) { create(:milestone, project: project, merge_requests: [merge_request]) }
       let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } }
 
+      it_behaves_like 'altered milestone notification on merge request' do
         before do
           notification.changed_milestone_merge_request(merge_request, new_milestone, merge_request.author)
         end
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.changed_milestone_merge_request(merge_request, new_milestone, merge_request.author) }
+      end
     end
 
     describe '#merge_request_unmergeable' do
@@ -1544,6 +1658,11 @@ describe NotificationService, :mailer do
         expect(email_recipients.size).to eq(1)
       end
 
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.merge_request_unmergeable(merge_request) }
+      end
+
       describe 'when merge_when_pipeline_succeeds is true' do
         before do
           merge_request.update(
@@ -1590,6 +1709,11 @@ describe NotificationService, :mailer do
         let(:issuable) { merge_request }
         let(:notification_trigger) { notification.close_mr(merge_request, @u_disabled) }
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.close_mr(merge_request, @u_disabled) }
+      end
     end
 
     describe '#merged_merge_request' do
@@ -1642,6 +1766,11 @@ describe NotificationService, :mailer do
         let(:issuable) { merge_request }
         let(:notification_trigger) { notification.merge_mr(merge_request, @u_disabled) }
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.merge_mr(merge_request, @u_disabled) }
+      end
     end
 
     describe '#reopen_merge_request' do
@@ -1672,6 +1801,11 @@ describe NotificationService, :mailer do
         let(:issuable) { merge_request }
         let(:notification_trigger) { notification.reopen_mr(merge_request, @u_disabled) }
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.reopen_mr(merge_request, @u_disabled) }
+      end
     end
 
     describe "#resolve_all_discussions" do
@@ -1695,6 +1829,11 @@ describe NotificationService, :mailer do
         let(:issuable) { merge_request }
         let(:notification_trigger) { notification.resolve_all_discussions(merge_request, @u_disabled) }
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.resolve_all_discussions(merge_request, @u_disabled) }
+      end
     end
   end
 
@@ -1719,6 +1858,11 @@ describe NotificationService, :mailer do
         should_not_email(@u_disabled)
       end
 
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { project }
+        let(:notification_trigger) { notification.project_was_moved(project, "gitlab/gitlab") }
+      end
+
       context 'users not having access to the new location' do
         it 'does not send email' do
           old_user = create(:user)
@@ -1762,6 +1906,11 @@ describe NotificationService, :mailer do
 
           should_only_email(@u_participating)
         end
+
+        it_behaves_like 'project emails are disabled' do
+          let(:notification_target)  { project }
+          let(:notification_trigger) { notification.project_exported(project, @u_participating) }
+        end
       end
 
       describe '#project_not_exported' do
@@ -1770,6 +1919,11 @@ describe NotificationService, :mailer do
 
           should_only_email(@u_participating)
         end
+
+        it_behaves_like 'project emails are disabled' do
+          let(:notification_target)  { project }
+          let(:notification_trigger) { notification.project_not_exported(project, @u_participating, ['error']) }
+        end
       end
     end
   end
@@ -1800,6 +1954,11 @@ describe NotificationService, :mailer do
         should_email(maintainer)
         should_not_email(developer)
       end
+
+      it_behaves_like 'group emails are disabled' do
+        let(:notification_target)  { group }
+        let(:notification_trigger) { group.request_access(added_user) }
+      end
     end
 
     describe '#decline_group_invite' do
@@ -1839,6 +1998,11 @@ describe NotificationService, :mailer do
           should_not_email_anyone
         end
       end
+
+      it_behaves_like 'group emails are disabled' do
+        let(:notification_target)  { group }
+        let(:notification_trigger) { group.add_guest(added_user) }
+      end
     end
   end
 
@@ -1859,6 +2023,11 @@ describe NotificationService, :mailer do
 
           should_only_email(project.owner)
         end
+
+        it_behaves_like 'project emails are disabled' do
+          let(:notification_target)  { project }
+          let(:notification_trigger) { project.request_access(added_user) }
+        end
       end
 
       context 'for a project in a group' do
@@ -1878,7 +2047,7 @@ describe NotificationService, :mailer do
       end
     end
 
-    describe '#decline_group_invite' do
+    describe '#decline_project_invite' do
       let(:member) { create(:user) }
 
       before do
@@ -1900,6 +2069,11 @@ describe NotificationService, :mailer do
         should_only_email(added_user)
       end
 
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { project }
+        let(:notification_trigger) { create_member! }
+      end
+
       context 'when notifications are disabled' do
         before do
           create_global_setting_for(added_user, :disabled)
@@ -2071,6 +2245,11 @@ describe NotificationService, :mailer do
             should_only_email(u_custom_notification_enabled, kind: :bcc)
           end
 
+          it_behaves_like 'project emails are disabled' do
+            let(:notification_target)  { pipeline }
+            let(:notification_trigger) { notification.pipeline_finished(pipeline) }
+          end
+
           context 'when the creator has group notification email set' do
             let(:group_notification_email) { 'user+group@example.com' }
 
@@ -2100,6 +2279,11 @@ describe NotificationService, :mailer do
             should_only_email(u_member, kind: :bcc)
           end
 
+          it_behaves_like 'project emails are disabled' do
+            let(:notification_target)  { pipeline }
+            let(:notification_trigger) { notification.pipeline_finished(pipeline) }
+          end
+
           context 'when the creator has group notification email set' do
             let(:group_notification_email) { 'user+group@example.com' }
 
@@ -2215,6 +2399,11 @@ describe NotificationService, :mailer do
           should_only_email(u_maintainer1, u_maintainer2, u_owner)
         end
 
+        it_behaves_like 'project emails are disabled' do
+          let(:notification_target)  { domain }
+          let(:notification_trigger) { notify! }
+        end
+
         it 'emails nobody if the project is missing' do
           domain.project = nil
 
@@ -2224,30 +2413,6 @@ describe NotificationService, :mailer do
         end
       end
     end
-
-    describe '#pages_domain_verification_failed' do
-      it 'emails current watching maintainers' do
-        notification.pages_domain_verification_failed(domain)
-
-        should_only_email(u_maintainer1, u_maintainer2, u_owner)
-      end
-    end
-
-    describe '#pages_domain_enabled' do
-      it 'emails current watching maintainers' do
-        notification.pages_domain_enabled(domain)
-
-        should_only_email(u_maintainer1, u_maintainer2, u_owner)
-      end
-    end
-
-    describe '#pages_domain_disabled' do
-      it 'emails current watching maintainers' do
-        notification.pages_domain_disabled(domain)
-
-        should_only_email(u_maintainer1, u_maintainer2, u_owner)
-      end
-    end
   end
 
   context 'Auto DevOps notifications' do
@@ -2266,6 +2431,11 @@ describe NotificationService, :mailer do
         should_email(owner, times: 1)         # Once for the disable pipeline.
         should_email(pipeline_user, times: 2) # Once for the new permission, once for the disable.
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { project }
+        let(:notification_trigger) { notification.autodevops_disabled(pipeline, [owner.email, pipeline_user.email]) }
+      end
     end
   end
 
@@ -2279,6 +2449,11 @@ describe NotificationService, :mailer do
 
         should_email(user)
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { project }
+        let(:notification_trigger) { notification.repository_cleanup_success(project, user) }
+      end
     end
 
     describe '#repository_cleanup_failure' do
@@ -2287,6 +2462,11 @@ describe NotificationService, :mailer do
 
         should_email(user)
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { project }
+        let(:notification_trigger) { notification.repository_cleanup_failure(project, user, 'Some error') }
+      end
     end
   end
 
@@ -2320,6 +2500,11 @@ describe NotificationService, :mailer do
 
         should_only_email(u_maintainer1, u_maintainer2, u_owner)
       end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { project }
+        let(:notification_trigger) { notification.remote_mirror_update_failed(remote_mirror) }
+      end
     end
   end
 

spec/services/projects/update_service_spec.rb

@@ -369,9 +369,28 @@ describe Projects::UpdateService do
       end
     end
 
+    context 'when updating #emails_disabled' do
+      it 'updates the attribute for the project owner' do
+        expect { update_project(project, user, emails_disabled: true) }
+          .to change { project.emails_disabled }
+          .to(true)
+      end
+
+      it 'does not update when not project owner' do
+        maintainer = create(:user)
+        project.add_user(maintainer, :maintainer)
+
+        expect { update_project(project, maintainer, emails_disabled: true) }
+          .not_to change { project.emails_disabled }
+      end
+    end
+
     context 'with external authorization enabled' do
       before do
         enable_external_authorization_service_check
+
+        allow(::Gitlab::ExternalAuthorization)
+          .to receive(:access_allowed?).with(user, 'default_label', project.full_path).and_call_original
       end
 
       it 'does not save the project with an error if the service denies access' do
@@ -402,8 +421,7 @@ describe Projects::UpdateService do
       end
 
       it 'does not check the label when it does not change' do
-        expect(::Gitlab::ExternalAuthorization)
-          .not_to receive(:access_allowed?)
+        expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).once
 
         update_project(project, user, { name: 'New name' })
       end

spec/support/helpers/email_helpers.rb

@@ -31,6 +31,10 @@ module EmailHelpers
     expect(ActionMailer::Base.deliveries).to be_empty
   end
 
+  def should_email_anyone
+    expect(ActionMailer::Base.deliveries).not_to be_empty
+  end
+
   def email_recipients(kind: :to)
     ActionMailer::Base.deliveries.flat_map(&kind)
   end

spec/support/shared_examples/services/notification_service_shared_examples.rb

+# frozen_string_literal: true
+
+# Note that we actually update the attribute on the target_project/group, rather than
+# using `allow`.  This is because there are some specs where, based on how the notification
+# is done, using an `allow` doesn't change the correct object.
+shared_examples 'project emails are disabled' do
+  let(:target_project) { notification_target.is_a?(Project) ? notification_target : notification_target.project }
+
+  before do
+    reset_delivered_emails!
+    target_project.clear_memoization(:emails_disabled)
+  end
+
+  it 'sends no emails with project emails disabled' do
+    target_project.update_attribute(:emails_disabled, true)
+
+    notification_trigger
+
+    should_not_email_anyone
+  end
+
+  it 'sends emails to someone' do
+    target_project.update_attribute(:emails_disabled, false)
+
+    notification_trigger
+
+    should_email_anyone
+  end
+end
+
+shared_examples 'group emails are disabled' do
+  let(:target_group) { notification_target.is_a?(Group) ? notification_target : notification_target.project.group }
+
+  before do
+    reset_delivered_emails!
+    target_group.clear_memoization(:emails_disabled)
+  end
+
+  it 'sends no emails with group emails disabled' do
+    target_group.update_attribute(:emails_disabled, true)
+
+    notification_trigger
+
+    should_not_email_anyone
+  end
+
+  it 'sends emails to someone' do
+    target_group.update_attribute(:emails_disabled, false)
+
+    notification_trigger
+
+    should_email_anyone
+  end
+end