Add latest changes from gitlab-org/gitlab@14-10-stable-eev14.10.0-rc42

3 files changed, 182 insertions, 15 deletions

diff --git a/spec/mailers/emails/in_product_marketing_spec.rb b/spec/mailers/emails/in_product_marketing_spec.rb
index 720e6f101a8..e62719f4283 100644
--- a/spec/mailers/emails/in_product_marketing_spec.rb
+++ b/spec/mailers/emails/in_product_marketing_spec.rb
@@ -69,7 +69,6 @@ RSpec.describe Emails::InProductMarketing do
       :team_short   | 0
       :trial_short  | 0
       :admin_verify | 0
-      :invite_team  | 0
     end
 
     with_them do
@@ -99,11 +98,7 @@ RSpec.describe Emails::InProductMarketing do
             is_expected.not_to have_body_text(CGI.unescapeHTML(message.invite_link))
           end
 
-          if track == :invite_team
-            is_expected.not_to have_body_text(/This is email \d of \d/)
-          else
-            is_expected.to have_body_text(message.progress)
-          end
+          is_expected.to have_body_text(message.progress)
         end
       end
     end
diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb
index 87776457473..f4483f7e8f5 100644
--- a/spec/mailers/emails/profile_spec.rb
+++ b/spec/mailers/emails/profile_spec.rb
@@ -416,4 +416,27 @@ RSpec.describe Emails::Profile do
       is_expected.to have_body_text /#{profile_two_factor_auth_path}/
     end
   end
+
+  describe 'added a new email address' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:email) { create(:email, user: user) }
+
+    subject { Notify.new_email_address_added_email(user, email) }
+
+    it_behaves_like 'an email sent from GitLab'
+    it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like 'a user cannot unsubscribe through footer link'
+
+    it 'is sent to the user' do
+      is_expected.to deliver_to user.email
+    end
+
+    it 'has the correct subject' do
+      is_expected.to have_subject /^New email address added$/i
+    end
+
+    it 'includes a link to the email address page' do
+      is_expected.to have_body_text /#{profile_emails_path}/
+    end
+  end
 end
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 978118ed1b1..b6ad66f41b5 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -317,6 +317,9 @@ RSpec.describe Notify do
     end
 
     context 'for merge requests' do
+      let(:push_user) { create(:user) }
+      let(:commit_limit) { NotificationService::NEW_COMMIT_EMAIL_DISPLAY_LIMIT }
+
       describe 'that are new' do
         subject { described_class.new_merge_request_email(merge_request.assignee_ids.first, merge_request.id) }
 
@@ -457,12 +460,6 @@ RSpec.describe Notify do
       end
 
       shared_examples 'a push to an existing merge request' do
-        let(:push_user) { create(:user) }
-
-        subject do
-          described_class.push_to_merge_request_email(recipient.id, merge_request.id, push_user.id, new_commits: merge_request.commits, existing_commits: existing_commits)
-        end
-
         it_behaves_like 'a multiple recipients email'
         it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
           let(:model) { merge_request }
@@ -487,16 +484,136 @@ RSpec.describe Notify do
         end
       end
 
-      describe 'that have new commits' do
-        let(:existing_commits) { [] }
+      shared_examples 'shows the compare url between first and last commits' do |count|
+        it 'shows the compare url between first and last commits' do
+          commit_id_1 = existing_commits.first[:short_id]
+          commit_id_2 = existing_commits.last[:short_id]
+
+          is_expected.to have_link("#{commit_id_1}...#{commit_id_2}", href: project_compare_url(project, from: commit_id_1, to: commit_id_2))
+          is_expected.to have_body_text("#{count} commits from branch `#{merge_request.target_branch}`")
+        end
+      end
+
+      shared_examples 'shows new commit urls' do |count|
+        it 'shows new commit urls' do
+          displayed_new_commits.each do |commit|
+            is_expected.to have_link(commit[:short_id], href: project_commit_url(project, commit[:short_id]))
+            is_expected.to have_body_text(commit[:title])
+          end
+        end
+
+        it 'does not show hidden new commit urls' do
+          hidden_new_commits.each do |commit|
+            is_expected.not_to have_link(commit[:short_id], href: project_commit_url(project, commit[:short_id]))
+            is_expected.not_to have_body_text(commit[:title])
+          end
+        end
+      end
+
+      describe 'that have no new commits' do
+        subject do
+          described_class.push_to_merge_request_email(recipient.id, merge_request.id, push_user.id, new_commits: [], total_new_commits_count: 0, existing_commits: [], total_existing_commits_count: 0)
+        end
 
         it_behaves_like 'a push to an existing merge request'
       end
 
