From b638568a3764e178c1c5e9720c2421a648efb89b Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Fri, 9 Sep 2022 08:30:19 -0400
Subject: [PATCH 01/16] Utilize show_diff_preview_in_email column

Add the front-end updates needed to alter and utilize the
show_diff_preview_in_email column.

This should be used to hide the diff content found in the _note_email
templates.  Replace it with a string text informing the user that the
GitLab instance should be visited to see the line in question.

Changelog: added
---
 .../permissions/components/settings_panel.vue | 26 ++++++++++++
 app/controllers/projects_controller.rb        |  1 +
 app/helpers/groups_helper.rb                  |  6 +++
 app/helpers/projects_helper.rb                |  8 ++++
 .../groups/settings/_permissions.html.haml    | 18 ++++++---
 app/views/notify/_note_email.html.haml        |  6 ++-
 app/views/notify/_note_email.text.erb         | 11 +++--
 lib/api/helpers/groups_helpers.rb             |  1 +
 locale/gitlab.pot                             | 18 +++++++++
 spec/helpers/groups_helper_spec.rb            | 40 +++++++++++++++++++
 spec/helpers/projects_helper_spec.rb          | 25 ++++++++++++
 11 files changed, 150 insertions(+), 10 deletions(-)

diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
index 1dc33628e2a655..4ec0de699d5f53 100644
--- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
+++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
@@ -125,6 +125,11 @@ export default {
       required: false,
       default: false,
     },
