diff options
Diffstat (limited to 'spec/requests/api')
67 files changed, 3880 insertions, 800 deletions
diff --git a/spec/requests/api/admin/ci/variables_spec.rb b/spec/requests/api/admin/ci/variables_spec.rb index bc2f0ba50a2..185fde17e1b 100644 --- a/spec/requests/api/admin/ci/variables_spec.rb +++ b/spec/requests/api/admin/ci/variables_spec.rb @@ -109,6 +109,22 @@ describe ::API::Admin::Ci::Variables do expect(response).to have_gitlab_http_status(:bad_request) end + + it 'does not allow values above 700 characters' do + too_long_message = <<~MESSAGE.strip + The encrypted value of the provided variable exceeds 1024 bytes. \ + Variables over 700 characters risk exceeding the limit. + MESSAGE + + expect do + post api('/admin/ci/variables', admin), + params: { key: 'too_long', value: SecureRandom.hex(701) } + end.not_to change { ::Ci::InstanceVariable.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to match('message' => + a_hash_including('encrypted_value' => [too_long_message])) + end end context 'authorized user with invalid permissions' do diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 86b3dd4095f..a423c92e2fb 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -356,28 +356,35 @@ describe API::Commits do } end + shared_examples_for "successfully creates the commit" do + it "creates the commit" do + expect(response).to have_gitlab_http_status(:created) + expect(json_response['title']).to eq(message) + expect(json_response['committer_name']).to eq(user.name) + expect(json_response['committer_email']).to eq(user.email) + end + end + it 'does not increment the usage counters using access token authentication' do expect(::Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_commits_count) post api(url, user), params: valid_c_params end - it 'a new file in project repo' do - post api(url, user), params: valid_c_params + context 'a new file in project repo' do + before do + post api(url, user), params: valid_c_params + end - expect(response).to have_gitlab_http_status(:created) - expect(json_response['title']).to eq(message) - expect(json_response['committer_name']).to eq(user.name) - expect(json_response['committer_email']).to eq(user.email) + it_behaves_like "successfully creates the commit" end - it 'a new file with utf8 chars in project repo' do - post api(url, user), params: valid_utf8_c_params + context 'a new file with utf8 chars in project repo' do + before do + post api(url, user), params: valid_utf8_c_params + end - expect(response).to have_gitlab_http_status(:created) - expect(json_response['title']).to eq(message) - expect(json_response['committer_name']).to eq(user.name) - expect(json_response['committer_email']).to eq(user.email) + it_behaves_like "successfully creates the commit" end it 'returns a 400 bad request if file exists' do diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index e8cc6bc71ae..1baa18b53ce 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -8,7 +8,7 @@ describe API::DeployKeys do let(:admin) { create(:admin) } let(:project) { create(:project, creator_id: user.id) } let(:project2) { create(:project, creator_id: user.id) } - let(:deploy_key) { create(:deploy_key, public: true) } + let(:deploy_key) { create(:deploy_key, public: true, user: user) } let!(:deploy_keys_project) do create(:deploy_keys_project, project: project, deploy_key: deploy_key) @@ -40,6 +40,32 @@ describe API::DeployKeys do expect(json_response).to be_an Array expect(json_response.first['id']).to eq(deploy_keys_project.deploy_key.id) end + + it 'returns all deploy keys with comments replaced with'\ + 'a simple identifier of username + hostname' do + get api('/deploy_keys', admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + + keys = json_response.map { |key_detail| key_detail['key'] } + expect(keys).to all(include("#{user.name} (#{Gitlab.config.gitlab.host}")) + end + + context 'N+1 queries' do + before do + get api('/deploy_keys', admin) + end + + it 'avoids N+1 queries', :request_store do + control_count = ActiveRecord::QueryRecorder.new { get api('/deploy_keys', admin) }.count + + create_list(:deploy_key, 2, public: true, user: create(:user)) + + expect { get api('/deploy_keys', admin) }.not_to exceed_query_limit(control_count) + end + end end end @@ -56,6 +82,25 @@ describe API::DeployKeys do expect(json_response).to be_an Array expect(json_response.first['title']).to eq(deploy_key.title) end + + context 'N+1 queries' do + before do + get api("/projects/#{project.id}/deploy_keys", admin) + end + + it 'avoids N+1 queries', :request_store do + control_count = ActiveRecord::QueryRecorder.new do + get api("/projects/#{project.id}/deploy_keys", admin) + end.count + + deploy_key = create(:deploy_key, user: create(:user)) + create(:deploy_keys_project, project: project, deploy_key: deploy_key) + + expect do + get api("/projects/#{project.id}/deploy_keys", admin) + end.not_to exceed_query_limit(control_count) + end + end end describe 'GET /projects/:id/deploy_keys/:key_id' do @@ -66,6 +111,13 @@ describe API::DeployKeys do expect(json_response['title']).to eq(deploy_key.title) end + it 'exposes key comment as a simple identifier of username + hostname' do + get api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['key']).to include("#{deploy_key.user_name} (#{Gitlab.config.gitlab.host})") + end + it 'returns 404 Not Found with invalid ID' do get api("/projects/#{project.id}/deploy_keys/404", admin) diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index decdcc66327..0425e0791eb 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -7,9 +7,9 @@ describe API::Events do let(:non_member) { create(:user) } let(:private_project) { create(:project, :private, creator_id: user.id, namespace: user.namespace) } let(:closed_issue) { create(:closed_issue, project: private_project, author: user) } - let!(:closed_issue_event) { create(:event, project: private_project, author: user, target: closed_issue, action: Event::CLOSED, created_at: Date.new(2016, 12, 30)) } + let!(:closed_issue_event) { create(:event, :closed, project: private_project, author: user, target: closed_issue, created_at: Date.new(2016, 12, 30)) } let(:closed_issue2) { create(:closed_issue, project: private_project, author: non_member) } - let!(:closed_issue_event2) { create(:event, project: private_project, author: non_member, target: closed_issue2, action: Event::CLOSED, created_at: Date.new(2016, 12, 30)) } + let!(:closed_issue_event2) { create(:event, :closed, project: private_project, author: non_member, target: closed_issue2, created_at: Date.new(2016, 12, 30)) } describe 'GET /events' do context 'when unauthenticated' do @@ -117,7 +117,7 @@ describe API::Events do context 'when the list of events includes wiki page events' do it 'returns information about the wiki event', :aggregate_failures do page = create(:wiki_page, project: private_project) - [Event::CREATED, Event::UPDATED, Event::DESTROYED].each do |action| + [:created, :updated, :destroyed].each do |action| create(:wiki_page_event, wiki_page: page, action: action, author: user) end diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb index 4ad5b4f9d49..59a9ed2f77d 100644 --- a/spec/requests/api/features_spec.rb +++ b/spec/requests/api/features_spec.rb @@ -2,11 +2,12 @@ require 'spec_helper' -describe API::Features do +describe API::Features, stub_feature_flags: false do let_it_be(:user) { create(:user) } let_it_be(:admin) { create(:admin) } before do + Feature.reset Flipper.unregister_groups Flipper.register(:perf_team) do |actor| actor.respond_to?(:admin) && actor.admin? @@ -38,9 +39,9 @@ describe API::Features do end before do - Feature.get('feature_1').enable - Feature.get('feature_2').disable - Feature.get('feature_3').enable Feature.group(:perf_team) + Feature.enable('feature_1') + Feature.disable('feature_2') + Feature.enable('feature_3', Feature.group(:perf_team)) end it 'returns a 401 for anonymous users' do @@ -226,10 +227,8 @@ describe API::Features do end context 'when the feature exists' do - let(:feature) { Feature.get(feature_name) } - before do - feature.disable # This also persists the feature on the DB + Feature.disable(feature_name) # This also persists the feature on the DB end context 'when passed value=true' do @@ -272,8 +271,8 @@ describe API::Features do context 'when feature is enabled and value=false is passed' do it 'disables the feature' do - feature.enable - expect(feature).to be_enabled + Feature.enable(feature_name) + expect(Feature.enabled?(feature_name)).to eq(true) post api("/features/#{feature_name}", admin), params: { value: 'false' } @@ -285,8 +284,8 @@ describe API::Features do end it 'disables the feature for the given Flipper group when passed feature_group=perf_team' do - feature.enable(Feature.group(:perf_team)) - expect(Feature.get(feature_name).enabled?(admin)).to be_truthy + Feature.enable(feature_name, Feature.group(:perf_team)) + expect(Feature.enabled?(feature_name, admin)).to be_truthy post api("/features/#{feature_name}", admin), params: { value: 'false', feature_group: 'perf_team' } @@ -298,8 +297,8 @@ describe API::Features do end it 'disables the feature for the given user when passed user=username' do - feature.enable(user) - expect(Feature.get(feature_name).enabled?(user)).to be_truthy + Feature.enable(feature_name, user) + expect(Feature.enabled?(feature_name, user)).to be_truthy post api("/features/#{feature_name}", admin), params: { value: 'false', user: user.username } @@ -313,7 +312,7 @@ describe API::Features do context 'with a pre-existing percentage of time value' do before do - feature.enable_percentage_of_time(50) + Feature.enable_percentage_of_time(feature_name, 50) end it 'updates the percentage of time if passed an integer' do @@ -332,7 +331,7 @@ describe API::Features do context 'with a pre-existing percentage of actors value' do before do - feature.enable_percentage_of_actors(42) + Feature.enable_percentage_of_actors(feature_name, 42) end it 'updates the percentage of actors if passed an integer' do @@ -377,14 +376,17 @@ describe API::Features do context 'when the gate value was set' do before do - Feature.get(feature_name).enable + Feature.enable(feature_name) end it 'deletes an enabled feature' do - delete api("/features/#{feature_name}", admin) + expect do + delete api("/features/#{feature_name}", admin) + Feature.reset + end.to change { Feature.persisted_name?(feature_name) } + .and change { Feature.enabled?(feature_name) } expect(response).to have_gitlab_http_status(:no_content) - expect(Feature.get(feature_name)).not_to be_enabled end end end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index e6406174391..198e4f64bcc 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -134,7 +134,8 @@ describe API::Files do context 'when PATs are used' do it_behaves_like 'repository files' do let(:token) { create(:personal_access_token, scopes: ['read_repository'], user: user) } - let(:current_user) { { personal_access_token: token } } + let(:current_user) { user } + let(:api_user) { { personal_access_token: token } } end end @@ -153,15 +154,17 @@ describe API::Files do describe "GET /projects/:id/repository/files/:file_path" do shared_examples_for 'repository files' do + let(:api_user) { current_user } + it 'returns 400 for invalid file path' do - get api(route(rouge_file_path), current_user), params: params + get api(route(rouge_file_path), api_user), params: params expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['error']).to eq(invalid_file_message) end it 'returns file attributes as json' do - get api(route(file_path), current_user), params: params + get api(route(file_path), api_user), params: params expect(response).to have_gitlab_http_status(:ok) expect(json_response['file_path']).to eq(CGI.unescape(file_path)) @@ -174,7 +177,7 @@ describe API::Files do it 'returns json when file has txt extension' do file_path = "bar%2Fbranch-test.txt" - get api(route(file_path), current_user), params: params + get api(route(file_path), api_user), params: params expect(response).to have_gitlab_http_status(:ok) expect(response.content_type).to eq('application/json') @@ -185,7 +188,7 @@ describe API::Files do file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee" params[:ref] = "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9" - get api(route(file_path), current_user), params: params + get api(route(file_path), api_user), params: params expect(response).to have_gitlab_http_status(:ok) expect(json_response['file_name']).to eq('commit.js.coffee') @@ -197,7 +200,7 @@ describe API::Files do url = route(file_path) + "/raw" expect(Gitlab::Workhorse).to receive(:send_git_blob) - get api(url, current_user), params: params + get api(url, api_user), params: params expect(response).to have_gitlab_http_status(:ok) expect(headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" @@ -206,7 +209,7 @@ describe API::Files do it 'returns blame file info' do url = route(file_path) + '/blame' - get api(url, current_user), params: params + get api(url, api_user), params: params expect(response).to have_gitlab_http_status(:ok) end @@ -214,7 +217,7 @@ describe API::Files do it 'sets inline content disposition by default' do url = route(file_path) + "/raw" - get api(url, current_user), params: params + get api(url, api_user), params: params expect(headers['Content-Disposition']).to eq(%q(inline; filename="popen.rb"; filename*=UTF-8''popen.rb)) end @@ -229,7 +232,7 @@ describe API::Files do let(:params) { { ref: 'master' } } it_behaves_like '404 response' do - let(:request) { get api(route('app%2Fmodels%2Fapplication%2Erb'), current_user), params: params } + let(:request) { get api(route('app%2Fmodels%2Fapplication%2Erb'), api_user), params: params } let(:message) { '404 File Not Found' } end end @@ -238,7 +241,7 @@ describe API::Files do include_context 'disabled repository' it_behaves_like '403 response' do - let(:request) { get api(route(file_path), current_user), params: params } + let(:request) { get api(route(file_path), api_user), params: params } end end end @@ -253,7 +256,8 @@ describe API::Files do context 'when PATs are used' do it_behaves_like 'repository files' do let(:token) { create(:personal_access_token, scopes: ['read_repository'], user: user) } - let(:current_user) { { personal_access_token: token } } + let(:current_user) { user } + let(:api_user) { { personal_access_token: token } } 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 index f0927487f85..3cc1468be02 100644 --- a/spec/requests/api/graphql/boards/board_lists_query_spec.rb +++ b/spec/requests/api/graphql/boards/board_lists_query_spec.rb @@ -65,41 +65,41 @@ describe 'get board lists' do end describe 'sorting and pagination' do + let_it_be(:current_user) { user } + let(:data_path) { [board_parent_type, :boards, :edges, 0, :node, :lists] } + + def pagination_query(params, page_info) + graphql_query_for( + board_parent_type, + { 'fullPath' => board_parent.full_path }, + <<~BOARDS + boards(first: 1) { + edges { + node { + #{query_graphql_field('lists', params, "#{page_info} edges { node { id } }")} + } + } + } + BOARDS + ) + end + + def pagination_results_data(data) + data.map { |list| list.dig('node', 'id') } + end + 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' + let(:lists) { [backlog_list, label_list2, label_list, closed_list] } 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 + it_behaves_like 'sorted paginated query' do + let(:sort_param) { } + let(:first_param) { 2 } + let(:expected_results) { lists.map { |list| list.to_global_id.to_s } } end end end @@ -126,12 +126,4 @@ describe 'get board lists' do 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/group/labels_query_spec.rb b/spec/requests/api/graphql/group/labels_query_spec.rb new file mode 100644 index 00000000000..6c34cbadf95 --- /dev/null +++ b/spec/requests/api/graphql/group/labels_query_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'getting group label information' do + include GraphqlHelpers + + let_it_be(:group) { create(:group, :public) } + let_it_be(:label_factory) { :group_label } + let_it_be(:label_attrs) { { group: group } } + + it_behaves_like 'querying a GraphQL type with labels' do + let(:path_prefix) { ['group'] } + + def make_query(fields) + graphql_query_for('group', { full_path: group.full_path }, fields) + end + end +end diff --git a/spec/requests/api/graphql/metrics/dashboard_query_spec.rb b/spec/requests/api/graphql/metrics/dashboard_query_spec.rb index 8b0965a815b..d9d9ea9ad61 100644 --- a/spec/requests/api/graphql/metrics/dashboard_query_spec.rb +++ b/spec/requests/api/graphql/metrics/dashboard_query_spec.rb @@ -9,25 +9,19 @@ describe 'Getting Metrics Dashboard' do let(:project) { create(:project) } let!(:environment) { create(:environment, project: project) } - let(:fields) do - <<~QUERY - #{all_graphql_fields_for('MetricsDashboard'.classify)} - QUERY - end - let(:query) do - %( - query { - project(fullPath:"#{project.full_path}") { - environments(name: "#{environment.name}") { - nodes { - metricsDashboard(path: "#{path}"){ - #{fields} - } - } - } - } - } + graphql_query_for( + 'project', { 'fullPath' => project.full_path }, + query_graphql_field( + :environments, { 'name' => environment.name }, + query_graphql_field( + :nodes, nil, + query_graphql_field( + :metricsDashboard, { 'path' => path }, + all_graphql_fields_for('MetricsDashboard'.classify) + ) + ) + ) ) end @@ -63,7 +57,29 @@ describe 'Getting Metrics Dashboard' do it 'returns metrics dashboard' do dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard'] - expect(dashboard).to eql("path" => path) + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => nil) + end + + context 'invalid dashboard' do + let(:path) { '.gitlab/dashboards/metrics.yml' } + let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "---\ndasboard: ''" }) } + + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["dashboard: can't be blank", "panel_groups: can't be blank"]) + end + end + + context 'empty dashboard' do + let(:path) { '.gitlab/dashboards/metrics.yml' } + let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "" }) } + + it 'returns metrics dashboard' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + + expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["dashboard: can't be blank", "panel_groups: can't be blank"]) + end end end @@ -72,7 +88,7 @@ describe 'Getting Metrics Dashboard' do it_behaves_like 'a working graphql query' - it 'return snil' do + it 'returns nil' do dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard'] expect(dashboard).to be_nil diff --git a/spec/requests/api/graphql/mutations/alert_management/alerts/create_alert_issue_spec.rb b/spec/requests/api/graphql/mutations/alert_management/alerts/create_alert_issue_spec.rb new file mode 100644 index 00000000000..5b5b2ec8788 --- /dev/null +++ b/spec/requests/api/graphql/mutations/alert_management/alerts/create_alert_issue_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Create an alert issue from an alert' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:alert) { create(:alert_management_alert, project: project) } + + let(:mutation) do + variables = { + project_path: project.full_path, + iid: alert.iid.to_s + } + graphql_mutation(:create_alert_issue, variables, + <<~QL + clientMutationId + errors + alert { + iid + issueIid + } + issue { + iid + title + } + QL + ) + end + + let(:mutation_response) { graphql_mutation_response(:create_alert_issue) } + + before do + project.add_developer(user) + end + + context 'when there is no issue associated with the alert' do + it 'creates an alert issue' do + post_graphql_mutation(mutation, current_user: user) + + new_issue = Issue.last! + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response.slice('alert', 'issue')).to eq( + 'alert' => { + 'iid' => alert.iid.to_s, + 'issueIid' => new_issue.iid.to_s + }, + 'issue' => { + 'iid' => new_issue.iid.to_s, + 'title' => new_issue.title + } + ) + end + end + + context 'when there is an issue already associated with the alert' do + before do + AlertManagement::CreateAlertIssueService.new(alert, user).execute + end + + it 'responds with an error' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response.slice('errors', 'issue')).to eq( + 'errors' => ['An issue already exists'], + 'issue' => nil + ) + end + end +end diff --git a/spec/requests/api/graphql/mutations/alert_management/alerts/set_assignees_spec.rb b/spec/requests/api/graphql/mutations/alert_management/alerts/set_assignees_spec.rb new file mode 100644 index 00000000000..6663281e093 --- /dev/null +++ b/spec/requests/api/graphql/mutations/alert_management/alerts/set_assignees_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Setting assignees of an alert' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { create(:user) } + let_it_be(:alert) { create(:alert_management_alert, project: project) } + let(:input) { { assignee_usernames: [current_user.username] } } + + let(:mutation) do + graphql_mutation( + :alert_set_assignees, + { project_path: project.full_path, iid: alert.iid.to_s }.merge(input), + <<~QL + clientMutationId + errors + alert { + assignees { + nodes { + username + } + } + } + QL + ) + end + + let(:mutation_response) { graphql_mutation_response(:alert_set_assignees) } + + before_all do + project.add_developer(current_user) + end + + it 'updates the assignee of the alert' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['alert']['assignees']['nodes'].first['username']).to eq(current_user.username) + expect(alert.reload.assignees).to contain_exactly(current_user) + end + + context 'with operation_mode specified' do + let(:input) do + { + assignee_usernames: [current_user.username], + operation_mode: Types::MutationOperationModeEnum.enum[:remove] + } + end + + before do + alert.assignees = [current_user] + end + + it 'updates the assignee of the alert' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['alert']['assignees']['nodes']).to be_empty + expect(alert.reload.assignees).to be_empty + 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 index fe50468134c..2a470bda689 100644 --- 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 @@ -15,16 +15,16 @@ describe 'Setting the status of an alert' do 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 - ) + graphql_mutation(:update_alert_status, variables.merge(input)) do + <<~QL + clientMutationId + errors + alert { + iid + status + } + QL + end end let(:mutation_response) { graphql_mutation_response(:update_alert_status) } diff --git a/spec/requests/api/graphql/mutations/commits/create_spec.rb b/spec/requests/api/graphql/mutations/commits/create_spec.rb new file mode 100644 index 00000000000..10a69932948 --- /dev/null +++ b/spec/requests/api/graphql/mutations/commits/create_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Creation of a new commit' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project, :public, :repository) } + let(:input) { { project_path: project.full_path, branch: branch, message: message, actions: actions } } + let(:branch) { 'master' } + let(:message) { 'Commit message' } + let(:actions) do + [ + { + action: 'CREATE', + filePath: 'NEW_FILE.md', + content: 'Hello' + } + ] + end + + let(:mutation) { graphql_mutation(:commit_create, input) } + let(:mutation_response) { graphql_mutation_response(:commit_create) } + + context 'the user is not allowed to create a commit' 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 commit' do + before do + project.add_developer(current_user) + end + + it 'creates a new commit' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['commit']).to include( + 'title' => message + ) + end + + context 'when branch is not correct' do + let(:branch) { 'unknown' } + + it_behaves_like 'a mutation that returns errors in the response', + errors: ['You can only create or edit files when you are on a branch'] + end + end +end diff --git a/spec/requests/api/graphql/mutations/container_expiration_policy/update_spec.rb b/spec/requests/api/graphql/mutations/container_expiration_policy/update_spec.rb new file mode 100644 index 00000000000..bc256a08f00 --- /dev/null +++ b/spec/requests/api/graphql/mutations/container_expiration_policy/update_spec.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Updating the container expiration policy' do + include GraphqlHelpers + using RSpec::Parameterized::TableSyntax + + let_it_be(:project, reload: true) { create(:project) } + let_it_be(:user) { create(:user) } + + let(:container_expiration_policy) { project.container_expiration_policy.reload } + let(:params) do + { + project_path: project.full_path, + cadence: 'EVERY_THREE_MONTHS', + keep_n: 'ONE_HUNDRED_TAGS', + older_than: 'FOURTEEN_DAYS' + } + end + let(:mutation) do + graphql_mutation(:update_container_expiration_policy, params, + <<~QL + containerExpirationPolicy { + cadence + keepN + nameRegexKeep + nameRegex + olderThan + } + errors + QL + ) + end + let(:mutation_response) { graphql_mutation_response(:update_container_expiration_policy) } + let(:container_expiration_policy_response) { mutation_response['containerExpirationPolicy'] } + + RSpec.shared_examples 'returning a success' do + it_behaves_like 'returning response status', :success + + it 'returns the updated container expiration policy' do + subject + + expect(mutation_response['errors']).to be_empty + expect(container_expiration_policy_response['cadence']).to eq(params[:cadence]) + expect(container_expiration_policy_response['keepN']).to eq(params[:keep_n]) + expect(container_expiration_policy_response['olderThan']).to eq(params[:older_than]) + end + end + + RSpec.shared_examples 'updating the container expiration policy' do + it_behaves_like 'updating the container expiration policy attributes', mode: :update, from: { cadence: '1d', keep_n: 10, older_than: '90d' }, to: { cadence: '3month', keep_n: 100, older_than: '14d' } + + it_behaves_like 'returning a success' + end + + RSpec.shared_examples 'denying access to container expiration policy' do + it_behaves_like 'not creating the container expiration policy' + + 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 container expiration policy' do + where(:user_role, :shared_examples_name) do + :maintainer | 'updating the container expiration policy' + :developer | 'updating the container expiration policy' + :reporter | 'denying access to container expiration policy' + :guest | 'denying access to container expiration policy' + :anonymous | 'denying access to container expiration policy' + end + + with_them do + before do + project.send("add_#{user_role}", user) unless user_role == :anonymous + end + + it_behaves_like params[:shared_examples_name] + end + end + + context 'without existing container expiration policy' do + let_it_be(:project, reload: true) { create(:project, :without_container_expiration_policy) } + + where(:user_role, :shared_examples_name) do + :maintainer | 'creating the container expiration policy' + :developer | 'creating the container expiration policy' + :reporter | 'denying access to container expiration policy' + :guest | 'denying access to container expiration policy' + :anonymous | 'denying access to container expiration policy' + end + + with_them do + before do + project.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/discussions/toggle_resolve_spec.rb b/spec/requests/api/graphql/mutations/discussions/toggle_resolve_spec.rb new file mode 100644 index 00000000000..95e967c039d --- /dev/null +++ b/spec/requests/api/graphql/mutations/discussions/toggle_resolve_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Toggling the resolve status of a discussion' do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:noteable) { create(:merge_request, source_project: project) } + let(:discussion) do + create(:diff_note_on_merge_request, noteable: noteable, project: project).to_discussion + end + let(:mutation) do + graphql_mutation(:discussion_toggle_resolve, { id: discussion.to_global_id.to_s, resolve: true }) + end + let(:mutation_response) { graphql_mutation_response(:discussion_toggle_resolve) } + + context 'when the user does not have permission' do + let_it_be(:current_user) { create(:user) } + + 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 permission' do + let_it_be(:current_user) { create(:user, developer_projects: [project]) } + + it 'returns the discussion without errors', :aggregate_failures do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response).to include( + 'discussion' => be_present, + 'errors' => be_empty + ) + end + + context 'when an error is encountered' do + before do + allow_next_instance_of(::Discussions::ResolveService) do |service| + allow(service).to receive(:execute).and_raise(ActiveRecord::RecordNotSaved) + end + end + + it_behaves_like 'a mutation that returns errors in the response', + errors: ['Discussion failed to be resolved'] + end + end +end diff --git a/spec/requests/api/graphql/mutations/jira_import/import_users_spec.rb b/spec/requests/api/graphql/mutations/jira_import/import_users_spec.rb new file mode 100644 index 00000000000..be0d843d5ff --- /dev/null +++ b/spec/requests/api/graphql/mutations/jira_import/import_users_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Importing Jira Users' do + include JiraServiceHelper + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let(:project_path) { project.full_path } + let(:start_at) { 7 } + + let(:mutation) do + variables = { + start_at: start_at, + project_path: project_path + } + + graphql_mutation(:jira_import_users, variables) + end + + def mutation_response + graphql_mutation_response(:jira_import_users) + end + + def jira_import + mutation_response['jiraUsers'] + end + + context 'with anonymous user' do + let(:current_user) { nil } + + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] + end + + context 'with user without permissions' do + let(:current_user) { user } + + before do + project.add_developer(current_user) + end + + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] + end + + context 'when the user has permissions' do + let(:current_user) { user } + + before do + project.add_maintainer(current_user) + end + + context 'when the project path is invalid' do + let(:project_path) { 'foobar' } + + it 'returns an an error' do + post_graphql_mutation(mutation, current_user: current_user) + + errors = json_response['errors'] + + expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) + end + end + + context 'when all params and permissions are ok' do + let(:importer) { instance_double(JiraImport::UsersImporter) } + + before do + expect(JiraImport::UsersImporter).to receive(:new).with(current_user, project, 7) + .and_return(importer) + end + + context 'when service returns a successful response' do + it 'returns imported users' do + users = [{ jira_account_id: '12a', jira_display_name: 'user 1' }] + result = ServiceResponse.success(payload: users) + + expect(importer).to receive(:execute).and_return(result) + + post_graphql_mutation(mutation, current_user: current_user) + + expect(jira_import.length).to eq(1) + expect(jira_import.first['jiraAccountId']).to eq('12a') + expect(jira_import.first['jiraDisplayName']).to eq('user 1') + end + end + + context 'when service returns an error response' do + it 'returns an error messaege' do + result = ServiceResponse.error(message: 'Some error') + + expect(importer).to receive(:execute).and_return(result) + + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['errors']).to eq(['Some error']) + end + end + 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 84110098400..296d33aec5d 100644 --- a/spec/requests/api/graphql/mutations/jira_import/start_spec.rb +++ b/spec/requests/api/graphql/mutations/jira_import/start_spec.rb @@ -29,10 +29,6 @@ describe 'Starting a Jira Import' do end context 'when the user does not have permission' do - before do - stub_feature_flags(jira_issue_import: true) - end - shared_examples 'Jira import does not start' do it 'does not start the Jira import' do post_graphql_mutation(mutation, current_user: current_user) @@ -83,53 +79,39 @@ describe 'Starting a Jira Import' do end end - context 'when feature jira_issue_import feature flag is disabled' do - before do - stub_feature_flags(jira_issue_import: false) - end - - it_behaves_like 'a mutation that returns errors in the response', errors: ['Jira import feature is disabled.'] + context 'when project has no Jira service' do + it_behaves_like 'a mutation that returns errors in the response', errors: ['Jira integration not configured.'] end - context 'when feature jira_issue_import feature flag is enabled' do + context 'when when project has Jira service' do + let!(:service) { create(:jira_service, project: project) } + before do - stub_feature_flags(jira_issue_import: true) - end + project.reload - context 'when project has no Jira service' do - it_behaves_like 'a mutation that returns errors in the response', errors: ['Jira integration not configured.'] + stub_jira_service_test end - context 'when when project has Jira service' do - let!(:service) { create(:jira_service, project: project) } + context 'when issues feature are disabled' do + let_it_be(:project, reload: true) { create(:project, :issues_disabled) } - before do - project.reload - - stub_jira_service_test - end - - context 'when issues feature are disabled' do - let_it_be(:project, reload: true) { create(:project, :issues_disabled) } - - it_behaves_like 'a mutation that returns errors in the response', errors: ['Cannot import because issues are not available in this project.'] - end + it_behaves_like 'a mutation that returns errors in the response', errors: ['Cannot import because issues are not available in this project.'] + end - context 'when jira_project_key not provided' do - let(:jira_project_key) { '' } + context 'when jira_project_key not provided' do + let(:jira_project_key) { '' } - it_behaves_like 'a mutation that returns errors in the response', errors: ['Unable to find Jira project to import data from.'] - end + 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 - it 'schedules a Jira import' do - post_graphql_mutation(mutation, current_user: current_user) + context 'when Jira import successfully scheduled' do + it 'schedules a Jira import' do + post_graphql_mutation(mutation, current_user: current_user) - expect(jira_import['jiraProjectKey']).to eq 'AA' - expect(jira_import['scheduledBy']['username']).to eq current_user.username - expect(project.latest_jira_import).not_to be_nil - expect(project.latest_jira_import).to be_scheduled - end + expect(jira_import['jiraProjectKey']).to eq 'AA' + expect(jira_import['scheduledBy']['username']).to eq current_user.username + expect(project.latest_jira_import).not_to be_nil + expect(project.latest_jira_import).to be_scheduled end end end diff --git a/spec/requests/api/graphql/mutations/merge_requests/create_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/create_spec.rb new file mode 100644 index 00000000000..5c63f655f1d --- /dev/null +++ b/spec/requests/api/graphql/mutations/merge_requests/create_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Creation of a new merge request' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + let(:input) do + { + project_path: project.full_path, + title: title, + source_branch: source_branch, + target_branch: target_branch + } + end + let(:title) { 'MergeRequest' } + let(:source_branch) { 'new_branch' } + let(:target_branch) { 'master' } + + let(:mutation) { graphql_mutation(:merge_request_create, input) } + let(:mutation_response) { graphql_mutation_response(:merge_request_create) } + + 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 merge request' do + before do + project.add_developer(current_user) + end + + it 'creates a new merge request' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['mergeRequest']).to include( + 'title' => title + ) + end + + context 'when source branch is equal to the target branch' do + let(:source_branch) { target_branch } + + it_behaves_like 'a mutation that returns errors in the response', + errors: ['Branch conflict You can\'t use same project/branch for source and target'] + end + end +end diff --git a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb new file mode 100644 index 00000000000..217f538c53e --- /dev/null +++ b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Mutations::Metrics::Dashboard::Annotations::Delete 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(:annotation) { create(:metrics_dashboard_annotation, environment: environment) } + let(:mutation) do + variables = { + id: GitlabSchema.id_from_object(annotation).to_s + } + + graphql_mutation(:delete_annotation, variables) + end + + def mutation_response + graphql_mutation_response(:delete_annotation) + end + + specify { expect(described_class).to require_graphql_authorizations(:delete_metrics_dashboard_annotation) } + + context 'when the user has permission to delete the annotation' do + before do + project.add_developer(current_user) + end + + context 'with valid params' do + it 'deletes the annotation' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { Metrics::Dashboard::Annotation.count }.by(-1) + end + end + + context 'with invalid params' do + let(:mutation) do + variables = { + id: 'invalid_id' + } + + graphql_mutation(:delete_annotation, variables) + end + + it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab id.'] + end + + context 'when the delete fails' do + let(:service_response) { { message: 'Annotation has not been deleted', status: :error, last_step: :delete } } + + before do + allow_next_instance_of(Metrics::Dashboard::Annotations::DeleteService) do |delete_service| + allow(delete_service).to receive(:execute).and_return(service_response) + end + end + it 'returns the error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['errors']).to eq([service_response[:message]]) + end + end + end + + context 'when the user does not have permission to delete the annotation' 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 delete the annotation' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { Metrics::Dashboard::Annotation.count } + end + 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 e1e5fe22887..9052f54b171 100644 --- a/spec/requests/api/graphql/mutations/snippets/create_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/create_spec.rb @@ -109,31 +109,21 @@ describe 'Creating a Snippet' do context 'when the project path is invalid' do let(:project_path) { 'foobar' } - it 'returns an an error' do - subject - errors = json_response['errors'] - - expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) - end + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] end context 'when the feature is disabled' do - it 'returns an an error' do + before do project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::DISABLED) - - subject - errors = json_response['errors'] - - expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) end + + it_behaves_like 'a mutation that returns top-level errors', + errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] end end - context 'when there are ActiveRecord validation errors' do - let(:title) { '' } - - it_behaves_like 'a mutation that returns errors in the response', errors: ["Title can't be blank"] - + shared_examples 'does not create snippet' do it 'does not create the Snippet' do expect do subject @@ -147,7 +137,21 @@ describe 'Creating a Snippet' do end end - context 'when there uploaded files' do + context 'when there are ActiveRecord validation errors' do + let(:title) { '' } + + it_behaves_like 'a mutation that returns errors in the response', errors: ["Title can't be blank"] + it_behaves_like 'does not create snippet' + end + + context 'when there non ActiveRecord errors' do + let(:file_name) { 'invalid://file/path' } + + it_behaves_like 'a mutation that returns errors in the response', errors: ['Repository Error creating the snippet - Invalid file name'] + it_behaves_like 'does not create snippet' + end + + context 'when there are uploaded files' do shared_examples 'expected files argument' do |file_value, expected_value| let(:uploaded_files) { file_value } 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 new file mode 100644 index 00000000000..4c048caaeee --- /dev/null +++ b/spec/requests/api/graphql/project/alert_management/alert/assignees_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'getting Alert Management Alert Assignees' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { create(:user) } + let_it_be(:first_alert) { create(:alert_management_alert, project: project, assignees: [current_user]) } + let_it_be(:second_alert) { create(:alert_management_alert, project: project) } + + let(:params) { {} } + + let(:fields) do + <<~QUERY + nodes { + iid + assignees { + nodes { + username + } + } + } + QUERY + end + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('alertManagementAlerts', params, fields) + ) + end + + let(:alerts) { graphql_data.dig('project', 'alertManagementAlerts', 'nodes') } + let(:assignees) { alerts.map { |alert| [alert['iid'], alert['assignees']['nodes']] }.to_h } + let(:first_assignees) { assignees[first_alert.iid.to_s] } + let(:second_assignees) { assignees[second_alert.iid.to_s] } + + before do + project.add_developer(current_user) + end + + it 'returns the correct assignees' do + post_graphql(query, current_user: current_user) + + expect(first_assignees.length).to eq(1) + expect(first_assignees.first).to include('username' => current_user.username) + expect(second_assignees).to be_empty + end + + it 'applies appropriate filters for non-visible users' do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(current_user, :read_user, current_user).and_return(false) + + post_graphql(query, current_user: current_user) + + expect(first_assignees).to be_empty + 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) + 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]) + + expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(base_count) + + third_assignees = assignees[third_alert.iid.to_s] + + expect(third_assignees.length).to eq(1) + expect(third_assignees.first).to include('username' => current_user.username) + 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 new file mode 100644 index 00000000000..df6bfa8c97b --- /dev/null +++ b/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'getting Alert Management Alert Notes' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { create(:user) } + let_it_be(:first_alert) { create(:alert_management_alert, project: project, assignees: [current_user]) } + let_it_be(:second_alert) { create(:alert_management_alert, project: project) } + let_it_be(:first_system_note) { create(:note_on_alert, noteable: first_alert, project: project) } + let_it_be(:second_system_note) { create(:note_on_alert, noteable: first_alert, project: project) } + + let(:params) { {} } + + let(:fields) do + <<~QUERY + nodes { + iid + notes { + nodes { + id + } + } + } + QUERY + end + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('alertManagementAlerts', params, fields) + ) + end + + let(:alerts_result) { graphql_data.dig('project', 'alertManagementAlerts', 'nodes') } + let(:notes_result) { alerts_result.map { |alert| [alert['iid'], alert['notes']['nodes']] }.to_h } + let(:first_notes_result) { notes_result[first_alert.iid.to_s] } + let(:second_notes_result) { notes_result[second_alert.iid.to_s] } + + before do + project.add_developer(current_user) + end + + it 'returns the notes ordered by createdAt' do + post_graphql(query, current_user: current_user) + + expect(first_notes_result.length).to eq(2) + expect(first_notes_result.first).to include('id' => first_system_note.to_global_id.to_s) + expect(first_notes_result.second).to include('id' => second_system_note.to_global_id.to_s) + 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) + end + + # An N+1 would mean a new alert would increase the query count + create(:alert_management_alert, project: project) + + expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(base_count) + expect(alerts_result.length).to eq(3) + 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 index ffd328429ef..a0d1ff7efc5 100644 --- 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 @@ -56,6 +56,22 @@ describe 'getting Alert Management Alert counts by status' do 'ignored' => 0 ) end + + context 'with search criteria' do + let(:params) { { search: alert_1.title } } + + it_behaves_like 'a working graphql query' + it 'returns the correct counts for each status' do + expect(alert_counts).to eq( + 'open' => 0, + 'all' => 1, + 'triggered' => 0, + 'acknowledged' => 0, + 'resolved' => 1, + 'ignored' => 0 + ) + end + 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 index c226e659364..c591895f295 100644 --- a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb @@ -10,12 +10,13 @@ describe 'getting Alert Management Alerts' do 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)} + #{all_graphql_fields_for('AlertManagementAlert', excluded: ['assignees'])} } QUERY end diff --git a/spec/requests/api/graphql/project/container_expiration_policy_spec.rb b/spec/requests/api/graphql/project/container_expiration_policy_spec.rb new file mode 100644 index 00000000000..d0563f9ff05 --- /dev/null +++ b/spec/requests/api/graphql/project/container_expiration_policy_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe 'getting a repository in a project' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { project.owner } + let_it_be(:container_expiration_policy) { project.container_expiration_policy } + + let(:fields) do + <<~QUERY + #{all_graphql_fields_for('container_expiration_policy'.classify)} + QUERY + end + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('containerExpirationPolicy', {}, fields) + ) + end + + before do + stub_config(registry: { enabled: true }) + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query' +end diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb index 91fce3eed92..3128f527356 100644 --- a/spec/requests/api/graphql/project/issues_spec.rb +++ b/spec/requests/api/graphql/project/issues_spec.rb @@ -124,7 +124,7 @@ describe 'getting an issue list for a project' do graphql_query_for( 'project', { 'fullPath' => sort_project.full_path }, - "issues(#{params}) { #{page_info} edges { node { iid dueDate } } }" + query_graphql_field('issues', params, "#{page_info} edges { node { iid dueDate} }") ) end diff --git a/spec/requests/api/graphql/project/jira_import_spec.rb b/spec/requests/api/graphql/project/jira_import_spec.rb index e063068eb1a..7be14696963 100644 --- a/spec/requests/api/graphql/project/jira_import_spec.rb +++ b/spec/requests/api/graphql/project/jira_import_spec.rb @@ -7,9 +7,30 @@ describe 'query Jira import data' do let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project, :private, :import_started, import_type: 'jira') } - let_it_be(:jira_import1) { create(:jira_import_state, :finished, project: project, jira_project_key: 'AA', user: current_user, created_at: 2.days.ago) } - let_it_be(:jira_import2) { create(:jira_import_state, :finished, project: project, jira_project_key: 'BB', user: current_user, created_at: 5.days.ago) } - + let_it_be(:jira_import1) do + create( + :jira_import_state, :finished, + project: project, + jira_project_key: 'AA', + user: current_user, + created_at: 2.days.ago, + failed_to_import_count: 2, + imported_issues_count: 2, + total_issue_count: 4 + ) + end + let_it_be(:jira_import2) do + create( + :jira_import_state, :finished, + project: project, + jira_project_key: 'BB', + user: current_user, + created_at: 5.days.ago, + failed_to_import_count: 1, + imported_issues_count: 2, + total_issue_count: 3 + ) + end let(:query) do %( query { @@ -23,6 +44,9 @@ describe 'query Jira import data' do scheduledBy { username } + importedIssuesCount + failedToImportCount + totalIssueCount } } } @@ -64,10 +88,16 @@ describe 'query Jira import data' do it 'retuns list of jira imports' do jira_proket_keys = jira_imports.map {|ji| ji['jiraProjectKey']} usernames = jira_imports.map {|ji| ji.dig('scheduledBy', 'username')} + imported_issues_count = jira_imports.map {|ji| ji.dig('importedIssuesCount')} + failed_issues_count = jira_imports.map {|ji| ji.dig('failedToImportCount')} + total_issue_count = jira_imports.map {|ji| ji.dig('totalIssueCount')} expect(jira_imports.size).to eq 2 expect(jira_proket_keys).to eq %w(BB AA) expect(usernames).to eq [current_user.username, current_user.username] + expect(imported_issues_count).to eq [2, 2] + expect(failed_issues_count).to eq [1, 2] + expect(total_issue_count).to eq [3, 4] end end diff --git a/spec/requests/api/graphql/project/jira_projects_spec.rb b/spec/requests/api/graphql/project/jira_projects_spec.rb new file mode 100644 index 00000000000..d67c89f18c9 --- /dev/null +++ b/spec/requests/api/graphql/project/jira_projects_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'query Jira projects' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project) } + + include_context 'jira projects request context' + + let(:services) { graphql_data_at(:project, :services, :edges) } + let(:jira_projects) { services.first.dig('node', 'projects', 'nodes') } + let(:projects_query) { 'projects' } + let(:query) do + %( + query { + project(fullPath: "#{project.full_path}") { + services(active: true, type: JIRA_SERVICE) { + edges { + node { + ... on JiraService { + %{projects_query} { + nodes { + key + name + projectId + } + } + } + } + } + } + } + } + ) % { projects_query: projects_query } + end + + context 'when user does not have access' do + it_behaves_like 'unauthorized users cannot read services' + end + + context 'when user can access project services' do + before do + project.add_maintainer(current_user) + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query' + + it 'retuns list of jira projects' do + project_keys = jira_projects.map { |jp| jp['key'] } + project_names = jira_projects.map { |jp| jp['name'] } + project_ids = jira_projects.map { |jp| jp['projectId'] } + + expect(jira_projects.size).to eq(2) + expect(project_keys).to eq(%w(EX ABC)) + expect(project_names).to eq(%w(Example Alphabetical)) + expect(project_ids).to eq([10000, 10001]) + end + + context 'with pagination' do + context 'when fetching limited number of projects' do + shared_examples_for 'fetches first project' do + it 'retuns first project from list of fetched projects' do + project_keys = jira_projects.map { |jp| jp['key'] } + project_names = jira_projects.map { |jp| jp['name'] } + project_ids = jira_projects.map { |jp| jp['projectId'] } + + expect(jira_projects.size).to eq(1) + expect(project_keys).to eq(%w(EX)) + expect(project_names).to eq(%w(Example)) + expect(project_ids).to eq([10000]) + end + end + + context 'without cursor' do + let(:projects_query) { 'projects(first: 1)' } + + it_behaves_like 'fetches first project' + end + + context 'with before cursor' do + let(:projects_query) { 'projects(before: "Mg==", first: 1)' } + + it_behaves_like 'fetches first project' + end + + context 'with after cursor' do + let(:projects_query) { 'projects(after: "MA==", first: 1)' } + + it_behaves_like 'fetches first project' + end + end + + context 'with valid but inexistent after cursor' do + let(:projects_query) { 'projects(after: "MTk==")' } + + it 'retuns empty list of jira projects' do + expect(jira_projects.size).to eq(0) + end + end + + context 'with invalid after cursor' do + let(:projects_query) { 'projects(after: "invalid==")' } + + it 'treats the invalid cursor as no cursor and returns list of jira projects' do + expect(jira_projects.size).to eq(2) + end + end + end + end +end diff --git a/spec/requests/api/graphql/project/labels_query_spec.rb b/spec/requests/api/graphql/project/labels_query_spec.rb new file mode 100644 index 00000000000..ecc43e0a3db --- /dev/null +++ b/spec/requests/api/graphql/project/labels_query_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'getting project label information' do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :public) } + let_it_be(:label_factory) { :label } + let_it_be(:label_attrs) { { project: project } } + + it_behaves_like 'querying a GraphQL type with labels' do + let(:path_prefix) { ['project'] } + + def make_query(fields) + graphql_query_for('project', { full_path: project.full_path }, fields) + end + end +end diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index 8d8c31c335d..643532bf2e2 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -37,6 +37,30 @@ describe 'getting merge request information nested in a project' do expect(merge_request_graphql_data['webUrl']).to be_present end + it 'includes author' do + post_graphql(query, current_user: current_user) + + expect(merge_request_graphql_data['author']['username']).to eq(merge_request.author.username) + end + + it 'includes correct mergedAt value when merged' do + time = 1.week.ago + merge_request.mark_as_merged + merge_request.metrics.update_columns(merged_at: time) + + post_graphql(query, current_user: current_user) + retrieved = merge_request_graphql_data['mergedAt'] + + expect(Time.zone.parse(retrieved)).to be_within(1.second).of(time) + end + + it 'includes nil mergedAt value when not merged' do + post_graphql(query, current_user: current_user) + retrieved = merge_request_graphql_data['mergedAt'] + + expect(retrieved).to be_nil + end + context 'permissions on the merge request' do it 'includes the permissions for the current user on a public project' do expected_permissions = { diff --git a/spec/requests/api/graphql/project/merge_requests_spec.rb b/spec/requests/api/graphql/project/merge_requests_spec.rb new file mode 100644 index 00000000000..49fdfe29874 --- /dev/null +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -0,0 +1,174 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'getting merge request listings nested in a project' do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:current_user) { create(:user) } + + let_it_be(:label) { create(:label) } + let_it_be(:merge_request_a) { create(:labeled_merge_request, :unique_branches, source_project: project, labels: [label]) } + let_it_be(:merge_request_b) { create(:merge_request, :closed, :unique_branches, source_project: project) } + let_it_be(:merge_request_c) { create(:labeled_merge_request, :closed, :unique_branches, source_project: project, labels: [label]) } + let_it_be(:merge_request_d) { create(:merge_request, :locked, :unique_branches, source_project: project) } + + let(:results) { graphql_data.dig('project', 'mergeRequests', 'nodes') } + + let(:search_params) { nil } + + def query_merge_requests(fields) + graphql_query_for( + :project, + { full_path: project.full_path }, + query_graphql_field(:merge_requests, search_params, [ + query_graphql_field(:nodes, nil, fields) + ]) + ) + end + + let(:query) do + query_merge_requests(all_graphql_fields_for('MergeRequest', max_depth: 1)) + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + # The following tests are needed to guarantee that we have correctly annotated + # all the gitaly calls. Selecting combinations of fields may mask this due to + # memoization. + context 'requesting a single field' do + let_it_be(:fresh_mr) { create(:merge_request, :unique_branches, source_project: project) } + let(:search_params) { { iids: [fresh_mr.iid.to_s] } } + + before do + project.repository.expire_branches_cache + 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 + end + end + end + + context 'selecting any single nested field' do + where(:field, :subfield, :is_connection) do + nested_fields_of('MergeRequest').flat_map do |name, field| + type = field_type(field) + is_connection = type.name.ends_with?('Connection') + type = field_type(type.fields['nodes']) if is_connection + + type.fields + .select { |_, field| !nested_fields?(field) && !required_arguments?(field) } + .map(&:first) + .map { |subfield| [name, subfield, is_connection] } + end + 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 + end + end + end + end + + shared_examples 'searching with parameters' do + let(:expected) do + mrs.map { |mr| a_hash_including('iid' => mr.iid.to_s, 'title' => mr.title) } + end + + it 'finds the right mrs' do + post_graphql(query, current_user: current_user) + + expect(results).to match_array(expected) + end + end + + context 'there are no search params' do + let(:search_params) { nil } + let(:mrs) { [merge_request_a, merge_request_b, merge_request_c, merge_request_d] } + + it_behaves_like 'searching with parameters' + end + + context 'the search params do not match anything' do + let(:search_params) { { iids: %w(foo bar baz) } } + let(:mrs) { [] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by iids' do + let(:search_params) { { iids: mrs.map(&:iid).map(&:to_s) } } + let(:mrs) { [merge_request_a, merge_request_c] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by state' do + let(:search_params) { { state: :closed } } + let(:mrs) { [merge_request_b, merge_request_c] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by source_branch' do + let(:search_params) { { source_branches: mrs.map(&:source_branch) } } + let(:mrs) { [merge_request_b, merge_request_c] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by target_branch' do + let(:search_params) { { target_branches: mrs.map(&:target_branch) } } + let(:mrs) { [merge_request_a, merge_request_d] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by label' do + let(:search_params) { { labels: [label.title] } } + let(:mrs) { [merge_request_a, merge_request_c] } + + it_behaves_like 'searching with parameters' + end + + context 'searching by combination' do + let(:search_params) { { state: :closed, labels: [label.title] } } + let(:mrs) { [merge_request_c] } + + it_behaves_like 'searching with parameters' + end +end diff --git a/spec/requests/api/graphql/project/pipeline_spec.rb b/spec/requests/api/graphql/project/pipeline_spec.rb new file mode 100644 index 00000000000..bed9a18577f --- /dev/null +++ b/spec/requests/api/graphql/project/pipeline_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'getting pipeline information nested in a project' do + include GraphqlHelpers + + let(:project) { create(:project, :repository, :public) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:current_user) { create(:user) } + let(:pipeline_graphql_data) { graphql_data['project']['pipeline'] } + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('pipeline', iid: pipeline.iid.to_s) + ) + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + it 'contains pipeline information' do + post_graphql(query, current_user: current_user) + + expect(pipeline_graphql_data).not_to be_nil + end +end diff --git a/spec/requests/api/graphql/project/release_spec.rb b/spec/requests/api/graphql/project/release_spec.rb new file mode 100644 index 00000000000..f8624a97a2b --- /dev/null +++ b/spec/requests/api/graphql/project/release_spec.rb @@ -0,0 +1,206 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'pp' + +describe 'Query.project(fullPath).release(tagName)' do + include GraphqlHelpers + include Presentable + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:milestone_1) { create(:milestone, project: project) } + let_it_be(:milestone_2) { create(:milestone, project: project) } + let_it_be(:release) { create(:release, :with_evidence, project: project, milestones: [milestone_1, milestone_2]) } + let_it_be(:release_link_1) { create(:release_link, release: release) } + let_it_be(:release_link_2) { create(:release_link, release: release) } + let_it_be(:developer) { create(:user) } + + let(:current_user) { developer } + + def query(rq = release_fields) + graphql_query_for(:project, { fullPath: project.full_path }, + query_graphql_field(:release, { tagName: release.tag }, rq)) + end + + let(:post_query) { post_graphql(query, current_user: current_user) } + let(:path_prefix) { %w[project release] } + + let(:data) { graphql_data.dig(*path) } + + before do + project.add_developer(developer) + end + + describe 'scalar fields' do + let(:path) { path_prefix } + let(:release_fields) do + query_graphql_field(%{ + tagName + tagPath + description + descriptionHtml + name + createdAt + releasedAt + }) + end + + before do + post_query + end + + it 'finds all release data' do + expect(data).to eq({ + 'tagName' => release.tag, + 'tagPath' => project_tag_path(project, release.tag), + 'description' => release.description, + 'descriptionHtml' => release.description_html, + 'name' => release.name, + 'createdAt' => release.created_at.iso8601, + 'releasedAt' => release.released_at.iso8601 + }) + end + end + + describe 'milestones' do + let(:path) { path_prefix + %w[milestones nodes] } + let(:release_fields) do + query_graphql_field(:milestones, nil, 'nodes { id title }') + end + + it 'finds all milestones associated to a release' do + post_query + + expected = release.milestones.map do |milestone| + { 'id' => global_id_of(milestone), 'title' => milestone.title } + end + + expect(data).to match_array(expected) + end + end + + describe 'author' do + let(:path) { path_prefix + %w[author] } + let(:release_fields) do + query_graphql_field(:author, nil, 'id username') + end + + it 'finds the author of the release' do + post_query + + expect(data).to eq({ + 'id' => global_id_of(release.author), + 'username' => release.author.username + }) + end + end + + describe 'commit' do + let(:path) { path_prefix + %w[commit] } + let(:release_fields) do + query_graphql_field(:commit, nil, 'sha') + end + + it 'finds the commit associated with the release' do + post_query + + expect(data).to eq({ 'sha' => release.commit.sha }) + end + end + + describe 'assets' do + describe 'assetsCount' do + let(:path) { path_prefix + %w[assets] } + let(:release_fields) do + query_graphql_field(:assets, nil, 'assetsCount') + end + + it 'returns the number of assets associated to the release' do + post_query + + expect(data).to eq({ 'assetsCount' => release.sources.size + release.links.size }) + end + end + + describe 'links' do + let(:path) { path_prefix + %w[assets links nodes] } + let(:release_fields) do + query_graphql_field(:assets, nil, + query_graphql_field(:links, nil, 'nodes { id name url external }')) + end + + it 'finds all release links' do + post_query + + expected = release.links.map do |link| + { + 'id' => global_id_of(link), + 'name' => link.name, + 'url' => link.url, + 'external' => link.external? + } + end + + expect(data).to match_array(expected) + end + end + + describe 'sources' do + let(:path) { path_prefix + %w[assets sources nodes] } + let(:release_fields) do + query_graphql_field(:assets, nil, + query_graphql_field(:sources, nil, 'nodes { format url }')) + end + + it 'finds all release sources' do + post_query + + expected = release.sources.map do |source| + { + 'format' => source.format, + 'url' => source.url + } + end + + expect(data).to match_array(expected) + end + end + + describe 'evidences' do + let(:path) { path_prefix + %w[evidences] } + let(:release_fields) do + query_graphql_field(:evidences, nil, 'nodes { id sha filepath collectedAt }') + end + + context 'for a developer' do + it 'finds all evidence fields' do + post_query + + evidence = release.evidences.first.present + expected = { + 'id' => global_id_of(evidence), + 'sha' => evidence.sha, + 'filepath' => evidence.filepath, + 'collectedAt' => evidence.collected_at.utc.iso8601 + } + + expect(data["nodes"].first).to eq(expected) + end + end + + context 'for a guest' do + let(:current_user) { create :user } + + before do + project.add_guest(current_user) + end + + it 'denies access' do + post_query + + expect(data['node']).to be_nil + end + end + end + end +end diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index 035894c8022..9a88b47eea6 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -62,6 +62,54 @@ describe 'getting project information' do end end + describe 'performance' do + before 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', :use_clean_rails_memory_store_caching, :request_store do + expect { run_query(10) }.to issue_same_number_of_queries_as { run_query(1) }.or_fewer.ignoring_cached_queries + 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/tasks/task_completion_status_spec.rb b/spec/requests/api/graphql/tasks/task_completion_status_spec.rb index c727750c0ce..c47406ea534 100644 --- a/spec/requests/api/graphql/tasks/task_completion_status_spec.rb +++ b/spec/requests/api/graphql/tasks/task_completion_status_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' describe 'getting task completion status information' do include GraphqlHelpers - DESCRIPTION_0_DONE = '- [ ] task 1\n- [ ] task 2' - DESCRIPTION_1_DONE = '- [x] task 1\n- [ ] task 2' - DESCRIPTION_2_DONE = '- [x] task 1\n- [x] task 2' + description_0_done = '- [ ] task 1\n- [ ] task 2' + description_1_done = '- [x] task 1\n- [ ] task 2' + description_2_done = '- [x] task 1\n- [x] task 2' let_it_be(:user1) { create(:user) } let_it_be(:project) { create(:project, :repository, :public) } @@ -42,7 +42,7 @@ describe 'getting task completion status information' do end end - [DESCRIPTION_0_DONE, DESCRIPTION_1_DONE, DESCRIPTION_2_DONE].each do |desc| + [description_0_done, description_1_done, description_2_done].each do |desc| context "with description #{desc}" do context 'when type is issue' do it_behaves_like 'graphql task completion status provider', 'issue' do diff --git a/spec/requests/api/graphql/user/group_member_query_spec.rb b/spec/requests/api/graphql/user/group_member_query_spec.rb new file mode 100644 index 00000000000..022ee79297c --- /dev/null +++ b/spec/requests/api/graphql/user/group_member_query_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'GroupMember' do + include GraphqlHelpers + + let_it_be(:member) { create(:group_member, :developer) } + let_it_be(:fields) do + <<~HEREDOC + nodes { + accessLevel { + integerValue + stringValue + } + group { + id + } + } + HEREDOC + end + let_it_be(:query) do + graphql_query_for('user', { id: member.user.to_global_id.to_s }, query_graphql_field("groupMemberships", {}, fields)) + end + + before do + post_graphql(query, current_user: member.user) + end + + it_behaves_like 'a working graphql query' + it_behaves_like 'a working membership object query' +end diff --git a/spec/requests/api/graphql/user/project_member_query_spec.rb b/spec/requests/api/graphql/user/project_member_query_spec.rb new file mode 100644 index 00000000000..397d2872189 --- /dev/null +++ b/spec/requests/api/graphql/user/project_member_query_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'ProjectMember' do + include GraphqlHelpers + + let_it_be(:member) { create(:project_member, :developer) } + let_it_be(:fields) do + <<~HEREDOC + nodes { + accessLevel { + integerValue + stringValue + } + project { + id + } + } + HEREDOC + end + let_it_be(:query) do + graphql_query_for('user', { id: member.user.to_global_id.to_s }, query_graphql_field("projectMemberships", {}, fields)) + end + + before do + post_graphql(query, current_user: member.user) + end + + it_behaves_like 'a working graphql query' + it_behaves_like 'a working membership object query' +end diff --git a/spec/requests/api/graphql/user_query_spec.rb b/spec/requests/api/graphql/user_query_spec.rb new file mode 100644 index 00000000000..5ac94bc7323 --- /dev/null +++ b/spec/requests/api/graphql/user_query_spec.rb @@ -0,0 +1,260 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'getting user information' do + include GraphqlHelpers + + let(:query) do + graphql_query_for(:user, user_params, user_fields) + end + + let(:user_fields) { all_graphql_fields_for('User', max_depth: 2) } + + context 'no parameters are provided' do + let(:user_params) { nil } + + it 'mentions the missing required parameters' do + post_graphql(query) + + expect_graphql_errors_to_include(/username/) + end + end + + context 'looking up a user by username' do + let_it_be(:project_a) { create(:project, :repository) } + let_it_be(:project_b) { create(:project, :repository) } + let_it_be(:user, reload: true) { create(:user, developer_projects: [project_a, project_b]) } + let_it_be(:authorised_user) { create(:user, developer_projects: [project_a, project_b]) } + let_it_be(:unauthorized_user) { create(:user) } + + let_it_be(:assigned_mr) do + create(:merge_request, :unique_branches, + source_project: project_a, assignees: [user]) + end + let_it_be(:assigned_mr_b) do + create(:merge_request, :unique_branches, + source_project: project_b, assignees: [user]) + end + let_it_be(:assigned_mr_c) do + create(:merge_request, :unique_branches, + source_project: project_b, assignees: [user]) + end + let_it_be(:authored_mr) do + create(:merge_request, :unique_branches, + source_project: project_a, author: user) + end + let_it_be(:authored_mr_b) do + create(:merge_request, :unique_branches, + source_project: project_b, author: user) + end + let_it_be(:authored_mr_c) do + create(:merge_request, :unique_branches, + source_project: project_b, author: user) + end + + let(:current_user) { authorised_user } + let(:authored_mrs) { graphql_data_at(:user, :authored_merge_requests, :nodes) } + let(:assigned_mrs) { graphql_data_at(:user, :assigned_merge_requests, :nodes) } + let(:user_params) { { username: user.username } } + + before do + post_graphql(query, current_user: current_user) + end + + context 'the user is an active user' do + it_behaves_like 'a working graphql query' + + it 'can access user profile fields' do + presenter = UserPresenter.new(user) + + expect(graphql_data['user']).to match( + a_hash_including( + 'id' => global_id_of(user), + 'state' => presenter.state, + 'name' => presenter.name, + 'username' => presenter.username, + 'webUrl' => presenter.web_url, + 'avatarUrl' => presenter.avatar_url + )) + end + + describe 'assignedMergeRequests' do + let(:user_fields) do + query_graphql_field(:assigned_merge_requests, mr_args, 'nodes { id }') + end + let(:mr_args) { nil } + + it_behaves_like 'a working graphql query' + + it 'can be found' do + expect(assigned_mrs).to contain_exactly( + a_hash_including('id' => global_id_of(assigned_mr)), + a_hash_including('id' => global_id_of(assigned_mr_b)), + a_hash_including('id' => global_id_of(assigned_mr_c)) + ) + end + + context 'applying filters' do + context 'filtering by IID without specifying a project' do + let(:mr_args) do + { iids: [assigned_mr_b.iid.to_s] } + end + + it 'return an argument error that mentions the missing fields' do + expect_graphql_errors_to_include(/projectPath/) + end + end + + context 'filtering by project path and IID' do + let(:mr_args) do + { project_path: project_b.full_path, iids: [assigned_mr_b.iid.to_s] } + end + + it 'selects the correct MRs' do + expect(assigned_mrs).to contain_exactly( + a_hash_including('id' => global_id_of(assigned_mr_b)) + ) + end + end + + context 'filtering by project path' do + let(:mr_args) do + { project_path: project_b.full_path } + end + + it 'selects the correct MRs' do + expect(assigned_mrs).to contain_exactly( + a_hash_including('id' => global_id_of(assigned_mr_b)), + a_hash_including('id' => global_id_of(assigned_mr_c)) + ) + end + end + end + + context 'the current user does not have access' do + let(:current_user) { unauthorized_user } + + it 'cannot be found' do + expect(assigned_mrs).to be_empty + end + end + end + + describe 'authoredMergeRequests' do + let(:user_fields) do + query_graphql_field(:authored_merge_requests, mr_args, 'nodes { id }') + end + let(:mr_args) { nil } + + it_behaves_like 'a working graphql query' + + it 'can be found' do + expect(authored_mrs).to contain_exactly( + a_hash_including('id' => global_id_of(authored_mr)), + a_hash_including('id' => global_id_of(authored_mr_b)), + a_hash_including('id' => global_id_of(authored_mr_c)) + ) + end + + context 'applying filters' do + context 'filtering by IID without specifying a project' do + let(:mr_args) do + { iids: [authored_mr_b.iid.to_s] } + end + + it 'return an argument error that mentions the missing fields' do + expect_graphql_errors_to_include(/projectPath/) + end + end + + context 'filtering by project path and IID' do + let(:mr_args) do + { project_path: project_b.full_path, iids: [authored_mr_b.iid.to_s] } + end + + it 'selects the correct MRs' do + expect(authored_mrs).to contain_exactly( + a_hash_including('id' => global_id_of(authored_mr_b)) + ) + end + end + + context 'filtering by project path' do + let(:mr_args) do + { project_path: project_b.full_path } + end + + it 'selects the correct MRs' do + expect(authored_mrs).to contain_exactly( + a_hash_including('id' => global_id_of(authored_mr_b)), + a_hash_including('id' => global_id_of(authored_mr_c)) + ) + end + end + end + + context 'the current user does not have access' do + let(:current_user) { unauthorized_user } + + it 'cannot be found' do + expect(authored_mrs).to be_empty + end + end + end + end + + context 'the user is private' do + before do + user.update(private_profile: true) + post_graphql(query, current_user: current_user) + end + + context 'we only request basic fields' do + let(:user_fields) { %i[id name username state web_url avatar_url] } + + it_behaves_like 'a working graphql query' + end + + context 'we request the authoredMergeRequests' do + let(:user_fields) { 'authoredMergeRequests { nodes { id } }' } + + it_behaves_like 'a working graphql query' + + it 'cannot be found' do + expect(authored_mrs).to be_empty + end + + context 'the current user is the user' do + let(:current_user) { user } + + it 'can be found' do + expect(authored_mrs).to include( + a_hash_including('id' => global_id_of(authored_mr)) + ) + end + end + end + + context 'we request the assignedMergeRequests' do + let(:user_fields) { 'assignedMergeRequests { nodes { id } }' } + + it_behaves_like 'a working graphql query' + + it 'cannot be found' do + expect(assigned_mrs).to be_empty + end + + context 'the current user is the user' do + let(:current_user) { user } + + it 'can be found' do + expect(assigned_mrs).to include( + a_hash_including('id' => global_id_of(assigned_mr)) + ) + end + end + end + end + end +end diff --git a/spec/requests/api/graphql/user_spec.rb b/spec/requests/api/graphql/user_spec.rb new file mode 100644 index 00000000000..097c75b3541 --- /dev/null +++ b/spec/requests/api/graphql/user_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'User' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + + shared_examples 'a working user query' do + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + it 'includes the user' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['user']).not_to be_nil + end + + it 'returns no user when global restricted_visibility_levels includes PUBLIC' do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + + post_graphql(query) + + expect(graphql_data['user']).to be_nil + end + end + + context 'when id parameter is used' do + let(:query) { graphql_query_for(:user, { id: current_user.to_global_id.to_s }) } + + it_behaves_like 'a working user query' + end + + context 'when username parameter is used' do + let(:query) { graphql_query_for(:user, { username: current_user.username.to_s }) } + + it_behaves_like 'a working user query' + end + + context 'when username and id parameter are used' do + let_it_be(:query) { graphql_query_for(:user, { id: current_user.to_global_id.to_s, username: current_user.username }, 'id') } + + it 'displays an error' do + post_graphql(query) + + expect(graphql_errors).to include( + a_hash_including('message' => a_string_matching(%r{Provide either a single username or id})) + ) + end + end +end diff --git a/spec/requests/api/graphql/users_spec.rb b/spec/requests/api/graphql/users_spec.rb new file mode 100644 index 00000000000..1e6d73cbd7d --- /dev/null +++ b/spec/requests/api/graphql/users_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Users' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user, created_at: 1.day.ago) } + let_it_be(:user1) { create(:user, created_at: 2.days.ago) } + let_it_be(:user2) { create(:user, created_at: 3.days.ago) } + let_it_be(:user3) { create(:user, created_at: 4.days.ago) } + + describe '.users' do + shared_examples 'a working users query' do + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + it 'includes a list of users' do + post_graphql(query) + + expect(graphql_data.dig('users', 'nodes')).not_to be_empty + end + end + + context 'with no arguments' do + let_it_be(:query) { graphql_query_for(:users, { usernames: [user1.username] }, 'nodes { id }') } + + it_behaves_like 'a working users query' + end + + context 'with a list of usernames' do + let(:query) { graphql_query_for(:users, { usernames: [user1.username] }, 'nodes { id }') } + + it_behaves_like 'a working users query' + end + + context 'with a list of IDs' do + let(:query) { graphql_query_for(:users, { ids: [user1.to_global_id.to_s] }, 'nodes { id }') } + + it_behaves_like 'a working users query' + end + + context 'when usernames and ids parameter are used' do + let_it_be(:query) { graphql_query_for(:users, { ids: user1.to_global_id.to_s, usernames: user1.username }, 'nodes { id }') } + + it 'displays an error' do + post_graphql(query) + + expect(graphql_errors).to include( + a_hash_including('message' => a_string_matching(%r{Provide either a list of usernames or ids})) + ) + end + end + end + + describe 'sorting and pagination' do + let_it_be(:data_path) { [:users] } + + def pagination_query(params, page_info) + graphql_query_for("users", params, "#{page_info} edges { node { id } }") + end + + def pagination_results_data(data) + data.map { |user| user.dig('node', 'id') } + end + + context 'when sorting by created_at' do + let_it_be(:ascending_users) { [user3, user2, user1, current_user].map(&:to_global_id).map(&:to_s) } + + context 'when ascending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'created_asc' } + let(:first_param) { 1 } + let(:expected_results) { ascending_users } + end + end + + context 'when descending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { 'created_desc' } + let(:first_param) { 1 } + let(:expected_results) { ascending_users.reverse } + end + end + end + end +end diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index f5c7a820abe..84be5ab0951 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -187,4 +187,62 @@ describe 'GraphQL' do end end end + + describe 'keyset pagination' do + let_it_be(:project) { create(:project, :public) } + let_it_be(:issues) { create_list(:issue, 10, project: project, created_at: Time.now.change(usec: 200)) } + + let(:page_size) { 6 } + let(:issues_edges) { %w(data project issues edges) } + let(:end_cursor) { %w(data project issues pageInfo endCursor) } + let(:query) do + <<~GRAPHQL + query project($fullPath: ID!, $first: Int, $after: String) { + project(fullPath: $fullPath) { + issues(first: $first, after: $after) { + edges { node { iid } } + pageInfo { endCursor } + } + } + } + GRAPHQL + end + + # TODO: Switch this to use `post_graphql` + # This is not performing an actual GraphQL request because the + # variables end up being strings when passed through the `post_graphql` + # helper. + # + # https://gitlab.com/gitlab-org/gitlab/-/issues/222432 + def execute_query(after: nil) + GitlabSchema.execute( + query, + context: { current_user: nil }, + variables: { + fullPath: project.full_path, + first: page_size, + after: after + } + ) + end + + it 'paginates datetimes correctly when they have millisecond data' do + # let's make sure we're actually querying a timestamp, just in case + expect(Gitlab::Graphql::Pagination::Keyset::QueryBuilder) + .to receive(:new).with(anything, anything, hash_including('created_at'), anything).and_call_original + + first_page = execute_query + edges = first_page.dig(*issues_edges) + cursor = first_page.dig(*end_cursor) + + expect(edges.count).to eq(6) + expect(edges.last['node']['iid']).to eq(issues[4].iid.to_s) + + second_page = execute_query(after: cursor) + edges = second_page.dig(*issues_edges) + + expect(edges.count).to eq(4) + expect(edges.last['node']['iid']).to eq(issues[0].iid.to_s) + end + end end diff --git a/spec/requests/api/group_export_spec.rb b/spec/requests/api/group_export_spec.rb index 02ae9d71702..9dd7797c768 100644 --- a/spec/requests/api/group_export_spec.rb +++ b/spec/requests/api/group_export_spec.rb @@ -82,6 +82,22 @@ describe API::GroupExport do expect(response).to have_gitlab_http_status(:not_found) end end + + context 'when the requests have exceeded the rate limit' do + before do + allow(Gitlab::ApplicationRateLimiter) + .to receive(:increment) + .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_download_export][:threshold] + 1) + end + + it 'throttles the endpoint' do + get api(download_path, user) + + expect(json_response["message"]) + .to include('error' => 'This endpoint has been requested too many times. Try again later.') + expect(response).to have_gitlab_http_status :too_many_requests + end + end end describe 'POST /groups/:group_id/export' do @@ -139,5 +155,23 @@ describe API::GroupExport do expect(response).to have_gitlab_http_status(:not_found) end end + + context 'when the requests have exceeded the rate limit' do + before do + group.add_owner(user) + + allow(Gitlab::ApplicationRateLimiter) + .to receive(:increment) + .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:group_export][:threshold] + 1) + end + + it 'throttles the endpoint' do + post api(path, user) + + expect(json_response["message"]) + .to include('error' => 'This endpoint has been requested too many times. Try again later.') + expect(response).to have_gitlab_http_status :too_many_requests + end + end end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 18feff85482..9a449499576 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -231,6 +231,27 @@ describe API::Groups do end end + context "when using top_level_only" do + let(:top_level_group) { create(:group, name: 'top-level-group') } + let(:subgroup) { create(:group, :nested, name: 'subgroup') } + let(:response_groups) { json_response.map { |group| group['name'] } } + + before do + top_level_group.add_owner(user1) + subgroup.add_owner(user1) + end + + it "doesn't return subgroups" do + get api("/groups", user1), params: { top_level_only: true } + + 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 include(top_level_group.name) + expect(response_groups).not_to include(subgroup.name) + end + end + context "when using sorting" do let(:group3) { create(:group, name: "a#{group1.name}", path: "z#{group1.path}") } let(:group4) { create(:group, name: "same-name", path: "y#{group1.path}") } @@ -415,6 +436,8 @@ describe API::Groups do it "returns one of user1's groups" do project = create(:project, namespace: group2, path: 'Foo') create(:project_group_link, project: project, group: group1) + group = create(:group) + link = create(:group_group_link, shared_group: group1, shared_with_group: group) get api("/groups/#{group1.id}", user1) @@ -439,6 +462,13 @@ describe API::Groups do expect(json_response['full_path']).to eq(group1.full_path) expect(json_response['parent_id']).to eq(group1.parent_id) expect(json_response['created_at']).to be_present + expect(json_response['shared_with_groups']).to be_an Array + expect(json_response['shared_with_groups'].length).to eq(1) + expect(json_response['shared_with_groups'][0]['group_id']).to eq(group.id) + expect(json_response['shared_with_groups'][0]['group_name']).to eq(group.name) + expect(json_response['shared_with_groups'][0]['group_full_path']).to eq(group.full_path) + expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access) + expect(json_response['shared_with_groups'][0]).to have_key('expires_at') expect(json_response['projects']).to be_an Array expect(json_response['projects'].length).to eq(2) expect(json_response['shared_projects']).to be_an Array @@ -505,7 +535,7 @@ describe API::Groups do .to contain_exactly(projects[:public].id, projects[:internal].id) end - it 'avoids N+1 queries' do + it 'avoids N+1 queries with project links' do get api("/groups/#{group1.id}", admin) control_count = ActiveRecord::QueryRecorder.new do @@ -518,6 +548,24 @@ describe API::Groups do get api("/groups/#{group1.id}", admin) end.not_to exceed_query_limit(control_count) end + + it 'avoids N+1 queries with shared group links' do + # setup at least 1 shared group, so that we record the queries that preload the nested associations too. + create(:group_group_link, shared_group: group1, shared_with_group: create(:group)) + + control_count = ActiveRecord::QueryRecorder.new do + get api("/groups/#{group1.id}", admin) + end.count + + # setup "n" more shared groups + create(:group_group_link, shared_group: group1, shared_with_group: create(:group)) + create(:group_group_link, shared_group: group1, shared_with_group: create(:group)) + + # test that no of queries for 1 shared group is same as for n shared groups + expect do + get api("/groups/#{group1.id}", admin) + end.not_to exceed_query_limit(control_count) + end end context "when authenticated as admin" do @@ -1507,4 +1555,173 @@ describe API::Groups do group2.add_owner(user1) end end + + describe "POST /groups/:id/share" do + shared_examples 'shares group with group' do + it "shares group with group" do + expires_at = 10.days.from_now.to_date + + expect do + post api("/groups/#{group.id}/share", user), params: { group_id: shared_with_group.id, group_access: Gitlab::Access::DEVELOPER, expires_at: expires_at } + end.to change { group.shared_with_group_links.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['shared_with_groups']).to be_an Array + expect(json_response['shared_with_groups'].length).to eq(1) + expect(json_response['shared_with_groups'][0]['group_id']).to eq(shared_with_group.id) + expect(json_response['shared_with_groups'][0]['group_name']).to eq(shared_with_group.name) + expect(json_response['shared_with_groups'][0]['group_full_path']).to eq(shared_with_group.full_path) + expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(Gitlab::Access::DEVELOPER) + expect(json_response['shared_with_groups'][0]['expires_at']).to eq(expires_at.to_s) + end + + it "returns a 400 error when group id is not given" do + post api("/groups/#{group.id}/share", user), params: { group_access: Gitlab::Access::DEVELOPER } + expect(response).to have_gitlab_http_status(:bad_request) + end + + it "returns a 400 error when access level is not given" do + post api("/groups/#{group.id}/share", user), params: { group_id: shared_with_group.id } + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns a 404 error when group does not exist' do + post api("/groups/#{group.id}/share", user), params: { group_id: non_existing_record_id, group_access: Gitlab::Access::DEVELOPER } + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "returns a 400 error when wrong params passed" do + post api("/groups/#{group.id}/share", user), params: { group_id: shared_with_group.id, group_access: non_existing_record_access_level } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'group_access does not have a valid value' + end + + it "returns a 409 error when link is not saved" do + allow(::Groups::GroupLinks::CreateService).to receive_message_chain(:new, :execute) + .and_return({ status: :error, http_status: 409, message: 'error' }) + + post api("/groups/#{group.id}/share", user), params: { group_id: shared_with_group.id, group_access: Gitlab::Access::DEVELOPER } + + expect(response).to have_gitlab_http_status(:conflict) + end + end + + context 'when authenticated as owner' do + let(:owner_group) { create(:group) } + let(:owner_user) { create(:user) } + + before do + owner_group.add_owner(owner_user) + end + + it_behaves_like 'shares group with group' do + let(:user) { owner_user } + let(:group) { owner_group } + let(:shared_with_group) { create(:group) } + end + end + + context 'when the user is not the owner of the group' do + let(:group) { create(:group) } + let(:user4) { create(:user) } + let(:expires_at) { 10.days.from_now.to_date } + + before do + group1.add_maintainer(user4) + end + + it 'does not create group share' do + post api("/groups/#{group1.id}/share", user4), params: { group_id: group.id, group_access: Gitlab::Access::DEVELOPER, expires_at: expires_at } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when authenticated as admin' do + it_behaves_like 'shares group with group' do + let(:user) { admin } + let(:group) { create(:group) } + let(:shared_with_group) { create(:group) } + end + end + end + + describe 'DELETE /groups/:id/share/:group_id' do + shared_examples 'deletes group share' do + it 'deletes a group share' do + expect do + delete api("/groups/#{shared_group.id}/share/#{shared_with_group.id}", user) + + expect(response).to have_gitlab_http_status(:no_content) + expect(shared_group.shared_with_group_links).to be_empty + end.to change { shared_group.shared_with_group_links.count }.by(-1) + end + + it 'requires the group id to be an integer' do + delete api("/groups/#{shared_group.id}/share/foo", user) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns a 404 error when group link does not exist' do + delete api("/groups/#{shared_group.id}/share/#{non_existing_record_id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns a 404 error when group does not exist' do + delete api("/groups/123/share/#{non_existing_record_id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when authenticated as owner' do + let(:group_a) { create(:group) } + + before do + create(:group_group_link, shared_group: group1, shared_with_group: group_a) + end + + it_behaves_like 'deletes group share' do + let(:user) { user1 } + let(:shared_group) { group1 } + let(:shared_with_group) { group_a } + end + end + + context 'when the user is not the owner of the group' do + let(:group_a) { create(:group) } + let(:user4) { create(:user) } + + before do + group1.add_maintainer(user4) + create(:group_group_link, shared_group: group1, shared_with_group: group_a) + end + + it 'does not remove group share' do + expect do + delete api("/groups/#{group1.id}/share/#{group_a.id}", user4) + + expect(response).to have_gitlab_http_status(:no_content) + end.not_to change { group1.shared_with_group_links } + end + end + + context 'when authenticated as admin' do + let(:group_b) { create(:group) } + + before do + create(:group_group_link, shared_group: group2, shared_with_group: group_b) + end + + it_behaves_like 'deletes group share' do + let(:user) { admin } + let(:shared_group) { group2 } + let(:shared_with_group) { group_b } + end + end + end end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 684f0329909..aa5e2367a2b 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -218,7 +218,15 @@ describe API::Internal::Base do get(api('/internal/authorized_keys'), params: { fingerprint: key.fingerprint, secret_token: secret_token }) expect(response).to have_gitlab_http_status(:ok) - expect(json_response["key"]).to eq(key.key) + expect(json_response['id']).to eq(key.id) + expect(json_response['key'].split[1]).to eq(key.key.split[1]) + end + + it 'exposes the comment of the key as a simple identifier of username + hostname' do + get(api('/internal/authorized_keys'), params: { fingerprint: key.fingerprint, secret_token: secret_token }) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['key']).to include("#{key.user_name} (#{Gitlab.config.gitlab.host})") end end @@ -239,11 +247,21 @@ describe API::Internal::Base do end context "sending the key" do - it "finds the key" do - get(api('/internal/authorized_keys'), params: { key: key.key.split[1], secret_token: secret_token }) + context "using an existing key" do + it "finds the key" do + get(api('/internal/authorized_keys'), params: { key: key.key.split[1], secret_token: secret_token }) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response["key"]).to eq(key.key) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['id']).to eq(key.id) + expect(json_response['key'].split[1]).to eq(key.key.split[1]) + end + + it 'exposes the comment of the key as a simple identifier of username + hostname' do + get(api('/internal/authorized_keys'), params: { fingerprint: key.fingerprint, secret_token: secret_token }) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['key']).to include("#{key.user_name} (#{Gitlab.config.gitlab.host})") + end end it "returns 404 with a partial key" do @@ -396,7 +414,7 @@ describe API::Internal::Base do context "git pull" do before do - allow(Feature).to receive(:persisted_names).and_return(%w[gitaly_mep_mep]) + stub_feature_flags(gitaly_mep_mep: true) end it "has the correct payload" do diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index 06878f57d43..315396c89c3 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -834,6 +834,12 @@ describe API::Issues do end end + describe 'PUT /projects/:id/issues/:issue_id' do + it_behaves_like 'issuable update endpoint' do + let(:entity) { issue } + end + end + describe 'DELETE /projects/:id/issues/:issue_iid' do it 'rejects a non member from deleting an issue' do delete api("/projects/#{project.id}/issues/#{issue.iid}", non_member) diff --git a/spec/requests/api/issues/put_projects_issues_spec.rb b/spec/requests/api/issues/put_projects_issues_spec.rb index 2ab8b9d7877..62a4d3b48b2 100644 --- a/spec/requests/api/issues/put_projects_issues_spec.rb +++ b/spec/requests/api/issues/put_projects_issues_spec.rb @@ -5,10 +5,6 @@ require 'spec_helper' describe API::Issues do let_it_be(:user) { create(:user) } let_it_be(:owner) { create(:owner) } - let_it_be(:project, reload: true) do - create(:project, :public, creator_id: owner.id, namespace: owner.namespace) - end - let(:user2) { create(:user) } let(:non_member) { create(:user) } let_it_be(:guest) { create(:user) } @@ -17,6 +13,11 @@ describe API::Issues do let(:admin) { create(:user, :admin) } let(:issue_title) { 'foo' } let(:issue_description) { 'closed' } + + let_it_be(:project, reload: true) do + create(:project, :public, creator_id: owner.id, namespace: owner.namespace) + end + let!(:closed_issue) do create :closed_issue, author: user, @@ -28,6 +29,7 @@ describe API::Issues do updated_at: 3.hours.ago, closed_at: 1.hour.ago end + let!(:confidential_issue) do create :issue, :confidential, @@ -37,6 +39,7 @@ describe API::Issues do created_at: generate(:past_time), updated_at: 2.hours.ago end + let!(:issue) do create :issue, author: user, @@ -48,18 +51,24 @@ describe API::Issues do title: issue_title, description: issue_description end + let_it_be(:label) do create(:label, title: 'label', color: '#FFAABB', project: project) end + let!(:label_link) { create(:label_link, label: label, target: issue) } let(:milestone) { create(:milestone, title: '1.0.0', project: project) } + let_it_be(:empty_milestone) do create(:milestone, title: '2.0.0', project: project) end - let!(:note) { create(:note_on_issue, author: user, project: project, noteable: issue) } + let!(:note) { create(:note_on_issue, author: user, project: project, noteable: issue) } let(:no_milestone_title) { 'None' } let(:any_milestone_title) { 'Any' } + let(:updated_title) { 'updated title' } + let(:issue_path) { "/projects/#{project.id}/issues/#{issue.iid}" } + let(:api_for_user) { api(issue_path, user) } before_all do project.add_reporter(user) @@ -72,108 +81,97 @@ describe API::Issues do describe 'PUT /projects/:id/issues/:issue_iid to update only title' do it 'updates a project issue' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { title: 'updated title' } - expect(response).to have_gitlab_http_status(:ok) + put api_for_user, params: { title: updated_title } - expect(json_response['title']).to eq('updated title') + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['title']).to eq(updated_title) end it 'returns 404 error if issue iid not found' do - put api("/projects/#{project.id}/issues/44444", user), - params: { title: 'updated title' } + put api("/projects/#{project.id}/issues/44444", user), params: { title: updated_title } + expect(response).to have_gitlab_http_status(:not_found) end it 'returns 404 error if issue id is used instead of the iid' do - put api("/projects/#{project.id}/issues/#{issue.id}", user), - params: { title: 'updated title' } + put api("/projects/#{project.id}/issues/#{issue.id}", user), params: { title: updated_title } + expect(response).to have_gitlab_http_status(:not_found) end it 'allows special label names' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), + put api_for_user, params: { - title: 'updated title', + title: updated_title, labels: 'label, label?, label&foo, ?, &' } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['labels']).to include 'label' - expect(json_response['labels']).to include 'label?' - expect(json_response['labels']).to include 'label&foo' - expect(json_response['labels']).to include '?' - expect(json_response['labels']).to include '&' end it 'allows special label names with labels param as array' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), + put api_for_user, params: { - title: 'updated title', + title: updated_title, labels: ['label', 'label?', 'label&foo, ?, &'] } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['labels']).to include 'label' - expect(json_response['labels']).to include 'label?' - expect(json_response['labels']).to include 'label&foo' - expect(json_response['labels']).to include '?' - expect(json_response['labels']).to include '&' + expect(json_response['labels']).to contain_exactly('label', 'label?', 'label&foo', '?', '&') end context 'confidential issues' do + let(:confidential_issue_path) { "/projects/#{project.id}/issues/#{confidential_issue.iid}" } + it 'returns 403 for non project members' do - put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", non_member), - params: { title: 'updated title' } + put api(confidential_issue_path, non_member), params: { title: updated_title } + expect(response).to have_gitlab_http_status(:forbidden) end it 'returns 403 for project members with guest role' do - put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", guest), - params: { title: 'updated title' } + put api(confidential_issue_path, guest), params: { title: updated_title } + expect(response).to have_gitlab_http_status(:forbidden) end it 'updates a confidential issue for project members' do - put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", user), - params: { title: 'updated title' } + put api(confidential_issue_path, user), params: { title: updated_title } + expect(response).to have_gitlab_http_status(:ok) - expect(json_response['title']).to eq('updated title') + expect(json_response['title']).to eq(updated_title) end it 'updates a confidential issue for author' do - put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", author), - params: { title: 'updated title' } + put api(confidential_issue_path, author), params: { title: updated_title } + expect(response).to have_gitlab_http_status(:ok) - expect(json_response['title']).to eq('updated title') + expect(json_response['title']).to eq(updated_title) end it 'updates a confidential issue for admin' do - put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", admin), - params: { title: 'updated title' } + put api(confidential_issue_path, admin), params: { title: updated_title } + expect(response).to have_gitlab_http_status(:ok) - expect(json_response['title']).to eq('updated title') + expect(json_response['title']).to eq(updated_title) end it 'sets an issue to confidential' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { confidential: true } + put api_for_user, params: { confidential: true } expect(response).to have_gitlab_http_status(:ok) expect(json_response['confidential']).to be_truthy end it 'makes a confidential issue public' do - put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", user), - params: { confidential: false } + put api(confidential_issue_path, user), params: { confidential: false } expect(response).to have_gitlab_http_status(:ok) expect(json_response['confidential']).to be_falsy end it 'does not update a confidential issue with wrong confidential flag' do - put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", user), - params: { confidential: 'foo' } + put api(confidential_issue_path, user), params: { confidential: 'foo' } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['error']).to eq('confidential is invalid') @@ -185,12 +183,12 @@ describe API::Issues do include_context 'includes Spam constants' def update_issue - put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: params + put api_for_user, params: params end let(:params) do { - title: 'updated title', + title: updated_title, description: 'content here', labels: 'label, label2' } @@ -224,7 +222,7 @@ describe API::Issues do it 'creates a new spam log entry' do expect { update_issue } - .to log_spam(title: 'updated title', description: 'content here', user_id: user.id, noteable_type: 'Issue') + .to log_spam(title: updated_title, description: 'content here', user_id: user.id, noteable_type: 'Issue') end end @@ -241,7 +239,7 @@ describe API::Issues do it 'creates a new spam log entry' do expect { update_issue } - .to log_spam(title: 'updated title', description: 'content here', user_id: user.id, noteable_type: 'Issue') + .to log_spam(title: updated_title, description: 'content here', user_id: user.id, noteable_type: 'Issue') end end end @@ -249,49 +247,39 @@ describe API::Issues do describe 'PUT /projects/:id/issues/:issue_iid to update assignee' do context 'support for deprecated assignee_id' do it 'removes assignee' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { assignee_id: 0 } + put api_for_user, params: { assignee_id: 0 } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['assignee']).to be_nil end it 'updates an issue with new assignee' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { assignee_id: user2.id } + put api_for_user, params: { assignee_id: user2.id } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['assignee']['name']).to eq(user2.name) end end it 'removes assignee' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { assignee_ids: [0] } + put api_for_user, params: { assignee_ids: [0] } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['assignees']).to be_empty end it 'updates an issue with new assignee' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { assignee_ids: [user2.id] } + put api_for_user, params: { assignee_ids: [user2.id] } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['assignees'].first['name']).to eq(user2.name) end context 'single assignee restrictions' do it 'updates an issue with several assignees but only one has been applied' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { assignee_ids: [user2.id, guest.id] } + put api_for_user, params: { assignee_ids: [user2.id, guest.id] } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['assignees'].size).to eq(1) end end @@ -301,16 +289,42 @@ describe API::Issues do let!(:label) { create(:label, title: 'dummy', project: project) } let!(:label_link) { create(:label_link, label: label, target: issue) } + it 'adds relevant labels' do + put api_for_user, params: { add_labels: '1, 2' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['labels']).to contain_exactly(label.title, '1', '2') + end + + context 'removes' do + let!(:label2) { create(:label, title: 'a-label', project: project) } + let!(:label_link2) { create(:label_link, label: label2, target: issue) } + + it 'removes relevant labels' do + put api_for_user, params: { remove_labels: label2.title } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['labels']).to eq([label.title]) + end + + it 'removes all labels' do + put api_for_user, 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 labels if not present' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { title: 'updated title' } + put api_for_user, params: { title: updated_title } + expect(response).to have_gitlab_http_status(:ok) expect(json_response['labels']).to eq([label.title]) end it 'removes all labels and touches the record' do Timecop.travel(1.minute.from_now) do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: { labels: '' } + put api_for_user, params: { labels: '' } end expect(response).to have_gitlab_http_status(:ok) @@ -320,7 +334,7 @@ describe API::Issues do it 'removes all labels and touches the record with labels param as array' do Timecop.travel(1.minute.from_now) do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: { labels: [''] } + put api_for_user, params: { labels: [''] } end expect(response).to have_gitlab_http_status(:ok) @@ -330,20 +344,19 @@ describe API::Issues do it 'updates labels and touches the record' do Timecop.travel(1.minute.from_now) do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { labels: 'foo,bar' } + put api_for_user, params: { labels: 'foo,bar' } end + expect(response).to have_gitlab_http_status(:ok) - expect(json_response['labels']).to include 'foo' - expect(json_response['labels']).to include 'bar' + expect(json_response['labels']).to contain_exactly('foo', 'bar') expect(json_response['updated_at']).to be > Time.now end it 'updates labels and touches the record with labels param as array' do Timecop.travel(1.minute.from_now) do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { labels: %w(foo bar) } + put api_for_user, params: { labels: %w(foo bar) } end + expect(response).to have_gitlab_http_status(:ok) expect(json_response['labels']).to include 'foo' expect(json_response['labels']).to include 'bar' @@ -351,36 +364,22 @@ describe API::Issues do end it 'allows special label names' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { labels: 'label:foo, label-bar,label_bar,label/bar,label?bar,label&bar,?,&' } + put api_for_user, params: { labels: 'label:foo, label-bar,label_bar,label/bar,label?bar,label&bar,?,&' } + expect(response).to have_gitlab_http_status(:ok) - expect(json_response['labels']).to include 'label:foo' - expect(json_response['labels']).to include 'label-bar' - expect(json_response['labels']).to include 'label_bar' - expect(json_response['labels']).to include 'label/bar' - expect(json_response['labels']).to include 'label?bar' - expect(json_response['labels']).to include 'label&bar' - expect(json_response['labels']).to include '?' - expect(json_response['labels']).to include '&' + expect(json_response['labels']).to contain_exactly('label:foo', 'label-bar', 'label_bar', 'label/bar', 'label?bar', 'label&bar', '?', '&') end it 'allows special label names with labels param as array' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { labels: ['label:foo', 'label-bar', 'label_bar', 'label/bar,label?bar,label&bar,?,&'] } + put api_for_user, params: { labels: ['label:foo', 'label-bar', 'label_bar', 'label/bar,label?bar,label&bar,?,&'] } + expect(response).to have_gitlab_http_status(:ok) - expect(json_response['labels']).to include 'label:foo' - expect(json_response['labels']).to include 'label-bar' - expect(json_response['labels']).to include 'label_bar' - expect(json_response['labels']).to include 'label/bar' - expect(json_response['labels']).to include 'label?bar' - expect(json_response['labels']).to include 'label&bar' - expect(json_response['labels']).to include '?' - expect(json_response['labels']).to include '&' + expect(json_response['labels']).to contain_exactly('label:foo', 'label-bar', 'label_bar', 'label/bar', 'label?bar', 'label&bar', '?', '&') end it 'returns 400 if title is too long' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { title: 'g' * 256 } + put api_for_user, params: { title: 'g' * 256 } + expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']['title']).to eq([ 'is too long (maximum is 255 characters)' @@ -390,16 +389,15 @@ describe API::Issues do describe 'PUT /projects/:id/issues/:issue_iid to update state and label' do it 'updates a project issue' do - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { labels: 'label2', state_event: 'close' } - expect(response).to have_gitlab_http_status(:ok) + put api_for_user, params: { labels: 'label2', state_event: 'close' } - expect(json_response['labels']).to include 'label2' + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['labels']).to contain_exactly('label2') expect(json_response['state']).to eq 'closed' end it 'reopens a project isssue' do - put api("/projects/#{project.id}/issues/#{closed_issue.iid}", user), params: { state_event: 'reopen' } + put api(issue_path, user), params: { state_event: 'reopen' } expect(response).to have_gitlab_http_status(:ok) expect(json_response['state']).to eq 'opened' @@ -411,42 +409,41 @@ describe API::Issues do it 'accepts the update date to be set' do update_time = 2.weeks.ago - put api("/projects/#{project.id}/issues/#{issue.iid}", user), - params: { title: 'some new title', updated_at: update_time } + put api_for_user, params: { title: 'some new title', updated_at: update_time } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['title']).to include 'some new title' + expect(json_response['title']).to eq('some new title') expect(Time.parse(json_response['updated_at'])).not_to be_like_time(update_time) end end context 'when admin or owner makes the request' do + let(:api_for_owner) { api(issue_path, owner) } + it 'not allow to set null for updated_at' do - put api("/projects/#{project.id}/issues/#{issue.iid}", owner), params: { updated_at: nil } + put api_for_owner, params: { updated_at: nil } expect(response).to have_gitlab_http_status(:bad_request) end it 'not allow to set blank for updated_at' do - put api("/projects/#{project.id}/issues/#{issue.iid}", owner), params: { updated_at: '' } + put api_for_owner, params: { updated_at: '' } expect(response).to have_gitlab_http_status(:bad_request) end it 'not allow to set invalid format for updated_at' do - put api("/projects/#{project.id}/issues/#{issue.iid}", owner), params: { updated_at: 'invalid-format' } + put api_for_owner, params: { updated_at: 'invalid-format' } expect(response).to have_gitlab_http_status(:bad_request) end it 'accepts the update date to be set' do update_time = 2.weeks.ago - put api("/projects/#{project.id}/issues/#{issue.iid}", owner), - params: { title: 'some new title', updated_at: update_time } + put api_for_owner, params: { title: 'some new title', updated_at: update_time } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['title']).to include 'some new title' - + expect(json_response['title']).to eq('some new title') expect(Time.parse(json_response['updated_at'])).to be_like_time(update_time) end end @@ -456,7 +453,7 @@ describe API::Issues do it 'creates a new project issue' do due_date = 2.weeks.from_now.strftime('%Y-%m-%d') - put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: { due_date: due_date } + put api_for_user, params: { due_date: due_date } expect(response).to have_gitlab_http_status(:ok) expect(json_response['due_date']).to eq(due_date) diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index bd8aeb65d3f..18b5c00d64f 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -271,6 +271,178 @@ describe API::Jobs do end end + describe 'GET /projects/:id/pipelines/:pipeline_id/bridges' do + let!(:bridge) { create(:ci_bridge, pipeline: pipeline) } + let(:downstream_pipeline) { create(:ci_pipeline) } + + let!(:pipeline_source) do + create(:ci_sources_pipeline, + source_pipeline: pipeline, + source_project: project, + source_job: bridge, + pipeline: downstream_pipeline, + project: downstream_pipeline.project) + end + + let(:query) { Hash.new } + + before do |example| + unless example.metadata[:skip_before_request] + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + end + end + + context 'authorized user' do + it 'returns pipeline bridges' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + end + + it 'returns correct values' do + expect(json_response).not_to be_empty + expect(json_response.first['commit']['id']).to eq project.commit.id + expect(json_response.first['id']).to eq bridge.id + expect(json_response.first['name']).to eq bridge.name + expect(json_response.first['stage']).to eq bridge.stage + end + + it 'returns pipeline data' do + json_bridge = json_response.first + + expect(json_bridge['pipeline']).not_to be_empty + expect(json_bridge['pipeline']['id']).to eq bridge.pipeline.id + expect(json_bridge['pipeline']['ref']).to eq bridge.pipeline.ref + expect(json_bridge['pipeline']['sha']).to eq bridge.pipeline.sha + expect(json_bridge['pipeline']['status']).to eq bridge.pipeline.status + end + + it 'returns downstream pipeline data' do + json_bridge = json_response.first + + expect(json_bridge['downstream_pipeline']).not_to be_empty + expect(json_bridge['downstream_pipeline']['id']).to eq downstream_pipeline.id + expect(json_bridge['downstream_pipeline']['ref']).to eq downstream_pipeline.ref + expect(json_bridge['downstream_pipeline']['sha']).to eq downstream_pipeline.sha + expect(json_bridge['downstream_pipeline']['status']).to eq downstream_pipeline.status + end + + context 'filter bridges' do + before do + create_bridge(pipeline, :pending) + create_bridge(pipeline, :running) + end + + context 'with one scope element' do + let(:query) { { 'scope' => 'pending' } } + + it :skip_before_request do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.count).to eq 1 + expect(json_response.first["status"]).to eq "pending" + end + end + + context 'with array of scope elements' do + let(:query) { { scope: %w(pending running) } } + + it :skip_before_request do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.count).to eq 2 + json_response.each { |r| expect(%w(pending running).include?(r['status'])).to be true } + end + end + end + + context 'respond 400 when scope contains invalid state' do + let(:query) { { scope: %w(unknown running) } } + + it { expect(response).to have_gitlab_http_status(:bad_request) } + end + + context 'bridges in different pipelines' do + let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } + let!(:bridge2) { create(:ci_bridge, pipeline: pipeline2) } + + it 'excludes bridges from other pipelines' do + json_response.each { |bridge| expect(bridge['pipeline']['id']).to eq(pipeline.id) } + end + end + + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + end.count + + 3.times { create_bridge(pipeline) } + + expect do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + end.not_to exceed_all_query_limit(control_count) + end + end + + context 'unauthorized user' do + context 'when user is not logged in' do + let(:api_user) { nil } + + it 'does not return bridges' do + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when user is guest' do + let(:api_user) { guest } + + it 'does not return bridges' do + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user has no read access for pipeline' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(api_user, :read_pipeline, pipeline).and_return(false) + end + + it 'does not return bridges' do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user) + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user has no read_build access for project' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(api_user, :read_build, project).and_return(false) + end + + it 'does not return bridges' do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user) + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + def create_bridge(pipeline, status = :created) + create(:ci_bridge, status: status, pipeline: pipeline).tap do |bridge| + downstream_pipeline = create(:ci_pipeline) + create(:ci_sources_pipeline, + source_pipeline: pipeline, + source_project: pipeline.project, + source_job: bridge, + pipeline: downstream_pipeline, + project: downstream_pipeline.project) + end + end + end + describe 'GET /projects/:id/jobs/:job_id' do before do |example| unless example.metadata[:skip_before_request] diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 1f17359a473..697f22e5f29 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -17,7 +17,7 @@ describe API::Labels do let(:user) { create(:user) } let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } - let!(:label1) { create(:label, title: 'label1', project: project) } + let!(:label1) { create(:label, description: 'the best label', title: 'label1', project: project) } let!(:priority_label) { create(:label, title: 'bug', project: project, priority: 3) } route_types = [:deprecated, :rest] @@ -219,7 +219,7 @@ describe API::Labels do 'closed_issues_count' => 1, 'open_merge_requests_count' => 0, 'name' => label1.name, - 'description' => nil, + 'description' => 'the best label', 'color' => a_string_matching(/^#\h{6}$/), 'text_color' => a_string_matching(/^#\h{6}$/), 'priority' => nil, diff --git a/spec/requests/api/lsif_data_spec.rb b/spec/requests/api/lsif_data_spec.rb deleted file mode 100644 index a1516046e3e..00000000000 --- a/spec/requests/api/lsif_data_spec.rb +++ /dev/null @@ -1,95 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -describe API::LsifData do - let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :repository) } - - let(:commit) { project.commit } - - describe 'GET lsif/info' do - subject do - endpoint_path = "/projects/#{project.id}/commits/#{commit.id}/lsif/info" - - get api(endpoint_path, user), params: { paths: ['main.go', 'morestrings/reverse.go'] } - - response - end - - context 'user does not have access to the project' do - before do - project.add_guest(user) - end - - it { is_expected.to have_gitlab_http_status(:forbidden) } - end - - context 'user has access to the project' do - before do - project.add_reporter(user) - end - - context 'there is no job artifact for the passed commit' do - it { is_expected.to have_gitlab_http_status(:not_found) } - end - - context 'lsif data is stored as a job artifact' do - let!(:pipeline) { create(:ci_pipeline, project: project, sha: commit.id) } - let!(:artifact) { create(:ci_job_artifact, :lsif, job: create(:ci_build, pipeline: pipeline)) } - - context 'code_navigation feature is disabled' do - before do - stub_feature_flags(code_navigation: false) - end - - it { is_expected.to have_gitlab_http_status(:not_found) } - end - - it 'returns code navigation info for a given path', :aggregate_failures do - expect(subject).to have_gitlab_http_status(:ok) - - data_for_main = response.parsed_body['main.go'] - expect(data_for_main.last).to eq({ - 'end_char' => 18, - 'end_line' => 8, - 'start_char' => 13, - 'start_line' => 8, - 'definition_url' => project_blob_path(project, "#{commit.id}/morestrings/reverse.go", anchor: 'L5'), - 'hover' => [{ - 'language' => 'go', - 'value' => Gitlab::Highlight.highlight(nil, 'func Func2(i int) string', language: 'go') - }] - }) - - data_for_reverse = response.parsed_body['morestrings/reverse.go'] - expect(data_for_reverse.last).to eq({ - 'end_char' => 9, - 'end_line' => 7, - 'start_char' => 8, - 'start_line' => 7, - 'definition_url' => project_blob_path(project, "#{commit.id}/morestrings/reverse.go", anchor: 'L6'), - 'hover' => [{ - 'language' => 'go', - 'value' => Gitlab::Highlight.highlight(nil, 'var b string', language: 'go') - }] - }) - end - - context 'the stored file is too large' do - before do - allow_any_instance_of(JobArtifactUploader).to receive(:cached_size).and_return(20.megabytes) - end - - it { is_expected.to have_gitlab_http_status(:payload_too_large) } - end - - context 'the user does not have access to the pipeline' do - let(:project) { create(:project, :repository, builds_access_level: ProjectFeature::DISABLED) } - - it { is_expected.to have_gitlab_http_status(:forbidden) } - end - end - end - end -end diff --git a/spec/requests/api/markdown_spec.rb b/spec/requests/api/markdown_spec.rb index 09342b06744..53e43430b1f 100644 --- a/spec/requests/api/markdown_spec.rb +++ b/spec/requests/api/markdown_spec.rb @@ -52,6 +52,7 @@ describe API::Markdown do context "when arguments are valid" do let_it_be(:project) { create(:project) } let_it_be(:issue) { create(:issue, project: project) } + let(:issue_url) { "http://#{Gitlab.config.gitlab.host}/#{issue.project.namespace.path}/#{issue.project.path}/-/issues/#{issue.iid}" } let(:text) { ":tada: Hello world! :100: #{issue.to_reference}" } context "when not using gfm" do @@ -88,7 +89,7 @@ describe API::Markdown do .and include('data-name="tada"') .and include('data-name="100"') .and include("#1") - .and exclude("<a href=\"#{IssuesHelper.url_for_issue(issue.iid, project)}\"") + .and exclude("<a href=\"#{issue_url}\"") .and exclude("#1</a>") end end @@ -104,16 +105,16 @@ describe API::Markdown do expect(json_response["html"]).to include("Hello world!") .and include('data-name="tada"') .and include('data-name="100"') - .and include("<a href=\"#{IssuesHelper.url_for_issue(issue.iid, project)}\"") + .and include("<a href=\"#{issue_url}\"") .and include("#1</a>") end end context 'with a public project and confidential issue' do - let(:public_project) { create(:project, :public) } - let(:confidential_issue) { create(:issue, :confidential, project: public_project, title: 'Confidential title') } + let(:public_project) { create(:project, :public) } + let(:issue) { create(:issue, :confidential, project: public_project, title: 'Confidential title') } - let(:text) { ":tada: Hello world! :100: #{confidential_issue.to_reference}" } + let(:text) { ":tada: Hello world! :100: #{issue.to_reference}" } let(:params) { { text: text, gfm: true, project: public_project.full_path } } shared_examples 'user without proper access' do @@ -141,7 +142,7 @@ describe API::Markdown do end context 'when logged in as author' do - let(:user) { confidential_issue.author } + let(:user) { issue.author } it 'renders the title or link' do expect(response).to have_gitlab_http_status(:created) @@ -149,7 +150,7 @@ describe API::Markdown do expect(json_response["html"]).to include('Hello world!') .and include('data-name="tada"') .and include('data-name="100"') - .and include("<a href=\"#{IssuesHelper.url_for_issue(confidential_issue.iid, public_project)}\"") + .and include("<a href=\"#{issue_url}\"") .and include("#1</a>") end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 14b22de9661..7a0077f853a 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1654,6 +1654,12 @@ describe API::MergeRequests do end end + describe 'PUT /projects/:id/merge_reuests/:merge_request_iid' do + it_behaves_like 'issuable update endpoint' do + let(:entity) { merge_request } + end + end + describe "POST /projects/:id/merge_requests/:merge_request_iid/context_commits" do let(:merge_request_iid) { merge_request.iid } let(:authenticated_user) { user } @@ -2023,6 +2029,34 @@ describe API::MergeRequests do end end + context "with a merge request that has force_remove_source_branch enabled" do + let(:source_repository) { merge_request.source_project.repository } + let(:source_branch) { merge_request.source_branch } + + before do + merge_request.update(merge_params: { 'force_remove_source_branch' => true }) + end + + it 'removes the source branch' do + put( + api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) + ) + + expect(response).to have_gitlab_http_status(:ok) + expect(source_repository.branch_exists?(source_branch)).to be_falsy + end + + it 'does not remove the source branch' do + put( + api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), + params: { should_remove_source_branch: false } + ) + + expect(response).to have_gitlab_http_status(:ok) + expect(source_repository.branch_exists?(source_branch)).to be_truthy + end + end + context "performing a ff-merge with squash" do let(:merge_request) { create(:merge_request, :rebased, source_project: project, squash: true) } diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index 80eae97f41a..5e775841f12 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -3,16 +3,22 @@ require 'spec_helper' describe 'OAuth tokens' do + include HttpBasicAuthHelpers + context 'Resource Owner Password Credentials' do - def request_oauth_token(user) - post '/oauth/token', params: { username: user.username, password: user.password, grant_type: 'password' } + def request_oauth_token(user, headers = {}) + post '/oauth/token', + params: { username: user.username, password: user.password, grant_type: 'password' }, + headers: headers end + let_it_be(:client) { create(:oauth_application) } + context 'when user has 2FA enabled' do it 'does not create an access token' do user = create(:user, :two_factor) - request_oauth_token(user) + request_oauth_token(user, client_basic_auth_header(client)) expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response['error']).to eq('invalid_grant') @@ -20,13 +26,46 @@ describe 'OAuth tokens' do end context 'when user does not have 2FA enabled' do - it 'creates an access token' do - user = create(:user) + # NOTE: using ROPS grant flow without client credentials will be deprecated + # and removed in the next version of Doorkeeper. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/219137 + context 'when no client credentials provided' do + it 'creates an access token' do + user = create(:user) + + request_oauth_token(user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['access_token']).not_to be_nil + end + end + + context 'when client credentials provided' do + context 'with valid credentials' do + it 'creates an access token' do + user = create(:user) + + request_oauth_token(user, client_basic_auth_header(client)) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['access_token']).not_to be_nil + end + end + + context 'with invalid credentials' do + it 'does not create an access token' do + # NOTE: remove this after update to Doorkeeper 5.5 or newer, see + # https://gitlab.com/gitlab-org/gitlab/-/issues/219137 + pending 'Enable this example after upgrading Doorkeeper to 5.5 or newer' + + user = create(:user) - request_oauth_token(user) + request_oauth_token(user, basic_auth_header(client.uid, 'invalid secret')) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['access_token']).not_to be_nil + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['error']).to eq('invalid_client') + end + end end end @@ -40,7 +79,7 @@ describe 'OAuth tokens' do before do user.block - request_oauth_token(user) + request_oauth_token(user, client_basic_auth_header(client)) end include_examples 'does not create an access token' @@ -50,7 +89,7 @@ describe 'OAuth tokens' do before do user.ldap_block - request_oauth_token(user) + request_oauth_token(user, client_basic_auth_header(client)) end include_examples 'does not create an access token' @@ -60,7 +99,7 @@ describe 'OAuth tokens' do before do user.update!(confirmed_at: nil) - request_oauth_token(user) + request_oauth_token(user, client_basic_auth_header(client)) end include_examples 'does not create an access token' diff --git a/spec/requests/api/project_container_repositories_spec.rb b/spec/requests/api/project_container_repositories_spec.rb index 91905635c3f..471fc99117b 100644 --- a/spec/requests/api/project_container_repositories_spec.rb +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -223,6 +223,40 @@ describe API::ProjectContainerRepositories do expect(response).to have_gitlab_http_status(:accepted) end end + + context 'with invalid regex' do + let(:invalid_regex) { '*v10.' } + let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" } + + RSpec.shared_examples 'rejecting the invalid regex' do |param_name| + it 'does not enqueue a job' do + expect(CleanupContainerRepositoryWorker).not_to receive(:perform_async) + + subject + end + + it_behaves_like 'returning response status', :bad_request + + it 'returns an error message' do + subject + + expect(json_response['error']).to include("#{param_name} is an invalid regexp") + end + end + + before do + stub_last_activity_update + stub_exclusive_lease(lease_key, timeout: 1.hour) + end + + %i[name_regex_delete name_regex name_regex_keep].each do |param_name| + context "for #{param_name}" do + let(:params) { { param_name => invalid_regex } } + + it_behaves_like 'rejecting the invalid regex', param_name + end + end + end end end diff --git a/spec/requests/api/project_events_spec.rb b/spec/requests/api/project_events_spec.rb index 489b83de434..f65c62f9402 100644 --- a/spec/requests/api/project_events_spec.rb +++ b/spec/requests/api/project_events_spec.rb @@ -7,7 +7,7 @@ describe API::ProjectEvents do let(:non_member) { create(:user) } let(:private_project) { create(:project, :private, creator_id: user.id, namespace: user.namespace) } let(:closed_issue) { create(:closed_issue, project: private_project, author: user) } - let!(:closed_issue_event) { create(:event, project: private_project, author: user, target: closed_issue, action: Event::CLOSED, created_at: Date.new(2016, 12, 30)) } + let!(:closed_issue_event) { create(:event, project: private_project, author: user, target: closed_issue, action: :closed, created_at: Date.new(2016, 12, 30)) } describe 'GET /projects/:id/events' do context 'when unauthenticated ' do @@ -29,9 +29,9 @@ describe API::ProjectEvents do context 'with inaccessible events' do let(:public_project) { create(:project, :public, creator_id: user.id, namespace: user.namespace) } let(:confidential_issue) { create(:closed_issue, confidential: true, project: public_project, author: user) } - let!(:confidential_event) { create(:event, project: public_project, author: user, target: confidential_issue, action: Event::CLOSED) } + let!(:confidential_event) { create(:event, project: public_project, author: user, target: confidential_issue, action: :closed) } let(:public_issue) { create(:closed_issue, project: public_project, author: user) } - let!(:public_event) { create(:event, project: public_project, author: user, target: public_issue, action: Event::CLOSED) } + let!(:public_event) { create(:event, project: public_project, author: user, target: public_issue, action: :closed) } it 'returns only accessible events' do get api("/projects/#{public_project.id}/events", non_member) @@ -53,19 +53,19 @@ describe API::ProjectEvents do before do create(:event, + :closed, project: public_project, target: create(:issue, project: public_project, title: 'Issue 1'), - action: Event::CLOSED, created_at: Date.parse('2018-12-10')) create(:event, + :closed, project: public_project, target: create(:issue, confidential: true, project: public_project, title: 'Confidential event'), - action: Event::CLOSED, created_at: Date.parse('2018-12-11')) create(:event, + :closed, project: public_project, target: create(:issue, project: public_project, title: 'Issue 2'), - action: Event::CLOSED, created_at: Date.parse('2018-12-12')) end diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb index ad872b88664..58034322a13 100644 --- a/spec/requests/api/project_export_spec.rb +++ b/spec/requests/api/project_export_spec.rb @@ -44,19 +44,6 @@ describe API::ProjectExport, :clean_gitlab_redis_cache do it_behaves_like '404 response' end - shared_examples_for 'when rate limit is exceeded' do - before do - allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true) - end - - it 'prevents requesting project export' do - request - - expect(response).to have_gitlab_http_status(:too_many_requests) - expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.') - end - end - describe 'GET /projects/:project_id/export' do shared_examples_for 'get project export status not found' do it_behaves_like '404 response' do @@ -247,7 +234,18 @@ describe API::ProjectExport, :clean_gitlab_redis_cache do context 'when rate limit is exceeded' do let(:request) { get api(download_path, admin) } - include_examples 'when rate limit is exceeded' + before do + allow(Gitlab::ApplicationRateLimiter) + .to receive(:increment) + .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:project_download_export][:threshold] + 1) + end + + it 'prevents requesting project export' do + request + + expect(response).to have_gitlab_http_status(:too_many_requests) + expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.') + end end end @@ -360,10 +358,19 @@ describe API::ProjectExport, :clean_gitlab_redis_cache do it_behaves_like 'post project export start' - context 'when rate limit is exceeded' do - let(:request) { post api(path, admin) } + context 'when rate limit is exceeded across projects' do + before do + allow(Gitlab::ApplicationRateLimiter) + .to receive(:increment) + .and_return(Gitlab::ApplicationRateLimiter.rate_limits[:project_export][:threshold] + 1) + end + + it 'prevents requesting project export' do + post api(path, admin) - include_examples 'when rate limit is exceeded' + expect(response).to have_gitlab_http_status(:too_many_requests) + expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.') + end end end diff --git a/spec/requests/api/project_repository_storage_moves_spec.rb b/spec/requests/api/project_repository_storage_moves_spec.rb index 7ceea0178f3..40966e31d0d 100644 --- a/spec/requests/api/project_repository_storage_moves_spec.rb +++ b/spec/requests/api/project_repository_storage_moves_spec.rb @@ -5,12 +5,45 @@ require 'spec_helper' describe API::ProjectRepositoryStorageMoves do include AccessMatchersForRequest - let(:user) { create(:admin) } - let!(:storage_move) { create(:project_repository_storage_move, :scheduled) } + let_it_be(:user) { create(:admin) } + let_it_be(:project) { create(:project) } + let_it_be(:storage_move) { create(:project_repository_storage_move, :scheduled, project: project) } - describe 'GET /project_repository_storage_moves' do + shared_examples 'get single project repository storage move' do + let(:project_repository_storage_move_id) { storage_move.id } + + def get_project_repository_storage_move + get api(url, 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 + + shared_examples 'get project repository storage move list' do def get_project_repository_storage_moves - get api('/project_repository_storage_moves', user) + get api(url, user) end it 'returns project repository storage moves' do @@ -30,16 +63,16 @@ describe API::ProjectRepositoryStorageMoves do control = ActiveRecord::QueryRecorder.new { get_project_repository_storage_moves } - create(:project_repository_storage_move, :scheduled) + create(:project_repository_storage_move, :scheduled, project: project) 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) + storage_move_oldest = create(:project_repository_storage_move, :scheduled, project: project, created_at: 2.days.ago) + storage_move_middle = create(:project_repository_storage_move, :scheduled, project: project, created_at: 1.day.ago) - get api('/project_repository_storage_moves', user) + get_project_repository_storage_moves json_ids = json_response.map {|storage_move| storage_move['id'] } expect(json_ids).to eq([ @@ -55,35 +88,68 @@ describe API::ProjectRepositoryStorageMoves do end end - describe 'GET /project_repository_storage_moves/:id' do - let(:project_repository_storage_move_id) { storage_move.id } + describe 'GET /project_repository_storage_moves' do + it_behaves_like 'get project repository storage move list' do + let(:url) { '/project_repository_storage_moves' } + end + end - def get_project_repository_storage_move - get api("/project_repository_storage_moves/#{project_repository_storage_move_id}", user) + describe 'GET /project_repository_storage_moves/:repository_storage_move_id' do + it_behaves_like 'get single project repository storage move' do + let(:url) { "/project_repository_storage_moves/#{project_repository_storage_move_id}" } end + end - it 'returns a project repository storage move' do - get_project_repository_storage_move + describe 'GET /projects/:id/repository_storage_moves' do + it_behaves_like 'get project repository storage move list' do + let(:url) { "/projects/#{project.id}/repository_storage_moves" } + end + end - 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) + describe 'GET /projects/:id/repository_storage_moves/:repository_storage_move_id' do + it_behaves_like 'get single project repository storage move' do + let(:url) { "/projects/#{project.id}/repository_storage_moves/#{project_repository_storage_move_id}" } end + end - context 'non-existent project repository storage move' do - let(:project_repository_storage_move_id) { non_existing_record_id } + describe 'POST /projects/:id/repository_storage_moves' do + let(:url) { "/projects/#{project.id}/repository_storage_moves" } + let(:destination_storage_name) { 'test_second_storage' } - it 'returns not found' do - get_project_repository_storage_move + def create_project_repository_storage_move + post api(url, user), params: { destination_storage_name: destination_storage_name } + end - expect(response).to have_gitlab_http_status(:not_found) - end + before do + stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' }) + end + + it 'schedules a project repository storage move' do + create_project_repository_storage_move + + storage_move = project.repository_storage_moves.last + + expect(response).to have_gitlab_http_status(:created) + 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('scheduled') + expect(json_response['source_storage_name']).to eq('default') + expect(json_response['destination_storage_name']).to eq(destination_storage_name) 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) } + it { expect { create_project_repository_storage_move }.to be_allowed_for(:admin) } + it { expect { create_project_repository_storage_move }.to be_denied_for(:user) } + end + + context 'destination_storage_name is missing' do + let(:destination_storage_name) { nil } + + it 'returns a validation error' do + create_project_repository_storage_move + + expect(response).to have_gitlab_http_status(:bad_request) + end end end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 3abcf1cb7ed..c3f29ec47a9 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -597,6 +597,10 @@ describe API::Projects do expect(response.header).to include('Links') expect(response.header['Links']).to include('pagination=keyset') expect(response.header['Links']).to include("id_after=#{public_project.id}") + + expect(response.header).to include('Link') + expect(response.header['Link']).to include('pagination=keyset') + expect(response.header['Link']).to include("id_after=#{public_project.id}") end it 'contains only the first project with per_page = 1' do @@ -613,12 +617,17 @@ describe API::Projects do expect(response.header).to include('Links') expect(response.header['Links']).to include('pagination=keyset') expect(response.header['Links']).to include("id_after=#{project3.id}") + + expect(response.header).to include('Link') + expect(response.header['Link']).to include('pagination=keyset') + expect(response.header['Link']).to include("id_after=#{project3.id}") end it 'does not include a next link when the page does not have any records' do get api('/projects', current_user), params: params.merge(id_after: Project.maximum(:id)) expect(response.header).not_to include('Links') + expect(response.header).not_to include('Link') end it 'returns an empty array when the page does not have any records' do @@ -644,6 +653,10 @@ describe API::Projects do expect(response.header).to include('Links') expect(response.header['Links']).to include('pagination=keyset') expect(response.header['Links']).to include("id_before=#{project3.id}") + + expect(response.header).to include('Link') + expect(response.header['Link']).to include('pagination=keyset') + expect(response.header['Link']).to include("id_before=#{project3.id}") end it 'contains only the last project with per_page = 1' do @@ -672,6 +685,11 @@ describe API::Projects do match[1] end + link = response.header['Link'] + url = link&.match(/<[^>]+(\/projects\?[^>]+)>; rel="next"/) do |match| + match[1] + end + ids += Gitlab::Json.parse(response.body).map { |p| p['id'] } end @@ -746,7 +764,8 @@ describe API::Projects do resolve_outdated_diff_discussions: false, remove_source_branch_after_merge: true, autoclose_referenced_issues: true, - only_allow_merge_if_pipeline_succeeds: false, + only_allow_merge_if_pipeline_succeeds: true, + allow_merge_on_skipped_pipeline: true, request_access_enabled: true, only_allow_merge_if_all_discussions_are_resolved: false, ci_config_path: 'a/custom/path', @@ -894,6 +913,22 @@ describe API::Projects do expect(json_response['only_allow_merge_if_pipeline_succeeds']).to be_truthy end + it 'sets a project as not allowing merge when pipeline is skipped' do + project_params = attributes_for(:project, allow_merge_on_skipped_pipeline: false) + + post api('/projects', user), params: project_params + + expect(json_response['allow_merge_on_skipped_pipeline']).to be_falsey + end + + it 'sets a project as allowing merge when pipeline is skipped' do + project_params = attributes_for(:project, allow_merge_on_skipped_pipeline: true) + + post api('/projects', user), params: project_params + + expect(json_response['allow_merge_on_skipped_pipeline']).to be_truthy + end + it 'sets a project as allowing merge even if discussions are unresolved' do project = attributes_for(:project, only_allow_merge_if_all_discussions_are_resolved: false) @@ -1227,16 +1262,36 @@ describe API::Projects do it 'sets a project as allowing merge even if build fails' do project = attributes_for(:project, only_allow_merge_if_pipeline_succeeds: false) + post api("/projects/user/#{user.id}", admin), params: project + expect(json_response['only_allow_merge_if_pipeline_succeeds']).to be_falsey end it 'sets a project as allowing merge only if pipeline succeeds' do project = attributes_for(:project, only_allow_merge_if_pipeline_succeeds: true) + post api("/projects/user/#{user.id}", admin), params: project + expect(json_response['only_allow_merge_if_pipeline_succeeds']).to be_truthy end + it 'sets a project as not allowing merge when pipeline is skipped' do + project = attributes_for(:project, allow_merge_on_skipped_pipeline: false) + + post api("/projects/user/#{user.id}", admin), params: project + + expect(json_response['allow_merge_on_skipped_pipeline']).to be_falsey + end + + it 'sets a project as allowing merge when pipeline is skipped' do + project = attributes_for(:project, allow_merge_on_skipped_pipeline: true) + + post api("/projects/user/#{user.id}", admin), params: project + + expect(json_response['allow_merge_on_skipped_pipeline']).to be_truthy + end + it 'sets a project as allowing merge even if discussions are unresolved' do project = attributes_for(:project, only_allow_merge_if_all_discussions_are_resolved: false) @@ -1376,6 +1431,7 @@ describe API::Projects do expect(json_response['shared_with_groups'][0]['group_name']).to eq(group.name) expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access) expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds) + expect(json_response['allow_merge_on_skipped_pipeline']).to eq(project.allow_merge_on_skipped_pipeline) expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) end end @@ -1443,6 +1499,7 @@ describe API::Projects do expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access) expect(json_response['shared_with_groups'][0]).to have_key('expires_at') expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds) + expect(json_response['allow_merge_on_skipped_pipeline']).to eq(project.allow_merge_on_skipped_pipeline) expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) expect(json_response['ci_default_git_depth']).to eq(project.ci_default_git_depth) expect(json_response['merge_method']).to eq(project.merge_method.to_s) @@ -2055,10 +2112,12 @@ describe API::Projects do end describe "POST /projects/:id/share" do - let(:group) { create(:group) } + let_it_be(:group) { create(:group, :private) } + let_it_be(:group_user) { create(:user) } before do group.add_developer(user) + group.add_developer(group_user) end it "shares project with group" do @@ -2074,6 +2133,14 @@ describe API::Projects do expect(json_response['expires_at']).to eq(expires_at.to_s) end + it 'updates project authorization' do + expect do + post api("/projects/#{project.id}/share", user), params: { group_id: group.id, group_access: Gitlab::Access::DEVELOPER } + end.to( + change { group_user.can?(:read_project, project) }.from(false).to(true) + ) + end + it "returns a 400 error when group id is not given" do post api("/projects/#{project.id}/share", user), params: { group_access: Gitlab::Access::DEVELOPER } expect(response).to have_gitlab_http_status(:bad_request) @@ -2123,9 +2190,12 @@ describe API::Projects do describe 'DELETE /projects/:id/share/:group_id' do context 'for a valid group' do - let(:group) { create(:group, :public) } + let_it_be(:group) { create(:group, :private) } + let_it_be(:group_user) { create(:user) } before do + group.add_developer(group_user) + create(:project_group_link, group: group, project: project) end @@ -2136,6 +2206,14 @@ describe API::Projects do expect(project.project_group_links).to be_empty end + it 'updates project authorization' do + expect do + delete api("/projects/#{project.id}/share/#{group.id}", user) + end.to( + change { group_user.can?(:read_project, project) }.from(true).to(false) + ) + end + it_behaves_like '412 response' do let(:request) { api("/projects/#{project.id}/share/#{group.id}", user) } end @@ -2438,6 +2516,21 @@ describe API::Projects do expect(json_response['container_expiration_policy']['keep_n']).to eq(1) expect(json_response['container_expiration_policy']['name_regex_keep']).to eq('foo.*') end + + it "doesn't update container_expiration_policy with invalid regex" do + project_param = { + container_expiration_policy_attributes: { + cadence: '1month', + keep_n: 1, + name_regex_keep: '[' + } + } + + put api("/projects/#{project3.id}", user4), params: project_param + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['container_expiration_policy.name_regex_keep']).to contain_exactly('not valid RE2 syntax: missing ]: [') + end end context 'when authenticated as project developer' do @@ -2477,11 +2570,11 @@ describe API::Projects do let(:admin) { create(:admin) } - it 'returns 500 when repository storage is unknown' do + it 'returns 400 when repository storage is unknown' do put(api("/projects/#{new_project.id}", admin), params: { repository_storage: unknown_storage }) - expect(response).to have_gitlab_http_status(:internal_server_error) - expect(json_response['message']).to match('ArgumentError') + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['repository_storage_moves']).to eq(['is invalid']) end it 'returns 200 when repository storage has changed' do diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 237782a681c..f4cb7f25990 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -10,7 +10,6 @@ describe API::Releases do let(:guest) { create(:user) } let(:non_project_member) { create(:user) } let(:commit) { create(:commit, project: project) } - let(:last_release) { project.releases.last } before do project.add_maintainer(maintainer) @@ -733,109 +732,6 @@ describe API::Releases do expect(response).to have_gitlab_http_status(:conflict) end end - - context 'Evidence collection' do - let(:params) do - { - name: 'New release', - tag_name: 'v0.1', - description: 'Super nice release', - released_at: released_at - }.compact - end - - around do |example| - Timecop.freeze { example.run } - end - - subject do - post api("/projects/#{project.id}/releases", maintainer), params: params - end - - context 'historical release' do - let(:released_at) { 3.weeks.ago } - - it 'does not execute CreateEvidenceWorker' do - expect { subject }.not_to change(CreateEvidenceWorker.jobs, :size) - end - - it 'does not create an Evidence object', :sidekiq_inline do - expect { subject }.not_to change(Releases::Evidence, :count) - end - - it 'is a historical release' do - subject - - expect(last_release.historical_release?).to be_truthy - end - - it 'is not an upcoming release' do - subject - - expect(last_release.upcoming_release?).to be_falsy - end - end - - context 'immediate release' do - let(:released_at) { nil } - - it 'sets `released_at` to the current dttm' do - subject - - expect(last_release.updated_at).to be_like_time(Time.now) - end - - it 'queues CreateEvidenceWorker' do - expect { subject }.to change(CreateEvidenceWorker.jobs, :size).by(1) - end - - it 'creates Evidence', :sidekiq_inline do - expect { subject }.to change(Releases::Evidence, :count).by(1) - end - - it 'is not a historical release' do - subject - - expect(last_release.historical_release?).to be_falsy - end - - it 'is not an upcoming release' do - subject - - expect(last_release.upcoming_release?).to be_falsy - end - end - - context 'upcoming release' do - let(:released_at) { 1.day.from_now } - - it 'queues CreateEvidenceWorker' do - expect { subject }.to change(CreateEvidenceWorker.jobs, :size).by(1) - end - - it 'queues CreateEvidenceWorker at the released_at timestamp' do - subject - - expect(CreateEvidenceWorker.jobs.last['at']).to eq(released_at.to_i) - end - - it 'creates Evidence', :sidekiq_inline do - expect { subject }.to change(Releases::Evidence, :count).by(1) - end - - it 'is not a historical release' do - subject - - expect(last_release.historical_release?).to be_falsy - end - - it 'is an upcoming release' do - subject - - expect(last_release.upcoming_release?).to be_truthy - end - end - end end describe 'PUT /projects/:id/releases/:tag_name' do diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 0c66bfd6c4d..55243e83017 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -177,6 +177,12 @@ describe API::Repositories do expect(headers['Content-Disposition']).to eq 'inline' end + it_behaves_like 'uncached response' do + before do + get api(route, current_user) + end + end + context 'when sha does not exist' do it_behaves_like '404 response' do let(:request) { get api(route.sub(sample_blob.oid, 'abcd9876'), current_user) } @@ -278,7 +284,7 @@ describe API::Repositories do context "when hotlinking detection is enabled" do before do - Feature.enable(:repository_archive_hotlinking_interception) + stub_feature_flags(repository_archive_hotlinking_interception: true) end it_behaves_like "hotlink interceptor" do diff --git a/spec/requests/api/resource_milestone_events_spec.rb b/spec/requests/api/resource_milestone_events_spec.rb new file mode 100644 index 00000000000..b2e92fde5ee --- /dev/null +++ b/spec/requests/api/resource_milestone_events_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::ResourceMilestoneEvents do + let!(:user) { create(:user) } + let!(:project) { create(:project, :public, namespace: user.namespace) } + let!(:milestone) { create(:milestone, project: project) } + + before do + project.add_developer(user) + end + + context 'when eventable is an Issue' do + it_behaves_like 'resource_milestone_events API', 'projects', 'issues', 'iid' do + let(:parent) { project } + let(:eventable) { create(:issue, project: project, author: user) } + end + end + + context 'when eventable is a Merge Request' do + it_behaves_like 'resource_milestone_events API', 'projects', 'merge_requests', 'iid' do + let(:parent) { project } + let(:eventable) { create(:merge_request, source_project: project, target_project: project, author: user) } + end + end +end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 7284f33f3af..774615757b9 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -648,6 +648,44 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end + context 'when job is for a release' do + let!(:job) { create(:ci_build, :release_options, pipeline: pipeline) } + + context 'when `release_steps` is passed by the runner' do + it 'exposes release info' do + request_job info: { features: { release_steps: true } } + + expect(response).to have_gitlab_http_status(:created) + expect(response.headers).not_to have_key('X-GitLab-Last-Update') + expect(json_response['steps']).to eq([ + { + "name" => "script", + "script" => ["make changelog | tee release_changelog.txt"], + "timeout" => 3600, + "when" => "on_success", + "allow_failure" => false + }, + { + "name" => "release", + "script" => + "release-cli create --ref \"$CI_COMMIT_SHA\" --name \"Release $CI_COMMIT_SHA\" --tag-name \"release-$CI_COMMIT_SHA\" --description \"Created using the release-cli $EXTRA_DESCRIPTION\"", + "timeout" => 3600, + "when" => "on_success", + "allow_failure" => false + } + ]) + end + end + + context 'when `release_steps` is not passed by the runner' do + it 'drops the job' do + request_job + + expect(response).to have_gitlab_http_status(:no_content) + end + end + end + context 'when job is made for merge request' do let(:pipeline) { create(:ci_pipeline, source: :merge_request_event, project: project, ref: 'feature', merge_request: merge_request) } let!(:job) { create(:ci_build, pipeline: pipeline, name: 'spinach', ref: 'feature', stage: 'test', stage_idx: 0) } @@ -1055,6 +1093,65 @@ describe API::Runner, :clean_gitlab_redis_shared_state do post api('/jobs/request'), params: new_params, headers: { 'User-Agent' => user_agent } end end + + context 'for web-ide job' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository) } + + let(:runner) { create(:ci_runner, :project, projects: [project]) } + let(:service) { Ci::CreateWebIdeTerminalService.new(project, user, ref: 'master').execute } + let(:pipeline) { service[:pipeline] } + let(:build) { pipeline.builds.first } + let(:job) { {} } + let(:config_content) do + 'terminal: { image: ruby, services: [mysql], before_script: [ls], tags: [tag-1], variables: { KEY: value } }' + end + + before do + stub_webide_config_file(config_content) + project.add_maintainer(user) + + pipeline + end + + context 'when runner has matching tag' do + before do + runner.update!(tag_list: ['tag-1']) + end + + it 'successfully picks job' do + request_job + + build.reload + + expect(build).to be_running + expect(build.runner).to eq(runner) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to include( + "id" => build.id, + "variables" => include("key" => 'KEY', "value" => 'value', "public" => true, "masked" => false), + "image" => a_hash_including("name" => 'ruby'), + "services" => all(a_hash_including("name" => 'mysql')), + "job_info" => a_hash_including("name" => 'terminal', "stage" => 'terminal')) + end + end + + context 'when runner does not have matching tags' do + it 'does not pick a job' do + request_job + + build.reload + + expect(build).to be_pending + expect(response).to have_gitlab_http_status(:no_content) + end + end + + def request_job(token = runner.token, **params) + post api('/jobs/request'), params: params.merge(token: token) + end + end end describe 'PUT /api/v4/jobs/:id' do @@ -1070,6 +1167,10 @@ describe API::Runner, :clean_gitlab_redis_shared_state do let(:send_request) { update_job(state: 'success') } end + it 'updates runner info' do + expect { update_job(state: 'success') }.to change { runner.reload.contacted_at } + end + context 'when status is given' do it 'mark job as succeeded' do update_job(state: 'success') @@ -1235,6 +1336,12 @@ describe API::Runner, :clean_gitlab_redis_shared_state do let(:send_request) { patch_the_trace } end + it 'updates runner info' do + runner.update!(contacted_at: 1.year.ago) + + expect { patch_the_trace }.to change { runner.reload.contacted_at } + end + context 'when request is valid' do it 'gets correct response' do expect(response).to have_gitlab_http_status(:accepted) @@ -1496,6 +1603,10 @@ describe API::Runner, :clean_gitlab_redis_shared_state do let(:send_request) { subject } end + it 'updates runner info' do + expect { subject }.to change { runner.reload.contacted_at } + end + shared_examples 'authorizes local file' do it 'succeeds' do subject @@ -1634,6 +1745,35 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end + context 'authorize uploading of an lsif artifact' do + before do + stub_feature_flags(code_navigation: job.project) + end + + it 'adds ProcessLsif header' do + authorize_artifacts_with_token_in_headers(artifact_type: :lsif) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['ProcessLsif']).to be_truthy + end + + it 'fails to authorize too large artifact' do + authorize_artifacts_with_token_in_headers(artifact_type: :lsif, filesize: 30.megabytes) + + expect(response).to have_gitlab_http_status(:payload_too_large) + end + + context 'code_navigation feature flag is disabled' do + it 'does not add ProcessLsif header' do + stub_feature_flags(code_navigation: false) + + authorize_artifacts_with_token_in_headers(artifact_type: :lsif) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + def authorize_artifacts(params = {}, request_headers = headers) post api("/jobs/#{job.id}/artifacts/authorize"), params: params, headers: request_headers end @@ -1655,6 +1795,10 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end + it 'updates runner info' do + expect { upload_artifacts(file_upload, headers_with_token) }.to change { runner.reload.contacted_at } + end + context 'when artifacts are being stored inside of tmp path' do before do # by configuring this path we allow to pass temp file from any path @@ -2140,6 +2284,10 @@ describe API::Runner, :clean_gitlab_redis_shared_state do let(:send_request) { download_artifact } end + it 'updates runner info' do + expect { download_artifact }.to change { runner.reload.contacted_at } + end + context 'when job has artifacts' do let(:job) { create(:ci_build) } let(:store) { JobArtifactUploader::Store::LOCAL } @@ -2207,7 +2355,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end - context 'when job does not has artifacts' do + context 'when job does not have artifacts' do it 'responds with not found' do download_artifact diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 261e54da6a8..67c258260bf 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -326,32 +326,6 @@ 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 diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 3894e0bf2d1..a02d804ee9b 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -15,6 +15,14 @@ describe API::Search do it { expect(json_response.size).to eq(size) } end + shared_examples 'ping counters' do |scope:, search: ''| + it 'increases usage ping searches counter' do + expect(Gitlab::UsageDataCounters::SearchCounter).to receive(:count).with(:all_searches) + + get api(endpoint, user), params: { scope: scope, search: search } + end + end + 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 } @@ -75,6 +83,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/projects' it_behaves_like 'pagination', scope: :projects + + it_behaves_like 'ping counters', scope: :projects end context 'for issues scope' do @@ -86,6 +96,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/issues' + it_behaves_like 'ping counters', scope: :issues + describe 'pagination' do before do create(:issue, project: project, title: 'another issue') @@ -104,6 +116,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests' + it_behaves_like 'ping counters', scope: :merge_requests + describe 'pagination' do before do create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch') @@ -125,6 +139,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + it_behaves_like 'ping counters', scope: :milestones + describe 'pagination' do before do create(:milestone, project: project, title: 'another milestone') @@ -161,6 +177,8 @@ describe API::Search do it_behaves_like 'pagination', scope: :users + it_behaves_like 'ping counters', scope: :users + context 'when users search feature is disabled' do before do stub_feature_flags(users_search: false) @@ -183,6 +201,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/snippets' + it_behaves_like 'ping counters', scope: :snippet_titles + describe 'pagination' do before do create(:snippet, :public, title: 'another snippet', content: 'snippet content') @@ -248,6 +268,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/projects' it_behaves_like 'pagination', scope: :projects + + it_behaves_like 'ping counters', scope: :projects end context 'for issues scope' do @@ -259,6 +281,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/issues' + it_behaves_like 'ping counters', scope: :issues + describe 'pagination' do before do create(:issue, project: project, title: 'another issue') @@ -277,6 +301,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests' + it_behaves_like 'ping counters', scope: :merge_requests + describe 'pagination' do before do create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch') @@ -295,6 +321,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + it_behaves_like 'ping counters', scope: :milestones + describe 'pagination' do before do create(:milestone, project: project, title: 'another milestone') @@ -326,6 +354,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics' + it_behaves_like 'ping counters', scope: :users + describe 'pagination' do before do create(:group_member, :developer, group: group) @@ -395,7 +425,7 @@ describe API::Search do end end - context 'when user does can not see the project' do + context 'when user can not see the project' do it 'returns 404 error' do project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) @@ -415,6 +445,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/issues' + it_behaves_like 'ping counters', scope: :issues + describe 'pagination' do before do create(:issue, project: project, title: 'another issue') @@ -435,6 +467,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/merge_requests' + it_behaves_like 'ping counters', scope: :merge_requests + describe 'pagination' do before do create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch') @@ -456,6 +490,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + it_behaves_like 'ping counters', scope: :milestones + describe 'pagination' do before do create(:milestone, project: project, title: 'another milestone') @@ -491,6 +527,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/user/basics' + it_behaves_like 'ping counters', scope: :users + describe 'pagination' do before do create(:project_member, :developer, project: project) @@ -521,6 +559,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/notes' + it_behaves_like 'ping counters', scope: :notes + describe 'pagination' do before do mr = create(:merge_request, source_project: project, target_branch: 'another_branch') @@ -542,6 +582,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/blobs' + it_behaves_like 'ping counters', scope: :wiki_blobs + describe 'pagination' do before do create(:wiki_page, wiki: wiki, title: 'home 2', content: 'Another page') @@ -561,6 +603,8 @@ describe API::Search do it_behaves_like 'response is correct', schema: 'public_api/v4/commits_details' it_behaves_like 'pagination', scope: :commits, search: 'merge' + + it_behaves_like 'ping counters', scope: :commits end context 'for commits scope with project path as id' do @@ -582,6 +626,8 @@ describe API::Search do it_behaves_like 'pagination', scope: :blobs, search: 'monitors' + it_behaves_like 'ping counters', scope: :blobs + context 'filters' do it 'by filename' do get api(endpoint, user), params: { scope: 'blobs', search: 'mon filename:PROCESS.md' } diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index a5b95bc59a5..e6dd1fecb69 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -38,6 +38,8 @@ describe API::Settings, 'Settings' do expect(json_response).not_to have_key('performance_bar_allowed_group_path') expect(json_response).not_to have_key('performance_bar_enabled') expect(json_response['snippet_size_limit']).to eq(50.megabytes) + expect(json_response['spam_check_endpoint_enabled']).to be_falsey + expect(json_response['spam_check_endpoint_url']).to be_nil end end @@ -50,7 +52,7 @@ describe API::Settings, 'Settings' do storages = Gitlab.config.repositories.storages .merge({ 'custom' => 'tmp/tests/custom_repositories' }) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) - Feature.get(:sourcegraph).enable + stub_feature_flags(sourcegraph: true) end it "updates application settings" do @@ -90,7 +92,9 @@ describe API::Settings, 'Settings' do push_event_activities_limit: 2, snippet_size_limit: 5, issues_create_limit: 300, - raw_blob_request_limit: 300 + raw_blob_request_limit: 300, + spam_check_endpoint_enabled: true, + spam_check_endpoint_url: 'https://example.com/spam_check' } expect(response).to have_gitlab_http_status(:ok) @@ -129,6 +133,8 @@ describe API::Settings, 'Settings' do 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) + expect(json_response['spam_check_endpoint_enabled']).to be_truthy + expect(json_response['spam_check_endpoint_url']).to eq('https://example.com/spam_check') end end @@ -390,5 +396,14 @@ describe API::Settings, 'Settings' do expect(json_response['error']).to eq('sourcegraph_url is missing') end end + + context "missing spam_check_endpoint_url value when spam_check_endpoint_enabled is true" do + it "returns a blank parameter error message" do + put api("/application/settings", admin), params: { spam_check_endpoint_enabled: true } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('spam_check_endpoint_url is missing') + end + end end end diff --git a/spec/requests/api/suggestions_spec.rb b/spec/requests/api/suggestions_spec.rb index df3f72e3447..ffb8c811622 100644 --- a/spec/requests/api/suggestions_spec.rb +++ b/spec/requests/api/suggestions_spec.rb @@ -7,8 +7,7 @@ describe API::Suggestions do let(:user) { create(:user) } let(:merge_request) do - create(:merge_request, source_project: project, - target_project: project) + create(:merge_request, source_project: project, target_project: project) end let(:position) do @@ -19,26 +18,45 @@ describe API::Suggestions do diff_refs: merge_request.diff_refs) end + let(:position2) do + Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 15, + diff_refs: merge_request.diff_refs) + end + let(:diff_note) do + create(:diff_note_on_merge_request, + noteable: merge_request, + position: position, + project: project) + end + + let(:diff_note2) do create(:diff_note_on_merge_request, noteable: merge_request, - position: position, - project: project) + position: position2, + project: project) + end + + let(:suggestion) do + create(:suggestion, note: diff_note, + from_content: " raise RuntimeError, \"System commands must be given as an array of strings\"\n", + to_content: " raise RuntimeError, 'Explosion'\n # explosion?") + end + + let(:unappliable_suggestion) do + create(:suggestion, :unappliable, note: diff_note2) end describe "PUT /suggestions/:id/apply" do let(:url) { "/suggestions/#{suggestion.id}/apply" } context 'when successfully applies patch' do - let(:suggestion) do - create(:suggestion, note: diff_note, - from_content: " raise RuntimeError, \"System commands must be given as an array of strings\"\n", - to_content: " raise RuntimeError, 'Explosion'\n # explosion?") - end - - it 'returns 200 with json content' do + it 'renders an ok response and returns json content' do project.add_maintainer(user) - put api(url, user), params: { id: suggestion.id } + put api(url, user) expect(response).to have_gitlab_http_status(:ok) expect(json_response) @@ -48,31 +66,105 @@ describe API::Suggestions do end context 'when not able to apply patch' do - let(:suggestion) do - create(:suggestion, :unappliable, note: diff_note) - end + let(:url) { "/suggestions/#{unappliable_suggestion.id}/apply" } - it 'returns 400 with json content' do + it 'renders a bad request error and returns json content' do project.add_maintainer(user) - put api(url, user), params: { id: suggestion.id } + put api(url, user) expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response).to eq({ 'message' => 'Suggestion is not appliable' }) + expect(json_response).to eq({ 'message' => 'A suggestion is not applicable.' }) + end + end + + context 'when suggestion is not found' do + let(:url) { "/suggestions/foo-123/apply" } + + it 'renders a not found error and returns json content' do + project.add_maintainer(user) + + put api(url, user) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response).to eq({ 'message' => 'Suggestion is not applicable as the suggestion was not found.' }) end end context 'when unauthorized' do - let(:suggestion) do - create(:suggestion, note: diff_note, - from_content: " raise RuntimeError, \"System commands must be given as an array of strings\"\n", - to_content: " raise RuntimeError, 'Explosion'\n # explosion?") + it 'renders a forbidden error and returns json content' do + project.add_reporter(user) + + put api(url, user) + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response).to eq({ 'message' => '403 Forbidden' }) end + end + end + + describe "PUT /suggestions/batch_apply" do + let(:suggestion2) do + create(:suggestion, note: diff_note2, + from_content: " \"PWD\" => path\n", + to_content: " *** FOO ***\n") + end - it 'returns 403 with json content' do + let(:url) { "/suggestions/batch_apply" } + + context 'when successfully applies multiple patches as a batch' do + it 'renders an ok response and returns json content' do + project.add_maintainer(user) + + put api(url, user), params: { ids: [suggestion.id, suggestion2.id] } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to all(include('id', 'from_line', 'to_line', + 'appliable', 'applied', + 'from_content', 'to_content')) + end + end + + context 'when not able to apply one or more of the patches' do + it 'renders a bad request error and returns json content' do + project.add_maintainer(user) + + put api(url, user), + params: { ids: [suggestion.id, unappliable_suggestion.id] } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq({ 'message' => 'A suggestion is not applicable.' }) + end + end + + context 'with missing suggestions' do + it 'renders a not found error and returns json content if any suggestion is not found' do + project.add_maintainer(user) + + put api(url, user), params: { ids: [suggestion.id, 'foo-123'] } + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response) + .to eq({ 'message' => 'Suggestions are not applicable as one or more suggestions were not found.' }) + end + + it 'renders a bad request error and returns json content when no suggestions are provided' do + project.add_maintainer(user) + + put api(url, user), params: {} + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response) + .to eq({ 'error' => "ids is missing" }) + end + end + + context 'when unauthorized' do + it 'renders a forbidden error and returns json content' do project.add_reporter(user) - put api(url, user), params: { id: suggestion.id } + put api(url, user), + params: { ids: [suggestion.id, suggestion2.id] } expect(response).to have_gitlab_http_status(:forbidden) expect(json_response).to eq({ 'message' => '403 Forbidden' }) diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index 844cd948411..ec9db5566e3 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe API::Terraform::State do + include HttpBasicAuthHelpers + let_it_be(:project) { create(:project) } let_it_be(:developer) { create(:user, developer_projects: [project]) } let_it_be(:maintainer) { create(:user, maintainer_projects: [project]) } @@ -10,7 +12,7 @@ describe API::Terraform::State do let!(:state) { create(:terraform_state, :with_file, project: project) } let(:current_user) { maintainer } - let(:auth_header) { basic_auth_header(current_user) } + let(:auth_header) { user_basic_auth_header(current_user) } let(:project_id) { project.id } let(:state_name) { state.name } let(:state_path) { "/projects/#{project_id}/terraform/state/#{state_name}" } @@ -23,7 +25,7 @@ describe API::Terraform::State do subject(:request) { get api(state_path), headers: auth_header } context 'without authentication' do - let(:auth_header) { basic_auth_header('failing_token') } + let(:auth_header) { basic_auth_header('bad', 'token') } it 'returns 401 if user is not authenticated' do request @@ -32,34 +34,71 @@ describe API::Terraform::State do end end - context 'with maintainer permissions' do - let(:current_user) { maintainer } + context 'personal acceess token authentication' do + context 'with maintainer permissions' do + let(:current_user) { maintainer } - it 'returns terraform state belonging to a project of given state name' do - request + it 'returns terraform state belonging to a project of given state name' do + request - expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to eq(state.file.read) + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(state.file.read) + end + + context 'for a project that does not exist' do + let(:project_id) { '0000' } + + it 'returns not found' do + request + + expect(response).to have_gitlab_http_status(:not_found) + end + end end - context 'for a project that does not exist' do - let(:project_id) { '0000' } + context 'with developer permissions' do + let(:current_user) { developer } - it 'returns not found' do + it 'returns forbidden if the user cannot access the state' do request - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:forbidden) end end end - context 'with developer permissions' do - let(:current_user) { developer } + context 'job token authentication' do + let(:auth_header) { job_basic_auth_header(job) } - it 'returns forbidden if the user cannot access the state' do - request + context 'with maintainer permissions' do + let(:job) { create(:ci_build, project: project, user: maintainer) } - expect(response).to have_gitlab_http_status(:forbidden) + it 'returns terraform state belonging to a project of given state name' do + request + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(state.file.read) + end + + context 'for a project that does not exist' do + let(:project_id) { '0000' } + + it 'returns not found' do + request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'with developer permissions' do + let(:job) { create(:ci_build, project: project, user: developer) } + + it 'returns forbidden if the user cannot access the state' do + request + + expect(response).to have_gitlab_http_status(:forbidden) + end end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 4a0f0eea088..e780f67bcab 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -3,18 +3,186 @@ require 'spec_helper' describe API::Users, :do_not_mock_admin_mode do - let(:user) { create(:user, username: 'user.with.dot') } - let(:admin) { create(:admin) } - let(:key) { create(:key, user: user) } - let(:gpg_key) { create(:gpg_key, user: user) } - let(:email) { create(:email, user: user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:user, reload: true) { create(:user, username: 'user.with.dot') } + let_it_be(:key) { create(:key, user: user) } + let_it_be(:gpg_key) { create(:gpg_key, user: user) } + let_it_be(:email) { create(:email, user: user) } let(:omniauth_user) { create(:omniauth_user) } - let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') } let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } - let(:not_existing_user_id) { (User.maximum('id') || 0 ) + 10 } - let(:not_existing_pat_id) { (PersonalAccessToken.maximum('id') || 0 ) + 10 } let(:private_user) { create(:user, private_profile: true) } + context 'admin notes' do + let_it_be(:admin) { create(:admin, note: '2019-10-06 | 2FA added | user requested | www.gitlab.com') } + let_it_be(:user, reload: true) { create(:user, note: '2018-11-05 | 2FA removed | user requested | www.gitlab.com') } + + describe 'POST /users' do + context 'when unauthenticated' do + it 'return authentication error' do + post api('/users') + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when authenticated' do + context 'as an admin' do + it 'contains the note of the user' do + optional_attributes = { note: 'Awesome Note' } + attributes = attributes_for(:user).merge(optional_attributes) + + post api('/users', admin), params: attributes + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['note']).to eq(optional_attributes[:note]) + end + end + + context 'as a regular user' do + it 'does not allow creating new user' do + post api('/users', user), params: attributes_for(:user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + describe 'GET /users/:id' do + context 'when unauthenticated' do + it 'does not contain the note of the user' do + get api("/users/#{user.id}") + + expect(json_response).not_to have_key('note') + end + end + + context 'when authenticated' do + context 'as an admin' do + it 'contains the note of the user' do + get api("/users/#{user.id}", admin) + + expect(json_response).to have_key('note') + expect(json_response['note']).to eq(user.note) + end + end + + context 'as a regular user' do + it 'does not contain the note of the user' do + get api("/users/#{user.id}", user) + + expect(json_response).not_to have_key('note') + end + end + end + end + + describe "PUT /users/:id" do + context 'when user is an admin' do + it "updates note of the user" do + new_note = '2019-07-07 | Email changed | user requested | www.gitlab.com' + + expect do + put api("/users/#{user.id}", admin), params: { note: new_note } + end.to change { user.reload.note } + .from('2018-11-05 | 2FA removed | user requested | www.gitlab.com') + .to(new_note) + + expect(response).to have_gitlab_http_status(:success) + expect(json_response['note']).to eq(new_note) + end + end + + context 'when user is not an admin' do + it "cannot update their own note" do + expect do + put api("/users/#{user.id}", user), params: { note: 'new note' } + end.not_to change { user.reload.note } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + describe 'GET /users/' do + context 'when unauthenticated' do + it "does not contain the note of users" do + get api("/users"), params: { username: user.username } + + expect(json_response.first).not_to have_key('note') + end + end + + context 'when authenticated' do + context 'as a regular user' do + it 'does not contain the note of users' do + get api("/users", user), params: { username: user.username } + + expect(json_response.first).not_to have_key('note') + end + end + + context 'as an admin' do + it 'contains the note of users' do + get api("/users", admin), params: { username: user.username } + + expect(response).to have_gitlab_http_status(:success) + expect(json_response.first).to have_key('note') + expect(json_response.first['note']).to eq '2018-11-05 | 2FA removed | user requested | www.gitlab.com' + end + end + end + end + + describe 'GET /user' do + context 'when authenticated' do + context 'as an admin' do + context 'accesses their own profile' do + it 'contains the note of the user' do + get api("/user", admin) + + expect(json_response).to have_key('note') + expect(json_response['note']).to eq(admin.note) + end + end + + context 'sudo' do + let(:admin_personal_access_token) { create(:personal_access_token, user: admin, scopes: %w[api sudo]).token } + + context 'accesses the profile of another regular user' do + it 'does not contain the note of the user' do + get api("/user?private_token=#{admin_personal_access_token}&sudo=#{user.id}") + + expect(json_response['id']).to eq(user.id) + expect(json_response).not_to have_key('note') + end + end + + context 'accesses the profile of another admin' do + let(:admin_2) {create(:admin, note: '2010-10-10 | 2FA added | admin requested | www.gitlab.com')} + + it 'contains the note of the user' do + get api("/user?private_token=#{admin_personal_access_token}&sudo=#{admin_2.id}") + + expect(json_response['id']).to eq(admin_2.id) + expect(json_response).to have_key('note') + expect(json_response['note']).to eq(admin_2.note) + end + end + end + end + + context 'as a regular user' do + it 'does not contain the note of the user' do + get api("/user", user) + + expect(json_response).not_to have_key('note') + end + end + end + end + end + shared_examples 'rendering user status' do it 'returns the status if there was one' do create(:user_status, user: user) @@ -257,8 +425,9 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns the correct order when sorted by id' do - admin - user + # order of let_it_be definitions: + # - admin + # - user get api('/users', admin), params: { order_by: 'id', sort: 'asc' } @@ -269,8 +438,6 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns users with 2fa enabled' do - admin - user user_with_2fa = create(:user, :two_factor_via_otp) get api('/users', admin), params: { two_factor: 'enabled' } @@ -298,18 +465,6 @@ describe API::Users, :do_not_mock_admin_mode do expect(response).to have_gitlab_http_status(:bad_request) end end - - context "when authenticated and ldap is enabled" do - it "returns non-ldap user" do - create :omniauth_user, provider: "ldapserver1" - - get api("/users", user), params: { skip_ldap: "true" } - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - expect(json_response.first["username"]).to eq user.username - end - end end describe "GET /users/:id" do @@ -492,10 +647,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe "POST /users" do - before do - admin - end - it "creates user" do expect do post api("/users", admin), params: attributes_for(:user, projects_limit: 3) @@ -734,8 +885,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe "PUT /users/:id" do - 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' } @@ -752,8 +901,7 @@ describe API::Users, :do_not_mock_admin_mode do end it "updates user with empty bio" do - user.bio = 'previous bio' - user.save! + user.update!(bio: 'previous bio') put api("/users/#{user.id}", admin), params: { bio: '' } @@ -870,7 +1018,7 @@ describe API::Users, :do_not_mock_admin_mode do end it "updates private profile to false when nil is given" do - user.update(private_profile: true) + user.update!(private_profile: true) put api("/users/#{user.id}", admin), params: { private_profile: nil } @@ -879,7 +1027,7 @@ describe API::Users, :do_not_mock_admin_mode do end it "does not modify private profile when field is not provided" do - user.update(private_profile: true) + user.update!(private_profile: true) put api("/users/#{user.id}", admin), params: {} @@ -891,7 +1039,7 @@ describe API::Users, :do_not_mock_admin_mode do theme = Gitlab::Themes.each.find { |t| t.id != Gitlab::Themes.default.id } scheme = Gitlab::ColorSchemes.each.find { |t| t.id != Gitlab::ColorSchemes.default.id } - user.update(theme_id: theme.id, color_scheme_id: scheme.id) + user.update!(theme_id: theme.id, color_scheme_id: scheme.id) put api("/users/#{user.id}", admin), params: {} @@ -901,6 +1049,8 @@ describe API::Users, :do_not_mock_admin_mode do end it "does not update admin status" do + admin_user = create(:admin) + put api("/users/#{admin_user.id}", admin), params: { can_create_group: false } expect(response).to have_gitlab_http_status(:ok) @@ -1071,10 +1221,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe "POST /users/:id/keys" do - before do - admin - end - it "does not create invalid ssh key" do post api("/users/#{user.id}/keys", admin), params: { title: "invalid key" } @@ -1096,6 +1242,16 @@ describe API::Users, :do_not_mock_admin_mode do end.to change { user.keys.count }.by(1) end + it 'creates SSH key with `expires_at` attribute' do + optional_attributes = { expires_at: '2016-01-21T00:00:00.000Z' } + attributes = attributes_for(:key).merge(optional_attributes) + + post api("/users/#{user.id}/keys", admin), params: attributes + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['expires_at']).to eq(optional_attributes[:expires_at]) + end + it "returns 400 for invalid ID" do post api("/users/0/keys", admin) expect(response).to have_gitlab_http_status(:bad_request) @@ -1104,9 +1260,7 @@ describe API::Users, :do_not_mock_admin_mode do describe 'GET /user/:id/keys' do it 'returns 404 for non-existing user' do - user_id = not_existing_user_id - - get api("/users/#{user_id}/keys") + get api("/users/#{non_existing_record_id}/keys") expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') @@ -1114,7 +1268,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns array of ssh keys' do user.keys << key - user.save get api("/users/#{user.id}/keys") @@ -1123,11 +1276,41 @@ describe API::Users, :do_not_mock_admin_mode do expect(json_response).to be_an Array expect(json_response.first['title']).to eq(key.title) end + + it 'returns array of ssh keys with comments replaced with'\ + 'a simple identifier of username + hostname' do + get api("/users/#{user.id}/keys") + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + + keys = json_response.map { |key_detail| key_detail['key'] } + expect(keys).to all(include("#{user.name} (#{Gitlab.config.gitlab.host}")) + end + + context 'N+1 queries' do + before do + get api("/users/#{user.id}/keys") + end + + it 'avoids N+1 queries', :request_store do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/users/#{user.id}/keys") + end.count + + create_list(:key, 2, user: user) + + expect do + get api("/users/#{user.id}/keys") + end.not_to exceed_all_query_limit(control_count) + end + end end describe 'GET /user/:user_id/keys' do it 'returns 404 for non-existing user' do - get api("/users/#{not_existing_user_id}/keys") + get api("/users/#{non_existing_record_id}/keys") expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') @@ -1135,7 +1318,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns array of ssh keys' do user.keys << key - user.save get api("/users/#{user.username}/keys") @@ -1147,13 +1329,9 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'DELETE /user/:id/keys/:key_id' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do - delete api("/users/#{user.id}/keys/42") + delete api("/users/#{user.id}/keys/#{non_existing_record_id}") expect(response).to have_gitlab_http_status(:unauthorized) end end @@ -1161,7 +1339,6 @@ describe API::Users, :do_not_mock_admin_mode do context 'when authenticated' do it 'deletes existing key' do user.keys << key - user.save expect do delete api("/users/#{user.id}/keys/#{key.id}", admin) @@ -1176,14 +1353,14 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 404 error if user not found' do user.keys << key - user.save + delete api("/users/0/keys/#{key.id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') end it 'returns 404 error if key not foud' do - delete api("/users/#{user.id}/keys/42", admin) + delete api("/users/#{user.id}/keys/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Key Not Found') end @@ -1191,10 +1368,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'POST /users/:id/keys' do - before do - admin - end - it 'does not create invalid GPG key' do post api("/users/#{user.id}/gpg_keys", admin) @@ -1203,7 +1376,8 @@ describe API::Users, :do_not_mock_admin_mode do end it 'creates GPG key' do - key_attrs = attributes_for :gpg_key + key_attrs = attributes_for :gpg_key, key: GpgHelpers::User2.public_key + expect do post api("/users/#{user.id}/gpg_keys", admin), params: key_attrs @@ -1219,10 +1393,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'GET /user/:id/gpg_keys' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do get api("/users/#{user.id}/gpg_keys") @@ -1240,7 +1410,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 error if key not foud' do - delete api("/users/#{user.id}/gpg_keys/42", admin) + delete api("/users/#{user.id}/gpg_keys/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -1248,7 +1418,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns array of GPG keys' do user.gpg_keys << gpg_key - user.save get api("/users/#{user.id}/gpg_keys", admin) @@ -1261,13 +1430,9 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'DELETE /user/:id/gpg_keys/:key_id' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do - delete api("/users/#{user.id}/keys/42") + delete api("/users/#{user.id}/keys/#{non_existing_record_id}") expect(response).to have_gitlab_http_status(:unauthorized) end @@ -1276,7 +1441,6 @@ describe API::Users, :do_not_mock_admin_mode do context 'when authenticated' do it 'deletes existing key' do user.gpg_keys << gpg_key - user.save expect do delete api("/users/#{user.id}/gpg_keys/#{gpg_key.id}", admin) @@ -1287,7 +1451,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 404 error if user not found' do user.keys << key - user.save delete api("/users/0/gpg_keys/#{gpg_key.id}", admin) @@ -1296,7 +1459,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 error if key not foud' do - delete api("/users/#{user.id}/gpg_keys/42", admin) + delete api("/users/#{user.id}/gpg_keys/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -1305,13 +1468,9 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'POST /user/:id/gpg_keys/:key_id/revoke' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do - post api("/users/#{user.id}/gpg_keys/42/revoke") + post api("/users/#{user.id}/gpg_keys/#{non_existing_record_id}/revoke") expect(response).to have_gitlab_http_status(:unauthorized) end @@ -1320,7 +1479,6 @@ describe API::Users, :do_not_mock_admin_mode do context 'when authenticated' do it 'revokes existing key' do user.gpg_keys << gpg_key - user.save expect do post api("/users/#{user.id}/gpg_keys/#{gpg_key.id}/revoke", admin) @@ -1331,7 +1489,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 404 error if user not found' do user.gpg_keys << gpg_key - user.save post api("/users/0/gpg_keys/#{gpg_key.id}/revoke", admin) @@ -1340,7 +1497,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 error if key not foud' do - post api("/users/#{user.id}/gpg_keys/42/revoke", admin) + post api("/users/#{user.id}/gpg_keys/#{non_existing_record_id}/revoke", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -1349,10 +1506,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe "POST /users/:id/emails" do - before do - admin - end - it "does not create invalid email" do post api("/users/#{user.id}/emails", admin), params: {} @@ -1390,10 +1543,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'GET /user/:id/emails' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do get api("/users/#{user.id}/emails") @@ -1410,7 +1559,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns array of emails' do user.emails << email - user.save get api("/users/#{user.id}/emails", admin) @@ -1429,13 +1577,9 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'DELETE /user/:id/emails/:email_id' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do - delete api("/users/#{user.id}/emails/42") + delete api("/users/#{user.id}/emails/#{non_existing_record_id}") expect(response).to have_gitlab_http_status(:unauthorized) end end @@ -1443,7 +1587,6 @@ describe API::Users, :do_not_mock_admin_mode do context 'when authenticated' do it 'deletes existing email' do user.emails << email - user.save expect do delete api("/users/#{user.id}/emails/#{email.id}", admin) @@ -1458,14 +1601,14 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 404 error if user not found' do user.emails << email - user.save + delete api("/users/0/emails/#{email.id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') end it 'returns 404 error if email not foud' do - delete api("/users/#{user.id}/emails/42", admin) + delete api("/users/#{user.id}/emails/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Email Not Found') end @@ -1479,19 +1622,16 @@ describe API::Users, :do_not_mock_admin_mode do end describe "DELETE /users/:id" do - let!(:namespace) { user.namespace } - let!(:issue) { create(:issue, author: user) } - - before do - admin - end + let_it_be(:issue) { create(:issue, author: user) } it "deletes user", :sidekiq_might_not_need_inline do + namespace_id = user.namespace.id + perform_enqueued_jobs { delete api("/users/#{user.id}", admin) } expect(response).to have_gitlab_http_status(:no_content) expect { User.find(user.id) }.to raise_error ActiveRecord::RecordNotFound - expect { Namespace.find(namespace.id) }.to raise_error ActiveRecord::RecordNotFound + expect { Namespace.find(namespace_id) }.to raise_error ActiveRecord::RecordNotFound end context "sole owner of a group" do @@ -1560,11 +1700,11 @@ describe API::Users, :do_not_mock_admin_mode do end describe "GET /user" do - let(:personal_access_token) { create(:personal_access_token, user: user).token } - shared_examples 'get user info' do |version| context 'with regular user' do context 'with personal access token' do + let(:personal_access_token) { create(:personal_access_token, user: user).token } + it 'returns 403 without private token when sudo is defined' do get api("/user?private_token=#{personal_access_token}&sudo=123", version: version) @@ -1632,7 +1772,6 @@ describe API::Users, :do_not_mock_admin_mode do context "when authenticated" do it "returns array of ssh keys" do user.keys << key - user.save get api("/user/keys", user) @@ -1642,6 +1781,36 @@ describe API::Users, :do_not_mock_admin_mode do expect(json_response.first["title"]).to eq(key.title) end + it 'returns array of ssh keys with comments replaced with'\ + 'a simple identifier of username + hostname' do + get api("/user/keys", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + + keys = json_response.map { |key_detail| key_detail['key'] } + expect(keys).to all(include("#{user.name} (#{Gitlab.config.gitlab.host}")) + end + + context 'N+1 queries' do + before do + get api("/user/keys", user) + end + + it 'avoids N+1 queries', :request_store do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/user/keys", user) + end.count + + create_list(:key, 2, user: user) + + expect do + get api("/user/keys", user) + end.not_to exceed_all_query_limit(control_count) + end + end + context "scopes" do let(:path) { "/user/keys" } let(:api_call) { method(:api) } @@ -1654,14 +1823,21 @@ describe API::Users, :do_not_mock_admin_mode do describe "GET /user/keys/:key_id" do it "returns single key" do user.keys << key - user.save + get api("/user/keys/#{key.id}", user) expect(response).to have_gitlab_http_status(:ok) expect(json_response["title"]).to eq(key.title) end + it 'exposes SSH key comment as a simple identifier of username + hostname' do + get api("/user/keys/#{key.id}", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['key']).to include("#{key.user_name} (#{Gitlab.config.gitlab.host})") + end + it "returns 404 Not Found within invalid ID" do - get api("/user/keys/42", user) + get api("/user/keys/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Key Not Found') @@ -1669,8 +1845,8 @@ describe API::Users, :do_not_mock_admin_mode do it "returns 404 error if admin accesses user's ssh key" do user.keys << key - user.save admin + get api("/user/keys/#{key.id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Key Not Found') @@ -1699,6 +1875,16 @@ describe API::Users, :do_not_mock_admin_mode do expect(response).to have_gitlab_http_status(:created) end + it 'creates SSH key with `expires_at` attribute' do + optional_attributes = { expires_at: '2016-01-21T00:00:00.000Z' } + attributes = attributes_for(:key).merge(optional_attributes) + + post api("/user/keys", user), params: attributes + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['expires_at']).to eq(optional_attributes[:expires_at]) + end + it "returns a 401 error if unauthorized" do post api("/user/keys"), params: { title: 'some title', key: 'some key' } expect(response).to have_gitlab_http_status(:unauthorized) @@ -1727,7 +1913,6 @@ describe API::Users, :do_not_mock_admin_mode do describe "DELETE /user/keys/:key_id" do it "deletes existed key" do user.keys << key - user.save expect do delete api("/user/keys/#{key.id}", user) @@ -1741,7 +1926,7 @@ describe API::Users, :do_not_mock_admin_mode do end it "returns 404 if key ID not found" do - delete api("/user/keys/42", user) + delete api("/user/keys/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Key Not Found') @@ -1749,7 +1934,7 @@ describe API::Users, :do_not_mock_admin_mode do it "returns 401 error if unauthorized" do user.keys << key - user.save + delete api("/user/keys/#{key.id}") expect(response).to have_gitlab_http_status(:unauthorized) end @@ -1773,7 +1958,6 @@ describe API::Users, :do_not_mock_admin_mode do context 'when authenticated' do it 'returns array of GPG keys' do user.gpg_keys << gpg_key - user.save get api('/user/gpg_keys', user) @@ -1795,7 +1979,6 @@ describe API::Users, :do_not_mock_admin_mode do describe 'GET /user/gpg_keys/:key_id' do it 'returns a single key' do user.gpg_keys << gpg_key - user.save get api("/user/gpg_keys/#{gpg_key.id}", user) @@ -1804,7 +1987,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 Not Found within invalid ID' do - get api('/user/gpg_keys/42', user) + get api("/user/gpg_keys/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -1812,7 +1995,6 @@ describe API::Users, :do_not_mock_admin_mode do it "returns 404 error if admin accesses user's GPG key" do user.gpg_keys << gpg_key - user.save get api("/user/gpg_keys/#{gpg_key.id}", admin) @@ -1836,7 +2018,8 @@ describe API::Users, :do_not_mock_admin_mode do describe 'POST /user/gpg_keys' do it 'creates a GPG key' do - key_attrs = attributes_for :gpg_key + key_attrs = attributes_for :gpg_key, key: GpgHelpers::User2.public_key + expect do post api('/user/gpg_keys', user), params: key_attrs @@ -1861,7 +2044,6 @@ describe API::Users, :do_not_mock_admin_mode do describe 'POST /user/gpg_keys/:key_id/revoke' do it 'revokes existing GPG key' do user.gpg_keys << gpg_key - user.save expect do post api("/user/gpg_keys/#{gpg_key.id}/revoke", user) @@ -1871,7 +2053,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 if key ID not found' do - post api('/user/gpg_keys/42/revoke', user) + post api("/user/gpg_keys/#{non_existing_record_id}/revoke", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -1879,7 +2061,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 401 error if unauthorized' do user.gpg_keys << gpg_key - user.save post api("/user/gpg_keys/#{gpg_key.id}/revoke") @@ -1896,7 +2077,6 @@ describe API::Users, :do_not_mock_admin_mode do describe 'DELETE /user/gpg_keys/:key_id' do it 'deletes existing GPG key' do user.gpg_keys << gpg_key - user.save expect do delete api("/user/gpg_keys/#{gpg_key.id}", user) @@ -1906,7 +2086,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 if key ID not found' do - delete api('/user/gpg_keys/42', user) + delete api("/user/gpg_keys/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -1914,7 +2094,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 401 error if unauthorized' do user.gpg_keys << gpg_key - user.save delete api("/user/gpg_keys/#{gpg_key.id}") @@ -1939,7 +2118,6 @@ describe API::Users, :do_not_mock_admin_mode do context "when authenticated" do it "returns array of emails" do user.emails << email - user.save get api("/user/emails", user) @@ -1961,22 +2139,22 @@ describe API::Users, :do_not_mock_admin_mode do describe "GET /user/emails/:email_id" do it "returns single email" do user.emails << email - user.save + get api("/user/emails/#{email.id}", user) expect(response).to have_gitlab_http_status(:ok) expect(json_response["email"]).to eq(email.email) end it "returns 404 Not Found within invalid ID" do - get api("/user/emails/42", user) + get api("/user/emails/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Email Not Found') end it "returns 404 error if admin accesses user's email" do user.emails << email - user.save admin + get api("/user/emails/#{email.id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Email Not Found') @@ -2021,7 +2199,6 @@ describe API::Users, :do_not_mock_admin_mode do describe "DELETE /user/emails/:email_id" do it "deletes existed email" do user.emails << email - user.save expect do delete api("/user/emails/#{email.id}", user) @@ -2035,7 +2212,7 @@ describe API::Users, :do_not_mock_admin_mode do end it "returns 404 if email ID not found" do - delete api("/user/emails/42", user) + delete api("/user/emails/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Email Not Found') @@ -2043,7 +2220,7 @@ describe API::Users, :do_not_mock_admin_mode do it "returns 401 error if unauthorized" do user.emails << email - user.save + delete api("/user/emails/#{email.id}") expect(response).to have_gitlab_http_status(:unauthorized) end @@ -2149,7 +2326,7 @@ describe API::Users, :do_not_mock_admin_mode do context 'performed by an admin user' do context 'for an active user' do let(:activity) { {} } - let(:user) { create(:user, username: 'user.with.dot', **activity) } + let(:user) { create(:user, **activity) } context 'with no recent activity' do let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.next.days.ago } } @@ -2234,10 +2411,6 @@ describe API::Users, :do_not_mock_admin_mode do describe 'POST /users/:id/block' do let(:blocked_user) { create(:user, state: 'blocked') } - before do - admin - end - it 'blocks existing user' do post api("/users/#{user.id}/block", admin) @@ -2280,10 +2453,6 @@ describe API::Users, :do_not_mock_admin_mode do let(:blocked_user) { create(:user, state: 'blocked') } let(:deactivated_user) { create(:user, state: 'deactivated') } - before do - admin - end - it 'unblocks existing user' do post api("/users/#{user.id}/unblock", admin) expect(response).to have_gitlab_http_status(:created) @@ -2477,21 +2646,21 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'GET /users/:user_id/impersonation_tokens' do - let!(:active_personal_access_token) { create(:personal_access_token, user: user) } - let!(:revoked_personal_access_token) { create(:personal_access_token, :revoked, user: user) } - let!(:expired_personal_access_token) { create(:personal_access_token, :expired, user: user) } - let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } - let!(:revoked_impersonation_token) { create(:personal_access_token, :impersonation, :revoked, user: user) } + let_it_be(:active_personal_access_token) { create(:personal_access_token, user: user) } + let_it_be(:revoked_personal_access_token) { create(:personal_access_token, :revoked, user: user) } + let_it_be(:expired_personal_access_token) { create(:personal_access_token, :expired, user: user) } + let_it_be(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + let_it_be(:revoked_impersonation_token) { create(:personal_access_token, :impersonation, :revoked, user: user) } it 'returns a 404 error if user not found' do - get api("/users/#{not_existing_user_id}/impersonation_tokens", admin) + get api("/users/#{non_existing_record_id}/impersonation_tokens", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') end it 'returns a 403 error when authenticated as normal user' do - get api("/users/#{not_existing_user_id}/impersonation_tokens", user) + get api("/users/#{non_existing_record_id}/impersonation_tokens", user) expect(response).to have_gitlab_http_status(:forbidden) expect(json_response['message']).to eq('403 Forbidden') @@ -2540,7 +2709,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns a 404 error if user not found' do - post api("/users/#{not_existing_user_id}/impersonation_tokens", admin), + post api("/users/#{non_existing_record_id}/impersonation_tokens", admin), params: { name: name, expires_at: expires_at @@ -2584,18 +2753,18 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'GET /users/:user_id/impersonation_tokens/:impersonation_token_id' do - let!(:personal_access_token) { create(:personal_access_token, user: user) } - let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + let_it_be(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } it 'returns 404 error if user not found' do - get api("/users/#{not_existing_user_id}/impersonation_tokens/1", admin) + get api("/users/#{non_existing_record_id}/impersonation_tokens/1", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') end it 'returns a 404 error if impersonation token not found' do - get api("/users/#{user.id}/impersonation_tokens/#{not_existing_pat_id}", admin) + get api("/users/#{user.id}/impersonation_tokens/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Impersonation Token Not Found') @@ -2625,18 +2794,18 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'DELETE /users/:user_id/impersonation_tokens/:impersonation_token_id' do - let!(:personal_access_token) { create(:personal_access_token, user: user) } - let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + let_it_be(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } it 'returns a 404 error if user not found' do - delete api("/users/#{not_existing_user_id}/impersonation_tokens/1", admin) + delete api("/users/#{non_existing_record_id}/impersonation_tokens/1", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') end it 'returns a 404 error if impersonation token not found' do - delete api("/users/#{user.id}/impersonation_tokens/#{not_existing_pat_id}", admin) + delete api("/users/#{user.id}/impersonation_tokens/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Impersonation Token Not Found') |