Add latest changes from gitlab-org/gitlab@master

15 files changed, 158 insertions, 94 deletions

diff --git a/app/models/concerns/resource_event_tools.rb b/app/models/resource_event.rb
index 7226b9573e1..9b3a211ad43 100644
--- a/app/models/concerns/resource_event_tools.rb
+++ b/app/models/resource_event.rb
@@ -1,16 +1,27 @@
 # frozen_string_literal: true
 
-module ResourceEventTools
-  extend ActiveSupport::Concern
+class ResourceEvent < ApplicationRecord
+  include Gitlab::Utils::StrongMemoize
+  include Importable
 
-  included do
-    belongs_to :user
+  self.abstract_class = true
 
-    validates :user, presence: { unless: :importing? }, on: :create
+  validates :user, presence: { unless: :importing? }, on: :create
 
-    validate :exactly_one_issuable
+  belongs_to :user
 
-    scope :created_after, ->(time) { where('created_at > ?', time) }
+  scope :created_after, ->(time) { where('created_at > ?', time) }
+
+  def discussion_id
+    strong_memoize(:discussion_id) do
+      Digest::SHA1.hexdigest(discussion_id_key.join("-"))
+    end
+  end
+
+  private
+
+  def discussion_id_key
+    [self.class.name, created_at, user_id]
   end
 
   def exactly_one_issuable
diff --git a/app/models/resource_label_event.rb b/app/models/resource_label_event.rb
index 59907f1b962..970d4e1e562 100644
--- a/app/models/resource_label_event.rb
+++ b/app/models/resource_label_event.rb
@@ -1,10 +1,7 @@
 # frozen_string_literal: true
 
-class ResourceLabelEvent < ApplicationRecord
-  include Importable
-  include Gitlab::Utils::StrongMemoize
+class ResourceLabelEvent < ResourceEvent
   include CacheMarkdownField
-  include ResourceEventTools
 
   cache_markdown_field :reference
 
@@ -13,8 +10,11 @@ class ResourceLabelEvent < ApplicationRecord
   belongs_to :label
 
   scope :inc_relations, -> { includes(:label, :user) }
+  scope :by_issue, ->(issue) { where(issue_id: issue.id) }
+  scope :by_merge_request, ->(merge_request) { where(merge_request_id: merge_request.id) }
 
   validates :label, presence: { unless: :importing? }, on: :create
+  validate :exactly_one_issuable
 
   after_save :expire_etag_cache
   after_destroy :expire_etag_cache
@@ -41,12 +41,6 @@ class ResourceLabelEvent < ApplicationRecord
     issue || merge_request
   end
 
-  def discussion_id(resource = nil)
-    strong_memoize(:discussion_id) do
-      Digest::SHA1.hexdigest(discussion_id_key.join("-"))
-    end
-  end
-
   def project
     issuable.project
   end
@@ -109,10 +103,6 @@ class ResourceLabelEvent < ApplicationRecord
   def resource_parent
     issuable.project || issuable.group
   end
-
-  def discussion_id_key
-    [self.class.name, created_at, user_id]
-  end
 end
 
 ResourceLabelEvent.prepend_if_ee('EE::ResourceLabelEvent')
diff --git a/app/models/resource_milestone_event.rb b/app/models/resource_milestone_event.rb
index ba43a1ee363..d362ebc307a 100644
--- a/app/models/resource_milestone_event.rb
+++ b/app/models/resource_milestone_event.rb
@@ -1,10 +1,6 @@
 # frozen_string_literal: true
 
-class ResourceMilestoneEvent < ApplicationRecord
-  include Gitlab::Utils::StrongMemoize
-  include Importable
-  include ResourceEventTools
-
+class ResourceMilestoneEvent < ResourceEvent
   belongs_to :issue
   belongs_to :merge_request
   belongs_to :milestone
@@ -12,6 +8,8 @@ class ResourceMilestoneEvent < ApplicationRecord
   scope :by_issue, ->(issue) { where(issue_id: issue.id) }
   scope :by_merge_request, ->(merge_request) { where(merge_request_id: merge_request.id) }
 
