Add latest changes from gitlab-org/gitlab@master

lib/gitlab/checks/snippet_check.rb

@@ -20,14 +20,11 @@ module Gitlab
         @logger.append_message("Running checks for ref: #{@branch_name || @tag_name}")
       end
-      def exec
+      def validate!
         if creation? || deletion?
           raise GitAccess::ForbiddenError, ERROR_MESSAGES[:create_delete_branch]
         end
-        # TODO: https://gitlab.com/gitlab-org/gitlab/issues/205628
-        # Check operation will not result in more than one file in the repository
-
         true
       end

lib/gitlab/git_access_snippet.rb

@@ -97,9 +97,8 @@ module Gitlab
     end
     def check_single_change_access(change)
-      change_access = Checks::SnippetCheck.new(change, logger: logger)
-
-      change_access.exec
+      Checks::SnippetCheck.new(change, logger: logger).validate!
+      Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet::MAX_FILE_COUNT, logger: logger).validate!
     rescue Checks::TimedLogger::TimeoutError
       raise TimeoutError, logger.full_message
     end

qa/qa/runtime/browser.rb

@@ -44,11 +44,21 @@ module QA
         new.visit(address, page_class, &block)
       end
+      # rubocop: disable Metrics/AbcSize
       def self.configure!
         RSpec.configure do |config|
           config.define_derived_metadata(file_path: %r{/qa/specs/features/}) do |metadata|
             metadata[:type] = :feature
           end
+
+          config.around(:each) do |example|
+            example.run
+
+            if example.metadata[:screenshot]
+              screenshot = example.metadata[:screenshot][:image] || example.metadata[:screenshot][:html]
+              example.metadata[:stdout] = %{[[ATTACHMENT|#{screenshot}]]}
+            end
+          end
         end
         Capybara.server_port = 9887 + ENV['TEST_ENV_NUMBER'].to_i
@@ -140,6 +150,7 @@ module QA
           config.default_normalize_ws = true
         end
       end
+      # rubocop: enable Metrics/AbcSize
       class Session
         include Capybara::DSL

spec/features/projects/show/user_manages_notifications_spec.rb

@@ -7,7 +7,6 @@ describe 'Projects > Show > User manages notifications', :js do
   before do
     sign_in(project.owner)
-    visit project_path(project)
   end
   def click_notifications_button
@@ -15,6 +14,7 @@ describe 'Projects > Show > User manages notifications', :js do
   end
   it 'changes the notification setting' do
+    visit project_path(project)
     click_notifications_button
     click_link 'On mention'
@@ -26,6 +26,7 @@ describe 'Projects > Show > User manages notifications', :js do
   end
   it 'changes the notification setting to disabled' do
+    visit project_path(project)
     click_notifications_button
     click_link 'Disabled'
@@ -50,11 +51,13 @@ describe 'Projects > Show > User manages notifications', :js do
         :reassign_merge_request,
         :merge_merge_request,
         :failed_pipeline,
+        :fixed_pipeline,
         :success_pipeline
       ]
     end
     it 'shows notification settings checkbox' do
+      visit project_path(project)
       click_notifications_button
       page.find('a[data-notification-level="custom"]').click
@@ -64,12 +67,27 @@ describe 'Projects > Show > User manages notifications', :js do
         end
       end
     end
+
+    context 'when ci_pipeline_fixed_notifications is disabled' do
+      before do
+        stub_feature_flags(ci_pipeline_fixed_notifications: false)
+      end
+
+      it 'hides fixed_pipeline checkbox' do
+        visit project_path(project)
+        click_notifications_button
+        page.find('a[data-notification-level="custom"]').click
+
+        expect(page).not_to have_selector("input[name='notification_setting[fixed_pipeline]']")
+      end
+    end
   end
   context 'when project emails are disabled' do
     let(:project) { create(:project, :public, :repository, emails_disabled: true) }
     it 'is disabled' do
+      visit project_path(project)
       expect(page).to have_selector('.notifications-btn.disabled', visible: true)
     end
   end

spec/frontend/diffs/components/diff_file_row_spec.js

@@ -44,12 +44,14 @@ describe('Diff File Row component', () => {
       level: 4,
       file: {},
       hideFileStats: false,
+      showTooltip: true,
     });
     expect(wrapper.find(ChangedFileIcon).props()).toEqual(
       expect.objectContaining({
         file: {},
         size: 16,
+        showTooltip: true,
       }),
     );
   });

spec/frontend/vue_shared/components/changed_file_icon_spec.js

@@ -5,6 +5,7 @@ import Icon from '~/vue_shared/components/icon.vue';
 const changedFile = () => ({ changed: true });
 const stagedFile = () => ({ changed: true, staged: true });
 const newFile = () => ({ changed: true, tempFile: true });
