From 5f80f7a4bf332c36b66f4d620bbc8b3e54d8f258 Mon Sep 17 00:00:00 2001
From: GitLab Bot <gitlab-bot@gitlab.com>
Date: Wed, 21 Oct 2020 06:09:17 +0000
Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master

---
 .../ide/components/jobs/detail.vue            |   9 +-
 .../javascripts/ide/components/jobs/item.vue  |   6 +-
 .../subscriptions/subscriptions.vue           |  12 +-
 app/assets/stylesheets/pages/projects.scss    | 164 ------------------
 app/helpers/defer_script_tag_helper.rb        |   4 +-
 app/models/project.rb                         |  27 ++-
 app/models/project_repository_storage_move.rb |  26 ++-
 .../hashed_storage/base_repository_service.rb |  11 +-
 .../migrate_repository_service.rb             |   6 +-
 app/views/shared/boards/_show.html.haml       |   3 +-
 ...fix-race-in-read-only-repository-logic.yml |   5 +
 .../229003-migrate-notifications-tooltip.yml  |   5 +
 doc/user/application_security/index.md        |   8 +
 locale/gitlab.pot                             |   9 +
 spec/frontend/sidebar/subscriptions_spec.js   |   2 +-
 spec/models/project_spec.rb                   |  17 +-
 .../merge_requests/show.html.haml_spec.rb     |   2 +-
 17 files changed, 106 insertions(+), 210 deletions(-)
 create mode 100644 changelogs/unreleased/220104-fix-race-in-read-only-repository-logic.yml
 create mode 100644 changelogs/unreleased/229003-migrate-notifications-tooltip.yml

diff --git a/app/assets/javascripts/ide/components/jobs/detail.vue b/app/assets/javascripts/ide/components/jobs/detail.vue
index a5ae8bbfe9a..d65304034c2 100644
--- a/app/assets/javascripts/ide/components/jobs/detail.vue
+++ b/app/assets/javascripts/ide/components/jobs/detail.vue
@@ -2,7 +2,7 @@
 /* eslint-disable vue/no-v-html */
 import { mapActions, mapState } from 'vuex';
 import { throttle } from 'lodash';
-import { GlIcon, GlTooltipDirective } from '@gitlab/ui';
+import { GlTooltipDirective, GlButton, GlIcon } from '@gitlab/ui';
 import { __ } from '../../../locale';
 import ScrollButton from './detail/scroll_button.vue';
 import JobDescription from './detail/description.vue';