+    canEnableDiffPreview: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
     canChangeVisibilityLevel: {
       type: Boolean,
       required: false,
@@ -282,6 +287,7 @@ export default {
       enforceAuthChecksOnUploads: true,
       highlightChangesClass: false,
       emailsEnabled: true,
+      showDiffPreviewInEmail: true,
       cveIdRequestEnabled: true,
       featureAccessLevelEveryone,
       featureAccessLevelMembers,
@@ -382,6 +388,9 @@ export default {
     monitorOperationsFeatureAccessLevelOptions() {
       return this.featureAccessLevelOptions.filter(([value]) => value <= this.monitorAccessLevel);
     },
+    findDiffPreviewValue() {
+      return !this.emailsDisabled && this.showDiffPreviewInEmail;
+    },
   },
 
   watch: {
@@ -1021,6 +1030,8 @@ export default {
         />
       </project-setting-row>
     </div>
+
+    <b>{{ s__('ProjectSettings|Email notifications') }} </b>
     <project-setting-row v-if="canDisableEmails" ref="email-settings" class="mb-3">
       <label class="js-emails-enabled">
         <input
@@ -1036,6 +1047,21 @@ export default {
         </gl-form-checkbox>
       </label>
     </project-setting-row>
+    <project-setting-row v-if="canEnableDiffPreview" class="mb-3 gl-px-7">
+      <input
+        :value="showDiffPreviewInEmail"
+        type="hidden"
+        name="project[project_setting_attributes][show_diff_preview_in_email]"
+      />
+      <gl-form-checkbox v-model="showDiffPreviewInEmail" :disabled="emailsDisabled">
+        {{ s__('ProjectSettings|Include diff previews') }}
+        <template #help>{{
+          s__(
+            'ProjectSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews.',
+          )
+        }}</template>
+      </gl-form-checkbox>
+    </project-setting-row>
     <project-setting-row class="mb-3">
       <input
         :value="showDefaultAwardEmojis"
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 1d02dad5abe6ce..772cafb0bb9196 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -469,6 +469,7 @@ def project_feature_attributes
   def project_setting_attributes
     %i[
       show_default_award_emojis
+      show_diff_preview_in_email
       squash_option
       mr_default_target_self
       warn_about_potentially_unwanted_characters
diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index dcb5e98da8f091..b9e53ae184593b 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -21,6 +21,12 @@ def can_disable_group_emails?(group)
     can?(current_user, :set_emails_disabled, group) && !group.parent&.emails_disabled?
   end
 
+  def can_set_group_diff_preview_in_email?(group)
+    return false unless group.parent&.show_diff_preview_in_email? || group.parent&.show_diff_preview_in_email?.nil?
+
+    can?(current_user, :set_show_diff_preview_in_email, group)
+  end
+
   def can_admin_group_member?(group)
     Ability.allowed?(current_user, :admin_group_member, group)
   end
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 9adc954de87ba9..2cfa710380f8c5 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -194,6 +194,12 @@ def can_disable_emails?(project, current_user)
     can?(current_user, :set_emails_disabled, project)
   end
 
+  def can_set_diff_preview_in_email?(project, current_user)
+    return false unless project.group&.show_diff_preview_in_email? || project.group&.show_diff_preview_in_email?.nil?
+
+    can?(current_user, :set_show_diff_preview_in_email, project)
+  end
+
   def last_push_event
     current_user&.recent_push(@project)
   end
@@ -387,6 +393,7 @@ def project_permissions_panel_data(project)
       packagesHelpPath: help_page_path('user/packages/index'),
       currentSettings: project_permissions_settings(project),
       canAddCatalogResource: can_add_catalog_resource?(project),
+      canEnableDiffPreview: can_set_diff_preview_in_email?(project, current_user),
       canChangeVisibilityLevel: can_change_visibility_level?(project, current_user),
       canDisableEmails: can_disable_emails?(project, current_user),
       allowedVisibilityOptions: project_allowed_visibility_levels(project),
@@ -767,6 +774,7 @@ def project_permissions_settings(project)
       containerRegistryEnabled: !!project.container_registry_enabled,
       lfsEnabled: !!project.lfs_enabled,
       emailsEnabled: project.emails_enabled?,
+      showDiffPreviewInEmail: project.show_diff_preview_in_email?,
       monitorAccessLevel: feature.monitor_access_level,
       showDefaultAwardEmojis: project.show_default_award_emojis?,
       warnAboutPotentiallyUnwantedCharacters: project.warn_about_potentially_unwanted_characters?,
diff --git a/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml
index 804eb4f6554c89..656679e6ca1b02 100644
--- a/app/views/groups/settings/_permissions.html.haml
+++ b/app/views/groups/settings/_permissions.html.haml
@@ -17,17 +17,25 @@
           checkbox_options: { disabled: !can_change_share_with_group_lock?(@group) },
           help_text: share_with_group_lock_help_text(@group)
 
+    .form-group.gl-mb-3
+      = f.gitlab_ui_checkbox_component :mentions_disabled,
+          s_('GroupSettings|Group mentions are disabled'),
+          checkbox_options: { checked: @group.mentions_disabled? },
+          help_text: s_('GroupSettings|Group members are not notified if the group is mentioned.')
+
+    %h5= _('Email notifications')
     .form-group.gl-mb-3
       = f.gitlab_ui_checkbox_component :emails_enabled,
           s_('GroupSettings|Enable email notifications'),
           checkbox_options: { checked: @group.emails_enabled?, disabled: !can_disable_group_emails?(@group) },
           help_text: s_('GroupSettings|Enable sending email notifications for this group and all its subgroups and projects')
 
-    .form-group.gl-mb-3
-      = f.gitlab_ui_checkbox_component :mentions_disabled,
-          s_('GroupSettings|Group mentions are disabled'),
-          checkbox_options: { checked: @group.mentions_disabled? },
-          help_text: s_('GroupSettings|Group members are not notified if the group is mentioned.')
+    .form-group.gl-mb-3.gl-px-7
+      = f.gitlab_ui_checkbox_component :show_diff_preview_in_email,
+          s_('GroupSettings|Include diff previews'),
+          checkbox_options: { checked: @group.show_diff_preview_in_email?,
+                              disabled: !@group.emails_enabled? },
+          help_text: s_('GroupSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews.')
 
     = render 'groups/settings/resource_access_token_creation', f: f, group: @group
     = render 'groups/settings/ip_restriction_registration_features_cta', f: f
diff --git a/app/views/notify/_note_email.html.haml b/app/views/notify/_note_email.html.haml
index 2f0e62981eced4..e065270048ecd5 100644
--- a/app/views/notify/_note_email.html.haml
+++ b/app/views/notify/_note_email.html.haml
@@ -6,6 +6,7 @@
 
 - author = local_assigns.fetch(:author) { note.author }
 - discussion = local_assigns.fetch(:discussion) { note.discussion } if note.part_of_discussion?
+- project = local_assigns.fetch(:project, @project)
 
 %p{ style: "color: #777777;" }
   = succeed ':' do
@@ -23,7 +24,7 @@
       - else
         = link_to 'discussion', target_url
 
-- if discussion&.diff_discussion? && discussion.on_text?
+- if discussion&.diff_discussion? && discussion.on_text? && project.show_diff_preview_in_email?
   - if include_stylesheet_link
     = content_for :head do
       = stylesheet_link_tag 'mailers/highlighted_diff_email'
@@ -33,6 +34,9 @@
       collection: discussion.truncated_diff_lines(diff_limit: diff_limit),
       as: :line,
       locals: { diff_file: discussion.diff_file }
+- else
+  Diff previews have been disabled for this Merge Request.  Visit GitLab for more information about this discussion
+
 
 .md{ style: note_style }
   = markdown(note.note, pipeline: :email, author: author, current_user: @recipient, issuable_reference_expansion_enabled: true)
diff --git a/app/views/notify/_note_email.text.erb b/app/views/notify/_note_email.text.erb
index b23ba464c06757..e46fbe395a4f29 100644
--- a/app/views/notify/_note_email.text.erb
+++ b/app/views/notify/_note_email.text.erb
@@ -3,6 +3,7 @@
 <% target_url = local_assigns.fetch(:target_url, @target_url) -%>
 <% author = local_assigns.fetch(:author) { note.author } -%>
 <% discussion = local_assigns.fetch(:discussion) { note.discussion } if note.part_of_discussion? -%>
+<% project = local_assigns.fetch(:project, @project) -%>
 
 <%= sanitize_name(author.name) -%>
 <%  if discussion.nil? -%>
@@ -21,10 +22,12 @@
 <%= " #{target_url}" -%>
 
 
-<%  if discussion&.diff_discussion? && discussion.on_text? -%>
+<%  if discussion&.diff_discussion? && discussion.on_text? && project.show_diff_preview_in_email? -%>
 <%    discussion.truncated_diff_lines(highlight: false, diff_limit: diff_limit).each do |line| -%>
-<%=     "> #{line.text}\n" -%>
-<%    end -%>
-
+ <%=     "> #{line.text}\n" -%>
+ <%  end -%>
+<% else -%>
+  <%= "Diff previews have been disabled for this Merge Request.  Visit GitLab for more information about this discussion" -%>
 <%  end -%>
+
 <%= note.note -%>
diff --git a/lib/api/helpers/groups_helpers.rb b/lib/api/helpers/groups_helpers.rb
index 08570f4b7031c9..ab7df044c73503 100644
--- a/lib/api/helpers/groups_helpers.rb
+++ b/lib/api/helpers/groups_helpers.rb
@@ -20,6 +20,7 @@ module GroupsHelpers
         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: '_(Deprecated)_ Disable email notifications. Use: emails_enabled'
         optional :emails_enabled, type: Boolean, desc: 'Enable email notifications'
+        optional :show_diff_preview_in_email, type: Boolean, desc: 'Include the code diff preview in merge request notification emails'
         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/locale/gitlab.pot b/locale/gitlab.pot
index 040d9c34d9b612..61ebf42ceb386a 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -19305,6 +19305,9 @@ msgstr ""
 msgid "Email notification for unknown sign-ins"
 msgstr ""
 
+msgid "Email notifications"
+msgstr ""
+
 msgid "Email patch"
 msgstr ""
 
@@ -24818,6 +24821,9 @@ msgstr ""
 msgid "GroupSettings|Enable email notifications"
 msgstr ""
 
+msgid "GroupSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews."
+msgstr ""
+
 msgid "GroupSettings|Enable overview background aggregation for Value Streams Dashboard"
 msgstr ""
 
@@ -24863,6 +24869,9 @@ msgstr ""
 msgid "GroupSettings|If the parent group's visibility is lower than the group's current visibility, visibility levels for subgroups and projects will be changed to match the new parent group's visibility."
 msgstr ""
 
+msgid "GroupSettings|Include diff previews"
+msgstr ""
+
 msgid "GroupSettings|Members cannot invite groups outside of %{group} and its subgroups"
 msgstr ""
 
@@ -40044,6 +40053,12 @@ msgstr ""
 msgid "ProjectSettings|Do not allow"
 msgstr ""
 
+msgid "ProjectSettings|Email notifications"
+msgstr ""
+
+msgid "ProjectSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews."
+msgstr ""
+
 msgid "ProjectSettings|Enable \"Delete source branch\" option by default"
 msgstr ""
 
@@ -40131,6 +40146,9 @@ msgstr ""
 msgid "ProjectSettings|If merge trains are enabled, merging is only possible if the branch can be rebased without conflicts."
 msgstr ""
 
+msgid "ProjectSettings|Include diff previews"
+msgstr ""
+
 msgid "ProjectSettings|Infrastructure"
 msgstr ""
 
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index 62795806852a16..fb72bc0603082a 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -301,6 +301,46 @@
     end
   end
 
+  describe '#can_set_group_diff_preview_in_email?' do
+    let_it_be(:current_user) { create(:user) }
+    let_it_be(:group) { create(:group, name: 'group') }
+    let_it_be(:subgroup) { create(:group, name: 'subgroup', parent: group) }
+
+    before do
+      allow(helper).to receive(:current_user) { current_user }
+    end
+
+    it 'returns true for the group owner' do
+      allow(helper).to receive(:can?).with(current_user, :set_show_diff_preview_in_email, group) { true }
+
+      expect(helper.can_set_group_diff_preview_in_email?(group)).to be_truthy
+    end
+
+    it 'returns false for anyone else' do
+      allow(helper).to receive(:can?).with(current_user, :set_show_diff_preview_in_email, group) { false }
+
+      expect(helper.can_set_group_diff_preview_in_email?(group)).to be_falsey
+    end
+
+    context 'when a group has a parent group' do
+      before do
+        allow(helper).to receive(:can?).with(current_user, :set_show_diff_preview_in_email, subgroup) { true }
+      end
+
+      it 'returns false if parent group is disabling diff previews' do
+        allow(group).to receive(:show_diff_preview_in_email?).and_return(false)
+
+        expect(helper.can_set_group_diff_preview_in_email?(subgroup)).to be_falsey
+      end
+
+      it 'returns true if parent group is not disabling diff previews' do
+        allow(group).to receive(:show_diff_preview_in_email?).and_return(true)
+
+        expect(helper.can_set_group_diff_preview_in_email?(subgroup)).to be_truthy
+      end
+    end
+  end
+
   describe '#can_update_default_branch_protection?' do
     let_it_be(:current_user) { create(:user) }
     let_it_be(:group) { create(:group) }
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index 468d28690af3ae..f3c37f5f56427e 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -118,6 +118,31 @@
     end
   end
 
+  describe '#can_set_diff_preview_in_email?' do
+    let_it_be(:user) { create(:project_member, :maintainer, user: create(:user), project: project).user }
+
+    it 'returns true for the project owner' do
+      allow(project).to receive(:show_diff_preview_in_email?).and_return(true)
+      allow(helper).to receive(:can?).with(project.owner, :set_show_diff_preview_in_email, project) { true }
+
+      expect(helper.can_set_diff_preview_in_email?(project, project.owner)).to be_truthy
+    end
+
+    it 'returns false for anyone else' do
+      allow(project).to receive(:show_diff_preview_in_email?).and_return(true)
+      allow(helper).to receive(:can?).with(user, :set_show_diff_preview_in_email, project) { false }
+
+      expect(helper.can_set_diff_preview_in_email?(project, user)).to be_falsey
+    end
+
+    it 'returns false if the parent group has diff previews disabled' do
+      project = create(:project, group: create(:group))
+      allow(project.group).to receive(:show_diff_preview_in_email?).and_return(false)
+
+      expect(helper.can_set_diff_preview_in_email?(project, project.owner)).to be_falsey
+    end
+  end
+
   describe '#load_pipeline_status' do
     it 'loads the pipeline status in batch' do
       helper.load_pipeline_status([project])
-- 
GitLab


From 42f97ea3992912764806fe390c15b1d4c52db38f Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Thu, 29 Sep 2022 08:13:30 -0400
Subject: [PATCH 02/16] Add updates from first review

* Switch from <.. || nil?> checking to use .equals(false) to clean up
  lines
* Properly group all email settings on group page
* Extract group page email settings to a partial file
* Move added header/descriptive strings to localization object for
  projects page.
* Move emails disabled message to footer of emails using a simliar if
  statment to not print  the message in every email.

Change the variable to be more descriptive about
the value it is supposed to represent.

Add a test for the string presence in the email output.

Ensure that the user can disable diffs in the webpage before allowing
the setting to be changed

Set the text line to be translated.

Remove extra line from template
---
 .../permissions/components/settings_panel.vue | 34 ++++++++-----------
 app/helpers/groups_helper.rb                  |  2 +-
 app/helpers/projects_helper.rb                |  2 +-
 .../groups/settings/_email_settings.html.haml | 19 +++++++++++
 .../groups/settings/_permissions.html.haml    | 21 +-----------
 app/views/layouts/notify.html.haml            | 10 ++++++
 app/views/layouts/notify.text.erb             | 10 ++++++
 app/views/notify/_note_email.html.haml        |  3 --
 app/views/notify/_note_email.text.erb         |  2 --
 spec/mailers/notify_spec.rb                   | 11 ++++++
 10 files changed, 68 insertions(+), 46 deletions(-)
 create mode 100644 app/views/groups/settings/_email_settings.html.haml

diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
index 4ec0de699d5f53..9ea1b62d9f2507 100644
--- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
+++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
@@ -81,6 +81,11 @@ export default {
       'ProjectSettings|Highlight the usage of hidden unicode characters. These have innocent uses for right-to-left languages, but can also be used in potential exploits.',
     ),
     confirmButtonText: __('Save changes'),
+    emailsLabel: s__('ProjectSettings|Email notifications'),
+    showDiffPreviewLabel: s__('ProjectSettings|Include diff previews'),
+    showDiffPreviewHelpText: s__(
+      'ProjectSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews.',
+    ),
   },
   VISIBILITY_LEVEL_PRIVATE_INTEGER,
   VISIBILITY_LEVEL_INTERNAL_INTEGER,
@@ -1031,21 +1036,16 @@ export default {
       </project-setting-row>
     </div>
 
-    <b>{{ s__('ProjectSettings|Email notifications') }} </b>
+    <h5>{{ $options.i18n.emailsLabel }}</h5>
     <project-setting-row v-if="canDisableEmails" ref="email-settings" class="mb-3">
-      <label class="js-emails-enabled">
-        <input
-          :value="emailsEnabled"
-          type="hidden"
-          name="project[project_setting_attributes][emails_enabled]"
-        />
-        <gl-form-checkbox v-model="emailsEnabled">
-          {{ s__('ProjectSettings|Enable email notifications') }}
-          <template #help>{{
-            s__('ProjectSettings|Enable sending email notifications for this project')
-          }}</template>
-        </gl-form-checkbox>
+      <label class="js-emails-disabled">
+        <input :value="emailsDisabled" type="hidden" name="project[emails_disabled]" />
+        <input v-model="emailsDisabled" type="checkbox" />
+        {{ s__('ProjectSettings|Disable email notifications') }}
       </label>
+      <span class="form-text text-muted">{{
+        s__('ProjectSettings|Override user notification preferences for all project members.')
+      }}</span>
     </project-setting-row>
     <project-setting-row v-if="canEnableDiffPreview" class="mb-3 gl-px-7">
       <input
@@ -1054,12 +1054,8 @@ export default {
         name="project[project_setting_attributes][show_diff_preview_in_email]"
       />
       <gl-form-checkbox v-model="showDiffPreviewInEmail" :disabled="emailsDisabled">
-        {{ s__('ProjectSettings|Include diff previews') }}
-        <template #help>{{
-          s__(
-            'ProjectSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews.',
-          )
-        }}</template>
+        {{ $options.i18n.showDiffPreviewLabel }}
+        <template #help>{{ $options.i18n.showDiffPreviewHelpText }}</template>
       </gl-form-checkbox>
     </project-setting-row>
     <project-setting-row class="mb-3">
diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index b9e53ae184593b..337f8e7f1ba487 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -22,7 +22,7 @@ def can_disable_group_emails?(group)
   end
 
   def can_set_group_diff_preview_in_email?(group)
-    return false unless group.parent&.show_diff_preview_in_email? || group.parent&.show_diff_preview_in_email?.nil?
+    return false if group.parent&.show_diff_preview_in_email?.equal?(false)
 
     can?(current_user, :set_show_diff_preview_in_email, group)
   end
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 2cfa710380f8c5..77f7341ef5789a 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -195,7 +195,7 @@ def can_disable_emails?(project, current_user)
   end
 
   def can_set_diff_preview_in_email?(project, current_user)
-    return false unless project.group&.show_diff_preview_in_email? || project.group&.show_diff_preview_in_email?.nil?
+    return false if project.group&.show_diff_preview_in_email?.equal?(false)
 
     can?(current_user, :set_show_diff_preview_in_email, project)
   end
diff --git a/app/views/groups/settings/_email_settings.html.haml b/app/views/groups/settings/_email_settings.html.haml
new file mode 100644
index 00000000000000..fb5a8f8b1022fb
--- /dev/null
+++ b/app/views/groups/settings/_email_settings.html.haml
@@ -0,0 +1,19 @@
+%h5= _('Email notifications')
+.form-group.gl-mb-3
+  = f.gitlab_ui_checkbox_component :mentions_disabled,
+      s_('GroupSettings|Group mentions are disabled'),
+      checkbox_options: { checked: @group.mentions_disabled? },
+      help_text: s_('GroupSettings|Group members are not notified if the group is mentioned.')
+
+.form-group.gl-mb-3
+  = f.gitlab_ui_checkbox_component :emails_enabled,
+      s_('GroupSettings|Enable email notifications'),
+      checkbox_options: { checked: @group.emails_enabled?, disabled: !can_disable_group_emails?(@group) },
+      help_text: s_('GroupSettings|Enable sending email notifications for this group and all its subgroups and projects')
+
+.form-group.gl-mb-3.gl-px-7
+  = f.gitlab_ui_checkbox_component :show_diff_preview_in_email,
+      s_('GroupSettings|Include diff previews'),
+      checkbox_options: { checked: @group.show_diff_preview_in_email?,
+                          disabled: !@group.emails_enabled? || !can_set_group_diff_preview_in_email?(@group) },
+      help_text: s_('GroupSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews.')
diff --git a/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml
index 656679e6ca1b02..10960bfafdbff9 100644
--- a/app/views/groups/settings/_permissions.html.haml
+++ b/app/views/groups/settings/_permissions.html.haml
@@ -17,27 +17,8 @@
           checkbox_options: { disabled: !can_change_share_with_group_lock?(@group) },
           help_text: share_with_group_lock_help_text(@group)
 
-    .form-group.gl-mb-3
-      = f.gitlab_ui_checkbox_component :mentions_disabled,
-          s_('GroupSettings|Group mentions are disabled'),
-          checkbox_options: { checked: @group.mentions_disabled? },
-          help_text: s_('GroupSettings|Group members are not notified if the group is mentioned.')
-
-    %h5= _('Email notifications')
-    .form-group.gl-mb-3
-      = f.gitlab_ui_checkbox_component :emails_enabled,
-          s_('GroupSettings|Enable email notifications'),
-          checkbox_options: { checked: @group.emails_enabled?, disabled: !can_disable_group_emails?(@group) },
-          help_text: s_('GroupSettings|Enable sending email notifications for this group and all its subgroups and projects')
-
-    .form-group.gl-mb-3.gl-px-7
-      = f.gitlab_ui_checkbox_component :show_diff_preview_in_email,
-          s_('GroupSettings|Include diff previews'),
-          checkbox_options: { checked: @group.show_diff_preview_in_email?,
-                              disabled: !@group.emails_enabled? },
-          help_text: s_('GroupSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews.')
-
     = render 'groups/settings/resource_access_token_creation', f: f, group: @group
+    = render 'groups/settings/email_settings', f: f, group: @group
     = render 'groups/settings/ip_restriction_registration_features_cta', f: f
     = render_if_exists 'groups/settings/ip_restriction', f: f, group: @group
     = render_if_exists 'groups/settings/allowed_email_domain', f: f, group: @group
diff --git a/app/views/layouts/notify.html.haml b/app/views/layouts/notify.html.haml
index 1f526ec221d4a6..2038533dfe045b 100644
--- a/app/views/layouts/notify.html.haml
+++ b/app/views/layouts/notify.html.haml
@@ -1,3 +1,10 @@
+- note = local_assigns.fetch(:note, @note)
+- show_no_diff_preview_message = true
+- unless note.nil?
+  - discussion = local_assigns.fetch(:discussion) { note.discussion } if note.part_of_discussion?
+  - project = local_assigns.fetch(:project, @project)
+  - show_no_diff_preview_message = discussion&.diff_discussion? && discussion.on_text? && !project.show_diff_preview_in_email?
+
 %html{ lang: I18n.locale }
   %head
     %meta{ content: "text/html; charset=utf-8", "http-equiv" => "Content-Type" }
@@ -20,6 +27,9 @@
       %p
         &mdash;
         %br
+        - if show_no_diff_preview_message
+          _('This project does not include diff previews in email notifications.')
+          %br
         - if @target_url
           - if @reply_by_email
             = _('Reply to this email directly or %{view_it_on_gitlab}.').html_safe % { view_it_on_gitlab: link_to(_("view it on GitLab"), @target_url) }
diff --git a/app/views/layouts/notify.text.erb b/app/views/layouts/notify.text.erb
index 4eae96dc3767a1..f359b33cda6792 100644
--- a/app/views/layouts/notify.text.erb
+++ b/app/views/layouts/notify.text.erb
@@ -1,8 +1,18 @@
+<% note = local_assigns.fetch(:note, @note) -%>
+<% show_no_diff_preview_message = true -%>
+<% unless @note.nil? -%>
+<%   discussion = local_assigns.fetch(:discussion) { note.discussion } if note.part_of_discussion? -%>
+<%   project = local_assigns.fetch(:project, @project) -%>
+<%   show_no_diff_preview_message = discussion&.diff_discussion? && discussion.on_text? && !project.show_diff_preview_in_email? -%>
+<% end -%>
 <%= text_header_message %>
 
 <%= yield -%>
 
 -- <%# signature marker %>
+<%  if show_no_diff_preview_message -%>
+<%=   "This project does not include diff previews in email notifications.\n" -%>
+<%  end -%>
 <%  if @target_url -%>
 <%    if @reply_by_email -%>
 <%=     "Reply to this email directly or view it on GitLab: #{@target_url}" -%>
diff --git a/app/views/notify/_note_email.html.haml b/app/views/notify/_note_email.html.haml
index e065270048ecd5..ff5b2c5b97852f 100644
--- a/app/views/notify/_note_email.html.haml
+++ b/app/views/notify/_note_email.html.haml
@@ -34,9 +34,6 @@
       collection: discussion.truncated_diff_lines(diff_limit: diff_limit),
       as: :line,
       locals: { diff_file: discussion.diff_file }
-- else
-  Diff previews have been disabled for this Merge Request.  Visit GitLab for more information about this discussion
-
 
 .md{ style: note_style }
   = markdown(note.note, pipeline: :email, author: author, current_user: @recipient, issuable_reference_expansion_enabled: true)
diff --git a/app/views/notify/_note_email.text.erb b/app/views/notify/_note_email.text.erb
index e46fbe395a4f29..0a4f862756b6e4 100644
--- a/app/views/notify/_note_email.text.erb
+++ b/app/views/notify/_note_email.text.erb
@@ -26,8 +26,6 @@
 <%    discussion.truncated_diff_lines(highlight: false, diff_limit: diff_limit).each do |line| -%>
  <%=     "> #{line.text}\n" -%>
  <%  end -%>
-<% else -%>
-  <%= "Diff previews have been disabled for this Merge Request.  Visit GitLab for more information about this discussion" -%>
 <%  end -%>
 
 <%= note.note -%>
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 618717a61df6bf..9d98cc55ff7335 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -1476,6 +1476,17 @@ def create_note
           end
         end
 
+        context 'when the project does not show diffs in emails' do
+          before do
+            allow(project).to receive(:show_diff_preview_in_email?).and_return(false)
+          end
+
+          it "does not show diff and displays a separate message" do
+            is_expected.to have_body_text 'This project does not include diff previews in email notifications'
+            is_expected.not_to have_body_text '<span class="p">}</span></span>'
+          end
+        end
+
         it 'includes diffs with character-level highlighting' do
           is_expected.to have_body_text '<span class="p">}</span></span>'
         end
-- 
GitLab


From a153df9519dd026169fb022701583ea3c102fd68 Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Thu, 27 Oct 2022 08:44:49 -0400
Subject: [PATCH 03/16] Update setup for test

Move the place where the note object is created for each test of the
relevant notify_spec file.

Changes suggested by @dskim_gitlab here:

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/98547#note_1150466952
---
 spec/mailers/notify_spec.rb | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 9d98cc55ff7335..ff334f19bd1d36 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -1464,8 +1464,6 @@ def create_note
       end
 
       shared_examples 'an email for a note on a diff discussion' do |model|
-        let(:note) { create(model, author: note_author) }
-
         context 'when note is not on text' do
           before do
             allow(note.discussion).to receive(:on_text?).and_return(false)
@@ -1524,7 +1522,7 @@ def create_note
 
       describe 'on a commit' do
         let(:commit) { project.commit }
-        let(:note) { create(:diff_note_on_commit) }
+        let(:note) { create(:diff_note_on_commit, author: note_author, project: project) }
 
         subject { described_class.note_commit_email(recipient.id, note.id) }
 
@@ -1536,7 +1534,7 @@ def create_note
       end
 
       describe 'on a merge request' do
-        let(:note) { create(:diff_note_on_merge_request) }
+        let(:note) { create(:diff_note_on_merge_request, author: note_author, noteable: merge_request, project: project) }
 
         subject { described_class.note_merge_request_email(recipient.id, note.id) }
 
-- 
GitLab


From b72ec95d5176e624cb70e110cb2a8e34b98cfeae Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Mon, 31 Oct 2022 09:54:51 -0400
Subject: [PATCH 04/16] Updates to match design

Ensure that the new parts of the pages match the design in
https://gitlab.com/gitlab-org/gitlab/-/issues/24733

Revert location of Group Mentions from an email setting back to
it's original location in Permissions.

Fix the text on the projects page which referenced "groups"
Switch the emails_disabled object to use the gl-checkbox to have
consistent styling for both sets of email notification settings.

Eliminate spacing between checkboxes by adding them to the
same project row.
---
 .../permissions/components/settings_panel.vue | 45 +++++++++++--------
 .../groups/settings/_email_settings.html.haml | 16 +++----
 .../groups/settings/_permissions.html.haml    |  6 +++
 locale/gitlab.pot                             |  4 +-
 4 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
index 9ea1b62d9f2507..36f507053dcabe 100644
--- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
+++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
@@ -84,7 +84,7 @@ export default {
     emailsLabel: s__('ProjectSettings|Email notifications'),
     showDiffPreviewLabel: s__('ProjectSettings|Include diff previews'),
     showDiffPreviewHelpText: s__(
-      'ProjectSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews.',
+      'ProjectSettings|Emails are not encrypted, concerned administrators may want to disable diff previews.',
     ),
   },
   VISIBILITY_LEVEL_PRIVATE_INTEGER,
@@ -393,8 +393,13 @@ export default {
     monitorOperationsFeatureAccessLevelOptions() {
       return this.featureAccessLevelOptions.filter(([value]) => value <= this.monitorAccessLevel);
     },
-    findDiffPreviewValue() {
-      return !this.emailsDisabled && this.showDiffPreviewInEmail;
+    findDiffPreviewValue: {
+      get() {
+        return !this.emailsDisabled && this.showDiffPreviewInEmail;
+      },
+      set(newValue) {
+        this.showDiffPreviewInEmail = newValue;
+      },
     },
   },
 
@@ -1038,25 +1043,29 @@ export default {
 
     <h5>{{ $options.i18n.emailsLabel }}</h5>
     <project-setting-row v-if="canDisableEmails" ref="email-settings" class="mb-3">
-      <label class="js-emails-disabled">
-        <input :value="emailsDisabled" type="hidden" name="project[emails_disabled]" />
-        <input v-model="emailsDisabled" type="checkbox" />
-        {{ s__('ProjectSettings|Disable email notifications') }}
-      </label>
-      <span class="form-text text-muted">{{
-        s__('ProjectSettings|Override user notification preferences for all project members.')
-      }}</span>
-    </project-setting-row>
-    <project-setting-row v-if="canEnableDiffPreview" class="mb-3 gl-px-7">
       <input
-        :value="showDiffPreviewInEmail"
+        class="js-emails-disabled"
+        :value="emailsDisabled"
         type="hidden"
-        name="project[project_setting_attributes][show_diff_preview_in_email]"
+        name="project[emails_disabled]"
       />
-      <gl-form-checkbox v-model="showDiffPreviewInEmail" :disabled="emailsDisabled">
-        {{ $options.i18n.showDiffPreviewLabel }}
-        <template #help>{{ $options.i18n.showDiffPreviewHelpText }}</template>
+      <gl-form-checkbox v-model="emailsDisabled">
+        {{ s__('ProjectSettings|Disable email notifications') }}
+        <template #help>{{
+          s__('ProjectSettings|Override user notification preferences for all project members.')
+        }}</template>
       </gl-form-checkbox>
+      <div v-if="canEnableDiffPreview" class="gl-px-7">
+        <input
+          :value="findDiffPreviewValue"
+          type="hidden"
+          name="project[project_setting_attributes][show_diff_preview_in_email]"
+        />
+        <gl-form-checkbox v-model="findDiffPreviewValue" :disabled="emailsDisabled">
+          {{ $options.i18n.showDiffPreviewLabel }}
+          <template #help>{{ $options.i18n.showDiffPreviewHelpText }}</template>
+        </gl-form-checkbox>
+      </div>
     </project-setting-row>
     <project-setting-row class="mb-3">
       <input
diff --git a/app/views/groups/settings/_email_settings.html.haml b/app/views/groups/settings/_email_settings.html.haml
index fb5a8f8b1022fb..9802582f674776 100644
--- a/app/views/groups/settings/_email_settings.html.haml
+++ b/app/views/groups/settings/_email_settings.html.haml
@@ -1,19 +1,13 @@
 %h5= _('Email notifications')
-.form-group.gl-mb-3
-  = f.gitlab_ui_checkbox_component :mentions_disabled,
-      s_('GroupSettings|Group mentions are disabled'),
-      checkbox_options: { checked: @group.mentions_disabled? },
-      help_text: s_('GroupSettings|Group members are not notified if the group is mentioned.')
 
 .form-group.gl-mb-3
   = f.gitlab_ui_checkbox_component :emails_enabled,
       s_('GroupSettings|Enable email notifications'),
       checkbox_options: { checked: @group.emails_enabled?, disabled: !can_disable_group_emails?(@group) },
       help_text: s_('GroupSettings|Enable sending email notifications for this group and all its subgroups and projects')
-
-.form-group.gl-mb-3.gl-px-7
-  = f.gitlab_ui_checkbox_component :show_diff_preview_in_email,
-      s_('GroupSettings|Include diff previews'),
-      checkbox_options: { checked: @group.show_diff_preview_in_email?,
+  .gl-px-7
+    = f.gitlab_ui_checkbox_component :show_diff_preview_in_email,
+        s_('GroupSettings|Include diff previews'),
+        checkbox_options: { checked: @group.show_diff_preview_in_email?,
                           disabled: !@group.emails_enabled? || !can_set_group_diff_preview_in_email?(@group) },
-      help_text: s_('GroupSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews.')
+        help_text: s_('GroupSettings|Emails are not encrypted, concerned administrators may want to disable diff previews.')
diff --git a/app/views/groups/settings/_permissions.html.haml b/app/views/groups/settings/_permissions.html.haml
index 10960bfafdbff9..66bd594adc71bc 100644
--- a/app/views/groups/settings/_permissions.html.haml
+++ b/app/views/groups/settings/_permissions.html.haml
@@ -17,6 +17,12 @@
           checkbox_options: { disabled: !can_change_share_with_group_lock?(@group) },
           help_text: share_with_group_lock_help_text(@group)
 
+    .form-group.gl-mb-3
+      = f.gitlab_ui_checkbox_component :mentions_disabled,
+          s_('GroupSettings|Group mentions are disabled'),
+          checkbox_options: { checked: @group.mentions_disabled? },
+          help_text: s_('GroupSettings|Group members are not notified if the group is mentioned.')
+
     = render 'groups/settings/resource_access_token_creation', f: f, group: @group
     = render 'groups/settings/email_settings', f: f, group: @group
     = render 'groups/settings/ip_restriction_registration_features_cta', f: f
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 61ebf42ceb386a..72306a3ab98764 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -24821,7 +24821,7 @@ msgstr ""
 msgid "GroupSettings|Enable email notifications"
 msgstr ""
 
-msgid "GroupSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews."
+msgid "GroupSettings|Emails are not encrypted, concerned administrators may want to disable diff previews."
 msgstr ""
 
 msgid "GroupSettings|Enable overview background aggregation for Value Streams Dashboard"
@@ -40056,7 +40056,7 @@ msgstr ""
 msgid "ProjectSettings|Email notifications"
 msgstr ""
 
-msgid "ProjectSettings|Emails are not encrypted, so groups with high security needs may want to disable diff previews."
+msgid "ProjectSettings|Emails are not encrypted, concerned administrators may want to disable diff previews."
 msgstr ""
 
 msgid "ProjectSettings|Enable \"Delete source branch\" option by default"
-- 
GitLab


From a5b29cbdcb221d7d9badeddbeac4af4105af99e3 Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Fri, 4 Nov 2022 10:34:21 -0400
Subject: [PATCH 05/16] Add jest testing for front-end components

Mirror the other tests for the Diff in Email setting to ensure that
the setting is properly hidden.

TODO: Check the workflow in testing also.
---
 .../permissions/components/settings_panel.vue |  8 +++--
 .../components/settings_panel_spec.js         | 36 ++++++++++++++++++-
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
index 36f507053dcabe..9f6f3dadef446e 100644
--- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
+++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
@@ -1055,7 +1055,11 @@ export default {
           s__('ProjectSettings|Override user notification preferences for all project members.')
         }}</template>
       </gl-form-checkbox>
-      <div v-if="canEnableDiffPreview" class="gl-px-7">
+      <project-setting-row
+        v-if="canEnableDiffPreview"
+        ref="enable-diff-preview-settings"
+        class="gl-px-7"
+      >
         <input
           :value="findDiffPreviewValue"
           type="hidden"
@@ -1065,7 +1069,7 @@ export default {
           {{ $options.i18n.showDiffPreviewLabel }}
           <template #help>{{ $options.i18n.showDiffPreviewHelpText }}</template>
         </gl-form-checkbox>
-      </div>
+      </project-setting-row>
     </project-setting-row>
     <project-setting-row class="mb-3">
       <input
diff --git a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
index 207ce8c1ffa6f1..635a38cda40332 100644
--- a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
+++ b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
@@ -1,4 +1,4 @@
-import { GlSprintf, GlToggle } from '@gitlab/ui';
+import { GlSprintf, GlToggle, GlFormCheckbox } from '@gitlab/ui';
 import { shallowMount, mount } from '@vue/test-utils';
 import ProjectFeatureSetting from '~/pages/projects/shared/permissions/components/project_feature_setting.vue';
 import CiCatalogSettings from '~/pages/projects/shared/permissions/components/ci_catalog_settings.vue';
@@ -122,6 +122,8 @@ describe('Settings Panel', () => {
     wrapper.find('[name="project[project_feature_attributes][pages_access_level]"]');
   const findCiCatalogSettings = () => wrapper.findComponent(CiCatalogSettings);
   const findEmailSettings = () => wrapper.findComponent({ ref: 'email-settings' });
+  const findShowDiffPreviewSetting = () =>
+    wrapper.findComponent({ ref: 'enable-diff-preview-settings' });
   const findShowDefaultAwardEmojis = () =>
     wrapper.find('input[name="project[project_setting_attributes][show_default_award_emojis]"]');
   const findWarnAboutPuc = () =>
@@ -686,6 +688,38 @@ describe('Settings Panel', () => {
     });
   });
 
+  describe('Show Diff Preview in Email', () => {
+    it('should show the diff preview toggle if diff previews can be disabled', () => {
+      wrapper = mountComponent({ canDisableEmails: true, canEnableDiffPreview: true });
+
+      expect(findShowDiffPreviewSetting().exists()).toBe(true);
+    });
+
+    it('should hide the diff preview toggle if diff previews cannot be disabled', () => {
+      wrapper = mountComponent({ canDisableEmails: false, canEnableDiffPreview: true });
+
+      expect(findShowDiffPreviewSetting().exists()).toBe(false);
+    });
+
+    it('Updates the hidden value when toggled', async () => {
+      wrapper = mountComponent({
+        canDisableEmails: true,
+        canEnableDiffPreview: true,
+        emailsDisabled: false,
+      });
+      const originalHiddenInputValue = findShowDiffPreviewSetting()
+        .find('input[type="hidden"]')
+        .attributes('value');
+      await findShowDiffPreviewSetting()
+        .findComponent(GlFormCheckbox)
+        .find('input')
+        .setChecked(false);
+      expect(
+        findShowDiffPreviewSetting().find('input[type="hidden"]').attributes('value'),
+      ).not.toEqual(originalHiddenInputValue);
+    });
+  });
+
   describe('Default award emojis', () => {
     it('should show the "Show default emoji reactions" input', () => {
       wrapper = mountComponent();
-- 
GitLab


From 425989c9e6ec538ce98aae12b6c545d329ce7b74 Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Thu, 10 Nov 2022 08:21:56 -0500
Subject: [PATCH 06/16] Add feature flag to hide setting

Add a development feature flag with which to hide the setting
on the groups and projects page.
---
 app/helpers/groups_helper.rb                              | 1 +
 app/helpers/projects_helper.rb                            | 1 +
 .../gitlab_com_derisk/diff_preview_in_email.yml           | 8 ++++++++
 3 files changed, 10 insertions(+)
 create mode 100644 config/feature_flags/gitlab_com_derisk/diff_preview_in_email.yml

diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index 337f8e7f1ba487..b0cd404414f09e 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -22,6 +22,7 @@ def can_disable_group_emails?(group)
   end
 
   def can_set_group_diff_preview_in_email?(group)
+    return false unless Feature.enabled?(:diff_preview_in_email, group)
     return false if group.parent&.show_diff_preview_in_email?.equal?(false)
 
     can?(current_user, :set_show_diff_preview_in_email, group)
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 77f7341ef5789a..b7769b6e1fcfb9 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -195,6 +195,7 @@ def can_disable_emails?(project, current_user)
   end
 
   def can_set_diff_preview_in_email?(project, current_user)
+    return false unless Feature.enabled?(:diff_preview_in_email, project)
     return false if project.group&.show_diff_preview_in_email?.equal?(false)
 
     can?(current_user, :set_show_diff_preview_in_email, project)
diff --git a/config/feature_flags/gitlab_com_derisk/diff_preview_in_email.yml b/config/feature_flags/gitlab_com_derisk/diff_preview_in_email.yml
new file mode 100644
index 00000000000000..71494f2504e09d
--- /dev/null
+++ b/config/feature_flags/gitlab_com_derisk/diff_preview_in_email.yml
@@ -0,0 +1,8 @@
+---
+name: diff_preview_in_email
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60007
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/382055
+milestone: '15.6'
+type: gitlab_com_derisk
+group: group::pipline_authoring
+default_enabled: false
-- 
GitLab


From d343d2ff36c4861b662456d2626106d42a710599 Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Thu, 10 Nov 2022 10:23:55 -0500
Subject: [PATCH 07/16] Expand Yarn tests to cover variable logic

Using the "full" mount, ensure that the logic of the disabling and
paths for the data are tested.
---
 .../components/settings_panel_spec.js         | 40 ++++++++++++++++---
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
index 635a38cda40332..41ecddaf601514 100644
--- a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
+++ b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
@@ -696,17 +696,45 @@ describe('Settings Panel', () => {
     });
 
     it('should hide the diff preview toggle if diff previews cannot be disabled', () => {
-      wrapper = mountComponent({ canDisableEmails: false, canEnableDiffPreview: true });
+      wrapper = mountComponent({ canDisableEmails: true, canEnableDiffPreview: false });
 
       expect(findShowDiffPreviewSetting().exists()).toBe(false);
     });
+    it('should hide the diff preview toggle if emails cannot be disabled', () => {
+      wrapper = mountComponent({ canDisableEmails: true, canEnableDiffPreview: false });
+
+      expect(findShowDiffPreviewSetting().exists()).toBe(false);
+    });
+
+    it('disables the checkbox when emails are disabled', async () => {
+      wrapper = mountComponent(
+        {
+          canDisableEmails: true,
+          canEnableDiffPreview: true,
+        },
+        mount,
+      );
+
+      // It seems like we need the "interactivity" to ensure that the disabled
+      // attribute appears.
+      await findEmailSettings().findComponent(GlFormCheckbox).find('input').setChecked(true);
+      expect(
+        findShowDiffPreviewSetting()
+          .findComponent(GlFormCheckbox)
+          .find('input')
+          .attributes('disabled'),
+      ).toBe('disabled');
+    });
 
     it('Updates the hidden value when toggled', async () => {
-      wrapper = mountComponent({
-        canDisableEmails: true,
-        canEnableDiffPreview: true,
-        emailsDisabled: false,
-      });
+      wrapper = mountComponent(
+        {
+          canDisableEmails: true,
+          canEnableDiffPreview: true,
+          emailsDisabled: false,
+        },
+        mount,
+      );
       const originalHiddenInputValue = findShowDiffPreviewSetting()
         .find('input[type="hidden"]')
         .attributes('value');
-- 
GitLab


From 04d3d57dab662fe77061d6f24ca84417c4ff4c32 Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Thu, 10 Nov 2022 11:20:10 -0500
Subject: [PATCH 08/16] Remove group.parent query in groups_helper

To avoid missing out on the current group's setting for diff's in
emails, remove the `.parent` attribute.

Update the test to account for the fact that the ancestor query
will look to the attribute directly instead of calling the
`?` function and update the group's setting.
---
 app/helpers/groups_helper.rb       | 2 +-
 spec/helpers/groups_helper_spec.rb | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index b0cd404414f09e..d892318b0be324 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -23,7 +23,7 @@ def can_disable_group_emails?(group)
 
   def can_set_group_diff_preview_in_email?(group)
     return false unless Feature.enabled?(:diff_preview_in_email, group)
-    return false if group.parent&.show_diff_preview_in_email?.equal?(false)
+    return false if group.show_diff_preview_in_email?.equal?(false)
 
     can?(current_user, :set_show_diff_preview_in_email, group)
   end
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index fb72bc0603082a..58a8360775399c 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -328,13 +328,13 @@
       end
 
       it 'returns false if parent group is disabling diff previews' do
-        allow(group).to receive(:show_diff_preview_in_email?).and_return(false)
+        group.update_attribute(:show_diff_preview_in_email, false)
 
         expect(helper.can_set_group_diff_preview_in_email?(subgroup)).to be_falsey
       end
 
       it 'returns true if parent group is not disabling diff previews' do
-        allow(group).to receive(:show_diff_preview_in_email?).and_return(true)
+        group.update_attribute(:show_diff_preview_in_email, true)
 
         expect(helper.can_set_group_diff_preview_in_email?(subgroup)).to be_truthy
       end
-- 
GitLab


From 17f45b4f4794b9db0919b29ed7bf0a2d67e74ffa Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Thu, 10 Nov 2022 14:19:06 -0500
Subject: [PATCH 09/16] Make ".js-emails-disabled" visible again

Move the 'js-emails-disabled' class to the gl-form-checkbox instead
of being on the hidden input.
---
 .../shared/permissions/components/settings_panel.vue     | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
index 9f6f3dadef446e..4e5621f0c08d81 100644
--- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
+++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
@@ -1043,13 +1043,8 @@ export default {
 
     <h5>{{ $options.i18n.emailsLabel }}</h5>
     <project-setting-row v-if="canDisableEmails" ref="email-settings" class="mb-3">
-      <input
-        class="js-emails-disabled"
-        :value="emailsDisabled"
-        type="hidden"
-        name="project[emails_disabled]"
-      />
-      <gl-form-checkbox v-model="emailsDisabled">
+      <input :value="emailsDisabled" type="hidden" name="project[emails_disabled]" />
+      <gl-form-checkbox v-model="emailsDisabled" class="js-emails-disabled">
         {{ s__('ProjectSettings|Disable email notifications') }}
         <template #help>{{
           s__('ProjectSettings|Override user notification preferences for all project members.')
-- 
GitLab


From eabd5297f57a0e59c20d9a54206d178e51277be6 Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Thu, 10 Nov 2022 14:52:41 -0500
Subject: [PATCH 10/16] Fix run-on sentence for description

Switch to two distinct sentences for the email diff description.
---
 .../projects/shared/permissions/components/settings_panel.vue | 2 +-
 app/views/groups/settings/_email_settings.html.haml           | 4 ++--
 locale/gitlab.pot                                             | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
index 4e5621f0c08d81..f3000c61a60837 100644
--- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
+++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
@@ -84,7 +84,7 @@ export default {
     emailsLabel: s__('ProjectSettings|Email notifications'),
     showDiffPreviewLabel: s__('ProjectSettings|Include diff previews'),
     showDiffPreviewHelpText: s__(
-      'ProjectSettings|Emails are not encrypted, concerned administrators may want to disable diff previews.',
+      'ProjectSettings|Emails are not encrypted. Concerned administrators may want to disable diff previews.',
     ),
   },
   VISIBILITY_LEVEL_PRIVATE_INTEGER,
diff --git a/app/views/groups/settings/_email_settings.html.haml b/app/views/groups/settings/_email_settings.html.haml
index 9802582f674776..b9820889695454 100644
--- a/app/views/groups/settings/_email_settings.html.haml
+++ b/app/views/groups/settings/_email_settings.html.haml
@@ -8,6 +8,6 @@
   .gl-px-7
     = f.gitlab_ui_checkbox_component :show_diff_preview_in_email,
         s_('GroupSettings|Include diff previews'),
-        checkbox_options: { checked: @group.show_diff_preview_in_email?,
+        checkbox_options: { checked: @group.show_diff_preview_in_email? & @group.emails_enabled?,,
                           disabled: !@group.emails_enabled? || !can_set_group_diff_preview_in_email?(@group) },
-        help_text: s_('GroupSettings|Emails are not encrypted, concerned administrators may want to disable diff previews.')
+        help_text: s_('GroupSettings|Emails are not encrypted. Concerned administrators may want to disable diff previews.')
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 72306a3ab98764..dd335f5ea1e26f 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -24821,7 +24821,7 @@ msgstr ""
 msgid "GroupSettings|Enable email notifications"
 msgstr ""
 
-msgid "GroupSettings|Emails are not encrypted, concerned administrators may want to disable diff previews."
+msgid "GroupSettings|Emails are not encrypted. Concerned administrators may want to disable diff previews."
 msgstr ""
 
 msgid "GroupSettings|Enable overview background aggregation for Value Streams Dashboard"
@@ -40056,7 +40056,7 @@ msgstr ""
 msgid "ProjectSettings|Email notifications"
 msgstr ""
 
-msgid "ProjectSettings|Emails are not encrypted, concerned administrators may want to disable diff previews."
+msgid "ProjectSettings|Emails are not encrypted. Concerned administrators may want to disable diff previews."
 msgstr ""
 
 msgid "ProjectSettings|Enable \"Delete source branch\" option by default"
-- 
GitLab


From 0b4980274a7cd6e1f9792e258c01fd381d8e6a8f Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Wed, 11 Oct 2023 08:45:54 -0400
Subject: [PATCH 11/16] Additional fixes for issues after rebase

- Remove extra comma
- Make Groups page respect the feature flag
- Ask about the parent groups status when checking if the project
  can set the diff-in-email status.
- Remove additional emails_disabled references
- Ensure that emails_enabled is properly saved as a
  project setting attribute
- Update the testing to account for emails_enabled
---
 .../permissions/components/settings_panel.vue  | 18 +++++++++++-------
 app/helpers/groups_helper.rb                   |  2 +-
 .../groups/settings/_email_settings.html.haml  | 13 +++++++------
 app/views/layouts/notify.html.haml             |  2 +-
 locale/gitlab.pot                              |  9 ++++++---
 .../components/settings_panel_spec.js          |  6 +++---
 6 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
index f3000c61a60837..5f841941ca9ba8 100644
--- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
+++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
@@ -395,7 +395,7 @@ export default {
     },
     findDiffPreviewValue: {
       get() {
-        return !this.emailsDisabled && this.showDiffPreviewInEmail;
+        return this.emailsEnabled && this.showDiffPreviewInEmail;
       },
       set(newValue) {
         this.showDiffPreviewInEmail = newValue;
@@ -1041,13 +1041,17 @@ export default {
       </project-setting-row>
     </div>
 
-    <h5>{{ $options.i18n.emailsLabel }}</h5>
     <project-setting-row v-if="canDisableEmails" ref="email-settings" class="mb-3">
-      <input :value="emailsDisabled" type="hidden" name="project[emails_disabled]" />
-      <gl-form-checkbox v-model="emailsDisabled" class="js-emails-disabled">
-        {{ s__('ProjectSettings|Disable email notifications') }}
+      <h5>{{ $options.i18n.emailsLabel }}</h5>
+      <input
+        :value="emailsEnabled"
+        type="hidden"
+        name="project[project_setting_attributes][emails_enabled]"
+      />
+      <gl-form-checkbox v-model="emailsEnabled">
+        {{ s__('ProjectSettings|Enable email notifications') }}
         <template #help>{{
-          s__('ProjectSettings|Override user notification preferences for all project members.')
+          s__('ProjectSettings|Enable sending email notifications for this project')
         }}</template>
       </gl-form-checkbox>
       <project-setting-row
@@ -1060,7 +1064,7 @@ export default {
           type="hidden"
           name="project[project_setting_attributes][show_diff_preview_in_email]"
         />
-        <gl-form-checkbox v-model="findDiffPreviewValue" :disabled="emailsDisabled">
+        <gl-form-checkbox v-model="findDiffPreviewValue" :disabled="!emailsEnabled">
           {{ $options.i18n.showDiffPreviewLabel }}
           <template #help>{{ $options.i18n.showDiffPreviewHelpText }}</template>
         </gl-form-checkbox>
diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index d892318b0be324..b0cd404414f09e 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -23,7 +23,7 @@ def can_disable_group_emails?(group)
 
   def can_set_group_diff_preview_in_email?(group)
     return false unless Feature.enabled?(:diff_preview_in_email, group)
-    return false if group.show_diff_preview_in_email?.equal?(false)
+    return false if group.parent&.show_diff_preview_in_email?.equal?(false)
 
     can?(current_user, :set_show_diff_preview_in_email, group)
   end
diff --git a/app/views/groups/settings/_email_settings.html.haml b/app/views/groups/settings/_email_settings.html.haml
index b9820889695454..b11ac38111daeb 100644
--- a/app/views/groups/settings/_email_settings.html.haml
+++ b/app/views/groups/settings/_email_settings.html.haml
@@ -5,9 +5,10 @@
       s_('GroupSettings|Enable email notifications'),
       checkbox_options: { checked: @group.emails_enabled?, disabled: !can_disable_group_emails?(@group) },
       help_text: s_('GroupSettings|Enable sending email notifications for this group and all its subgroups and projects')
-  .gl-px-7
-    = f.gitlab_ui_checkbox_component :show_diff_preview_in_email,
-        s_('GroupSettings|Include diff previews'),
-        checkbox_options: { checked: @group.show_diff_preview_in_email? & @group.emails_enabled?,,
-                          disabled: !@group.emails_enabled? || !can_set_group_diff_preview_in_email?(@group) },
-        help_text: s_('GroupSettings|Emails are not encrypted. Concerned administrators may want to disable diff previews.')
+  - if Feature.enabled?(:diff_preview_in_email, @group)
+    .gl-px-7
+      = f.gitlab_ui_checkbox_component :show_diff_preview_in_email,
+          s_('GroupSettings|Include diff previews'),
+          checkbox_options: { checked: @group.show_diff_preview_in_email? & @group.emails_enabled?,
+                            disabled: !@group.emails_enabled? || !can_set_group_diff_preview_in_email?(@group) },
+          help_text: s_('GroupSettings|Emails are not encrypted. Concerned administrators may want to disable diff previews.')
diff --git a/app/views/layouts/notify.html.haml b/app/views/layouts/notify.html.haml
index 2038533dfe045b..7199631abf3766 100644
--- a/app/views/layouts/notify.html.haml
+++ b/app/views/layouts/notify.html.haml
@@ -28,7 +28,7 @@
         &mdash;
         %br
         - if show_no_diff_preview_message
-          _('This project does not include diff previews in email notifications.')
+          = _('This project does not include diff previews in email notifications.')
           %br
         - if @target_url
           - if @reply_by_email
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index dd335f5ea1e26f..03b46ca03e5373 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -24812,6 +24812,9 @@ msgstr ""
 msgid "GroupSettings|Duo features"
 msgstr ""
 
+msgid "GroupSettings|Emails are not encrypted. Concerned administrators may want to disable diff previews."
+msgstr ""
+
 msgid "GroupSettings|Enable GitLab Duo features for this group %{link_start}Learn more%{link_end}."
 msgstr ""
 
@@ -24821,9 +24824,6 @@ msgstr ""
 msgid "GroupSettings|Enable email notifications"
 msgstr ""
 
-msgid "GroupSettings|Emails are not encrypted. Concerned administrators may want to disable diff previews."
-msgstr ""
-
 msgid "GroupSettings|Enable overview background aggregation for Value Streams Dashboard"
 msgstr ""
 
@@ -52650,6 +52650,9 @@ msgstr ""
 msgid "This project does not have a wiki homepage yet"
 msgstr ""
 
+msgid "This project does not include diff previews in email notifications."
+msgstr ""
+
 msgid "This project has no active access tokens."
 msgstr ""
 
diff --git a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
index 41ecddaf601514..6c732ce7d41ae3 100644
--- a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
+++ b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
@@ -29,7 +29,7 @@ const defaultProps = {
     analyticsAccessLevel: 20,
     containerRegistryAccessLevel: 20,
     lfsEnabled: true,
-    emailsDisabled: false,
+    emailsEnabled: true,
     packagesEnabled: true,
     showDefaultAwardEmojis: true,
     warnAboutPotentiallyUnwantedCharacters: true,
@@ -717,7 +717,7 @@ describe('Settings Panel', () => {
 
       // It seems like we need the "interactivity" to ensure that the disabled
       // attribute appears.
-      await findEmailSettings().findComponent(GlFormCheckbox).find('input').setChecked(true);
+      await findEmailSettings().findComponent(GlFormCheckbox).find('input').setChecked(false);
       expect(
         findShowDiffPreviewSetting()
           .findComponent(GlFormCheckbox)
@@ -731,7 +731,7 @@ describe('Settings Panel', () => {
         {
           canDisableEmails: true,
           canEnableDiffPreview: true,
-          emailsDisabled: false,
+          emailsEnabled: true,
         },
         mount,
       );
-- 
GitLab


From 2a0a009bb7227f1737c42e1224ffd2d7d2830174 Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Tue, 2 Apr 2024 10:58:02 -0400
Subject: [PATCH 12/16] Reintroduce label for js-emails-enabled

Reintroduce the label element to the project general settings page
to fix failing tests.
---
 .../permissions/components/settings_panel.vue | 26 ++++++++++---------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
index 5f841941ca9ba8..c55193dff17557 100644
--- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
+++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
@@ -1042,18 +1042,20 @@ export default {
     </div>
 
     <project-setting-row v-if="canDisableEmails" ref="email-settings" class="mb-3">
-      <h5>{{ $options.i18n.emailsLabel }}</h5>
-      <input
-        :value="emailsEnabled"
-        type="hidden"
-        name="project[project_setting_attributes][emails_enabled]"
-      />
-      <gl-form-checkbox v-model="emailsEnabled">
-        {{ s__('ProjectSettings|Enable email notifications') }}
-        <template #help>{{
-          s__('ProjectSettings|Enable sending email notifications for this project')
-        }}</template>
-      </gl-form-checkbox>
+      <label class="js-emails-enabled">
+        <h5>{{ $options.i18n.emailsLabel }}</h5>
+        <input
+          :value="emailsEnabled"
+          type="hidden"
+          name="project[project_setting_attributes][emails_enabled]"
+        />
+        <gl-form-checkbox v-model="emailsEnabled">
+          {{ s__('ProjectSettings|Enable email notifications') }}
+          <template #help>{{
+            s__('ProjectSettings|Enable sending email notifications for this project')
+          }}</template>
+        </gl-form-checkbox>
+      </label>
       <project-setting-row
         v-if="canEnableDiffPreview"
         ref="enable-diff-preview-settings"
-- 
GitLab


From 150b00894cbfda057b809091ef00bf1e6bd1bf68 Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Wed, 3 Apr 2024 10:39:55 -0400
Subject: [PATCH 13/16] Switch to canSetDiffPreviewInEmail

Per review suggestion, use a variable name that uses the same name as
the function that sets it.  Convert from canEnableDiffPreview to
canSetDiffPreviewInEmail.
---
 .../shared/permissions/components/settings_panel.vue   |  4 ++--
 app/helpers/projects_helper.rb                         |  2 +-
 .../permissions/components/settings_panel_spec.js      | 10 +++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
index c55193dff17557..a811741d2a100c 100644
--- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
+++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue
@@ -130,7 +130,7 @@ export default {
       required: false,
       default: false,
     },
-    canEnableDiffPreview: {
+    canSetDiffPreviewInEmail: {
       type: Boolean,
       required: false,
       default: false,
@@ -1057,7 +1057,7 @@ export default {
         </gl-form-checkbox>
       </label>
       <project-setting-row
-        v-if="canEnableDiffPreview"
+        v-if="canSetDiffPreviewInEmail"
         ref="enable-diff-preview-settings"
         class="gl-px-7"
       >
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index b7769b6e1fcfb9..483aa9fa201980 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -394,7 +394,7 @@ def project_permissions_panel_data(project)
       packagesHelpPath: help_page_path('user/packages/index'),
       currentSettings: project_permissions_settings(project),
       canAddCatalogResource: can_add_catalog_resource?(project),
-      canEnableDiffPreview: can_set_diff_preview_in_email?(project, current_user),
+      canSetDiffPreviewInEmail: can_set_diff_preview_in_email?(project, current_user),
       canChangeVisibilityLevel: can_change_visibility_level?(project, current_user),
       canDisableEmails: can_disable_emails?(project, current_user),
       allowedVisibilityOptions: project_allowed_visibility_levels(project),
diff --git a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
index 6c732ce7d41ae3..3e5f1e0c228728 100644
--- a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
+++ b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js
@@ -690,18 +690,18 @@ describe('Settings Panel', () => {
 
   describe('Show Diff Preview in Email', () => {
     it('should show the diff preview toggle if diff previews can be disabled', () => {
-      wrapper = mountComponent({ canDisableEmails: true, canEnableDiffPreview: true });
+      wrapper = mountComponent({ canDisableEmails: true, canSetDiffPreviewInEmail: true });
 
       expect(findShowDiffPreviewSetting().exists()).toBe(true);
     });
 
     it('should hide the diff preview toggle if diff previews cannot be disabled', () => {
-      wrapper = mountComponent({ canDisableEmails: true, canEnableDiffPreview: false });
+      wrapper = mountComponent({ canDisableEmails: true, canSetDiffPreviewInEmail: false });
 
       expect(findShowDiffPreviewSetting().exists()).toBe(false);
     });
     it('should hide the diff preview toggle if emails cannot be disabled', () => {
-      wrapper = mountComponent({ canDisableEmails: true, canEnableDiffPreview: false });
+      wrapper = mountComponent({ canDisableEmails: true, canSetDiffPreviewInEmail: false });
 
       expect(findShowDiffPreviewSetting().exists()).toBe(false);
     });
@@ -710,7 +710,7 @@ describe('Settings Panel', () => {
       wrapper = mountComponent(
         {
           canDisableEmails: true,
-          canEnableDiffPreview: true,
+          canSetDiffPreviewInEmail: true,
         },
         mount,
       );
@@ -730,7 +730,7 @@ describe('Settings Panel', () => {
       wrapper = mountComponent(
         {
           canDisableEmails: true,
-          canEnableDiffPreview: true,
+          canSetDiffPreviewInEmail: true,
           emailsEnabled: true,
         },
         mount,
-- 
GitLab


From a59bdcd7c3763fe7c08a800fded45b5480c0b6c9 Mon Sep 17 00:00:00 2001
From: Miguel Rincon <mrincon@gitlab.com>
Date: Thu, 4 Apr 2024 15:31:25 +0000
Subject: [PATCH 14/16] Apply 2 suggestion(s) to 2 file(s)

---
 app/helpers/projects_helper.rb                                  | 2 +-
 .../feature_flags/gitlab_com_derisk/diff_preview_in_email.yml   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 483aa9fa201980..b2e8ec84541268 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -195,7 +195,7 @@ def can_disable_emails?(project, current_user)
   end
 
   def can_set_diff_preview_in_email?(project, current_user)
-    return false unless Feature.enabled?(:diff_preview_in_email, project)
+    return false unless Feature.enabled?(:diff_preview_in_email, project.group)
     return false if project.group&.show_diff_preview_in_email?.equal?(false)
 
     can?(current_user, :set_show_diff_preview_in_email, project)
diff --git a/config/feature_flags/gitlab_com_derisk/diff_preview_in_email.yml b/config/feature_flags/gitlab_com_derisk/diff_preview_in_email.yml
index 71494f2504e09d..d00718ea96ebf2 100644
--- a/config/feature_flags/gitlab_com_derisk/diff_preview_in_email.yml
+++ b/config/feature_flags/gitlab_com_derisk/diff_preview_in_email.yml
@@ -4,5 +4,5 @@ introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60007
 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/382055
 milestone: '15.6'
 type: gitlab_com_derisk
-group: group::pipline_authoring
+group: group::code review
 default_enabled: false
-- 
GitLab


From b854309dca21a588f1ee75b9e7aec7c9508f8272 Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Thu, 11 Apr 2024 08:31:40 -0400
Subject: [PATCH 15/16] Redo diff_preview testing

Test the calls to can_disable_diff_preview with actual roles in the
group instead of creating the response to the call.

Show that only owners are able to able to change the values
and that the parent group has influence on the ability to change
the value.
---
 spec/helpers/groups_helper_spec.rb   | 45 +++++++++++++++++-----------
 spec/helpers/projects_helper_spec.rb | 20 ++++++-------
 2 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index 58a8360775399c..0c1707a8b1c09b 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -302,41 +302,52 @@
   end
 
   describe '#can_set_group_diff_preview_in_email?' do
-    let_it_be(:current_user) { create(:user) }
     let_it_be(:group) { create(:group, name: 'group') }
     let_it_be(:subgroup) { create(:group, name: 'subgroup', parent: group) }
 
+    let_it_be(:current_user) { create(:user) }
+    let_it_be(:group_owner)  { create(:group_member, :owner, group: group, user: create(:user)).user }
+    let_it_be(:group_maintainer) { create(:group_member, :maintainer, group: group, user: create(:user)).user }
+
     before do
-      allow(helper).to receive(:current_user) { current_user }
+      group.update_attribute(:show_diff_preview_in_email, true)
+      stub_feature_flags(diff_preview_in_email: true)
     end
 
-    it 'returns true for the group owner' do
-      allow(helper).to receive(:can?).with(current_user, :set_show_diff_preview_in_email, group) { true }
+    it 'returns true for an owner of the group' do
+      allow(helper).to receive(:current_user) { group_owner }
 
       expect(helper.can_set_group_diff_preview_in_email?(group)).to be_truthy
     end
 
-    it 'returns false for anyone else' do
-      allow(helper).to receive(:can?).with(current_user, :set_show_diff_preview_in_email, group) { false }
+    it 'returns false for a maintainer of the group' do
+      allow(helper).to receive(:current_user) { group_maintainer }
 
       expect(helper.can_set_group_diff_preview_in_email?(group)).to be_falsey
     end
 
-    context 'when a group has a parent group' do
-      before do
-        allow(helper).to receive(:can?).with(current_user, :set_show_diff_preview_in_email, subgroup) { true }
-      end
+    it 'returns false for anyone else' do
+      allow(helper).to receive(:current_user) { current_user }
 
-      it 'returns false if parent group is disabling diff previews' do
-        group.update_attribute(:show_diff_preview_in_email, false)
+      expect(helper.can_set_group_diff_preview_in_email?(group)).to be_falsey
+    end
 
-        expect(helper.can_set_group_diff_preview_in_email?(subgroup)).to be_falsey
-      end
+    context 'respects the settings of a parent group' do
+      context 'when a parent group has disabled diff previews ' do
+        before do
+          group.update_attribute(:show_diff_preview_in_email, false)
+        end
 
-      it 'returns true if parent group is not disabling diff previews' do
-        group.update_attribute(:show_diff_preview_in_email, true)
+        it 'returns false for all users' do
+          allow(helper).to receive(:current_user) { group_owner }
+          expect(helper.can_set_group_diff_preview_in_email?(subgroup)).to be_falsey
 
-        expect(helper.can_set_group_diff_preview_in_email?(subgroup)).to be_truthy
+          allow(helper).to receive(:current_user) { group_maintainer }
+          expect(helper.can_set_group_diff_preview_in_email?(subgroup)).to be_falsey
+
+          allow(helper).to receive(:current_user) { current_user }
+          expect(helper.can_set_group_diff_preview_in_email?(subgroup)).to be_falsey
+        end
       end
     end
   end
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index f3c37f5f56427e..d915c543d3c3fe 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -119,27 +119,27 @@
   end
 
   describe '#can_set_diff_preview_in_email?' do
+    stub_feature_flags(diff_preview_in_email: true)
     let_it_be(:user) { create(:project_member, :maintainer, user: create(:user), project: project).user }
 
     it 'returns true for the project owner' do
-      allow(project).to receive(:show_diff_preview_in_email?).and_return(true)
-      allow(helper).to receive(:can?).with(project.owner, :set_show_diff_preview_in_email, project) { true }
-
       expect(helper.can_set_diff_preview_in_email?(project, project.owner)).to be_truthy
     end
 
     it 'returns false for anyone else' do
-      allow(project).to receive(:show_diff_preview_in_email?).and_return(true)
-      allow(helper).to receive(:can?).with(user, :set_show_diff_preview_in_email, project) { false }
-
       expect(helper.can_set_diff_preview_in_email?(project, user)).to be_falsey
     end
 
-    it 'returns false if the parent group has diff previews disabled' do
-      project = create(:project, group: create(:group))
-      allow(project.group).to receive(:show_diff_preview_in_email?).and_return(false)
+    context 'respects the settings of a parent group' do
+      context 'when a parent group has disabled diff previews ' do
+        it 'returns false for all users' do
+          new_project = create(:project, group: create(:group))
+          new_project.group.update_attribute(:show_diff_preview_in_email, false)
 
-      expect(helper.can_set_diff_preview_in_email?(project, project.owner)).to be_falsey
+          expect(helper.can_set_diff_preview_in_email?(new_project, new_project.owner)).to be_falsey
+          expect(helper.can_set_diff_preview_in_email?(new_project, user)).to be_falsey
+        end
+      end
     end
   end
 
-- 
GitLab


From b9e3580406d473c6fd7331341fcf4e5f6be7636a Mon Sep 17 00:00:00 2001
From: Joseph Snyder <joe.snyder@kitware.com>
Date: Thu, 11 Apr 2024 16:28:48 -0400
Subject: [PATCH 16/16] Add nil return comment for diff_preview function

Add an in-line comment to the show_diff_preview_in_email function which
tells about the fact that a nil return will be used as a "true" value
---
 app/models/namespace_setting.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/models/namespace_setting.rb b/app/models/namespace_setting.rb
index 64634c03f677d3..63a956e8d78ed2 100644
--- a/app/models/namespace_setting.rb
+++ b/app/models/namespace_setting.rb
@@ -73,6 +73,7 @@ def emails_enabled?
     all_ancestors_have_emails_enabled?
   end
 
+  # Where this function is used, a returned "nil" is considered a truthy value
   def show_diff_preview_in_email?
     return show_diff_preview_in_email unless namespace.has_parent?
 
-- 
GitLab