Commit 85414834 authored by Brett Walker's avatar Brett Walker Committed by Douglas Barbosa Alexandre
Browse files

Allow disabling group/project email notifications

- Adds UI to configure in group and project settings
- Removes notification configuration for users when
disabled at group or project level
parent fca952c4595f
......@@ -176,6 +176,7 @@ def group_params_attributes
[
:avatar,
:description,
:emails_disabled,
:lfs_enabled,
:name,
:path,
......
......@@ -361,6 +361,7 @@ def project_params_attributes
:container_registry_enabled,
:default_branch,
:description,
:emails_disabled,
:external_authorization_classification_label,
:import_url,
:issues_tracker,
......
......@@ -172,6 +172,13 @@ def find_fork_of(project)
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
......
......@@ -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 @@ def notification_level
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 @@ def excluded_watcher_action?
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)
......
......@@ -631,6 +631,13 @@ def ancestors_upto(top = nil, hierarchy_order: nil)
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?
......
......@@ -24,6 +24,7 @@ def self.supported_events
def execute(push_data)
return unless supported_events.include?(push_data[:object_kind])
return if project.emails_disabled?
EmailsOnPushWorker.perform_async(
project_id,
......
......@@ -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
......
......@@ -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
......
......@@ -46,6 +46,11 @@ def reject_parent_id!
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)
......
......@@ -321,6 +321,9 @@ def accept_project_invite(project_member)
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 @@ def accept_group_invite(group_member)
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 @@ def project_not_exported(project, current_user, errors)
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 @@ def pipeline_finished(pipeline, recipients = nil)
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 @@ def issue_due(issue)
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
......
......@@ -9,6 +9,7 @@ class UpdateService < BaseService
# rubocop: disable CodeReuse/ActiveRecord
def execute
remove_unallowed_params
validate!
ensure_wiki_exists if enabling_wiki?
......@@ -54,6 +55,10 @@ def validate!
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
......
---
title: Allow email notifications to be disabled for all members of a group or project
merge_request: 30755
author: Dustin Spicuzza
type: added
# frozen_string_literal: true
class AddProjectEmailsDisabled < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
add_column :projects, :emails_disabled, :boolean
end
end
# frozen_string_literal: true
class AddGroupEmailsDisabled < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
add_column :namespaces, :emails_disabled, :boolean
end
end
......@@ -2175,6 +2175,7 @@
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 @@
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"
......
......@@ -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
......
......@@ -16,6 +16,19 @@
)
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'
......
......@@ -853,4 +853,64 @@ def project_rugged(project)
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
......@@ -9,6 +9,38 @@
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
......
......@@ -20,4 +20,24 @@
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
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment