From cc525fdc62c46e45a5f3d790d38c557ba9614360 Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Thu, 27 Jul 2023 15:00:48 -0400
Subject: [PATCH 1/5] Update backend for namespaces for emails_enabled

Add a delegate to use the namespace_settings for the emails_enabled
data.

Add functions to namespace_setting to answer the `emails_enabled?`
question by checking all of the ancestor namespaces.

Add parameters to the API which show the new attribute

Update the documentation for the API as well.

Changelog: added
---
 app/controllers/groups_controller.rb          |  1 +
 app/models/group.rb                           |  3 +-
 app/models/namespace.rb                       | 13 ++--
 app/models/namespace_setting.rb               | 10 +++
 doc/api/groups.md                             | 12 +++-
 lib/api/entities/group.rb                     |  3 +-
 lib/api/helpers/groups_helpers.rb             |  3 +-
 spec/features/groups/show_spec.rb             |  2 +-
 ...group_notification_settings_finder_spec.rb |  4 +-
 spec/helpers/groups_helper_spec.rb            |  4 +-
 spec/models/group_spec.rb                     | 11 +--
 spec/models/namespace_setting_spec.rb         | 68 ++++++++++++-------
 spec/models/namespace_spec.rb                 | 11 +--
 spec/models/notification_recipient_spec.rb    |  4 +-
 spec/models/project_setting_spec.rb           |  2 +-
 15 files changed, 101 insertions(+), 50 deletions(-)

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 72a5c4d3e69ef8..5b9b3b7de11949 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/models/group.rb b/app/models/group.rb
index 919b80ccffb1e1..8a43a40cfb7f5b 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -307,7 +307,8 @@ def ids_with_disabled_email(groups)
 
       inner_query = inner_groups
         .self_and_ancestors
-        .where(emails_disabled: true)
+        .joins(:namespace_settings)
+        .where('namespace_settings.emails_enabled IS FALSE')
         .select('1')
         .limit(1)
 
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 733b89fcaf254d..8181792e3ca48f 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
@@ -383,16 +385,15 @@ 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
+      !emails_enabled?
     end
   end
 
   def emails_enabled?
-    !emails_disabled?
+    # If no namespace_settings, we can assume it has not changed from enabled
+    return true unless namespace_settings
+
+    namespace_settings.emails_enabled?
   end
 
   def lfs_enabled?
diff --git a/app/models/namespace_setting.rb b/app/models/namespace_setting.rb
index 3befcdeaec5d9a..675ae54831d691 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).exists?
+  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 6b17af63853ecb..efe32a2525ca69 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)_ 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)_ 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 d18a29ce4d451b..986e9f1a1abc4b 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 # deprecated
+      expose :emails_enabled
       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 f7802938d8b415..fbe13bfe8f741d 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/features/groups/show_spec.rb b/spec/features/groups/show_spec.rb
index 8450322945c24c..cf18f3cb4e58f2 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 ac59a42d813a07..5720f40ccff30e 100644
--- a/spec/finders/user_group_notification_settings_finder_spec.rb
+++ b/spec/finders/user_group_notification_settings_finder_spec.rb
@@ -136,10 +136,10 @@ def attributes(&proc)
     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(: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(: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]) }
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index 0db15541b99e75..f5cadfc2761d15 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 0ae4b35af7a9c5..37b4a9011f4783 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 e9822d974477d7..c7449e047b0200 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 9974aac3c6ccaa..a460e47a0e5e33 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 2275bea4c7fbed..f19c0a68f871de 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 719e51018ac147..8ad232b7e0c58c 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
-- 
GitLab


From b4e1b8b0bdf06bae630cf306aa4e82b18fab4676 Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Fri, 8 Sep 2023 12:00:59 -0400
Subject: [PATCH 2/5] Remove preloading and memoization for emails_disabled

Following the suggestions here

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127899#note_1549836926
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127899#note_1567730433

Apply the suggested changes
---
 ...user_group_notification_settings_finder.rb | 20 +++++---
 app/models/group.rb                           |  6 +--
 app/models/namespace.rb                       | 10 ++--
 lib/api/entities/group.rb                     |  4 +-
 .../profiles/notifications_controller_spec.rb |  2 +-
 spec/features/groups/group_settings_spec.rb   |  2 +-
 ...group_notification_settings_finder_spec.rb | 50 +++++++++----------
 .../notification_service_shared_examples.rb   |  2 +-
 8 files changed, 52 insertions(+), 44 deletions(-)

diff --git a/app/finders/user_group_notification_settings_finder.rb b/app/finders/user_group_notification_settings_finder.rb
index c6a1a6b36d1a44..c267ee35ce38cc 100644
--- a/app/finders/user_group_notification_settings_finder.rb
+++ b/app/finders/user_group_notification_settings_finder.rb
@@ -11,11 +11,19 @@ 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 +53,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 8a43a40cfb7f5b..492f419593180a 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -300,15 +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
         .joins(:namespace_settings)
-        .where('namespace_settings.emails_enabled IS FALSE')
+        .where(namespace_settings: { emails_enabled: false })
         .select('1')
         .limit(1)
 
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 8181792e3ca48f..5abda6196c1f60 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -206,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)
@@ -384,16 +384,16 @@ def find_fork_of(project)
 
   # any ancestor can disable emails for all descendants
   def emails_disabled?
-    strong_memoize(:emails_disabled_memoized) do
-      !emails_enabled?
-    end
+    !emails_enabled?
   end
 
   def emails_enabled?
     # If no namespace_settings, we can assume it has not changed from enabled
     return true unless namespace_settings
 
-    namespace_settings.emails_enabled?
+    strong_memoize(:emails_enabled_memoized) do
+      namespace_settings.emails_enabled?
+    end
   end
 
   def lfs_enabled?
diff --git a/lib/api/entities/group.rb b/lib/api/entities/group.rb
index 986e9f1a1abc4b..fe16edc79d69dc 100644
--- a/lib/api/entities/group.rb
+++ b/lib/api/entities/group.rb
@@ -10,8 +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 # deprecated
-      expose :emails_enabled
+      expose(:emails_disabled, documentation: { type: 'boolean' }) { |project, options| project.emails_disabled? }
+      expose :emails_enabled, documentation: { type: 'boolean' }
       expose :mentions_disabled
       expose :lfs_enabled?, as: :lfs_enabled
       expose :default_branch_protection
diff --git a/spec/controllers/profiles/notifications_controller_spec.rb b/spec/controllers/profiles/notifications_controller_spec.rb
index 22c0a62a6a1495..ef49eb911ba720 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 a248a2b471a228..0437e5df6e9e0f 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/finders/user_group_notification_settings_finder_spec.rb b/spec/finders/user_group_notification_settings_finder_spec.rb
index 5720f40ccff30e..83d0d343c04450 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_enabled: false, 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_enabled: false) }
-    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/support/shared_examples/services/notification_service_shared_examples.rb b/spec/support/shared_examples/services/notification_service_shared_examples.rb
index df1ae67a590da5..c53872ca4bc402 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
-- 
GitLab


From 86ec1b38058f9721b7c6dd9dc0a4784f173eaa5f Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Fri, 22 Sep 2023 11:10:24 -0400
Subject: [PATCH 3/5] Enhance deprecations messages

Per review comments, add a link to the MR when marking the
emails_disabled entry as deprecated.
---
 doc/api/groups.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/api/groups.md b/doc/api/groups.md
index efe32a2525ca69..879e0626305e98 100644
--- a/doc/api/groups.md
+++ b/doc/api/groups.md
@@ -824,7 +824,7 @@ 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       | _(Deprecated)_ Disable email notifications. Use `emails_enabled` instead |
+| `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. |
@@ -982,7 +982,7 @@ 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       | _(Deprecated)_ Disable email notifications. Use `emails_enabled` instead |
+| `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. |
-- 
GitLab


From d1166b247fb09cf1888feeb4bb7741a39c8403fb Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Fri, 22 Sep 2023 11:37:57 -0400
Subject: [PATCH 4/5] Additional test updates for Group API entity fix

Update the places in groups_spec.rb where to account for the entity
properly sending the values from the database.
---
 spec/requests/api/groups_spec.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb
index 662e11f7cfbfe4..ee1f3deab5033b 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")
-- 
GitLab


From 138c0cb3e48f9e9e8a6550f77c71f6246b21b76d Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Mon, 16 Oct 2023 08:33:40 -0400
Subject: [PATCH 5/5] Additional review fixes

Fix copy/paste error where a variable in a group file was named project.
Streamline the creation of the preloading for notifications
Remove an unneeded "!" when checking for ancestors by using `none?`
---
 app/finders/user_group_notification_settings_finder.rb | 5 +----
 app/models/namespace_setting.rb                        | 2 +-
 lib/api/entities/group.rb                              | 2 +-
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/app/finders/user_group_notification_settings_finder.rb b/app/finders/user_group_notification_settings_finder.rb
index c267ee35ce38cc..8d06d3d18caebc 100644
--- a/app/finders/user_group_notification_settings_finder.rb
+++ b/app/finders/user_group_notification_settings_finder.rb
@@ -18,10 +18,7 @@ def execute
     end
 
     group_sources = group_notifications.map(&:source)
-    ActiveRecord::Associations::Preloader.new(
-      records: group_sources,
-      associations: { namespace_settings: [] }
-    ).call
+    ActiveRecord::Associations::Preloader.new(records: group_sources, associations: :namespace_settings).call
 
     group_notifications
   end
diff --git a/app/models/namespace_setting.rb b/app/models/namespace_setting.rb
index 675ae54831d691..13d2c5a62e2be0 100644
--- a/app/models/namespace_setting.rb
+++ b/app/models/namespace_setting.rb
@@ -96,7 +96,7 @@ def allow_runner_registration_token?
   private
 
   def all_ancestors_have_emails_enabled?
-    !self.class.where(namespace_id: namespace.self_and_ancestors, emails_enabled: false).exists?
+    self.class.where(namespace_id: namespace.self_and_ancestors, emails_enabled: false).none?
   end
 
   def all_ancestors_allow_diff_preview_in_email?
diff --git a/lib/api/entities/group.rb b/lib/api/entities/group.rb
index fe16edc79d69dc..1a1765c2e0a96d 100644
--- a/lib/api/entities/group.rb
+++ b/lib/api/entities/group.rb
@@ -10,7 +10,7 @@ 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, documentation: { type: 'boolean' }) { |project, options| project.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
-- 
GitLab