Add latest changes from gitlab-org/gitlab@master

16 files changed, 98 insertions, 244 deletions

diff --git a/app/assets/javascripts/error_tracking/components/error_details.vue b/app/assets/javascripts/error_tracking/components/error_details.vue
index 9cf141589db..14b2e59009a 100644
--- a/app/assets/javascripts/error_tracking/components/error_details.vue
+++ b/app/assets/javascripts/error_tracking/components/error_details.vue
@@ -118,13 +118,17 @@ export default {
     <div v-if="loading" class="py-3">
       <gl-loading-icon :size="3" />
     </div>
-
     <div v-else-if="showDetails" class="error-details">
       <div class="top-area align-items-center justify-content-between py-3">
         <span v-if="!loadingStacktrace && stacktrace" v-html="reported"></span>
         <form ref="sentryIssueForm" :action="projectIssuesPath" method="POST">
           <gl-form-input class="hidden" name="issue[title]" :value="issueTitle" />
           <input name="issue[description]" :value="issueDescription" type="hidden" />
+          <gl-form-input
+            :value="error.id"
+            class="hidden"
+            name="issue[sentry_issue_attributes][sentry_issue_identifier]"
+          />
           <gl-form-input :value="csrfToken" class="hidden" name="authenticity_token" />
           <loading-button
             v-if="!error.gitlab_issue"
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 5cbfabebe39..229374c3929 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -237,7 +237,10 @@ class Projects::IssuesController < Projects::ApplicationController
   end
 
   def issue_params
-    params.require(:issue).permit(*issue_params_attributes)
+    params.require(:issue).permit(
+      *issue_params_attributes,
+      sentry_issue_attributes: [:sentry_issue_identifier]
+    )
   end
 
   def issue_params_attributes
diff --git a/app/models/issue.rb b/app/models/issue.rb
index d49f3c28000..f16fd8e63ec 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -45,6 +45,8 @@ class Issue < ApplicationRecord
   has_many :user_mentions, class_name: "IssueUserMention"
   has_one :sentry_issue
 
+  accepts_nested_attributes_for :sentry_issue
+
   validates :project, presence: true
 
   alias_attribute :parent_ids, :project_id
diff --git a/app/models/sentry_issue.rb b/app/models/sentry_issue.rb
index 678e09b5370..6be52f99562 100644
--- a/app/models/sentry_issue.rb
+++ b/app/models/sentry_issue.rb
@@ -4,5 +4,7 @@ class SentryIssue < ApplicationRecord
   belongs_to :issue
 
   validates :issue, uniqueness: true, presence: true
-  validates :sentry_issue_identifier, presence: true
+  validates :sentry_issue_identifier,
+    uniqueness: true,
+    presence: true
 end
diff --git a/doc/development/dangerbot.md b/doc/development/dangerbot.md
index 0b2e2b43512..40eb4294617 100644
--- a/doc/development/dangerbot.md
+++ b/doc/development/dangerbot.md
@@ -95,6 +95,12 @@ through in CI. However, you can speed these cycles up somewhat by emptying the
 `.gitlab/ci/rails.gitlab-ci.yml` file in your merge request. Just don't forget
 to revert the change before merging!
 
+To enable the Dangerfile on another existing GitLab project, run the following extra steps, based on [this procedure](https://danger.systems/guides/getting_started.html#creating-a-bot-account-for-danger-to-use):
+
+1. Add `@gitlab-bot` to the project as a `reporter`.
+1. Add the `@gitlab-bot`'s `GITLAB_API_PRIVATE_TOKEN` value as a value for a new CI/CD
+   variable named `DANGER_GITLAB_API_TOKEN`.
+
 You should add the `~Danger bot` label to the merge request before sending it
 for review.
 
diff --git a/doc/user/project/clusters/serverless/index.md b/doc/user/project/clusters/serverless/index.md
index 55bce10a49d..81aff05a32a 100644
--- a/doc/user/project/clusters/serverless/index.md
+++ b/doc/user/project/clusters/serverless/index.md
@@ -13,7 +13,7 @@ GitLab supports several ways deploy Serverless applications in both Kubernetes E
 
 Currently we support:
 
-- [Knative](#knative): Build Knative applications with Knative and `gitlabktl` on GKE.
+- [Knative](#knative): Build Knative applications with Knative and `gitlabktl` on GKE and EKS.
 - [AWS Lambda](aws.md): Create serverless applications via the Serverless Framework and GitLab CI.
 
 ## Knative
@@ -89,7 +89,7 @@ The minimum recommended cluster size to run Knative is 3-nodes, 6 vCPUs, and 22.
    for other platforms [Install kubectl](https://kubernetes.io/docs/tasks/tools/install-kubectl/).
 
 1. The Ingress is now available at this address and will route incoming requests to the proper service based on the DNS
-   name in the request. To support this, a wildcard DNS A record should be created for the desired domain name. For example,
+   name in the request. To support this, a wildcard DNS record should be created for the desired domain name. For example,
    if your Knative base domain is `knative.info` then you need to create an A record or CNAME record with domain `*.knative.info`
    pointing the ip address or hostname of the Ingress.
 
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 8a60127ff59..6d33cb214ee 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -188,7 +188,7 @@ module API
     end
 
     class BasicProjectDetails < ProjectIdentity
-      include ::API::ProjectsBatchCounting
+      include ::API::ProjectsRelationBuilder
 
       expose :default_branch, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) }
       # Avoids an N+1 query: https://github.com/mbleigh/acts-as-taggable-on/issues/91#issuecomment-168273770
@@ -430,7 +430,7 @@ module API
           options: { only_owned: true, limit: projects_limit }
         ).execute
 
-        Entities::Project.preload_and_batch_count!(projects)
+        Entities::Project.prepare_relation(projects)
       end
 
       expose :shared_projects, using: Entities::Project do |group, options|
@@ -440,7 +440,7 @@ module API
           options: { only_shared: true, limit: projects_limit }
         ).execute
 
-        Entities::Project.preload_and_batch_count!(projects)
+        Entities::Project.prepare_relation(projects)
       end
 
       def projects_limit
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index b9cfc16fb23..6c88b61eee8 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -231,7 +231,7 @@ module API
 
         projects, options = with_custom_attributes(projects, options)
 
-        present options[:with].preload_and_batch_count!(projects), options
+        present options[:with].prepare_relation(projects), options
       end
 
       desc 'Get a list of subgroups in this group.' do
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index ea7087d2f46..a1fce9e8b20 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -75,17 +75,15 @@ module API
         mutually_exclusive :import_url, :template_name, :template_project_id
       end
 
-      def find_projects
+      def load_projects
         ProjectsFinder.new(current_user: current_user, params: project_finder_params).execute
       end
 
-      # Prepare the full projects query
-      # None of this is supposed to actually execute any database query
-      def prepare_query(projects)
+      def present_projects(projects, options = {})
         projects = reorder_projects(projects)
         projects = apply_filters(projects)
-
-        projects, options = with_custom_attributes(projects)
+        projects = paginate(projects)
+        projects, options = with_custom_attributes(projects, options)
 
         options = options.reverse_merge(
           with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails,
@@ -93,23 +91,9 @@ module API
           current_user: current_user,
           license: false
         )
-
         options[:with] = Entities::BasicProjectDetails if params[:simple]
 
-        projects = options[:with].preload_relation(projects, options)
-
-        [projects, options]
-      end
-
-      def prepare_and_present(project_relation)
-        projects, options = prepare_query(project_relation)
-
-        projects = paginate_and_retrieve!(projects)
-
-        # Refresh count caches
-        options[:with].execute_batch_counting(projects)
-
-        present projects, options
+        present options[:with].prepare_relation(projects, options), options
       end
 
       def translate_params_for_compatibility(params)
@@ -134,7 +118,7 @@ module API
 
         params[:user] = user
 
-        prepare_and_present find_projects
+        present_projects load_projects
       end
 
       desc 'Get projects starred by a user' do
@@ -150,7 +134,7 @@ module API
         not_found!('User') unless user
 
         starred_projects = StarredProjectsFinder.new(user, params: project_finder_params, current_user: current_user).execute
-        prepare_and_present starred_projects
+        present_projects starred_projects
       end
     end
 
@@ -166,7 +150,7 @@ module API
         use :with_custom_attributes
       end
       get do
-        prepare_and_present find_projects
+        present_projects load_projects
       end
 
       desc 'Create new project' do
@@ -303,7 +287,7 @@ module API
       get ':id/forks' do
         forks = ForkProjectsFinder.new(user_project, params: project_finder_params, current_user: current_user).execute
 
-        prepare_and_present forks
+        present_projects forks
       end
 
       desc 'Check pages access of this project'
diff --git a/lib/api/projects_batch_counting.rb b/lib/api/projects_batch_counting.rb
deleted file mode 100644
index 4e5124c8791..00000000000
--- a/lib/api/projects_batch_counting.rb
+++ /dev/null
@@ -1,27 +0,0 @@
-# frozen_string_literal: true
-
-module API
-  module ProjectsBatchCounting
-    extend ActiveSupport::Concern
-
-    class_methods do
-      # This adds preloading to the query and executes batch counting
-      # Side-effect: The query will be executed during batch counting
-      def preload_and_batch_count!(projects_relation)
-        preload_relation(projects_relation).tap do |projects|
-          execute_batch_counting(projects)
-        end
-      end
-
-      def execute_batch_counting(projects)
-        ::Projects::BatchForksCountService.new(forks_counting_projects(projects)).refresh_cache
-
-        ::Projects::BatchOpenIssuesCountService.new(projects).refresh_cache
-      end
-
-      def forks_counting_projects(projects)
-        projects
-      end
-    end
-  end
-end
diff --git a/lib/api/projects_relation_builder.rb b/lib/api/projects_relation_builder.rb
new file mode 100644
index 00000000000..263468c9aa6
--- /dev/null
+++ b/lib/api/projects_relation_builder.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module API
+  module ProjectsRelationBuilder
+    extend ActiveSupport::Concern
+
+    class_methods do
+      def prepare_relation(projects_relation, options = {})
+        projects_relation = preload_relation(projects_relation, options)
+        execute_batch_counting(projects_relation)
+        projects_relation
+      end
+
+      def preload_relation(projects_relation, options = {})
+        projects_relation
+      end
+
+      def forks_counting_projects(projects_relation)
+        projects_relation
+      end
+
+      def batch_forks_counting(projects_relation)
+        ::Projects::BatchForksCountService.new(forks_counting_projects(projects_relation)).refresh_cache
+      end
+
+      def batch_open_issues_counting(projects_relation)
+        ::Projects::BatchOpenIssuesCountService.new(projects_relation).refresh_cache
+      end
+
+      def execute_batch_counting(projects_relation)
+        batch_forks_counting(projects_relation)
+        batch_open_issues_counting(projects_relation)
+      end
+    end
+  end
+end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 51d5a0d2144..e4e2487f1e1 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1073,6 +1073,24 @@ describe Projects::IssuesController do
         expect(issue.time_estimate).to eq(7200)
       end
     end
+
+    context 'when created from sentry error' do
+      subject { post_new_issue(sentry_issue_attributes: { sentry_issue_identifier: 1234567 }) }
+
+      it 'creates an issue' do
+        expect { subject }.to change(Issue, :count)
+      end
+
+      it 'creates a sentry issue' do
+        expect { subject }.to change(SentryIssue, :count)
+      end
+
+      it 'with existing issue it will not create an issue' do
+        post_new_issue(sentry_issue_attributes: { sentry_issue_identifier: 1234567 })
+
+        expect { subject }.not_to change(Issue, :count)
+      end
+    end
   end
 
   describe 'POST #mark_as_spam' do
diff --git a/spec/frontend/error_tracking/components/error_details_spec.js b/spec/frontend/error_tracking/components/error_details_spec.js
index 632ed373545..6dc4980aaec 100644
--- a/spec/frontend/error_tracking/components/error_details_spec.js
+++ b/spec/frontend/error_tracking/components/error_details_spec.js
@@ -110,7 +110,7 @@ describe('ErrorDetails', () => {
       beforeEach(() => {
         store.state.details.loading = false;
         store.state.details.error = {
-          id: 1,
+          id: 129381,
           title: 'Issue title',
           external_url: 'http://sentry.gitlab.net/gitlab',
           first_seen: '2017-05-26T13:32:48Z',
@@ -121,6 +121,13 @@ describe('ErrorDetails', () => {
         mountComponent();
       });
 
+      it('should send sentry_issue_identifier', () => {
+        const sentryErrorIdInput = wrapper.find(
+          'glforminput-stub[name="issue[sentry_issue_attributes][sentry_issue_identifier]"',
+        );
+        expect(sentryErrorIdInput.attributes('value')).toBe('129381');
+      });
+
       it('should set the form values with title and description', () => {
         const csrfTokenInput = wrapper.find('glforminput-stub[name="authenticity_token"]');
         const issueTitleInput = wrapper.find('glforminput-stub[name="issue[title]"]');
diff --git a/spec/lib/api/projects_batch_counting_spec.rb b/spec/lib/api/projects_batch_counting_spec.rb
deleted file mode 100644
index 6094952bb52..00000000000
--- a/spec/lib/api/projects_batch_counting_spec.rb
+++ /dev/null
@@ -1,72 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe API::ProjectsBatchCounting do
-  subject do
-    Class.new do
-      include ::API::ProjectsBatchCounting
-    end
-  end
-
-  describe '.preload_and_batch_count!' do
-    let(:projects) { double }
-    let(:preloaded_projects) { double }
-
-    it 'preloads the relation' do
-      allow(subject).to receive(:execute_batch_counting).with(preloaded_projects)
-
-      expect(subject).to receive(:preload_relation).with(projects).and_return(preloaded_projects)
-
-      expect(subject.preload_and_batch_count!(projects)).to eq(preloaded_projects)
-    end
-
-    it 'executes batch counting' do
-      allow(subject).to receive(:preload_relation).with(projects).and_return(preloaded_projects)
-
-      expect(subject).to receive(:execute_batch_counting).with(preloaded_projects)
-
-      subject.preload_and_batch_count!(projects)
-    end
-  end
-
-  describe '.execute_batch_counting' do
-    let(:projects) { create_list(:project, 2) }
-    let(:count_service) { double }
-
-    it 'counts forks' do
-      allow(::Projects::BatchForksCountService).to receive(:new).with(projects).and_return(count_service)
-
-      expect(count_service).to receive(:refresh_cache)
-
-      subject.execute_batch_counting(projects)
-    end
-
-    it 'counts open issues' do
-      allow(::Projects::BatchOpenIssuesCountService).to receive(:new).with(projects).and_return(count_service)
-
-      expect(count_service).to receive(:refresh_cache)
-
-      subject.execute_batch_counting(projects)
-    end
-
-    context 'custom fork counting' do
-      subject do
-        Class.new do
-          include ::API::ProjectsBatchCounting
-          def self.forks_counting_projects(projects)
-            [projects.first]
-          end
-        end
-      end
-
-      it 'counts forks for other projects' do
-        allow(::Projects::BatchForksCountService).to receive(:new).with([projects.first]).and_return(count_service)
-
-        expect(count_service).to receive(:refresh_cache)
-
-        subject.execute_batch_counting(projects)
-      end
-    end
-  end
-end
diff --git a/spec/models/sentry_issue_spec.rb b/spec/models/sentry_issue_spec.rb
index b3f7d2628db..48f9adf64af 100644
--- a/spec/models/sentry_issue_spec.rb
+++ b/spec/models/sentry_issue_spec.rb
@@ -13,5 +13,6 @@ describe SentryIssue do
     it { is_expected.to validate_presence_of(:issue) }
     it { is_expected.to validate_uniqueness_of(:issue) }
     it { is_expected.to validate_presence_of(:sentry_issue_identifier) }
+    it { is_expected.to validate_uniqueness_of(:sentry_issue_identifier).with_message("has already been taken") }
   end
 end
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 5161de6de5e..cda2dd7d2f4 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -155,35 +155,6 @@ describe API::Projects do
       project4
     end
 
-    # This is a regression spec for https://gitlab.com/gitlab-org/gitlab/issues/37919
-    context 'batch counting forks and open issues and refreshing count caches' do
-      # We expect to count these projects (only the ones on the first page, not all matching ones)
-      let(:projects) { Project.public_to_user(nil).order(id: :desc).first(per_page) }
-      let(:per_page) { 2 }
-      let(:count_service) { double }
-
-      before do
-        # Create more projects, so we have more than one page
-        create_list(:project, 5, :public)
-      end
-
-      it 'batch counts project forks' do
-        expect(::Projects::BatchForksCountService).to receive(:new).with(projects).and_return(count_service)
-        expect(count_service).to receive(:refresh_cache)
-
-        get api("/projects?per_page=#{per_page}")
-        expect(response.status).to eq 200
-      end
-
-      it 'batch counts open issues' do
-        expect(::Projects::BatchOpenIssuesCountService).to receive(:new).with(projects).and_return(count_service)
-        expect(count_service).to receive(:refresh_cache)
-
-        get api("/projects?per_page=#{per_page}")
-        expect(response.status).to eq 200
-      end
-    end
-
     context 'when unauthenticated' do
       it_behaves_like 'projects response' do
         let(:filter) { { search: project.name } }
@@ -599,87 +570,6 @@ describe API::Projects do
         let(:projects) { Project.all }
       end
     end
-
-    context 'with keyset pagination' do
-      let(:current_user) { user }
-      let(:projects) { [public_project, project, project2, project3] }
-
-      context 'headers and records' do
-        let(:params) { { pagination: 'keyset', order_by: :id, sort: :asc, per_page: 1 } }
-
-        it 'includes a pagination header with link to the next page' do
-          get api('/projects', current_user), params: params
-
-          expect(response.header).to include('Links')
-          expect(response.header['Links']).to include('pagination=keyset')
-          expect(response.header['Links']).to include("id_after=#{public_project.id}")
-        end
-
-        it 'contains only the first project with per_page = 1' do
-          get api('/projects', current_user), params: params
-
-          expect(response).to have_gitlab_http_status(200)
-          expect(json_response).to be_an Array
-          expect(json_response.map { |p| p['id'] }).to contain_exactly(public_project.id)
-        end
-
-        it 'does not include a link if the end has reached and there is no more data' do
-          get api('/projects', current_user), params: params.merge(id_after: project2.id)
-
-          expect(response.header).not_to include('Links')
-        end
-
-        it 'responds with 501 if order_by is different from id' do
-          get api('/projects', current_user), params: params.merge(order_by: :created_at)
-
-          expect(response).to have_gitlab_http_status(405)
-        end
-      end
-
-      context 'with descending sorting' do
-        let(:params) { { pagination: 'keyset', order_by: :id, sort: :desc, per_page: 1 } }
-
-        it 'includes a pagination header with link to the next page' do
-          get api('/projects', current_user), params: params
-
-          expect(response.header).to include('Links')
-          expect(response.header['Links']).to include('pagination=keyset')
-          expect(response.header['Links']).to include("id_before=#{project3.id}")
-        end
-
-        it 'contains only the last project with per_page = 1' do
-          get api('/projects', current_user), params: params
-
-          expect(response).to have_gitlab_http_status(200)
-          expect(json_response).to be_an Array
-          expect(json_response.map { |p| p['id'] }).to contain_exactly(project3.id)
-        end
-      end
-
-      context 'retrieving the full relation' do
-        let(:params) { { pagination: 'keyset', order_by: :id, sort: :desc, per_page: 2 } }
-
-        it 'returns all projects' do
-          url = '/projects'
-          requests = 0
-          ids = []
-
-          while url && requests <= 5 # circuit breaker
-            requests += 1
-            get api(url, current_user), params: params
-
-            links = response.header['Links']
-            url = links&.match(/<[^>]+(\/projects\?[^>]+)>; rel="next"/) do |match|
-              match[1]
-            end
-
-            ids += JSON.parse(response.body).map { |p| p['id'] }
-          end
-
-          expect(ids).to contain_exactly(*projects.map(&:id))
-        end
-      end
-    end
   end
 
   describe 'POST /projects' do