Add latest changes from gitlab-org/gitlab@master

15 files changed, 232 insertions, 46 deletions

diff --git a/spec/features/discussion_comments/merge_request_spec.rb b/spec/features/discussion_comments/merge_request_spec.rb
index f60d7da6a30..a90ff3721d3 100644
--- a/spec/features/discussion_comments/merge_request_spec.rb
+++ b/spec/features/discussion_comments/merge_request_spec.rb
@@ -8,8 +8,6 @@ RSpec.describe 'Thread Comments Merge Request', :js do
   let(:merge_request) { create(:merge_request, source_project: project) }
 
   before do
-    stub_feature_flags(remove_resolve_note: false)
-
     project.add_maintainer(user)
     sign_in(user)
 
diff --git a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb
index a4e9df604a9..34d78880991 100644
--- a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb
+++ b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb
@@ -18,10 +18,6 @@ RSpec.describe 'Resolving all open threads in a merge request from an issue', :j
     end
   end
 
-  before do
-    stub_feature_flags(remove_resolve_note: false)
-  end
-
   describe 'as a user with access to the project' do
     before do
       project.add_maintainer(user)
@@ -37,7 +33,7 @@ RSpec.describe 'Resolving all open threads in a merge request from an issue', :j
 
     context 'resolving the thread' do
       before do
-        click_button 'Resolve thread'
+        find('button[data-qa-selector="resolve_discussion_button"]').click
       end
 
       it 'hides the link for creating a new issue' do
diff --git a/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb
index 99dc71f0559..ac3471e8401 100644
--- a/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb
+++ b/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb
@@ -14,10 +14,6 @@ RSpec.describe 'Resolve an open thread in a merge request by creating an issue',
     "a[title=\"#{title}\"][href=\"#{url}\"]"
   end
 
-  before do
-    stub_feature_flags(remove_resolve_note: false)
-  end
-
   describe 'As a user with access to the project' do
     before do
       project.add_maintainer(user)
@@ -39,7 +35,7 @@ RSpec.describe 'Resolve an open thread in a merge request by creating an issue',
 
     context 'resolving the thread' do
       before do
-        click_button 'Resolve thread'
+        find('button[data-qa-selector="resolve_discussion_button"]').click
       end
 
       it 'hides the link for creating a new issue' do
diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
index caa04059469..9a3f97a0943 100644
--- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
+++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb
@@ -15,10 +15,6 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
           diff_refs: merge_request.diff_refs)
   end
 
-  before do
-    stub_feature_flags(remove_resolve_note: false)
-  end
-
   context 'no threads' do
     before do
       project.add_maintainer(user)
@@ -67,7 +63,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to mark thread as resolved' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
         end
 
         expect(page).to have_selector('.discussion-body', visible: false)
@@ -84,7 +80,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to unresolve thread' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
           click_button 'Unresolve thread'
         end
 
@@ -96,7 +92,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
       describe 'resolved thread' do
         before do
           page.within '.diff-content' do
-            click_button 'Resolve thread'
+            find('button[data-qa-selector="resolve_discussion_button"]').click
           end
 
           visit_merge_request
@@ -197,7 +193,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to resolve from reply form without a comment' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
         end
 
         page.within '.line-resolve-all-container' do
@@ -234,7 +230,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'hides jump to next button when all resolved' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
         end
 
         expect(page).to have_selector('.discussion-next-btn', visible: false)
@@ -264,7 +260,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
         visit_merge_request
       end
 
-      it 'does not mark thread as resolved when resolving single note' do
+      it 'marks thread as resolved when resolving single note' do
         page.within("#note_#{note.id}") do
           first('.line-resolve-btn').click
 
@@ -273,15 +269,13 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
           expect(first('.line-resolve-btn')['aria-label']).to eq("Resolved by #{user.name}")
         end
 
-        expect(page).to have_content('Last updated')
-
         page.within '.line-resolve-all-container' do
-          expect(page).to have_content('1 unresolved thread')
+          expect(page).to have_content('All threads resolved')
         end
       end
 
       it 'resolves thread' do
-        resolve_buttons = page.all('.note .line-resolve-btn', count: 2)
+        resolve_buttons = page.all('.note .line-resolve-btn', count: 1)
         resolve_buttons.each do |button|
           button.click
         end
@@ -332,7 +326,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
       it 'allows user to mark all threads as resolved' do
         page.all('.discussion-reply-holder', count: 2).each do |reply_holder|
           page.within reply_holder do
-            click_button 'Resolve thread'
+            find('button[data-qa-selector="resolve_discussion_button"]').click
           end
         end
 
@@ -344,7 +338,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to quickly scroll to next unresolved thread' do
         page.within('.discussion-reply-holder', match: :first) do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
         end
 
         page.within '.line-resolve-all-container' do
