Merge branch 'prevent_diff_in_email' into 'master'

Add setting to not display code diffs in MR review emails

See merge request gitlab-org/gitlab!79229

app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue

@@ -199,6 +199,7 @@ export default {
       requestAccessEnabled: true,
       highlightChangesClass: false,
       emailsDisabled: false,
+      showDiffPreviewInEmail: true,
       cveIdRequestEnabled: true,
       featureAccessLevelEveryone,
       featureAccessLevelMembers,
@@ -760,6 +761,24 @@ export default {
         s__('ProjectSettings|Override user notification preferences for all project members.')
       }}</span>
     </project-setting-row>
+    <project-setting-row class="mb-3">
+      <input
+        :value="showDiffPreviewInEmail"
+        type="hidden"
+        name="project[project_setting_attributes][show_diff_preview_in_email]"
+      />
+      <gl-form-checkbox
+        v-model="showDiffPreviewInEmail"
+        name="project[project_setting_attributes][show_diff_preview_in_email]"
+      >
+        {{ s__('ProjectSettings|Include diff preview in merge request notification emails') }}
+        <template #help>{{
+          s__(
+            'ProjectSettings|Include the code diff preview on comment threads in merge request notification emails.',
+          )
+        }}</template>
+      </gl-form-checkbox>
+    </project-setting-row>
     <project-setting-row class="mb-3">
       <input
         :value="showDefaultAwardEmojis"

app/controllers/projects_controller.rb

@@ -419,6 +419,7 @@ class ProjectsController < Projects::ApplicationController
     %i[
       show_default_award_emojis
       squash_option
+      show_diff_preview_in_email
       mr_default_target_self
       warn_about_potentially_unwanted_characters
     ]

app/helpers/projects_helper.rb

@@ -586,6 +586,7 @@ module ProjectsHelper
       metricsDashboardAccessLevel: feature.metrics_dashboard_access_level,
       operationsAccessLevel: feature.operations_access_level,
       showDefaultAwardEmojis: project.show_default_award_emojis?,
+      showDiffPreviewInEmail: project.show_diff_preview_in_email?,
       warnAboutPotentiallyUnwantedCharacters: project.warn_about_potentially_unwanted_characters?,
       securityAndComplianceAccessLevel: project.security_and_compliance_access_level,
       containerRegistryAccessLevel: feature.container_registry_access_level

app/models/project.rb

@@ -433,6 +433,7 @@ class Project < ApplicationRecord
   alias_method :container_registry_enabled, :container_registry_enabled?
   delegate :show_default_award_emojis, :show_default_award_emojis=, :show_default_award_emojis?,
     :warn_about_potentially_unwanted_characters, :warn_about_potentially_unwanted_characters=, :warn_about_potentially_unwanted_characters?,
+    :show_diff_preview_in_email, :show_diff_preview_in_email=, :show_diff_preview_in_email?,
     to: :project_setting, allow_nil: true
   delegate :scheduled?, :started?, :in_progress?, :failed?, :finished?,
     prefix: :import, to: :import_state, allow_nil: true

app/policies/project_policy.rb

@@ -229,6 +229,7 @@ class ProjectPolicy < BasePolicy
     enable :set_note_created_at
     enable :set_emails_disabled
     enable :set_show_default_award_emojis
+    enable :set_show_diff_preview_in_email
     enable :set_warn_about_potentially_unwanted_characters
   end
 

app/views/notify/_note_email.html.haml

@@ -20,8 +20,7 @@
         discussion on #{link_to(discussion.file_path, target_url)}
       - 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?
   = content_for :head do
     = stylesheet_link_tag 'mailers/highlighted_diff_email'
 

app/views/notify/_note_email.text.erb

@@ -20,7 +20,7 @@
 <%  end -%>
 
 
-<%  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 -%>

db/migrate/20220131192643_add_show_diff_preview_in_email_to_project_settings.rb

+# frozen_string_literal: true
+
+class AddShowDiffPreviewInEmailToProjectSettings < Gitlab::Database::Migration[1.0]
+  enable_lock_retries!
+
+  def change
+    add_column :project_settings, :show_diff_preview_in_email, :boolean, default: true, null: false
+  end
+end

db/schema_migrations/20220131192643

+3b9068f109685dcfa8a0a1fda886cca3909d29cbc280cf70ed9f3d927def12ac
\ No newline at end of file

db/structure.sql

