diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 72a5c4d3e69ef8378efada955da9577743e0da65..5b9b3b7de119495908566c04327a272c9c74d249 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -279,6 +279,7 @@ def group_params_attributes :avatar, :description, :emails_disabled, + :emails_enabled, :show_diff_preview_in_email, :mentions_disabled, :lfs_enabled, diff --git a/app/finders/user_group_notification_settings_finder.rb b/app/finders/user_group_notification_settings_finder.rb index c6a1a6b36d1a448deb7ff8a7a569ae160b164a7f..8d06d3d18caebcecad71fff3e638b8c96f056a86 100644 --- a/app/finders/user_group_notification_settings_finder.rb +++ b/app/finders/user_group_notification_settings_finder.rb @@ -11,11 +11,16 @@ def execute @loaded_groups_with_ancestors = groups_with_ancestors.index_by(&:id) @loaded_notification_settings = user.notification_settings_for_groups(groups_with_ancestors).preload_source_route.index_by(&:source_id) - preload_emails_disabled + preload_emails_enabled - groups.map do |group| + group_notifications = groups.map do |group| find_notification_setting_for(group) end + + group_sources = group_notifications.map(&:source) + ActiveRecord::Associations::Preloader.new(records: group_sources, associations: :namespace_settings).call + + group_notifications end private @@ -45,18 +50,18 @@ def should_copy?(parent_setting) parent_setting.level != NotificationSetting.levels[:global] || parent_setting.notification_email.present? end - # This method preloads the `emails_disabled` strong memoized method for the given groups. + # This method preloads the `emails_enabled` strong memoized method for the given groups. # - # For each group, look up the ancestor hierarchy and look for any group where emails_disabled is true. + # For each group, look up the ancestor hierarchy and look for any group where emails_enabled is false. # The lookup is implemented with an EXISTS subquery, so we can look up the ancestor chain for each group individually. # The query will return groups where at least one ancestor has the `emails_disabled` set to true. # # After the query, we set the instance variable. - def preload_emails_disabled + def preload_emails_enabled group_ids_with_disabled_email = Group.ids_with_disabled_email(groups.to_a) groups.each do |group| - group.emails_disabled_memoized = group_ids_with_disabled_email.include?(group.id) if group.parent_id + group.emails_enabled_memoized = group_ids_with_disabled_email.exclude?(group.id) if group.parent_id end end end diff --git a/app/models/group.rb b/app/models/group.rb index 919b80ccffb1e1eb4b0ae9feeeef5acb19695b8b..492f419593180a6df98381eb9b92ae92d3a3d144 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -300,14 +300,15 @@ def preset_root_ancestor_for(groups) groups.drop(1).each { |group| group.root_ancestor = root } end - # Returns the ids of the passed group models where the `emails_disabled` - # column is set to true anywhere in the ancestor hierarchy. + # Returns the ids of the passed group models where the `emails_enabled` + # column is set to false anywhere in the ancestor hierarchy. def ids_with_disabled_email(groups) inner_groups = Group.where('id = namespaces_with_emails_disabled.id') inner_query = inner_groups .self_and_ancestors - .where(emails_disabled: true) + .joins(:namespace_settings) + .where(namespace_settings: { emails_enabled: false }) .select('1') .limit(1) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 733b89fcaf254de0fe5c4fe60fa76f2ba9da3b13..5abda6196c1f606c64eaa92062358aba35bed55d 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -138,6 +138,8 @@ class Namespace < ApplicationRecord to: :namespace_settings delegate :runner_registration_enabled, :runner_registration_enabled?, :runner_registration_enabled=, to: :namespace_settings + delegate :emails_enabled, :emails_enabled=, + to: :namespace_settings, allow_nil: true delegate :allow_runner_registration_token, :allow_runner_registration_token=, to: :namespace_settings @@ -204,7 +206,7 @@ class Namespace < ApplicationRecord # Make sure that the name is same as strong_memoize name in root_ancestor # method - attr_writer :root_ancestor, :emails_disabled_memoized + attr_writer :root_ancestor, :emails_enabled_memoized class << self def sti_class_for(type_name) @@ -382,17 +384,16 @@ def find_fork_of(project) # any ancestor can disable emails for all descendants def emails_disabled? - strong_memoize(:emails_disabled_memoized) do - if parent_id - self_and_ancestors.where(emails_disabled: true).exists? - else - !!emails_disabled - end - end + !emails_enabled? end def emails_enabled? - !emails_disabled? + # If no namespace_settings, we can assume it has not changed from enabled + return true unless namespace_settings + + strong_memoize(:emails_enabled_memoized) do + namespace_settings.emails_enabled? + end end def lfs_enabled? diff --git a/app/models/namespace_setting.rb b/app/models/namespace_setting.rb index 3befcdeaec5d9a826f7a1be5244afed25dc798fd..13d2c5a62e2be091ef4d736ce828b1efa9d0da8c 100644 --- a/app/models/namespace_setting.rb +++ b/app/models/namespace_setting.rb @@ -63,6 +63,12 @@ def prevent_sharing_groups_outside_hierarchy namespace.root_ancestor.prevent_sharing_groups_outside_hierarchy end + def emails_enabled? + return emails_enabled unless namespace.has_parent? + + all_ancestors_have_emails_enabled? + end + def show_diff_preview_in_email? return show_diff_preview_in_email unless namespace.has_parent? @@ -89,6 +95,10 @@ def allow_runner_registration_token? private + def all_ancestors_have_emails_enabled? + self.class.where(namespace_id: namespace.self_and_ancestors, emails_enabled: false).none? + end + def all_ancestors_allow_diff_preview_in_email? !self.class.where(namespace_id: namespace.self_and_ancestors, show_diff_preview_in_email: false).exists? end diff --git a/doc/api/groups.md b/doc/api/groups.md index 6b17af63853ecb9f0b3ea047b45ff7ffa9dfde2c..879e0626305e98ef97086c726c9438e73838dd28 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -59,6 +59,7 @@ GET /groups "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, + "emails_enabled": null, "mentions_disabled": null, "lfs_enabled": true, "default_branch_protection": 2, @@ -97,6 +98,7 @@ GET /groups?statistics=true "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, + "emails_enabled": null, "mentions_disabled": null, "lfs_enabled": true, "default_branch_protection": 2, @@ -181,6 +183,7 @@ GET /groups/:id/subgroups "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, + "emails_enabled": null, "mentions_disabled": null, "lfs_enabled": true, "default_branch_protection": 2, @@ -242,6 +245,7 @@ GET /groups/:id/descendant_groups "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, + "emails_enabled": null, "mentions_disabled": null, "lfs_enabled": true, "default_branch_protection": 2, @@ -267,6 +271,7 @@ GET /groups/:id/descendant_groups "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, + "emails_enabled": null, "mentions_disabled": null, "lfs_enabled": true, "default_branch_protection": 2, @@ -467,6 +472,7 @@ Example response: "pages_access_level":"enabled", "security_and_compliance_access_level":"enabled", "emails_disabled":null, + "emails_enabled": null, "shared_runners_enabled":true, "lfs_enabled":true, "creator_id":1, @@ -818,7 +824,8 @@ Parameters: | `avatar` | mixed | no | Image file for avatar of the group. [Introduced in GitLab 12.9](https://gitlab.com/gitlab-org/gitlab/-/issues/36681) | | `default_branch_protection` | integer | no | See [Options for `default_branch_protection`](#options-for-default_branch_protection). Default to the global level default branch protection setting. | | `description` | string | no | The group's description. | -| `emails_disabled` | boolean | no | Disable email notifications. | +| `emails_disabled` | boolean | no | _([Deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127899) in GitLab 16.5.)_ Disable email notifications. Use `emails_enabled` instead. | +| `emails_enabled` | boolean | no | Enable email notifications. | | `lfs_enabled` | boolean | no | Enable/disable Large File Storage (LFS) for the projects in this group. | | `mentions_disabled` | boolean | no | Disable the capability of a group from getting mentioned. | | `parent_id` | integer | no | The parent group ID for creating nested group. | @@ -975,7 +982,8 @@ PUT /groups/:id | `avatar` | mixed | no | Image file for avatar of the group. [Introduced in GitLab 12.9](https://gitlab.com/gitlab-org/gitlab/-/issues/36681) | | `default_branch_protection` | integer | no | See [Options for `default_branch_protection`](#options-for-default_branch_protection). | | `description` | string | no | The description of the group. | -| `emails_disabled` | boolean | no | Disable email notifications. | +| `emails_disabled` | boolean | no | _([Deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127899) in GitLab 16.5.)_ Disable email notifications. Use `emails_enabled` instead. | +| `emails_enabled` | boolean | no | Enable email notifications. | | `lfs_enabled` | boolean | no | Enable/disable Large File Storage (LFS) for the projects in this group. | | `mentions_disabled` | boolean | no | Disable the capability of a group from getting mentioned. | | `prevent_sharing_groups_outside_hierarchy` | boolean | no | See [Prevent group sharing outside the group hierarchy](../user/group/access_and_permissions.md#prevent-group-sharing-outside-the-group-hierarchy). This attribute is only available on top-level groups. [Introduced in GitLab 14.1](https://gitlab.com/gitlab-org/gitlab/-/issues/333721) | diff --git a/lib/api/entities/group.rb b/lib/api/entities/group.rb index d18a29ce4d451b7b4530e90e13e96f30486ce22a..1a1765c2e0a96d2a6a701b005566f3a7e900934f 100644 --- a/lib/api/entities/group.rb +++ b/lib/api/entities/group.rb @@ -10,7 +10,8 @@ class Group < BasicGroupDetails expose :project_creation_level_str, as: :project_creation_level expose :auto_devops_enabled expose :subgroup_creation_level_str, as: :subgroup_creation_level - expose :emails_disabled + expose(:emails_disabled, documentation: { type: 'boolean' }) { |group, options| group.emails_disabled? } + expose :emails_enabled, documentation: { type: 'boolean' } expose :mentions_disabled expose :lfs_enabled?, as: :lfs_enabled expose :default_branch_protection diff --git a/lib/api/helpers/groups_helpers.rb b/lib/api/helpers/groups_helpers.rb index f7802938d8b415198b5bc04349c26ac7bd974ef6..fbe13bfe8f741d3fb34b088064e77828d8d9add3 100644 --- a/lib/api/helpers/groups_helpers.rb +++ b/lib/api/helpers/groups_helpers.rb @@ -18,7 +18,8 @@ module GroupsHelpers optional :project_creation_level, type: String, values: ::Gitlab::Access.project_creation_string_values, desc: 'Determine if developers can create projects in the group', as: :project_creation_level_str optional :auto_devops_enabled, type: Boolean, desc: 'Default to Auto DevOps pipeline for all projects within this group' optional :subgroup_creation_level, type: String, values: ::Gitlab::Access.subgroup_creation_string_values, desc: 'Allowed to create subgroups', as: :subgroup_creation_level_str - optional :emails_disabled, type: Boolean, desc: 'Disable email notifications' + optional :emails_disabled, type: Boolean, desc: '_(Deprecated)_ Disable email notifications. Use: emails_enabled' + optional :emails_enabled, type: Boolean, desc: 'Enable email notifications' optional :mentions_disabled, type: Boolean, desc: 'Disable a group from getting mentioned' optional :lfs_enabled, type: Boolean, desc: 'Enable/disable LFS for the projects in this group' optional :request_access_enabled, type: Boolean, desc: 'Allow users to request member access' diff --git a/spec/controllers/profiles/notifications_controller_spec.rb b/spec/controllers/profiles/notifications_controller_spec.rb index 22c0a62a6a1495a11c7302319d1c2c4b0c2d7b65..ef49eb911ba7202ae6c9e2f73c75a349bdbf31d8 100644 --- a/spec/controllers/profiles/notifications_controller_spec.rb +++ b/spec/controllers/profiles/notifications_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Profiles::NotificationsController do +RSpec.describe Profiles::NotificationsController, feature_category: :team_planning do let(:user) do create(:user) do |user| user.emails.create!(email: 'original@example.com', confirmed_at: Time.current) diff --git a/spec/features/groups/group_settings_spec.rb b/spec/features/groups/group_settings_spec.rb index a248a2b471a2284351b9e4804d3c28e2f81eb383..0437e5df6e9e0f127e3edaf51ec719e4620e0bb9 100644 --- a/spec/features/groups/group_settings_spec.rb +++ b/spec/features/groups/group_settings_spec.rb @@ -329,7 +329,7 @@ def save_permissions_group end def updated_emails_disabled? - group.reload.clear_memoization(:emails_disabled_memoized) + group.reload.clear_memoization(:emails_enabled_memoized) group.emails_disabled? end end diff --git a/spec/features/groups/show_spec.rb b/spec/features/groups/show_spec.rb index 8450322945c24c181fd24ac366052957aef863f2..cf18f3cb4e58f2476d075f4d91444ff038337854 100644 --- a/spec/features/groups/show_spec.rb +++ b/spec/features/groups/show_spec.rb @@ -276,7 +276,7 @@ end it 'is disabled if emails are disabled' do - group.update!(emails_disabled: true) + group.update!(emails_enabled: false) visit path diff --git a/spec/finders/user_group_notification_settings_finder_spec.rb b/spec/finders/user_group_notification_settings_finder_spec.rb index ac59a42d813a0703a7d545980362c29652cefdf0..83d0d343c044507a00c974b4cf67da15a4a09bce 100644 --- a/spec/finders/user_group_notification_settings_finder_spec.rb +++ b/spec/finders/user_group_notification_settings_finder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe UserGroupNotificationSettingsFinder do +RSpec.describe UserGroupNotificationSettingsFinder, feature_category: :team_planning do let_it_be(:user) { create(:user) } subject { described_class.new(user, Group.where(id: groups.map(&:id))).execute } @@ -127,38 +127,38 @@ def attributes(&proc) expect(result.count).to eq(3) end - end - end - context 'preloading `emails_disabled`' do - let_it_be(:root_group) { create(:group) } - let_it_be(:sub_group) { create(:group, parent: root_group) } - let_it_be(:sub_sub_group) { create(:group, parent: sub_group) } + context 'preloading `emails_enabled`' do + let_it_be(:root_group) { create(:group) } + let_it_be(:sub_group) { create(:group, parent: root_group) } + let_it_be(:sub_sub_group) { create(:group, parent: sub_group) } - let_it_be(:another_root_group) { create(:group) } - let_it_be(:sub_group_with_emails_disabled) { create(:group, emails_disabled: true, parent: another_root_group) } - let_it_be(:another_sub_sub_group) { create(:group, parent: sub_group_with_emails_disabled) } + let_it_be(:another_root_group) { create(:group) } + let_it_be(:sub_group_with_emails_disabled) { create(:group, emails_enabled: false, parent: another_root_group) } + let_it_be(:another_sub_sub_group) { create(:group, parent: sub_group_with_emails_disabled) } - let_it_be(:root_group_with_emails_disabled) { create(:group, emails_disabled: true) } - let_it_be(:group) { create(:group, parent: root_group_with_emails_disabled) } + let_it_be(:root_group_with_emails_disabled) { create(:group, emails_enabled: false) } + let_it_be(:group) { create(:group, parent: root_group_with_emails_disabled) } - let(:groups) { Group.where(id: [sub_sub_group, another_sub_sub_group, group]) } + let(:groups) { Group.where(id: [sub_sub_group, another_sub_sub_group, group]) } - before do - described_class.new(user, groups).execute - end + before do + described_class.new(user, groups).execute + end - it 'preloads the `group.emails_disabled` method' do - recorder = ActiveRecord::QueryRecorder.new do - groups.each(&:emails_disabled?) - end + it 'preloads the `group.emails_enabled` method' do + recorder = ActiveRecord::QueryRecorder.new do + groups.each(&:emails_enabled?) + end - expect(recorder.count).to eq(0) - end + expect(recorder.count).to eq(0) + end - it 'preloads the `group.emails_disabled` method correctly' do - groups.each do |group| - expect(group.emails_disabled?).to eq(Group.find(group.id).emails_disabled?) # compare the memoized and the freshly loaded value + it 'preloads the `group.emails_enabled` method correctly' do + groups.each do |group| + expect(group.emails_enabled?).to eq(Group.find(group.id).emails_enabled?) # compare the memoized and the freshly loaded value + end + end end end end diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 0db15541b99e755e918b6aeb5272942e307a157b..f5cadfc2761d153d2f95144073c0fb1fa172c3a4 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -288,13 +288,13 @@ end it 'returns false if parent group is disabling emails' do - allow(group).to receive(:emails_disabled?).and_return(true) + allow(group).to receive(:emails_enabled?).and_return(false) expect(helper.can_disable_group_emails?(subgroup)).to be_falsey end it 'returns true if parent group is not disabling emails' do - allow(group).to receive(:emails_disabled?).and_return(false) + allow(group).to receive(:emails_enabled?).and_return(true) expect(helper.can_disable_group_emails?(subgroup)).to be_truthy end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 0ae4b35af7a9c5f247fc362d1e21bd15649970c6..37b4a9011f4783caa0c2a49037f67e53f9b56c20 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2930,18 +2930,21 @@ def define_cache_expectations(cache_key) end describe '.ids_with_disabled_email' do - let_it_be(:parent_1) { create(:group, emails_disabled: true) } + let_it_be(:parent_1) { create(:group) } let_it_be(:child_1) { create(:group, parent: parent_1) } - let_it_be(:parent_2) { create(:group, emails_disabled: false) } + let_it_be(:parent_2) { create(:group) } let_it_be(:child_2) { create(:group, parent: parent_2) } - let_it_be(:other_group) { create(:group, emails_disabled: false) } + let_it_be(:other_group) { create(:group) } shared_examples 'returns namespaces with disabled email' do subject(:group_ids_where_email_is_disabled) { described_class.ids_with_disabled_email([child_1, child_2, other_group]) } - it { is_expected.to eq(Set.new([child_1.id])) } + it do + parent_1.update_attribute(:emails_enabled, false) + is_expected.to eq(Set.new([child_1.id])) + end end it_behaves_like 'returns namespaces with disabled email' diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index e9822d974477d739d9e2b974aa2295ba0d0e00e5..c7449e047b02003907cd2a890e45723a49286502 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -216,32 +216,54 @@ end end - describe '#emails_enabled?' do - context 'when a group has no parent' - let(:settings) { create(:namespace_settings, emails_enabled: true) } - let(:grandparent) { create(:group) } + context 'when a group has parent groups' do + let(:grandparent) { create(:group, namespace_settings: settings) } let(:parent) { create(:group, parent: grandparent) } - let(:group) { create(:group, parent: parent, namespace_settings: settings) } + let!(:group) { create(:group, parent: parent) } - context 'when the groups setting is changed' do - it 'returns false when the attribute is false' do - group.update_attribute(:emails_disabled, true) + context "when a parent group has disabled diff previews" do + let(:settings) { create(:namespace_settings, show_diff_preview_in_email: false) } - expect(group.emails_enabled?).to be_falsey + it 'returns false' do + expect(group.show_diff_preview_in_email?).to be_falsey end end - context 'when a group has a parent' do - it 'returns true when no parent has disabled emails' do - expect(group.emails_enabled?).to be_truthy + context 'when all parent groups have enabled diff previews' do + let(:settings) { create(:namespace_settings, show_diff_preview_in_email: true) } + + it 'returns true' do + expect(group.show_diff_preview_in_email?).to be_truthy end + end + end + end - context 'when ancestor emails are disabled' do - it 'returns false' do - grandparent.update_attribute(:emails_disabled, true) + describe '#emails_enabled?' do + context 'when a group has no parent' + let(:settings) { create(:namespace_settings, emails_enabled: true) } + let(:grandparent) { create(:group) } + let(:parent) { create(:group, parent: grandparent) } + let(:group) { create(:group, parent: parent, namespace_settings: settings) } - expect(group.emails_enabled?).to be_falsey - end + context 'when the groups setting is changed' do + it 'returns false when the attribute is false' do + group.update_attribute(:emails_enabled, false) + + expect(group.emails_enabled?).to be_falsey + end + end + + context 'when a group has a parent' do + it 'returns true when no parent has disabled emails' do + expect(group.emails_enabled?).to be_truthy + end + + context 'when ancestor emails are disabled' do + it 'returns false' do + grandparent.update_attribute(:emails_enabled, false) + + expect(group.emails_enabled?).to be_falsey end end end @@ -251,19 +273,19 @@ let(:parent) { create(:group, parent: grandparent) } let!(:group) { create(:group, parent: parent) } - context "when a parent group has disabled diff previews" do - let(:settings) { create(:namespace_settings, show_diff_preview_in_email: false) } + context "when a parent group has emails disabled" do + let(:settings) { create(:namespace_settings, emails_enabled: false) } it 'returns false' do - expect(group.show_diff_preview_in_email?).to be_falsey + expect(group.emails_enabled?).to be_falsey end end - context 'when all parent groups have enabled diff previews' do - let(:settings) { create(:namespace_settings, show_diff_preview_in_email: true) } + context 'when all parent groups have emails enabled' do + let(:settings) { create(:namespace_settings, emails_enabled: true) } it 'returns true' do - expect(group.show_diff_preview_in_email?).to be_truthy + expect(group.emails_enabled?).to be_truthy end end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 9974aac3c6ccaa269007d91320db2c0cd2d041ad..a460e47a0e5e334a5f910bbe373f1dd25801e937 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1863,20 +1863,22 @@ describe '#emails_disabled?' do context 'when not a subgroup' do + let(:group) { create(:group) } + it 'returns false' do - group = create(:group, emails_disabled: false) + group.update_attribute(:emails_enabled, true) expect(group.emails_disabled?).to be_falsey end it 'returns true' do - group = create(:group, emails_disabled: true) + group.update_attribute(:emails_enabled, false) expect(group.emails_disabled?).to be_truthy end it 'does not query the db when there is no parent group' do - group = create(:group, emails_disabled: true) + group.update_attribute(:emails_enabled, false) expect { group.emails_disabled? }.not_to exceed_query_limit(0) end @@ -1903,7 +1905,8 @@ describe '#emails_enabled?' do context 'without a persisted namespace_setting object' do - let(:group) { build(:group, emails_disabled: false) } + let(:group_settings) { create(:namespace_settings) } + let(:group) { build(:group, emails_disabled: false, namespace_settings: group_settings) } it "is the opposite of emails_disabled" do expect(group.emails_enabled?).to be_truthy diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb index 2275bea4c7fbed5233a1595f1896e5fd3ab7f5f5..f19c0a68f871de441a76e2937d8ef62c70425a9b 100644 --- a/spec/models/notification_recipient_spec.rb +++ b/spec/models/notification_recipient_spec.rb @@ -14,7 +14,7 @@ context 'when emails are disabled' do it 'returns false if group disabled' do - expect(project.namespace).to receive(:emails_disabled?).and_return(true) + expect(project.namespace).to receive(:emails_enabled?).and_return(false) expect(recipient).to receive(:emails_disabled?).and_call_original expect(recipient.notifiable?).to eq false end @@ -28,7 +28,7 @@ context 'when emails are enabled' do it 'returns true if group enabled' do - expect(project.namespace).to receive(:emails_disabled?).and_return(false) + expect(project.namespace).to receive(:emails_enabled?).and_return(true) expect(recipient).to receive(:emails_disabled?).and_call_original expect(recipient.notifiable?).to eq true end diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 719e51018ac1475006b5aa2e7fc958eff3296f21..8ad232b7e0c58c82e8a9b97ad6e74b08e6fb373f 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -214,7 +214,7 @@ def valid_target_platform_combinations context 'when emails are enabled in parent group' do before do - allow(project.namespace).to receive(:emails_disabled?).and_return(false) + allow(project.namespace).to receive(:emails_enabled?).and_return(true) end it 'returns true' do diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 662e11f7cfbfe4102e825b9e9f9fd65924ab5060..ee1f3deab5033b5251b2397b57362d14ca8598f0 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -572,7 +572,8 @@ def response_project_ids(json_response, key) expect(json_response['require_two_factor_authentication']).to eq(group1.require_two_factor_authentication) expect(json_response['two_factor_grace_period']).to eq(group1.two_factor_grace_period) expect(json_response['auto_devops_enabled']).to eq(group1.auto_devops_enabled) - expect(json_response['emails_disabled']).to eq(group1.emails_disabled) + expect(json_response['emails_disabled']).to eq(group1.emails_disabled?) + expect(json_response['emails_enabled']).to eq(group1.emails_enabled?) expect(json_response['mentions_disabled']).to eq(group1.mentions_disabled) expect(json_response['project_creation_level']).to eq('maintainer') expect(json_response['subgroup_creation_level']).to eq('maintainer') @@ -895,7 +896,8 @@ def make_upload_request expect(json_response['require_two_factor_authentication']).to eq(false) expect(json_response['two_factor_grace_period']).to eq(48) expect(json_response['auto_devops_enabled']).to eq(nil) - expect(json_response['emails_disabled']).to eq(nil) + expect(json_response['emails_disabled']).to eq(false) + expect(json_response['emails_enabled']).to eq(true) expect(json_response['mentions_disabled']).to eq(nil) expect(json_response['project_creation_level']).to eq("noone") expect(json_response['subgroup_creation_level']).to eq("maintainer") diff --git a/spec/support/shared_examples/services/notification_service_shared_examples.rb b/spec/support/shared_examples/services/notification_service_shared_examples.rb index df1ae67a590da5161f011f1e068bcf779fcc7af1..c53872ca4bc402068e1ed2ac46ac6256f269f3a4 100644 --- a/spec/support/shared_examples/services/notification_service_shared_examples.rb +++ b/spec/support/shared_examples/services/notification_service_shared_examples.rb @@ -45,7 +45,7 @@ before do reset_delivered_emails! - target_group.clear_memoization(:emails_disabled_memoized) + target_group.clear_memoization(:emails_enabled_memoized) end it 'sends no emails with group emails disabled' do