Add latest changes from gitlab-org/gitlab@13-11-stable-eev13.11.0-rc43

6 files changed, 132 insertions, 22 deletions

diff --git a/spec/requests/api/graphql/mutations/boards/issues/issue_move_list_spec.rb b/spec/requests/api/graphql/mutations/boards/issues/issue_move_list_spec.rb
index e24ab0b07f2..46ec22e7ef8 100644
--- a/spec/requests/api/graphql/mutations/boards/issues/issue_move_list_spec.rb
+++ b/spec/requests/api/graphql/mutations/boards/issues/issue_move_list_spec.rb
@@ -21,7 +21,8 @@ RSpec.describe 'Reposition and move issue within board lists' do
   let(:mutation_name) { mutation_class.graphql_name }
   let(:mutation_result_identifier) { mutation_name.camelize(:lower) }
   let(:current_user) { user }
-  let(:params) { { board_id: board.to_global_id.to_s, project_path: project.full_path, iid: issue1.iid.to_s } }
+  let(:board_id) { global_id_of(board) }
+  let(:params) { { board_id: board_id, project_path: project.full_path, iid: issue1.iid.to_s } }
   let(:issue_move_params) do
     {
       from_list_id: list1.id,
@@ -34,16 +35,44 @@ RSpec.describe 'Reposition and move issue within board lists' do
   end
 
   shared_examples 'returns an error' do
-    it 'fails with error' do
-      message = "The resource that you are attempting to access does not exist or you don't have "\
-                "permission to perform this action"
+    let(:message) do
+      "The resource that you are attempting to access does not exist or you don't have " \
+        "permission to perform this action"
+    end
 
+    it 'fails with error' do
       post_graphql_mutation(mutation(params), current_user: current_user)
 
       expect(graphql_errors).to include(a_hash_including('message' => message))
     end
   end
 
+  context 'when the board_id is not a board' do
+    let(:board_id) { global_id_of(project) }
+    let(:issue_move_params) do
+      { move_after_id: existing_issue1.id, move_before_id: existing_issue2.id }
+    end
+
+    it_behaves_like 'returns an error' do
+      let(:message) { include('does not represent an instance of') }
+    end
+  end
+
+  # This test aims to distinguish between the failures to authorize
+  # :read_issue_board and :update_issue
+  context 'when the user cannot read the issue board' do
+    let(:issue_move_params) do
+      { move_after_id: existing_issue1.id, move_before_id: existing_issue2.id }
+    end
+
+    before do
+      allow(Ability).to receive(:allowed?).with(any_args).and_return(true)
+      allow(Ability).to receive(:allowed?).with(current_user, :read_issue_board, board).and_return(false)
+    end
+
+    it_behaves_like 'returns an error'
+  end
+
   context 'when user has access to resources' do
     context 'when repositioning an issue' do
       let(:issue_move_params) { { move_after_id: existing_issue1.id, move_before_id: existing_issue2.id } }
diff --git a/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb
index 97873b01338..bcede4d37dd 100644
--- a/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb
+++ b/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb
@@ -5,11 +5,12 @@ require 'spec_helper'
 RSpec.describe 'Setting assignees of a merge request' do
   include GraphqlHelpers
 
-  let(:current_user) { create(:user) }
-  let(:merge_request) { create(:merge_request) }
-  let(:project) { merge_request.project }
-  let(:assignee) { create(:user) }
-  let(:assignee2) { create(:user) }
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:current_user) { create(:user, developer_projects: [project]) }
+  let_it_be(:assignee) { create(:user) }
+  let_it_be(:assignee2) { create(:user) }
+  let_it_be_with_reload(:merge_request) { create(:merge_request, source_project: project) }
+
   let(:input) { { assignee_usernames: [assignee.username] } }
   let(:expected_result) do
     [{ 'username' => assignee.username }]
@@ -44,10 +45,19 @@ RSpec.describe 'Setting assignees of a merge request' do
     mutation_response['mergeRequest']['assignees']['nodes']
   end
 
+  def run_mutation!
+    recorder = ActiveRecord::QueryRecorder.new do
+      post_graphql_mutation(mutation, current_user: current_user)
+    end
+
+    expect(recorder.count).to be <= db_query_limit
+  end
+
   before do
-    project.add_developer(current_user)
     project.add_developer(assignee)
     project.add_developer(assignee2)
+
+    merge_request.update!(assignees: [])
   end
 
   it 'returns an error if the user is not allowed to update the merge request' do
@@ -56,23 +66,29 @@ RSpec.describe 'Setting assignees of a merge request' do
     expect(graphql_errors).not_to be_empty
   end
 
-  it 'does not allow members without the right permission to add assignees' do
-    user = create(:user)
-    project.add_guest(user)
+  context 'when the current user does not have permission to add assignees' do
+    let(:current_user) { create(:user) }
+    let(:db_query_limit) { 27 }
 
-    post_graphql_mutation(mutation, current_user: user)
+    it 'does not change the assignees' do
+      project.add_guest(current_user)
 
-    expect(graphql_errors).not_to be_empty
+      expect { run_mutation! }.not_to change { merge_request.reset.assignees.pluck(:id) }
+
+      expect(graphql_errors).not_to be_empty
+    end
   end
 
   context 'with assignees already assigned' do
+    let(:db_query_limit) { 39 }
+
     before do
       merge_request.assignees = [assignee2]
       merge_request.save!
     end
 
     it 'replaces the assignee' do
-      post_graphql_mutation(mutation, current_user: current_user)
+      run_mutation!
 
       expect(response).to have_gitlab_http_status(:success)
       expect(mutation_assignee_nodes).to match_array(expected_result)
@@ -80,6 +96,7 @@ RSpec.describe 'Setting assignees of a merge request' do
   end
 
   context 'when passing an empty list of assignees' do
+    let(:db_query_limit) { 31 }
     let(:input) { { assignee_usernames: [] } }
 
     before do
@@ -88,7 +105,7 @@ RSpec.describe 'Setting assignees of a merge request' do
     end
 
     it 'removes assignee' do
-      post_graphql_mutation(mutation, current_user: current_user)
+      run_mutation!
 
       expect(response).to have_gitlab_http_status(:success)
       expect(mutation_assignee_nodes).to eq([])
@@ -96,7 +113,9 @@ RSpec.describe 'Setting assignees of a merge request' do
   end
 
   context 'when passing append as true' do
-    let(:input) { { assignee_usernames: [assignee2.username], operation_mode: Types::MutationOperationModeEnum.enum[:append] } }
+    let(:mode) { Types::MutationOperationModeEnum.enum[:append] }
+    let(:input) { { assignee_usernames: [assignee2.username], operation_mode: mode } }
+    let(:db_query_limit) { 20 }
 
     before do
       # In CE, APPEND is a NOOP as you can't have multiple assignees
@@ -108,7 +127,7 @@ RSpec.describe 'Setting assignees of a merge request' do
     end
 
     it 'does not replace the assignee in CE' do
-      post_graphql_mutation(mutation, current_user: current_user)
+      run_mutation!
 
       expect(response).to have_gitlab_http_status(:success)
       expect(mutation_assignee_nodes).to match_array(expected_result)
@@ -116,7 +135,9 @@ RSpec.describe 'Setting assignees of a merge request' do
   end
 
   context 'when passing remove as true' do
-    let(:input) { { assignee_usernames: [assignee.username], operation_mode: Types::MutationOperationModeEnum.enum[:remove] } }
+    let(:db_query_limit) { 31 }
+    let(:mode) { Types::MutationOperationModeEnum.enum[:remove] }
+    let(:input) { { assignee_usernames: [assignee.username], operation_mode: mode } }
     let(:expected_result) { [] }
 
     before do
@@ -125,7 +146,7 @@ RSpec.describe 'Setting assignees of a merge request' do
     end
 
     it 'removes the users in the list, while adding none' do
-      post_graphql_mutation(mutation, current_user: current_user)
+      run_mutation!
 
       expect(response).to have_gitlab_http_status(:success)
       expect(mutation_assignee_nodes).to match_array(expected_result)
