From 2d96e61ceb1a3f26283dfba43f85d99488752296 Mon Sep 17 00:00:00 2001
From: GitLab Bot <gitlab-bot@gitlab.com>
Date: Tue, 17 Dec 2019 21:07:52 +0000
Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master

---
 .../users/components/delete_user_modal.vue    |   2 +-
 .../clusters/clusters/gcp/_form.html.haml     |   2 +-
 ...-box-for-delete-user-and-contributions.yml |   5 +
 ...it-user-mentions-commit-id-column-type.yml |   5 +
 .../cloud_run_feature_enabled_by_default.yml  |   5 +
 ...mit_user_mentions_commit_id_column_type.rb |  36 +++++
 ..._mentions_commit_id_column_type_cleanup.rb |  25 ++++
 db/schema.rb                                  |   6 +-
 doc/administration/gitaly/praefect.md         |  70 ++++-----
 doc/api/issue_links.md                        |   2 +-
 .../components/delete_user_modal_spec.js      | 136 +++++++++++++-----
 11 files changed, 220 insertions(+), 74 deletions(-)
 create mode 100644 changelogs/unreleased/28365-bypass-confirmation-box-for-delete-user-and-contributions.yml
 create mode 100644 changelogs/unreleased/42738-change-commit-user-mentions-commit-id-column-type.yml
 create mode 100644 changelogs/unreleased/cloud_run_feature_enabled_by_default.yml
 create mode 100644 db/migrate/20191212140117_change_commit_user_mentions_commit_id_column_type.rb
 create mode 100644 db/post_migrate/20191212162434_change_commit_user_mentions_commit_id_column_type_cleanup.rb