+const deletedFile = () => ({ changed: false, tempFile: false, staged: false, deleted: true });
 const unchangedFile = () => ({ changed: false, tempFile: false, staged: false, deleted: false });
 describe('Changed file icon', () => {
@@ -58,6 +59,7 @@ describe('Changed file icon', () => {
     ${changedFile()} | ${'file-modified'}       | ${'Modified'} | ${'with file changed'}
     ${stagedFile()}  | ${'file-modified-solid'} | ${'Modified'} | ${'with file staged'}
     ${newFile()}     | ${'file-addition'}       | ${'Added'}    | ${'with file new'}
+    ${deletedFile()} | ${'file-deletion'}       | ${'Deleted'}  | ${'with file deleted'}
   `('$desc', ({ file, iconName, tooltipText }) => {
     beforeEach(() => {
       factory({ file });

spec/lib/gitlab/checks/push_file_count_check_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Checks::PushFileCountCheck do
+  let(:snippet) { create(:personal_snippet, :repository) }
+  let(:changes) { { oldrev: oldrev, newrev: newrev, ref: ref } }
+  let(:timeout) { Gitlab::GitAccess::INTERNAL_TIMEOUT }
+  let(:logger) { Gitlab::Checks::TimedLogger.new(timeout: timeout) }
+
+  subject { described_class.new(changes, repository: snippet.repository, limit: 1, logger: logger) }
+
+  describe '#validate!' do
+    using RSpec::Parameterized::TableSyntax
+
+    before do
+      allow(snippet.repository).to receive(:new_commits).and_return(
+        snippet.repository.commits_between(oldrev, newrev)
+      )
+    end
+
+    context 'initial creation' do
+      let(:oldrev) { Gitlab::Git::EMPTY_TREE_ID }
+      let(:newrev) { TestEnv::BRANCH_SHA["snippet/single-file"] }
+      let(:ref) { "refs/heads/snippet/single-file" }
+
+      it 'allows creation' do
+        expect { subject.validate! }.not_to raise_error
+      end
+    end
+
+    where(:old, :new, :valid, :message) do
+      'single-file' | 'edit-file'            | true  | nil
+      'single-file' | 'multiple-files'       | false | 'The repository can contain at most 1 file(s).'
+      'single-file' | 'no-files'             | false | 'The repository must contain at least 1 file.'
+      'edit-file'   | 'rename-and-edit-file' | true  | nil
+    end
+
+    with_them do
+      let(:oldrev) { TestEnv::BRANCH_SHA["snippet/#{old}"] }
+      let(:newrev) { TestEnv::BRANCH_SHA["snippet/#{new}"] }
+      let(:ref) { "refs/heads/snippet/#{new}" }
+
+      it "verifies" do
+        if valid
+          expect { subject.validate! }.not_to raise_error
+        else
+          expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, message)
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/checks/snippet_check_spec.rb

@@ -10,16 +10,16 @@ describe Gitlab::Checks::SnippetCheck do
   subject { Gitlab::Checks::SnippetCheck.new(changes, logger: logger) }
-  describe '#exec' do
+  describe '#validate!' do
     it 'does not raise any error' do
-      expect { subject.exec }.not_to raise_error
+      expect { subject.validate! }.not_to raise_error
     end
     context 'trying to delete the branch' do
       let(:newrev) { '0000000000000000000000000000000000000000' }
       it 'raises an error' do
-        expect { subject.exec }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can not create or delete branches.')
+        expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can not create or delete branches.')
       end
     end
@@ -28,14 +28,14 @@ describe Gitlab::Checks::SnippetCheck do
       let(:ref) { 'refs/heads/feature' }
       it 'raises an error' do
-        expect { subject.exec }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can not create or delete branches.')
+        expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, 'You can not create or delete branches.')
       end
       context "when branch is 'master'" do
         let(:ref) { 'refs/heads/master' }
         it "allows the operation" do
-          expect { subject.exec }.not_to raise_error
+          expect { subject.validate! }.not_to raise_error
         end
       end
     end

spec/lib/gitlab/git_access_snippet_spec.rb

@@ -188,12 +188,15 @@ describe Gitlab::GitAccessSnippet do
   end
   context 'when changes are specific' do
-    let(:changes) { 'oldrev newrev ref' }
+    let(:changes) { "2d1db523e11e777e49377cfb22d368deec3f0793 ddd0f15ae83993f5cb66a927a28673882e99100b master" }
     let(:user) { snippet.author }
     it 'does not raise error if SnippetCheck does not raise error' do
       expect_next_instance_of(Gitlab::Checks::SnippetCheck) do |check|
-        expect(check).to receive(:exec).and_call_original
+        expect(check).to receive(:validate!).and_call_original
+      end
+      expect_next_instance_of(Gitlab::Checks::PushFileCountCheck) do |check|
+        expect(check).to receive(:validate!)
       end
       expect { push_access_check }.not_to raise_error
@@ -201,7 +204,7 @@ describe Gitlab::GitAccessSnippet do
     it 'raises error if SnippetCheck raises error' do
       expect_next_instance_of(Gitlab::Checks::SnippetCheck) do |check|
-        allow(check).to receive(:exec).and_raise(Gitlab::GitAccess::ForbiddenError, 'foo')
+        allow(check).to receive(:validate!).and_raise(Gitlab::GitAccess::ForbiddenError, 'foo')
       end
       expect { push_access_check }.to raise_forbidden('foo')

spec/models/ci/pipeline_spec.rb

@@ -2509,27 +2509,53 @@ describe Ci::Pipeline, :mailer do
       end
     end
-    context 'with success pipeline' do
-      before do
-        perform_enqueued_jobs do
+    shared_examples 'enqueues the notification worker' do
+      it 'enqueues PipelineUpdateCiRefStatusWorker' do
+        expect(PipelineUpdateCiRefStatusWorker).to receive(:perform_async).with(pipeline.id)
+        expect(PipelineNotificationWorker).not_to receive(:perform_async).with(pipeline.id)
+
+        pipeline.succeed
+      end
+
+      context 'when ci_pipeline_fixed_notifications is disabled' do
+        before do
+          stub_feature_flags(ci_pipeline_fixed_notifications: false)
+        end
+
+        it 'enqueues PipelineNotificationWorker' do
+          expect(PipelineUpdateCiRefStatusWorker).not_to receive(:perform_async).with(pipeline.id)
+          expect(PipelineNotificationWorker).to receive(:perform_async).with(pipeline.id)
+
           pipeline.succeed
         end
       end
-      it_behaves_like 'sending a notification'
+    end
+    context 'with success pipeline' do
+      it_behaves_like 'sending a notification' do
+        before do
+          perform_enqueued_jobs do
+            pipeline.succeed
+          end
+        end
+      end
+
+      it_behaves_like 'enqueues the notification worker'
     end
     context 'with failed pipeline' do
-      before do
-        perform_enqueued_jobs do
-          create(:ci_build, :failed, pipeline: pipeline)
-          create(:generic_commit_status, :failed, pipeline: pipeline)
-          pipeline.drop
+      it_behaves_like 'sending a notification' do
+        before do
+          perform_enqueued_jobs do
+            create(:ci_build, :failed, pipeline: pipeline)
+            create(:generic_commit_status, :failed, pipeline: pipeline)
+            pipeline.drop
+          end
         end
       end
-      it_behaves_like 'sending a notification'
+      it_behaves_like 'enqueues the notification worker'
     end
     context 'with skipped pipeline' do

spec/requests/api/internal/base_spec.rb

@@ -12,6 +12,7 @@ describe API::Internal::Base do
   let_it_be(:personal_snippet) { create(:personal_snippet, :repository, author: user) }
   let_it_be(:project_snippet) { create(:project_snippet, :repository, author: user, project: project) }
+  let(:snippet_changes) { "#{TestEnv::BRANCH_SHA['snippet/single-file']} #{TestEnv::BRANCH_SHA['snippet/edit-file']} refs/heads/snippet/edit-file" }
   describe "GET /internal/check" do
     it do
@@ -336,7 +337,7 @@ describe API::Internal::Base do
       end
       context 'git push with personal snippet' do
-        subject { push(key, personal_snippet, env: env.to_json) }
+        subject { push(key, personal_snippet, env: env.to_json, changes: snippet_changes) }
         it 'responds with success' do
           subject
@@ -371,7 +372,7 @@ describe API::Internal::Base do
       end
       context 'git push with project snippet' do
-        subject { push(key, project_snippet, env: env.to_json) }
+        subject { push(key, project_snippet, env: env.to_json, changes: snippet_changes) }
         it 'responds with success' do
           subject
@@ -1104,9 +1105,11 @@ describe API::Internal::Base do
     )
   end
-  def push(key, container, protocol = 'ssh', env: nil)
+  def push(key, container, protocol = 'ssh', env: nil, changes: nil)
+    changes ||= 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master'
+
     params = {
-      changes: 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master',
+      changes: changes,
       key_id: key.id,
       project: full_path_for(container),
       gl_repository: gl_repository_for(container),

spec/support/capybara.rb

@@ -126,6 +126,12 @@ RSpec.configure do |config|
     Capybara.raise_server_errors = false
     example.run
+
+    if example.metadata[:screenshot]
+      screenshot = example.metadata[:screenshot][:image] || example.metadata[:screenshot][:html]
+      example.metadata[:stdout] = %{[[ATTACHMENT|#{screenshot}]]}
+    end
+
   ensure
     Capybara.raise_server_errors = true
   end

spec/support/helpers/test_env.rb

@@ -61,6 +61,11 @@ module TestEnv
     'merge-commit-analyze-before'        => '1adbdef',
     'merge-commit-analyze-side-branch'   => '8a99451',
     'merge-commit-analyze-after'         => '646ece5',
+    'snippet/single-file'                => '43e4080',
+    'snippet/multiple-files'             => 'b80faa8',
+    'snippet/rename-and-edit-file'       => '220a1e4',
+    'snippet/edit-file'                  => 'c2f074f',
+    'snippet/no-files'                   => '671aaa8',
     '2-mb-file'                          => 'bf12d25',
     'before-create-delete-modify-move'   => '845009f',
     'between-create-delete-modify-move'  => '3f5f443',