diff --git a/spec/requests/api/graphql/mutations/merge_requests/set_labels_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/set_labels_spec.rb
index 34d347c76fd..0d0cc66c52a 100644
--- a/spec/requests/api/graphql/mutations/merge_requests/set_labels_spec.rb
+++ b/spec/requests/api/graphql/mutations/merge_requests/set_labels_spec.rb
@@ -52,7 +52,7 @@ RSpec.describe 'Setting labels of a merge request' do
   end
 
   it 'sets the merge request labels, removing existing ones' do
-    merge_request.update(labels: [label2])
+    merge_request.update!(labels: [label2])
 
     post_graphql_mutation(mutation, current_user: current_user)
 
diff --git a/spec/requests/api/graphql/mutations/release_asset_links/delete_spec.rb b/spec/requests/api/graphql/mutations/release_asset_links/delete_spec.rb
new file mode 100644
index 00000000000..57489c82ec2
--- /dev/null
+++ b/spec/requests/api/graphql/mutations/release_asset_links/delete_spec.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Deletes a release asset link' do
+  include GraphqlHelpers
+
+  let_it_be(:project) { create(:project, :private, :repository) }
+  let_it_be(:release) { create(:release, project: project) }
+  let_it_be(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } }
+  let_it_be(:release_link) { create(:release_link, release: release) }
+
+  let(:current_user) { maintainer }
+  let(:mutation_name) { :release_asset_link_delete }
+  let(:mutation_arguments) { { id: release_link.to_global_id.to_s } }
+
+  let(:mutation) do
+    graphql_mutation(mutation_name, mutation_arguments, <<~FIELDS)
+      link {
+        id
+        name
+        url
+        linkType
+        directAssetUrl
+        external
+      }
+      errors
+    FIELDS
+  end
+
+  let(:delete_link) { post_graphql_mutation(mutation, current_user: current_user) }
+  let(:mutation_response) { graphql_mutation_response(mutation_name)&.with_indifferent_access }
+
+  it 'deletes the release asset link and returns the deleted link', :aggregate_failures do
+    delete_link
+
+    expected_response = {
+      id: release_link.to_global_id.to_s,
+      name: release_link.name,
+      url: release_link.url,
+      linkType: release_link.link_type.upcase,
+      directAssetUrl: end_with(release_link.filepath),
+      external: true
+    }.with_indifferent_access
+
+    expect(mutation_response[:link]).to match(expected_response)
+    expect(mutation_response[:errors]).to eq([])
+  end
+end
diff --git a/spec/requests/api/graphql/mutations/snippets/create_spec.rb b/spec/requests/api/graphql/mutations/snippets/create_spec.rb
index 1c2260070ec..d944c9e9e57 100644
--- a/spec/requests/api/graphql/mutations/snippets/create_spec.rb
+++ b/spec/requests/api/graphql/mutations/snippets/create_spec.rb
@@ -211,5 +211,9 @@ RSpec.describe 'Creating a Snippet' do
         end
       end
     end
+
+    it_behaves_like 'has spam protection' do
+      let(:mutation_class) { ::Mutations::Snippets::Create }
+    end
   end
 end
diff --git a/spec/requests/api/graphql/mutations/snippets/update_spec.rb b/spec/requests/api/graphql/mutations/snippets/update_spec.rb
index 43dc8d8bc44..28ab593526a 100644
--- a/spec/requests/api/graphql/mutations/snippets/update_spec.rb
+++ b/spec/requests/api/graphql/mutations/snippets/update_spec.rb
@@ -157,6 +157,9 @@ RSpec.describe 'Updating a Snippet' do
     it_behaves_like 'graphql update actions'
     it_behaves_like 'when the snippet is not found'
     it_behaves_like 'snippet edit usage data counters'
+    it_behaves_like 'has spam protection' do
+      let(:mutation_class) { ::Mutations::Snippets::Update }
+    end
   end
 
   describe 'ProjectSnippet' do
@@ -201,6 +204,10 @@ RSpec.describe 'Updating a Snippet' do
       end
 
       it_behaves_like 'snippet edit usage data counters'
+
+      it_behaves_like 'has spam protection' do
+        let(:mutation_class) { ::Mutations::Snippets::Update }
+      end
     end
 
     it_behaves_like 'when the snippet is not found'