diff --git a/app/assets/javascripts/pages/admin/users/components/delete_user_modal.vue b/app/assets/javascripts/pages/admin/users/components/delete_user_modal.vue
index 78aaa9df0ec..b43d6ba17d7 100644
--- a/app/assets/javascripts/pages/admin/users/components/delete_user_modal.vue
+++ b/app/assets/javascripts/pages/admin/users/components/delete_user_modal.vue
@@ -109,7 +109,7 @@ export default {
     <template>
       <p v-html="text"></p>
       <p v-html="confirmationTextLabel"></p>
-      <form ref="form" :action="deleteUserUrl" method="post">
+      <form ref="form" :action="deleteUserUrl" method="post" @submit.prevent>
         <input ref="method" type="hidden" name="_method" value="delete" />
         <input :value="csrfToken" type="hidden" name="authenticity_token" />
         <gl-form-input
diff --git a/app/views/clusters/clusters/gcp/_form.html.haml b/app/views/clusters/clusters/gcp/_form.html.haml
index 95670a2ec87..ab01569b8fd 100644
--- a/app/views/clusters/clusters/gcp/_form.html.haml
+++ b/app/views/clusters/clusters/gcp/_form.html.haml
@@ -64,7 +64,7 @@
       %p.form-text.text-muted
         = s_('ClusterIntegration|Learn more about %{help_link_start_machine_type}machine types%{help_link_end} and %{help_link_start_pricing}pricing%{help_link_end}.').html_safe % { help_link_start_machine_type: help_link_start % { url: machine_type_link_url }, help_link_start_pricing: help_link_start % { url: pricing_link_url }, help_link_end: help_link_end }
 
-    - if Feature.enabled?(:create_cloud_run_clusters, clusterable)
+    - if Feature.enabled?(:create_cloud_run_clusters, clusterable, default_enabled: true)
       .form-group
         = provider_gcp_field.check_box :cloud_run, { label: s_('ClusterIntegration|Enable Cloud Run on GKE (beta)'),
           label_class: 'label-bold' }
diff --git a/changelogs/unreleased/28365-bypass-confirmation-box-for-delete-user-and-contributions.yml b/changelogs/unreleased/28365-bypass-confirmation-box-for-delete-user-and-contributions.yml
new file mode 100644
index 00000000000..27a85ffe008
--- /dev/null
+++ b/changelogs/unreleased/28365-bypass-confirmation-box-for-delete-user-and-contributions.yml
@@ -0,0 +1,5 @@
+---
+title: Fix delete user dialog bypass caused by hitting enter
+merge_request: 17343
+author:
+type: fixed
diff --git a/changelogs/unreleased/42738-change-commit-user-mentions-commit-id-column-type.yml b/changelogs/unreleased/42738-change-commit-user-mentions-commit-id-column-type.yml
new file mode 100644
index 00000000000..cd6e2ba6770
--- /dev/null
+++ b/changelogs/unreleased/42738-change-commit-user-mentions-commit-id-column-type.yml
@@ -0,0 +1,5 @@
+---
+title: Change commit_id type on commit_user_mentions table
+merge_request: 21651
+author:
+type: fixed
diff --git a/changelogs/unreleased/cloud_run_feature_enabled_by_default.yml b/changelogs/unreleased/cloud_run_feature_enabled_by_default.yml
new file mode 100644
index 00000000000..5ad41ae95ac
--- /dev/null
+++ b/changelogs/unreleased/cloud_run_feature_enabled_by_default.yml
@@ -0,0 +1,5 @@
+---
+title: Re-enable the cloud run feature
+merge_request: https://gitlab.com/gitlab-org/gitlab/merge_requests/21762
+author:
+type: fixed
diff --git a/db/migrate/20191212140117_change_commit_user_mentions_commit_id_column_type.rb b/db/migrate/20191212140117_change_commit_user_mentions_commit_id_column_type.rb
new file mode 100644
index 00000000000..f30cdab3441
--- /dev/null
+++ b/db/migrate/20191212140117_change_commit_user_mentions_commit_id_column_type.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+class ChangeCommitUserMentionsCommitIdColumnType < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  OLD_INDEX = 'commit_user_mentions_on_commit_id_and_note_id_index'
+  OLD_TMP_INDEX = 'temp_commit_id_and_note_id_index'
+  NEW_TMP_INDEX = 'temp_commit_id_for_type_change_and_note_id_index'
+  NEW_INDEX = 'commit_id_and_note_id_index'
+
+  def up
+    # the initial index name is too long and fails during migration. Renaming the index first.
+    add_concurrent_index :commit_user_mentions, [:commit_id, :note_id], name: OLD_TMP_INDEX
+    remove_concurrent_index_by_name :commit_user_mentions, OLD_INDEX
+
+    change_column_type_concurrently :commit_user_mentions, :commit_id, :string
+
+    # change_column_type_concurrently creates a new index for new column `commit_id_for_type` based on existing
+    # `temp_commit_id_and_note_id_index` naming it `temp_commit_id_for_type_change_and_note_id_index`, yet keeping
+    # `temp_commit_id_and_note_id_index` for `commit_id`, that will be cleaned
+    # by `cleanup_concurrent_column_type_change :commit_user_mentions, :commit_id` in a later migration.
+    #
+    # So we'll rename `temp_commit_id_for_type_change_and_note_id_index` to initialy intended name: `commit_id_and_note_id_index`.
+
+    add_concurrent_index :commit_user_mentions, [:commit_id_for_type_change, :note_id], name: NEW_INDEX
+    remove_concurrent_index_by_name :commit_user_mentions, NEW_TMP_INDEX
+  end
+
+  def down
+    cleanup_concurrent_column_type_change :commit_user_mentions, :commit_id
+  end
+end
diff --git a/db/post_migrate/20191212162434_change_commit_user_mentions_commit_id_column_type_cleanup.rb b/db/post_migrate/20191212162434_change_commit_user_mentions_commit_id_column_type_cleanup.rb
new file mode 100644
index 00000000000..aed9d335af9
--- /dev/null
+++ b/db/post_migrate/20191212162434_change_commit_user_mentions_commit_id_column_type_cleanup.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class ChangeCommitUserMentionsCommitIdColumnTypeCleanup < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  NEW_INDEX = 'commit_id_for_type_change_and_note_id_index'
+  OLD_INDEX = 'commit_user_mentions_on_commit_id_and_note_id_index'
+
+  def up
+    cleanup_concurrent_column_type_change :commit_user_mentions, :commit_id
+  end
+
+  def down
+    change_column_type_concurrently :commit_user_mentions, :commit_id, :binary
+
+    # change_column_type_concurrently creates a new index based on existing commit_id_and_note_id_index` naming it
+    # `commit_id_for_type_change_and_note_id_index` so we'll rename it back to its original name.
+    add_concurrent_index :commit_user_mentions, [:commit_id_for_type_change, :note_id], name: OLD_INDEX
+    remove_concurrent_index_by_name :commit_user_mentions, NEW_INDEX
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 67d305a3505..2422bb35cbd 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_12_17_160632) do
+ActiveRecord::Schema.define(version: 2019_12_16_183532) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -1217,11 +1217,11 @@ ActiveRecord::Schema.define(version: 2019_12_17_160632) do
 
   create_table "commit_user_mentions", force: :cascade do |t|
     t.integer "note_id", null: false
