diff options
Diffstat (limited to 'spec/requests/api/graphql')
22 files changed, 1047 insertions, 337 deletions
diff --git a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb index e086ce02942..db8a412e45c 100644 --- a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb +++ b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Getting Ci Cd Setting' do let(:fields) do <<~QUERY - #{all_graphql_fields_for('ProjectCiCdSetting')} + #{all_graphql_fields_for('ProjectCiCdSetting', max_depth: 1)} QUERY end @@ -43,8 +43,10 @@ RSpec.describe 'Getting Ci Cd Setting' do it_behaves_like 'a working graphql query' - specify { expect(settings_data['mergePipelinesEnabled']).to eql project.ci_cd_settings.merge_pipelines_enabled? } - specify { expect(settings_data['mergeTrainsEnabled']).to eql project.ci_cd_settings.merge_trains_enabled? } - specify { expect(settings_data['project']['id']).to eql "gid://gitlab/Project/#{project.id}" } + it 'fetches the settings data' do + expect(settings_data['mergePipelinesEnabled']).to eql project.ci_cd_settings.merge_pipelines_enabled? + expect(settings_data['mergeTrainsEnabled']).to eql project.ci_cd_settings.merge_trains_enabled? + expect(settings_data['keepLatestArtifact']).to eql project.ci_keep_latest_artifact? + end end end diff --git a/spec/requests/api/graphql/ci/config_spec.rb b/spec/requests/api/graphql/ci/config_spec.rb index b682470e0a1..8ede6e1538c 100644 --- a/spec/requests/api/graphql/ci/config_spec.rb +++ b/spec/requests/api/graphql/ci/config_spec.rb @@ -7,7 +7,8 @@ RSpec.describe 'Query.ciConfig' do subject(:post_graphql_query) { post_graphql(query, current_user: user) } - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository, creator: user, namespace: user.namespace) } let_it_be(:content) do File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci_includes.yml')) @@ -16,20 +17,41 @@ RSpec.describe 'Query.ciConfig' do let(:query) do %( query { - ciConfig(content: "#{content}") { + ciConfig(projectPath: "#{project.full_path}", content: "#{content}", dryRun: false) { status errors stages { - name - groups { + nodes { name - size - jobs { - name - groupName - stage - needs { + groups { + nodes { name + size + jobs { + nodes { + name + groupName + stage + script + beforeScript + afterScript + allowFailure + only { + refs + } + when + except { + refs + } + environment + tags + needs { + nodes { + name + } + } + } + } } } } @@ -39,53 +61,259 @@ RSpec.describe 'Query.ciConfig' do ) end - before do - post_graphql_query + it_behaves_like 'a working graphql query' do + before do + post_graphql_query + end end - it_behaves_like 'a working graphql query' - it 'returns the correct structure' do + post_graphql_query + expect(graphql_data['ciConfig']).to eq( "status" => "VALID", "errors" => [], "stages" => - [ - { - "name" => "build", - "groups" => - [ + { + "nodes" => + [ + { + "name" => "build", + "groups" => { - "name" => "rspec", - "size" => 2, - "jobs" => + "nodes" => [ - { "name" => "rspec 0 1", "groupName" => "rspec", "stage" => "build", "needs" => [] }, - { "name" => "rspec 0 2", "groupName" => "rspec", "stage" => "build", "needs" => [] } + { + "name" => "rspec", + "size" => 2, + "jobs" => + { + "nodes" => + [ + { + "name" => "rspec 0 1", + "groupName" => "rspec", + "stage" => "build", + "script" => ["rake spec"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => false, + "only" => { "refs" => %w[branches master] }, + "when" => "on_success", + "except" => nil, + "environment" => nil, + "tags" => %w[ruby postgres], + "needs" => { "nodes" => [] } + }, + { + "name" => "rspec 0 2", + "groupName" => "rspec", + "stage" => "build", + "script" => ["rake spec"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => true, + "only" => { "refs" => %w[branches tags] }, + "when" => "on_failure", + "except" => nil, + "environment" => nil, + "tags" => [], + "needs" => { "nodes" => [] } + } + ] + } + }, + { + "name" => "spinach", "size" => 1, "jobs" => + { + "nodes" => + [ + { + "name" => "spinach", + "groupName" => "spinach", + "stage" => "build", + "script" => ["rake spinach"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => false, + "only" => { "refs" => %w[branches tags] }, + "when" => "on_success", + "except" => { "refs" => ["tags"] }, + "environment" => nil, + "tags" => [], + "needs" => { "nodes" => [] } + } + ] + } + } ] - }, + } + }, + { + "name" => "test", + "groups" => { - "name" => "spinach", "size" => 1, "jobs" => + "nodes" => [ - { "name" => "spinach", "groupName" => "spinach", "stage" => "build", "needs" => [] } + { + "name" => "docker", + "size" => 1, + "jobs" => + { + "nodes" => [ + { + "name" => "docker", + "groupName" => "docker", + "stage" => "test", + "script" => ["curl http://dockerhub/URL"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => true, + "only" => { "refs" => %w[branches tags] }, + "when" => "manual", + "except" => { "refs" => ["branches"] }, + "environment" => nil, + "tags" => [], + "needs" => { "nodes" => [{ "name" => "spinach" }, { "name" => "rspec 0 1" }] } + } + ] + } + } ] } - ] - }, + }, + { + "name" => "deploy", + "groups" => + { + "nodes" => + [ + { + "name" => "deploy_job", + "size" => 1, + "jobs" => + { + "nodes" => [ + { + "name" => "deploy_job", + "groupName" => "deploy_job", + "stage" => "deploy", + "script" => ["echo 'done'"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => false, + "only" => { "refs" => %w[branches tags] }, + "when" => "on_success", + "except" => nil, + "environment" => "production", + "tags" => [], + "needs" => { "nodes" => [] } + } + ] + } + } + ] + } + } + ] + } + ) + end + + context 'when the config file includes other files' do + let_it_be(:content) do + YAML.dump( + include: 'other_file.yml', + rspec: { + script: 'rspec' + } + ) + end + + before do + allow_next_instance_of(Repository) do |repository| + allow(repository).to receive(:blob_data_at).with(an_instance_of(String), 'other_file.yml') do + YAML.dump( + build: { + script: 'build' + } + ) + end + end + + post_graphql_query + end + + it_behaves_like 'a working graphql query' + + it 'returns the correct structure with included files' do + expect(graphql_data['ciConfig']).to eq( + "status" => "VALID", + "errors" => [], + "stages" => { - "name" => "test", - "groups" => + "nodes" => [ { - "name" => "docker", - "size" => 1, - "jobs" => [ - { "name" => "docker", "groupName" => "docker", "stage" => "test", "needs" => [{ "name" => "spinach" }, { "name" => "rspec 0 1" }] } - ] + "name" => "test", + "groups" => + { + "nodes" => + [ + { + "name" => "build", + "size" => 1, + "jobs" => + { + "nodes" => + [ + { + "name" => "build", + "stage" => "test", + "groupName" => "build", + "script" => ["build"], + "afterScript" => [], + "beforeScript" => [], + "allowFailure" => false, + "environment" => nil, + "except" => nil, + "only" => { "refs" => %w[branches tags] }, + "when" => "on_success", + "tags" => [], + "needs" => { "nodes" => [] } +} + ] + } + }, + { + "name" => "rspec", + "size" => 1, + "jobs" => + { + "nodes" => + [ + { "name" => "rspec", + "stage" => "test", + "groupName" => "rspec", + "script" => ["rspec"], + "afterScript" => [], + "beforeScript" => [], + "allowFailure" => false, + "environment" => nil, + "except" => nil, + "only" => { "refs" => %w[branches tags] }, + "when" => "on_success", + "tags" => [], + "needs" => { "nodes" => [] } } + ] + } + } + ] + } } ] } - ] - ) + ) + end end end diff --git a/spec/requests/api/graphql/ci/jobs_spec.rb b/spec/requests/api/graphql/ci/jobs_spec.rb index 19954c4e52f..3fb89d6e815 100644 --- a/spec/requests/api/graphql/ci/jobs_spec.rb +++ b/spec/requests/api/graphql/ci/jobs_spec.rb @@ -7,48 +7,74 @@ RSpec.describe 'Query.project.pipeline' do let_it_be(:project) { create(:project, :repository, :public) } let_it_be(:user) { create(:user) } - def first(field) - [field.pluralize, 'nodes', 0] + def all(*fields) + fields.flat_map { |f| [f, :nodes] } end describe '.stages.groups.jobs' do let(:pipeline) do pipeline = create(:ci_pipeline, project: project, user: user) - stage = create(:ci_stage_entity, pipeline: pipeline, name: 'first') - create(:commit_status, stage_id: stage.id, pipeline: pipeline, name: 'my test job') + stage = create(:ci_stage_entity, project: project, pipeline: pipeline, name: 'first') + create(:ci_build, stage_id: stage.id, pipeline: pipeline, name: 'my test job') pipeline end - let(:jobs_graphql_data) { graphql_data.dig(*%w[project pipeline], *first('stage'), *first('group'), 'jobs', 'nodes') } + let(:jobs_graphql_data) { graphql_data_at(:project, :pipeline, *all(:stages, :groups, :jobs)) } + + let(:first_n) { var('Int') } let(:query) do - %( - query { - project(fullPath: "#{project.full_path}") { - pipeline(iid: "#{pipeline.iid}") { - stages { - nodes { - name - groups { - nodes { - name - jobs { - nodes { - name - pipeline { - id - } - } - } - } - } + with_signature([first_n], wrap_fields(query_graphql_path([ + [:project, { full_path: project.full_path }], + [:pipeline, { iid: pipeline.iid.to_s }], + [:stages, { first: first_n }] + ], stage_fields))) + end + + let(:stage_fields) do + <<~FIELDS + nodes { + name + groups { + nodes { + name + jobs { + nodes { + name + needs { + nodes { #{all_graphql_fields_for('CiBuildNeed')} } + } + pipeline { + id } } } } } - ) + } + FIELDS + end + + context 'when there are build needs' do + before do + pipeline.statuses.each do |build| + create_list(:ci_build_need, 2, build: build) + end + end + + it 'reports the build needs' do + post_graphql(query, current_user: user) + + expect(jobs_graphql_data).to contain_exactly a_hash_including( + 'needs' => a_hash_including( + 'nodes' => contain_exactly( + a_hash_including('name' => String), + a_hash_including('name' => String) + ) + ) + ) + end end it 'returns the jobs of a pipeline stage' do @@ -57,60 +83,43 @@ RSpec.describe 'Query.project.pipeline' do expect(jobs_graphql_data).to contain_exactly(a_hash_including('name' => 'my test job')) end - it 'avoids N+1 queries', :aggregate_failures do - control_count = ActiveRecord::QueryRecorder.new do - post_graphql(query, current_user: user) + describe 'performance' do + before do + build_stage = create(:ci_stage_entity, position: 2, name: 'build', project: project, pipeline: pipeline) + test_stage = create(:ci_stage_entity, position: 3, name: 'test', project: project, pipeline: pipeline) + create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 1 2') + create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 2 2') + create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 1 2') + create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 2 2') end - build_stage = create(:ci_stage_entity, name: 'build', pipeline: pipeline) - test_stage = create(:ci_stage_entity, name: 'test', pipeline: pipeline) - create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 1 2') - create(:commit_status, pipeline: pipeline, stage_id: build_stage.id, name: 'docker 2 2') - create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 1 2') - create(:commit_status, pipeline: pipeline, stage_id: test_stage.id, name: 'rspec 2 2') + it 'can find the first stage' do + post_graphql(query, current_user: user, variables: first_n.with(1)) - expect do - post_graphql(query, current_user: user) - end.not_to exceed_query_limit(control_count) + expect(jobs_graphql_data).to contain_exactly(a_hash_including('name' => 'my test job')) + end - expect(response).to have_gitlab_http_status(:ok) + it 'can find all stages' do + post_graphql(query, current_user: user, variables: first_n.with(3)) - build_stage = graphql_data.dig('project', 'pipeline', 'stages', 'nodes').find do |stage| - stage['name'] == 'build' + expect(jobs_graphql_data).to contain_exactly( + a_hash_including('name' => 'my test job'), + a_hash_including('name' => 'docker 1 2'), + a_hash_including('name' => 'docker 2 2'), + a_hash_including('name' => 'rspec 1 2'), + a_hash_including('name' => 'rspec 2 2') + ) end - test_stage = graphql_data.dig('project', 'pipeline', 'stages', 'nodes').find do |stage| - stage['name'] == 'test' + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new do + post_graphql(query, current_user: user, variables: first_n.with(1)) + end + + expect do + post_graphql(query, current_user: user, variables: first_n.with(3)) + end.not_to exceed_query_limit(control_count) end - docker_group = build_stage.dig('groups', 'nodes').first - rspec_group = test_stage.dig('groups', 'nodes').first - - expect(docker_group['name']).to eq('docker') - expect(rspec_group['name']).to eq('rspec') - - docker_jobs = docker_group.dig('jobs', 'nodes') - rspec_jobs = rspec_group.dig('jobs', 'nodes') - - expect(docker_jobs).to eq([ - { - 'name' => 'docker 1 2', - 'pipeline' => { 'id' => pipeline.to_global_id.to_s } - }, - { - 'name' => 'docker 2 2', - 'pipeline' => { 'id' => pipeline.to_global_id.to_s } - } - ]) - - expect(rspec_jobs).to eq([ - { - 'name' => 'rspec 1 2', - 'pipeline' => { 'id' => pipeline.to_global_id.to_s } - }, - { - 'name' => 'rspec 2 2', - 'pipeline' => { 'id' => pipeline.to_global_id.to_s } - } - ]) end end diff --git a/spec/requests/api/graphql/ci/pipelines_spec.rb b/spec/requests/api/graphql/ci/pipelines_spec.rb index 414ddabbac9..7933251b8e9 100644 --- a/spec/requests/api/graphql/ci/pipelines_spec.rb +++ b/spec/requests/api/graphql/ci/pipelines_spec.rb @@ -6,53 +6,59 @@ RSpec.describe 'Query.project(fullPath).pipelines' do include GraphqlHelpers let_it_be(:project) { create(:project, :repository, :public) } - let_it_be(:first_user) { create(:user) } - let_it_be(:second_user) { create(:user) } + let_it_be(:user) { create(:user) } describe '.jobs' do - let_it_be(:query) do - %( - query { - project(fullPath: "#{project.full_path}") { - pipelines { - nodes { - jobs { - nodes { - name - } - } - } - } - } - } - ) + let(:first_n) { var('Int') } + let(:query_path) do + [ + [:project, { full_path: project.full_path }], + [:pipelines, { first: first_n }], + [:nodes], + [:jobs], + [:nodes] + ] end - it 'fetches the jobs without an N+1' do + let(:query) do + with_signature([first_n], wrap_fields(query_graphql_path(query_path, :name))) + end + + before_all do pipeline = create(:ci_pipeline, project: project) create(:ci_build, pipeline: pipeline, name: 'Job 1') - - control_count = ActiveRecord::QueryRecorder.new do - post_graphql(query, current_user: first_user) - end - pipeline = create(:ci_pipeline, project: project) create(:ci_build, pipeline: pipeline, name: 'Job 2') + end - expect do - post_graphql(query, current_user: second_user) - end.not_to exceed_query_limit(control_count) + it 'limits the results' do + post_graphql(query, current_user: user, variables: first_n.with(1)) - expect(response).to have_gitlab_http_status(:ok) + expect(graphql_data_at(*query_path.map(&:first))).to contain_exactly a_hash_including( + 'name' => 'Job 2' + ) + end - pipelines_data = graphql_data.dig('project', 'pipelines', 'nodes') + it 'fetches all results' do + post_graphql(query, current_user: user) - job_names = pipelines_data.map do |pipeline_data| - jobs_data = pipeline_data.dig('jobs', 'nodes') - jobs_data.map { |job_data| job_data['name'] } - end.flatten + expect(graphql_data_at(*query_path.map(&:first))).to contain_exactly( + a_hash_including('name' => 'Job 1'), + a_hash_including('name' => 'Job 2') + ) + end + + it 'fetches the jobs without an N+1' do + first_user = create(:personal_access_token).user + second_user = create(:personal_access_token).user + + control_count = ActiveRecord::QueryRecorder.new do + post_graphql(query, current_user: first_user, variables: first_n.with(1)) + end - expect(job_names).to contain_exactly('Job 1', 'Job 2') + expect do + post_graphql(query, current_user: second_user) + end.not_to exceed_query_limit(control_count) end end @@ -80,7 +86,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do create(:ci_build, :dast, name: 'DAST Job 1', pipeline: pipeline) create(:ci_build, :sast, name: 'SAST Job 1', pipeline: pipeline) - post_graphql(query, current_user: first_user) + post_graphql(query, current_user: user) expect(response).to have_gitlab_http_status(:ok) @@ -96,9 +102,9 @@ RSpec.describe 'Query.project(fullPath).pipelines' do end describe 'upstream' do - let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: first_user) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } let_it_be(:upstream_project) { create(:project, :repository, :public) } - let_it_be(:upstream_pipeline) { create(:ci_pipeline, project: upstream_project, user: first_user) } + let_it_be(:upstream_pipeline) { create(:ci_pipeline, project: upstream_project, user: user) } let(:upstream_pipelines_graphql_data) { graphql_data.dig(*%w[project pipelines nodes]).first['upstream'] } let(:query) do @@ -120,7 +126,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do before do create(:ci_sources_pipeline, source_pipeline: upstream_pipeline, pipeline: pipeline ) - post_graphql(query, current_user: first_user) + post_graphql(query, current_user: user) end it_behaves_like 'a working graphql query' @@ -131,15 +137,18 @@ RSpec.describe 'Query.project(fullPath).pipelines' do context 'when fetching the upstream pipeline from the pipeline' do it 'avoids N+1 queries' do + first_user = create(:user) + second_user = create(:user) + control_count = ActiveRecord::QueryRecorder.new do post_graphql(query, current_user: first_user) end - pipeline_2 = create(:ci_pipeline, project: project, user: first_user) - upstream_pipeline_2 = create(:ci_pipeline, project: upstream_project, user: first_user) + pipeline_2 = create(:ci_pipeline, project: project, user: user) + upstream_pipeline_2 = create(:ci_pipeline, project: upstream_project, user: user) create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_2, pipeline: pipeline_2 ) - pipeline_3 = create(:ci_pipeline, project: project, user: first_user) - upstream_pipeline_3 = create(:ci_pipeline, project: upstream_project, user: first_user) + pipeline_3 = create(:ci_pipeline, project: project, user: user) + upstream_pipeline_3 = create(:ci_pipeline, project: upstream_project, user: user) create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_3, pipeline: pipeline_3 ) expect do @@ -152,12 +161,12 @@ RSpec.describe 'Query.project(fullPath).pipelines' do end describe 'downstream' do - let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: first_user) } - let(:pipeline_2) { create(:ci_pipeline, project: project, user: first_user) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } + let(:pipeline_2) { create(:ci_pipeline, project: project, user: user) } let_it_be(:downstream_project) { create(:project, :repository, :public) } - let_it_be(:downstream_pipeline_a) { create(:ci_pipeline, project: downstream_project, user: first_user) } - let_it_be(:downstream_pipeline_b) { create(:ci_pipeline, project: downstream_project, user: first_user) } + let_it_be(:downstream_pipeline_a) { create(:ci_pipeline, project: downstream_project, user: user) } + let_it_be(:downstream_pipeline_b) { create(:ci_pipeline, project: downstream_project, user: user) } let(:pipelines_graphql_data) { graphql_data.dig(*%w[project pipelines nodes]) } @@ -183,7 +192,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downstream_pipeline_a) create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downstream_pipeline_b) - post_graphql(query, current_user: first_user) + post_graphql(query, current_user: user) end it_behaves_like 'a working graphql query' @@ -198,16 +207,19 @@ RSpec.describe 'Query.project(fullPath).pipelines' do context 'when fetching the downstream pipelines from the pipeline' do it 'avoids N+1 queries' do + first_user = create(:user) + second_user = create(:user) + control_count = ActiveRecord::QueryRecorder.new do post_graphql(query, current_user: first_user) end - downstream_pipeline_2a = create(:ci_pipeline, project: downstream_project, user: first_user) + downstream_pipeline_2a = create(:ci_pipeline, project: downstream_project, user: user) create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downstream_pipeline_2a) - downsteam_pipeline_3a = create(:ci_pipeline, project: downstream_project, user: first_user) + downsteam_pipeline_3a = create(:ci_pipeline, project: downstream_project, user: user) create(:ci_sources_pipeline, source_pipeline: pipeline, pipeline: downsteam_pipeline_3a) - downstream_pipeline_2b = create(:ci_pipeline, project: downstream_project, user: first_user) + downstream_pipeline_2b = create(:ci_pipeline, project: downstream_project, user: user) create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downstream_pipeline_2b) downsteam_pipeline_3b = create(:ci_pipeline, project: downstream_project, user: first_user) create(:ci_sources_pipeline, source_pipeline: pipeline_2, pipeline: downsteam_pipeline_3b) diff --git a/spec/requests/api/graphql/group/group_members_spec.rb b/spec/requests/api/graphql/group/group_members_spec.rb index 3554e22cdf2..452610ab18f 100644 --- a/spec/requests/api/graphql/group/group_members_spec.rb +++ b/spec/requests/api/graphql/group/group_members_spec.rb @@ -10,15 +10,13 @@ RSpec.describe 'getting group members information' do let_it_be(:user_1) { create(:user, username: 'user') } let_it_be(:user_2) { create(:user, username: 'test') } - let(:member_data) { graphql_data['group']['groupMembers']['edges'] } - before_all do [user_1, user_2].each { |user| parent_group.add_guest(user) } end context 'when the request is correct' do it_behaves_like 'a working graphql query' do - before_all do + before do fetch_members end end @@ -80,12 +78,10 @@ RSpec.describe 'getting group members information' do end context 'when unauthenticated' do - it 'returns nothing' do + it 'returns visible members' do fetch_members(current_user: nil) - expect(graphql_errors).to be_nil - expect(response).to have_gitlab_http_status(:success) - expect(member_data).to be_empty + expect_array_response(user_1, user_2) end end @@ -112,8 +108,8 @@ RSpec.describe 'getting group members information' do def expect_array_response(*items) expect(response).to have_gitlab_http_status(:success) - expect(member_data).to be_an Array - expect(member_data.map { |node| node["node"]["user"]["id"] }) - .to match_array(items.map { |u| global_id_of(u) }) + member_gids = graphql_data_at(:group, :group_members, :edges, :node, :user, :id) + + expect(member_gids).to match_array(items.map { |u| global_id_of(u) }) end end diff --git a/spec/requests/api/graphql/mutations/alert_management/alerts/todo/create_spec.rb b/spec/requests/api/graphql/mutations/alert_management/alerts/todo/create_spec.rb index e5803f50474..cd423d7764a 100644 --- a/spec/requests/api/graphql/mutations/alert_management/alerts/todo/create_spec.rb +++ b/spec/requests/api/graphql/mutations/alert_management/alerts/todo/create_spec.rb @@ -42,6 +42,8 @@ RSpec.describe 'Creating a todo for the alert' do context 'todo already exists' do before do + stub_feature_flags(multiple_todos: false) + create(:todo, :pending, project: project, user: user, target: alert) end diff --git a/spec/requests/api/graphql/mutations/alert_management/http_integration/create_spec.rb b/spec/requests/api/graphql/mutations/alert_management/http_integration/create_spec.rb index a285cebc805..e594d67aab4 100644 --- a/spec/requests/api/graphql/mutations/alert_management/http_integration/create_spec.rb +++ b/spec/requests/api/graphql/mutations/alert_management/http_integration/create_spec.rb @@ -39,21 +39,7 @@ RSpec.describe 'Creating a new HTTP Integration' do project.add_maintainer(current_user) end - it 'creates a new integration' do - post_graphql_mutation(mutation, current_user: current_user) - - new_integration = ::AlertManagement::HttpIntegration.last! - integration_response = mutation_response['integration'] - - expect(response).to have_gitlab_http_status(:success) - expect(integration_response['id']).to eq(GitlabSchema.id_from_object(new_integration).to_s) - expect(integration_response['type']).to eq('HTTP') - expect(integration_response['name']).to eq(new_integration.name) - expect(integration_response['active']).to eq(new_integration.active) - expect(integration_response['token']).to eq(new_integration.token) - expect(integration_response['url']).to eq(new_integration.url) - expect(integration_response['apiUrl']).to eq(nil) - end + it_behaves_like 'creating a new HTTP integration' [:project_path, :active, :name].each do |argument| context "without required argument #{argument}" do diff --git a/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb b/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb new file mode 100644 index 00000000000..283badeaf33 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'CiCdSettingsUpdate' do + include GraphqlHelpers + + let_it_be(:project) { create(:project, ci_keep_latest_artifact: true) } + let(:variables) { { full_path: project.full_path, keep_latest_artifact: false } } + let(:mutation) { graphql_mutation(:ci_cd_settings_update, variables) } + + context 'when unauthorized' do + let(:user) { create(:user) } + + shared_examples 'unauthorized' do + it 'returns an error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).not_to be_empty + end + end + + context 'when not a project member' do + it_behaves_like 'unauthorized' + end + + context 'when a non-admin project member' do + before do + project.add_developer(user) + end + + it_behaves_like 'unauthorized' + end + end + + context 'when authorized' do + let_it_be(:user) { project.owner } + + it 'updates ci cd settings' do + post_graphql_mutation(mutation, current_user: user) + + project.reload + + expect(response).to have_gitlab_http_status(:success) + expect(project.ci_keep_latest_artifact).to eq(false) + end + + context 'when bad arguments are provided' do + let(:variables) { { full_path: '', keep_latest_artifact: false } } + + it 'returns the errors' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).not_to be_empty + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb new file mode 100644 index 00000000000..749373e7b8d --- /dev/null +++ b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Updating the package settings' do + include GraphqlHelpers + using RSpec::Parameterized::TableSyntax + + let_it_be(:user) { create(:user) } + + let(:params) do + { + namespace_path: namespace.full_path, + maven_duplicates_allowed: false, + maven_duplicate_exception_regex: 'foo-.*' + } + end + + let(:mutation) do + graphql_mutation(:update_namespace_package_settings, params) do + <<~QL + packageSettings { + mavenDuplicatesAllowed + mavenDuplicateExceptionRegex + } + errors + QL + end + end + + let(:mutation_response) { graphql_mutation_response(:update_namespace_package_settings) } + let(:package_settings_response) { mutation_response['packageSettings'] } + + RSpec.shared_examples 'returning a success' do + it_behaves_like 'returning response status', :success + + it 'returns the updated package settings', :aggregate_failures do + subject + + expect(mutation_response['errors']).to be_empty + expect(package_settings_response['mavenDuplicatesAllowed']).to eq(params[:maven_duplicates_allowed]) + expect(package_settings_response['mavenDuplicateExceptionRegex']).to eq(params[:maven_duplicate_exception_regex]) + end + end + + RSpec.shared_examples 'rejecting invalid regex' do + context "for field mavenDuplicateExceptionRegex" do + let_it_be(:invalid_regex) { '][' } + + let(:params) do + { + :namespace_path => namespace.full_path, + 'mavenDuplicateExceptionRegex' => invalid_regex + } + end + + it_behaves_like 'returning response status', :success + + it_behaves_like 'not creating the namespace package setting' + + it 'returns an error', :aggregate_failures do + subject + + expect(graphql_errors.size).to eq(1) + expect(graphql_errors.first['message']).to include("#{invalid_regex} is an invalid regexp") + end + end + end + + RSpec.shared_examples 'accepting the mutation request updating the package settings' do + it_behaves_like 'updating the namespace package setting attributes', + from: { maven_duplicates_allowed: true, maven_duplicate_exception_regex: 'SNAPSHOT' }, + to: { maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'foo-.*' } + + it_behaves_like 'returning a success' + it_behaves_like 'rejecting invalid regex' + end + + RSpec.shared_examples 'accepting the mutation request creating the package settings' do + it_behaves_like 'creating the namespace package setting' + it_behaves_like 'returning a success' + it_behaves_like 'rejecting invalid regex' + end + + RSpec.shared_examples 'denying the mutation request' do + it_behaves_like 'not creating the namespace package setting' + + it_behaves_like 'returning response status', :success + + it 'returns no response' do + subject + + expect(mutation_response).to be_nil + end + end + + describe 'post graphql mutation' do + subject { post_graphql_mutation(mutation, current_user: user) } + + context 'with existing package settings' do + let_it_be(:package_settings, reload: true) { create(:namespace_package_setting, :group) } + let_it_be(:namespace, reload: true) { package_settings.namespace } + + where(:user_role, :shared_examples_name) do + :maintainer | 'accepting the mutation request updating the package settings' + :developer | 'accepting the mutation request updating the package settings' + :reporter | 'denying the mutation request' + :guest | 'denying the mutation request' + :anonymous | 'denying the mutation request' + end + + with_them do + before do + namespace.send("add_#{user_role}", user) unless user_role == :anonymous + end + + it_behaves_like params[:shared_examples_name] + end + end + + context 'without existing package settings' do + let_it_be(:namespace, reload: true) { create(:group) } + let(:package_settings) { namespace.package_settings } + + where(:user_role, :shared_examples_name) do + :maintainer | 'accepting the mutation request creating the package settings' + :developer | 'accepting the mutation request creating the package settings' + :reporter | 'denying the mutation request' + :guest | 'denying the mutation request' + :anonymous | 'denying the mutation request' + end + + with_them do + before do + namespace.send("add_#{user_role}", user) unless user_role == :anonymous + end + + it_behaves_like params[:shared_examples_name] + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/releases/create_spec.rb b/spec/requests/api/graphql/mutations/releases/create_spec.rb index d745eb3083d..79bdcec7944 100644 --- a/spec/requests/api/graphql/mutations/releases/create_spec.rb +++ b/spec/requests/api/graphql/mutations/releases/create_spec.rb @@ -116,11 +116,9 @@ RSpec.describe 'Creation of a new release' do context 'when all available mutation arguments are provided' do it_behaves_like 'no errors' - # rubocop: disable CodeReuse/ActiveRecord it 'returns the new release data' do create_release - release = mutation_response[:release] expected_direct_asset_url = Gitlab::Routing.url_helpers.project_release_url(project, Release.find_by(tag: tag_name)) << "/downloads#{asset_link[:directAssetPath]}" expected_attributes = { @@ -139,21 +137,17 @@ RSpec.describe 'Creation of a new release' do directAssetUrl: expected_direct_asset_url }] } + }, + milestones: { + nodes: [ + { title: '12.3' }, + { title: '12.4' } + ] } - } - - expect(release).to include(expected_attributes) + }.with_indifferent_access - # Right now the milestones are returned in a non-deterministic order. - # This `milestones` test should be moved up into the expect(release) - # above (and `.to include` updated to `.to eq`) once - # https://gitlab.com/gitlab-org/gitlab/-/issues/259012 is addressed. - expect(release['milestones']['nodes']).to match_array([ - { 'title' => '12.4' }, - { 'title' => '12.3' } - ]) + expect(mutation_response[:release]).to eq(expected_attributes) end - # rubocop: enable CodeReuse/ActiveRecord end context 'when only the required mutation arguments are provided' do diff --git a/spec/requests/api/graphql/mutations/releases/update_spec.rb b/spec/requests/api/graphql/mutations/releases/update_spec.rb index 19320c3393c..c9a6c3abd57 100644 --- a/spec/requests/api/graphql/mutations/releases/update_spec.rb +++ b/spec/requests/api/graphql/mutations/releases/update_spec.rb @@ -116,15 +116,7 @@ RSpec.describe 'Updating an existing release' do it 'updates the correct field and returns the release' do update_release - expect(mutation_response[:release]).to include(expected_attributes.merge(updates).except(:milestones)) - - # Right now the milestones are returned in a non-deterministic order. - # Because of this, we need to test milestones separately to allow - # for them to be returned in any order. - # Once https://gitlab.com/gitlab-org/gitlab/-/issues/259012 has been - # fixed, this special milestone handling can be removed. - expected_milestones = expected_attributes.merge(updates)[:milestones] - expect(mutation_response[:release][:milestones][:nodes]).to match_array(expected_milestones[:nodes]) + expect(mutation_response[:release]).to eq(expected_attributes.merge(updates)) end end diff --git a/spec/requests/api/graphql/namespace/package_settings_spec.rb b/spec/requests/api/graphql/namespace/package_settings_spec.rb new file mode 100644 index 00000000000..6af098e902f --- /dev/null +++ b/spec/requests/api/graphql/namespace/package_settings_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting namespace package settings in a namespace' do + include GraphqlHelpers + + let_it_be(:package_settings) { create(:namespace_package_setting) } + let_it_be(:namespace) { package_settings.namespace } + let_it_be(:current_user) { namespace.owner } + let(:package_settings_response) { graphql_data.dig('namespace', 'packageSettings') } + let(:fields) { all_graphql_fields_for('PackageSettings') } + + let(:query) do + graphql_query_for( + 'namespace', + { 'fullPath' => namespace.full_path }, + query_graphql_field('package_settings', {}, fields) + ) + end + + subject { post_graphql(query, current_user: current_user) } + + it_behaves_like 'a working graphql query' do + before do + subject + end + + it 'matches the JSON schema' do + expect(package_settings_response).to match_schema('graphql/namespace/package_settings') + end + end +end diff --git a/spec/requests/api/graphql/packages/package_composer_details_spec.rb b/spec/requests/api/graphql/packages/package_composer_details_spec.rb new file mode 100644 index 00000000000..1a2cf4a972a --- /dev/null +++ b/spec/requests/api/graphql/packages/package_composer_details_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe 'package composer details' do + using RSpec::Parameterized::TableSyntax + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:package) { create(:composer_package, project: project) } + let_it_be(:composer_metadatum) do + # we are forced to manually create the metadatum, without using the factory to force the sha to be a string + # and avoid an error where gitaly can't find the repository + create(:composer_metadatum, package: package, target_sha: 'foo_sha', composer_json: { name: 'name', type: 'type', license: 'license', version: 1 }) + end + + let(:query) do + graphql_query_for( + 'packageComposerDetails', + { id: package_global_id }, + all_graphql_fields_for('PackageComposerDetails', max_depth: 2) + ) + end + + let(:user) { project.owner } + let(:package_global_id) { package.to_global_id.to_s } + let(:package_composer_details_response) { graphql_data.dig('packageComposerDetails') } + + subject { post_graphql(query, current_user: user) } + + it_behaves_like 'a working graphql query' do + before do + subject + end + + it 'matches the JSON schema' do + expect(package_composer_details_response).to match_schema('graphql/packages/package_composer_details') + end + end +end diff --git a/spec/requests/api/graphql/project/alert_management/alert/assignees_spec.rb b/spec/requests/api/graphql/project/alert_management/alert/assignees_spec.rb index dd001a73349..9ab94f1d749 100644 --- a/spec/requests/api/graphql/project/alert_management/alert/assignees_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alert/assignees_spec.rb @@ -60,19 +60,35 @@ RSpec.describe 'getting Alert Management Alert Assignees' do expect(second_assignees).to be_empty end - it 'avoids N+1 queries' do - base_count = ActiveRecord::QueryRecorder.new do - post_graphql(query, current_user: current_user) + describe 'performance' do + let(:first_n) { var('Int') } + let(:params) { { first: first_n } } + let(:limited_query) { with_signature([first_n], query) } + + before do + create(:alert_management_alert, project: project, assignees: [current_user]) + end + + it 'can limit results' do + post_graphql(limited_query, current_user: current_user, variables: first_n.with(1)) + + expect(alerts.size).to eq 1 + expect(alerts).not_to include(a_hash_including('iid' => first_alert.iid.to_s)) end - # An N+1 would mean a new alert would increase the query count - third_alert = create(:alert_management_alert, project: project, assignees: [current_user]) + it 'can include all results' do + post_graphql(limited_query, current_user: current_user) - expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(base_count) + expect(alerts.size).to be > 1 + expect(alerts).to include(a_hash_including('iid' => first_alert.iid.to_s)) + end - third_assignees = assignees[third_alert.iid.to_s] + it 'avoids N+1 queries' do + base_count = ActiveRecord::QueryRecorder.new do + post_graphql(limited_query, current_user: current_user, variables: first_n.with(1)) + end - expect(third_assignees.length).to eq(1) - expect(third_assignees.first).to include('username' => current_user.username) + expect { post_graphql(limited_query, current_user: current_user) }.not_to exceed_query_limit(base_count) + end end end diff --git a/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb b/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb index 1350cba119b..5d46f370756 100644 --- a/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb @@ -65,16 +65,28 @@ RSpec.describe 'getting Alert Management Alert Notes' do expect(second_notes_result).to be_empty end - it 'avoids N+1 queries' do - base_count = ActiveRecord::QueryRecorder.new do - post_graphql(query, current_user: current_user) + describe 'performance' do + let(:first_n) { var('Int') } + let(:params) { { first: first_n } } + + before do + # An N+1 would mean a new alert would increase the query count + create(:alert_management_alert, project: project) end - # An N+1 would mean a new alert would increase the query count - create(:alert_management_alert, project: project) + it 'avoids N+1 queries' do + q = with_signature([first_n], query) - expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(base_count) - expect(alerts_result.length).to eq(3) + base_count = ActiveRecord::QueryRecorder.new do + post_graphql(q, current_user: current_user, variables: first_n.with(1)) + expect(alerts_result.length).to eq(1) + end + + expect do + post_graphql(q, current_user: current_user, variables: first_n.with(3)) + expect(alerts_result.length).to eq(3) + end.not_to exceed_query_limit(base_count) + end end context 'for non-system notes' do diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index fae52fe814d..e1b867ad097 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -9,12 +9,13 @@ RSpec.describe 'getting merge request information nested in a project' do let(:current_user) { create(:user) } let(:merge_request_graphql_data) { graphql_data['project']['mergeRequest'] } let!(:merge_request) { create(:merge_request, source_project: project) } + let(:mr_fields) { all_graphql_fields_for('MergeRequest') } let(:query) do graphql_query_for( 'project', { 'fullPath' => project.full_path }, - query_graphql_field('mergeRequest', iid: merge_request.iid.to_s) + query_graphql_field('mergeRequest', { iid: merge_request.iid.to_s }, mr_fields) ) end @@ -43,6 +44,38 @@ RSpec.describe 'getting merge request information nested in a project' do expect(merge_request_graphql_data['author']['username']).to eq(merge_request.author.username) end + context 'the merge_request has reviewers' do + let(:mr_fields) do + <<~SELECT + reviewers { nodes { id username } } + participants { nodes { id username } } + SELECT + end + + before do + merge_request.reviewers << create_list(:user, 2) + end + + it 'includes reviewers' do + expected = merge_request.reviewers.map do |r| + a_hash_including('id' => global_id_of(r), 'username' => r.username) + end + + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project, :merge_request, :reviewers, :nodes)).to match_array(expected) + expect(graphql_data_at(:project, :merge_request, :participants, :nodes)).to include(*expected) + end + + it 'suppresses reviewers if reviewers are not allowed' do + stub_feature_flags(merge_request_reviewers: false) + + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project, :merge_request, :reviewers)).to be_nil + end + end + it 'includes diff stats' do be_natural = an_instance_of(Integer).and(be >= 0) @@ -219,4 +252,41 @@ RSpec.describe 'getting merge request information nested in a project' do expect(merge_request_graphql_data['mergeStatus']).to eq('checking') end end + + # see: https://gitlab.com/gitlab-org/gitlab/-/issues/297358 + context 'when the notes have been preloaded (by participants)' do + let(:query) do + <<~GQL + query($path: ID!) { + project(fullPath: $path) { + mrs: mergeRequests(first: 1) { + nodes { + participants { nodes { id } } + notes(first: 1) { + pageInfo { endCursor hasPreviousPage hasNextPage } + nodes { id } + } + } + } + } + } + GQL + end + + before do + create_list(:note_on_merge_request, 3, project: project, noteable: merge_request) + end + + it 'does not error' do + post_graphql(query, + current_user: current_user, + variables: { path: project.full_path }) + + expect(graphql_data_at(:project, :mrs, :nodes, :notes, :pageInfo)).to contain_exactly a_hash_including( + 'endCursor' => String, + 'hasNextPage' => true, + 'hasPreviousPage' => false + ) + end + end end diff --git a/spec/requests/api/graphql/project/merge_requests_spec.rb b/spec/requests/api/graphql/project/merge_requests_spec.rb index c05a620bb62..c85cb8b2ffe 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -23,9 +23,7 @@ RSpec.describe 'getting merge request listings nested in a project' do graphql_query_for( :project, { full_path: project.full_path }, - query_graphql_field(:merge_requests, search_params, [ - query_graphql_field(:nodes, nil, fields) - ]) + query_nodes(:merge_requests, fields, args: search_params) ) end @@ -50,24 +48,22 @@ RSpec.describe 'getting merge request listings nested in a project' do project.repository.expire_branches_cache end + let(:graphql_data) do + GitlabSchema.execute(query, context: { current_user: current_user }).to_h['data'] + end + context 'selecting any single scalar field' do where(:field) do scalar_fields_of('MergeRequest').map { |name| [name] } end with_them do - it_behaves_like 'a working graphql query' do - let(:query) do - query_merge_requests([:iid, field].uniq) - end - - before do - post_graphql(query, current_user: current_user) - end - - it 'selects the correct MR' do - expect(results).to contain_exactly(a_hash_including('iid' => fresh_mr.iid.to_s)) - end + let(:query) do + query_merge_requests([:iid, field].uniq) + end + + it 'selects the correct MR' do + expect(results).to contain_exactly(a_hash_including('iid' => fresh_mr.iid.to_s)) end end end @@ -87,19 +83,13 @@ RSpec.describe 'getting merge request listings nested in a project' do end with_them do - it_behaves_like 'a working graphql query' do - let(:query) do - fld = is_connection ? query_graphql_field(:nodes, nil, [subfield]) : subfield - query_merge_requests([:iid, query_graphql_field(field, nil, [fld])]) - end - - before do - post_graphql(query, current_user: current_user) - end - - it 'selects the correct MR' do - expect(results).to contain_exactly(a_hash_including('iid' => fresh_mr.iid.to_s)) - end + let(:query) do + fld = is_connection ? query_graphql_field(:nodes, nil, [subfield]) : subfield + query_merge_requests([:iid, query_graphql_field(field, nil, [fld])]) + end + + it 'selects the correct MR' do + expect(results).to contain_exactly(a_hash_including('iid' => fresh_mr.iid.to_s)) end end end @@ -254,6 +244,115 @@ RSpec.describe 'getting merge request listings nested in a project' do include_examples 'N+1 query check' end + + context 'when requesting reviewers' do + let(:requested_fields) { ['reviewers { nodes { username } }'] } + + before do + merge_request_a.reviewers << create(:user) + merge_request_a.reviewers << create(:user) + merge_request_c.reviewers << create(:user) + end + + it 'returns the reviewers' do + execute_query + + expect(results).to include a_hash_including('reviewers' => { + 'nodes' => match_array(merge_request_a.reviewers.map do |r| + a_hash_including('username' => r.username) + end) + }) + end + + context 'the feature flag is disabled' do + before do + stub_feature_flags(merge_request_reviewers: false) + end + + it 'does not return reviewers' do + execute_query + + expect(results).to all(match a_hash_including('reviewers' => be_nil)) + end + end + + include_examples 'N+1 query check' + end + end + + describe 'performance' do + let(:mr_fields) do + <<~SELECT + assignees { nodes { username } } + reviewers { nodes { username } } + participants { nodes { username } } + headPipeline { status } + SELECT + end + + let(:query) do + <<~GQL + query($first: Int) { + project(fullPath: "#{project.full_path}") { + mergeRequests(first: $first) { + nodes { #{mr_fields} } + } + } + } + GQL + end + + before_all do + project.add_developer(current_user) + mrs = create_list(:merge_request, 10, :closed, :with_head_pipeline, + source_project: project, + author: current_user) + mrs.each do |mr| + mr.assignees << create(:user) + mr.assignees << current_user + mr.reviewers << create(:user) + mr.reviewers << current_user + end + end + + before do + # Confounding factor: makes DB calls in EE + allow(Gitlab::Database).to receive(:read_only?).and_return(false) + end + + def run_query(number) + # Ensure that we have a fresh request store and batch-context between runs + result = run_with_clean_state(query, + context: { current_user: current_user }, + variables: { first: number } + ) + + graphql_dig_at(result.to_h, :data, :project, :merge_requests, :nodes) + end + + def user_collection + { 'nodes' => all(match(a_hash_including('username' => be_present))) } + end + + it 'returns appropriate results' do + mrs = run_query(2) + + expect(mrs.size).to eq(2) + expect(mrs).to all( + match( + a_hash_including( + 'assignees' => user_collection, + 'reviewers' => user_collection, + 'participants' => user_collection, + 'headPipeline' => { 'status' => be_present } + ))) + end + + it 'can lookahead to eliminate N+1 queries' do + baseline = ActiveRecord::QueryRecorder.new { run_query(1) } + + expect { run_query(10) }.not_to exceed_query_limit(baseline) + end end describe 'sorting and pagination' do diff --git a/spec/requests/api/graphql/project/project_members_spec.rb b/spec/requests/api/graphql/project/project_members_spec.rb index cb937432ef7..984a0adb8c6 100644 --- a/spec/requests/api/graphql/project/project_members_spec.rb +++ b/spec/requests/api/graphql/project/project_members_spec.rb @@ -11,15 +11,13 @@ RSpec.describe 'getting project members information' do let_it_be(:user_1) { create(:user, username: 'user') } let_it_be(:user_2) { create(:user, username: 'test') } - let(:member_data) { graphql_data['project']['projectMembers']['edges'] } - before_all do [user_1, user_2].each { |user| parent_group.add_guest(user) } end context 'when the request is correct' do it_behaves_like 'a working graphql query' do - before_all do + before do fetch_members(project: parent_project) end end @@ -114,8 +112,7 @@ RSpec.describe 'getting project members information' do def expect_array_response(*items) expect(response).to have_gitlab_http_status(:success) - expect(member_data).to be_an Array - expect(member_data.map { |node| node['node']['user']['id'] }) - .to match_array(items.map { |u| global_id_of(u) }) + member_gids = graphql_data_at(:project, :project_members, :edges, :node, :user, :id) + expect(member_gids).to match_array(items.map { |u| global_id_of(u) }) end end diff --git a/spec/requests/api/graphql/project/release_spec.rb b/spec/requests/api/graphql/project/release_spec.rb index 99b15ff00b1..ccc2825da25 100644 --- a/spec/requests/api/graphql/project/release_spec.rb +++ b/spec/requests/api/graphql/project/release_spec.rb @@ -76,11 +76,11 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do it 'finds all milestones associated to a release' do post_query - expected = release.milestones.map do |milestone| + expected = release.milestones.order_by_dates_and_title.map do |milestone| { 'id' => global_id_of(milestone), 'title' => milestone.title } end - expect(data).to match_array(expected) + expect(data).to eq(expected) end end @@ -427,4 +427,33 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do end end end + + describe 'milestone order' do + let(:path) { path_prefix } + let(:current_user) { stranger } + let_it_be(:project) { create(:project, :public) } + let_it_be_with_reload(:release) { create(:release, project: project) } + + let(:release_fields) do + query_graphql_field(%{ + milestones { + nodes { + title + } + } + }) + end + + let(:actual_milestone_title_order) do + post_query + + data.dig('milestones', 'nodes').map { |m| m['title'] } + end + + before do + release.update!(milestones: [milestone_2, milestone_1]) + end + + it_behaves_like 'correct release milestone order' + end end diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index b29f9ae913f..2cdd7273b18 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -6,26 +6,21 @@ RSpec.describe 'getting project information' do include GraphqlHelpers let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, :repository, group: group) } + let_it_be(:project, reload: true) { create(:project, :repository, group: group) } let_it_be(:current_user) { create(:user) } - let(:fields) { all_graphql_fields_for(Project, max_depth: 2, excluded: %w(jiraImports services)) } + let(:project_fields) { all_graphql_fields_for('project'.to_s.classify, max_depth: 1) } let(:query) do - graphql_query_for(:project, { full_path: project.full_path }, fields) + graphql_query_for(:project, { full_path: project.full_path }, project_fields) end context 'when the user has full access to the project' do - let(:full_access_query) do - graphql_query_for(:project, { full_path: project.full_path }, - all_graphql_fields_for('Project', max_depth: 2)) - end - before do project.add_maintainer(current_user) end it 'includes the project', :use_clean_rails_memory_store_caching, :request_store do - post_graphql(full_access_query, current_user: current_user) + post_graphql(query, current_user: current_user) expect(graphql_data['project']).not_to be_nil end @@ -49,12 +44,12 @@ RSpec.describe 'getting project information' do end context 'when there are pipelines present' do + let(:project_fields) { query_nodes(:pipelines) } + before do create(:ci_pipeline, project: project) end - let(:fields) { query_nodes(:pipelines) } - it 'is included in the pipelines connection' do post_graphql(query, current_user: current_user) @@ -108,55 +103,6 @@ RSpec.describe 'getting project information' do end end - describe 'performance' do - before_all do - project.add_developer(current_user) - mrs = create_list(:merge_request, 10, :closed, :with_head_pipeline, - source_project: project, - author: current_user) - mrs.each do |mr| - mr.assignees << create(:user) - mr.assignees << current_user - end - end - - def run_query(number) - q = <<~GQL - query { - project(fullPath: "#{project.full_path}") { - mergeRequests(first: #{number}) { - nodes { - assignees { nodes { username } } - headPipeline { status } - } - } - } - } - GQL - - post_graphql(q, current_user: current_user) - end - - it 'returns appropriate results' do - run_query(2) - - mrs = graphql_data.dig('project', 'mergeRequests', 'nodes') - - expect(mrs.size).to eq(2) - expect(mrs).to all( - match( - a_hash_including( - 'assignees' => { 'nodes' => all(match(a_hash_including('username' => be_present))) }, - 'headPipeline' => { 'status' => be_present } - ))) - end - - it 'can lookahead to eliminate N+1 queries' do - baseline = ActiveRecord::QueryRecorder.new { run_query(1) } - expect { run_query(10) }.not_to exceed_query_limit(baseline) - end - end - context 'when the user does not have access to the project' do it 'returns an empty field' do post_graphql(query, current_user: current_user) diff --git a/spec/requests/api/graphql/user/starred_projects_query_spec.rb b/spec/requests/api/graphql/user/starred_projects_query_spec.rb index b098058a735..6cb02068f2a 100644 --- a/spec/requests/api/graphql/user/starred_projects_query_spec.rb +++ b/spec/requests/api/graphql/user/starred_projects_query_spec.rb @@ -17,7 +17,13 @@ RSpec.describe 'Getting starredProjects of the user' do let_it_be(:user, reload: true) { create(:user) } let(:user_fields) { 'starredProjects { nodes { id } }' } - let(:starred_projects) { graphql_data_at(:user, :starred_projects, :nodes) } + let(:current_user) { nil } + + let(:starred_projects) do + post_graphql(query, current_user: current_user) + + graphql_data_at(:user, :starred_projects, :nodes) + end before do project_b.add_reporter(user) @@ -26,11 +32,13 @@ RSpec.describe 'Getting starredProjects of the user' do user.toggle_star(project_a) user.toggle_star(project_b) user.toggle_star(project_c) - - post_graphql(query) end - it_behaves_like 'a working graphql query' + it_behaves_like 'a working graphql query' do + before do + post_graphql(query) + end + end it 'found only public project' do expect(starred_projects).to contain_exactly( @@ -41,10 +49,6 @@ RSpec.describe 'Getting starredProjects of the user' do context 'the current user is the user' do let(:current_user) { user } - before do - post_graphql(query, current_user: current_user) - end - it 'found all projects' do expect(starred_projects).to contain_exactly( a_hash_including('id' => global_id_of(project_a)), @@ -56,11 +60,10 @@ RSpec.describe 'Getting starredProjects of the user' do context 'the current user is a member of a private project the user starred' do let_it_be(:other_user) { create(:user) } + let(:current_user) { other_user } before do project_b.add_reporter(other_user) - - post_graphql(query, current_user: other_user) end it 'finds public and member projects' do @@ -74,7 +77,6 @@ RSpec.describe 'Getting starredProjects of the user' do context 'the user has a private profile' do before do user.update!(private_profile: true) - post_graphql(query, current_user: current_user) end context 'the current user does not have access to view the private profile of the user' do diff --git a/spec/requests/api/graphql/users_spec.rb b/spec/requests/api/graphql/users_spec.rb index 72d86c10df1..22b68fbc9bb 100644 --- a/spec/requests/api/graphql/users_spec.rb +++ b/spec/requests/api/graphql/users_spec.rb @@ -54,6 +54,52 @@ RSpec.describe 'Users' do ) end end + + context 'when admins is true' do + let_it_be(:admin) { create(:user, :admin) } + let_it_be(:another_admin) { create(:user, :admin) } + + let(:query) { graphql_query_for(:users, { admins: true }, 'nodes { id }') } + + context 'current user is not an admin' do + let(:post_query) { post_graphql(query, current_user: current_user) } + + it_behaves_like 'a working users query' + + it 'includes all non-admin users', :aggregate_failures do + post_graphql(query) + + expect(graphql_data.dig('users', 'nodes')).to include( + { "id" => user1.to_global_id.to_s }, + { "id" => user2.to_global_id.to_s }, + { "id" => user3.to_global_id.to_s }, + { "id" => current_user.to_global_id.to_s }, + { "id" => admin.to_global_id.to_s }, + { "id" => another_admin.to_global_id.to_s } + ) + end + end + + context 'when current user is an admin' do + it_behaves_like 'a working users query' + + it 'includes only admins', :aggregate_failures do + post_graphql(query, current_user: admin) + + expect(graphql_data.dig('users', 'nodes')).to include( + { "id" => another_admin.to_global_id.to_s }, + { "id" => admin.to_global_id.to_s } + ) + + expect(graphql_data.dig('users', 'nodes')).not_to include( + { "id" => user1.to_global_id.to_s }, + { "id" => user2.to_global_id.to_s }, + { "id" => user3.to_global_id.to_s }, + { "id" => current_user.to_global_id.to_s } + ) + end + end + end end describe 'sorting and pagination' do |