diff options
Diffstat (limited to 'spec/requests/api')
66 files changed, 4380 insertions, 612 deletions
diff --git a/spec/requests/api/admin/ci/variables_spec.rb b/spec/requests/api/admin/ci/variables_spec.rb new file mode 100644 index 00000000000..bc2f0ba50a2 --- /dev/null +++ b/spec/requests/api/admin/ci/variables_spec.rb @@ -0,0 +1,210 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ::API::Admin::Ci::Variables do + let_it_be(:admin) { create(:admin) } + let_it_be(:user) { create(:user) } + + describe 'GET /admin/ci/variables' do + let!(:variable) { create(:ci_instance_variable) } + + it 'returns instance-level variables for admins', :aggregate_failures do + get api('/admin/ci/variables', admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_a(Array) + end + + it 'does not return instance-level variables for regular users' do + get api('/admin/ci/variables', user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'does not return instance-level variables for unauthorized users' do + get api('/admin/ci/variables') + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + describe 'GET /admin/ci/variables/:key' do + let!(:variable) { create(:ci_instance_variable) } + + it 'returns instance-level variable details for admins', :aggregate_failures do + get api("/admin/ci/variables/#{variable.key}", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['value']).to eq(variable.value) + expect(json_response['protected']).to eq(variable.protected?) + expect(json_response['variable_type']).to eq(variable.variable_type) + end + + it 'responds with 404 Not Found if requesting non-existing variable' do + get api('/admin/ci/variables/non_existing_variable', admin) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'does not return instance-level variable details for regular users' do + get api("/admin/ci/variables/#{variable.key}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'does not return instance-level variable details for unauthorized users' do + get api("/admin/ci/variables/#{variable.key}") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + describe 'POST /admin/ci/variables' do + context 'authorized user with proper permissions' do + let!(:variable) { create(:ci_instance_variable) } + + it 'creates variable for admins', :aggregate_failures do + expect do + post api('/admin/ci/variables', admin), + params: { + key: 'TEST_VARIABLE_2', + value: 'PROTECTED_VALUE_2', + protected: true, + masked: true + } + end.to change { ::Ci::InstanceVariable.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('PROTECTED_VALUE_2') + expect(json_response['protected']).to be_truthy + expect(json_response['masked']).to be_truthy + expect(json_response['variable_type']).to eq('env_var') + end + + it 'creates variable with optional attributes', :aggregate_failures do + expect do + post api('/admin/ci/variables', admin), + params: { + variable_type: 'file', + key: 'TEST_VARIABLE_2', + value: 'VALUE_2' + } + end.to change { ::Ci::InstanceVariable.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['protected']).to be_falsey + expect(json_response['masked']).to be_falsey + expect(json_response['variable_type']).to eq('file') + end + + it 'does not allow to duplicate variable key' do + expect do + post api('/admin/ci/variables', admin), + params: { key: variable.key, value: 'VALUE_2' } + end.not_to change { ::Ci::InstanceVariable.count } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'authorized user with invalid permissions' do + it 'does not create variable' do + post api('/admin/ci/variables', user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'unauthorized user' do + it 'does not create variable' do + post api('/admin/ci/variables') + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + describe 'PUT /admin/ci/variables/:key' do + let!(:variable) { create(:ci_instance_variable) } + + context 'authorized user with proper permissions' do + it 'updates variable data', :aggregate_failures do + put api("/admin/ci/variables/#{variable.key}", admin), + params: { + variable_type: 'file', + value: 'VALUE_1_UP', + protected: true, + masked: true + } + + expect(response).to have_gitlab_http_status(:ok) + expect(variable.reload.value).to eq('VALUE_1_UP') + expect(variable.reload).to be_protected + expect(json_response['variable_type']).to eq('file') + expect(json_response['masked']).to be_truthy + end + + it 'responds with 404 Not Found if requesting non-existing variable' do + put api('/admin/ci/variables/non_existing_variable', admin) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'authorized user with invalid permissions' do + it 'does not update variable' do + put api("/admin/ci/variables/#{variable.key}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'unauthorized user' do + it 'does not update variable' do + put api("/admin/ci/variables/#{variable.key}") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + describe 'DELETE /admin/ci/variables/:key' do + let!(:variable) { create(:ci_instance_variable) } + + context 'authorized user with proper permissions' do + it 'deletes variable' do + expect do + delete api("/admin/ci/variables/#{variable.key}", admin) + + expect(response).to have_gitlab_http_status(:no_content) + end.to change { ::Ci::InstanceVariable.count }.by(-1) + end + + it 'responds with 404 Not Found if requesting non-existing variable' do + delete api('/admin/ci/variables/non_existing_variable', admin) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'authorized user with invalid permissions' do + it 'does not delete variable' do + delete api("/admin/ci/variables/#{variable.key}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'unauthorized user' do + it 'does not delete variable' do + delete api("/admin/ci/variables/#{variable.key}") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end +end diff --git a/spec/requests/api/appearance_spec.rb b/spec/requests/api/appearance_spec.rb index 70be3adf723..f8c3db70d16 100644 --- a/spec/requests/api/appearance_spec.rb +++ b/spec/requests/api/appearance_spec.rb @@ -31,6 +31,7 @@ describe API::Appearance, 'Appearance' do expect(json_response['message_background_color']).to eq('#E75E40') expect(json_response['message_font_color']).to eq('#FFFFFF') expect(json_response['new_project_guidelines']).to eq('') + expect(json_response['profile_image_guidelines']).to eq('') expect(json_response['title']).to eq('') end end @@ -51,7 +52,8 @@ describe API::Appearance, 'Appearance' do put api("/application/appearance", admin), params: { title: "GitLab Test Instance", description: "gitlab-test.example.com", - new_project_guidelines: "Please read the FAQs for help." + new_project_guidelines: "Please read the FAQs for help.", + profile_image_guidelines: "Custom profile image guidelines" } expect(response).to have_gitlab_http_status(:ok) @@ -66,6 +68,7 @@ describe API::Appearance, 'Appearance' do expect(json_response['message_background_color']).to eq('#E75E40') expect(json_response['message_font_color']).to eq('#FFFFFF') expect(json_response['new_project_guidelines']).to eq('Please read the FAQs for help.') + expect(json_response['profile_image_guidelines']).to eq('Custom profile image guidelines') expect(json_response['title']).to eq('GitLab Test Instance') end end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 97f880dd3cd..f2dc5b1c045 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -16,6 +16,7 @@ describe API::Branches do before do project.add_maintainer(user) + project.repository.add_branch(user, 'ends-with.txt', branch_sha) end describe "GET /projects/:id/repository/branches" do @@ -240,6 +241,12 @@ describe API::Branches do it_behaves_like 'repository branch' end + context 'when branch contains dot txt' do + let(:branch_name) { project.repository.find_branch('ends-with.txt').name } + + it_behaves_like 'repository branch' + end + context 'when branch contains a slash' do let(:branch_name) { branch_with_slash.name } @@ -623,7 +630,7 @@ describe API::Branches do post api(route, user), params: { branch: 'new_design3', ref: 'foo' } expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq('Invalid reference name: new_design3') + expect(json_response['message']).to eq('Invalid reference name: foo') end end diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb index b820b227fff..ef2415a0cde 100644 --- a/spec/requests/api/deployments_spec.rb +++ b/spec/requests/api/deployments_spec.rb @@ -439,7 +439,7 @@ describe API::Deployments do let!(:merge_request3) { create(:merge_request, source_project: project2, target_project: project2) } it 'returns the relevant merge requests linked to a deployment for a project' do - deployment.merge_requests << [merge_request1, merge_request2] + deployment.link_merge_requests(MergeRequest.where(id: [merge_request1.id, merge_request2.id])) subject diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb index ce72a416c33..4ad5b4f9d49 100644 --- a/spec/requests/api/features_spec.rb +++ b/spec/requests/api/features_spec.rb @@ -198,7 +198,7 @@ describe API::Features do end end - it 'creates a feature with the given percentage if passed an integer' do + it 'creates a feature with the given percentage of time if passed an integer' do post api("/features/#{feature_name}", admin), params: { value: '50' } expect(response).to have_gitlab_http_status(:created) @@ -210,6 +210,19 @@ describe API::Features do { 'key' => 'percentage_of_time', 'value' => 50 } ]) end + + it 'creates a feature with the given percentage of actors if passed an integer' do + post api("/features/#{feature_name}", admin), params: { value: '50', key: 'percentage_of_actors' } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to eq( + 'name' => 'my_feature', + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'percentage_of_actors', 'value' => 50 } + ]) + end end context 'when the feature exists' do @@ -298,7 +311,7 @@ describe API::Features do end end - context 'with a pre-existing percentage value' do + context 'with a pre-existing percentage of time value' do before do feature.enable_percentage_of_time(50) end @@ -316,6 +329,25 @@ describe API::Features do ]) end end + + context 'with a pre-existing percentage of actors value' do + before do + feature.enable_percentage_of_actors(42) + end + + it 'updates the percentage of actors if passed an integer' do + post api("/features/#{feature_name}", admin), params: { value: '74', key: 'percentage_of_actors' } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to eq( + 'name' => 'my_feature', + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'percentage_of_actors', 'value' => 74 } + ]) + end + end end end diff --git a/spec/requests/api/freeze_periods_spec.rb b/spec/requests/api/freeze_periods_spec.rb new file mode 100644 index 00000000000..0b7828ebedf --- /dev/null +++ b/spec/requests/api/freeze_periods_spec.rb @@ -0,0 +1,475 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::FreezePeriods do + let_it_be(:project) { create(:project, :repository, :private) } + let_it_be(:user) { create(:user) } + let_it_be(:admin) { create(:admin) } + let(:api_user) { user } + let(:invalid_cron) { '0 0 0 * *' } + let(:last_freeze_period) { project.freeze_periods.last } + + describe 'GET /projects/:id/freeze_periods' do + context 'when the user is the admin' do + let!(:freeze_period) { create(:ci_freeze_period, project: project, created_at: 2.days.ago) } + + it 'returns 200 HTTP status' do + get api("/projects/#{project.id}/freeze_periods", admin) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when the user is the maintainer' do + before do + project.add_maintainer(user) + end + + context 'when there are two freeze_periods' do + let!(:freeze_period_1) { create(:ci_freeze_period, project: project, created_at: 2.days.ago) } + let!(:freeze_period_2) { create(:ci_freeze_period, project: project, created_at: 1.day.ago) } + + it 'returns 200 HTTP status' do + get api("/projects/#{project.id}/freeze_periods", user) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns freeze_periods ordered by created_at ascending' do + get api("/projects/#{project.id}/freeze_periods", user) + + expect(json_response.count).to eq(2) + expect(freeze_period_ids).to eq([freeze_period_1.id, freeze_period_2.id]) + end + + it 'matches response schema' do + get api("/projects/#{project.id}/freeze_periods", user) + + expect(response).to match_response_schema('public_api/v4/freeze_periods') + end + end + + context 'when there are no freeze_periods' do + it 'returns 200 HTTP status' do + get api("/projects/#{project.id}/freeze_periods", user) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns an empty response' do + get api("/projects/#{project.id}/freeze_periods", user) + + expect(json_response).to be_empty + end + end + end + + context 'when user is a guest' do + before do + project.add_guest(user) + end + + let!(:freeze_period) do + create(:ci_freeze_period, project: project) + end + + it 'responds 403 Forbidden' do + get api("/projects/#{project.id}/freeze_periods", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + it 'responds 404 Not Found' do + get api("/projects/#{project.id}/freeze_periods", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :public) } + + it 'responds 403 Forbidden' do + get api("/projects/#{project.id}/freeze_periods", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + describe 'GET /projects/:id/freeze_periods/:freeze_period_id' do + context 'when there is a freeze period' do + let!(:freeze_period) do + create(:ci_freeze_period, project: project) + end + + context 'when the user is the admin' do + let!(:freeze_period) { create(:ci_freeze_period, project: project, created_at: 2.days.ago) } + + it 'responds 200 OK' do + get api("/projects/#{project.id}/freeze_periods/#{freeze_period.id}", admin) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when the user is the maintainer' do + before do + project.add_maintainer(user) + end + + it 'responds 200 OK' do + get api("/projects/#{project.id}/freeze_periods/#{freeze_period.id}", user) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns a freeze period' do + get api("/projects/#{project.id}/freeze_periods/#{freeze_period.id}", user) + + expect(json_response).to include( + 'id' => freeze_period.id, + 'freeze_start' => freeze_period.freeze_start, + 'freeze_end' => freeze_period.freeze_end, + 'cron_timezone' => freeze_period.cron_timezone) + end + + it 'matches response schema' do + get api("/projects/#{project.id}/freeze_periods/#{freeze_period.id}", user) + + expect(response).to match_response_schema('public_api/v4/freeze_period') + end + end + + context 'when user is a guest' do + before do + project.add_guest(user) + end + + it 'responds 403 Forbidden' do + get api("/projects/#{project.id}/freeze_periods/#{freeze_period.id}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + context 'when project is public' do + let(:project) { create(:project, :public) } + + context 'when freeze_period exists' do + it 'responds 403 Forbidden' do + get api("/projects/#{project.id}/freeze_periods/#{freeze_period.id}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when freeze_period does not exist' do + it 'responds 403 Forbidden' do + get api("/projects/#{project.id}/freeze_periods/0", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + end + end + + describe 'POST /projects/:id/freeze_periods' do + let(:params) do + { + freeze_start: '0 23 * * 5', + freeze_end: '0 7 * * 1', + cron_timezone: 'UTC' + } + end + + subject { post api("/projects/#{project.id}/freeze_periods", api_user), params: params } + + context 'when the user is the admin' do + let(:api_user) { admin } + + it 'accepts the request' do + subject + + expect(response).to have_gitlab_http_status(:created) + end + end + + context 'when user is the maintainer' do + before do + project.add_maintainer(user) + end + + context 'with valid params' do + it 'accepts the request' do + subject + + expect(response).to have_gitlab_http_status(:created) + end + + it 'creates a new freeze period' do + expect do + subject + end.to change { Ci::FreezePeriod.count }.by(1) + + expect(last_freeze_period.freeze_start).to eq('0 23 * * 5') + expect(last_freeze_period.freeze_end).to eq('0 7 * * 1') + expect(last_freeze_period.cron_timezone).to eq('UTC') + end + + it 'matches response schema' do + subject + + expect(response).to match_response_schema('public_api/v4/freeze_period') + end + end + + context 'with incomplete params' do + let(:params) do + { + freeze_start: '0 23 * * 5', + cron_timezone: 'UTC' + } + end + + it 'responds 400 Bad Request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq("freeze_end is missing") + end + end + + context 'with invalid params' do + let(:params) do + { + freeze_start: '0 23 * * 5', + freeze_end: invalid_cron, + cron_timezone: 'UTC' + } + end + + it 'responds 400 Bad Request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['freeze_end']).to eq([" is invalid syntax"]) + end + end + end + + context 'when user is a developer' do + before do + project.add_developer(user) + end + + it 'responds 403 Forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is a reporter' do + before do + project.add_reporter(user) + end + + it 'responds 403 Forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + it 'responds 403 Forbidden' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :public) } + + it 'responds 403 Forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + describe 'PUT /projects/:id/freeze_periods/:freeze_period_id' do + let(:params) { { freeze_start: '0 22 * * 5', freeze_end: '5 4 * * sun' } } + let!(:freeze_period) { create :ci_freeze_period, project: project } + + subject { put api("/projects/#{project.id}/freeze_periods/#{freeze_period.id}", api_user), params: params } + + context 'when user is the admin' do + let(:api_user) { admin } + + it 'accepts the request' do + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when user is the maintainer' do + before do + project.add_maintainer(user) + end + + context 'with valid params' do + it 'accepts the request' do + subject + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'performs the update' do + subject + + freeze_period.reload + + expect(freeze_period.freeze_start).to eq(params[:freeze_start]) + expect(freeze_period.freeze_end).to eq(params[:freeze_end]) + end + + it 'matches response schema' do + subject + + expect(response).to match_response_schema('public_api/v4/freeze_period') + end + end + + context 'with invalid params' do + let(:params) { { freeze_start: invalid_cron } } + + it 'responds 400 Bad Request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['freeze_start']).to eq([" is invalid syntax"]) + end + end + end + + context 'when user is a reporter' do + before do + project.add_reporter(user) + end + + it 'responds 403 Forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + it 'responds 404 Not Found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :public) } + + it 'responds 403 Forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + describe 'DELETE /projects/:id/freeze_periods/:freeze_period_id' do + let!(:freeze_period) { create :ci_freeze_period, project: project } + let(:freeze_period_id) { freeze_period.id } + + subject { delete api("/projects/#{project.id}/freeze_periods/#{freeze_period_id}", api_user) } + + context 'when user is the admin' do + let(:api_user) { admin } + + it 'accepts the request' do + subject + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'when user is the maintainer' do + before do + project.add_maintainer(user) + end + + it 'accepts the request' do + subject + + expect(response).to have_gitlab_http_status(:no_content) + end + + it 'destroys the freeze period' do + expect do + subject + end.to change { Ci::FreezePeriod.count }.by(-1) + end + + context 'when it is a non-existing freeze period id' do + let(:freeze_period_id) { 0 } + + it '404' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when user is a reporter' do + before do + project.add_reporter(user) + end + + it 'responds 403 Forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is not a project member' do + it 'responds 404 Not Found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when project is public' do + let(:project) { create(:project, :public) } + + it 'responds 403 Forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + def freeze_period_ids + json_response.map do |freeze_period_hash| + freeze_period_hash.fetch('id')&.to_i + end + end +end diff --git a/spec/requests/api/graphql/boards/board_lists_query_spec.rb b/spec/requests/api/graphql/boards/board_lists_query_spec.rb new file mode 100644 index 00000000000..f0927487f85 --- /dev/null +++ b/spec/requests/api/graphql/boards/board_lists_query_spec.rb @@ -0,0 +1,137 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'get board lists' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:unauth_user) { create(:user) } + let_it_be(:project) { create(:project, creator_id: user.id, namespace: user.namespace ) } + let_it_be(:group) { create(:group, :private) } + let_it_be(:project_label) { create(:label, project: project, name: 'Development') } + let_it_be(:project_label2) { create(:label, project: project, name: 'Testing') } + let_it_be(:group_label) { create(:group_label, group: group, name: 'Development') } + let_it_be(:group_label2) { create(:group_label, group: group, name: 'Testing') } + + let(:params) { '' } + let(:board) { } + let(:board_parent_type) { board_parent.class.to_s.downcase } + let(:board_data) { graphql_data[board_parent_type]['boards']['edges'].first['node'] } + let(:lists_data) { board_data['lists']['edges'] } + let(:start_cursor) { board_data['lists']['pageInfo']['startCursor'] } + let(:end_cursor) { board_data['lists']['pageInfo']['endCursor'] } + + def query(list_params = params) + graphql_query_for( + board_parent_type, + { 'fullPath' => board_parent.full_path }, + <<~BOARDS + boards(first: 1) { + edges { + node { + #{field_with_params('lists', list_params)} { + pageInfo { + startCursor + endCursor + } + edges { + node { + #{all_graphql_fields_for('board_lists'.classify)} + } + } + } + } + } + } + BOARDS + ) + end + + shared_examples 'group and project board lists query' do + let!(:board) { create(:board, resource_parent: board_parent) } + + context 'when the user does not have access to the board' do + it 'returns nil' do + post_graphql(query, current_user: unauth_user) + + expect(graphql_data[board_parent_type]).to be_nil + end + end + + context 'when user can read the board' do + before do + board_parent.add_reporter(user) + end + + describe 'sorting and pagination' do + context 'when using default sorting' do + let!(:label_list) { create(:list, board: board, label: label, position: 10) } + let!(:label_list2) { create(:list, board: board, label: label2, position: 2) } + let!(:backlog_list) { create(:backlog_list, board: board) } + let(:closed_list) { board.lists.find_by(list_type: :closed) } + + before do + post_graphql(query, current_user: user) + end + + it_behaves_like 'a working graphql query' + + context 'when ascending' do + let(:lists) { [backlog_list, label_list2, label_list, closed_list] } + let(:expected_list_gids) do + lists.map { |list| list.to_global_id.to_s } + end + + it 'sorts lists' do + expect(grab_ids).to eq expected_list_gids + end + + context 'when paginating' do + let(:params) { 'first: 2' } + + it 'sorts boards' do + expect(grab_ids).to eq expected_list_gids.first(2) + + cursored_query = query("after: \"#{end_cursor}\"") + post_graphql(cursored_query, current_user: user) + + response_data = grab_list_data(response.body) + + expect(grab_ids(response_data)).to eq expected_list_gids.drop(2).first(2) + end + end + end + end + end + end + end + + describe 'for a project' do + let(:board_parent) { project } + let(:label) { project_label } + let(:label2) { project_label2 } + + it_behaves_like 'group and project board lists query' + end + + describe 'for a group' do + let(:board_parent) { group } + let(:label) { group_label } + let(:label2) { group_label2 } + + before do + allow(board_parent).to receive(:multiple_issue_boards_available?).and_return(false) + end + + it_behaves_like 'group and project board lists query' + end + + def grab_ids(data = lists_data) + data.map { |list| list.dig('node', 'id') } + end + + def grab_list_data(response_body) + Gitlab::Json.parse(response_body)['data'][board_parent_type]['boards']['edges'][0]['node']['lists']['edges'] + end +end diff --git a/spec/requests/api/graphql/current_user/todos_query_spec.rb b/spec/requests/api/graphql/current_user/todos_query_spec.rb index 82deba0d92c..321e1062a96 100644 --- a/spec/requests/api/graphql/current_user/todos_query_spec.rb +++ b/spec/requests/api/graphql/current_user/todos_query_spec.rb @@ -9,6 +9,7 @@ describe 'Query current user todos' do let_it_be(:commit_todo) { create(:on_commit_todo, user: current_user, project: create(:project, :repository)) } let_it_be(:issue_todo) { create(:todo, user: current_user, target: create(:issue)) } let_it_be(:merge_request_todo) { create(:todo, user: current_user, target: create(:merge_request)) } + let_it_be(:design_todo) { create(:todo, user: current_user, target: create(:design)) } let(:fields) do <<~QUERY @@ -34,7 +35,8 @@ describe 'Query current user todos' do is_expected.to include( a_hash_including('id' => commit_todo.to_global_id.to_s), a_hash_including('id' => issue_todo.to_global_id.to_s), - a_hash_including('id' => merge_request_todo.to_global_id.to_s) + a_hash_including('id' => merge_request_todo.to_global_id.to_s), + a_hash_including('id' => design_todo.to_global_id.to_s) ) end @@ -42,7 +44,8 @@ describe 'Query current user todos' do is_expected.to include( a_hash_including('targetType' => 'COMMIT'), a_hash_including('targetType' => 'ISSUE'), - a_hash_including('targetType' => 'MERGEREQUEST') + a_hash_including('targetType' => 'MERGEREQUEST'), + a_hash_including('targetType' => 'DESIGN') ) end end diff --git a/spec/requests/api/graphql/gitlab_schema_spec.rb b/spec/requests/api/graphql/gitlab_schema_spec.rb index cf409ea6c2d..266c98d6f08 100644 --- a/spec/requests/api/graphql/gitlab_schema_spec.rb +++ b/spec/requests/api/graphql/gitlab_schema_spec.rb @@ -190,7 +190,7 @@ describe 'GitlabSchema configurations' do variables: {}.to_s, complexity: 181, depth: 13, - duration: 7 + duration_s: 7 } expect_any_instance_of(Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer).to receive(:duration).and_return(7) diff --git a/spec/requests/api/graphql/group/milestones_spec.rb b/spec/requests/api/graphql/group/milestones_spec.rb index f8e3c0026f5..bad0024e7a3 100644 --- a/spec/requests/api/graphql/group/milestones_spec.rb +++ b/spec/requests/api/graphql/group/milestones_spec.rb @@ -7,7 +7,7 @@ describe 'Milestones through GroupQuery' do let_it_be(:user) { create(:user) } let_it_be(:now) { Time.now } - let_it_be(:group) { create(:group, :private) } + let_it_be(:group) { create(:group) } let_it_be(:milestone_1) { create(:milestone, group: group) } let_it_be(:milestone_2) { create(:milestone, group: group, state: :closed, start_date: now, due_date: now + 1.day) } let_it_be(:milestone_3) { create(:milestone, group: group, start_date: now, due_date: now + 2.days) } @@ -17,10 +17,6 @@ describe 'Milestones through GroupQuery' do let(:milestone_data) { graphql_data['group']['milestones']['edges'] } describe 'Get list of milestones from a group' do - before do - group.add_developer(user) - end - context 'when the request is correct' do before do fetch_milestones(user) @@ -51,6 +47,48 @@ describe 'Milestones through GroupQuery' do end end + context 'when including milestones from decendants' do + let_it_be(:accessible_group) { create(:group, :private, parent: group) } + let_it_be(:accessible_project) { create(:project, group: accessible_group) } + let_it_be(:inaccessible_group) { create(:group, :private, parent: group) } + let_it_be(:inaccessible_project) { create(:project, :private, group: group) } + let_it_be(:submilestone_1) { create(:milestone, group: accessible_group) } + let_it_be(:submilestone_2) { create(:milestone, project: accessible_project) } + let_it_be(:submilestone_3) { create(:milestone, group: inaccessible_group) } + let_it_be(:submilestone_4) { create(:milestone, project: inaccessible_project) } + + let(:args) { { include_descendants: true } } + + before do + accessible_group.add_developer(user) + end + + it 'returns milestones also from subgroups and subprojects visible to user' do + fetch_milestones(user, args) + + expect_array_response( + milestone_1.to_global_id.to_s, milestone_2.to_global_id.to_s, + milestone_3.to_global_id.to_s, milestone_4.to_global_id.to_s, + submilestone_1.to_global_id.to_s, submilestone_2.to_global_id.to_s + ) + end + + context 'when group_milestone_descendants is disabled' do + before do + stub_feature_flags(group_milestone_descendants: false) + end + + it 'ignores descendant milestones' do + fetch_milestones(user, args) + + expect_array_response( + milestone_1.to_global_id.to_s, milestone_2.to_global_id.to_s, + milestone_3.to_global_id.to_s, milestone_4.to_global_id.to_s + ) + end + end + end + def fetch_milestones(user = nil, args = {}) post_graphql(milestones_query(args), current_user: user) end diff --git a/spec/requests/api/graphql/metrics/dashboard/annotations_spec.rb b/spec/requests/api/graphql/metrics/dashboard/annotations_spec.rb index f5a5f0a9ec2..cb35411b7a5 100644 --- a/spec/requests/api/graphql/metrics/dashboard/annotations_spec.rb +++ b/spec/requests/api/graphql/metrics/dashboard/annotations_spec.rb @@ -21,6 +21,7 @@ describe 'Getting Metrics Dashboard Annotations' do create(:metrics_dashboard_annotation, environment: environment, starting_at: to.advance(minutes: 5), dashboard_path: path) end + let(:args) { "from: \"#{from}\", to: \"#{to}\"" } let(:fields) do <<~QUERY #{all_graphql_fields_for('MetricsDashboardAnnotation'.classify)} @@ -47,63 +48,40 @@ describe 'Getting Metrics Dashboard Annotations' do ) end - context 'feature flag metrics_dashboard_annotations' do - let(:args) { "from: \"#{from}\", to: \"#{to}\"" } + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end - before do - project.add_developer(current_user) - end + it_behaves_like 'a working graphql query' - context 'is off' do - before do - stub_feature_flags(metrics_dashboard_annotations: false) - post_graphql(query, current_user: current_user) - end + it 'returns annotations' do + annotations = graphql_data.dig('project', 'environments', 'nodes')[0].dig('metricsDashboard', 'annotations', 'nodes') - it 'returns empty nodes array' do - annotations = graphql_data.dig('project', 'environments', 'nodes')[0].dig('metricsDashboard', 'annotations', 'nodes') + expect(annotations).to match_array [{ + "description" => annotation.description, + "id" => annotation.to_global_id.to_s, + "panelId" => annotation.panel_xid, + "startingAt" => annotation.starting_at.iso8601, + "endingAt" => nil + }] + end - expect(annotations).to be_empty - end - end + context 'arguments' do + context 'from is missing' do + let(:args) { "to: \"#{from}\"" } - context 'is on' do - before do - stub_feature_flags(metrics_dashboard_annotations: true) + it 'returns error' do post_graphql(query, current_user: current_user) - end - it_behaves_like 'a working graphql query' - - it 'returns annotations' do - annotations = graphql_data.dig('project', 'environments', 'nodes')[0].dig('metricsDashboard', 'annotations', 'nodes') - - expect(annotations).to match_array [{ - "description" => annotation.description, - "id" => annotation.to_global_id.to_s, - "panelId" => annotation.panel_xid, - "startingAt" => annotation.starting_at.to_s, - "endingAt" => nil - }] + expect(graphql_errors[0]).to include("message" => "Field 'annotations' is missing required arguments: from") end + end - context 'arguments' do - context 'from is missing' do - let(:args) { "to: \"#{from}\"" } - - it 'returns error' do - post_graphql(query, current_user: current_user) - - expect(graphql_errors[0]).to include("message" => "Field 'annotations' is missing required arguments: from") - end - end - - context 'to is missing' do - let(:args) { "from: \"#{from}\"" } + context 'to is missing' do + let(:args) { "from: \"#{from}\"" } - it_behaves_like 'a working graphql query' - end - end + it_behaves_like 'a working graphql query' end end end diff --git a/spec/requests/api/graphql/mutations/alert_management/alerts/update_alert_status_spec.rb b/spec/requests/api/graphql/mutations/alert_management/alerts/update_alert_status_spec.rb new file mode 100644 index 00000000000..fe50468134c --- /dev/null +++ b/spec/requests/api/graphql/mutations/alert_management/alerts/update_alert_status_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Setting the status of an alert' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let(:alert) { create(:alert_management_alert, project: project) } + let(:input) { { status: 'ACKNOWLEDGED' } } + + let(:mutation) do + variables = { + project_path: project.full_path, + iid: alert.iid.to_s + } + graphql_mutation(:update_alert_status, variables.merge(input), + <<~QL + clientMutationId + errors + alert { + iid + status + } + QL + ) + end + + let(:mutation_response) { graphql_mutation_response(:update_alert_status) } + + before do + project.add_developer(user) + end + + it 'updates the status of the alert' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['alert']['status']).to eq(input[:status]) + end +end diff --git a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb index 3fdeccc84f9..83dec7dd3e2 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb @@ -23,7 +23,7 @@ describe 'Adding an AwardEmoji' do end shared_examples 'a mutation that does not create an AwardEmoji' do - it do + specify do expect do post_graphql_mutation(mutation, current_user: current_user) end.not_to change { AwardEmoji.count } diff --git a/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb index c78f0c7ca27..a2997db6cae 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb @@ -24,7 +24,7 @@ describe 'Removing an AwardEmoji' do end shared_examples 'a mutation that does not destroy an AwardEmoji' do - it do + specify do expect do post_graphql_mutation(mutation, current_user: current_user) end.not_to change { AwardEmoji.count } diff --git a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb index bc796b34db4..e1180c85c6b 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb @@ -23,7 +23,7 @@ describe 'Toggling an AwardEmoji' do end shared_examples 'a mutation that does not create or destroy an AwardEmoji' do - it do + specify do expect do post_graphql_mutation(mutation, current_user: current_user) end.not_to change { AwardEmoji.count } diff --git a/spec/requests/api/graphql/mutations/branches/create_spec.rb b/spec/requests/api/graphql/mutations/branches/create_spec.rb new file mode 100644 index 00000000000..b3c378ec2bc --- /dev/null +++ b/spec/requests/api/graphql/mutations/branches/create_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Creation of a new branch' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project, :public, :empty_repo) } + let(:input) { { project_path: project.full_path, name: new_branch, ref: ref } } + let(:new_branch) { 'new_branch' } + let(:ref) { 'master' } + + let(:mutation) { graphql_mutation(:create_branch, input) } + let(:mutation_response) { graphql_mutation_response(:create_branch) } + + context 'the user is not allowed to create a branch' do + it_behaves_like 'a mutation that returns top-level errors', + errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + end + + context 'when user has permissions to create a branch' do + before do + project.add_developer(current_user) + end + + it 'creates a new branch' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['branch']).to include( + 'name' => new_branch, + 'commit' => a_hash_including('id') + ) + end + + context 'when ref is not correct' do + let(:new_branch) { 'another_branch' } + let(:ref) { 'unknown' } + + it_behaves_like 'a mutation that returns errors in the response', + errors: ['Invalid reference name: unknown'] + end + end +end diff --git a/spec/requests/api/graphql/mutations/design_management/delete_spec.rb b/spec/requests/api/graphql/mutations/design_management/delete_spec.rb new file mode 100644 index 00000000000..10376305b3e --- /dev/null +++ b/spec/requests/api/graphql/mutations/design_management/delete_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe "deleting designs" do + include GraphqlHelpers + include DesignManagementTestHelpers + + let(:developer) { create(:user) } + let(:current_user) { developer } + let(:issue) { create(:issue) } + let(:project) { issue.project } + let(:designs) { create_designs } + let(:variables) { {} } + + let(:mutation) do + input = { + project_path: project.full_path, + iid: issue.iid, + filenames: designs.map(&:filename) + }.merge(variables) + graphql_mutation(:design_management_delete, input) + end + + let(:mutation_response) { graphql_mutation_response(:design_management_delete) } + + def mutate! + post_graphql_mutation(mutation, current_user: current_user) + end + + before do + enable_design_management + + project.add_developer(developer) + end + + shared_examples 'a failed request' do + let(:the_error) { be_present } + + it 'reports an error' do + mutate! + + expect(graphql_errors).to include(a_hash_including('message' => the_error)) + end + end + + context 'the designs list is empty' do + it_behaves_like 'a failed request' do + let(:designs) { [] } + let(:the_error) { a_string_matching %r/was provided invalid value/ } + end + end + + context 'the designs list contains filenames we cannot find' do + it_behaves_like 'a failed request' do + let(:designs) { %w/foo bar baz/.map { |fn| OpenStruct.new(filename: fn) } } + let(:the_error) { a_string_matching %r/filenames were not found/ } + end + end + + context 'the current user does not have developer access' do + it_behaves_like 'a failed request' do + let(:current_user) { create(:user) } + let(:the_error) { a_string_matching %r/you don't have permission/ } + end + end + + context "when the issue does not exist" do + it_behaves_like 'a failed request' do + let(:variables) { { iid: "1234567890" } } + let(:the_error) { a_string_matching %r/does not exist/ } + end + end + + context "when saving the designs raises an error" do + let(:designs) { create_designs(1) } + + it "responds with errors" do + expect_next_instance_of(::DesignManagement::DeleteDesignsService) do |service| + expect(service) + .to receive(:execute) + .and_return({ status: :error, message: "Something went wrong" }) + end + + mutate! + + expect(mutation_response).to include('errors' => include(eq "Something went wrong")) + end + end + + context 'one of the designs is already deleted' do + let(:designs) do + create_designs(2).push(create(:design, :with_file, deleted: true, issue: issue)) + end + + it 'reports an error' do + mutate! + + expect(graphql_errors).to be_present + end + end + + context 'when the user names designs to delete' do + before do + create_designs(1) + end + + let!(:designs) { create_designs(2) } + + it 'deletes the designs' do + expect { mutate! } + .to change { issue.reset.designs.current.count }.from(3).to(1) + end + + it 'has no errors' do + mutate! + + expect(mutation_response).to include('errors' => be_empty) + end + end + + private + + def create_designs(how_many = 2) + create_list(:design, how_many, :with_file, issue: issue) + end +end diff --git a/spec/requests/api/graphql/mutations/design_management/upload_spec.rb b/spec/requests/api/graphql/mutations/design_management/upload_spec.rb new file mode 100644 index 00000000000..22adc064406 --- /dev/null +++ b/spec/requests/api/graphql/mutations/design_management/upload_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true +require "spec_helper" + +describe "uploading designs" do + include GraphqlHelpers + include DesignManagementTestHelpers + include WorkhorseHelpers + + let(:current_user) { create(:user) } + let(:issue) { create(:issue) } + let(:project) { issue.project } + let(:files) { [fixture_file_upload("spec/fixtures/dk.png")] } + let(:variables) { {} } + + let(:mutation) do + input = { + project_path: project.full_path, + iid: issue.iid, + files: files + }.merge(variables) + graphql_mutation(:design_management_upload, input) + end + + let(:mutation_response) { graphql_mutation_response(:design_management_upload) } + + before do + enable_design_management + + project.add_developer(current_user) + end + + it "returns an error if the user is not allowed to upload designs" do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).to be_present + end + + it "succeeds (backward compatibility)" do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).not_to be_present + end + + it 'succeeds' do + file_path_in_params = ['designManagementUploadInput', 'files', 0] + params = mutation_to_apollo_uploads_param(mutation, files: [file_path_in_params]) + + workhorse_post_with_file(api('/', current_user, version: 'graphql'), + params: params, + file_key: '1' + ) + + expect(graphql_errors).not_to be_present + end + + it "responds with the created designs" do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to include( + "designs" => a_collection_containing_exactly( + a_hash_including("filename" => "dk.png") + ) + ) + end + + it "can respond with skipped designs" do + 2.times do + post_graphql_mutation(mutation, current_user: current_user) + files.each(&:rewind) + end + + expect(mutation_response).to include( + "skippedDesigns" => a_collection_containing_exactly( + a_hash_including("filename" => "dk.png") + ) + ) + end + + context "when the issue does not exist" do + let(:variables) { { iid: "123" } } + + it "returns an error" do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + end + end + + context "when saving the designs raises an error" do + it "responds with errors" do + expect_next_instance_of(::DesignManagement::SaveDesignsService) do |service| + expect(service).to receive(:execute).and_return({ status: :error, message: "Something went wrong" }) + end + + post_graphql_mutation(mutation, current_user: current_user) + expect(mutation_response["errors"].first).to eq("Something went wrong") + end + end +end diff --git a/spec/requests/api/graphql/mutations/jira_import/start_spec.rb b/spec/requests/api/graphql/mutations/jira_import/start_spec.rb index 014da5d1e1a..84110098400 100644 --- a/spec/requests/api/graphql/mutations/jira_import/start_spec.rb +++ b/spec/requests/api/graphql/mutations/jira_import/start_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe 'Starting a Jira Import' do + include JiraServiceHelper include GraphqlHelpers let_it_be(:user) { create(:user) } @@ -104,6 +105,8 @@ describe 'Starting a Jira Import' do before do project.reload + + stub_jira_service_test end context 'when issues feature are disabled' do @@ -118,7 +121,7 @@ describe 'Starting a Jira Import' do it_behaves_like 'a mutation that returns errors in the response', errors: ['Unable to find Jira project to import data from.'] end - context 'when jira import successfully scheduled' do + context 'when Jira import successfully scheduled' do it 'schedules a Jira import' do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb new file mode 100644 index 00000000000..8568dc8ffc0 --- /dev/null +++ b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb @@ -0,0 +1,231 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Mutations::Metrics::Dashboard::Annotations::Create do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project, :private, :repository) } + let_it_be(:environment) { create(:environment, project: project) } + let_it_be(:cluster) { create(:cluster, projects: [project]) } + let(:dashboard_path) { 'config/prometheus/common_metrics.yml' } + let(:starting_at) { Time.current.iso8601 } + let(:ending_at) { 1.hour.from_now.iso8601 } + let(:description) { 'test description' } + + def mutation_response + graphql_mutation_response(:create_annotation) + end + + specify { expect(described_class).to require_graphql_authorizations(:create_metrics_dashboard_annotation) } + + context 'when annotation source is environment' do + let(:mutation) do + variables = { + environment_id: GitlabSchema.id_from_object(environment).to_s, + starting_at: starting_at, + ending_at: ending_at, + dashboard_path: dashboard_path, + description: description + } + + graphql_mutation(:create_annotation, variables) + end + + context 'when the user does not have permission' do + before do + project.add_reporter(current_user) + end + + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] + + it 'does not create the annotation' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { Metrics::Dashboard::Annotation.count } + end + end + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + it 'creates the annotation' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { Metrics::Dashboard::Annotation.count }.by(1) + end + + it 'returns the created annotation' do + post_graphql_mutation(mutation, current_user: current_user) + + annotation = Metrics::Dashboard::Annotation.first + annotation_id = GitlabSchema.id_from_object(annotation).to_s + + expect(mutation_response['annotation']['description']).to match(description) + expect(mutation_response['annotation']['startingAt'].to_time).to match(starting_at.to_time) + expect(mutation_response['annotation']['endingAt'].to_time).to match(ending_at.to_time) + expect(mutation_response['annotation']['id']).to match(annotation_id) + expect(annotation.environment_id).to eq(environment.id) + end + + context 'when environment_id is missing' do + let(:mutation) do + variables = { + environment_id: nil, + starting_at: starting_at, + ending_at: ending_at, + dashboard_path: dashboard_path, + description: description + } + + graphql_mutation(:create_annotation, variables) + end + + it_behaves_like 'a mutation that returns top-level errors', errors: [described_class::ANNOTATION_SOURCE_ARGUMENT_ERROR] + end + + context 'when environment_id is invalid' do + let(:mutation) do + variables = { + environment_id: 'invalid_id', + starting_at: starting_at, + ending_at: ending_at, + dashboard_path: dashboard_path, + description: description + } + + graphql_mutation(:create_annotation, variables) + end + + it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab id.'] + end + end + end + + context 'when annotation source is cluster' do + let(:mutation) do + variables = { + cluster_id: GitlabSchema.id_from_object(cluster).to_s, + starting_at: starting_at, + ending_at: ending_at, + dashboard_path: dashboard_path, + description: description + } + + graphql_mutation(:create_annotation, variables) + end + + context 'with permission' do + before do + project.add_developer(current_user) + end + + it 'creates the annotation' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { Metrics::Dashboard::Annotation.count }.by(1) + end + + it 'returns the created annotation' do + post_graphql_mutation(mutation, current_user: current_user) + + annotation = Metrics::Dashboard::Annotation.first + annotation_id = GitlabSchema.id_from_object(annotation).to_s + + expect(mutation_response['annotation']['description']).to match(description) + expect(mutation_response['annotation']['startingAt'].to_time).to match(starting_at.to_time) + expect(mutation_response['annotation']['endingAt'].to_time).to match(ending_at.to_time) + expect(mutation_response['annotation']['id']).to match(annotation_id) + expect(annotation.cluster_id).to eq(cluster.id) + end + + context 'when cluster_id is missing' do + let(:mutation) do + variables = { + cluster_id: nil, + starting_at: starting_at, + ending_at: ending_at, + dashboard_path: dashboard_path, + description: description + } + + graphql_mutation(:create_annotation, variables) + end + + it_behaves_like 'a mutation that returns top-level errors', errors: [described_class::ANNOTATION_SOURCE_ARGUMENT_ERROR] + end + end + + context 'without permission' do + before do + project.add_guest(current_user) + end + + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] + + it 'does not create the annotation' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { Metrics::Dashboard::Annotation.count } + end + end + + context 'when cluster_id is invalid' do + let(:mutation) do + variables = { + cluster_id: 'invalid_id', + starting_at: starting_at, + ending_at: ending_at, + dashboard_path: dashboard_path, + description: description + } + + graphql_mutation(:create_annotation, variables) + end + + it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab id.'] + end + end + + context 'when both environment_id and cluster_id are provided' do + let(:mutation) do + variables = { + environment_id: GitlabSchema.id_from_object(environment).to_s, + cluster_id: GitlabSchema.id_from_object(cluster).to_s, + starting_at: starting_at, + ending_at: ending_at, + dashboard_path: dashboard_path, + description: description + } + + graphql_mutation(:create_annotation, variables) + end + + it_behaves_like 'a mutation that returns top-level errors', errors: [described_class::ANNOTATION_SOURCE_ARGUMENT_ERROR] + end + + context 'when a non-cluster or environment id is provided' do + let(:mutation) do + variables = { + environment_id: GitlabSchema.id_from_object(project).to_s, + starting_at: starting_at, + ending_at: ending_at, + dashboard_path: dashboard_path, + description: description + } + + graphql_mutation(:create_annotation, variables) + end + + before do + project.add_developer(current_user) + end + + it_behaves_like 'a mutation that returns top-level errors', errors: [described_class::INVALID_ANNOTATION_SOURCE_ERROR] + 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 cef7fc5cbe3..e1e5fe22887 100644 --- a/spec/requests/api/graphql/mutations/snippets/create_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/create_spec.rb @@ -13,6 +13,7 @@ describe 'Creating a Snippet' do let(:file_name) { 'Initial file_name' } let(:visibility_level) { 'public' } let(:project_path) { nil } + let(:uploaded_files) { nil } let(:mutation) do variables = { @@ -21,7 +22,8 @@ describe 'Creating a Snippet' do visibility_level: visibility_level, file_name: file_name, title: title, - project_path: project_path + project_path: project_path, + uploaded_files: uploaded_files } graphql_mutation(:create_snippet, variables) @@ -31,6 +33,8 @@ describe 'Creating a Snippet' do graphql_mutation_response(:create_snippet) end + subject { post_graphql_mutation(mutation, current_user: current_user) } + context 'when the user does not have permission' do let(:current_user) { nil } @@ -39,7 +43,7 @@ describe 'Creating a Snippet' do it 'does not create the Snippet' do expect do - post_graphql_mutation(mutation, current_user: current_user) + subject end.not_to change { Snippet.count } end @@ -48,7 +52,7 @@ describe 'Creating a Snippet' do it 'does not create the snippet when the user is not authorized' do expect do - post_graphql_mutation(mutation, current_user: current_user) + subject end.not_to change { Snippet.count } end end @@ -60,12 +64,12 @@ describe 'Creating a Snippet' do context 'with PersonalSnippet' do it 'creates the Snippet' do expect do - post_graphql_mutation(mutation, current_user: current_user) + subject end.to change { Snippet.count }.by(1) end it 'returns the created Snippet' do - post_graphql_mutation(mutation, current_user: current_user) + subject expect(mutation_response['snippet']['blob']['richData']).to be_nil expect(mutation_response['snippet']['blob']['plainData']).to match(content) @@ -86,12 +90,12 @@ describe 'Creating a Snippet' do it 'creates the Snippet' do expect do - post_graphql_mutation(mutation, current_user: current_user) + subject end.to change { Snippet.count }.by(1) end it 'returns the created Snippet' do - post_graphql_mutation(mutation, current_user: current_user) + subject expect(mutation_response['snippet']['blob']['richData']).to be_nil expect(mutation_response['snippet']['blob']['plainData']).to match(content) @@ -106,7 +110,7 @@ describe 'Creating a Snippet' do let(:project_path) { 'foobar' } it 'returns an an error' do - post_graphql_mutation(mutation, current_user: current_user) + subject errors = json_response['errors'] expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) @@ -117,7 +121,7 @@ describe 'Creating a Snippet' do it 'returns an an error' do project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::DISABLED) - post_graphql_mutation(mutation, current_user: current_user) + subject errors = json_response['errors'] expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) @@ -132,15 +136,41 @@ describe 'Creating a Snippet' do it 'does not create the Snippet' do expect do - post_graphql_mutation(mutation, current_user: current_user) + subject end.not_to change { Snippet.count } end it 'does not return Snippet' do - post_graphql_mutation(mutation, current_user: current_user) + subject expect(mutation_response['snippet']).to be_nil end end + + context 'when there uploaded files' do + shared_examples 'expected files argument' do |file_value, expected_value| + let(:uploaded_files) { file_value } + + it do + expect(::Snippets::CreateService).to receive(:new).with(nil, user, hash_including(files: expected_value)) + + subject + end + end + + it_behaves_like 'expected files argument', nil, nil + it_behaves_like 'expected files argument', %w(foo bar), %w(foo bar) + it_behaves_like 'expected files argument', 'foo', %w(foo) + + context 'when files has an invalid value' do + let(:uploaded_files) { [1] } + + it 'returns an error' do + subject + + expect(json_response['errors']).to be + end + end + end end end diff --git a/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb b/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb index 351d2db8973..cb9aeea74b2 100644 --- a/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/destroy_spec.rb @@ -6,9 +6,10 @@ describe 'Destroying a Snippet' do include GraphqlHelpers let(:current_user) { snippet.author } + let(:snippet_gid) { snippet.to_global_id.to_s } let(:mutation) do variables = { - id: snippet.to_global_id.to_s + id: snippet_gid } graphql_mutation(:destroy_snippet, variables) @@ -49,9 +50,11 @@ describe 'Destroying a Snippet' do end describe 'PersonalSnippet' do - it_behaves_like 'graphql delete actions' do - let_it_be(:snippet) { create(:personal_snippet) } - end + let_it_be(:snippet) { create(:personal_snippet) } + + it_behaves_like 'graphql delete actions' + + it_behaves_like 'when the snippet is not found' end describe 'ProjectSnippet' do @@ -85,5 +88,7 @@ describe 'Destroying a Snippet' do end end end + + it_behaves_like 'when the snippet is not found' end end diff --git a/spec/requests/api/graphql/mutations/snippets/mark_as_spam_spec.rb b/spec/requests/api/graphql/mutations/snippets/mark_as_spam_spec.rb index 05e3f7e6806..6d4dce3f6f1 100644 --- a/spec/requests/api/graphql/mutations/snippets/mark_as_spam_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/mark_as_spam_spec.rb @@ -10,9 +10,11 @@ describe 'Mark snippet as spam', :do_not_mock_admin_mode do let_it_be(:snippet) { create(:personal_snippet) } let_it_be(:user_agent_detail) { create(:user_agent_detail, subject: snippet) } let(:current_user) { snippet.author } + + let(:snippet_gid) { snippet.to_global_id.to_s } let(:mutation) do variables = { - id: snippet.to_global_id.to_s + id: snippet_gid } graphql_mutation(:mark_as_spam_snippet, variables) @@ -23,13 +25,15 @@ describe 'Mark snippet as spam', :do_not_mock_admin_mode do end shared_examples 'does not mark the snippet as spam' do - it do + specify do expect do post_graphql_mutation(mutation, current_user: current_user) end.not_to change { snippet.reload.user_agent_detail.submitted } end end + it_behaves_like 'when the snippet is not found' + context 'when the user does not have permission' do let(:current_user) { other_user } diff --git a/spec/requests/api/graphql/mutations/snippets/update_spec.rb b/spec/requests/api/graphql/mutations/snippets/update_spec.rb index 1035e3346e1..968ea5aed52 100644 --- a/spec/requests/api/graphql/mutations/snippets/update_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/update_spec.rb @@ -15,9 +15,10 @@ describe 'Updating a Snippet' do let(:updated_file_name) { 'Updated file_name' } let(:current_user) { snippet.author } + let(:snippet_gid) { GitlabSchema.id_from_object(snippet).to_s } let(:mutation) do variables = { - id: GitlabSchema.id_from_object(snippet).to_s, + id: snippet_gid, content: updated_content, description: updated_description, visibility_level: 'public', @@ -90,16 +91,18 @@ describe 'Updating a Snippet' do end describe 'PersonalSnippet' do - it_behaves_like 'graphql update actions' do - let(:snippet) do - create(:personal_snippet, - :private, - file_name: original_file_name, - title: original_title, - content: original_content, - description: original_description) - end + let(:snippet) do + create(:personal_snippet, + :private, + file_name: original_file_name, + title: original_title, + content: original_content, + description: original_description) end + + it_behaves_like 'graphql update actions' + + it_behaves_like 'when the snippet is not found' end describe 'ProjectSnippet' do @@ -142,5 +145,7 @@ describe 'Updating a Snippet' do end end end + + it_behaves_like 'when the snippet is not found' end end diff --git a/spec/requests/api/graphql/project/alert_management/alert_status_counts_spec.rb b/spec/requests/api/graphql/project/alert_management/alert_status_counts_spec.rb new file mode 100644 index 00000000000..ffd328429ef --- /dev/null +++ b/spec/requests/api/graphql/project/alert_management/alert_status_counts_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe 'getting Alert Management Alert counts by status' do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:current_user) { create(:user) } + let_it_be(:alert_1) { create(:alert_management_alert, :resolved, project: project) } + let_it_be(:alert_2) { create(:alert_management_alert, project: project) } + let_it_be(:other_project_alert) { create(:alert_management_alert) } + let(:params) { {} } + + let(:fields) do + <<~QUERY + #{all_graphql_fields_for('AlertManagementAlertStatusCountsType'.classify)} + QUERY + end + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('alertManagementAlertStatusCounts', params, fields) + ) + end + + context 'with alert data' do + let(:alert_counts) { graphql_data.dig('project', 'alertManagementAlertStatusCounts') } + + context 'without project permissions' do + let(:user) { create(:user) } + + before do + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query' + it { expect(alert_counts).to be nil } + end + + context 'with project permissions' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query' + it 'returns the correct counts for each status' do + expect(alert_counts).to eq( + 'open' => 1, + 'all' => 2, + 'triggered' => 1, + 'acknowledged' => 0, + 'resolved' => 1, + 'ignored' => 0 + ) + end + end + end +end diff --git a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb new file mode 100644 index 00000000000..c226e659364 --- /dev/null +++ b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe 'getting Alert Management Alerts' do + include GraphqlHelpers + + let_it_be(:payload) { { 'custom' => { 'alert' => 'payload' } } } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:current_user) { create(:user) } + let_it_be(:resolved_alert) { create(:alert_management_alert, :all_fields, :resolved, project: project, issue: nil, severity: :low) } + let_it_be(:triggered_alert) { create(:alert_management_alert, :all_fields, project: project, severity: :critical, payload: payload) } + let_it_be(:other_project_alert) { create(:alert_management_alert, :all_fields) } + let(:params) { {} } + + let(:fields) do + <<~QUERY + nodes { + #{all_graphql_fields_for('AlertManagementAlert'.classify)} + } + QUERY + end + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('alertManagementAlerts', params, fields) + ) + end + + context 'with alert data' do + let(:alerts) { graphql_data.dig('project', 'alertManagementAlerts', 'nodes') } + + context 'without project permissions' do + let(:user) { create(:user) } + + before do + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query' + + it { expect(alerts).to be nil } + end + + context 'with project permissions' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end + + let(:first_alert) { alerts.first } + let(:second_alert) { alerts.second } + + it_behaves_like 'a working graphql query' + + it { expect(alerts.size).to eq(2) } + + it 'returns the correct properties of the alerts' do + expect(first_alert).to include( + 'iid' => triggered_alert.iid.to_s, + 'issueIid' => triggered_alert.issue_iid.to_s, + 'title' => triggered_alert.title, + 'description' => triggered_alert.description, + 'severity' => triggered_alert.severity.upcase, + 'status' => 'TRIGGERED', + 'monitoringTool' => triggered_alert.monitoring_tool, + 'service' => triggered_alert.service, + 'hosts' => triggered_alert.hosts, + 'eventCount' => triggered_alert.events, + 'startedAt' => triggered_alert.started_at.strftime('%Y-%m-%dT%H:%M:%SZ'), + 'endedAt' => nil, + 'details' => { 'custom.alert' => 'payload' }, + 'createdAt' => triggered_alert.created_at.strftime('%Y-%m-%dT%H:%M:%SZ'), + 'updatedAt' => triggered_alert.updated_at.strftime('%Y-%m-%dT%H:%M:%SZ') + ) + + expect(second_alert).to include( + 'iid' => resolved_alert.iid.to_s, + 'issueIid' => nil, + 'status' => 'RESOLVED', + 'endedAt' => resolved_alert.ended_at.strftime('%Y-%m-%dT%H:%M:%SZ') + ) + end + + context 'with iid given' do + let(:params) { { iid: resolved_alert.iid.to_s } } + + it_behaves_like 'a working graphql query' + + it { expect(alerts.size).to eq(1) } + it { expect(first_alert['iid']).to eq(resolved_alert.iid.to_s) } + end + + context 'with statuses given' do + let(:params) { 'statuses: [TRIGGERED, ACKNOWLEDGED]' } + + it_behaves_like 'a working graphql query' + + it { expect(alerts.size).to eq(1) } + it { expect(first_alert['iid']).to eq(triggered_alert.iid.to_s) } + end + + context 'sorting data given' do + let(:params) { 'sort: SEVERITY_DESC' } + let(:iids) { alerts.map { |a| a['iid'] } } + + it_behaves_like 'a working graphql query' + + it 'sorts in the correct order' do + expect(iids).to eq [resolved_alert.iid.to_s, triggered_alert.iid.to_s] + end + + context 'ascending order' do + let(:params) { 'sort: SEVERITY_ASC' } + + it 'sorts in the correct order' do + expect(iids).to eq [triggered_alert.iid.to_s, resolved_alert.iid.to_s] + end + end + end + + context 'searching' do + let(:params) { { search: resolved_alert.title } } + + it_behaves_like 'a working graphql query' + + it { expect(alerts.size).to eq(1) } + it { expect(first_alert['iid']).to eq(resolved_alert.iid.to_s) } + + context 'unknown criteria' do + let(:params) { { search: 'something random' } } + + it { expect(alerts.size).to eq(0) } + end + end + end + end +end diff --git a/spec/requests/api/graphql/project/grafana_integration_spec.rb b/spec/requests/api/graphql/project/grafana_integration_spec.rb index e7155934b3a..c9bc6c1a68e 100644 --- a/spec/requests/api/graphql/project/grafana_integration_spec.rb +++ b/spec/requests/api/graphql/project/grafana_integration_spec.rb @@ -35,7 +35,7 @@ describe 'Getting Grafana Integration' do it_behaves_like 'a working graphql query' - it { expect(integration_data).to be nil } + specify { expect(integration_data).to be nil } end context 'with project admin permissions' do @@ -45,16 +45,16 @@ describe 'Getting Grafana Integration' do it_behaves_like 'a working graphql query' - it { expect(integration_data['token']).to eql grafana_integration.masked_token } - it { expect(integration_data['grafanaUrl']).to eql grafana_integration.grafana_url } + specify { expect(integration_data['token']).to eql grafana_integration.masked_token } + specify { expect(integration_data['grafanaUrl']).to eql grafana_integration.grafana_url } - it do + specify do expect( integration_data['createdAt'] ).to eql grafana_integration.created_at.strftime('%Y-%m-%dT%H:%M:%SZ') end - it do + specify do expect( integration_data['updatedAt'] ).to eql grafana_integration.updated_at.strftime('%Y-%m-%dT%H:%M:%SZ') diff --git a/spec/requests/api/graphql/project/issue/design_collection/version_spec.rb b/spec/requests/api/graphql/project/issue/design_collection/version_spec.rb new file mode 100644 index 00000000000..04f445b4318 --- /dev/null +++ b/spec/requests/api/graphql/project/issue/design_collection/version_spec.rb @@ -0,0 +1,216 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Query.project(fullPath).issue(iid).designCollection.version(sha)' do + include GraphqlHelpers + include DesignManagementTestHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:developer) { create(:user) } + let_it_be(:stranger) { create(:user) } + let_it_be(:old_version) do + create(:design_version, issue: issue, + created_designs: create_list(:design, 3, issue: issue)) + end + let_it_be(:version) do + create(:design_version, issue: issue, + modified_designs: old_version.designs, + created_designs: create_list(:design, 2, issue: issue)) + end + + let(:current_user) { developer } + + def query(vq = version_fields) + graphql_query_for(:project, { fullPath: project.full_path }, + query_graphql_field(:issue, { iid: issue.iid.to_s }, + query_graphql_field(:design_collection, nil, + query_graphql_field(:version, { sha: version.sha }, vq)))) + end + + let(:post_query) { post_graphql(query, current_user: current_user) } + let(:path_prefix) { %w[project issue designCollection version] } + + let(:data) { graphql_data.dig(*path) } + + before do + enable_design_management + project.add_developer(developer) + end + + describe 'scalar fields' do + let(:path) { path_prefix } + let(:version_fields) { query_graphql_field(:sha) } + + before do + post_query + end + + { id: ->(x) { x.to_global_id.to_s }, sha: ->(x) { x.sha } }.each do |field, value| + describe ".#{field}" do + let(:version_fields) { query_graphql_field(field) } + + it "retrieves the #{field}" do + expect(data).to match(a_hash_including(field.to_s => value[version])) + end + end + end + end + + describe 'design_at_version' do + let(:path) { path_prefix + %w[designAtVersion] } + let(:design) { issue.designs.visible_at_version(version).to_a.sample } + let(:design_at_version) { build(:design_at_version, design: design, version: version) } + + let(:version_fields) do + query_graphql_field(:design_at_version, dav_params, 'id filename') + end + + shared_examples :finds_dav do + it 'finds all the designs as of the given version' do + post_query + + expect(data).to match( + a_hash_including( + 'id' => global_id_of(design_at_version), + 'filename' => design.filename + )) + end + + context 'when the current_user is not authorized' do + let(:current_user) { stranger } + + it 'returns nil' do + post_query + + expect(data).to be_nil + end + end + end + + context 'by ID' do + let(:dav_params) { { id: global_id_of(design_at_version) } } + + include_examples :finds_dav + end + + context 'by filename' do + let(:dav_params) { { filename: design.filename } } + + include_examples :finds_dav + end + + context 'by design_id' do + let(:dav_params) { { design_id: global_id_of(design) } } + + include_examples :finds_dav + end + end + + describe 'designs_at_version' do + let(:path) { path_prefix + %w[designsAtVersion edges] } + let(:version_fields) do + query_graphql_field(:designs_at_version, dav_params, 'edges { node { id filename } }') + end + + let(:dav_params) { nil } + + let(:results) do + issue.designs.visible_at_version(version).map do |d| + dav = build(:design_at_version, design: d, version: version) + { 'id' => global_id_of(dav), 'filename' => d.filename } + end + end + + it 'finds all the designs as of the given version' do + post_query + + expect(data.pluck('node')).to match_array(results) + end + + describe 'filtering' do + let(:designs) { issue.designs.sample(3) } + let(:filenames) { designs.map(&:filename) } + let(:ids) do + designs.map { |d| global_id_of(build(:design_at_version, design: d, version: version)) } + end + + before do + post_query + end + + describe 'by filename' do + let(:dav_params) { { filenames: filenames } } + + it 'finds the designs by filename' do + expect(data.map { |e| e.dig('node', 'id') }).to match_array(ids) + end + end + + describe 'by design-id' do + let(:dav_params) { { ids: designs.map { |d| global_id_of(d) } } } + + it 'finds the designs by id' do + expect(data.map { |e| e.dig('node', 'filename') }).to match_array(filenames) + end + end + end + + describe 'pagination' do + let(:end_cursor) { graphql_data_at(*path_prefix, :designs_at_version, :page_info, :end_cursor) } + + let(:ids) do + ::DesignManagement::Design.visible_at_version(version).order(:id).map do |d| + global_id_of(build(:design_at_version, design: d, version: version)) + end + end + + let(:version_fields) do + query_graphql_field(:designs_at_version, { first: 2 }, fields) + end + + let(:cursored_query) do + frag = query_graphql_field(:designs_at_version, { after: end_cursor }, fields) + query(frag) + end + + let(:fields) { ['pageInfo { endCursor }', 'edges { node { id } }'] } + + def response_values(data = graphql_data) + data.dig(*path).map { |e| e.dig('node', 'id') } + end + + it 'sorts designs for reliable pagination' do + post_graphql(query, current_user: current_user) + + expect(response_values).to match_array(ids.take(2)) + + post_graphql(cursored_query, current_user: current_user) + + new_data = Gitlab::Json.parse(response.body).fetch('data') + + expect(response_values(new_data)).to match_array(ids.drop(2)) + end + end + end + + describe 'designs' do + let(:path) { path_prefix + %w[designs edges] } + let(:version_fields) do + query_graphql_field(:designs, nil, 'edges { node { id filename } }') + end + + let(:results) do + version.designs.map do |design| + { 'id' => global_id_of(design), 'filename' => design.filename } + end + end + + it 'finds all the designs as of the given version' do + post_query + + expect(data.pluck('node')).to match_array(results) + end + end +end diff --git a/spec/requests/api/graphql/project/issue/design_collection/versions_spec.rb b/spec/requests/api/graphql/project/issue/design_collection/versions_spec.rb new file mode 100644 index 00000000000..18787bf925d --- /dev/null +++ b/spec/requests/api/graphql/project/issue/design_collection/versions_spec.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Getting versions related to an issue' do + include GraphqlHelpers + include DesignManagementTestHelpers + + let_it_be(:issue) { create(:issue) } + + let_it_be(:version_a) do + create(:design_version, issue: issue) + end + let_it_be(:version_b) do + create(:design_version, issue: issue) + end + let_it_be(:version_c) do + create(:design_version, issue: issue) + end + let_it_be(:version_d) do + create(:design_version, issue: issue) + end + + let_it_be(:owner) { issue.project.owner } + + def version_query(params = version_params) + query_graphql_field(:versions, params, version_query_fields) + end + + let(:version_params) { nil } + + let(:version_query_fields) { ['edges { node { sha } }'] } + + let(:project) { issue.project } + let(:current_user) { owner } + + let(:query) { make_query } + + def make_query(vq = version_query) + graphql_query_for(:project, { fullPath: project.full_path }, + query_graphql_field(:issue, { iid: issue.iid.to_s }, + query_graphql_field(:design_collection, {}, vq))) + end + + let(:design_collection) do + graphql_data_at(:project, :issue, :design_collection) + end + + def response_values(data = graphql_data, key = 'sha') + path = %w[project issue designCollection versions edges] + data.dig(*path).map { |e| e.dig('node', key) } + end + + before do + enable_design_management + end + + it 'returns the design filename' do + post_graphql(query, current_user: current_user) + + expect(response_values).to match_array([version_a, version_b, version_c, version_d].map(&:sha)) + end + + describe 'filter by sha' do + let(:sha) { version_b.sha } + + let(:version_params) { { earlier_or_equal_to_sha: sha } } + + it 'finds only those versions at or before the given cut-off' do + post_graphql(query, current_user: current_user) + + expect(response_values).to contain_exactly(version_a.sha, version_b.sha) + end + end + + describe 'filter by id' do + let(:id) { global_id_of(version_c) } + + let(:version_params) { { earlier_or_equal_to_id: id } } + + it 'finds only those versions at or before the given cut-off' do + post_graphql(query, current_user: current_user) + + expect(response_values).to contain_exactly(version_a.sha, version_b.sha, version_c.sha) + end + end + + describe 'pagination' do + let(:end_cursor) { design_collection.dig('versions', 'pageInfo', 'endCursor') } + + let(:ids) { issue.design_collection.versions.ordered.map(&:sha) } + + let(:query) { make_query(version_query(first: 2)) } + + let(:cursored_query) do + make_query(version_query(after: end_cursor)) + end + + let(:version_query_fields) { ['pageInfo { endCursor }', 'edges { node { sha } }'] } + + it 'sorts designs for reliable pagination' do + post_graphql(query, current_user: current_user) + + expect(response_values).to match_array(ids.take(2)) + + post_graphql(cursored_query, current_user: current_user) + + new_data = Gitlab::Json.parse(response.body).fetch('data') + + expect(response_values(new_data)).to match_array(ids.drop(2)) + end + end +end diff --git a/spec/requests/api/graphql/project/issue/designs/designs_spec.rb b/spec/requests/api/graphql/project/issue/designs/designs_spec.rb new file mode 100644 index 00000000000..b6fd0d91bda --- /dev/null +++ b/spec/requests/api/graphql/project/issue/designs/designs_spec.rb @@ -0,0 +1,388 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Getting designs related to an issue' do + include GraphqlHelpers + include DesignManagementTestHelpers + + let_it_be(:design) { create(:design, :with_smaller_image_versions, versions_count: 1) } + let_it_be(:current_user) { design.project.owner } + let(:design_query) do + <<~NODE + designs { + edges { + node { + id + filename + fullPath + event + image + imageV432x230 + } + } + } + NODE + end + let(:issue) { design.issue } + let(:project) { issue.project } + let(:query) { make_query } + let(:design_collection) do + graphql_data_at(:project, :issue, :design_collection) + end + let(:design_response) do + design_collection.dig('designs', 'edges').first['node'] + end + + def make_query(dq = design_query) + designs_field = query_graphql_field(:design_collection, {}, dq) + issue_field = query_graphql_field(:issue, { iid: issue.iid.to_s }, designs_field) + + graphql_query_for(:project, { fullPath: project.full_path }, issue_field) + end + + def design_image_url(design, ref: nil, size: nil) + Gitlab::UrlBuilder.build(design, ref: ref, size: size) + end + + context 'when the feature is available' do + before do + enable_design_management + end + + it 'returns the design properties correctly' do + version_sha = design.versions.first.sha + + post_graphql(query, current_user: current_user) + + expect(design_response).to eq( + 'id' => design.to_global_id.to_s, + 'event' => 'CREATION', + 'fullPath' => design.full_path, + 'filename' => design.filename, + 'image' => design_image_url(design, ref: version_sha), + 'imageV432x230' => design_image_url(design, ref: version_sha, size: :v432x230) + ) + end + + context 'when the v432x230-sized design image has not been processed' do + before do + allow_next_instance_of(DesignManagement::DesignV432x230Uploader) do |uploader| + allow(uploader).to receive(:file).and_return(nil) + end + end + + it 'returns nil for the v432x230-sized design image' do + post_graphql(query, current_user: current_user) + + expect(design_response['imageV432x230']).to be_nil + end + end + + describe 'pagination' do + before do + create_list(:design, 5, :with_file, issue: issue) + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end + + let(:issue) { create(:issue) } + + let(:end_cursor) { design_collection.dig('designs', 'pageInfo', 'endCursor') } + + let(:ids) { issue.designs.order(:id).map { |d| global_id_of(d) } } + + let(:query) { make_query(designs_fragment(first: 2)) } + + let(:design_query_fields) { 'pageInfo { endCursor } edges { node { id } }' } + + let(:cursored_query) do + make_query(designs_fragment(after: end_cursor)) + end + + def designs_fragment(params) + query_graphql_field(:designs, params, design_query_fields) + end + + def response_ids(data = graphql_data) + path = %w[project issue designCollection designs edges] + data.dig(*path).map { |e| e.dig('node', 'id') } + end + + it 'sorts designs for reliable pagination' do + expect(response_ids).to match_array(ids.take(2)) + + post_graphql(cursored_query, current_user: current_user) + + new_data = Gitlab::Json.parse(response.body).fetch('data') + + expect(response_ids(new_data)).to match_array(ids.drop(2)) + end + end + + context 'with versions' do + let_it_be(:version) { design.versions.take } + let(:design_query) do + <<~NODE + designs { + edges { + node { + filename + versions { + edges { + node { + id + sha + } + } + } + } + } + } + NODE + end + + it 'includes the version id' do + post_graphql(query, current_user: current_user) + + version_id = design_response['versions']['edges'].first['node']['id'] + + expect(version_id).to eq(version.to_global_id.to_s) + end + + it 'includes the version sha' do + post_graphql(query, current_user: current_user) + + version_sha = design_response['versions']['edges'].first['node']['sha'] + + expect(version_sha).to eq(version.sha) + end + end + + describe 'viewing a design board at a particular version' do + let_it_be(:issue) { design.issue } + let_it_be(:second_design, reload: true) { create(:design, :with_smaller_image_versions, issue: issue, versions_count: 1) } + let_it_be(:deleted_design) { create(:design, :with_versions, issue: issue, deleted: true, versions_count: 1) } + let(:all_versions) { issue.design_versions.ordered.reverse } + let(:design_query) do + <<~NODE + designs(atVersion: "#{version.to_global_id}") { + edges { + node { + id + image + imageV432x230 + event + versions { + edges { + node { + id + } + } + } + } + } + } + NODE + end + let(:design_response) do + design_collection['designs']['edges'] + end + + def global_id(object) + object.to_global_id.to_s + end + + # Filters just design nodes from the larger `design_response` + def design_nodes + design_response.map do |response| + response['node'] + end + end + + # Filters just version nodes from the larger `design_response` + def version_nodes + design_response.map do |response| + response.dig('node', 'versions', 'edges') + end + end + + context 'viewing the original version, when one design was created' do + let(:version) { all_versions.first } + + before do + post_graphql(query, current_user: current_user) + end + + it 'only returns the first design' do + expect(design_nodes).to contain_exactly( + a_hash_including('id' => global_id(design)) + ) + end + + it 'returns the correct full-sized design image' do + expect(design_nodes).to contain_exactly( + a_hash_including('image' => design_image_url(design, ref: version.sha)) + ) + end + + it 'returns the correct v432x230-sized design image' do + expect(design_nodes).to contain_exactly( + a_hash_including('imageV432x230' => design_image_url(design, ref: version.sha, size: :v432x230)) + ) + end + + it 'returns the correct event for the design in this version' do + expect(design_nodes).to contain_exactly( + a_hash_including('event' => 'CREATION') + ) + end + + it 'only returns one version record for the design (the original version)' do + expect(version_nodes).to eq([ + [{ 'node' => { 'id' => global_id(version) } }] + ]) + end + end + + context 'viewing the second version, when one design was created' do + let(:version) { all_versions.second } + + before do + post_graphql(query, current_user: current_user) + end + + it 'only returns the first two designs' do + expect(design_nodes).to contain_exactly( + a_hash_including('id' => global_id(design)), + a_hash_including('id' => global_id(second_design)) + ) + end + + it 'returns the correct full-sized design images' do + expect(design_nodes).to contain_exactly( + a_hash_including('image' => design_image_url(design, ref: version.sha)), + a_hash_including('image' => design_image_url(second_design, ref: version.sha)) + ) + end + + it 'returns the correct v432x230-sized design images' do + expect(design_nodes).to contain_exactly( + a_hash_including('imageV432x230' => design_image_url(design, ref: version.sha, size: :v432x230)), + a_hash_including('imageV432x230' => design_image_url(second_design, ref: version.sha, size: :v432x230)) + ) + end + + it 'returns the correct events for the designs in this version' do + expect(design_nodes).to contain_exactly( + a_hash_including('event' => 'NONE'), + a_hash_including('event' => 'CREATION') + ) + end + + it 'returns the correct versions records for both designs' do + expect(version_nodes).to eq([ + [{ 'node' => { 'id' => global_id(design.versions.first) } }], + [{ 'node' => { 'id' => global_id(second_design.versions.first) } }] + ]) + end + end + + context 'viewing the last version, when one design was deleted and one was updated' do + let(:version) { all_versions.last } + let!(:second_design_update) do + create(:design_action, :with_image_v432x230, design: second_design, version: version, event: 'modification') + end + + before do + post_graphql(query, current_user: current_user) + end + + it 'does not include the deleted design' do + # The design does exist in the version + expect(version.designs).to include(deleted_design) + + # But the GraphQL API does not include it in these results + expect(design_nodes).to contain_exactly( + a_hash_including('id' => global_id(design)), + a_hash_including('id' => global_id(second_design)) + ) + end + + it 'returns the correct full-sized design images' do + expect(design_nodes).to contain_exactly( + a_hash_including('image' => design_image_url(design, ref: version.sha)), + a_hash_including('image' => design_image_url(second_design, ref: version.sha)) + ) + end + + it 'returns the correct v432x230-sized design images' do + expect(design_nodes).to contain_exactly( + a_hash_including('imageV432x230' => design_image_url(design, ref: version.sha, size: :v432x230)), + a_hash_including('imageV432x230' => design_image_url(second_design, ref: version.sha, size: :v432x230)) + ) + end + + it 'returns the correct events for the designs in this version' do + expect(design_nodes).to contain_exactly( + a_hash_including('event' => 'NONE'), + a_hash_including('event' => 'MODIFICATION') + ) + end + + it 'returns all versions records for the designs' do + expect(version_nodes).to eq([ + [ + { 'node' => { 'id' => global_id(design.versions.first) } } + ], + [ + { 'node' => { 'id' => global_id(second_design.versions.second) } }, + { 'node' => { 'id' => global_id(second_design.versions.first) } } + ] + ]) + end + end + end + + describe 'a design with note annotations' do + let_it_be(:note) { create(:diff_note_on_design, noteable: design) } + + let(:design_query) do + <<~NODE + designs { + edges { + node { + notesCount + notes { + edges { + node { + id + } + } + } + } + } + } + NODE + end + + let(:design_response) do + design_collection['designs']['edges'].first['node'] + end + + before do + post_graphql(query, current_user: current_user) + end + + it 'returns the notes for the design' do + expect(design_response.dig('notes', 'edges')).to eq( + ['node' => { 'id' => note.to_global_id.to_s }] + ) + end + + it 'returns a note_count for the design' do + expect(design_response['notesCount']).to eq(1) + end + end + end +end diff --git a/spec/requests/api/graphql/project/issue/designs/notes_spec.rb b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb new file mode 100644 index 00000000000..0207bb9123a --- /dev/null +++ b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Getting designs related to an issue' do + include GraphqlHelpers + include DesignManagementTestHelpers + + let_it_be(:project) { create(:project, :public) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:design) { create(:design, :with_file, versions_count: 1, issue: issue) } + let_it_be(:current_user) { project.owner } + let_it_be(:note) { create(:diff_note_on_design, noteable: design, project: project) } + + before do + enable_design_management + + note + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + it 'is not too deep for anonymous users' do + note_fields = <<~FIELDS + id + author { name } + FIELDS + + post_graphql(query(note_fields), current_user: nil) + + designs_data = graphql_data['project']['issue']['designs']['designs'] + design_data = designs_data['edges'].first['node'] + note_data = design_data['notes']['edges'].first['node'] + + expect(note_data['id']).to eq(note.to_global_id.to_s) + end + + def query(note_fields = all_graphql_fields_for(Note)) + design_node = <<~NODE + designs { + edges { + node { + notes { + edges { + node { + #{note_fields} + } + } + } + } + } + } + NODE + graphql_query_for( + 'project', + { 'fullPath' => design.project.full_path }, + query_graphql_field( + 'issue', + { iid: design.issue.iid.to_s }, + query_graphql_field( + 'designs', {}, design_node + ) + ) + ) + end +end diff --git a/spec/requests/api/graphql/project/issue_spec.rb b/spec/requests/api/graphql/project/issue_spec.rb new file mode 100644 index 00000000000..92d2f9d0d31 --- /dev/null +++ b/spec/requests/api/graphql/project/issue_spec.rb @@ -0,0 +1,189 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Query.project(fullPath).issue(iid)' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:issue_b) { create(:issue, project: project) } + let_it_be(:developer) { create(:user) } + let(:current_user) { developer } + + let_it_be(:project_params) { { 'fullPath' => project.full_path } } + let_it_be(:issue_params) { { 'iid' => issue.iid.to_s } } + let_it_be(:issue_fields) { 'title' } + + let(:query) do + graphql_query_for('project', project_params, project_fields) + end + + let(:project_fields) do + query_graphql_field(:issue, issue_params, issue_fields) + end + + shared_examples 'being able to fetch a design-like object by ID' do + let(:design) { design_a } + let(:path) { %w[project issue designCollection] + [GraphqlHelpers.fieldnamerize(object_field_name)] } + + let(:design_fields) do + [ + query_graphql_field(:filename), + query_graphql_field(:project, nil, query_graphql_field(:id)) + ] + end + + let(:design_collection_fields) do + query_graphql_field(object_field_name, object_params, object_fields) + end + + let(:object_fields) { design_fields } + + context 'the ID is passed' do + let(:object_params) { { id: global_id_of(object) } } + let(:result_fields) { {} } + + let(:expected_fields) do + result_fields.merge({ 'filename' => design.filename, 'project' => id_hash(project) }) + end + + it 'retrieves the object' do + post_query + + data = graphql_data.dig(*path) + + expect(data).to match(a_hash_including(expected_fields)) + end + + context 'the user is unauthorized' do + let(:current_user) { create(:user) } + + it_behaves_like 'a failure to find anything' + end + end + + context 'without parameters' do + let(:object_params) { nil } + + it 'raises an error' do + post_query + + expect(graphql_errors).to include(no_argument_error) + end + end + + context 'attempting to retrieve an object from a different issue' do + let(:object_params) { { id: global_id_of(object_on_other_issue) } } + + it_behaves_like 'a failure to find anything' + end + end + + before do + project.add_developer(developer) + end + + let(:post_query) { post_graphql(query, current_user: current_user) } + + describe '.designCollection' do + include DesignManagementTestHelpers + + let_it_be(:design_a) { create(:design, issue: issue) } + let_it_be(:version_a) { create(:design_version, issue: issue, created_designs: [design_a]) } + + let(:issue_fields) do + query_graphql_field(:design_collection, dc_params, design_collection_fields) + end + + let(:dc_params) { nil } + let(:design_collection_fields) { nil } + + before do + enable_design_management + end + + describe '.design' do + let(:object) { design } + let(:object_field_name) { :design } + + let(:no_argument_error) do + custom_graphql_error(path, a_string_matching(%r/id or filename/)) + end + + let_it_be(:object_on_other_issue) { create(:design, issue: issue_b) } + + it_behaves_like 'being able to fetch a design-like object by ID' + + it_behaves_like 'being able to fetch a design-like object by ID' do + let(:object_params) { { filename: design.filename } } + end + end + + describe '.version' do + let(:version) { version_a } + let(:path) { %w[project issue designCollection version] } + + let(:design_collection_fields) do + query_graphql_field(:version, version_params, 'id sha') + end + + context 'no parameters' do + let(:version_params) { nil } + + it 'raises an error' do + post_query + + expect(graphql_errors).to include(custom_graphql_error(path, a_string_matching(%r/id or sha/))) + end + end + + shared_examples 'a successful query for a version' do + it 'finds the version' do + post_query + + data = graphql_data.dig(*path) + + expect(data).to match( + a_hash_including('id' => global_id_of(version), + 'sha' => version.sha) + ) + end + end + + context '(sha: STRING_TYPE)' do + let(:version_params) { { sha: version.sha } } + + it_behaves_like 'a successful query for a version' + end + + context '(id: ID_TYPE)' do + let(:version_params) { { id: global_id_of(version) } } + + it_behaves_like 'a successful query for a version' + end + end + + describe '.designAtVersion' do + it_behaves_like 'being able to fetch a design-like object by ID' do + let(:object) { build(:design_at_version, design: design, version: version) } + let(:object_field_name) { :design_at_version } + + let(:version) { version_a } + + let(:result_fields) { { 'version' => id_hash(version) } } + let(:object_fields) do + design_fields + [query_graphql_field(:version, nil, query_graphql_field(:id))] + end + + let(:no_argument_error) { missing_required_argument(path, :id) } + + let(:object_on_other_issue) { build(:design_at_version, issue: issue_b) } + end + end + end + + def id_hash(object) + a_hash_including('id' => global_id_of(object)) + end +end diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb index 4ce7a3912a3..91fce3eed92 100644 --- a/spec/requests/api/graphql/project/issues_spec.rb +++ b/spec/requests/api/graphql/project/issues_spec.rb @@ -45,8 +45,8 @@ describe 'getting an issue list for a project' do it 'includes discussion locked' do post_graphql(query, current_user: current_user) - expect(issues_data[0]['node']['discussionLocked']).to eq false - expect(issues_data[1]['node']['discussionLocked']).to eq true + expect(issues_data[0]['node']['discussionLocked']).to eq(false) + expect(issues_data[1]['node']['discussionLocked']).to eq(true) end context 'when limiting the number of results' do @@ -79,7 +79,7 @@ describe 'getting an issue list for a project' do post_graphql(query) - expect(issues_data).to eq [] + expect(issues_data).to eq([]) end end @@ -118,131 +118,138 @@ describe 'getting an issue list for a project' do end describe 'sorting and pagination' do - let(:start_cursor) { graphql_data['project']['issues']['pageInfo']['startCursor'] } - let(:end_cursor) { graphql_data['project']['issues']['pageInfo']['endCursor'] } + let_it_be(:data_path) { [:project, :issues] } - context 'when sorting by due date' do - let(:sort_project) { create(:project, :public) } - - let!(:due_issue1) { create(:issue, project: sort_project, due_date: 3.days.from_now) } - let!(:due_issue2) { create(:issue, project: sort_project, due_date: nil) } - let!(:due_issue3) { create(:issue, project: sort_project, due_date: 2.days.ago) } - let!(:due_issue4) { create(:issue, project: sort_project, due_date: nil) } - let!(:due_issue5) { create(:issue, project: sort_project, due_date: 1.day.ago) } - - let(:params) { 'sort: DUE_DATE_ASC' } - - def query(issue_params = params) - graphql_query_for( - 'project', - { 'fullPath' => sort_project.full_path }, - <<~ISSUES - issues(#{issue_params}) { - pageInfo { - endCursor - } - edges { - node { - iid - dueDate - } - } - } - ISSUES - ) - end + def pagination_query(params, page_info) + graphql_query_for( + 'project', + { 'fullPath' => sort_project.full_path }, + "issues(#{params}) { #{page_info} edges { node { iid dueDate } } }" + ) + end - before do - post_graphql(query, current_user: current_user) - end + def pagination_results_data(data) + data.map { |issue| issue.dig('node', 'iid').to_i } + end - it_behaves_like 'a working graphql query' + context 'when sorting by due date' do + let_it_be(:sort_project) { create(:project, :public) } + let_it_be(:due_issue1) { create(:issue, project: sort_project, due_date: 3.days.from_now) } + let_it_be(:due_issue2) { create(:issue, project: sort_project, due_date: nil) } + let_it_be(:due_issue3) { create(:issue, project: sort_project, due_date: 2.days.ago) } + let_it_be(:due_issue4) { create(:issue, project: sort_project, due_date: nil) } + let_it_be(:due_issue5) { create(:issue, project: sort_project, due_date: 1.day.ago) } context 'when ascending' do - it 'sorts issues' do - expect(grab_iids).to eq [due_issue3.iid, due_issue5.iid, due_issue1.iid, due_issue4.iid, due_issue2.iid] - end - - context 'when paginating' do - let(:params) { 'sort: DUE_DATE_ASC, first: 2' } - - it 'sorts issues' do - expect(grab_iids).to eq [due_issue3.iid, due_issue5.iid] - - cursored_query = query("sort: DUE_DATE_ASC, after: \"#{end_cursor}\"") - post_graphql(cursored_query, current_user: current_user) - response_data = JSON.parse(response.body)['data']['project']['issues']['edges'] - - expect(grab_iids(response_data)).to eq [due_issue1.iid, due_issue4.iid, due_issue2.iid] - end + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'DUE_DATE_ASC' } + let(:first_param) { 2 } + let(:expected_results) { [due_issue3.iid, due_issue5.iid, due_issue1.iid, due_issue4.iid, due_issue2.iid] } end end context 'when descending' do - let(:params) { 'sort: DUE_DATE_DESC' } - - it 'sorts issues' do - expect(grab_iids).to eq [due_issue1.iid, due_issue5.iid, due_issue3.iid, due_issue4.iid, due_issue2.iid] + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'DUE_DATE_DESC' } + let(:first_param) { 2 } + let(:expected_results) { [due_issue1.iid, due_issue5.iid, due_issue3.iid, due_issue4.iid, due_issue2.iid] } end + end + end - context 'when paginating' do - let(:params) { 'sort: DUE_DATE_DESC, first: 2' } - - it 'sorts issues' do - expect(grab_iids).to eq [due_issue1.iid, due_issue5.iid] - - cursored_query = query("sort: DUE_DATE_DESC, after: \"#{end_cursor}\"") - post_graphql(cursored_query, current_user: current_user) - response_data = JSON.parse(response.body)['data']['project']['issues']['edges'] + context 'when sorting by relative position' do + let_it_be(:sort_project) { create(:project, :public) } + let_it_be(:relative_issue1) { create(:issue, project: sort_project, relative_position: 2000) } + let_it_be(:relative_issue2) { create(:issue, project: sort_project, relative_position: nil) } + let_it_be(:relative_issue3) { create(:issue, project: sort_project, relative_position: 1000) } + let_it_be(:relative_issue4) { create(:issue, project: sort_project, relative_position: nil) } + let_it_be(:relative_issue5) { create(:issue, project: sort_project, relative_position: 500) } - expect(grab_iids(response_data)).to eq [due_issue3.iid, due_issue4.iid, due_issue2.iid] - end + context 'when ascending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'RELATIVE_POSITION_ASC' } + let(:first_param) { 2 } + let(:expected_results) { [relative_issue5.iid, relative_issue3.iid, relative_issue1.iid, relative_issue4.iid, relative_issue2.iid] } end end end - context 'when sorting by relative position' do - let(:sort_project) { create(:project, :public) } - - let!(:relative_issue1) { create(:issue, project: sort_project, relative_position: 2000) } - let!(:relative_issue2) { create(:issue, project: sort_project, relative_position: nil) } - let!(:relative_issue3) { create(:issue, project: sort_project, relative_position: 1000) } - let!(:relative_issue4) { create(:issue, project: sort_project, relative_position: nil) } - let!(:relative_issue5) { create(:issue, project: sort_project, relative_position: 500) } - - let(:params) { 'sort: RELATIVE_POSITION_ASC' } - - def query(issue_params = params) - graphql_query_for( - 'project', - { 'fullPath' => sort_project.full_path }, - "issues(#{issue_params}) { pageInfo { endCursor} edges { node { iid dueDate } } }" - ) + context 'when sorting by priority' do + let_it_be(:sort_project) { create(:project, :public) } + let_it_be(:early_milestone) { create(:milestone, project: sort_project, due_date: 10.days.from_now) } + let_it_be(:late_milestone) { create(:milestone, project: sort_project, due_date: 30.days.from_now) } + let_it_be(:priority_label1) { create(:label, project: sort_project, priority: 1) } + let_it_be(:priority_label2) { create(:label, project: sort_project, priority: 5) } + let_it_be(:priority_issue1) { create(:issue, project: sort_project, labels: [priority_label1], milestone: late_milestone) } + let_it_be(:priority_issue2) { create(:issue, project: sort_project, labels: [priority_label2]) } + let_it_be(:priority_issue3) { create(:issue, project: sort_project, milestone: early_milestone) } + let_it_be(:priority_issue4) { create(:issue, project: sort_project) } + + context 'when ascending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'PRIORITY_ASC' } + let(:first_param) { 2 } + let(:expected_results) { [priority_issue3.iid, priority_issue1.iid, priority_issue2.iid, priority_issue4.iid] } + end end - before do - post_graphql(query, current_user: current_user) + context 'when descending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'PRIORITY_DESC' } + let(:first_param) { 2 } + let(:expected_results) { [priority_issue1.iid, priority_issue3.iid, priority_issue2.iid, priority_issue4.iid] } + end end + end - it_behaves_like 'a working graphql query' + context 'when sorting by label priority' do + let_it_be(:sort_project) { create(:project, :public) } + let_it_be(:label1) { create(:label, project: sort_project, priority: 1) } + let_it_be(:label2) { create(:label, project: sort_project, priority: 5) } + let_it_be(:label3) { create(:label, project: sort_project, priority: 10) } + let_it_be(:label_issue1) { create(:issue, project: sort_project, labels: [label1]) } + let_it_be(:label_issue2) { create(:issue, project: sort_project, labels: [label2]) } + let_it_be(:label_issue3) { create(:issue, project: sort_project, labels: [label1, label3]) } + let_it_be(:label_issue4) { create(:issue, project: sort_project) } context 'when ascending' do - it 'sorts issues' do - expect(grab_iids).to eq [relative_issue5.iid, relative_issue3.iid, relative_issue1.iid, relative_issue4.iid, relative_issue2.iid] + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'LABEL_PRIORITY_ASC' } + let(:first_param) { 2 } + let(:expected_results) { [label_issue3.iid, label_issue1.iid, label_issue2.iid, label_issue4.iid] } end + end - context 'when paginating' do - let(:params) { 'sort: RELATIVE_POSITION_ASC, first: 2' } + context 'when descending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'LABEL_PRIORITY_DESC' } + let(:first_param) { 2 } + let(:expected_results) { [label_issue2.iid, label_issue3.iid, label_issue1.iid, label_issue4.iid] } + end + end + end - it 'sorts issues' do - expect(grab_iids).to eq [relative_issue5.iid, relative_issue3.iid] + context 'when sorting by milestone due date' do + let_it_be(:sort_project) { create(:project, :public) } + let_it_be(:early_milestone) { create(:milestone, project: sort_project, due_date: 10.days.from_now) } + let_it_be(:late_milestone) { create(:milestone, project: sort_project, due_date: 30.days.from_now) } + let_it_be(:milestone_issue1) { create(:issue, project: sort_project) } + let_it_be(:milestone_issue2) { create(:issue, project: sort_project, milestone: early_milestone) } + let_it_be(:milestone_issue3) { create(:issue, project: sort_project, milestone: late_milestone) } - cursored_query = query("sort: RELATIVE_POSITION_ASC, after: \"#{end_cursor}\"") - post_graphql(cursored_query, current_user: current_user) - response_data = JSON.parse(response.body)['data']['project']['issues']['edges'] + context 'when ascending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'MILESTONE_DUE_ASC' } + let(:first_param) { 2 } + let(:expected_results) { [milestone_issue2.iid, milestone_issue3.iid, milestone_issue1.iid] } + end + end - expect(grab_iids(response_data)).to eq [relative_issue1.iid, relative_issue4.iid, relative_issue2.iid] - end + context 'when descending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'MILESTONE_DUE_DESC' } + let(:first_param) { 2 } + let(:expected_results) { [milestone_issue3.iid, milestone_issue2.iid, milestone_issue1.iid] } end end end diff --git a/spec/requests/api/graphql/project/jira_import_spec.rb b/spec/requests/api/graphql/project/jira_import_spec.rb index 43e1bb13342..e063068eb1a 100644 --- a/spec/requests/api/graphql/project/jira_import_spec.rb +++ b/spec/requests/api/graphql/project/jira_import_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'query jira import data' do +describe 'query Jira import data' do include GraphqlHelpers let_it_be(:current_user) { create(:user) } @@ -18,6 +18,7 @@ describe 'query jira import data' do jiraImports { nodes { jiraProjectKey + createdAt scheduledAt scheduledBy { username diff --git a/spec/requests/api/graphql/query_spec.rb b/spec/requests/api/graphql/query_spec.rb new file mode 100644 index 00000000000..26b4c6eafd7 --- /dev/null +++ b/spec/requests/api/graphql/query_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Query' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:developer) { create(:user) } + let(:current_user) { developer } + + describe '.designManagement' do + include DesignManagementTestHelpers + + let_it_be(:version) { create(:design_version, issue: issue) } + let_it_be(:design) { version.designs.first } + let(:query_result) { graphql_data.dig(*path) } + let(:query) { graphql_query_for(:design_management, nil, dm_fields) } + + before do + enable_design_management + project.add_developer(developer) + post_graphql(query, current_user: current_user) + end + + shared_examples 'a query that needs authorization' do + context 'the current user is not able to read designs' do + let(:current_user) { create(:user) } + + it 'does not retrieve the record' do + expect(query_result).to be_nil + end + + it 'raises an error' do + expect(graphql_errors).to include( + a_hash_including('message' => a_string_matching(%r{you don't have permission})) + ) + end + end + end + + describe '.version' do + let(:path) { %w[designManagement version] } + + let(:dm_fields) do + query_graphql_field(:version, { 'id' => global_id_of(version) }, 'id sha') + end + + it_behaves_like 'a working graphql query' + it_behaves_like 'a query that needs authorization' + + context 'the current user is able to read designs' do + it 'fetches the expected data' do + expect(query_result).to eq('id' => global_id_of(version), 'sha' => version.sha) + end + end + end + + describe '.designAtVersion' do + let_it_be(:design_at_version) do + ::DesignManagement::DesignAtVersion.new(design: design, version: version) + end + + let(:path) { %w[designManagement designAtVersion] } + + let(:dm_fields) do + query_graphql_field(:design_at_version, { 'id' => global_id_of(design_at_version) }, <<~FIELDS) + id + filename + version { id sha } + design { id } + issue { title iid } + project { id fullPath } + FIELDS + end + + it_behaves_like 'a working graphql query' + it_behaves_like 'a query that needs authorization' + + context 'the current user is able to read designs' do + it 'fetches the expected data, including the correct associations' do + expect(query_result).to eq( + 'id' => global_id_of(design_at_version), + 'filename' => design_at_version.design.filename, + 'version' => { 'id' => global_id_of(version), 'sha' => version.sha }, + 'design' => { 'id' => global_id_of(design) }, + 'issue' => { 'title' => issue.title, 'iid' => issue.iid.to_s }, + 'project' => { 'id' => global_id_of(project), 'fullPath' => project.full_path } + ) + end + end + end + end +end diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index 783dd730dd9..f5c7a820abe 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -9,7 +9,7 @@ describe 'GraphQL' do context 'logging' do shared_examples 'logging a graphql query' do let(:expected_params) do - { query_string: query, variables: variables.to_s, duration: anything, depth: 1, complexity: 1 } + { query_string: query, variables: variables.to_s, duration_s: anything, depth: 1, complexity: 1 } end it 'logs a query with the expected params' do diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 30c1f99569b..18feff85482 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -6,15 +6,15 @@ describe API::Groups do include GroupAPIHelpers include UploadHelpers - let(:user1) { create(:user, can_create_group: false) } - let(:user2) { create(:user) } - let(:user3) { create(:user) } - let(:admin) { create(:admin) } - let!(:group1) { create(:group, avatar: File.open(uploaded_image_temp_path)) } - let!(:group2) { create(:group, :private) } - let!(:project1) { create(:project, namespace: group1) } - let!(:project2) { create(:project, namespace: group2) } - let!(:project3) { create(:project, namespace: group1, path: 'test', visibility_level: Gitlab::VisibilityLevel::PRIVATE) } + let_it_be(:user1) { create(:user, can_create_group: false) } + let_it_be(:user2) { create(:user) } + let_it_be(:user3) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:group1) { create(:group, avatar: File.open(uploaded_image_temp_path)) } + let_it_be(:group2) { create(:group, :private) } + let_it_be(:project1) { create(:project, namespace: group1) } + let_it_be(:project2) { create(:project, namespace: group2) } + let_it_be(:project3) { create(:project, namespace: group1, path: 'test', visibility_level: Gitlab::VisibilityLevel::PRIVATE) } before do group1.add_owner(user1) @@ -90,6 +90,17 @@ describe API::Groups do get api("/groups", admin) end.not_to exceed_query_limit(control) end + + context 'when statistics are requested' do + it 'does not include statistics' do + get api("/groups"), params: { statistics: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.first).not_to include 'statistics' + end + end end context "when authenticated as user" do @@ -330,7 +341,7 @@ describe API::Groups do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(response_groups).to eq([group2.id, group3.id]) + expect(response_groups).to contain_exactly(group2.id, group3.id) end end end @@ -642,6 +653,33 @@ describe API::Groups do expect(json_response['default_branch_protection']).to eq(::Gitlab::Access::MAINTAINER_PROJECT_ACCESS) end + context 'updating the `default_branch_protection` attribute' do + subject do + put api("/groups/#{group1.id}", user1), params: { default_branch_protection: ::Gitlab::Access::PROTECTION_NONE } + end + + context 'for users who have the ability to update default_branch_protection' do + it 'updates the attribute' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_NONE) + end + end + + context 'for users who does not have the ability to update default_branch_protection`' do + it 'does not update the attribute' do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user1, :update_default_branch_protection, group1) { false } + + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['default_branch_protection']).not_to eq(Gitlab::Access::PROTECTION_NONE) + end + end + end + context 'malicious group name' do subject { put api("/groups/#{group1.id}", user1), params: { name: "<SCRIPT>alert('DOUBLE-ATTACK!')</SCRIPT>" } } @@ -889,6 +927,181 @@ describe API::Groups do end end + describe "GET /groups/:id/projects/shared" do + let!(:project4) do + create(:project, namespace: group2, path: 'test_project', visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + let(:path) { "/groups/#{group1.id}/projects/shared" } + + before do + create(:project_group_link, project: project2, group: group1) + create(:project_group_link, project: project4, group: group1) + end + + context 'when authenticated as user' do + it 'returns the shared projects in the group' do + get api(path, user1) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.length).to eq(2) + project_ids = json_response.map { |project| project['id'] } + expect(project_ids).to match_array([project2.id, project4.id]) + expect(json_response.first['visibility']).to be_present + end + + it 'returns shared projects with min access level or higher' do + user = create(:user) + + project2.add_guest(user) + project4.add_reporter(user) + + get api(path, user), params: { min_access_level: Gitlab::Access::REPORTER } + + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(1) + expect(json_response.first['id']).to eq(project4.id) + end + + it 'returns the shared projects of the group with simple representation' do + get api(path, user1), params: { simple: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.length).to eq(2) + project_ids = json_response.map { |project| project['id'] } + expect(project_ids).to match_array([project2.id, project4.id]) + expect(json_response.first['visibility']).not_to be_present + end + + it 'filters the shared projects in the group based on visibility' do + internal_project = create(:project, :internal, namespace: create(:group)) + + create(:project_group_link, project: internal_project, group: group1) + + get api(path, user1), params: { visibility: 'internal' } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(1) + expect(json_response.first['id']).to eq(internal_project.id) + end + + it 'filters the shared projects in the group based on search params' do + get api(path, user1), params: { search: 'test_project' } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(1) + expect(json_response.first['id']).to eq(project4.id) + end + + it 'does not return the projects owned by the group' do + get api(path, user1) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an(Array) + project_ids = json_response.map { |project| project['id'] } + + expect(project_ids).not_to include(project1.id) + end + + it 'returns 404 for a non-existing group' do + get api("/groups/0000/projects/shared", user1) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'does not return a group not attached to the user' do + group = create(:group, :private) + + get api("/groups/#{group.id}/projects/shared", user1) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'only returns shared projects to which user has access' do + project4.add_developer(user3) + + get api(path, user3) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.length).to eq(1) + expect(json_response.first['id']).to eq(project4.id) + end + + it 'only returns the projects starred by user' do + user1.starred_projects = [project2] + + get api(path, user1), params: { starred: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.length).to eq(1) + expect(json_response.first['id']).to eq(project2.id) + end + end + + context "when authenticated as admin" do + subject { get api(path, admin) } + + it "returns shared projects of an existing group" do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.length).to eq(2) + project_ids = json_response.map { |project| project['id'] } + expect(project_ids).to match_array([project2.id, project4.id]) + end + + context 'for a non-existent group' do + let(:path) { "/groups/000/projects/shared" } + + it 'returns 404 for a non-existent group' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new do + subject + end.count + + create(:project_group_link, project: create(:project), group: group1) + + expect do + subject + end.not_to exceed_query_limit(control_count) + end + end + + context 'when using group path in URL' do + let(:path) { "/groups/#{group1.path}/projects/shared" } + + it 'returns the right details' do + get api(path, admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.length).to eq(2) + project_ids = json_response.map { |project| project['id'] } + expect(project_ids).to match_array([project2.id, project4.id]) + end + + it 'returns 404 for a non-existent group' do + get api('/groups/unknown/projects/shared', admin) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + describe 'GET /groups/:id/subgroups' do let!(:subgroup1) { create(:group, parent: group1) } let!(:subgroup2) { create(:group, :private, parent: group1) } @@ -911,6 +1124,17 @@ describe API::Groups do expect(response).to have_gitlab_http_status(:not_found) end + + context 'when statistics are requested' do + it 'does not include statistics' do + get api("/groups/#{group1.id}/subgroups"), params: { statistics: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.first).not_to include 'statistics' + end + end end context 'when authenticated as user' do @@ -1111,6 +1335,33 @@ describe API::Groups do it { expect { subject }.not_to change { Group.count } } end + context 'when creating a group with `default_branch_protection` attribute' do + let(:params) { attributes_for_group_api default_branch_protection: Gitlab::Access::PROTECTION_NONE } + + subject { post api("/groups", user3), params: params } + + context 'for users who have the ability to create a group with `default_branch_protection`' do + it 'creates group with the specified branch protection level' do + subject + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_NONE) + end + end + + context 'for users who do not have the ability to create a group with `default_branch_protection`' do + it 'does not create the group with the specified branch protection level' do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user3, :create_group_with_default_branch_protection) { false } + + subject + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['default_branch_protection']).not_to eq(Gitlab::Access::PROTECTION_NONE) + end + end + end + it "does not create group, duplicate" do post api("/groups", user3), params: { name: 'Duplicate Test', path: group2.path } diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 98904a4d79f..d65c89f48ea 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -328,6 +328,8 @@ describe API::Helpers do it 'returns a 401 response' do expect { authenticate! }.to raise_error /401/ + + expect(env[described_class::API_RESPONSE_STATUS_CODE]).to eq(401) end end @@ -340,6 +342,8 @@ describe API::Helpers do it 'does not raise an error' do expect { authenticate! }.not_to raise_error + + expect(env[described_class::API_RESPONSE_STATUS_CODE]).to be_nil end end end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 93c2233e021..684f0329909 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -323,18 +323,6 @@ describe API::Internal::Base do end end - shared_examples 'snippets with disabled feature flag' do - context 'when feature flag :version_snippets is disabled' do - it 'returns 401' do - stub_feature_flags(version_snippets: false) - - subject - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - end - shared_examples 'snippet success' do it 'responds with success' do subject @@ -344,18 +332,6 @@ describe API::Internal::Base do end end - shared_examples 'snippets with web protocol' do - it_behaves_like 'snippet success' - - context 'with disabled version flag' do - before do - stub_feature_flags(version_snippets: false) - end - - it_behaves_like 'snippet success' - end - end - context 'git push with personal snippet' do subject { push(key, personal_snippet, env: env.to_json, changes: snippet_changes) } @@ -369,12 +345,6 @@ describe API::Internal::Base do expect(user.reload.last_activity_on).to be_nil end - it_behaves_like 'snippets with disabled feature flag' - - it_behaves_like 'snippets with web protocol' do - subject { push(key, personal_snippet, 'web', env: env.to_json, changes: snippet_changes) } - end - it_behaves_like 'sets hook env' do let(:gl_repository) { Gitlab::GlRepository::SNIPPET.identifier_for_container(personal_snippet) } end @@ -392,12 +362,6 @@ describe API::Internal::Base do expect(json_response["gl_repository"]).to eq("snippet-#{personal_snippet.id}") expect(user.reload.last_activity_on).to eql(Date.today) end - - it_behaves_like 'snippets with disabled feature flag' - - it_behaves_like 'snippets with web protocol' do - subject { pull(key, personal_snippet, 'web') } - end end context 'git push with project snippet' do @@ -413,12 +377,6 @@ describe API::Internal::Base do expect(user.reload.last_activity_on).to be_nil end - it_behaves_like 'snippets with disabled feature flag' - - it_behaves_like 'snippets with web protocol' do - subject { push(key, project_snippet, 'web', env: env.to_json, changes: snippet_changes) } - end - it_behaves_like 'sets hook env' do let(:gl_repository) { Gitlab::GlRepository::SNIPPET.identifier_for_container(project_snippet) } end @@ -434,14 +392,6 @@ describe API::Internal::Base do expect(json_response["gl_repository"]).to eq("snippet-#{project_snippet.id}") expect(user.reload.last_activity_on).to eql(Date.today) end - - it_behaves_like 'snippets with disabled feature flag' do - subject { pull(key, project_snippet) } - end - - it_behaves_like 'snippets with web protocol' do - subject { pull(key, project_snippet, 'web') } - end end context "git pull" do @@ -491,23 +441,25 @@ describe API::Internal::Base do allow(Gitlab::CurrentSettings).to receive(:receive_max_input_size) { 1 } end - it 'returns custom git config' do + it 'returns maxInputSize and partial clone git config' do push(key, project) expect(json_response["git_config_options"]).to be_present + expect(json_response["git_config_options"]).to include("receive.maxInputSize=1048576") expect(json_response["git_config_options"]).to include("uploadpack.allowFilter=true") expect(json_response["git_config_options"]).to include("uploadpack.allowAnySHA1InWant=true") end context 'when gitaly_upload_pack_filter feature flag is disabled' do before do - stub_feature_flags(gitaly_upload_pack_filter: { enabled: false, thing: project }) + stub_feature_flags(gitaly_upload_pack_filter: false) end - it 'does not include allowFilter and allowAnySha1InWant in the git config options' do + it 'returns only maxInputSize and not partial clone git config' do push(key, project) expect(json_response["git_config_options"]).to be_present + expect(json_response["git_config_options"]).to include("receive.maxInputSize=1048576") expect(json_response["git_config_options"]).not_to include("uploadpack.allowFilter=true") expect(json_response["git_config_options"]).not_to include("uploadpack.allowAnySHA1InWant=true") end @@ -515,12 +467,28 @@ describe API::Internal::Base do end context 'when receive_max_input_size is empty' do - it 'returns an empty git config' do + before do allow(Gitlab::CurrentSettings).to receive(:receive_max_input_size) { nil } + end + it 'returns partial clone git config' do push(key, project) - expect(json_response["git_config_options"]).to be_empty + expect(json_response["git_config_options"]).to be_present + expect(json_response["git_config_options"]).to include("uploadpack.allowFilter=true") + expect(json_response["git_config_options"]).to include("uploadpack.allowAnySHA1InWant=true") + end + + context 'when gitaly_upload_pack_filter feature flag is disabled' do + before do + stub_feature_flags(gitaly_upload_pack_filter: false) + end + + it 'returns an empty git config' do + push(key, project) + + expect(json_response["git_config_options"]).to be_empty + end end end end @@ -949,6 +917,23 @@ describe API::Internal::Base do expect(json_response['status']).to be_falsy end end + + context 'for design repositories' do + let(:gl_repository) { Gitlab::GlRepository::DESIGN.identifier_for_container(project) } + + it 'does not allow access' do + post(api('/internal/allowed'), + params: { + key_id: key.id, + project: project.full_path, + gl_repository: gl_repository, + secret_token: secret_token, + protocol: 'ssh' + }) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end end describe 'POST /internal/post_receive', :clean_gitlab_redis_shared_state do diff --git a/spec/requests/api/issues/get_group_issues_spec.rb b/spec/requests/api/issues/get_group_issues_spec.rb index 3ec5f380390..5c925d2a32e 100644 --- a/spec/requests/api/issues/get_group_issues_spec.rb +++ b/spec/requests/api/issues/get_group_issues_spec.rb @@ -3,26 +3,26 @@ require 'spec_helper' describe API::Issues do - let_it_be(:user) { create(:user) } - let(:user2) { create(:user) } - let(:non_member) { create(:user) } - let_it_be(:guest) { create(:user) } - let_it_be(:author) { create(:author) } - let_it_be(:assignee) { create(:assignee) } - let(:admin) { create(:user, :admin) } - let(:issue_title) { 'foo' } - let(:issue_description) { 'closed' } - let(:no_milestone_title) { 'None' } - let(:any_milestone_title) { 'Any' } + let_it_be(:user2) { create(:user) } + let_it_be(:admin) { create(:user, :admin) } + let_it_be(:non_member) { create(:user) } + let_it_be(:user) { create(:user) } + let_it_be(:guest) { create(:user) } + let_it_be(:author) { create(:author) } + let_it_be(:assignee) { create(:assignee) } + let_it_be(:issue_title) { 'foo' } + let_it_be(:issue_description) { 'closed' } + let_it_be(:no_milestone_title) { 'None' } + let_it_be(:any_milestone_title) { 'Any' } before do stub_licensed_features(multiple_issue_assignees: false, issue_weights: false) end describe 'GET /groups/:id/issues' do - let!(:group) { create(:group) } - let!(:group_project) { create(:project, :public, :repository, creator_id: user.id, namespace: group) } - let!(:private_mrs_project) do + let_it_be(:group) { create(:group) } + let_it_be(:group_project) { create(:project, :public, :repository, creator_id: user.id, namespace: group) } + let_it_be(:private_mrs_project) do create(:project, :public, :repository, creator_id: user.id, namespace: group, merge_requests_access_level: ProjectFeature::PRIVATE) end @@ -455,6 +455,29 @@ describe API::Issues do it_behaves_like 'labeled issues with labels and label_name params' end + context 'with archived projects' do + let_it_be(:archived_issue) do + create( + :issue, author: user, assignees: [user], + project: create(:project, :public, :archived, creator_id: user.id, namespace: group) + ) + end + + it 'returns only non archived projects issues' do + get api(base_url, user) + + expect_paginated_array_response([group_closed_issue.id, group_confidential_issue.id, group_issue.id]) + end + + it 'returns issues from archived projects if non_archived it set to false' do + get api(base_url, user), params: { non_archived: false } + + expect_paginated_array_response( + [archived_issue.id, group_closed_issue.id, group_confidential_issue.id, group_issue.id] + ) + end + end + it 'returns an array of issues found by iids' do get api(base_url, user), params: { iids: [group_issue.iid] } diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index 00169c1529f..06878f57d43 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -780,28 +780,20 @@ describe API::Issues do end context 'filtering by non_archived' do - let_it_be(:group1) { create(:group) } - let_it_be(:archived_project) { create(:project, :archived, namespace: group1) } - let_it_be(:active_project) { create(:project, namespace: group1) } - let_it_be(:issue1) { create(:issue, project: active_project) } - let_it_be(:issue2) { create(:issue, project: active_project) } - let_it_be(:issue3) { create(:issue, project: archived_project) } + let_it_be(:archived_project) { create(:project, :archived, creator_id: user.id, namespace: user.namespace) } + let_it_be(:archived_issue) { create(:issue, author: user, project: archived_project) } + let_it_be(:active_issue) { create(:issue, author: user, project: project) } - before do - archived_project.add_developer(user) - active_project.add_developer(user) - end - - it 'returns issues from non archived projects only by default' do - get api("/groups/#{group1.id}/issues", user), params: { scope: 'all' } + it 'returns issues from non archived projects by default' do + get api('/issues', user) - expect_paginated_array_response([issue2.id, issue1.id]) + expect_paginated_array_response(active_issue.id, issue.id, closed_issue.id) end - it 'returns issues from archived and non archived projects when non_archived is false' do - get api("/groups/#{group1.id}/issues", user), params: { non_archived: false, scope: 'all' } + it 'returns issues from archived project with non_archived set as false' do + get api("/issues", user), params: { non_archived: false } - expect_paginated_array_response([issue3.id, issue2.id, issue1.id]) + expect_paginated_array_response(active_issue.id, archived_issue.id, issue.id, closed_issue.id) end end end diff --git a/spec/requests/api/issues/post_projects_issues_spec.rb b/spec/requests/api/issues/post_projects_issues_spec.rb index 1444f43003f..2e1e5d3204e 100644 --- a/spec/requests/api/issues/post_projects_issues_spec.rb +++ b/spec/requests/api/issues/post_projects_issues_spec.rb @@ -403,7 +403,7 @@ describe API::Issues do end before do - expect_next_instance_of(Spam::SpamCheckService) do |spam_service| + expect_next_instance_of(Spam::SpamActionService) do |spam_service| expect(spam_service).to receive_messages(check_for_spam?: true) end expect_next_instance_of(Spam::AkismetService) do |akismet_service| diff --git a/spec/requests/api/issues/put_projects_issues_spec.rb b/spec/requests/api/issues/put_projects_issues_spec.rb index ffc5e2b1db8..2ab8b9d7877 100644 --- a/spec/requests/api/issues/put_projects_issues_spec.rb +++ b/spec/requests/api/issues/put_projects_issues_spec.rb @@ -182,6 +182,8 @@ describe API::Issues do end describe 'PUT /projects/:id/issues/:issue_iid with spam filtering' do + include_context 'includes Spam constants' + def update_issue put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: params end @@ -195,11 +197,12 @@ describe API::Issues do end before do - expect_next_instance_of(Spam::SpamCheckService) do |spam_service| + expect_next_instance_of(Spam::SpamActionService) do |spam_service| expect(spam_service).to receive_messages(check_for_spam?: true) end - expect_next_instance_of(Spam::AkismetService) do |akismet_service| - expect(akismet_service).to receive_messages(spam?: true) + + expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service| + expect(verdict_service).to receive(:execute).and_return(DISALLOW) end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index af2ce7f7aef..14b22de9661 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -66,17 +66,36 @@ describe API::MergeRequests do end context 'when merge request is unchecked' do + let(:check_service_class) { MergeRequests::MergeabilityCheckService } + let(:mr_entity) { json_response.find { |mr| mr['id'] == merge_request.id } } + before do merge_request.mark_as_unchecked! end - it 'checks mergeability asynchronously' do - expect_next_instance_of(MergeRequests::MergeabilityCheckService) do |service| - expect(service).not_to receive(:execute) - expect(service).to receive(:async_execute) + context 'with merge status recheck projection' do + it 'checks mergeability asynchronously' do + expect_next_instance_of(check_service_class) do |service| + expect(service).not_to receive(:execute) + expect(service).to receive(:async_execute).and_call_original + end + + get(api(endpoint_path, user), params: { with_merge_status_recheck: true }) + + expect_successful_response_with_paginated_array + expect(mr_entity['merge_status']).to eq('checking') end + end - get api(endpoint_path, user) + context 'without merge status recheck projection' do + it 'does not enqueue a merge status recheck' do + expect(check_service_class).not_to receive(:new) + + get api(endpoint_path, user) + + expect_successful_response_with_paginated_array + expect(mr_entity['merge_status']).to eq('unchecked') + end end end @@ -776,8 +795,8 @@ describe API::MergeRequests do end describe "GET /groups/:id/merge_requests" do - let!(:group) { create(:group, :public) } - let!(:project) { create(:project, :public, :repository, creator: user, namespace: group, only_allow_merge_if_pipeline_succeeds: false) } + let_it_be(:group) { create(:group, :public) } + let_it_be(:project) { create(:project, :public, :repository, creator: user, namespace: group, only_allow_merge_if_pipeline_succeeds: false) } let(:endpoint_path) { "/groups/#{group.id}/merge_requests" } before do @@ -787,9 +806,9 @@ describe API::MergeRequests do it_behaves_like 'merge requests list' context 'when have subgroups' do - let!(:group) { create(:group, :public) } - let!(:subgroup) { create(:group, parent: group) } - let!(:project) { create(:project, :public, :repository, creator: user, namespace: subgroup, only_allow_merge_if_pipeline_succeeds: false) } + let_it_be(:group) { create(:group, :public) } + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:project) { create(:project, :public, :repository, creator: user, namespace: subgroup, only_allow_merge_if_pipeline_succeeds: false) } it_behaves_like 'merge requests list' end @@ -1535,7 +1554,7 @@ describe API::MergeRequests do end context 'forked projects', :sidekiq_might_not_need_inline do - let!(:user2) { create(:user) } + let_it_be(:user2) { create(:user) } let(:project) { create(:project, :public, :repository) } let!(:forked_project) { fork_project(project, user2, repository: true) } let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } @@ -2308,6 +2327,33 @@ describe API::MergeRequests do end end + context 'with labels' do + include_context 'with labels' + + let(:api_base) { api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) } + + it 'when adding labels, keeps existing labels and adds new' do + put api_base, params: { add_labels: '1, 2' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['labels']).to contain_exactly(label.title, label2.title, '1', '2') + end + + it 'when removing labels, only removes those specified' do + put api_base, params: { remove_labels: "#{label.title}" } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['labels']).to eq([label2.title]) + end + + it 'when removing all labels, keeps no labels' do + put api_base, params: { remove_labels: "#{label.title}, #{label2.title}" } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['labels']).to be_empty + end + end + it 'does not update state when title is empty' do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: { state_event: 'close', title: nil } diff --git a/spec/requests/api/metrics/dashboard/annotations_spec.rb b/spec/requests/api/metrics/dashboard/annotations_spec.rb index 0b51c46e474..6377ef2435a 100644 --- a/spec/requests/api/metrics/dashboard/annotations_spec.rb +++ b/spec/requests/api/metrics/dashboard/annotations_spec.rb @@ -11,77 +11,125 @@ describe API::Metrics::Dashboard::Annotations do let(:ending_at) { 1.hour.from_now.iso8601 } let(:params) { attributes_for(:metrics_dashboard_annotation, environment: environment, starting_at: starting_at, ending_at: ending_at, dashboard_path: dashboard)} - describe 'POST /environments/:environment_id/metrics_dashboard/annotations' do - before :all do + shared_examples 'POST /:source_type/:id/metrics_dashboard/annotations' do |source_type| + let(:url) { "/#{source_type.pluralize}/#{source.id}/metrics_dashboard/annotations" } + + before do project.add_developer(user) end - context 'feature flag metrics_dashboard_annotations' do - context 'is on' do - before do - stub_feature_flags(metrics_dashboard_annotations: { enabled: true, thing: project }) - end - context 'with correct permissions' do - context 'with valid parameters' do - it 'creates a new annotation', :aggregate_failures do - post api("/environments/#{environment.id}/metrics_dashboard/annotations", user), params: params - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['environment_id']).to eq(environment.id) - expect(json_response['starting_at'].to_time).to eq(starting_at.to_time) - expect(json_response['ending_at'].to_time).to eq(ending_at.to_time) - expect(json_response['description']).to eq(params[:description]) - expect(json_response['dashboard_path']).to eq(dashboard) - end + context "with :source_type == #{source_type.pluralize}" do + context 'with correct permissions' do + context 'with valid parameters' do + it 'creates a new annotation', :aggregate_failures do + post api(url, user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response["#{source_type}_id"]).to eq(source.id) + expect(json_response['starting_at'].to_time).to eq(starting_at.to_time) + expect(json_response['ending_at'].to_time).to eq(ending_at.to_time) + expect(json_response['description']).to eq(params[:description]) + expect(json_response['dashboard_path']).to eq(dashboard) end + end - context 'with invalid parameters' do - it 'returns error messsage' do - post api("/environments/#{environment.id}/metrics_dashboard/annotations", user), - params: { dashboard_path: nil, starting_at: nil, description: nil } + context 'with invalid parameters' do + it 'returns error messsage' do + post api(url, user), params: { dashboard_path: '', starting_at: nil, description: nil } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to include({ "starting_at" => ["can't be blank"], "description" => ["can't be blank"], "dashboard_path" => ["can't be blank"] }) - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to include({ "starting_at" => ["can't be blank"], "description" => ["can't be blank"], "dashboard_path" => ["can't be blank"] }) end + end - context 'with undeclared params' do - before do - params[:undeclared_param] = 'xyz' - end - it 'filters out undeclared params' do - expect(::Metrics::Dashboard::Annotations::CreateService).to receive(:new).with(user, hash_excluding(:undeclared_param)) + context 'with undeclared params' do + before do + params[:undeclared_param] = 'xyz' + end - post api("/environments/#{environment.id}/metrics_dashboard/annotations", user), params: params - end + it 'filters out undeclared params' do + expect(::Metrics::Dashboard::Annotations::CreateService).to receive(:new).with(user, hash_excluding(:undeclared_param)) + + post api(url, user), params: params end end - context 'without correct permissions' do - let_it_be(:guest) { create(:user) } + context 'with special characers in dashboard_path in request body' do + let(:dashboard_escaped) { 'config/prometheus/common_metrics%26copy.yml' } + let(:dashboard_unescaped) { 'config/prometheus/common_metrics©.yml' } - before do - project.add_guest(guest) + shared_examples 'special characters unescaped' do + let(:expected_params) do + { + 'starting_at' => starting_at.to_time, + 'ending_at' => ending_at.to_time, + "#{source_type}" => source, + 'dashboard_path' => dashboard_unescaped, + 'description' => params[:description] + } + end + + it 'unescapes the dashboard_path', :aggregate_failures do + expect(::Metrics::Dashboard::Annotations::CreateService).to receive(:new).with(user, expected_params) + + post api(url, user), params: params + end end - it 'returns error messsage' do - post api("/environments/#{environment.id}/metrics_dashboard/annotations", guest), params: params + context 'with escaped characters' do + it_behaves_like 'special characters unescaped' do + let(:dashboard) { dashboard_escaped } + end + end - expect(response).to have_gitlab_http_status(:forbidden) + context 'with unescaped characers' do + it_behaves_like 'special characters unescaped' do + let(:dashboard) { dashboard_unescaped } + end end end end - context 'is off' do + + context 'without correct permissions' do + let_it_be(:guest) { create(:user) } + before do - stub_feature_flags(metrics_dashboard_annotations: { enabled: false, thing: project }) + project.add_guest(guest) end - it 'returns error messsage' do - post api("/environments/#{environment.id}/metrics_dashboard/annotations", user), params: params + it 'returns error message' do + post api(url, guest), params: params - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:forbidden) end end end end + + describe 'environment' do + it_behaves_like 'POST /:source_type/:id/metrics_dashboard/annotations', 'environment' do + let(:source) { environment } + end + end + + describe 'group cluster' do + it_behaves_like 'POST /:source_type/:id/metrics_dashboard/annotations', 'cluster' do + let_it_be(:group) { create(:group) } + let_it_be(:cluster) { create(:cluster_for_group, groups: [group]) } + + before do + group.add_developer(user) + end + + let(:source) { cluster } + end + end + + describe 'project cluster' do + it_behaves_like 'POST /:source_type/:id/metrics_dashboard/annotations', 'cluster' do + let_it_be(:cluster) { create(:cluster, projects: [project]) } + + let(:source) { cluster } + end + end end diff --git a/spec/requests/api/metrics/user_starred_dashboards_spec.rb b/spec/requests/api/metrics/user_starred_dashboards_spec.rb new file mode 100644 index 00000000000..8f9394a0e20 --- /dev/null +++ b/spec/requests/api/metrics/user_starred_dashboards_spec.rb @@ -0,0 +1,164 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::Metrics::UserStarredDashboards do + let_it_be(:user) { create(:user) } + let_it_be(:dashboard_yml) { fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml') } + let_it_be(:dashboard) { '.gitlab/dashboards/find&seek.yml' } + let_it_be(:project) { create(:project, :private, :repository, :custom_repo, namespace: user.namespace, files: { dashboard => dashboard_yml }) } + let(:url) { "/projects/#{project.id}/metrics/user_starred_dashboards" } + let(:params) do + { + dashboard_path: CGI.escape(dashboard) + } + end + + describe 'POST /projects/:id/metrics/user_starred_dashboards' do + before do + project.add_reporter(user) + end + + context 'with correct permissions' do + context 'with valid parameters' do + context 'dashboard_path as url param url escaped' do + it 'creates a new user starred metrics dashboard', :aggregate_failures do + post api(url, user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['project_id']).to eq(project.id) + expect(json_response['user_id']).to eq(user.id) + expect(json_response['dashboard_path']).to eq(dashboard) + end + end + + context 'dashboard_path in request body unescaped' do + let(:params) do + { + dashboard_path: dashboard + } + end + + it 'creates a new user starred metrics dashboard', :aggregate_failures do + post api(url, user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['project_id']).to eq(project.id) + expect(json_response['user_id']).to eq(user.id) + expect(json_response['dashboard_path']).to eq(dashboard) + end + end + end + + context 'with invalid parameters' do + it 'returns error message' do + post api(url, user), params: { dashboard_path: '' } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('dashboard_path is empty') + end + + context 'user is missing' do + it 'returns 404 not found' do + post api(url, nil), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'project is missing' do + it 'returns 404 not found' do + post api("/projects/#{project.id + 1}/user_starred_dashboards", user), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + + context 'without correct permissions' do + it 'returns 404 not found' do + post api(url, create(:user)), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'DELETE /projects/:id/metrics/user_starred_dashboards' do + let_it_be(:user_starred_dashboard_1) { create(:metrics_users_starred_dashboard, user: user, project: project, dashboard_path: dashboard) } + let_it_be(:user_starred_dashboard_2) { create(:metrics_users_starred_dashboard, user: user, project: project) } + let_it_be(:other_user_starred_dashboard) { create(:metrics_users_starred_dashboard, project: project) } + let_it_be(:other_project_starred_dashboard) { create(:metrics_users_starred_dashboard, user: user) } + + before do + project.add_reporter(user) + end + + context 'with correct permissions' do + context 'with valid parameters' do + context 'dashboard_path as url param url escaped' do + it 'deletes given user starred metrics dashboard', :aggregate_failures do + delete api(url, user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['deleted_rows']).to eq(1) + expect(::Metrics::UsersStarredDashboard.all.pluck(:dashboard_path)).not_to include(dashboard) + end + end + + context 'dashboard_path in request body unescaped' do + let(:params) do + { + dashboard_path: dashboard + } + end + + it 'deletes given user starred metrics dashboard', :aggregate_failures do + delete api(url, user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['deleted_rows']).to eq(1) + expect(::Metrics::UsersStarredDashboard.all.pluck(:dashboard_path)).not_to include(dashboard) + end + end + + context 'dashboard_path has not been specified' do + it 'deletes all starred dashboards for that user within given project', :aggregate_failures do + delete api(url, user), params: {} + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['deleted_rows']).to eq(2) + expect(::Metrics::UsersStarredDashboard.all).to contain_exactly(other_user_starred_dashboard, other_project_starred_dashboard) + end + end + end + + context 'with invalid parameters' do + context 'user is missing' do + it 'returns 404 not found' do + post api(url, nil), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'project is missing' do + it 'returns 404 not found' do + post api("/projects/#{project.id + 1}/user_starred_dashboards", user), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + + context 'without correct permissions' do + it 'returns 404 not found' do + post api(url, create(:user)), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 14b292db045..98eaf36b14e 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -67,7 +67,7 @@ describe API::PipelineSchedules do end def active?(str) - (str == 'active') ? true : false + str == 'active' end end end diff --git a/spec/requests/api/pipelines_spec.rb b/spec/requests/api/pipelines_spec.rb index f43fa5b4185..f57223f1de5 100644 --- a/spec/requests/api/pipelines_spec.rb +++ b/spec/requests/api/pipelines_spec.rb @@ -72,8 +72,8 @@ describe API::Pipelines do end context 'when scope is branches or tags' do - let!(:pipeline_branch) { create(:ci_pipeline, project: project) } - let!(:pipeline_tag) { create(:ci_pipeline, project: project, ref: 'v1.0.0', tag: true) } + let_it_be(:pipeline_branch) { create(:ci_pipeline, project: project) } + let_it_be(:pipeline_tag) { create(:ci_pipeline, project: project, ref: 'v1.0.0', tag: true) } context 'when scope is branches' do it 'returns matched pipelines' do @@ -161,7 +161,7 @@ describe API::Pipelines do end context 'when name is specified' do - let!(:pipeline) { create(:ci_pipeline, project: project, user: user) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } context 'when name exists' do it 'returns matched pipelines' do @@ -185,7 +185,7 @@ describe API::Pipelines do end context 'when username is specified' do - let!(:pipeline) { create(:ci_pipeline, project: project, user: user) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } context 'when username exists' do it 'returns matched pipelines' do @@ -209,8 +209,8 @@ describe API::Pipelines do end context 'when yaml_errors is specified' do - let!(:pipeline1) { create(:ci_pipeline, project: project, yaml_errors: 'Syntax error') } - let!(:pipeline2) { create(:ci_pipeline, project: project) } + let_it_be(:pipeline1) { create(:ci_pipeline, project: project, yaml_errors: 'Syntax error') } + let_it_be(:pipeline2) { create(:ci_pipeline, project: project) } context 'when yaml_errors is true' do it 'returns matched pipelines' do @@ -242,9 +242,9 @@ describe API::Pipelines do end context 'when updated_at filters are specified' do - let!(:pipeline1) { create(:ci_pipeline, project: project, updated_at: 2.days.ago) } - let!(:pipeline2) { create(:ci_pipeline, project: project, updated_at: 4.days.ago) } - let!(:pipeline3) { create(:ci_pipeline, project: project, updated_at: 1.hour.ago) } + let_it_be(:pipeline1) { create(:ci_pipeline, project: project, updated_at: 2.days.ago) } + let_it_be(:pipeline2) { create(:ci_pipeline, project: project, updated_at: 4.days.ago) } + let_it_be(:pipeline3) { create(:ci_pipeline, project: project, updated_at: 1.hour.ago) } it 'returns pipelines with last update date in specified datetime range' do get api("/projects/#{project.id}/pipelines", user), params: { updated_before: 1.day.ago, updated_after: 3.days.ago } @@ -614,7 +614,7 @@ describe API::Pipelines do end context 'when the pipeline has jobs' do - let!(:build) { create(:ci_build, project: project, pipeline: pipeline) } + let_it_be(:build) { create(:ci_build, project: project, pipeline: pipeline) } it 'destroys associated jobs' do delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) @@ -654,12 +654,12 @@ describe API::Pipelines do describe 'POST /projects/:id/pipelines/:pipeline_id/retry' do context 'authorized user' do - let!(:pipeline) do + let_it_be(:pipeline) do create(:ci_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) end - let!(:build) { create(:ci_build, :failed, pipeline: pipeline) } + let_it_be(:build) { create(:ci_build, :failed, pipeline: pipeline) } it 'retries failed builds' do expect do @@ -683,12 +683,12 @@ describe API::Pipelines do end describe 'POST /projects/:id/pipelines/:pipeline_id/cancel' do - let!(:pipeline) do + let_it_be(:pipeline) do create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) end - let!(:build) { create(:ci_build, :running, pipeline: pipeline) } + let_it_be(:build) { create(:ci_build, :running, pipeline: pipeline) } context 'authorized user' do it 'retries failed builds', :sidekiq_might_not_need_inline do @@ -700,7 +700,7 @@ describe API::Pipelines do end context 'user without proper access rights' do - let!(:reporter) { create(:user) } + let_it_be(:reporter) { create(:user) } before do project.add_reporter(reporter) @@ -714,4 +714,73 @@ describe API::Pipelines do end end end + + describe 'GET /projects/:id/pipelines/:pipeline_id/test_report' do + context 'authorized user' do + subject { get api("/projects/#{project.id}/pipelines/#{pipeline.id}/test_report", user) } + + let(:pipeline) { create(:ci_pipeline, project: project) } + + context 'when feature is enabled' do + before do + stub_feature_flags(junit_pipeline_view: true) + end + + context 'when pipeline does not have a test report' do + it 'returns an empty test report' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['total_count']).to eq(0) + end + end + + context 'when pipeline has a test report' do + let(:pipeline) { create(:ci_pipeline, :with_test_reports, project: project) } + + it 'returns the test report' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['total_count']).to eq(4) + end + end + + context 'when pipeline has corrupt test reports' do + before do + job = create(:ci_build, pipeline: pipeline) + create(:ci_job_artifact, :junit_with_corrupted_data, job: job, project: project) + end + + it 'returns a suite_error' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['test_suites'].first['suite_error']).to eq('JUnit XML parsing failed: 1:1: FATAL: Document is empty') + end + end + end + + context 'when feature is disabled' do + before do + stub_feature_flags(junit_pipeline_view: false) + end + + it 'renders empty response' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'unauthorized user' do + it 'does not return project pipelines' do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/test_report", non_member) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq '404 Project Not Found' + end + end + end end diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb index 859a3cca44f..ad872b88664 100644 --- a/spec/requests/api/project_export_spec.rb +++ b/spec/requests/api/project_export_spec.rb @@ -411,7 +411,9 @@ describe API::ProjectExport, :clean_gitlab_redis_cache do it 'starts', :sidekiq_might_not_need_inline do params = { description: "Foo" } - expect_any_instance_of(Projects::ImportExport::ExportService).to receive(:execute) + expect_next_instance_of(Projects::ImportExport::ExportService) do |service| + expect(service).to receive(:execute) + end post api(path, project.owner), params: params expect(response).to have_gitlab_http_status(:accepted) diff --git a/spec/requests/api/project_milestones_spec.rb b/spec/requests/api/project_milestones_spec.rb index a40878fc807..c5911d51706 100644 --- a/spec/requests/api/project_milestones_spec.rb +++ b/spec/requests/api/project_milestones_spec.rb @@ -24,13 +24,13 @@ describe API::ProjectMilestones do project.add_reporter(reporter) end - it 'returns 404 response when the project does not exists' do + it 'returns 404 response when the project does not exist' do delete api("/projects/0/milestones/#{milestone.id}", user) expect(response).to have_gitlab_http_status(:not_found) end - it 'returns 404 response when the milestone does not exists' do + it 'returns 404 response when the milestone does not exist' do delete api("/projects/#{project.id}/milestones/0", user) expect(response).to have_gitlab_http_status(:not_found) @@ -44,7 +44,7 @@ describe API::ProjectMilestones do end describe 'PUT /projects/:id/milestones/:milestone_id to test observer on close' do - it 'creates an activity event when an milestone is closed' do + it 'creates an activity event when a milestone is closed' do expect(Event).to receive(:create!) put api("/projects/#{project.id}/milestones/#{milestone.id}", user), @@ -91,7 +91,7 @@ describe API::ProjectMilestones do end end - context 'when no such resources' do + context 'when no such resource' do before do group.add_developer(user) end diff --git a/spec/requests/api/project_repository_storage_moves_spec.rb b/spec/requests/api/project_repository_storage_moves_spec.rb new file mode 100644 index 00000000000..7ceea0178f3 --- /dev/null +++ b/spec/requests/api/project_repository_storage_moves_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::ProjectRepositoryStorageMoves do + include AccessMatchersForRequest + + let(:user) { create(:admin) } + let!(:storage_move) { create(:project_repository_storage_move, :scheduled) } + + describe 'GET /project_repository_storage_moves' do + def get_project_repository_storage_moves + get api('/project_repository_storage_moves', user) + end + + it 'returns project repository storage moves' do + get_project_repository_storage_moves + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(response).to match_response_schema('public_api/v4/project_repository_storage_moves') + expect(json_response.size).to eq(1) + expect(json_response.first['id']).to eq(storage_move.id) + expect(json_response.first['state']).to eq(storage_move.human_state_name) + end + + it 'avoids N+1 queries', :request_store do + # prevent `let` from polluting the control + get_project_repository_storage_moves + + control = ActiveRecord::QueryRecorder.new { get_project_repository_storage_moves } + + create(:project_repository_storage_move, :scheduled) + + expect { get_project_repository_storage_moves }.not_to exceed_query_limit(control) + end + + it 'returns the most recently created first' do + storage_move_oldest = create(:project_repository_storage_move, :scheduled, created_at: 2.days.ago) + storage_move_middle = create(:project_repository_storage_move, :scheduled, created_at: 1.day.ago) + + get api('/project_repository_storage_moves', user) + + json_ids = json_response.map {|storage_move| storage_move['id'] } + expect(json_ids).to eq([ + storage_move.id, + storage_move_middle.id, + storage_move_oldest.id + ]) + end + + describe 'permissions' do + it { expect { get_project_repository_storage_moves }.to be_allowed_for(:admin) } + it { expect { get_project_repository_storage_moves }.to be_denied_for(:user) } + end + end + + describe 'GET /project_repository_storage_moves/:id' do + let(:project_repository_storage_move_id) { storage_move.id } + + def get_project_repository_storage_move + get api("/project_repository_storage_moves/#{project_repository_storage_move_id}", user) + end + + it 'returns a project repository storage move' do + get_project_repository_storage_move + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/project_repository_storage_move') + expect(json_response['id']).to eq(storage_move.id) + expect(json_response['state']).to eq(storage_move.human_state_name) + end + + context 'non-existent project repository storage move' do + let(:project_repository_storage_move_id) { non_existing_record_id } + + it 'returns not found' do + get_project_repository_storage_move + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe 'permissions' do + it { expect { get_project_repository_storage_move }.to be_allowed_for(:admin) } + it { expect { get_project_repository_storage_move }.to be_denied_for(:user) } + end + end +end diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 89ade15c1f6..22189dc3299 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -94,21 +94,11 @@ describe API::ProjectSnippets do expect(json_response['title']).to eq(snippet.title) expect(json_response['description']).to eq(snippet.description) - expect(json_response['file_name']).to eq(snippet.file_name) + expect(json_response['file_name']).to eq(snippet.file_name_on_repo) expect(json_response['ssh_url_to_repo']).to eq(snippet.ssh_url_to_repo) expect(json_response['http_url_to_repo']).to eq(snippet.http_url_to_repo) end - context 'when feature flag :version_snippets is disabled' do - before do - stub_feature_flags(version_snippets: false) - - get api("/projects/#{project.id}/snippets/#{snippet.id}", user) - end - - it_behaves_like 'snippet response without repository URLs' - end - it 'returns 404 for invalid snippet id' do get api("/projects/#{project.id}/snippets/#{non_existing_record_id}", user) @@ -129,7 +119,7 @@ describe API::ProjectSnippets do title: 'Test Title', file_name: 'test.rb', description: 'test description', - code: 'puts "hello world"', + content: 'puts "hello world"', visibility: 'public' } end @@ -148,19 +138,7 @@ describe API::ProjectSnippets do blob = snippet.repository.blob_at('master', params[:file_name]) - expect(blob.data).to eq params[:code] - end - - context 'when feature flag :version_snippets is disabled' do - it 'does not create snippet repository' do - stub_feature_flags(version_snippets: false) - - expect do - subject - end.to change { ProjectSnippet.count }.by(1) - - expect(snippet.repository_exists?).to be_falsey - end + expect(blob.data).to eq params[:content] end end @@ -202,7 +180,7 @@ describe API::ProjectSnippets do expect(response).to have_gitlab_http_status(:created) snippet = ProjectSnippet.find(json_response['id']) - expect(snippet.content).to eq(params[:code]) + expect(snippet.content).to eq(params[:content]) expect(snippet.description).to eq(params[:description]) expect(snippet.title).to eq(params[:title]) expect(snippet.file_name).to eq(params[:file_name]) @@ -219,7 +197,7 @@ describe API::ProjectSnippets do expect(response).to have_gitlab_http_status(:created) snippet = ProjectSnippet.find(json_response['id']) - expect(snippet.content).to eq(params[:code]) + expect(snippet.content).to eq(params[:content]) expect(snippet.description).to eq(params[:description]) expect(snippet.title).to eq(params[:title]) expect(snippet.file_name).to eq(params[:file_name]) @@ -230,43 +208,44 @@ describe API::ProjectSnippets do subject { post api("/projects/#{project.id}/snippets/", admin), params: params } end - it 'creates a new snippet with content parameter' do - params[:content] = params.delete(:code) + it 'returns 400 for missing parameters' do + params.delete(:title) post api("/projects/#{project.id}/snippets/", admin), params: params - expect(response).to have_gitlab_http_status(:created) - snippet = ProjectSnippet.find(json_response['id']) - expect(snippet.content).to eq(params[:content]) - expect(snippet.description).to eq(params[:description]) - expect(snippet.title).to eq(params[:title]) - expect(snippet.file_name).to eq(params[:file_name]) - expect(snippet.visibility_level).to eq(Snippet::PUBLIC) + expect(response).to have_gitlab_http_status(:bad_request) end - it 'returns 400 when both code and content parameters specified' do - params[:content] = params[:code] + it 'returns 400 if content is blank' do + params[:content] = '' post api("/projects/#{project.id}/snippets/", admin), params: params expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('code, content are mutually exclusive') + expect(json_response['error']).to eq 'content is empty' end - it 'returns 400 for missing parameters' do - params.delete(:title) + it 'returns 400 if title is blank' do + params[:title] = '' post api("/projects/#{project.id}/snippets/", admin), params: params expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'title is empty' end - it 'returns 400 for empty code field' do - params[:code] = '' + context 'when save fails because the repository could not be created' do + before do + allow_next_instance_of(Snippets::CreateService) do |instance| + allow(instance).to receive(:create_repository).and_raise(Snippets::CreateService::CreateRepositoryError) + end + end - post api("/projects/#{project.id}/snippets/", admin), params: params + it 'returns 400' do + post api("/projects/#{project.id}/snippets", admin), params: params - expect(response).to have_gitlab_http_status(:bad_request) + expect(response).to have_gitlab_http_status(:bad_request) + end end context 'when the snippet is spam' do @@ -320,7 +299,7 @@ describe API::ProjectSnippets do new_content = 'New content' new_description = 'New description' - update_snippet(params: { code: new_content, description: new_description, visibility: 'private' }) + update_snippet(params: { content: new_content, description: new_description, visibility: 'private' }) expect(response).to have_gitlab_http_status(:ok) snippet.reload @@ -341,13 +320,6 @@ describe API::ProjectSnippets do expect(snippet.description).to eq(new_description) end - it 'returns 400 when both code and content parameters specified' do - update_snippet(params: { code: 'some content', content: 'other content' }) - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('code, content are mutually exclusive') - end - it 'returns 404 for invalid snippet id' do update_snippet(snippet_id: non_existing_record_id, params: { title: 'foo' }) @@ -361,12 +333,17 @@ describe API::ProjectSnippets do expect(response).to have_gitlab_http_status(:bad_request) end - it 'returns 400 for empty code field' do - new_content = '' + it 'returns 400 if content is blank' do + update_snippet(params: { content: '' }) + + expect(response).to have_gitlab_http_status(:bad_request) + end - update_snippet(params: { code: new_content }) + it 'returns 400 if title is blank' do + update_snippet(params: { title: '' }) expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'title is empty' end it_behaves_like 'update with repository actions' do @@ -460,14 +437,13 @@ describe API::ProjectSnippets do end describe 'GET /projects/:project_id/snippets/:id/raw' do - let(:snippet) { create(:project_snippet, author: admin, project: project) } + let_it_be(:snippet) { create(:project_snippet, :repository, author: admin, project: project) } it 'returns raw text' do get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/raw", admin) expect(response).to have_gitlab_http_status(:ok) expect(response.content_type).to eq 'text/plain' - expect(response.body).to eq(snippet.content) end it 'returns 404 for invalid snippet id' do @@ -482,5 +458,11 @@ describe API::ProjectSnippets do let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/123/raw", admin) } end end + + it_behaves_like 'snippet blob content' do + let_it_be(:snippet_with_empty_repo) { create(:project_snippet, :empty_repo, author: admin, project: project) } + + subject { get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/raw", snippet.author) } + end end end diff --git a/spec/requests/api/project_statistics_spec.rb b/spec/requests/api/project_statistics_spec.rb index 1f48c081043..89809a97b96 100644 --- a/spec/requests/api/project_statistics_spec.rb +++ b/spec/requests/api/project_statistics_spec.rb @@ -3,23 +3,23 @@ require 'spec_helper' describe API::ProjectStatistics do - let(:maintainer) { create(:user) } - let(:public_project) { create(:project, :public) } + let_it_be(:developer) { create(:user) } + let_it_be(:public_project) { create(:project, :public) } before do - public_project.add_maintainer(maintainer) + public_project.add_developer(developer) end describe 'GET /projects/:id/statistics' do - let!(:fetch_statistics1) { create(:project_daily_statistic, project: public_project, fetch_count: 30, date: 29.days.ago) } - let!(:fetch_statistics2) { create(:project_daily_statistic, project: public_project, fetch_count: 4, date: 3.days.ago) } - let!(:fetch_statistics3) { create(:project_daily_statistic, project: public_project, fetch_count: 3, date: 2.days.ago) } - let!(:fetch_statistics4) { create(:project_daily_statistic, project: public_project, fetch_count: 2, date: 1.day.ago) } - let!(:fetch_statistics5) { create(:project_daily_statistic, project: public_project, fetch_count: 1, date: Date.today) } - let!(:fetch_statistics_other_project) { create(:project_daily_statistic, project: create(:project), fetch_count: 29, date: 29.days.ago) } + let_it_be(:fetch_statistics1) { create(:project_daily_statistic, project: public_project, fetch_count: 30, date: 29.days.ago) } + let_it_be(:fetch_statistics2) { create(:project_daily_statistic, project: public_project, fetch_count: 4, date: 3.days.ago) } + let_it_be(:fetch_statistics3) { create(:project_daily_statistic, project: public_project, fetch_count: 3, date: 2.days.ago) } + let_it_be(:fetch_statistics4) { create(:project_daily_statistic, project: public_project, fetch_count: 2, date: 1.day.ago) } + let_it_be(:fetch_statistics5) { create(:project_daily_statistic, project: public_project, fetch_count: 1, date: Date.today) } + let_it_be(:fetch_statistics_other_project) { create(:project_daily_statistic, project: create(:project), fetch_count: 29, date: 29.days.ago) } it 'returns the fetch statistics of the last 30 days' do - get api("/projects/#{public_project.id}/statistics", maintainer) + get api("/projects/#{public_project.id}/statistics", developer) expect(response).to have_gitlab_http_status(:ok) fetches = json_response['fetches'] @@ -32,7 +32,7 @@ describe API::ProjectStatistics do it 'excludes the fetch statistics older than 30 days' do create(:project_daily_statistic, fetch_count: 31, project: public_project, date: 30.days.ago) - get api("/projects/#{public_project.id}/statistics", maintainer) + get api("/projects/#{public_project.id}/statistics", developer) expect(response).to have_gitlab_http_status(:ok) fetches = json_response['fetches'] @@ -41,11 +41,11 @@ describe API::ProjectStatistics do expect(fetches['days'].last).to eq({ 'count' => fetch_statistics1.fetch_count, 'date' => fetch_statistics1.date.to_s }) end - it 'responds with 403 when the user is not a maintainer of the repository' do - developer = create(:user) - public_project.add_developer(developer) + it 'responds with 403 when the user is not a developer of the repository' do + guest = create(:user) + public_project.add_guest(guest) - get api("/projects/#{public_project.id}/statistics", developer) + get api("/projects/#{public_project.id}/statistics", guest) expect(response).to have_gitlab_http_status(:forbidden) expect(json_response['message']).to eq('403 Forbidden') diff --git a/spec/requests/api/project_templates_spec.rb b/spec/requests/api/project_templates_spec.rb index 5dabce20043..caeb465080e 100644 --- a/spec/requests/api/project_templates_spec.rb +++ b/spec/requests/api/project_templates_spec.rb @@ -3,15 +3,29 @@ require 'spec_helper' describe API::ProjectTemplates do - let_it_be(:public_project) { create(:project, :public) } + let_it_be(:public_project) { create(:project, :public, path: 'path.with.dot') } let_it_be(:private_project) { create(:project, :private) } let_it_be(:developer) { create(:user) } + let(:url_encoded_path) { "#{public_project.namespace.path}%2F#{public_project.path}" } + before do private_project.add_developer(developer) end + shared_examples 'accepts project paths with dots' do + it do + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + describe 'GET /projects/:id/templates/:type' do + it_behaves_like 'accepts project paths with dots' do + subject { get api("/projects/#{url_encoded_path}/templates/dockerfiles") } + end + it 'returns dockerfiles' do get api("/projects/#{public_project.id}/templates/dockerfiles") @@ -75,6 +89,10 @@ describe API::ProjectTemplates do expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('public_api/v4/template_list') end + + it_behaves_like 'accepts project paths with dots' do + subject { get api("/projects/#{url_encoded_path}/templates/licenses") } + end end describe 'GET /projects/:id/templates/:type/:key' do @@ -144,6 +162,10 @@ describe API::ProjectTemplates do expect(response).to match_response_schema('public_api/v4/license') end + it_behaves_like 'accepts project paths with dots' do + subject { get api("/projects/#{url_encoded_path}/templates/gitlab_ci_ymls/Android") } + end + shared_examples 'path traversal attempt' do |template_type| it 'rejects invalid filenames' do get api("/projects/#{public_project.id}/templates/#{template_type}/%2e%2e%2fPython%2ea") @@ -173,5 +195,9 @@ describe API::ProjectTemplates do expect(content).to include('Project Placeholder') expect(content).to include("Copyright (C) #{Time.now.year} Fullname Placeholder") end + + it_behaves_like 'accepts project paths with dots' do + subject { get api("/projects/#{url_encoded_path}/templates/licenses/mit") } + end end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 853155cea7a..0deff138e2e 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -24,7 +24,7 @@ shared_examples 'languages and percentages JSON response' do get api("/projects/#{project.id}/languages", user) expect(response).to have_gitlab_http_status(:ok) - expect(JSON.parse(response.body)).to eq(expected_languages) + expect(Gitlab::Json.parse(response.body)).to eq(expected_languages) end end @@ -672,7 +672,7 @@ describe API::Projects do match[1] end - ids += JSON.parse(response.body).map { |p| p['id'] } + ids += Gitlab::Json.parse(response.body).map { |p| p['id'] } end expect(ids).to contain_exactly(*projects.map(&:id)) @@ -1806,7 +1806,7 @@ describe API::Projects do first_user = json_response.first expect(first_user['username']).to eq(user.username) expect(first_user['name']).to eq(user.name) - expect(first_user.keys).to contain_exactly(*%w[name username id state avatar_url web_url]) + expect(first_user.keys).to include(*%w[name username id state avatar_url web_url]) end end diff --git a/spec/requests/api/remote_mirrors_spec.rb b/spec/requests/api/remote_mirrors_spec.rb index 3eaec6e2520..3029b8443b0 100644 --- a/spec/requests/api/remote_mirrors_spec.rb +++ b/spec/requests/api/remote_mirrors_spec.rb @@ -78,10 +78,6 @@ describe API::RemoteMirrors do let(:route) { ->(id) { "/projects/#{project.id}/remote_mirrors/#{id}" } } let(:mirror) { project.remote_mirrors.first } - before do - stub_feature_flags(keep_divergent_refs: false) - end - it 'requires `admin_remote_mirror` permission' do put api(route[mirror.id], developer) @@ -100,24 +96,7 @@ describe API::RemoteMirrors do expect(response).to have_gitlab_http_status(:success) expect(json_response['enabled']).to eq(false) expect(json_response['only_protected_branches']).to eq(true) - - # Deleted due to lack of feature availability - expect(json_response['keep_divergent_refs']).to be_nil - end - - context 'with the `keep_divergent_refs` feature enabled' do - before do - stub_feature_flags(keep_divergent_refs: { enabled: true, project: project }) - end - - it 'updates the `keep_divergent_refs` attribute' do - project.add_maintainer(user) - - put api(route[mirror.id], user), params: { keep_divergent_refs: 'true' } - - expect(response).to have_gitlab_http_status(:success) - expect(json_response['keep_divergent_refs']).to eq(true) - end + expect(json_response['keep_divergent_refs']).to eq(true) end end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index bc2da8a2b9a..7284f33f3af 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -471,7 +471,8 @@ describe API::Runner, :clean_gitlab_redis_shared_state do 'sha' => job.sha, 'before_sha' => job.before_sha, 'ref_type' => 'branch', - 'refspecs' => ["+refs/heads/#{job.ref}:refs/remotes/origin/#{job.ref}"], + 'refspecs' => ["+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}", + "+refs/heads/#{job.ref}:refs/remotes/origin/#{job.ref}"], 'depth' => project.ci_default_git_depth } end @@ -578,7 +579,9 @@ describe API::Runner, :clean_gitlab_redis_shared_state do expect(response).to have_gitlab_http_status(:created) expect(json_response['git_info']['refspecs']) - .to contain_exactly('+refs/tags/*:refs/tags/*', '+refs/heads/*:refs/remotes/origin/*') + .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}", + '+refs/tags/*:refs/tags/*', + '+refs/heads/*:refs/remotes/origin/*') end end end @@ -638,7 +641,9 @@ describe API::Runner, :clean_gitlab_redis_shared_state do expect(response).to have_gitlab_http_status(:created) expect(json_response['git_info']['refspecs']) - .to contain_exactly('+refs/tags/*:refs/tags/*', '+refs/heads/*:refs/remotes/origin/*') + .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}", + '+refs/tags/*:refs/tags/*', + '+refs/heads/*:refs/remotes/origin/*') end end end @@ -998,6 +1003,53 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end + describe 'a job with excluded artifacts' do + context 'when excluded paths are defined' do + let(:job) do + create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'test', + stage: 'deploy', stage_idx: 1, + options: { artifacts: { paths: ['abc'], exclude: ['cde'] } }) + end + + context 'when a runner supports this feature' do + it 'exposes excluded paths when the feature is enabled' do + stub_feature_flags(ci_artifacts_exclude: true) + + request_job info: { features: { artifacts_exclude: true } } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response.dig('artifacts').first).to include('exclude' => ['cde']) + end + + it 'does not expose excluded paths when the feature is disabled' do + stub_feature_flags(ci_artifacts_exclude: false) + + request_job info: { features: { artifacts_exclude: true } } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response.dig('artifacts').first).not_to have_key('exclude') + end + end + + context 'when a runner does not support this feature' do + it 'does not expose the build at all' do + stub_feature_flags(ci_artifacts_exclude: true) + + request_job + + expect(response).to have_gitlab_http_status(:no_content) + end + end + end + + it 'does not expose excluded paths when these are empty' do + request_job + + expect(response).to have_gitlab_http_status(:created) + expect(json_response.dig('artifacts').first).not_to have_key('exclude') + end + end + def request_job(token = runner.token, **params) new_params = params.merge(token: token, last_update: last_update) post api('/jobs/request'), params: new_params, headers: { 'User-Agent' => user_agent } diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 164be8f0da6..261e54da6a8 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -3,35 +3,34 @@ require 'spec_helper' describe API::Runners do - let(:admin) { create(:user, :admin) } - let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:group_guest) { create(:user) } - let(:group_reporter) { create(:user) } - let(:group_developer) { create(:user) } - let(:group_maintainer) { create(:user) } - - let(:project) { create(:project, creator_id: user.id) } - let(:project2) { create(:project, creator_id: user.id) } - - let(:group) { create(:group).tap { |group| group.add_owner(user) } } - let(:subgroup) { create(:group, parent: group) } - - let!(:shared_runner) { create(:ci_runner, :instance, description: 'Shared runner') } - let!(:project_runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } - let!(:two_projects_runner) { create(:ci_runner, :project, description: 'Two projects runner', projects: [project, project2]) } - let!(:group_runner_a) { create(:ci_runner, :group, description: 'Group runner A', groups: [group]) } - let!(:group_runner_b) { create(:ci_runner, :group, description: 'Group runner B', groups: [subgroup]) } - - before do - # Set project access for users - create(:group_member, :guest, user: group_guest, group: group) - create(:group_member, :reporter, user: group_reporter, group: group) - create(:group_member, :developer, user: group_developer, group: group) - create(:group_member, :maintainer, user: group_maintainer, group: group) - create(:project_member, :maintainer, user: user, project: project) - create(:project_member, :maintainer, user: user, project: project2) - create(:project_member, :reporter, user: user2, project: project) + let_it_be(:admin) { create(:user, :admin) } + let_it_be(:user) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:group_guest) { create(:user) } + let_it_be(:group_reporter) { create(:user) } + let_it_be(:group_developer) { create(:user) } + let_it_be(:group_maintainer) { create(:user) } + + let_it_be(:project) { create(:project, creator_id: user.id) } + let_it_be(:project2) { create(:project, creator_id: user.id) } + + let_it_be(:group) { create(:group).tap { |group| group.add_owner(user) } } + let_it_be(:subgroup) { create(:group, parent: group) } + + let_it_be(:shared_runner, reload: true) { create(:ci_runner, :instance, description: 'Shared runner') } + let_it_be(:project_runner, reload: true) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } + let_it_be(:two_projects_runner) { create(:ci_runner, :project, description: 'Two projects runner', projects: [project, project2]) } + let_it_be(:group_runner_a) { create(:ci_runner, :group, description: 'Group runner A', groups: [group]) } + let_it_be(:group_runner_b) { create(:ci_runner, :group, description: 'Group runner B', groups: [subgroup]) } + + before_all do + group.add_guest(group_guest) + group.add_reporter(group_reporter) + group.add_developer(group_developer) + group.add_maintainer(group_maintainer) + project.add_maintainer(user) + project2.add_maintainer(user) + project.add_reporter(user2) end describe 'GET /runners' do @@ -327,6 +326,32 @@ describe API::Runners do expect(response).to have_gitlab_http_status(:unauthorized) end end + + context 'FF hide_token_from_runners_api is enabled' do + before do + stub_feature_flags(hide_token_from_runners_api: true) + end + + it "does not return runner's token" do + get api("/runners/#{shared_runner.id}", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).not_to have_key('token') + end + end + + context 'FF hide_token_from_runners_api is disabled' do + before do + stub_feature_flags(hide_token_from_runners_api: false) + end + + it "returns runner's token" do + get api("/runners/#{shared_runner.id}", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to have_key('token') + end + end end describe 'PUT /runners/:id' do @@ -603,10 +628,10 @@ describe API::Runners do describe 'GET /runners/:id/jobs' do let_it_be(:job_1) { create(:ci_build) } - let!(:job_2) { create(:ci_build, :running, runner: shared_runner, project: project) } - let!(:job_3) { create(:ci_build, :failed, runner: shared_runner, project: project) } - let!(:job_4) { create(:ci_build, :running, runner: project_runner, project: project) } - let!(:job_5) { create(:ci_build, :failed, runner: project_runner, project: project) } + let_it_be(:job_2) { create(:ci_build, :running, runner: shared_runner, project: project) } + let_it_be(:job_3) { create(:ci_build, :failed, runner: shared_runner, project: project) } + let_it_be(:job_4) { create(:ci_build, :running, runner: project_runner, project: project) } + let_it_be(:job_5) { create(:ci_build, :failed, runner: project_runner, project: project) } context 'admin user' do context 'when runner exists' do @@ -952,7 +977,7 @@ describe API::Runners do describe 'POST /projects/:id/runners' do context 'authorized user' do - let(:project_runner2) { create(:ci_runner, :project, projects: [project2]) } + let_it_be(:project_runner2) { create(:ci_runner, :project, projects: [project2]) } it 'enables specific runner' do expect do diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 6ff5fbd7925..3894e0bf2d1 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -15,10 +15,36 @@ describe API::Search do it { expect(json_response.size).to eq(size) } end - describe 'GET /search' do + shared_examples 'pagination' do |scope:, search: ''| + it 'returns a different result for each page' do + get api(endpoint, user), params: { scope: scope, search: search, page: 1, per_page: 1 } + first = json_response.first + + get api(endpoint, user), params: { scope: scope, search: search, page: 2, per_page: 1 } + second = Gitlab::Json.parse(response.body).first + + expect(first).not_to eq(second) + end + + it 'returns 1 result when per_page is 1' do + get api(endpoint, user), params: { scope: scope, search: search, per_page: 1 } + + expect(json_response.count).to eq(1) + end + + it 'returns 2 results when per_page is 2' do + get api(endpoint, user), params: { scope: scope, search: search, per_page: 2 } + + expect(Gitlab::Json.parse(response.body).count).to eq(2) + end + end + + describe 'GET /search' do + let(:endpoint) { '/search' } + context 'when user is not authenticated' do it 'returns 401 error' do - get api('/search'), params: { scope: 'projects', search: 'awesome' } + get api(endpoint), params: { scope: 'projects', search: 'awesome' } expect(response).to have_gitlab_http_status(:unauthorized) end @@ -26,7 +52,7 @@ describe API::Search do context 'when scope is not supported' do it 'returns 400 error' do - get api('/search', user), params: { scope: 'unsupported', search: 'awesome' } + get api(endpoint, user), params: { scope: 'unsupported', search: 'awesome' } expect(response).to have_gitlab_http_status(:bad_request) end @@ -34,7 +60,7 @@ describe API::Search do context 'when scope is missing' do it 'returns 400 error' do - get api('/search', user), params: { search: 'awesome' } + get api(endpoint, user), params: { search: 'awesome' } expect(response).to have_gitlab_http_status(:bad_request) end @@ -43,30 +69,48 @@ describe API::Search do context 'with correct params' do context 'for projects scope' do before do - get api('/search', user), params: { scope: 'projects', search: 'awesome' } + get api(endpoint, user), params: { scope: 'projects', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/projects' + + it_behaves_like 'pagination', scope: :projects end context 'for issues scope' do before do create(:issue, project: project, title: 'awesome issue') - get api('/search', user), params: { scope: 'issues', search: 'awesome' } + get api(endpoint, user), params: { scope: 'issues', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/issues' + + describe 'pagination' do + before do + create(:issue, project: project, title: 'another issue') + end + + include_examples 'pagination', scope: :issues + end end context 'for merge_requests scope' do before do create(:merge_request, source_project: repo_project, title: 'awesome mr') - get api('/search', user), params: { scope: 'merge_requests', search: 'awesome' } + get api(endpoint, user), params: { scope: 'merge_requests', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests' + + describe 'pagination' do + before do + create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch') + end + + include_examples 'pagination', scope: :merge_requests + end end context 'for milestones scope' do @@ -76,10 +120,18 @@ describe API::Search do context 'when user can read project milestones' do before do - get api('/search', user), params: { scope: 'milestones', search: 'awesome' } + get api(endpoint, user), params: { scope: 'milestones', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + + describe 'pagination' do + before do + create(:milestone, project: project, title: 'another milestone') + end + + include_examples 'pagination', scope: :milestones + end end context 'when user cannot read project milestones' do @@ -89,7 +141,7 @@ describe API::Search do end it 'returns empty array' do - get api('/search', user), params: { scope: 'milestones', search: 'awesome' } + get api(endpoint, user), params: { scope: 'milestones', search: 'awesome' } milestones = json_response @@ -102,16 +154,18 @@ describe API::Search do before do create(:user, name: 'billy') - get api('/search', user), params: { scope: 'users', search: 'billy' } + get api(endpoint, user), params: { scope: 'users', search: 'billy' } end it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics' + it_behaves_like 'pagination', scope: :users + context 'when users search feature is disabled' do before do - allow(Feature).to receive(:disabled?).with(:users_search, default_enabled: true).and_return(true) + stub_feature_flags(users_search: false) - get api('/search', user), params: { scope: 'users', search: 'billy' } + get api(endpoint, user), params: { scope: 'users', search: 'billy' } end it 'returns 400 error' do @@ -124,28 +178,28 @@ describe API::Search do before do create(:snippet, :public, title: 'awesome snippet', content: 'snippet content') - get api('/search', user), params: { scope: 'snippet_titles', search: 'awesome' } + get api(endpoint, user), params: { scope: 'snippet_titles', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/snippets' - end - context 'for snippet_blobs scope' do - before do - create(:snippet, :public, title: 'awesome snippet', content: 'snippet content') + describe 'pagination' do + before do + create(:snippet, :public, title: 'another snippet', content: 'snippet content') + end - get api('/search', user), params: { scope: 'snippet_blobs', search: 'content' } + include_examples 'pagination', scope: :snippet_titles end - - it_behaves_like 'response is correct', schema: 'public_api/v4/snippets' end end end describe "GET /groups/:id/search" do + let(:endpoint) { "/groups/#{group.id}/-/search" } + context 'when user is not authenticated' do it 'returns 401 error' do - get api("/groups/#{group.id}/search"), params: { scope: 'projects', search: 'awesome' } + get api(endpoint), params: { scope: 'projects', search: 'awesome' } expect(response).to have_gitlab_http_status(:unauthorized) end @@ -153,7 +207,7 @@ describe API::Search do context 'when scope is not supported' do it 'returns 400 error' do - get api("/groups/#{group.id}/search", user), params: { scope: 'unsupported', search: 'awesome' } + get api(endpoint, user), params: { scope: 'unsupported', search: 'awesome' } expect(response).to have_gitlab_http_status(:bad_request) end @@ -161,7 +215,7 @@ describe API::Search do context 'when scope is missing' do it 'returns 400 error' do - get api("/groups/#{group.id}/search", user), params: { search: 'awesome' } + get api(endpoint, user), params: { search: 'awesome' } expect(response).to have_gitlab_http_status(:bad_request) end @@ -188,40 +242,66 @@ describe API::Search do context 'with correct params' do context 'for projects scope' do before do - get api("/groups/#{group.id}/search", user), params: { scope: 'projects', search: 'awesome' } + get api(endpoint, user), params: { scope: 'projects', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/projects' + + it_behaves_like 'pagination', scope: :projects end context 'for issues scope' do before do create(:issue, project: project, title: 'awesome issue') - get api("/groups/#{group.id}/search", user), params: { scope: 'issues', search: 'awesome' } + get api(endpoint, user), params: { scope: 'issues', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/issues' + + describe 'pagination' do + before do + create(:issue, project: project, title: 'another issue') + end + + include_examples 'pagination', scope: :issues + end end context 'for merge_requests scope' do before do create(:merge_request, source_project: repo_project, title: 'awesome mr') - get api("/groups/#{group.id}/search", user), params: { scope: 'merge_requests', search: 'awesome' } + get api(endpoint, user), params: { scope: 'merge_requests', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests' + + describe 'pagination' do + before do + create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch') + end + + include_examples 'pagination', scope: :merge_requests + end end context 'for milestones scope' do before do create(:milestone, project: project, title: 'awesome milestone') - get api("/groups/#{group.id}/search", user), params: { scope: 'milestones', search: 'awesome' } + get api(endpoint, user), params: { scope: 'milestones', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + + describe 'pagination' do + before do + create(:milestone, project: project, title: 'another milestone') + end + + include_examples 'pagination', scope: :milestones + end end context 'for milestones scope with group path as id' do @@ -241,16 +321,24 @@ describe API::Search do user = create(:user, name: 'billy') create(:group_member, :developer, user: user, group: group) - get api("/groups/#{group.id}/search", user), params: { scope: 'users', search: 'billy' } + get api(endpoint, user), params: { scope: 'users', search: 'billy' } end it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics' + describe 'pagination' do + before do + create(:group_member, :developer, group: group) + end + + include_examples 'pagination', scope: :users + end + context 'when users search feature is disabled' do before do - allow(Feature).to receive(:disabled?).with(:users_search, default_enabled: true).and_return(true) + stub_feature_flags(users_search: false) - get api("/groups/#{group.id}/search", user), params: { scope: 'users', search: 'billy' } + get api(endpoint, user), params: { scope: 'users', search: 'billy' } end it 'returns 400 error' do @@ -273,9 +361,11 @@ describe API::Search do end describe "GET /projects/:id/search" do + let(:endpoint) { "/projects/#{project.id}/search" } + context 'when user is not authenticated' do it 'returns 401 error' do - get api("/projects/#{project.id}/search"), params: { scope: 'issues', search: 'awesome' } + get api(endpoint), params: { scope: 'issues', search: 'awesome' } expect(response).to have_gitlab_http_status(:unauthorized) end @@ -283,7 +373,7 @@ describe API::Search do context 'when scope is not supported' do it 'returns 400 error' do - get api("/projects/#{project.id}/search", user), params: { scope: 'unsupported', search: 'awesome' } + get api(endpoint, user), params: { scope: 'unsupported', search: 'awesome' } expect(response).to have_gitlab_http_status(:bad_request) end @@ -291,7 +381,7 @@ describe API::Search do context 'when scope is missing' do it 'returns 400 error' do - get api("/projects/#{project.id}/search", user), params: { search: 'awesome' } + get api(endpoint, user), params: { search: 'awesome' } expect(response).to have_gitlab_http_status(:bad_request) end @@ -309,7 +399,7 @@ describe API::Search do it 'returns 404 error' do project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) - get api("/projects/#{project.id}/search", user), params: { scope: 'issues', search: 'awesome' } + get api(endpoint, user), params: { scope: 'issues', search: 'awesome' } expect(response).to have_gitlab_http_status(:not_found) end @@ -320,20 +410,38 @@ describe API::Search do before do create(:issue, project: project, title: 'awesome issue') - get api("/projects/#{project.id}/search", user), params: { scope: 'issues', search: 'awesome' } + get api(endpoint, user), params: { scope: 'issues', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/issues' + + describe 'pagination' do + before do + create(:issue, project: project, title: 'another issue') + end + + include_examples 'pagination', scope: :issues + end end context 'for merge_requests scope' do + let(:endpoint) { "/projects/#{repo_project.id}/search" } + before do create(:merge_request, source_project: repo_project, title: 'awesome mr') - get api("/projects/#{repo_project.id}/search", user), params: { scope: 'merge_requests', search: 'awesome' } + get api(endpoint, user), params: { scope: 'merge_requests', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests' + + describe 'pagination' do + before do + create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch') + end + + include_examples 'pagination', scope: :merge_requests + end end context 'for milestones scope' do @@ -343,10 +451,18 @@ describe API::Search do context 'when user can read milestones' do before do - get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } + get api(endpoint, user), params: { scope: 'milestones', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + + describe 'pagination' do + before do + create(:milestone, project: project, title: 'another milestone') + end + + include_examples 'pagination', scope: :milestones + end end context 'when user cannot read project milestones' do @@ -356,7 +472,7 @@ describe API::Search do end it 'returns empty array' do - get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } + get api(endpoint, user), params: { scope: 'milestones', search: 'awesome' } milestones = json_response @@ -370,16 +486,24 @@ describe API::Search do user1 = create(:user, name: 'billy') create(:project_member, :developer, user: user1, project: project) - get api("/projects/#{project.id}/search", user), params: { scope: 'users', search: 'billy' } + get api(endpoint, user), params: { scope: 'users', search: 'billy' } end it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics' + describe 'pagination' do + before do + create(:project_member, :developer, project: project) + end + + include_examples 'pagination', scope: :users + end + context 'when users search feature is disabled' do before do - allow(Feature).to receive(:disabled?).with(:users_search, default_enabled: true).and_return(true) + stub_feature_flags(users_search: false) - get api("/projects/#{project.id}/search", user), params: { scope: 'users', search: 'billy' } + get api(endpoint, user), params: { scope: 'users', search: 'billy' } end it 'returns 400 error' do @@ -392,29 +516,51 @@ describe API::Search do before do create(:note_on_merge_request, project: project, note: 'awesome note') - get api("/projects/#{project.id}/search", user), params: { scope: 'notes', search: 'awesome' } + get api(endpoint, user), params: { scope: 'notes', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/notes' + + describe 'pagination' do + before do + mr = create(:merge_request, source_project: project, target_branch: 'another_branch') + create(:note, project: project, noteable: mr, note: 'another note') + end + + include_examples 'pagination', scope: :notes + end end context 'for wiki_blobs scope' do + let(:wiki) { create(:project_wiki, project: project) } + before do - wiki = create(:project_wiki, project: project) - create(:wiki_page, wiki: wiki, attrs: { title: 'home', content: "Awesome page" }) + create(:wiki_page, wiki: wiki, title: 'home', content: "Awesome page") - get api("/projects/#{project.id}/search", user), params: { scope: 'wiki_blobs', search: 'awesome' } + get api(endpoint, user), params: { scope: 'wiki_blobs', search: 'awesome' } end it_behaves_like 'response is correct', schema: 'public_api/v4/blobs' + + describe 'pagination' do + before do + create(:wiki_page, wiki: wiki, title: 'home 2', content: 'Another page') + end + + include_examples 'pagination', scope: :wiki_blobs, search: 'page' + end end context 'for commits scope' do + let(:endpoint) { "/projects/#{repo_project.id}/search" } + before do - get api("/projects/#{repo_project.id}/search", user), params: { scope: 'commits', search: '498214de67004b1da3d820901307bed2a68a8ef6' } + get api(endpoint, user), params: { scope: 'commits', search: '498214de67004b1da3d820901307bed2a68a8ef6' } end it_behaves_like 'response is correct', schema: 'public_api/v4/commits_details' + + it_behaves_like 'pagination', scope: :commits, search: 'merge' end context 'for commits scope with project path as id' do @@ -426,15 +572,19 @@ describe API::Search do end context 'for blobs scope' do + let(:endpoint) { "/projects/#{repo_project.id}/search" } + before do - get api("/projects/#{repo_project.id}/search", user), params: { scope: 'blobs', search: 'monitors' } + get api(endpoint, user), params: { scope: 'blobs', search: 'monitors' } end it_behaves_like 'response is correct', schema: 'public_api/v4/blobs', size: 2 + it_behaves_like 'pagination', scope: :blobs, search: 'monitors' + context 'filters' do it 'by filename' do - get api("/projects/#{repo_project.id}/search", user), params: { scope: 'blobs', search: 'mon filename:PROCESS.md' } + get api(endpoint, user), params: { scope: 'blobs', search: 'mon filename:PROCESS.md' } expect(response).to have_gitlab_http_status(:ok) expect(json_response.size).to eq(2) @@ -443,21 +593,21 @@ describe API::Search do end it 'by path' do - get api("/projects/#{repo_project.id}/search", user), params: { scope: 'blobs', search: 'mon path:markdown' } + get api(endpoint, user), params: { scope: 'blobs', search: 'mon path:markdown' } expect(response).to have_gitlab_http_status(:ok) expect(json_response.size).to eq(8) end it 'by extension' do - get api("/projects/#{repo_project.id}/search", user), params: { scope: 'blobs', search: 'mon extension:md' } + get api(endpoint, user), params: { scope: 'blobs', search: 'mon extension:md' } expect(response).to have_gitlab_http_status(:ok) expect(json_response.size).to eq(11) end it 'by ref' do - get api("/projects/#{repo_project.id}/search", user), params: { scope: 'blobs', search: 'This file is used in tests for ci_environments_status', ref: 'pages-deploy' } + get api(endpoint, user), params: { scope: 'blobs', search: 'This file is used in tests for ci_environments_status', ref: 'pages-deploy' } expect(response).to have_gitlab_http_status(:ok) expect(json_response.size).to eq(1) diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 4a8b8f70dff..a5b95bc59a5 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -88,7 +88,9 @@ describe API::Settings, 'Settings' do allow_local_requests_from_system_hooks: false, push_event_hooks_limit: 2, push_event_activities_limit: 2, - snippet_size_limit: 5 + snippet_size_limit: 5, + issues_create_limit: 300, + raw_blob_request_limit: 300 } expect(response).to have_gitlab_http_status(:ok) @@ -125,6 +127,8 @@ describe API::Settings, 'Settings' do expect(json_response['push_event_hooks_limit']).to eq(2) expect(json_response['push_event_activities_limit']).to eq(2) expect(json_response['snippet_size_limit']).to eq(5) + expect(json_response['issues_create_limit']).to eq(300) + expect(json_response['raw_blob_request_limit']).to eq(300) end end @@ -155,6 +159,14 @@ describe API::Settings, 'Settings' do expect(json_response['allow_local_requests_from_hooks_and_services']).to eq(true) end + it 'disables ability to switch to legacy storage' do + put api("/application/settings", admin), + params: { hashed_storage_enabled: false } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['hashed_storage_enabled']).to eq(true) + end + context 'external policy classification settings' do let(:settings) do { diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 3e30dc537e4..c12c95ae2e0 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe API::Snippets do - let!(:user) { create(:user) } + let_it_be(:user) { create(:user) } describe 'GET /snippets/' do it 'returns snippets available' do @@ -90,7 +90,7 @@ describe API::Snippets do describe 'GET /snippets/:id/raw' do let_it_be(:author) { create(:user) } - let_it_be(:snippet) { create(:personal_snippet, :private, author: author) } + let_it_be(:snippet) { create(:personal_snippet, :repository, :private, author: author) } it 'requires authentication' do get api("/snippets/#{snippet.id}", nil) @@ -103,7 +103,6 @@ describe API::Snippets do expect(response).to have_gitlab_http_status(:ok) expect(response.content_type).to eq 'text/plain' - expect(response.body).to eq(snippet.content) end it 'forces attachment content disposition' do @@ -134,6 +133,12 @@ describe API::Snippets do expect(response).to have_gitlab_http_status(:ok) end + + it_behaves_like 'snippet blob content' do + let_it_be(:snippet_with_empty_repo) { create(:personal_snippet, :empty_repo, :private, author: author) } + + subject { get api("/snippets/#{snippet.id}/raw", snippet.author) } + end end describe 'GET /snippets/:id' do @@ -155,22 +160,12 @@ describe API::Snippets do expect(json_response['title']).to eq(private_snippet.title) expect(json_response['description']).to eq(private_snippet.description) - expect(json_response['file_name']).to eq(private_snippet.file_name) + expect(json_response['file_name']).to eq(private_snippet.file_name_on_repo) expect(json_response['visibility']).to eq(private_snippet.visibility) expect(json_response['ssh_url_to_repo']).to eq(private_snippet.ssh_url_to_repo) expect(json_response['http_url_to_repo']).to eq(private_snippet.http_url_to_repo) end - context 'when feature flag :version_snippets is disabled' do - before do - stub_feature_flags(version_snippets: false) - - get api("/snippets/#{private_snippet.id}", author) - end - - it_behaves_like 'snippet response without repository URLs' - end - it 'shows private snippets to an admin' do get api("/snippets/#{private_snippet.id}", admin) @@ -200,7 +195,7 @@ describe API::Snippets do end describe 'POST /snippets/' do - let(:params) do + let(:base_params) do { title: 'Test Title', file_name: 'test.rb', @@ -209,12 +204,14 @@ describe API::Snippets do visibility: 'public' } end + let(:params) { base_params.merge(extra_params) } + let(:extra_params) { {} } + + subject { post api("/snippets/", user), params: params } shared_examples 'snippet creation' do let(:snippet) { Snippet.find(json_response["id"]) } - subject { post api("/snippets/", user), params: params } - it 'creates a new snippet' do expect do subject @@ -240,18 +237,6 @@ describe API::Snippets do expect(blob.data).to eq params[:content] end - - context 'when feature flag :version_snippets is disabled' do - it 'does not create snippet repository' do - stub_feature_flags(version_snippets: false) - - expect do - subject - end.to change { PersonalSnippet.count }.by(1) - - expect(snippet.repository_exists?).to be_falsey - end - end end context 'with restricted visibility settings' do @@ -270,7 +255,7 @@ describe API::Snippets do let(:user) { create(:user, :external) } it 'does not create a new snippet' do - post api("/snippets/", user), params: params + subject expect(response).to have_gitlab_http_status(:forbidden) end @@ -279,16 +264,44 @@ describe API::Snippets do it 'returns 400 for missing parameters' do params.delete(:title) - post api("/snippets/", user), params: params + subject expect(response).to have_gitlab_http_status(:bad_request) end - context 'when the snippet is spam' do - def create_snippet(snippet_params = {}) - post api('/snippets', user), params: params.merge(snippet_params) + it 'returns 400 if content is blank' do + params[:content] = '' + + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'content is empty' + end + + it 'returns 400 if title is blank' do + params[:title] = '' + + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'title is empty' + end + + context 'when save fails because the repository could not be created' do + before do + allow_next_instance_of(Snippets::CreateService) do |instance| + allow(instance).to receive(:create_repository).and_raise(Snippets::CreateService::CreateRepositoryError) + end end + it 'returns 400' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when the snippet is spam' do before do allow_next_instance_of(Spam::AkismetService) do |instance| allow(instance).to receive(:spam?).and_return(true) @@ -296,23 +309,25 @@ describe API::Snippets do end context 'when the snippet is private' do + let(:extra_params) { { visibility: 'private' } } + it 'creates the snippet' do - expect { create_snippet(visibility: 'private') } - .to change { Snippet.count }.by(1) + expect { subject }.to change { Snippet.count }.by(1) end end context 'when the snippet is public' do + let(:extra_params) { { visibility: 'public' } } + it 'rejects the shippet' do - expect { create_snippet(visibility: 'public') } - .not_to change { Snippet.count } + expect { subject }.not_to change { Snippet.count } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']).to eq({ "error" => "Spam detected" }) end it 'creates a spam log' do - expect { create_snippet(visibility: 'public') } + expect { subject } .to log_spam(title: 'Test Title', user_id: user.id, noteable_type: 'PersonalSnippet') end end @@ -320,8 +335,9 @@ describe API::Snippets do end describe 'PUT /snippets/:id' do + let_it_be(:other_user) { create(:user) } + let(:visibility_level) { Snippet::PUBLIC } - let(:other_user) { create(:user) } let(:snippet) do create(:personal_snippet, :repository, author: user, visibility_level: visibility_level) end @@ -373,6 +389,20 @@ describe API::Snippets do expect(response).to have_gitlab_http_status(:bad_request) end + it 'returns 400 if content is blank' do + update_snippet(params: { content: '' }) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'content is empty' + end + + it 'returns 400 if title is blank' do + update_snippet(params: { title: '' }) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'title is empty' + end + it_behaves_like 'update with repository actions' do let(:snippet_without_repo) { create(:personal_snippet, author: user, visibility_level: visibility_level) } end @@ -424,6 +454,32 @@ describe API::Snippets do end end + context "when admin" do + let(:admin) { create(:admin) } + let(:token) { create(:personal_access_token, user: admin, scopes: [:sudo]) } + + subject do + put api("/snippets/#{snippet.id}", admin, personal_access_token: token), params: { visibility: 'private', sudo: user.id } + end + + context 'when sudo is defined' do + it 'returns 200 and updates snippet visibility' do + expect(snippet.visibility).not_to eq('private') + + subject + + expect(response).to have_gitlab_http_status(:success) + expect(json_response["visibility"]).to eq 'private' + end + + it 'does not commit data' do + expect_any_instance_of(SnippetRepository).not_to receive(:multi_files_action) + + subject + end + end + end + def update_snippet(snippet_id: snippet.id, params: {}, requester: user) put api("/snippets/#{snippet_id}", requester), params: params end diff --git a/spec/requests/api/statistics_spec.rb b/spec/requests/api/statistics_spec.rb index f03c1e9ca64..5aea5c225a0 100644 --- a/spec/requests/api/statistics_spec.rb +++ b/spec/requests/api/statistics_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' describe API::Statistics, 'Statistics' do include ProjectForksHelper - TABLES_TO_ANALYZE = %w[ + tables_to_analyze = %w[ projects users namespaces @@ -62,7 +62,7 @@ describe API::Statistics, 'Statistics' do # Make sure the reltuples have been updated # to get a correct count on postgresql - TABLES_TO_ANALYZE.each do |table| + tables_to_analyze.each do |table| ActiveRecord::Base.connection.execute("ANALYZE #{table}") end diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index 88c277f4e08..844cd948411 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -78,6 +78,14 @@ describe API::Terraform::State do expect(response).to have_gitlab_http_status(:ok) end + + context 'on Unicorn', :unicorn do + it 'updates the state' do + expect { request }.to change { Terraform::State.count }.by(0) + + expect(response).to have_gitlab_http_status(:ok) + end + end end context 'without body' do @@ -112,6 +120,14 @@ describe API::Terraform::State do expect(response).to have_gitlab_http_status(:ok) end + + context 'on Unicorn', :unicorn do + it 'creates a new state' do + expect { request }.to change { Terraform::State.count }.by(1) + + expect(response).to have_gitlab_http_status(:ok) + end + end end context 'without body' do diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index 1aa5e21dddb..0bdc71a30e9 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -159,6 +159,46 @@ describe API::Todos do expect { get api('/todos', john_doe) }.not_to exceed_query_limit(control) expect(response).to have_gitlab_http_status(:ok) end + + context 'when there is a Design Todo' do + let!(:design_todo) { create_todo_for_mentioned_in_design } + + def create_todo_for_mentioned_in_design + issue = create(:issue, project: project_1) + create(:todo, :mentioned, + user: john_doe, + project: project_1, + target: create(:design, issue: issue), + author: create(:user), + note: create(:note, project: project_1, note: "I am note, hear me roar")) + end + + def api_request + get api('/todos', john_doe) + end + + before do + api_request + end + + specify do + expect(response).to have_gitlab_http_status(:ok) + end + + it 'avoids N+1 queries', :request_store do + control = ActiveRecord::QueryRecorder.new { api_request } + + create_todo_for_mentioned_in_design + + expect { api_request }.not_to exceed_query_limit(control) + end + + it 'includes the Design Todo in the response' do + expect(json_response).to include( + a_hash_including('id' => design_todo.id) + ) + end + end end describe 'POST /todos/:id/mark_as_done' do @@ -235,6 +275,7 @@ describe API::Todos do expect(json_response['state']).to eq('pending') expect(json_response['action_name']).to eq('marked') expect(json_response['created_at']).to be_present + expect(json_response['updated_at']).to be_present end it 'returns 304 there already exist a todo on that issuable' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 864f6f77f39..4a0f0eea088 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -734,7 +734,7 @@ describe API::Users, :do_not_mock_admin_mode do end describe "PUT /users/:id" do - let!(:admin_user) { create(:admin) } + let_it_be(:admin_user) { create(:admin) } it "returns 200 OK on success" do put api("/users/#{user.id}", admin), params: { bio: 'new test bio' } @@ -2405,8 +2405,8 @@ describe API::Users, :do_not_mock_admin_mode do end context "user activities", :clean_gitlab_redis_shared_state do - let!(:old_active_user) { create(:user, last_activity_on: Time.utc(2000, 1, 1)) } - let!(:newly_active_user) { create(:user, last_activity_on: 2.days.ago.midday) } + let_it_be(:old_active_user) { create(:user, last_activity_on: Time.utc(2000, 1, 1)) } + let_it_be(:newly_active_user) { create(:user, last_activity_on: 2.days.ago.midday) } context 'last activity as normal user' do it 'has no permission' do diff --git a/spec/requests/api/wikis_spec.rb b/spec/requests/api/wikis_spec.rb index 7bd9a178a8d..43a5cb446bb 100644 --- a/spec/requests/api/wikis_spec.rb +++ b/spec/requests/api/wikis_spec.rb @@ -25,8 +25,8 @@ describe API::Wikis do shared_examples_for 'returns list of wiki pages' do context 'when wiki has pages' do let!(:pages) do - [create(:wiki_page, wiki: project_wiki, attrs: { title: 'page1', content: 'content of page1' }), - create(:wiki_page, wiki: project_wiki, attrs: { title: 'page2.with.dot', content: 'content of page2' })] + [create(:wiki_page, wiki: project_wiki, title: 'page1', content: 'content of page1'), + create(:wiki_page, wiki: project_wiki, title: 'page2.with.dot', content: 'content of page2')] end it 'returns the list of wiki pages without content' do |