-    t.binary "commit_id", null: false
     t.integer "mentioned_users_ids", array: true
     t.integer "mentioned_projects_ids", array: true
     t.integer "mentioned_groups_ids", array: true
-    t.index ["commit_id", "note_id"], name: "commit_user_mentions_on_commit_id_and_note_id_index"
+    t.string "commit_id", null: false
+    t.index ["commit_id", "note_id"], name: "commit_id_and_note_id_index"
     t.index ["note_id"], name: "index_commit_user_mentions_on_note_id", unique: true
   end
 
diff --git a/doc/administration/gitaly/praefect.md b/doc/administration/gitaly/praefect.md
index 21a1e7ea691..6193a40ac4f 100644
--- a/doc/administration/gitaly/praefect.md
+++ b/doc/administration/gitaly/praefect.md
@@ -66,7 +66,7 @@ We need to manage the following secrets and make them match across hosts:
 #### Praefect
 
 On the Praefect node we disable all other services, including Gitaly. We list each
-Gitaly node that will be connected to Praefect under `praefect['storage_nodes']`.
+Gitaly node that will be connected to Praefect as members of the `praefect` hash in `praefect['virtual_storages']`.
 
 In the example below, the Gitaly nodes are named `gitaly-N`. Note that one
 node is designated as primary by setting the primary to `true`.
@@ -84,15 +84,6 @@ unicorn['enable'] = false
 sidekiq['enable'] = false
 gitlab_workhorse['enable'] = false
 gitaly['enable'] = false
-```
-
-##### Set up Praefect and its Gitaly nodes
-
-In the example below, the Gitaly nodes are named `gitaly-X`. Note that one node is designated as
-primary, by setting the primary to `true`:
-
-```ruby
-# /etc/gitlab/gitlab.rb on praefect server
 
 # Prevent database connections during 'gitlab-ctl reconfigure'
 gitlab_rails['rake_cache_clear'] = false
@@ -104,27 +95,27 @@ praefect['enable'] = true
 # firewalls to restrict access to this address/port.
 praefect['listen_addr'] = '0.0.0.0:2305'
 
-# virtual_storage_name must match the same storage name given to praefect in git_data_dirs
-praefect['virtual_storage_name'] = 'praefect'
-
 # Replace PRAEFECT_EXTERNAL_TOKEN with a real secret
 praefect['auth_token'] = 'PRAEFECT_EXTERNAL_TOKEN'
 
 # Replace each instance of PRAEFECT_INTERNAL_TOKEN below with a real
 # secret, distinct from PRAEFECT_EXTERNAL_TOKEN.