@@ -17,6 +17,7 @@ export default {
     GlTooltip: GlTooltipDirective,
   },
   components: {
+    GlButton,
     GlIcon,
     ScrollButton,
     JobDescription,
@@ -75,9 +76,9 @@ export default {
 <template>
   <div class="ide-pipeline build-page d-flex flex-column flex-fill">
     <header class="ide-job-header d-flex align-items-center">
-      <button class="btn btn-default btn-sm d-flex" @click="setDetailJob(null)">
-        <gl-icon name="chevron-left" /> {{ __('View jobs') }}
-      </button>
+      <gl-button category="secondary" icon="chevron-left" size="small" @click="setDetailJob(null)">
+        {{ __('View jobs') }}
+      </gl-button>
     </header>
     <div class="top-bar d-flex border-left-0 mr-3">
       <job-description :job="detailJob" />
diff --git a/app/assets/javascripts/ide/components/jobs/item.vue b/app/assets/javascripts/ide/components/jobs/item.vue
index db3630bc1d1..f84315b63d2 100644
--- a/app/assets/javascripts/ide/components/jobs/item.vue
+++ b/app/assets/javascripts/ide/components/jobs/item.vue
@@ -1,9 +1,11 @@
 <script>
+import { GlButton } from '@gitlab/ui';
 import JobDescription from './detail/description.vue';
 
 export default {
   components: {
     JobDescription,
+    GlButton,
   },
   props: {
     job: {
@@ -28,9 +30,9 @@ export default {
   <div class="ide-job-item">
     <job-description :job="job" class="gl-mr-3" />
     <div class="ml-auto align-self-center">
-      <button v-if="job.started" type="button" class="btn btn-default btn-sm" @click="clickViewLog">
+      <gl-button v-if="job.started" category="secondary" size="small" @click="clickViewLog">
         {{ __('View log') }}
-      </button>
+      </gl-button>
     </div>
   </div>
 </template>
diff --git a/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue b/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue
index 0457aad8795..6e004084077 100644
--- a/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue
+++ b/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue
@@ -1,9 +1,8 @@
 <script>
-import { GlIcon } from '@gitlab/ui';
+import { GlIcon, GlTooltipDirective } from '@gitlab/ui';
 import { __ } from '~/locale';
 import Tracking from '~/tracking';
 import toggleButton from '~/vue_shared/components/toggle_button.vue';
-import tooltip from '~/vue_shared/directives/tooltip';
 import eventHub from '../../event_hub';
 
 const ICON_ON = 'notifications';
@@ -13,7 +12,7 @@ const LABEL_OFF = __('Notifications off');
 
 export default {
   directives: {
-    tooltip,
+    GlTooltip: GlTooltipDirective,
   },
   components: {
     GlIcon,
@@ -110,12 +109,9 @@ export default {
   <div>
     <span
       ref="tooltip"
-      v-tooltip
-      class="sidebar-collapsed-icon"
+      v-gl-tooltip.viewport.left
       :title="notificationTooltip"
-      data-container="body"
-      data-placement="left"
-      data-boundary="viewport"
+      class="sidebar-collapsed-icon"
       @click="onClickCollapsedIcon"
     >
       <gl-icon
diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss
index 938d8d34717..9f80d7b57ab 100644
--- a/app/assets/stylesheets/pages/projects.scss
+++ b/app/assets/stylesheets/pages/projects.scss
@@ -252,15 +252,6 @@
   }
 }
 
-.split-one {
-  display: inline-table;
-  margin-right: 12px;
-
-  > a {
-    margin: -1px;
-  }
-}
-
 .save-project-loader {
   margin-top: 50px;
   margin-bottom: 50px;
@@ -505,92 +496,6 @@
   }
 }
 
-.create-project-options {
-  display: flex;
-
-  @include media-breakpoint-down(xs) {
-    display: block;
-  }
-
-  .first-column {
-    @include media-breakpoint-up(xs) {
-      max-width: 50%;
-      padding-right: 30px;
-    }
-
-    @include media-breakpoint-down(xs) {
-      max-width: 100%;
-      width: 100%;
-    }
-  }
-
-  .second-column {
-    @include media-breakpoint-up(xs) {
-      width: 50%;
-      flex: 1;
-      padding-left: 30px;
-      position: relative;
-    }
-
-    @include media-breakpoint-down(xs) {
-      max-width: 100%;
-      width: 100%;
-      padding-left: 0;
-      position: relative;
-    }
-
-    // Mobile
-    @include media-breakpoint-down(xs) {
-      padding-top: 30px;
-    }
-
-    &::before {
-      content: 'OR';
-      position: absolute;
-      left: -10px;
-      top: 50%;
-      z-index: 10;
-      padding: $gl-padding-8 0;
-      text-align: center;
-      background-color: $white;
-      color: $gl-text-color-tertiary;
-      transform: translateY(-50%);
-      font-size: 12px;
-      font-weight: $gl-font-weight-bold;
-      line-height: 20px;
-
-      // Mobile
-      @include media-breakpoint-down(xs) {
-        left: 50%;
-        top: 0;
-        transform: translateX(-50%);
-        padding: 0 $gl-padding-8;
-      }
-    }
-
-    &::after {
-      content: '';
-      position: absolute;
-      background-color: $border-color;
-      bottom: 0;
-      left: 0;
-      right: auto;
-      height: 100%;
-      width: 1px;
-      top: 0;
-
-      // Mobile
-      @include media-breakpoint-down(xs) {
-        top: 10px;
-        left: 10px;
-        right: 10px;
-        height: 1px;
-        width: auto;
-      }
-    }
-  }
-}
-
 .project-stats,
 .project-buttons {
   .scrolling-tabs-container {
@@ -754,17 +659,6 @@ pre.light-well {
   }
 }
 
-.project-footer {
-  margin-top: 20px;
-
-  .btn-remove {
-    @include btn-middle;
-    @include btn-red;
-
-    float: left !important;
-  }
-}
-
 /*
  * Projects list rendered on dashboard and user page
  */
@@ -1059,24 +953,6 @@ pre.light-well {
   }
 }
 
-.cannot-be-merged,
-.cannot-be-merged:hover {
-  color: $red-500;
-  margin-top: 2px;
-  position: relative;
-  z-index: 2;
-}
-
-.private-forks-notice .private-fork-icon {
-  i:nth-child(1) {
-    color: $green-600;
-  }
-
-  i:nth-child(2) {
-    color: $white;
-  }
-}
-
 .new-protected-branch,
 .new-protected-tag {
   label {
@@ -1233,34 +1109,6 @@ pre.light-well {
   }
 }
 
-.variables-table {
-  table-layout: fixed;
-
-  &.table-responsive {
-    border: 0;
-  }
-
-  .variable-key {
-    max-width: 120px;
-    overflow: hidden;
-    word-wrap: break-word;
-    white-space: nowrap;
-    text-overflow: ellipsis;
-  }
-
-  .variable-value {
-    max-width: 150px;
-    overflow: hidden;
-    word-wrap: break-word;
-    white-space: nowrap;
-    text-overflow: ellipsis;
-  }
-
-  .variable-menu {
-    text-align: right;
-  }
-}
-
 .services-installation-info .row {
   margin-bottom: 10px;
 }
@@ -1286,18 +1134,6 @@ pre.light-well {
   padding-bottom: 37px;
 }
 
-.project-ci-body {
-  .incorrect-syntax {
-    font-size: 18px;
-    color: $red-500;
-  }
-
-  .correct-syntax {
-    font-size: 18px;
-    color: $green-500;
-  }
-}
-
 .project-ci-linter {
   .ci-editor {
     height: 400px;
diff --git a/app/helpers/defer_script_tag_helper.rb b/app/helpers/defer_script_tag_helper.rb
index d91c6d52683..be927c67aaa 100644
--- a/app/helpers/defer_script_tag_helper.rb
+++ b/app/helpers/defer_script_tag_helper.rb
@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
 module DeferScriptTagHelper
-  # Override the default ActionView `javascript_include_tag` helper to support page specific deferred loading
+  # Override the default ActionView `javascript_include_tag` helper to support page specific deferred loading.
+  # PLEASE NOTE: `defer` is also critical so that we don't run JavaScript entrypoints before the DOM is ready.
+  # Please see https://gitlab.com/groups/gitlab-org/-/epics/4538#note_432159769.
   def javascript_include_tag(*sources)
     super(*sources, defer: true)
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index dbedd6d120c..08a9843dbbd 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -2090,21 +2090,36 @@ class Project < ApplicationRecord
     (auto_devops || build_auto_devops)&.predefined_variables
   end
 
-  # Tries to set repository as read_only, checking for existing Git transfers in progress beforehand
+  RepositoryReadOnlyError = Class.new(StandardError)
+
+  # Tries to set repository as read_only, checking for existing Git transfers in
+  # progress beforehand. Setting a repository read-only will fail if it is
+  # already in that state.
   #
-  # @return [Boolean] true when set to read_only or false when an existing git transfer is in progress
+  # @return nil. Failures will raise an exception
   def set_repository_read_only!
     with_lock do
-      break false if git_transfer_in_progress?
+      raise RepositoryReadOnlyError, _('Git transfer in progress') if
+        git_transfer_in_progress?
 
-      update_column(:repository_read_only, true)
+      raise RepositoryReadOnlyError, _('Repository already read-only') if
+        self.class.where(id: id).pick(:repository_read_only)
+
+      raise ActiveRecord::RecordNotSaved, _('Database update failed') unless
+        update_column(:repository_read_only, true)
+
+      nil
     end
   end
 
-  # Set repository as writable again
+  # Set repository as writable again. Unlike setting it read-only, this will
+  # succeed if the repository is already writable.
   def set_repository_writable!
     with_lock do
-      update_column(:repository_read_only, false)
+      raise ActiveRecord::RecordNotSaved, _('Database update failed') unless
+        update_column(:repository_read_only, false)
+
+      nil
     end
   end
 
diff --git a/app/models/project_repository_storage_move.rb b/app/models/project_repository_storage_move.rb
index 76f428fe925..3429dbe3a85 100644
--- a/app/models/project_repository_storage_move.rb
+++ b/app/models/project_repository_storage_move.rb
@@ -46,8 +46,15 @@ class ProjectRepositoryStorageMove < ApplicationRecord
       transition replicated: :cleanup_failed
     end
 
-    after_transition initial: :scheduled do |storage_move|
-      storage_move.project.update_column(:repository_read_only, true)
+    around_transition initial: :scheduled do |storage_move, block|
+      block.call
+
+      begin
+        storage_move.project.set_repository_read_only!
+      rescue => err
+        errors.add(:project, err.message)
+        next false
+      end
 
       storage_move.run_after_commit do
         ProjectUpdateRepositoryStorageWorker.perform_async(
@@ -56,17 +63,18 @@ class ProjectRepositoryStorageMove < ApplicationRecord
           storage_move.id
         )
       end
+
+      true
     end
 
-    after_transition started: :replicated do |storage_move|
-      storage_move.project.update_columns(
-        repository_read_only: false,
-        repository_storage: storage_move.destination_storage_name
-      )
+    before_transition started: :replicated do |storage_move|
+      storage_move.project.set_repository_writable!
+
+      storage_move.project.update_column(:repository_storage, storage_move.destination_storage_name)
     end
 
-    after_transition started: :failed do |storage_move|
-      storage_move.project.update_column(:repository_read_only, false)
+    before_transition started: :failed do |storage_move|
+      storage_move.project.set_repository_writable!
     end
 
     state :initial, value: 1
diff --git a/app/services/projects/hashed_storage/base_repository_service.rb b/app/services/projects/hashed_storage/base_repository_service.rb
index 065bf8725be..349d4d367be 100644
--- a/app/services/projects/hashed_storage/base_repository_service.rb
+++ b/app/services/projects/hashed_storage/base_repository_service.rb
@@ -79,13 +79,12 @@ module Projects
       end
 
       def try_to_set_repository_read_only!
-        # Mitigate any push operation to start during migration
-        unless project.set_repository_read_only!
-          migration_error = "Target repository '#{old_disk_path}' cannot be made read-only as there is a git transfer in progress"
-          logger.error migration_error
+        project.set_repository_read_only!
+      rescue Project::RepositoryReadOnlyError => err
+        migration_error = "Target repository '#{old_disk_path}' cannot be made read-only: #{err.message}"
+        logger.error migration_error
 
-          raise RepositoryInUseError, migration_error
-        end
+        raise RepositoryInUseError, migration_error
       end
 
       def wiki_path_suffix
diff --git a/app/services/projects/hashed_storage/migrate_repository_service.rb b/app/services/projects/hashed_storage/migrate_repository_service.rb
index fd62ac37d27..27f4efc2a8d 100644
--- a/app/services/projects/hashed_storage/migrate_repository_service.rb
+++ b/app/services/projects/hashed_storage/migrate_repository_service.rb
@@ -21,8 +21,10 @@ module Projects
           project.storage_version = nil
         end
 
-        project.repository_read_only = false
-        project.save!(validate: false)
+        project.transaction do
+          project.save!(validate: false)
+          project.set_repository_writable!
+        end
 
         if result && block_given?
           yield
diff --git a/app/views/shared/boards/_show.html.haml b/app/views/shared/boards/_show.html.haml
index c3137120034..9993342df38 100644
--- a/app/views/shared/boards/_show.html.haml
+++ b/app/views/shared/boards/_show.html.haml
@@ -15,9 +15,8 @@
   %script#js-board-modal-filter{ type: "text/x-template" }= render "shared/issuable/search_bar", type: :boards_modal, show_sorting_dropdown: false
   %script#js-board-promotion{ type: "text/x-template" }= render_if_exists "shared/promotions/promote_issue_board"
 
+= render 'shared/issuable/search_bar', type: :boards, board: board
 #board-app.boards-app.position-relative{ "v-cloak" => "true", data: board_data, ":class" => "{ 'is-compact': detailIssueVisible }" }
-  = render 'shared/issuable/search_bar', type: :boards, board: board
-
   - if Feature.enabled?(:boards_with_swimlanes, current_board_parent) || Feature.enabled?(:graphql_board_lists, current_board_parent)
     %board-content{ "v-cloak" => "true",
       "ref" => "board_content",
diff --git a/changelogs/unreleased/220104-fix-race-in-read-only-repository-logic.yml b/changelogs/unreleased/220104-fix-race-in-read-only-repository-logic.yml
new file mode 100644
index 00000000000..41a78821860
--- /dev/null
+++ b/changelogs/unreleased/220104-fix-race-in-read-only-repository-logic.yml
@@ -0,0 +1,5 @@
+---
+title: Fix a race condition checking whether a project is read-only
+merge_request: 45160
+author:
+type: fixed
diff --git a/changelogs/unreleased/229003-migrate-notifications-tooltip.yml b/changelogs/unreleased/229003-migrate-notifications-tooltip.yml
new file mode 100644
index 00000000000..71ca7ce9674
--- /dev/null
+++ b/changelogs/unreleased/229003-migrate-notifications-tooltip.yml
@@ -0,0 +1,5 @@
+---
+title: Migrate collapsed notification tooltip
+merge_request: 45453
+author: artychan
+type: other
diff --git a/doc/user/application_security/index.md b/doc/user/application_security/index.md
index 413a9f894e2..4e9b5ac36d8 100644
--- a/doc/user/application_security/index.md
+++ b/doc/user/application_security/index.md
@@ -566,3 +566,11 @@ Additionally, we provide a dedicated project containing the versioned legacy tem
 This can be useful for offline setups or anyone wishing to use [Auto DevOps](../../topics/autodevops/index.md).
 
 Instructions are available in the [legacy template project](https://gitlab.com/gitlab-org/auto-devops-v12-10).
+
+#### Vulnerabilities are found, but the job succeeds. How can I have a pipeline fail instead?
+
+This is the current default behavior, because the job's status indicates success or failure of the analyzer itself.
+Analyzer results are displayed in the [job logs](../../ci/pipelines/#expand-and-collapse-job-log-sections),
+[Merge Request widget](sast/index.md#overview)
+or [Security Dashboard](security_dashboard/index.md).
+There is [an open issue](https://gitlab.com/gitlab-org/gitlab/-/issues/235772) in which changes to this behavior are being discussed.
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 1c91540ee88..67c638a6827 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -8411,6 +8411,9 @@ msgstr ""
 msgid "Data is still calculating..."
 msgstr ""
 
+msgid "Database update failed"
+msgstr ""
+
 msgid "Datasource name not found"
 msgstr ""
 
@@ -12249,6 +12252,9 @@ msgstr ""
 msgid "Git strategy for pipelines"
 msgstr ""
 
+msgid "Git transfer in progress"
+msgstr ""
+
 msgid "Git version"
 msgstr ""
 
@@ -22297,6 +22303,9 @@ msgstr ""
 msgid "Repository Settings"
 msgstr ""
 
+msgid "Repository already read-only"
+msgstr ""
+
 msgid "Repository check"
 msgstr ""
 
diff --git a/spec/frontend/sidebar/subscriptions_spec.js b/spec/frontend/sidebar/subscriptions_spec.js
index dddb9c2bba9..428441656b3 100644
--- a/spec/frontend/sidebar/subscriptions_spec.js
+++ b/spec/frontend/sidebar/subscriptions_spec.js
@@ -94,7 +94,7 @@ describe('Subscriptions', () => {
 
     it('sets the correct display text', () => {
       expect(wrapper.find('.issuable-header-text').text()).toContain(subscribeDisabledDescription);
-      expect(wrapper.find({ ref: 'tooltip' }).attributes('data-original-title')).toBe(
+      expect(wrapper.find({ ref: 'tooltip' }).attributes('title')).toBe(
         subscribeDisabledDescription,
       );
     });
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 53a213891e9..8a0c5847394 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -3003,14 +3003,23 @@ RSpec.describe Project do
   describe '#set_repository_read_only!' do
     let(:project) { create(:project) }
 
-    it 'returns true when there is no existing git transfer in progress' do
-      expect(project.set_repository_read_only!).to be_truthy
+    it 'makes the repository read-only' do
+      expect { project.set_repository_read_only! }
+        .to change(project, :repository_read_only?)
+        .from(false)
+        .to(true)
     end
 
-    it 'returns false when there is an existing git transfer in progress' do
+    it 'raises an error if the project is already read-only' do
+      project.set_repository_read_only!
+
+      expect { project.set_repository_read_only! }.to raise_error(described_class::RepositoryReadOnlyError, /already read-only/)
+    end
+
+    it 'raises an error when there is an existing git transfer in progress' do
       allow(project).to receive(:git_transfer_in_progress?) { true }
 
-      expect(project.set_repository_read_only!).to be_falsey
+      expect { project.set_repository_read_only! }.to raise_error(described_class::RepositoryReadOnlyError, /in progress/)
     end
   end
 
diff --git a/spec/views/projects/merge_requests/show.html.haml_spec.rb b/spec/views/projects/merge_requests/show.html.haml_spec.rb
index 1acc07dabb6..9b4f2774c5b 100644
--- a/spec/views/projects/merge_requests/show.html.haml_spec.rb
+++ b/spec/views/projects/merge_requests/show.html.haml_spec.rb
@@ -18,7 +18,7 @@ RSpec.describe 'projects/merge_requests/show.html.haml' do
 
         render
 
-        expect(rendered).not_to have_css('.cannot-be-merged')
+        expect(rendered).not_to have_css('.merge-icon')
       end
     end
   end