+      describe 'that have fewer than the commit truncation limit' do
+        let(:new_commits) { merge_request.commits }
+        let(:displayed_new_commits) { new_commits }
+        let(:hidden_new_commits) { [] }
+
+        subject do
+          described_class.push_to_merge_request_email(
+            recipient.id, merge_request.id, push_user.id,
+            new_commits: new_commits, total_new_commits_count: new_commits.length,
+            existing_commits: [], total_existing_commits_count: 0
+          )
+        end
+
+        it_behaves_like 'a push to an existing merge request'
+        it_behaves_like 'shows new commit urls'
+      end
+
+      describe 'that have more than the commit truncation limit' do
+        let(:new_commits) do
+          Array.new(commit_limit + 10) do |i|
+            {
+              short_id: SecureRandom.hex(4),
+              title: "This is commit #{i}"
+            }
+          end
+        end
+
+        let(:displayed_new_commits) { new_commits.first(commit_limit) }
+        let(:hidden_new_commits) { new_commits.last(10) }
+
+        subject do
+          described_class.push_to_merge_request_email(
+            recipient.id, merge_request.id, push_user.id,
+            new_commits: displayed_new_commits, total_new_commits_count: commit_limit + 10,
+            existing_commits: [], total_existing_commits_count: 0
+          )
+        end
+
+        it_behaves_like 'a push to an existing merge request'
+        it_behaves_like 'shows new commit urls'
+
+        it 'shows "and more" message' do
+          is_expected.to have_body_text("And 10 more")
+        end
+      end
+
       describe 'that have new commits on top of an existing one' do
         let(:existing_commits) { [merge_request.commits.first] }
 
+        subject do
+          described_class.push_to_merge_request_email(
+            recipient.id, merge_request.id, push_user.id,
+            new_commits: merge_request.commits, total_new_commits_count: merge_request.commits.length,
+            existing_commits: existing_commits, total_existing_commits_count: existing_commits.length
+          )
+        end
+
+        it_behaves_like 'a push to an existing merge request'
+
+        it 'shows the existing commit' do
+          commit_id = existing_commits.first.short_id
+          is_expected.to have_link(commit_id, href: project_commit_url(project, commit_id))
+          is_expected.to have_body_text("1 commit from branch `#{merge_request.target_branch}`")
+        end
+      end
+
+      describe 'that have new commits on top of two existing ones' do
+        let(:existing_commits) { [merge_request.commits.first, merge_request.commits.second] }
+
+        subject do
+          described_class.push_to_merge_request_email(
+            recipient.id, merge_request.id, push_user.id,
+            new_commits: merge_request.commits, total_new_commits_count: merge_request.commits.length,
+            existing_commits: existing_commits, total_existing_commits_count: existing_commits.length
+          )
+        end
+
+        it_behaves_like 'a push to an existing merge request'
+        it_behaves_like 'shows the compare url between first and last commits', 2
+      end
+
+      describe 'that have new commits on top of more than two existing ones' do
+        let(:existing_commits) do
+          [merge_request.commits.first] + [double(:commit)] * 3 + [merge_request.commits.second]
+        end
+
+        subject do
+          described_class.push_to_merge_request_email(
+            recipient.id, merge_request.id, push_user.id,
+            new_commits: merge_request.commits, total_new_commits_count: merge_request.commits.length,
+            existing_commits: existing_commits, total_existing_commits_count: existing_commits.length
+          )
+        end
+
         it_behaves_like 'a push to an existing merge request'
+        it_behaves_like 'shows the compare url between first and last commits', 5
       end
     end
 
@@ -2064,14 +2181,46 @@ RSpec.describe Notify do
     context 'when diff note' do
       let!(:notes) { create_list(:diff_note_on_merge_request, 3, review: review, project: project, author: review.author, noteable: merge_request) }
 
-      it 'links to notes' do
+      it 'links to notes and discussions', :aggregate_failures do
+        reply_note = create(:diff_note_on_merge_request, review: review, project: project, author: review.author, noteable: merge_request, in_reply_to: notes.first)
+
         review.notes.each do |note|
           # Text part
           expect(subject.text_part.body.raw_source).to include(
             project_merge_request_url(project, merge_request, anchor: "note_#{note.id}")
           )
+
+          if note == reply_note
+            expect(subject.text_part.body.raw_source).to include("commented on a discussion on #{note.discussion.file_path}")
+          else
+            expect(subject.text_part.body.raw_source).to include("started a new discussion on #{note.discussion.file_path}")
+          end
         end
       end
+
+      it 'includes only one link to the highlighted_diff_email' do
+        expect(subject.html_part.body.raw_source).to include('assets/mailers/highlighted_diff_email').once
+      end
+
+      it 'avoids N+1 cached queries when rendering html', :use_sql_query_cache, :request_store do
+        control_count = ActiveRecord::QueryRecorder.new(query_recorder_debug: true, skip_cached: false) do
+          subject.html_part
+        end
+
+        create_list(:diff_note_on_merge_request, 3, review: review, project: project, author: review.author, noteable: merge_request)
+
+        expect { described_class.new_review_email(recipient.id, review.id).html_part }.not_to exceed_all_query_limit(control_count)
+      end
+
+      it 'avoids N+1 cached queries when rendering text', :use_sql_query_cache, :request_store do
+        control_count = ActiveRecord::QueryRecorder.new(query_recorder_debug: true, skip_cached: false) do
+          subject.text_part
+        end
+
+        create_list(:diff_note_on_merge_request, 3, review: review, project: project, author: review.author, noteable: merge_request)
+
+        expect { described_class.new_review_email(recipient.id, review.id).text_part }.not_to exceed_all_query_limit(control_count)
+      end
     end
 
     it 'contains review author name' do