+  validate :exactly_one_issuable
+
   enum action: {
          add: 1,
          remove: 2
@@ -23,8 +21,4 @@ class ResourceMilestoneEvent < ApplicationRecord
   def self.issuable_attrs
     %i(issue merge_request).freeze
   end
-
-  def resource
-    issue || merge_request
-  end
 end
diff --git a/app/models/resource_weight_event.rb b/app/models/resource_weight_event.rb
index ab288798aed..e0cc0c87a83 100644
--- a/app/models/resource_weight_event.rb
+++ b/app/models/resource_weight_event.rb
@@ -1,26 +1,9 @@
 # frozen_string_literal: true
 
-class ResourceWeightEvent < ApplicationRecord
-  include Gitlab::Utils::StrongMemoize
-
-  validates :user, presence: true
+class ResourceWeightEvent < ResourceEvent
   validates :issue, presence: true
 
-  belongs_to :user
   belongs_to :issue
 
   scope :by_issue, ->(issue) { where(issue_id: issue.id) }
-  scope :created_after, ->(time) { where('created_at > ?', time) }
-
-  def discussion_id(resource = nil)
-    strong_memoize(:discussion_id) do
-      Digest::SHA1.hexdigest(discussion_id_key.join("-"))
-    end
-  end
-
-  private
-
-  def discussion_id_key
-    [self.class.name, created_at, user_id]
-  end
 end
diff --git a/changelogs/unreleased/207126-more-descriptive-error-messages-in-migration-helpers.yml b/changelogs/unreleased/207126-more-descriptive-error-messages-in-migration-helpers.yml
new file mode 100644
index 00000000000..36ed216a505
--- /dev/null
+++ b/changelogs/unreleased/207126-more-descriptive-error-messages-in-migration-helpers.yml
@@ -0,0 +1,5 @@
+---
+title: Improve error messages of failed migrations
+merge_request: 25457
+author:
+type: changed
diff --git a/doc/api/scim.md b/doc/api/scim.md
index cdd635d0627..eaa56b0d0dd 100644
--- a/doc/api/scim.md
+++ b/doc/api/scim.md
@@ -122,7 +122,7 @@ Parameters:
 | `userName`   | string      | yes | Username of the user. |
 | `emails`     | JSON string | yes | Work email. |
 | `name`       | JSON string | yes | Name of the user. |
-| `meta`       | string      | no  | Resource type (`User'). |
+| `meta`       | string      | no  | Resource type (`User`). |
 
 Example request:
 
diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md
index 643ccd45898..c768c833e7c 100644
--- a/doc/ci/variables/README.md
+++ b/doc/ci/variables/README.md
@@ -571,9 +571,12 @@ Below you can find supported syntax reference:
    - `$VARIABLE =~ /^content.*/`
    - `$VARIABLE_1 !~ /^content.*/` (introduced in GitLab 11.11)
 
-   It is possible perform pattern matching against a variable and regular
-   expression. Expression like this evaluates to truth if matches are found
-   when using `=~`. It evaluates to truth if matches are not found when `!~` is used.
+   Variable pattern matching with regular expressions uses the
+   [RE2 regular expression syntax](https://github.com/google/re2/wiki/Syntax).
+   Expressions evaluate as `true` if:
+
+   - Matches are found when using `=~`.
+   - Matches are *not* found when using `!~`.
 
    Pattern matching is case-sensitive by default. Use `i` flag modifier, like
    `/pattern/i` to make a pattern case-insensitive.
diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 8931ee43a8a..255ae3f7c13 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -857,7 +857,10 @@ In this example, if the first rule:
 
 `rules:if` differs slightly from `only:variables` by accepting only a single
 expression string, rather than an array of them. Any set of expressions to be
-evaluated should be conjoined into a single expression using `&&` or `||`. For example:
+evaluated should be conjoined into a single expression using `&&` or `||`, and use
+the [variable matching syntax](../variables/README.md#supported-syntax).
+
+For example:
 
 ```yaml
 job:
diff --git a/doc/user/project/deploy_tokens/index.md b/doc/user/project/deploy_tokens/index.md
index 728f09ca787..4479653417c 100644
--- a/doc/user/project/deploy_tokens/index.md
+++ b/doc/user/project/deploy_tokens/index.md
@@ -9,11 +9,11 @@ at midnight UTC and that they can be only managed by [maintainers](../../permiss
 
 ## Creating a Deploy Token
 
-You can create as many deploy tokens as you like from the settings of your project:
+You can create as many deploy tokens as you like from the settings of your project. Alternatively, you can also create [group-scoped deploy tokens](#group-deploy-token).
 
 1. Log in to your GitLab account.
-1. Go to the project you want to create Deploy Tokens for.
-1. Go to **Settings** > **Repository**.
+1. Go to the project (or group) you want to create Deploy Tokens for.
+1. Go to **{settings}** **Settings** > **CI / CD**.
 1. Click on "Expand" on **Deploy Tokens** section.
 1. Choose a name, expiry date (optional), and username (optional) for the token.
 1. Choose the [desired scopes](#limiting-scopes-of-a-deploy-token).
@@ -77,6 +77,22 @@ docker login -u <username> -p <deploy_token> registry.example.com
 Just replace `<username>` and `<deploy_token>` with the proper values. Then you can simply
 pull images from your Container Registry.
 
+### Group Deploy Token
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/21765) in GitLab 12.9.
+
+A deploy token created at the group level can be used across all projects that
+belong either to the specific group or to one of its subgroups.
+
+To use a group deploy token:
+
+1. [Create](#creating-a-deploy-token) a deploy token for a group.
+1. Use it the same way you use a project deploy token when
+   [cloning a repository](#git-clone-a-repository).
+
+The scopes applied to a group deploy token (such as `read_repository`) will
+apply consistently when cloning the repository of related projects.
+
 ### GitLab Deploy Token
 
 > [Introduced][ce-18414] in GitLab 10.8.
diff --git a/doc/user/search/index.md b/doc/user/search/index.md
index 70ab9af0bcc..407578fd4df 100644
--- a/doc/user/search/index.md
+++ b/doc/user/search/index.md
@@ -41,7 +41,7 @@ groups:
    - [Label](../project/labels.md)
    - My-reaction
    - Confidential
-   - Epic ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/195704) in GitLab 12.8)
+   - Epic ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/195704) in GitLab 12.9)
    - Search for this text
 1. Select or type the operator to use for filtering the attribute. The following operators are
    available:
diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb
index f75e943671b..82a84508959 100644
--- a/lib/gitlab/database/migration_helpers.rb
+++ b/lib/gitlab/database/migration_helpers.rb
@@ -215,7 +215,7 @@ module Gitlab
         fk_name = name || concurrent_foreign_key_name(source, column)
 
         unless foreign_key_exists?(source, name: fk_name)
-          raise "cannot find #{fk_name} on #{source} table"
+          raise missing_schema_object_message(source, "foreign key", fk_name)
         end
 
         disable_statement_timeout do
@@ -931,7 +931,10 @@ module Gitlab
       def column_for(table, name)
         name = name.to_s
 
-        columns(table).find { |column| column.name == name }
+        column = columns(table).find { |column| column.name == name }
+        raise(missing_schema_object_message(table, "column", name)) if column.nil?
+
+        column
       end
 
       # This will replace the first occurrence of a string in a column with
@@ -1166,6 +1169,18 @@ into similar problems in the future (e.g. when new tables are created).
 
       private
 
+      def missing_schema_object_message(table, type, name)
+        <<~MESSAGE
+          Could not find #{type} "#{name}" on table "#{table}" which was referenced during the migration.
+          This issue could be caused by the database schema straying from the expected state.
+
+          To resolve this issue, please verify:
+            1. all previous migrations have completed
+            2. the database objects used in this migration match the Rails definition in schema.rb or structure.sql
+
+        MESSAGE
+      end
+
       def tables_match?(target_table, foreign_key_table)
         target_table.blank? || foreign_key_table == target_table
       end
diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb
index ce6e8c731e2..1fd6157ce43 100644
--- a/spec/lib/gitlab/database/migration_helpers_spec.rb
+++ b/spec/lib/gitlab/database/migration_helpers_spec.rb
@@ -383,7 +383,8 @@ describe Gitlab::Database::MigrationHelpers do
         it 'raises an error' do
           expect(model).to receive(:foreign_key_exists?).and_return(false)
 
-          expect { model.validate_foreign_key(:projects, :user_id) }.to raise_error(/cannot find/)
+          error_message = /Could not find foreign key "fk_name" on table "projects"/
+          expect { model.validate_foreign_key(:projects, :user_id, name: :fk_name) }.to raise_error(error_message)
         end
       end
     end
@@ -587,6 +588,8 @@ describe Gitlab::Database::MigrationHelpers do
   end
 
   describe '#add_column_with_default' do
+    let(:column) { Project.columns.find { |c| c.name == "id" } }
+
     context 'outside of a transaction' do
       context 'when a column limit is not set' do
         before do
@@ -601,6 +604,9 @@ describe Gitlab::Database::MigrationHelpers do
 
           expect(model).to receive(:change_column_default)
             .with(:projects, :foo, 10)
+
+          expect(model).to receive(:column_for)
+            .with(:projects, :foo).and_return(column)
         end
 
         it 'adds the column while allowing NULL values' do
@@ -655,6 +661,7 @@ describe Gitlab::Database::MigrationHelpers do
         it 'adds the column with a limit' do
           allow(model).to receive(:transaction_open?).and_return(false)
           allow(model).to receive(:transaction).and_yield
+          allow(model).to receive(:column_for).with(:projects, :foo).and_return(column)
           allow(model).to receive(:update_column_in_batches).with(:projects, :foo, 10)
           allow(model).to receive(:change_column_null).with(:projects, :foo, false)
           allow(model).to receive(:change_column_default).with(:projects, :foo, 10)
@@ -721,50 +728,68 @@ describe Gitlab::Database::MigrationHelpers do
 
       before do
         allow(model).to receive(:transaction_open?).and_return(false)
-        allow(model).to receive(:column_for).and_return(old_column)
       end
 
-      it 'renames a column concurrently' do
-        expect(model).to receive(:check_trigger_permissions!).with(:users)
+      context 'when the column to rename exists' do
+        before do
+          allow(model).to receive(:column_for).and_return(old_column)
+        end
 
-        expect(model).to receive(:install_rename_triggers_for_postgresql)
-          .with(trigger_name, '"users"', '"old"', '"new"')
+        it 'renames a column concurrently' do
+          expect(model).to receive(:check_trigger_permissions!).with(:users)
 
-        expect(model).to receive(:add_column)
-          .with(:users, :new, :integer,
-               limit: old_column.limit,
-               precision: old_column.precision,
-               scale: old_column.scale)
+          expect(model).to receive(:install_rename_triggers_for_postgresql)
+            .with(trigger_name, '"users"', '"old"', '"new"')
 
-        expect(model).to receive(:change_column_default)
-          .with(:users, :new, old_column.default)
+          expect(model).to receive(:add_column)
+            .with(:users, :new, :integer,
+                 limit: old_column.limit,
+                 precision: old_column.precision,
+                 scale: old_column.scale)
 
-        expect(model).to receive(:update_column_in_batches)
+          expect(model).to receive(:change_column_default)
+            .with(:users, :new, old_column.default)
 
-        expect(model).to receive(:change_column_null).with(:users, :new, false)
+          expect(model).to receive(:update_column_in_batches)
 
-        expect(model).to receive(:copy_indexes).with(:users, :old, :new)
-        expect(model).to receive(:copy_foreign_keys).with(:users, :old, :new)
+          expect(model).to receive(:change_column_null).with(:users, :new, false)
+
+          expect(model).to receive(:copy_indexes).with(:users, :old, :new)
+          expect(model).to receive(:copy_foreign_keys).with(:users, :old, :new)
+
+          model.rename_column_concurrently(:users, :old, :new)
+        end
 
-        model.rename_column_concurrently(:users, :old, :new)
+        context 'when default is false' do
+          let(:old_column) do
+            double(:column,
+                 type: :boolean,
+                 limit: nil,
+                 default: false,
+                 null: false,
+                 precision: nil,
+                 scale: nil)
+          end
+
+          it 'copies the default to the new column' do
+            expect(model).to receive(:change_column_default)
+              .with(:users, :new, old_column.default)
+
+            model.rename_column_concurrently(:users, :old, :new)
+          end
+        end
       end
 
-      context 'when default is false' do
-        let(:old_column) do
-          double(:column,
-               type: :boolean,
-               limit: nil,
-               default: false,
-               null: false,
-               precision: nil,
-               scale: nil)
+      context 'when the column to be renamed does not exist' do
+        before do
+          allow(model).to receive(:columns).and_return([])
         end
 
-        it 'copies the default to the new column' do
-          expect(model).to receive(:change_column_default)
-            .with(:users, :new, old_column.default)
+        it 'raises an error with appropriate message' do
+          expect(model).to receive(:check_trigger_permissions!).with(:users)
 
-          model.rename_column_concurrently(:users, :old, :new)
+          error_message = /Could not find column "missing_column" on table "users"/
+          expect { model.rename_column_concurrently(:users, :missing_column, :new) }.to raise_error(error_message)
         end
       end
     end
@@ -1133,8 +1158,9 @@ describe Gitlab::Database::MigrationHelpers do
       expect(column.name).to eq('id')
     end
 
-    it 'returns nil when a column does not exist' do
-      expect(model.column_for(:users, :kittens)).to be_nil
+    it 'raises an error when a column does not exist' do
+      error_message = /Could not find column "kittens" on table "users"/
+      expect { model.column_for(:users, :kittens) }.to raise_error(error_message)
     end
   end
 
diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb
index a92f5ee93e1..ca887b485a2 100644
--- a/spec/models/resource_label_event_spec.rb
+++ b/spec/models/resource_label_event_spec.rb
@@ -10,6 +10,10 @@ RSpec.describe ResourceLabelEvent, type: :model do
 
   it_behaves_like 'having unique enum values'
 
+  it_behaves_like 'a resource event'
+  it_behaves_like 'a resource event for issues'
+  it_behaves_like 'a resource event for merge requests'
+
   describe 'associations' do
     it { is_expected.to belong_to(:user) }
     it { is_expected.to belong_to(:issue) }
diff --git a/spec/models/resource_weight_event_spec.rb b/spec/models/resource_weight_event_spec.rb
index 2f00204512e..11b633e1dcf 100644
--- a/spec/models/resource_weight_event_spec.rb
+++ b/spec/models/resource_weight_event_spec.rb
@@ -3,6 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe ResourceWeightEvent, type: :model do
+  it_behaves_like 'a resource event'
+  it_behaves_like 'a resource event for issues'
+
   let_it_be(:user1) { create(:user) }
   let_it_be(:user2) { create(:user) }
 
@@ -11,13 +14,11 @@ RSpec.describe ResourceWeightEvent, type: :model do
   let_it_be(:issue3) { create(:issue, author: user2) }
 
   describe 'validations' do
-    it { is_expected.not_to allow_value(nil).for(:user) }
     it { is_expected.not_to allow_value(nil).for(:issue) }
     it { is_expected.to allow_value(nil).for(:weight) }
   end
 
   describe 'associations' do
-    it { is_expected.to belong_to(:user) }
     it { is_expected.to belong_to(:issue) }
   end
 
diff --git a/spec/support/shared_examples/resource_events.rb b/spec/support/shared_examples/resource_events.rb
index d7e7349ad6c..963453666c9 100644
--- a/spec/support/shared_examples/resource_events.rb
+++ b/spec/support/shared_examples/resource_events.rb
@@ -10,8 +10,21 @@ shared_examples 'a resource event' do
   let_it_be(:issue2) { create(:issue, author: user1) }
   let_it_be(:issue3) { create(:issue, author: user2) }
 
+  describe 'importable' do
+    it { is_expected.to respond_to(:importing?) }
+    it { is_expected.to respond_to(:imported?) }
+  end
+
   describe 'validations' do
     it { is_expected.not_to allow_value(nil).for(:user) }
+
+    context 'when importing' do
+      before do
+        allow(subject).to receive(:importing?).and_return(true)
+      end
+
+      it { is_expected.to allow_value(nil).for(:user) }
+    end
   end
 
   describe 'associations' do