@@ -416,7 +410,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to mark thread as resolved' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
         end
 
         page.within '.diff-content .note' do
@@ -431,7 +425,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to unresolve thread' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
           click_button 'Unresolve thread'
         end
 
@@ -459,7 +453,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
 
       it 'allows user to comment & unresolve thread' do
         page.within '.diff-content' do
-          click_button 'Resolve thread'
+          find('button[data-qa-selector="resolve_discussion_button"]').click
 
           find_field('Reply…').click
 
diff --git a/spec/frontend/notes/components/noteable_discussion_spec.js b/spec/frontend/notes/components/noteable_discussion_spec.js
index dd65351ef88..735bc2b70dd 100644
--- a/spec/frontend/notes/components/noteable_discussion_spec.js
+++ b/spec/frontend/notes/components/noteable_discussion_spec.js
@@ -124,14 +124,7 @@ describe('noteable_discussion component', () => {
         ...getJSONFixture(discussionWithTwoUnresolvedNotes)[0],
         expanded: true,
       };
-      discussion.notes = discussion.notes.map((note) => ({
-        ...note,
-        resolved: false,
-        current_user: {
-          ...note.current_user,
-          can_resolve: true,
-        },
-      }));
+      discussion.resolved = false;
 
       wrapper.setProps({ discussion });
 
diff --git a/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js b/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js
index b18bad1ad9b..8ab0b87d2ee 100644
--- a/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js
+++ b/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js
@@ -120,8 +120,8 @@ describe('WikiForm', () => {
 
   it.each`
     persisted | titleHelpText                                                                                              | titleHelpLink
-    ${true}   | ${'You can move this page by adding the path to the beginning of the title.'}                              | ${'/help/user/project/wiki/index#moving-a-wiki-page'}
-    ${false}  | ${'You can specify the full path for the new file. We will automatically create any missing directories.'} | ${'/help/user/project/wiki/index#creating-a-new-wiki-page'}
+    ${true}   | ${'You can move this page by adding the path to the beginning of the title.'}                              | ${'/help/user/project/wiki/index#move-a-wiki-page'}
+    ${false}  | ${'You can specify the full path for the new file. We will automatically create any missing directories.'} | ${'/help/user/project/wiki/index#create-a-new-wiki-page'}
   `(
     'shows appropriate title help text and help link for when persisted=$persisted',
     async ({ persisted, titleHelpLink, titleHelpText }) => {
diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb
index a32e566fc90..764186dc226 100644
--- a/spec/mailers/emails/profile_spec.rb
+++ b/spec/mailers/emails/profile_spec.rb
@@ -212,6 +212,57 @@ RSpec.describe Emails::Profile do
     end
   end
 
+  describe 'notification email for expired ssh key' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:fingerprints) { ["aa:bb:cc:dd:ee:zz"] }
+
+    context 'when valid' do
+      subject { Notify.ssh_key_expired_email(user, fingerprints) }
+
+      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 /Your SSH key has expired/
+      end
+
+      it 'mentions the ssh keu has expired' do
+        is_expected.to have_body_text /Your SSH keys with the following fingerprints has expired/
+      end
+
+      it 'includes a link to ssh key page' do
+        is_expected.to have_body_text /#{profile_keys_url}/
+      end
+
+      it 'includes the email reason' do
+        is_expected.to have_body_text /You're receiving this email because of your account on localhost/
+      end
+    end
+
+    context 'when invalid' do
+      context 'when user does not exist' do
+        it do
+          expect { Notify.ssh_key_expired_email(nil) }.not_to change { ActionMailer::Base.deliveries.count }
+        end
+      end
+
+      context 'when user is not active' do
+        before do
+          user.block!
+        end
+
+        it do
+          expect { Notify.ssh_key_expired_email(user) }.not_to change { ActionMailer::Base.deliveries.count }
+        end
+      end
+    end
+  end
+
   describe 'user unknown sign in email' do
     let_it_be(:user) { create(:user) }
     let_it_be(:ip) { '169.0.0.1' }
diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb
index 3d33a39d353..195ec1fa83b 100644
--- a/spec/models/key_spec.rb
+++ b/spec/models/key_spec.rb
@@ -75,6 +75,18 @@ RSpec.describe Key, :mailer do
           .to eq([key_3, key_1, key_2])
       end
     end
+
+    describe '.expired_today_and_not_notified' do
+      let_it_be(:user) { create(:user) }
+      let_it_be(:expired_today_not_notified) { create(:key, expires_at: Time.current, user: user) }
+      let_it_be(:expired_today_already_notified) { create(:key, expires_at: Time.current, user: user, expiry_notification_delivered_at: Time.current) }
+      let_it_be(:expired_yesterday) { create(:key, expires_at: 1.day.ago, user: user) }
+      let_it_be(:future_expiry) { create(:key, expires_at: 1.day.from_now, user: user) }
+
+      it 'returns tokens that have expired today' do
+        expect(described_class.expired_today_and_not_notified).to contain_exactly(expired_today_not_notified)
+      end
+    end
   end
 
   context "validation of uniqueness (based on fingerprint uniqueness)" do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index abd32a88db4..9498aa75289 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -85,6 +85,7 @@ RSpec.describe User do
     it { is_expected.to have_many(:group_members) }
     it { is_expected.to have_many(:groups) }
     it { is_expected.to have_many(:keys).dependent(:destroy) }
+    it { is_expected.to have_many(:expired_today_and_unnotified_keys) }
     it { is_expected.to have_many(:deploy_keys).dependent(:nullify) }
     it { is_expected.to have_many(:group_deploy_keys) }
     it { is_expected.to have_many(:events).dependent(:delete_all) }
@@ -1000,6 +1001,18 @@ RSpec.describe User do
       end
     end
 
+    describe '.with_ssh_key_expired_today' do
+      let_it_be(:user1) { create(:user) }
+      let_it_be(:expired_today_not_notified) { create(:key, expires_at: Time.current, user: user1) }
+
+      let_it_be(:user2) { create(:user) }
+      let_it_be(:expired_today_already_notified) { create(:key, expires_at: Time.current, user: user2, expiry_notification_delivered_at: Time.current) }
+
+      it 'returns users whose token has expired today' do
+        expect(described_class.with_ssh_key_expired_today).to contain_exactly(user1)
+      end
+    end
+
     describe '.active_without_ghosts' do
       let_it_be(:user1) { create(:user, :external) }
       let_it_be(:user2) { create(:user, state: 'blocked') }
diff --git a/spec/services/keys/expiry_notification_service_spec.rb b/spec/services/keys/expiry_notification_service_spec.rb
new file mode 100644
index 00000000000..4cfd576a15f
--- /dev/null
+++ b/spec/services/keys/expiry_notification_service_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Keys::ExpiryNotificationService do
+  let_it_be_with_reload(:user) { create(:user) }
+  let_it_be_with_reload(:expired_key) { create(:key, expires_at: Time.current, user: user) }
+
+  let(:params) { { keys: keys } }
+
+  subject { described_class.new(user, params) }
+
+  context 'with expired key', :mailer do
+    let(:keys) { user.keys }
+
+    it 'sends a notification' do
+      perform_enqueued_jobs do
+        subject.execute
+      end
+      should_email(user)
+    end
+
+    it 'uses notification service to send email to the user' do
+      expect_next_instance_of(NotificationService) do |notification_service|
+        expect(notification_service).to receive(:ssh_key_expired).with(expired_key.user, [expired_key.fingerprint])
+      end
+
+      subject.execute
+    end
+
+    it 'updates notified column' do
+      expect { subject.execute }.to change { expired_key.reload.expiry_notification_delivered_at }
+    end
+
+    context 'when user does not have permission to receive notification' do
+      before do
+        user.block!
+      end
+
+      it 'does not send notification' do
+        perform_enqueued_jobs do
+          subject.execute
+        end
+        should_not_email(user)
+      end
+
+      it 'does not update notified column' do
+        expect { subject.execute }.not_to change { expired_key.reload.expiry_notification_delivered_at }
+      end
+    end
+  end
+end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index ad833bb7c71..63bfc090a0d 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -288,6 +288,27 @@ RSpec.describe NotificationService, :mailer do
         end
       end
     end
+
+    describe '#ssh_key_expired' do
+      let_it_be(:user) { create(:user) }
+      let_it_be(:fingerprints) { ["aa:bb:cc:dd:ee:zz"] }
+
+      subject { notification.ssh_key_expired(user, fingerprints) }
+
+      it 'sends email to the token owner' do
+        expect { subject }.to have_enqueued_email(user, fingerprints, mail: "ssh_key_expired_email")
+      end
+
+      context 'when user is not allowed to receive notifications' do
+        before do
+          user.block!
+        end
+
+        it 'does not send email to the token owner' do
+          expect { subject }.not_to have_enqueued_email(user, fingerprints, mail: "ssh_key_expired_email")
+        end
+      end
+    end
   end
 
   describe '#unknown_sign_in' do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index d12b960d4fc..84c38ca0ce2 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -25,7 +25,7 @@ ENV["RAILS_ENV"] = 'test'
 ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true'
 ENV["RSPEC_ALLOW_INVALID_URLS"] = 'true'
 
-require File.expand_path('../config/environment', __dir__)
+require_relative '../config/environment'
 
 require 'rspec/mocks'
 require 'rspec/rails'
@@ -72,6 +72,8 @@ Dir[Rails.root.join("spec/support/shared_contexts/*.rb")].sort.each { |f| requir
 Dir[Rails.root.join("spec/support/shared_examples/*.rb")].sort.each { |f| require f }
 Dir[Rails.root.join("spec/support/**/*.rb")].sort.each { |f| require f }
 
+require_relative '../tooling/quality/test_level'
+
 quality_level = Quality::TestLevel.new
 
 RSpec.configure do |config|
diff --git a/spec/support/shared_examples/features/discussion_comments_shared_example.rb b/spec/support/shared_examples/features/discussion_comments_shared_example.rb
index 86ba2821c78..808e0be6be2 100644
--- a/spec/support/shared_examples/features/discussion_comments_shared_example.rb
+++ b/spec/support/shared_examples/features/discussion_comments_shared_example.rb
@@ -304,7 +304,7 @@ RSpec.shared_examples 'thread comments for issue, epic and merge request' do |re
           let(:reply_id) { find("#{comments_selector} .note:last-of-type", match: :first)['data-note-id'] }
 
           it 'can be replied to after resolving' do
-            click_button "Resolve thread"
+            find('button[data-qa-selector="resolve_discussion_button"]').click
             wait_for_requests
 
             refresh
@@ -316,7 +316,7 @@ RSpec.shared_examples 'thread comments for issue, epic and merge request' do |re
           it 'shows resolved thread when toggled' do
             submit_reply('a')
 
-            click_button "Resolve thread"
+            find('button[data-qa-selector="resolve_discussion_button"]').click
             wait_for_requests
 
             expect(page).to have_selector(".note-row-#{note_id}", visible: true)
diff --git a/spec/lib/quality/test_level_spec.rb b/spec/tooling/quality/test_level_spec.rb
index 32960cd571b..89abe337347 100644
--- a/spec/lib/quality/test_level_spec.rb
+++ b/spec/tooling/quality/test_level_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require 'fast_spec_helper'
+require_relative '../../../tooling/quality/test_level'
 
 RSpec.describe Quality::TestLevel do
   describe '#pattern' do
@@ -197,7 +197,7 @@ RSpec.describe Quality::TestLevel do
     it 'raises an error for an unknown level' do
       expect { subject.level_for('spec/unknown/foo_spec.rb') }
         .to raise_error(described_class::UnknownTestLevelError,
-          %r{Test level for spec/unknown/foo_spec.rb couldn't be set. Please rename the file properly or change the test level detection regexes in .+/lib/quality/test_level.rb.})
+          %r{Test level for spec/unknown/foo_spec.rb couldn't be set. Please rename the file properly or change the test level detection regexes in .+/tooling/quality/test_level.rb.})
     end
   end
 
diff --git a/spec/workers/ssh_keys/expired_notification_worker_spec.rb b/spec/workers/ssh_keys/expired_notification_worker_spec.rb
new file mode 100644
index 00000000000..249ee404870
--- /dev/null
+++ b/spec/workers/ssh_keys/expired_notification_worker_spec.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe SshKeys::ExpiredNotificationWorker, type: :worker do
+  subject(:worker) { described_class.new }
+
+  it 'uses a cronjob queue' do
+    expect(worker.sidekiq_options_hash).to include(
+      'queue' => 'cronjob:ssh_keys_expired_notification',
+      'queue_namespace' => :cronjob
+    )
+  end
+
+  describe '#perform' do
+    let_it_be(:user) { create(:user) }
+
+    context 'with expiring key today' do
+      let_it_be_with_reload(:expired_today) { create(:key, expires_at: Time.current, user: user) }
+
+      it 'invoke the notification service' do
+        expect_next_instance_of(Keys::ExpiryNotificationService) do |expiry_service|
+          expect(expiry_service).to receive(:execute)
+        end
+
+        worker.perform
+      end
+
+      it 'updates notified column' do
+        expect { worker.perform }.to change { expired_today.reload.expiry_notification_delivered_at }
+      end
+
+      include_examples 'an idempotent worker' do
+        subject do
+          perform_multiple(worker: worker)
+        end
+      end
+
+      context 'when feature is not enabled' do
+        before do
+          stub_feature_flags(ssh_key_expiration_email_notification: false)
+        end
+
+        it 'does not update notified column' do
+          expect { worker.perform }.not_to change { expired_today.reload.expiry_notification_delivered_at }
+        end
+      end
+    end
+
+    context 'when key has expired in the past' do
+      let_it_be(:expired_past) { create(:key, expires_at: 1.day.ago, user: user) }
+
+      it 'does not update notified column' do
+        expect { worker.perform }.not_to change { expired_past.reload.expiry_notification_delivered_at }
+      end
+    end
+  end
+end