@@ -18696,6 +18696,7 @@ CREATE TABLE project_settings (
     merge_commit_template text,
     has_shimo boolean DEFAULT false NOT NULL,
     squash_commit_template text,
+    show_diff_preview_in_email boolean DEFAULT true NOT NULL,
     CONSTRAINT check_3a03e7557a CHECK ((char_length(previous_default_branch) <= 4096)),
     CONSTRAINT check_b09644994b CHECK ((char_length(squash_commit_template) <= 500)),
     CONSTRAINT check_bde223416c CHECK ((show_default_award_emojis IS NOT NULL)),

locale/gitlab.pot

@@ -28300,6 +28300,12 @@ 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 preview in merge request notification emails"
+msgstr ""
+
+msgid "ProjectSettings|Include the code diff preview on comment threads in merge request notification emails."
+msgstr ""
+
 msgid "ProjectSettings|Internal"
 msgstr ""
 

spec/features/projects/settings/project_settings_spec.rb

@@ -85,6 +85,36 @@ RSpec.describe 'Projects settings' do
     end
   end
 
+  context 'show diffs in emails', :js do
+    it 'does not hide diffs by default' do
+      visit edit_project_path(project)
+
+      show_diff_preview_in_email_input = find('input[name="project[project_setting_attributes][show_diff_preview_in_email]"]', visible: :hidden)
+
+      expect(show_diff_preview_in_email_input.value).to eq('true')
+    end
+
+    it 'hides diffs in emails when toggled' do
+      visit edit_project_path(project)
+
+      show_diff_preview_in_email_input = find('input[name="project[project_setting_attributes][show_diff_preview_in_email]"]', visible: :hidden)
+      show_diff_preview_in_email_checkbox = find('input[name="project[project_setting_attributes][show_diff_preview_in_email]"][type=checkbox]')
+
+      expect(show_diff_preview_in_email_input.value).to eq('true')
+
+      show_diff_preview_in_email_checkbox.click
+
+      expect(show_diff_preview_in_email_input.value).to eq('false')
+
+      page.within('.sharing-permissions') do
+        find('[data-testid="project-features-save-button"]').click
+      end
+      wait_for_requests
+
+      expect(show_diff_preview_in_email_input.value).to eq('false')
+    end
+  end
+
   def expect_toggle_state(state)
     is_collapsed = state == :collapsed
 

spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js

@@ -28,6 +28,7 @@ const defaultProps = {
     emailsDisabled: false,
     packagesEnabled: true,
     showDefaultAwardEmojis: true,
+    showDiffPreviewInEmail: true,
     warnAboutPotentiallyUnwantedCharacters: true,
   },
   isGitlabCom: true,
@@ -101,6 +102,9 @@ describe('Settings Panel', () => {
   const findEmailSettings = () => wrapper.find({ ref: 'email-settings' });
   const findShowDefaultAwardEmojis = () =>
     wrapper.find('input[name="project[project_setting_attributes][show_default_award_emojis]"]');
+
+  const findShowDiffPreviewInEmail = () =>
+    wrapper.find('input[name="project[project_setting_attributes][show_diff_preview_in_email]"]');
   const findWarnAboutPuc = () =>
     wrapper.find(
       'input[name="project[project_setting_attributes][warn_about_potentially_unwanted_characters]"]',
@@ -585,6 +589,13 @@ describe('Settings Panel', () => {
       expect(findShowDefaultAwardEmojis().exists()).toBe(true);
     });
   });
+  describe('Hide diffs in email', () => {
+    it('should show the "Hide Diffs in email" input', () => {
+      wrapper = mountComponent();
+
+      expect(findShowDiffPreviewInEmail().exists()).toBe(true);
+    });
+  });
 
   describe('Warn about potentially unwanted characters', () => {
     it('should have a "Warn about Potentially Unwanted Characters" input', () => {

spec/helpers/projects_helper_spec.rb

@@ -964,6 +964,7 @@ RSpec.describe ProjectsHelper do
         metricsDashboardAccessLevel: project.project_feature.metrics_dashboard_access_level,
         operationsAccessLevel: project.project_feature.operations_access_level,
         showDefaultAwardEmojis: project.show_default_award_emojis?,
+        showDiffPreviewInEmail: project.show_diff_preview_in_email?,
         securityAndComplianceAccessLevel: project.security_and_compliance_access_level,
         containerRegistryAccessLevel: project.project_feature.container_registry_access_level
       )

spec/requests/api/project_attributes.yml

@@ -145,6 +145,7 @@ project_setting:
     - project_id
     - push_rule_id
     - show_default_award_emojis
+    - show_diff_preview_in_email
     - updated_at
     - cve_id_request_enabled
     - mr_default_target_self