-praefect['storage_nodes'] = {
-  'gitaly-1' => {
-    'address' => 'tcp://gitaly-1.internal:8075',
-    'token'   => 'PRAEFECT_INTERNAL_TOKEN',
-    'primary' => true
-  },
-  'gitaly-2' => {
-    'address' => 'tcp://gitaly-2.internal:8075',
-    'token'   => 'PRAEFECT_INTERNAL_TOKEN'
-  },
-  'gitaly-3' => {
-    'address' => 'tcp://gitaly-3.internal:8075',
-    'token'   => 'PRAEFECT_INTERNAL_TOKEN'
+# Name of storage hash must match storage name in git_data_dirs on GitLab server.
+praefect['virtual_storages'] = {
+  'praefect' => {
+    'gitaly-1' => {
+      'address' => 'tcp://gitaly-1.internal:8075',
+      'token'   => 'PRAEFECT_INTERNAL_TOKEN',
+      'primary' => true
+    },
+    'gitaly-2' => {
+      'address' => 'tcp://gitaly-2.internal:8075',
+      'token'   => 'PRAEFECT_INTERNAL_TOKEN'
+    },
+    'gitaly-3' => {
+      'address' => 'tcp://gitaly-3.internal:8075',
+      'token'   => 'PRAEFECT_INTERNAL_TOKEN'
+    }
   }
 }
 ```
@@ -140,7 +131,7 @@ auth tokens from Praefect instead of GitLab.
 Below is an example configuration for `gitaly-1`, the only difference for the
 other Gitaly nodes is the storage name under `git_data_dirs`.
 
-Note that `gitaly['auth_token']` matches the `token` value listed under `praefect['storage_nodes']`
+Note that `gitaly['auth_token']` matches the `token` value listed under `praefect['virtual_storages']`
 on the Praefect node.
 
 ```ruby
@@ -155,6 +146,7 @@ grafana['enable'] = false
 unicorn['enable'] = false
 sidekiq['enable'] = false
 gitlab_workhorse['enable'] = false
+prometheus_monitoring['enable'] = false
 
 # Prevent database connections during 'gitlab-ctl reconfigure'
 gitlab_rails['rake_cache_clear'] = false
@@ -197,7 +189,7 @@ is present, there should be two storages available to GitLab:
 # Replace PRAEFECT_EXTERNAL_TOKEN below with real secret.
 git_data_dirs({
   "default" => {
-    "gitaly_address" => "tcp://gitaly.internal"
+    "path" => "/var/opt/gitlab/git-data"
   },
   "praefect" => {
     "gitaly_address" => "tcp://praefect.internal:2305",
@@ -212,7 +204,9 @@ gitlab_shell['secret_token'] = 'GITLAB_SHELL_SECRET_TOKEN'
 Note that the storage name used is the same as the `praefect['virtual_storage_name']` set
 on the Praefect node.
 
-Restart GitLab using `gitlab-ctl restart` on the GitLab node.
+Save your changes and [reconfigure GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure).
+
+Run `gitlab-rake gitlab:gitaly:check` to confirm that GitLab can reach Praefect.
 
 ### Testing Praefect
 
@@ -220,6 +214,18 @@ To test Praefect, first set it as the default storage node for new projects
 using **Admin Area > Settings > Repository > Repository storage**. Next,
 create a new project and check the "Initialize repository with a README" box.
 
-If you receive a 503 error, check `/var/log/gitlab/gitlab-rails/production.log`.
-A `GRPC::Unavailable (14:failed to connect to all addresses)` error indicates
-that GitLab was unable to connect to Praefect.
+If you receive an error, check `/var/log/gitlab/gitlab-rails/production.log`.
+
+Here are common errors and potential causes:
+
+- 500 response code
+  - **ActionView::Template::Error (7:permission denied)**
+    - `praefect['auth_token']` and `gitlab_rails['gitaly_token']` do not match on the GitLab server.
+  - **Unable to save project. Error: 7:permission denied**
+    - Secret token in `praefect['storage_nodes']` on GitLab server does not match the
+      value in `gitaly['auth_token']` on one or more Gitaly servers.
+- 503 response code
+  - **GRPC::Unavailable (14:failed to connect to all addresses)**
+    - GitLab was unable to reach Praefect.
+  - **GRPC::Unavailable (14:all SubCons are in TransientFailure...)**
+    - Praefect cannot reach one or more of its child Gitaly nodes.
diff --git a/doc/api/issue_links.md b/doc/api/issue_links.md
index 280431fa87c..b7e21310a19 100644
--- a/doc/api/issue_links.md
+++ b/doc/api/issue_links.md
@@ -57,7 +57,7 @@ Parameters:
 Creates a two-way relation between two issues. User must be allowed to update both issues in order to succeed.
 
 ```
-POST /projects/:id/issues/:issue_iid/links
+POST /projects/:id/issues/:issue_iid/links/:target_project_id/:target_issue_iid
 ```
 
 | Attribute   | Type    | Required | Description                          |
diff --git a/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js b/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js
index 57802a41bb5..3efebc69011 100644
--- a/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js
+++ b/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js
@@ -3,26 +3,46 @@ import { GlButton, GlFormInput } from '@gitlab/ui';
 import DeleteUserModal from '~/pages/admin/users/components/delete_user_modal.vue';
 import ModalStub from './stubs/modal_stub';
 
+const TEST_DELETE_USER_URL = 'delete-url';
+const TEST_BLOCK_USER_URL = 'block-url';
+const TEST_CSRF = 'csrf';
+
 describe('User Operation confirmation modal', () => {
   let wrapper;
+  let formSubmitSpy;
 
   const findButton = variant =>
     wrapper
       .findAll(GlButton)
       .filter(w => w.attributes('variant') === variant)
       .at(0);
+  const findForm = () => wrapper.find('form');
+  const findUsernameInput = () => wrapper.find(GlFormInput);
+  const findPrimaryButton = () => findButton('danger');
+  const findSecondaryButton = () => findButton('warning');
+  const findAuthenticityToken = () => new FormData(findForm().element).get('authenticity_token');
+  const getUsername = () => findUsernameInput().attributes('value');
+  const getMethodParam = () => new FormData(findForm().element).get('_method');
+  const getFormAction = () => findForm().attributes('action');
+
+  const setUsername = username => {
+    findUsernameInput().vm.$emit('input', username);
+  };
+
+  const username = 'username';
+  const badUsername = 'bad_username';
 
   const createComponent = (props = {}) => {
     wrapper = shallowMount(DeleteUserModal, {
       propsData: {
+        username,
         title: 'title',
         content: 'content',
         action: 'action',
         secondaryAction: 'secondaryAction',
-        deleteUserUrl: 'delete-url',
-        blockUserUrl: 'block-url',
-        username: 'username',
-        csrfToken: 'csrf',
+        deleteUserUrl: TEST_DELETE_USER_URL,
+        blockUserUrl: TEST_BLOCK_USER_URL,
+        csrfToken: TEST_CSRF,
         ...props,
       },
       stubs: {
@@ -32,6 +52,10 @@ describe('User Operation confirmation modal', () => {
     });
   };
 
+  beforeEach(() => {
+    formSubmitSpy = jest.spyOn(HTMLFormElement.prototype, 'submit').mockImplementation();
+  });
+
   afterEach(() => {
     wrapper.destroy();
     wrapper = null;
@@ -42,44 +66,84 @@ describe('User Operation confirmation modal', () => {
     expect(wrapper.element).toMatchSnapshot();
   });
 
-  it.each`
-    variant      | prop               | action
-    ${'danger'}  | ${'deleteUserUrl'} | ${'delete'}
-    ${'warning'} | ${'blockUserUrl'}  | ${'block'}
-  `('closing modal with $variant button triggers $action', ({ variant, prop }) => {
-    createComponent();
-    const form = wrapper.find('form');
-    jest.spyOn(form.element, 'submit').mockReturnValue();
-    const modalButton = findButton(variant);
-    modalButton.vm.$emit('click');
-    return wrapper.vm.$nextTick().then(() => {
-      expect(form.element.submit).toHaveBeenCalled();
-      expect(form.element.action).toContain(wrapper.props(prop));
-      expect(new FormData(form.element).get('authenticity_token')).toEqual(
-        wrapper.props('csrfToken'),
-      );
+  describe('on created', () => {
+    beforeEach(() => {
+      createComponent();
+    });
+
+    it('has disabled buttons', () => {
+      expect(findPrimaryButton().attributes('disabled')).toBeTruthy();
+      expect(findSecondaryButton().attributes('disabled')).toBeTruthy();
     });
   });
 
-  it('disables buttons by default', () => {
-    createComponent();
-    const blockButton = findButton('warning');
-    const deleteButton = findButton('danger');
-    expect(blockButton.attributes().disabled).toBeTruthy();
-    expect(deleteButton.attributes().disabled).toBeTruthy();
+  describe('with incorrect username', () => {
+    beforeEach(() => {
+      createComponent();
+      setUsername(badUsername);
+
+      return wrapper.vm.$nextTick();
+    });
+
+    it('shows incorrect username', () => {
+      expect(getUsername()).toEqual(badUsername);
+    });
+
+    it('has disabled buttons', () => {
+      expect(findPrimaryButton().attributes('disabled')).toBeTruthy();
+      expect(findSecondaryButton().attributes('disabled')).toBeTruthy();
+    });
   });
 
-  it('enables button when username is typed', () => {
-    createComponent({
-      username: 'some-username',
-    });
-    wrapper.find(GlFormInput).vm.$emit('input', 'some-username');
-    const blockButton = findButton('warning');
-    const deleteButton = findButton('danger');
+  describe('with correct username', () => {
+    beforeEach(() => {
+      createComponent();
+      setUsername(username);
 
-    return wrapper.vm.$nextTick().then(() => {
-      expect(blockButton.attributes().disabled).toBeFalsy();
-      expect(deleteButton.attributes().disabled).toBeFalsy();
+      return wrapper.vm.$nextTick();
+    });
+
+    it('shows correct username', () => {
+      expect(getUsername()).toEqual(username);
+    });
+
+    it('has enabled buttons', () => {
+      expect(findPrimaryButton().attributes('disabled')).toBeFalsy();
+      expect(findSecondaryButton().attributes('disabled')).toBeFalsy();
+    });
+
+    describe('when primary action is submitted', () => {
+      beforeEach(() => {
+        findPrimaryButton().vm.$emit('click');
+
+        return wrapper.vm.$nextTick();
+      });
+
+      it('clears the input', () => {
+        expect(getUsername()).toEqual('');
+      });
+
+      it('has correct form attributes and calls submit', () => {
+        expect(getFormAction()).toBe(TEST_DELETE_USER_URL);
+        expect(getMethodParam()).toBe('delete');
+        expect(findAuthenticityToken()).toBe(TEST_CSRF);
+        expect(formSubmitSpy).toHaveBeenCalled();
+      });
+    });
+
+    describe('when secondary action is submitted', () => {
+      beforeEach(() => {
+        findSecondaryButton().vm.$emit('click');
+
+        return wrapper.vm.$nextTick();
+      });
+
+      it('has correct form attributes and calls submit', () => {
+        expect(getFormAction()).toBe(TEST_BLOCK_USER_URL);
+        expect(getMethodParam()).toBe('put');
+        expect(findAuthenticityToken()).toBe(TEST_CSRF);
+        expect(formSubmitSpy).toHaveBeenCalled();
+      });
     });
   });
 });