diff options
Diffstat (limited to 'spec/requests')
56 files changed, 2770 insertions, 463 deletions
diff --git a/spec/requests/api/boards_spec.rb b/spec/requests/api/boards_spec.rb index de79e8c4c5c..0b9c0c2ebe9 100644 --- a/spec/requests/api/boards_spec.rb +++ b/spec/requests/api/boards_spec.rb @@ -63,7 +63,7 @@ describe API::Boards do end end - describe "POST /groups/:id/boards/lists", :nested_groups do + describe "POST /groups/:id/boards/lists" do set(:group) { create(:group) } set(:board_parent) { create(:group, parent: group ) } let(:url) { "/groups/#{board_parent.id}/boards/#{board.id}/lists" } diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 8b503777443..f9c8b42afa8 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -65,7 +65,7 @@ describe API::Branches do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { get api(route, current_user) } end end @@ -175,7 +175,7 @@ describe API::Branches do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { get api(route, current_user) } end end @@ -337,7 +337,7 @@ describe API::Branches do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { put api(route, current_user) } end end @@ -471,7 +471,7 @@ describe API::Branches do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { put api(route, current_user) } end end @@ -547,7 +547,7 @@ describe API::Branches do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { post api(route, current_user) } end end diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index b5e45f99109..1be8883bd3c 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -8,10 +8,6 @@ describe API::CommitStatuses do let(:developer) { create_user(:developer) } let(:sha) { commit.id } - let(:commit_status) do - create(:commit_status, status: :pending, pipeline: pipeline) - end - describe "GET /projects/:id/repository/commits/:sha/statuses" do let(:get_url) { "/projects/#{project.id}/repository/commits/#{sha}/statuses" } @@ -239,6 +235,26 @@ describe API::CommitStatuses do expect(CommitStatus.count).to eq 1 end end + + context 'when a pipeline id is specified' do + let!(:first_pipeline) { project.ci_pipelines.create(source: :push, sha: commit.id, ref: 'master', status: 'created') } + let!(:other_pipeline) { project.ci_pipelines.create(source: :push, sha: commit.id, ref: 'master', status: 'created') } + + subject do + post api(post_url, developer), params: { + pipeline_id: other_pipeline.id, + state: 'success', + ref: 'master' + } + end + + it 'update the correct pipeline' do + subject + + expect(first_pipeline.reload.status).to eq('created') + expect(other_pipeline.reload.status).to eq('success') + end + end end context 'when retrying a commit status' do diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index f104da6ebba..5e6ff40e8cf 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -126,6 +126,12 @@ describe API::Commits do end end + context "with empty ref_name parameter" do + let(:route) { "/projects/#{project_id}/repository/commits?ref_name=" } + + it_behaves_like 'project commits' + end + context "path optional parameter" do it "returns project commits matching provided path parameter" do path = 'files/ruby/popen.rb' @@ -281,7 +287,7 @@ describe API::Commits do end it 'does not increment the usage counters using access token authentication' do - expect(::Gitlab::WebIdeCommitsCounter).not_to receive(:increment) + expect(::Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_commits_count) post api(url, user), params: valid_c_params end @@ -320,67 +326,132 @@ describe API::Commits do end end - context 'when the API user is a guest' do + context 'when committing to a new branch' do def last_commit_id(project, branch_name) project.repository.find_branch(branch_name)&.dereferenced_target&.id end - let(:public_project) { create(:project, :public, :repository) } - let!(:url) { "/projects/#{public_project.id}/repository/commits" } - let(:guest) { create(:user).tap { |u| public_project.add_guest(u) } } + before do + valid_c_params[:start_branch] = 'master' + valid_c_params[:branch] = 'patch' + end - it 'returns a 403' do - post api(url, guest), params: valid_c_params + context 'when the API user is a guest' do + let(:public_project) { create(:project, :public, :repository) } + let(:url) { "/projects/#{public_project.id}/repository/commits" } + let(:guest) { create(:user).tap { |u| public_project.add_guest(u) } } - expect(response).to have_gitlab_http_status(403) - end + it 'returns a 403' do + post api(url, guest), params: valid_c_params - context 'when start_project is provided' do - context 'when posting to a forked project the user owns' do - let!(:forked_project) { fork_project(public_project, guest, namespace: guest.namespace, repository: true) } - let!(:url) { "/projects/#{forked_project.id}/repository/commits" } + expect(response).to have_gitlab_http_status(403) + end - before do - valid_c_params[:start_branch] = "master" - valid_c_params[:branch] = "patch" - end + context 'when start_project is provided' do + context 'when posting to a forked project the user owns' do + let(:forked_project) { fork_project(public_project, guest, namespace: guest.namespace, repository: true) } + let(:url) { "/projects/#{forked_project.id}/repository/commits" } + + context 'identified by Integer (id)' do + before do + valid_c_params[:start_project] = public_project.id + end + + it 'adds a new commit to forked_project and returns a 201' do + expect_request_with_status(201) { post api(url, guest), params: valid_c_params } + .to change { last_commit_id(forked_project, valid_c_params[:branch]) } + .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) } + end + end - context 'identified by Integer (id)' do - before do - valid_c_params[:start_project] = public_project.id + context 'identified by String (full_path)' do + before do + valid_c_params[:start_project] = public_project.full_path + end + + it 'adds a new commit to forked_project and returns a 201' do + expect_request_with_status(201) { post api(url, guest), params: valid_c_params } + .to change { last_commit_id(forked_project, valid_c_params[:branch]) } + .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) } + end + end + + context 'when branch already exists' do + before do + valid_c_params.delete(:start_branch) + valid_c_params[:branch] = 'master' + valid_c_params[:start_project] = public_project.id + end + + it 'returns a 400' do + post api(url, guest), params: valid_c_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq("A branch called 'master' already exists. Switch to that branch in order to make changes") + end + + context 'when force is set to true' do + before do + valid_c_params[:force] = true + end + + it 'adds a new commit to forked_project and returns a 201' do + expect_request_with_status(201) { post api(url, guest), params: valid_c_params } + .to change { last_commit_id(forked_project, valid_c_params[:branch]) } + .and not_change { last_commit_id(public_project, valid_c_params[:branch]) } + end + end end - it 'adds a new commit to forked_project and returns a 201' do - expect { post api(url, guest), params: valid_c_params } - .to change { last_commit_id(forked_project, valid_c_params[:branch]) } - .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) } + context 'when start_sha is also provided' do + let(:forked_project) { fork_project(public_project, guest, namespace: guest.namespace, repository: false) } + let(:start_sha) { public_project.repository.commit.parent.sha } + + before do + # initialize an empty repository to force fetching from the original project + forked_project.repository.create_if_not_exists + + valid_c_params[:start_project] = public_project.id + valid_c_params[:start_sha] = start_sha + valid_c_params.delete(:start_branch) + end + + it 'fetches the start_sha from the original project to use as parent commit and returns a 201' do + expect_request_with_status(201) { post api(url, guest), params: valid_c_params } + .to change { last_commit_id(forked_project, valid_c_params[:branch]) } + .and not_change { last_commit_id(forked_project, 'master') } - expect(response).to have_gitlab_http_status(201) + last_commit = forked_project.repository.find_branch(valid_c_params[:branch]).dereferenced_target + expect(last_commit.parent_id).to eq(start_sha) + end end end - context 'identified by String (full_path)' do + context 'when the target project is not part of the fork network of start_project' do + let(:unrelated_project) { create(:project, :public, :repository, creator: guest) } + let(:url) { "/projects/#{unrelated_project.id}/repository/commits" } + before do - valid_c_params[:start_project] = public_project.full_path + valid_c_params[:start_branch] = 'master' + valid_c_params[:branch] = 'patch' + valid_c_params[:start_project] = public_project.id end - it 'adds a new commit to forked_project and returns a 201' do - expect { post api(url, guest), params: valid_c_params } - .to change { last_commit_id(forked_project, valid_c_params[:branch]) } - .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) } + it 'returns a 403' do + post api(url, guest), params: valid_c_params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(403) end end end - context 'when the target project is not part of the fork network of start_project' do - let(:unrelated_project) { create(:project, :public, :repository, creator: guest) } - let!(:url) { "/projects/#{unrelated_project.id}/repository/commits" } + context 'when posting to a forked project the user does not have write access' do + let(:forked_project) { fork_project(public_project, user, namespace: user.namespace, repository: true) } + let(:url) { "/projects/#{forked_project.id}/repository/commits" } before do - valid_c_params[:start_branch] = "master" - valid_c_params[:branch] = "patch" + valid_c_params[:start_branch] = 'master' + valid_c_params[:branch] = 'patch' valid_c_params[:start_project] = public_project.id end @@ -392,20 +463,68 @@ describe API::Commits do end end - context 'when posting to a forked project the user does not have write access' do - let!(:forked_project) { fork_project(public_project, user, namespace: user.namespace, repository: true) } - let!(:url) { "/projects/#{forked_project.id}/repository/commits" } + context 'when start_sha is provided' do + let(:start_sha) { project.repository.commit.parent.sha } before do - valid_c_params[:start_branch] = "master" - valid_c_params[:branch] = "patch" - valid_c_params[:start_project] = public_project.id + valid_c_params[:start_sha] = start_sha + valid_c_params.delete(:start_branch) end - it 'returns a 403' do - post api(url, guest), params: valid_c_params + it 'returns a 400 if start_branch is also provided' do + valid_c_params[:start_branch] = 'master' + post api(url, user), params: valid_c_params - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(400) + expect(json_response['error']).to eq('start_branch, start_sha are mutually exclusive') + end + + it 'returns a 400 if branch already exists' do + valid_c_params[:branch] = 'master' + post api(url, user), params: valid_c_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq("A branch called 'master' already exists. Switch to that branch in order to make changes") + end + + it 'returns a 400 if start_sha does not exist' do + valid_c_params[:start_sha] = '1' * 40 + post api(url, user), params: valid_c_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq("Cannot find start_sha '#{valid_c_params[:start_sha]}'") + end + + it 'returns a 400 if start_sha is not a full SHA' do + valid_c_params[:start_sha] = start_sha.slice(0, 7) + post api(url, user), params: valid_c_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq("Invalid start_sha '#{valid_c_params[:start_sha]}'") + end + + it 'uses the start_sha as parent commit and returns a 201' do + expect_request_with_status(201) { post api(url, user), params: valid_c_params } + .to change { last_commit_id(project, valid_c_params[:branch]) } + .and not_change { last_commit_id(project, 'master') } + + last_commit = project.repository.find_branch(valid_c_params[:branch]).dereferenced_target + expect(last_commit.parent_id).to eq(start_sha) + end + + context 'when force is set to true and branch already exists' do + before do + valid_c_params[:force] = true + valid_c_params[:branch] = 'master' + end + + it 'uses the start_sha as parent commit and returns a 201' do + expect_request_with_status(201) { post api(url, user), params: valid_c_params } + .to change { last_commit_id(project, valid_c_params[:branch]) } + + last_commit = project.repository.find_branch(valid_c_params[:branch]).dereferenced_target + expect(last_commit.parent_id).to eq(start_sha) + end end end end @@ -736,7 +855,7 @@ describe API::Commits do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { get api(route, current_user) } end end @@ -825,7 +944,7 @@ describe API::Commits do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { get api(route, current_user) } end end @@ -968,7 +1087,7 @@ describe API::Commits do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { get api(route, current_user) } end end @@ -1067,7 +1186,7 @@ describe API::Commits do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { get api(route, current_user) } end end @@ -1169,7 +1288,7 @@ describe API::Commits do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { post api(route, current_user), params: { branch: 'master' } } end end @@ -1324,7 +1443,7 @@ describe API::Commits do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { post api(route, current_user), params: { branch: branch } } end end @@ -1435,7 +1554,7 @@ describe API::Commits do context 'when repository is disabled' do include_context 'disabled repository' - it_behaves_like '403 response' do + it_behaves_like '404 response' do let(:request) { post api(route, current_user), params: { note: 'My comment' } } end end diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb index ca1ffe3c524..ef09c6effbb 100644 --- a/spec/requests/api/discussions_spec.rb +++ b/spec/requests/api/discussions_spec.rb @@ -9,6 +9,61 @@ describe API::Discussions do project.add_developer(user) end + context 'with cross-reference system notes', :request_store do + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + let(:new_merge_request) { create(:merge_request) } + let(:commit) { new_merge_request.project.commit } + let!(:note) { create(:system_note, noteable: merge_request, project: project, note: cross_reference) } + let!(:note_metadata) { create(:system_note_metadata, note: note, action: 'cross_reference') } + let(:cross_reference) { "test commit #{commit.to_reference(project)}" } + let(:pat) { create(:personal_access_token, user: user) } + + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/discussions" } + + before do + project.add_developer(user) + new_merge_request.project.add_developer(user) + end + + it 'returns only the note that the user should see' do + hidden_merge_request = create(:merge_request) + new_cross_reference = "test commit #{hidden_merge_request.project.commit}" + new_note = create(:system_note, noteable: merge_request, project: project, note: new_cross_reference) + create(:system_note_metadata, note: new_note, action: 'cross_reference') + + get api(url, user, personal_access_token: pat) + expect(response).to have_gitlab_http_status(200) + expect(json_response.count).to eq(1) + expect(json_response.first['notes'].count).to eq(1) + + parsed_note = json_response.first['notes'].first + expect(parsed_note['id']).to eq(note.id) + expect(parsed_note['body']).to eq(cross_reference) + expect(parsed_note['system']).to be true + end + + it 'avoids Git calls and N+1 SQL queries' do + expect_any_instance_of(Repository).not_to receive(:find_commit).with(commit.id) + + control = ActiveRecord::QueryRecorder.new do + get api(url, user, personal_access_token: pat) + end + + expect(response).to have_gitlab_http_status(200) + + RequestStore.clear! + + new_note = create(:system_note, noteable: merge_request, project: project, note: cross_reference) + create(:system_note_metadata, note: new_note, action: 'cross_reference') + + RequestStore.clear! + + expect { get api(url, user, personal_access_token: pat) }.not_to exceed_query_limit(control) + expect(response).to have_gitlab_http_status(200) + end + end + context 'when noteable is an Issue' do let!(:issue) { create(:issue, project: project, author: user) } let!(:issue_note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: user) } diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb index 8fc7fdc8632..745f3c55ac8 100644 --- a/spec/requests/api/environments_spec.rb +++ b/spec/requests/api/environments_spec.rb @@ -34,6 +34,47 @@ describe API::Environments do expect(json_response.first['project'].keys).to contain_exactly(*project_data_keys) expect(json_response.first).not_to have_key("last_deployment") end + + context 'when filtering' do + let!(:environment2) { create(:environment, project: project) } + + it 'returns environment by name' do + get api("/projects/#{project.id}/environments?name=#{environment.name}", user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(1) + expect(json_response.first['name']).to eq(environment.name) + end + + it 'returns no environment by non-existent name' do + get api("/projects/#{project.id}/environments?name=test", user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(0) + end + + it 'returns environments by name_like' do + get api("/projects/#{project.id}/environments?search=envir", user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(2) + end + + it 'returns no environment by non-existent name_like' do + get api("/projects/#{project.id}/environments?search=test", user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(0) + end + end end context 'as non member' do diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 1ad536258ba..21b67357543 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -186,6 +186,14 @@ describe API::Files do expect(headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" end + it 'returns blame file info' do + url = route(file_path) + '/blame' + + get api(url, current_user), params: params + + expect(response).to have_gitlab_http_status(200) + end + it 'sets inline content disposition by default' do url = route(file_path) + "/raw" @@ -252,6 +260,160 @@ describe API::Files do end end + describe 'GET /projects/:id/repository/files/:file_path/blame' do + shared_examples_for 'repository blame files' do + let(:expected_blame_range_sizes) do + [3, 2, 1, 2, 1, 1, 1, 1, 8, 1, 3, 1, 2, 1, 4, 1, 2, 2] + end + + let(:expected_blame_range_commit_ids) do + %w[ + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 570e7b2abdd848b95f2f578043fc23bd6f6fd24d + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 570e7b2abdd848b95f2f578043fc23bd6f6fd24d + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 570e7b2abdd848b95f2f578043fc23bd6f6fd24d + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 913c66a37b4a45b9769037c55c2d238bd0942d2e + ] + end + + it 'returns file attributes in headers' do + head api(route(file_path) + '/blame', current_user), params: params + + expect(response).to have_gitlab_http_status(200) + expect(response.headers['X-Gitlab-File-Path']).to eq(CGI.unescape(file_path)) + expect(response.headers['X-Gitlab-File-Name']).to eq('popen.rb') + expect(response.headers['X-Gitlab-Last-Commit-Id']).to eq('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') + expect(response.headers['X-Gitlab-Content-Sha256']) + .to eq('c440cd09bae50c4632cc58638ad33c6aa375b6109d811e76a9cc3a613c1e8887') + end + + it 'returns blame file attributes as json' do + get api(route(file_path) + '/blame', current_user), params: params + + expect(response).to have_gitlab_http_status(200) + expect(json_response.map { |x| x['lines'].size }).to eq(expected_blame_range_sizes) + expect(json_response.map { |x| x['commit']['id'] }).to eq(expected_blame_range_commit_ids) + range = json_response[0] + expect(range['lines']).to eq(["require 'fileutils'", "require 'open3'", '']) + expect(range['commit']['id']).to eq('913c66a37b4a45b9769037c55c2d238bd0942d2e') + expect(range['commit']['parent_ids']).to eq(['cfe32cf61b73a0d5e9f13e774abde7ff789b1660']) + expect(range['commit']['message']) + .to eq("Files, encoding and much more\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n") + + expect(range['commit']['authored_date']).to eq('2014-02-27T08:14:56.000Z') + expect(range['commit']['author_name']).to eq('Dmitriy Zaporozhets') + expect(range['commit']['author_email']).to eq('dmitriy.zaporozhets@gmail.com') + + expect(range['commit']['committed_date']).to eq('2014-02-27T08:14:56.000Z') + expect(range['commit']['committer_name']).to eq('Dmitriy Zaporozhets') + expect(range['commit']['committer_email']).to eq('dmitriy.zaporozhets@gmail.com') + end + + it 'returns blame file info for files with dots' do + url = route('.gitignore') + '/blame' + + get api(url, current_user), params: params + + expect(response).to have_gitlab_http_status(200) + end + + it 'returns file by commit sha' do + # This file is deleted on HEAD + file_path = 'files%2Fjs%2Fcommit%2Ejs%2Ecoffee' + params[:ref] = '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' + + get api(route(file_path) + '/blame', current_user), params: params + + expect(response).to have_gitlab_http_status(200) + end + + context 'when mandatory params are not given' do + it_behaves_like '400 response' do + let(:request) { get api(route('any%2Ffile/blame'), current_user) } + end + end + + context 'when file_path does not exist' do + let(:params) { { ref: 'master' } } + + it_behaves_like '404 response' do + let(:request) { get api(route('app%2Fmodels%2Fapplication%2Erb/blame'), current_user), params: params } + let(:message) { '404 File Not Found' } + end + end + + context 'when commit does not exist' do + let(:params) { { ref: '1111111111111111111111111111111111111111' } } + + it_behaves_like '404 response' do + let(:request) { get api(route(file_path + '/blame'), current_user), params: params } + let(:message) { '404 Commit Not Found' } + end + end + + context 'when repository is disabled' do + include_context 'disabled repository' + + it_behaves_like '403 response' do + let(:request) { get api(route(file_path + '/blame'), current_user), params: params } + end + end + end + + context 'when unauthenticated', 'and project is public' do + it_behaves_like 'repository blame files' do + let(:project) { create(:project, :public, :repository) } + let(:current_user) { nil } + end + end + + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:request) { get api(route(file_path)), params: params } + let(:message) { '404 Project Not Found' } + end + end + + context 'when authenticated', 'as a developer' do + it_behaves_like 'repository blame files' do + let(:current_user) { user } + end + end + + context 'when authenticated', 'as a guest' do + it_behaves_like '403 response' do + let(:request) { get api(route(file_path) + '/blame', guest), params: params } + end + end + + context 'when PATs are used' do + it 'returns blame file by commit sha' do + token = create(:personal_access_token, scopes: ['read_repository'], user: user) + + # This file is deleted on HEAD + file_path = 'files%2Fjs%2Fcommit%2Ejs%2Ecoffee' + params[:ref] = '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' + + get api(route(file_path) + '/blame', personal_access_token: token), params: params + + expect(response).to have_gitlab_http_status(200) + end + end + end + describe "GET /projects/:id/repository/files/:file_path/raw" do shared_examples_for 'repository raw files' do it 'returns raw file info' do diff --git a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb new file mode 100644 index 00000000000..3982125a38a --- /dev/null +++ b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Adding an AwardEmoji' do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:awardable) { create(:note) } + let(:project) { awardable.project } + let(:emoji_name) { 'thumbsup' } + let(:mutation) do + variables = { + awardable_id: GitlabSchema.id_from_object(awardable).to_s, + name: emoji_name + } + + graphql_mutation(:add_award_emoji, variables) + end + + def mutation_response + graphql_mutation_response(:add_award_emoji) + end + + shared_examples 'a mutation that does not create an AwardEmoji' do + it do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { AwardEmoji.count } + end + end + + context 'when the user does not have permission' do + it_behaves_like 'a mutation that does not create an AwardEmoji' + + 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 the user has permission' do + before do + project.add_developer(current_user) + end + + context 'when the given awardable is not an Awardable' do + let(:awardable) { create(:label) } + + it_behaves_like 'a mutation that does not create an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do + let(:awardable) { create(:system_note) } + + it_behaves_like 'a mutation that does not create an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable an Awardable' do + it 'creates an emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(1) + end + + it 'returns the emoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['awardEmoji']['name']).to eq(emoji_name) + end + + context 'when there were active record validation errors' do + before do + expect_next_instance_of(AwardEmoji) do |award| + expect(award).to receive(:valid?).at_least(:once).and_return(false) + expect(award).to receive_message_chain( + :errors, + :full_messages + ).and_return(['Error 1', 'Error 2']) + end + end + + it_behaves_like 'a mutation that does not create an AwardEmoji' + + it_behaves_like 'a mutation that returns errors in the response', errors: ['Error 1', 'Error 2'] + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb new file mode 100644 index 00000000000..c78f0c7ca27 --- /dev/null +++ b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Removing an AwardEmoji' do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:awardable) { create(:note) } + let(:project) { awardable.project } + let(:emoji_name) { 'thumbsup' } + let(:input) { { awardable_id: GitlabSchema.id_from_object(awardable).to_s, name: emoji_name } } + + let(:mutation) do + graphql_mutation(:remove_award_emoji, input) + end + + def mutation_response + graphql_mutation_response(:remove_award_emoji) + end + + def create_award_emoji(user) + create(:award_emoji, name: emoji_name, awardable: awardable, user: user ) + end + + shared_examples 'a mutation that does not destroy an AwardEmoji' do + it do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { AwardEmoji.count } + end + end + + shared_examples 'a mutation that does not authorize the user' do + it_behaves_like 'a mutation that does not destroy an AwardEmoji' + + 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 the current_user does not own the award emoji' do + let!(:award_emoji) { create_award_emoji(create(:user)) } + + it_behaves_like 'a mutation that does not authorize the user' + end + + context 'when the current_user owns the award emoji' do + let!(:award_emoji) { create_award_emoji(current_user) } + + context 'when the given awardable is not an Awardable' do + let(:awardable) { create(:label) } + + it_behaves_like 'a mutation that does not destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable' do + it 'removes the emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(-1) + end + + it 'returns no errors' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).to be_nil + end + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb new file mode 100644 index 00000000000..31145730f10 --- /dev/null +++ b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Toggling an AwardEmoji' do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:awardable) { create(:note) } + let(:project) { awardable.project } + let(:emoji_name) { 'thumbsup' } + let(:mutation) do + variables = { + awardable_id: GitlabSchema.id_from_object(awardable).to_s, + name: emoji_name + } + + graphql_mutation(:toggle_award_emoji, variables) + end + + def mutation_response + graphql_mutation_response(:toggle_award_emoji) + end + + shared_examples 'a mutation that does not create or destroy an AwardEmoji' do + it do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { AwardEmoji.count } + end + end + + def create_award_emoji(user) + create(:award_emoji, name: emoji_name, awardable: awardable, user: user ) + end + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + context 'when the given awardable is not an Awardable' do + let(:awardable) { create(:label) } + + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do + let(:awardable) { create(:system_note) } + + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable' do + context 'when no emoji has been awarded by the current_user yet' do + # Create an award emoji for another user. This therefore tests that + # toggling is correctly scoped to the user's emoji only. + let!(:award_emoji) { create_award_emoji(create(:user)) } + + it 'creates an emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(1) + end + + it 'returns the emoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['awardEmoji']['name']).to eq(emoji_name) + end + + it 'returns toggledOn as true' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['toggledOn']).to eq(true) + end + + context 'when there were active record validation errors' do + before do + expect_next_instance_of(AwardEmoji) do |award| + expect(award).to receive(:valid?).at_least(:once).and_return(false) + expect(award).to receive_message_chain(:errors, :full_messages).and_return(['Error 1', 'Error 2']) + end + end + + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns errors in the response', errors: ['Error 1', 'Error 2'] + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + end + end + + context 'when an emoji has been awarded by the current_user' do + let!(:award_emoji) { create_award_emoji(current_user) } + + it 'removes the emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(-1) + end + + it 'returns no errors' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).to be_nil + end + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + + it 'returns toggledOn as false' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['toggledOn']).to eq(false) + end + end + end + end + + context 'when the user does not have permission' do + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + 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 +end diff --git a/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb new file mode 100644 index 00000000000..b04fcb9aece --- /dev/null +++ b/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Adding a DiffNote' do + include GraphqlHelpers + + set(:current_user) { create(:user) } + let(:noteable) { create(:merge_request, source_project: project, target_project: project) } + let(:project) { create(:project, :repository) } + let(:diff_refs) { noteable.diff_refs } + let(:mutation) do + variables = { + noteable_id: GitlabSchema.id_from_object(noteable).to_s, + body: 'Body text', + position: { + paths: { + old_path: 'files/ruby/popen.rb', + new_path: 'files/ruby/popen2.rb' + }, + new_line: 14, + base_sha: diff_refs.base_sha, + head_sha: diff_refs.head_sha, + start_sha: diff_refs.start_sha + } + } + + graphql_mutation(:create_diff_note, variables) + end + + def mutation_response + graphql_mutation_response(:create_diff_note) + end + + it_behaves_like 'a Note mutation when the user does not have permission' + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + it_behaves_like 'a Note mutation that creates a Note' + + it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote + + context do + let(:diff_refs) { build(:merge_request).diff_refs } # Allow fake diff refs so arguments are valid + + it_behaves_like 'a Note mutation when the given resource id is not for a Noteable' + end + + it 'returns the note with the correct position' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['body']).to eq('Body text') + mutation_position_response = mutation_response['note']['position'] + expect(mutation_position_response['positionType']).to eq('text') + expect(mutation_position_response['filePath']).to eq('files/ruby/popen2.rb') + expect(mutation_position_response['oldPath']).to eq('files/ruby/popen.rb') + expect(mutation_position_response['newPath']).to eq('files/ruby/popen2.rb') + expect(mutation_position_response['newLine']).to eq(14) + end + end +end diff --git a/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb new file mode 100644 index 00000000000..3ba6c689024 --- /dev/null +++ b/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Adding an image DiffNote' do + include GraphqlHelpers + + set(:current_user) { create(:user) } + let(:noteable) { create(:merge_request, source_project: project, target_project: project) } + let(:project) { create(:project, :repository) } + let(:diff_refs) { noteable.diff_refs } + let(:mutation) do + variables = { + noteable_id: GitlabSchema.id_from_object(noteable).to_s, + body: 'Body text', + position: { + paths: { + old_path: 'files/images/any_image.png', + new_path: 'files/images/any_image2.png' + }, + width: 100, + height: 200, + x: 1, + y: 2, + base_sha: diff_refs.base_sha, + head_sha: diff_refs.head_sha, + start_sha: diff_refs.start_sha + } + } + + graphql_mutation(:create_image_diff_note, variables) + end + + def mutation_response + graphql_mutation_response(:create_image_diff_note) + end + + it_behaves_like 'a Note mutation when the user does not have permission' + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + it_behaves_like 'a Note mutation that creates a Note' + + it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote + + context do + let(:diff_refs) { build(:merge_request).diff_refs } # Allow fake diff refs so arguments are valid + + it_behaves_like 'a Note mutation when the given resource id is not for a Noteable' + end + + it 'returns the note with the correct position' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['body']).to eq('Body text') + mutation_position_response = mutation_response['note']['position'] + expect(mutation_position_response['filePath']).to eq('files/images/any_image2.png') + expect(mutation_position_response['oldPath']).to eq('files/images/any_image.png') + expect(mutation_position_response['newPath']).to eq('files/images/any_image2.png') + expect(mutation_position_response['positionType']).to eq('image') + expect(mutation_position_response['width']).to eq(100) + expect(mutation_position_response['height']).to eq(200) + expect(mutation_position_response['x']).to eq(1) + expect(mutation_position_response['y']).to eq(2) + end + end +end diff --git a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb new file mode 100644 index 00000000000..14aaa430ac9 --- /dev/null +++ b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Adding a Note' do + include GraphqlHelpers + + set(:current_user) { create(:user) } + let(:noteable) { create(:merge_request, source_project: project, target_project: project) } + let(:project) { create(:project) } + let(:discussion) { nil } + let(:mutation) do + variables = { + noteable_id: GitlabSchema.id_from_object(noteable).to_s, + discussion_id: (GitlabSchema.id_from_object(discussion).to_s if discussion), + body: 'Body text' + } + + graphql_mutation(:create_note, variables) + end + + def mutation_response + graphql_mutation_response(:create_note) + end + + it_behaves_like 'a Note mutation when the user does not have permission' + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + it_behaves_like 'a Note mutation that creates a Note' + + it_behaves_like 'a Note mutation when there are active record validation errors' + + it_behaves_like 'a Note mutation when the given resource id is not for a Noteable' + + it 'returns the note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['body']).to eq('Body text') + end + + describe 'creating Notes in reply to a discussion' do + context 'when the user does not have permission to create notes on the discussion' do + let(:discussion) { create(:discussion_note).to_discussion } + + it_behaves_like 'a mutation that returns top-level errors', + errors: ["The discussion does not exist or you don't have permission to perform this action"] + end + + context 'when the user has permission to create notes on the discussion' do + let(:discussion) { create(:discussion_note, project: project).to_discussion } + + it 'creates a Note in a discussion' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['discussion']['id']).to eq(discussion.to_global_id.to_s) + end + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/notes/destroy_spec.rb b/spec/requests/api/graphql/mutations/notes/destroy_spec.rb new file mode 100644 index 00000000000..337a6e6f6e6 --- /dev/null +++ b/spec/requests/api/graphql/mutations/notes/destroy_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Destroying a Note' do + include GraphqlHelpers + + let!(:note) { create(:note) } + let(:mutation) do + variables = { + id: GitlabSchema.id_from_object(note).to_s + } + + graphql_mutation(:destroy_note, variables) + end + + def mutation_response + graphql_mutation_response(:destroy_note) + end + + context 'when the user does not have permission' do + let(: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'] + + it 'does not destroy the Note' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { Note.count } + end + end + + context 'when the user has permission' do + let(:current_user) { note.author } + + it_behaves_like 'a Note mutation when the given resource id is not for a Note' + + it 'destroys the Note' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { Note.count }.by(-1) + end + + it 'returns an empty Note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('note') + expect(mutation_response['note']).to be_nil + end + end +end diff --git a/spec/requests/api/graphql/mutations/notes/update_spec.rb b/spec/requests/api/graphql/mutations/notes/update_spec.rb new file mode 100644 index 00000000000..958f640995a --- /dev/null +++ b/spec/requests/api/graphql/mutations/notes/update_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Updating a Note' do + include GraphqlHelpers + + let!(:note) { create(:note, note: original_body) } + let(:original_body) { 'Initial body text' } + let(:updated_body) { 'Updated body text' } + let(:mutation) do + variables = { + id: GitlabSchema.id_from_object(note).to_s, + body: updated_body + } + + graphql_mutation(:update_note, variables) + end + + def mutation_response + graphql_mutation_response(:update_note) + end + + context 'when the user does not have permission' do + let(: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'] + + it 'does not update the Note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(note.reload.note).to eq(original_body) + end + end + + context 'when the user has permission' do + let(:current_user) { note.author } + + it_behaves_like 'a Note mutation when the given resource id is not for a Note' + + it 'updates the Note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(note.reload.note).to eq(updated_body) + end + + it 'returns the updated Note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['body']).to eq(updated_body) + end + + context 'when there are ActiveRecord validation errors' do + let(:updated_body) { '' } + + it_behaves_like 'a mutation that returns errors in the response', errors: ["Note can't be blank"] + + it 'does not update the Note' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(note.reload.note).to eq(original_body) + end + + it 'returns the Note with its original body' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['note']['body']).to eq(original_body) + end + end + end +end diff --git a/spec/requests/api/graphql/namespace/projects_spec.rb b/spec/requests/api/graphql/namespace/projects_spec.rb index de1cd9586b6..815e9531ecf 100644 --- a/spec/requests/api/graphql/namespace/projects_spec.rb +++ b/spec/requests/api/graphql/namespace/projects_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'getting projects', :nested_groups do +describe 'getting projects' do include GraphqlHelpers let(:group) { create(:group) } @@ -58,9 +58,7 @@ describe 'getting projects', :nested_groups do it 'finds only public projects' do post_graphql(query, current_user: nil) - expect(graphql_data['namespace']['projects']['edges'].size).to eq(1) - project = graphql_data['namespace']['projects']['edges'][0]['node'] - expect(project['id']).to eq(public_project.to_global_id.to_s) + expect(graphql_data['namespace']).to be_nil end end end diff --git a/spec/requests/api/graphql/project/issue/notes_spec.rb b/spec/requests/api/graphql/project/issue/notes_spec.rb new file mode 100644 index 00000000000..bfc89434370 --- /dev/null +++ b/spec/requests/api/graphql/project/issue/notes_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'getting notes for an issue' do + include GraphqlHelpers + + let(:noteable) { create(:issue) } + let(:noteable_data) { graphql_data['project']['issue'] } + + def noteable_query(noteable_fields) + <<~QRY + { + project(fullPath: "#{noteable.project.full_path}") { + issue(iid: "#{noteable.iid}") { + #{noteable_fields} + } + } + } + QRY + end + + it_behaves_like 'exposing regular notes on a noteable in GraphQL' +end diff --git a/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb b/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb new file mode 100644 index 00000000000..e260e4463f4 --- /dev/null +++ b/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'getting notes for a merge request' do + include GraphqlHelpers + + let(:noteable) { create(:merge_request) } + + def noteable_query(noteable_fields) + <<~QRY + { + project(fullPath: "#{noteable.project.full_path}") { + id + mergeRequest(iid: "#{noteable.iid}") { + #{noteable_fields} + } + } + } + QRY + end + let(:noteable_data) { graphql_data['project']['mergeRequest'] } + + it_behaves_like "exposing regular notes on a noteable in GraphQL" + + context 'diff notes on a merge request' do + let(:project) { noteable.project } + let!(:note) { create(:diff_note_on_merge_request, noteable: noteable, project: project) } + let(:user) { note.author } + + let(:query) do + noteable_query( + <<~NOTES + notes { + edges { + node { + #{all_graphql_fields_for('Note')} + } + } + } + NOTES + ) + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: user) + end + end + + it 'includes the note' do + post_graphql(query, current_user: user) + + expect(graphql_data['project']['mergeRequest']['notes']['edges'].last['node']['body']) + .to eq(note.note) + end + + context 'the position of the diffnote' do + it 'includes a correct position' do + post_graphql(query, current_user: user) + + note_data = noteable_data['notes']['edges'].last['node'] + + expect(note_data['position']['positionType']).to eq('text') + expect(note_data['position']['newLine']).to be_present + expect(note_data['position']['x']).not_to be_present + expect(note_data['position']['y']).not_to be_present + expect(note_data['position']['width']).not_to be_present + expect(note_data['position']['height']).not_to be_present + end + + context 'with a note on an image' do + let(:note) { create(:image_diff_note_on_merge_request, noteable: noteable, project: project) } + + it 'includes a correct position' do + post_graphql(query, current_user: user) + + note_data = noteable_data['notes']['edges'].last['node'] + + expect(note_data['position']['positionType']).to eq('image') + expect(note_data['position']['x']).to be_present + expect(note_data['position']['y']).to be_present + expect(note_data['position']['width']).to be_present + expect(note_data['position']['height']).to be_present + expect(note_data['position']['newLine']).not_to be_present + end + end + end + end +end diff --git a/spec/requests/api/graphql/project/project_statistics_spec.rb b/spec/requests/api/graphql/project/project_statistics_spec.rb index 8683fa1f390..14a3f37b779 100644 --- a/spec/requests/api/graphql/project/project_statistics_spec.rb +++ b/spec/requests/api/graphql/project/project_statistics_spec.rb @@ -34,10 +34,10 @@ describe 'rendering namespace statistics' do context 'when the project is public' do let(:project) { create(:project, :public) } - it 'includes the statistics regardless of the user' do + it 'hides statistics for unauthenticated requests' do post_graphql(query, current_user: nil) - expect(graphql_data['project']['statistics']).to be_present + expect(graphql_data['project']['statistics']).to be_blank end end end diff --git a/spec/requests/api/graphql/project/repository_spec.rb b/spec/requests/api/graphql/project/repository_spec.rb index 67af612a4a0..261433a3d6a 100644 --- a/spec/requests/api/graphql/project/repository_spec.rb +++ b/spec/requests/api/graphql/project/repository_spec.rb @@ -34,4 +34,28 @@ describe 'getting a repository in a project' do expect(graphql_data['project']).to be(nil) end end + + context 'when the repository is only accessible to members' do + let(:project) do + create(:project, :public, :repository, repository_access_level: ProjectFeature::PRIVATE) + end + + it 'returns a repository for the owner' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']['repository']).not_to be_nil + end + + it 'returns nil for the repository for other users' do + post_graphql(query, current_user: create(:user)) + + expect(graphql_data['project']['repository']).to be_nil + end + + it 'returns nil for the repository for other users' do + post_graphql(query, current_user: nil) + + expect(graphql_data['project']['repository']).to be_nil + end + end end diff --git a/spec/requests/api/graphql/project/tree/tree_spec.rb b/spec/requests/api/graphql/project/tree/tree_spec.rb index b07aa1e12d3..94128cc21ee 100644 --- a/spec/requests/api/graphql/project/tree/tree_spec.rb +++ b/spec/requests/api/graphql/project/tree/tree_spec.rb @@ -33,6 +33,12 @@ describe 'getting a tree in a project' do expect(graphql_data['project']['repository']['tree']['submodules']['edges']).to eq([]) expect(graphql_data['project']['repository']['tree']['blobs']['edges']).to eq([]) end + + it 'returns null commit' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']['repository']['last_commit']).to be_nil + end end context 'when ref does not exist' do @@ -45,6 +51,12 @@ describe 'getting a tree in a project' do expect(graphql_data['project']['repository']['tree']['submodules']['edges']).to eq([]) expect(graphql_data['project']['repository']['tree']['blobs']['edges']).to eq([]) end + + it 'returns null commit' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']['repository']['last_commit']).to be_nil + end end context 'when ref and path exist' do @@ -61,6 +73,12 @@ describe 'getting a tree in a project' do expect(graphql_data['project']['repository']['tree']['blobs']['edges'].size).to be > 0 expect(graphql_data['project']['repository']['tree']['submodules']['edges'].size).to be > 0 end + + it 'returns tree latest commit' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']['repository']['tree']['lastCommit']).to be_present + end end context 'when current user is nil' do diff --git a/spec/requests/api/graphql/tasks/task_completion_status_spec.rb b/spec/requests/api/graphql/tasks/task_completion_status_spec.rb new file mode 100644 index 00000000000..c457a6d7c25 --- /dev/null +++ b/spec/requests/api/graphql/tasks/task_completion_status_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +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' + + set(:user1) { create(:user) } + set(:project) { create(:project, :repository, :public) } + + let(:fields) do + <<~QUERY + taskCompletionStatus { + count, + completedCount + } + QUERY + end + + def create_task_completion_status_query_for(type, iid) + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field(type, { iid: iid }, fields) + ) + end + + shared_examples_for 'graphql task completion status provider' do |type| + it 'returns the expected task completion status' do + post_graphql(create_task_completion_status_query_for(type, item.iid), current_user: user1) + + expect(response).to have_gitlab_http_status(200) + + task_completion_status = graphql_data.dig('project', type, 'taskCompletionStatus') + expect(task_completion_status).not_to be_nil + expect(task_completion_status['count']).to eq(item.task_completion_status[:count]) + expect(task_completion_status['completedCount']).to eq(item.task_completion_status[:completed_count]) + end + end + + [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 + let(:item) { create(:issue, project: project, description: desc) } + end + end + + context 'when type is merge request' do + it_behaves_like 'graphql task completion status provider', 'mergeRequest' do + let(:item) { create(:merge_request, author: user1, source_project: project, description: desc) } + end + end + end + end +end diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index 656d6f8b50b..54401ec4085 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -6,16 +6,6 @@ describe 'GraphQL' do let(:query) { graphql_query_for('echo', 'text' => 'Hello world' ) } - context 'graphql is disabled by feature flag' do - before do - stub_feature_flags(graphql: false) - end - - it 'does not generate a route for GraphQL' do - expect { post_graphql(query) }.to raise_error(ActionController::RoutingError) - end - end - context 'logging' do shared_examples 'logging a graphql query' do let(:expected_params) do @@ -131,4 +121,35 @@ describe 'GraphQL' do end end end + + describe 'testing for Gitaly calls' do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + + let(:query) do + graphql_query_for('project', { 'fullPath' => project.full_path }, %w(id)) + end + + before do + project.add_developer(user) + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: user) + end + end + + context 'when Gitaly is called' do + before do + allow(Gitlab::GitalyClient).to receive(:get_request_count).and_return(1, 2) + end + + it "logs a warning that the 'calls_gitaly' field declaration is missing" do + expect(Gitlab::Sentry).to receive(:track_exception).once + + post_graphql(query, current_user: user) + end + end + end end diff --git a/spec/requests/api/group_clusters_spec.rb b/spec/requests/api/group_clusters_spec.rb new file mode 100644 index 00000000000..46e3dd650cc --- /dev/null +++ b/spec/requests/api/group_clusters_spec.rb @@ -0,0 +1,452 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::GroupClusters do + include KubernetesHelpers + + let(:current_user) { create(:user) } + let(:developer_user) { create(:user) } + let(:group) { create(:group, :private) } + + before do + group.add_developer(developer_user) + group.add_maintainer(current_user) + end + + describe 'GET /groups/:id/clusters' do + let!(:extra_cluster) { create(:cluster, :provided_by_gcp, :group) } + + let!(:clusters) do + create_list(:cluster, 5, :provided_by_gcp, :group, :production_environment, + groups: [group]) + end + + context 'non-authorized user' do + it 'responds with 403' do + get api("/groups/#{group.id}/clusters", developer_user) + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'authorized user' do + before do + get api("/groups/#{group.id}/clusters", current_user) + end + + it 'responds with 200' do + expect(response).to have_gitlab_http_status(200) + end + + it 'includes pagination headers' do + expect(response).to include_pagination_headers + end + + it 'only include authorized clusters' do + cluster_ids = json_response.map { |cluster| cluster['id'] } + + expect(cluster_ids).to match_array(clusters.pluck(:id)) + expect(cluster_ids).not_to include(extra_cluster.id) + end + end + end + + describe 'GET /groups/:id/clusters/:cluster_id' do + let(:cluster_id) { cluster.id } + + let(:platform_kubernetes) do + create(:cluster_platform_kubernetes, :configured) + end + + let(:cluster) do + create(:cluster, :group, :provided_by_gcp, :with_domain, + platform_kubernetes: platform_kubernetes, + user: current_user, + groups: [group]) + end + + context 'non-authorized user' do + it 'responds with 403' do + get api("/groups/#{group.id}/clusters/#{cluster_id}", developer_user) + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'authorized user' do + before do + get api("/groups/#{group.id}/clusters/#{cluster_id}", current_user) + end + + it 'returns specific cluster' do + expect(json_response['id']).to eq(cluster.id) + end + + it 'returns cluster information' do + expect(json_response['provider_type']).to eq('gcp') + expect(json_response['platform_type']).to eq('kubernetes') + expect(json_response['environment_scope']).to eq('*') + expect(json_response['cluster_type']).to eq('group_type') + expect(json_response['domain']).to eq('example.com') + end + + it 'returns group information' do + cluster_group = json_response['group'] + + expect(cluster_group['id']).to eq(group.id) + expect(cluster_group['name']).to eq(group.name) + expect(cluster_group['web_url']).to eq(group.web_url) + end + + it 'returns kubernetes platform information' do + platform = json_response['platform_kubernetes'] + + expect(platform['api_url']).to eq('https://kubernetes.example.com') + expect(platform['ca_cert']).to be_present + end + + it 'returns user information' do + user = json_response['user'] + + expect(user['id']).to eq(current_user.id) + expect(user['username']).to eq(current_user.username) + end + + it 'returns GCP provider information' do + gcp_provider = json_response['provider_gcp'] + + expect(gcp_provider['cluster_id']).to eq(cluster.id) + expect(gcp_provider['status_name']).to eq('created') + expect(gcp_provider['gcp_project_id']).to eq('test-gcp-project') + expect(gcp_provider['zone']).to eq('us-central1-a') + expect(gcp_provider['machine_type']).to eq('n1-standard-2') + expect(gcp_provider['num_nodes']).to eq(3) + expect(gcp_provider['endpoint']).to eq('111.111.111.111') + end + + context 'when cluster has no provider' do + let(:cluster) do + create(:cluster, :group, :provided_by_user, + groups: [group]) + end + + it 'does not include GCP provider info' do + expect(json_response['provider_gcp']).not_to be_present + end + end + + context 'with non-existing cluster' do + let(:cluster_id) { 123 } + + it 'returns 404' do + expect(response).to have_gitlab_http_status(404) + end + end + end + end + + shared_context 'kubernetes calls stubbed' do + before do + stub_kubeclient_discover(api_url) + end + end + + describe 'POST /groups/:id/clusters/user' do + include_context 'kubernetes calls stubbed' + + let(:api_url) { 'https://kubernetes.example.com' } + let(:authorization_type) { 'rbac' } + + let(:platform_kubernetes_attributes) do + { + api_url: api_url, + token: 'sample-token', + authorization_type: authorization_type + } + end + + let(:cluster_params) do + { + name: 'test-cluster', + domain: 'domain.example.com', + managed: false, + platform_kubernetes_attributes: platform_kubernetes_attributes + } + end + + context 'non-authorized user' do + it 'responds with 403' do + post api("/groups/#{group.id}/clusters/user", developer_user), params: cluster_params + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'authorized user' do + before do + post api("/groups/#{group.id}/clusters/user", current_user), params: cluster_params + end + + context 'with valid params' do + it 'responds with 201' do + expect(response).to have_gitlab_http_status(201) + end + + it 'creates a new Cluster::Cluster' do + cluster_result = Clusters::Cluster.find(json_response["id"]) + platform_kubernetes = cluster_result.platform + + expect(cluster_result).to be_user + expect(cluster_result).to be_kubernetes + expect(cluster_result.group).to eq(group) + expect(cluster_result.name).to eq('test-cluster') + expect(cluster_result.domain).to eq('domain.example.com') + expect(cluster_result.managed).to be_falsy + expect(platform_kubernetes.rbac?).to be_truthy + expect(platform_kubernetes.api_url).to eq(api_url) + expect(platform_kubernetes.token).to eq('sample-token') + end + end + + context 'when user does not indicate authorization type' do + let(:platform_kubernetes_attributes) do + { + api_url: api_url, + token: 'sample-token' + } + end + + it 'defaults to RBAC' do + cluster_result = Clusters::Cluster.find(json_response['id']) + + expect(cluster_result.platform_kubernetes.rbac?).to be_truthy + end + end + + context 'when user sets authorization type as ABAC' do + let(:authorization_type) { 'abac' } + + it 'creates an ABAC cluster' do + cluster_result = Clusters::Cluster.find(json_response['id']) + + expect(cluster_result.platform.abac?).to be_truthy + end + end + + context 'with invalid params' do + let(:api_url) { 'invalid_api_url' } + + it 'responds with 400' do + expect(response).to have_gitlab_http_status(400) + end + + it 'does not create a new Clusters::Cluster' do + expect(group.reload.clusters).to be_empty + end + + it 'returns validation errors' do + expect(json_response['message']['platform_kubernetes.api_url'].first).to be_present + end + end + end + + context 'when user tries to add multiple clusters' do + before do + create(:cluster, :provided_by_gcp, :group, + groups: [group]) + + post api("/groups/#{group.id}/clusters/user", current_user), params: cluster_params + end + + it 'responds with 400' do + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']['base'].first).to include('Instance does not support multiple Kubernetes clusters') + end + end + + context 'non-authorized user' do + before do + post api("/groups/#{group.id}/clusters/user", developer_user), params: cluster_params + end + + it 'responds with 403' do + expect(response).to have_gitlab_http_status(403) + + expect(json_response['message']).to eq('403 Forbidden') + end + end + end + + describe 'PUT /groups/:id/clusters/:cluster_id' do + include_context 'kubernetes calls stubbed' + + let(:api_url) { 'https://kubernetes.example.com' } + + let(:update_params) do + { + domain: domain, + platform_kubernetes_attributes: platform_kubernetes_attributes + } + end + + let(:domain) { 'new-domain.com' } + let(:platform_kubernetes_attributes) { {} } + + let(:cluster) do + create(:cluster, :group, :provided_by_gcp, + groups: [group], domain: 'old-domain.com') + end + + context 'non-authorized user' do + it 'responds with 403' do + put api("/groups/#{group.id}/clusters/#{cluster.id}", developer_user), params: update_params + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'authorized user' do + before do + put api("/groups/#{group.id}/clusters/#{cluster.id}", current_user), params: update_params + + cluster.reload + end + + context 'with valid params' do + it 'responds with 200' do + expect(response).to have_gitlab_http_status(200) + end + + it 'updates cluster attributes' do + expect(cluster.domain).to eq('new-domain.com') + end + end + + context 'with invalid params' do + let(:domain) { 'invalid domain' } + + it 'responds with 400' do + expect(response).to have_gitlab_http_status(400) + end + + it 'does not update cluster attributes' do + expect(cluster.domain).to eq('old-domain.com') + end + + it 'returns validation errors' do + expect(json_response['message']['domain'].first).to match('contains invalid characters (valid characters: [a-z0-9\\-])') + end + end + + context 'with a GCP cluster' do + context 'when user tries to change GCP specific fields' do + let(:platform_kubernetes_attributes) do + { + api_url: 'https://new-api-url.com', + token: 'new-sample-token' + } + end + + it 'responds with 400' do + expect(response).to have_gitlab_http_status(400) + end + + it 'returns validation error' do + expect(json_response['message']['platform_kubernetes.base'].first).to eq('Cannot modify managed Kubernetes cluster') + end + end + + context 'when user tries to change domain' do + let(:domain) { 'new-domain.com' } + + it 'responds with 200' do + expect(response).to have_gitlab_http_status(200) + end + end + end + + context 'with an user cluster' do + let(:api_url) { 'https://new-api-url.com' } + + let(:cluster) do + create(:cluster, :group, :provided_by_user, + groups: [group]) + end + + let(:platform_kubernetes_attributes) do + { + api_url: api_url, + token: 'new-sample-token' + } + end + + let(:update_params) do + { + name: 'new-name', + platform_kubernetes_attributes: platform_kubernetes_attributes + } + end + + it 'responds with 200' do + expect(response).to have_gitlab_http_status(200) + end + + it 'updates platform kubernetes attributes' do + platform_kubernetes = cluster.platform_kubernetes + + expect(cluster.name).to eq('new-name') + expect(platform_kubernetes.api_url).to eq('https://new-api-url.com') + expect(platform_kubernetes.token).to eq('new-sample-token') + end + end + + context 'with a cluster that does not belong to user' do + let(:cluster) { create(:cluster, :group, :provided_by_user) } + + it 'responds with 404' do + expect(response).to have_gitlab_http_status(404) + end + end + end + end + + describe 'DELETE /groups/:id/clusters/:cluster_id' do + let(:cluster_params) { { cluster_id: cluster.id } } + + let(:cluster) do + create(:cluster, :group, :provided_by_gcp, + groups: [group]) + end + + context 'non-authorized user' do + it 'responds with 403' do + delete api("/groups/#{group.id}/clusters/#{cluster.id}", developer_user), params: cluster_params + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'authorized user' do + before do + delete api("/groups/#{group.id}/clusters/#{cluster.id}", current_user), params: cluster_params + end + + it 'responds with 204' do + expect(response).to have_gitlab_http_status(204) + end + + it 'deletes the cluster' do + expect(Clusters::Cluster.exists?(id: cluster.id)).to be_falsy + end + + context 'with a cluster that does not belong to user' do + let(:cluster) { create(:cluster, :group, :provided_by_user) } + + it 'responds with 404' do + expect(response).to have_gitlab_http_status(404) + end + end + end + end +end diff --git a/spec/requests/api/group_container_repositories_spec.rb b/spec/requests/api/group_container_repositories_spec.rb new file mode 100644 index 00000000000..0a41e455d01 --- /dev/null +++ b/spec/requests/api/group_container_repositories_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::GroupContainerRepositories do + set(:group) { create(:group, :private) } + set(:project) { create(:project, :private, group: group) } + let(:reporter) { create(:user) } + let(:guest) { create(:user) } + + let(:root_repository) { create(:container_repository, :root, project: project) } + let(:test_repository) { create(:container_repository, project: project) } + + let(:users) do + { + anonymous: nil, + guest: guest, + reporter: reporter + } + end + + let(:api_user) { reporter } + + before do + group.add_reporter(reporter) + group.add_guest(guest) + + stub_feature_flags(container_registry_api: true) + stub_container_registry_config(enabled: true) + + root_repository + test_repository + end + + describe 'GET /groups/:id/registry/repositories' do + let(:url) { "/groups/#{group.id}/registry/repositories" } + + subject { get api(url, api_user) } + + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + it_behaves_like 'returns repositories for allowed users', :reporter, 'group' do + let(:object) { group } + end + + context 'with invalid group id' do + let(:url) { '/groups/123412341234/registry/repositories' } + + it 'returns not found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/requests/api/group_labels_spec.rb b/spec/requests/api/group_labels_spec.rb index 3769f8b78e4..0be4e2e9121 100644 --- a/spec/requests/api/group_labels_spec.rb +++ b/spec/requests/api/group_labels_spec.rb @@ -14,12 +14,25 @@ describe API::GroupLabels do get api("/groups/#{group.id}/labels", user) expect(response).to have_gitlab_http_status(200) - expect(response).to match_response_schema('public_api/v4/group_labels') expect(response).to include_pagination_headers expect(json_response).to be_an Array + expect(json_response).to all(match_schema('public_api/v4/labels/label')) expect(json_response.size).to eq(2) expect(json_response.map {|r| r['name'] }).to contain_exactly('feature', 'bug') end + + context 'when the with_counts parameter is set' do + it 'includes counts in the response' do + get api("/groups/#{group.id}/labels", user), params: { with_counts: true } + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response).to all(match_schema('public_api/v4/labels/label_with_counts')) + expect(json_response.size).to eq(2) + expect(json_response.map { |r| r['open_issues_count'] }).to contain_exactly(0, 0) + end + end end describe 'POST /groups/:id/labels' do @@ -94,7 +107,7 @@ describe API::GroupLabels do expect(response).to have_gitlab_http_status(400) end - it "does not delete parent's group labels", :nested_groups do + it "does not delete parent's group labels" do subgroup = create(:group, parent: group) subgroup_label = create(:group_label, title: 'feature', group: subgroup) @@ -127,7 +140,7 @@ describe API::GroupLabels do expect(json_response['description']).to eq('test') end - it "does not update parent's group label", :nested_groups do + it "does not update parent's group label" do subgroup = create(:group, parent: group) subgroup_label = create(:group_label, title: 'feature', group: subgroup) diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index c41408fba65..50f36141aed 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -530,7 +530,7 @@ describe API::Groups do expect(json_response.length).to eq(2) end - it "returns projects including those in subgroups", :nested_groups do + it "returns projects including those in subgroups" do subgroup = create(:group, parent: group1) create(:project, group: subgroup) create(:project, group: subgroup) @@ -642,7 +642,7 @@ describe API::Groups do end end - describe 'GET /groups/:id/subgroups', :nested_groups do + describe 'GET /groups/:id/subgroups' do let!(:subgroup1) { create(:group, parent: group1) } let!(:subgroup2) { create(:group, :private, parent: group1) } let!(:subgroup3) { create(:group, :private, parent: group2) } @@ -786,7 +786,7 @@ describe API::Groups do expect(response).to have_gitlab_http_status(403) end - context 'as owner', :nested_groups do + context 'as owner' do before do group2.add_owner(user1) end @@ -798,15 +798,15 @@ describe API::Groups do end end - context 'as maintainer', :nested_groups do + context 'as maintainer' do before do group2.add_maintainer(user1) end - it 'cannot create subgroups' do + it 'can create subgroups' do post api("/groups", user1), params: { parent_id: group2.id, name: 'foo', path: 'foo' } - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(201) end end end @@ -825,7 +825,7 @@ describe API::Groups do expect(json_response["visibility"]).to eq(Gitlab::VisibilityLevel.string_level(Gitlab::CurrentSettings.current_application_settings.default_group_visibility)) end - it "creates a nested group", :nested_groups do + it "creates a nested group" do parent = create(:group) parent.add_owner(user3) group = attributes_for(:group, { parent_id: parent.id }) diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index ed907841bd8..1c69f5dbb67 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -226,10 +226,8 @@ describe API::Helpers do allow_any_instance_of(self.class).to receive(:rack_response) allow(Gitlab::Sentry).to receive(:enabled?).and_return(true) - stub_application_setting( - sentry_enabled: true, - sentry_dsn: "dummy://12345:67890@sentry.localdomain/sentry/42" - ) + stub_sentry_settings + configure_sentry Raven.client.configuration.encoding = 'json' end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index fcbff19bd61..3ab1818bebb 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -474,7 +474,7 @@ describe API::Internal do 'ssh', { authentication_abilities: [:read_project, :download_code, :push_code], - namespace_path: project.namespace.name, + namespace_path: project.namespace.path, project_path: project.path, redirected_path: nil } @@ -753,7 +753,7 @@ describe API::Internal do end describe 'GET /internal/merge_request_urls' do - let(:repo_name) { "#{project.namespace.name}/#{project.path}" } + let(:repo_name) { "#{project.full_path}" } let(:changes) { URI.escape("#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch") } before do @@ -765,7 +765,7 @@ describe API::Internal do expect(json_response).to match [{ "branch_name" => "new_branch", - "url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", + "url" => "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", "new_merge_request" => true }] end @@ -786,7 +786,7 @@ describe API::Internal do expect(json_response).to match [{ "branch_name" => "new_branch", - "url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", + "url" => "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", "new_merge_request" => true }] end @@ -927,7 +927,7 @@ describe API::Internal do expect(json_response['merge_request_urls']).to match [{ "branch_name" => branch_name, - "url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{branch_name}", + "url" => "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{branch_name}", "new_merge_request" => true }] end @@ -970,7 +970,7 @@ describe API::Internal do expect(json_response['merge_request_urls']).to match [{ 'branch_name' => branch_name, - 'url' => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/1", + 'url' => "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/1", 'new_merge_request' => false }] end diff --git a/spec/requests/api/issues/get_group_issues_spec.rb b/spec/requests/api/issues/get_group_issues_spec.rb index 8b02cf56e9f..c487471e4a1 100644 --- a/spec/requests/api/issues/get_group_issues_spec.rb +++ b/spec/requests/api/issues/get_group_issues_spec.rb @@ -23,7 +23,11 @@ describe API::Issues do describe 'GET /groups/:id/issues' do let!(:group) { create(:group) } - let!(:group_project) { create(:project, :public, creator_id: user.id, namespace: group) } + let!(:group_project) { create(:project, :public, :repository, creator_id: user.id, namespace: group) } + let!(:private_mrs_project) do + create(:project, :public, :repository, creator_id: user.id, namespace: group, merge_requests_access_level: ProjectFeature::PRIVATE) + end + let!(:group_closed_issue) do create :closed_issue, author: user, @@ -78,7 +82,7 @@ describe API::Issues do end end - context 'when group has subgroups', :nested_groups do + context 'when group has subgroups' do let(:subgroup_1) { create(:group, parent: group) } let(:subgroup_2) { create(:group, parent: subgroup_1) } @@ -234,6 +238,30 @@ describe API::Issues do it_behaves_like 'group issues statistics' end end + + context "when returns issue merge_requests_count for different access levels" do + let!(:merge_request1) do + create(:merge_request, + :simple, + author: user, + source_project: private_mrs_project, + target_project: private_mrs_project, + description: "closes #{group_issue.to_reference(private_mrs_project)}") + end + let!(:merge_request2) do + create(:merge_request, + :simple, + author: user, + source_project: group_project, + target_project: group_project, + description: "closes #{group_issue.to_reference}") + end + + it_behaves_like 'accessible merge requests count' do + let(:api_url) { base_url } + let(:target_issue) { group_issue } + end + end end end @@ -314,6 +342,14 @@ describe API::Issues do group_project.add_reporter(user) end + it 'exposes known attributes' do + get api(base_url, admin) + + expect(response).to have_gitlab_http_status(200) + expect(json_response.last.keys).to include(*%w(id iid project_id title description)) + expect(json_response.last).not_to have_key('subscribed') + end + it 'returns all group issues (including opened and closed)' do get api(base_url, admin) diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb index 0b0f754ab57..521d6b88734 100644 --- a/spec/requests/api/issues/get_project_issues_spec.rb +++ b/spec/requests/api/issues/get_project_issues_spec.rb @@ -4,8 +4,9 @@ require 'spec_helper' describe API::Issues do set(:user) { create(:user) } - set(:project) do - create(:project, :public, creator_id: user.id, namespace: user.namespace) + set(:project) { create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace) } + set(:private_mrs_project) do + create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace, merge_requests_access_level: ProjectFeature::PRIVATE) end let(:user2) { create(:user) } @@ -60,9 +61,28 @@ describe API::Issues do let(:no_milestone_title) { 'None' } let(:any_milestone_title) { 'Any' } + let!(:merge_request1) do + create(:merge_request, + :simple, + author: user, + source_project: project, + target_project: project, + description: "closes #{issue.to_reference}") + end + let!(:merge_request2) do + create(:merge_request, + :simple, + author: user, + source_project: private_mrs_project, + target_project: private_mrs_project, + description: "closes #{issue.to_reference(private_mrs_project)}") + end + before(:all) do project.add_reporter(user) project.add_guest(guest) + private_mrs_project.add_reporter(user) + private_mrs_project.add_guest(guest) end before do @@ -257,6 +277,11 @@ describe API::Issues do expect_paginated_array_response(issue.id) end + it_behaves_like 'accessible merge requests count' do + let(:api_url) { "/projects/#{project.id}/issues" } + let(:target_issue) { issue } + end + context 'with labeled issues' do let(:label_b) { create(:label, title: 'foo', project: project) } let(:label_c) { create(:label, title: 'bar', project: project) } @@ -550,6 +575,7 @@ describe API::Issues do expect(json_response['assignee']).to be_a Hash expect(json_response['author']).to be_a Hash expect(json_response['confidential']).to be_falsy + expect(json_response['subscribed']).to be_truthy end it 'exposes the closed_at attribute' do @@ -636,34 +662,26 @@ describe API::Issues do expect(json_response['iid']).to eq(confidential_issue.iid) end end - end - - describe 'GET :id/issues/:issue_iid/closed_by' do - let(:merge_request) do - create(:merge_request, - :simple, - author: user, - source_project: project, - target_project: project, - description: "closes #{issue.to_reference}") - end - before do - create(:merge_requests_closing_issues, issue: issue, merge_request: merge_request) + it_behaves_like 'accessible merge requests count' do + let(:api_url) { "/projects/#{project.id}/issues/#{issue.iid}" } + let(:target_issue) { issue } end + end + describe 'GET :id/issues/:issue_iid/closed_by' do context 'when unauthenticated' do it 'return public project issues' do get api("/projects/#{project.id}/issues/#{issue.iid}/closed_by") - expect_paginated_array_response(merge_request.id) + expect_paginated_array_response(merge_request1.id) end end it 'returns merge requests that will close issue on merge' do get api("/projects/#{project.id}/issues/#{issue.iid}/closed_by", user) - expect_paginated_array_response(merge_request.id) + expect_paginated_array_response(merge_request1.id) end context 'when no merge requests will close issue' do @@ -721,13 +739,6 @@ describe API::Issues do end it 'returns merge requests that mentioned a issue' do - create(:merge_request, - :simple, - author: user, - source_project: project, - target_project: project, - description: 'Some description') - get_related_merge_requests(project.id, issue.iid, user) expect_paginated_array_response(related_mr.id) diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index f32ffd1c77b..27cf66629fe 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -4,8 +4,9 @@ require 'spec_helper' describe API::Issues do set(:user) { create(:user) } - set(:project) do - create(:project, :public, creator_id: user.id, namespace: user.namespace) + set(:project) { create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace) } + set(:private_mrs_project) do + create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace, merge_requests_access_level: ProjectFeature::PRIVATE) end let(:user2) { create(:user) } @@ -63,6 +64,8 @@ describe API::Issues do before(:all) do project.add_reporter(user) project.add_guest(guest) + private_mrs_project.add_reporter(user) + private_mrs_project.add_guest(guest) end before do @@ -213,6 +216,10 @@ describe API::Issues do expect_paginated_array_response([issue.id, closed_issue.id]) expect(json_response.first['title']).to eq(issue.title) expect(json_response.last).to have_key('web_url') + # Calculating the value of subscribed field triggers Markdown + # processing. We can't do that for multiple issues / merge + # requests in a single API request. + expect(json_response.last).not_to have_key('subscribed') end it 'returns an array of closed issues' do @@ -725,6 +732,30 @@ describe API::Issues do end end end + + context "when returns issue merge_requests_count for different access levels" do + let!(:merge_request1) do + create(:merge_request, + :simple, + author: user, + source_project: private_mrs_project, + target_project: private_mrs_project, + description: "closes #{issue.to_reference(private_mrs_project)}") + end + let!(:merge_request2) do + create(:merge_request, + :simple, + author: user, + source_project: project, + target_project: project, + description: "closes #{issue.to_reference}") + end + + it_behaves_like 'accessible merge requests count' do + let(:api_url) { "/issues" } + let(:target_issue) { issue } + end + end end describe 'DELETE /projects/:id/issues/:issue_iid' do diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 518181e4d93..ad0974f55a3 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -11,65 +11,76 @@ describe API::Labels do end describe 'GET /projects/:id/labels' do - it 'returns all available labels to the project' do - group = create(:group) - group_label = create(:group_label, title: 'feature', group: group) - project.update(group: group) - create(:labeled_issue, project: project, labels: [group_label], author: user) - create(:labeled_issue, project: project, labels: [label1], author: user, state: :closed) - create(:labeled_merge_request, labels: [priority_label], author: user, source_project: project ) + let(:group) { create(:group) } + let!(:group_label) { create(:group_label, title: 'feature', group: group) } - expected_keys = %w( - id name color text_color description - open_issues_count closed_issues_count open_merge_requests_count - subscribed priority is_project_label - ) + before do + project.update!(group: group) + end + it 'returns all available labels to the project' do get api("/projects/#{project.id}/labels", user) expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers - expect(json_response).to be_an Array + expect(json_response).to all(match_schema('public_api/v4/labels/project_label')) expect(json_response.size).to eq(3) - expect(json_response.first.keys).to match_array expected_keys expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, priority_label.name, label1.name]) + end - label1_response = json_response.find { |l| l['name'] == label1.title } - group_label_response = json_response.find { |l| l['name'] == group_label.title } - priority_label_response = json_response.find { |l| l['name'] == priority_label.title } - - expect(label1_response['open_issues_count']).to eq(0) - expect(label1_response['closed_issues_count']).to eq(1) - expect(label1_response['open_merge_requests_count']).to eq(0) - expect(label1_response['name']).to eq(label1.name) - expect(label1_response['color']).to be_present - expect(label1_response['text_color']).to be_present - expect(label1_response['description']).to be_nil - expect(label1_response['priority']).to be_nil - expect(label1_response['subscribed']).to be_falsey - expect(label1_response['is_project_label']).to be_truthy - - expect(group_label_response['open_issues_count']).to eq(1) - expect(group_label_response['closed_issues_count']).to eq(0) - expect(group_label_response['open_merge_requests_count']).to eq(0) - expect(group_label_response['name']).to eq(group_label.name) - expect(group_label_response['color']).to be_present - expect(group_label_response['text_color']).to be_present - expect(group_label_response['description']).to be_nil - expect(group_label_response['priority']).to be_nil - expect(group_label_response['subscribed']).to be_falsey - expect(group_label_response['is_project_label']).to be_falsey - - expect(priority_label_response['open_issues_count']).to eq(0) - expect(priority_label_response['closed_issues_count']).to eq(0) - expect(priority_label_response['open_merge_requests_count']).to eq(1) - expect(priority_label_response['name']).to eq(priority_label.name) - expect(priority_label_response['color']).to be_present - expect(priority_label_response['text_color']).to be_present - expect(priority_label_response['description']).to be_nil - expect(priority_label_response['priority']).to eq(3) - expect(priority_label_response['subscribed']).to be_falsey - expect(priority_label_response['is_project_label']).to be_truthy + context 'when the with_counts parameter is set' do + before do + create(:labeled_issue, project: project, labels: [group_label], author: user) + create(:labeled_issue, project: project, labels: [label1], author: user, state: :closed) + create(:labeled_merge_request, labels: [priority_label], author: user, source_project: project ) + end + + it 'includes counts in the response' do + get api("/projects/#{project.id}/labels", user), params: { with_counts: true } + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to all(match_schema('public_api/v4/labels/project_label_with_counts')) + expect(json_response.size).to eq(3) + expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, priority_label.name, label1.name]) + + label1_response = json_response.find { |l| l['name'] == label1.title } + group_label_response = json_response.find { |l| l['name'] == group_label.title } + priority_label_response = json_response.find { |l| l['name'] == priority_label.title } + + expect(label1_response).to include('open_issues_count' => 0, + 'closed_issues_count' => 1, + 'open_merge_requests_count' => 0, + 'name' => label1.name, + 'description' => nil, + 'color' => a_string_matching(/^#\h{6}$/), + 'text_color' => a_string_matching(/^#\h{6}$/), + 'priority' => nil, + 'subscribed' => false, + 'is_project_label' => true) + + expect(group_label_response).to include('open_issues_count' => 1, + 'closed_issues_count' => 0, + 'open_merge_requests_count' => 0, + 'name' => group_label.name, + 'description' => nil, + 'color' => a_string_matching(/^#\h{6}$/), + 'text_color' => a_string_matching(/^#\h{6}$/), + 'priority' => nil, + 'subscribed' => false, + 'is_project_label' => false) + + expect(priority_label_response).to include('open_issues_count' => 0, + 'closed_issues_count' => 0, + 'open_merge_requests_count' => 1, + 'name' => priority_label.name, + 'description' => nil, + 'color' => a_string_matching(/^#\h{6}$/), + 'text_color' => a_string_matching(/^#\h{6}$/), + 'priority' => 3, + 'subscribed' => false, + 'is_project_label' => true) + end end end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 55f38079b1f..26f6e705528 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -99,7 +99,7 @@ describe API::Members do end end - describe 'GET /:source_type/:id/members/all', :nested_groups do + describe 'GET /:source_type/:id/members/all' do let(:nested_user) { create(:user) } let(:project_user) { create(:user) } let(:linked_group_user) { create(:user) } @@ -238,7 +238,7 @@ describe API::Members do end context 'access levels' do - it 'does not create the member if group level is higher', :nested_groups do + it 'does not create the member if group level is higher' do parent = create(:group) group.update(parent: parent) @@ -252,7 +252,7 @@ describe API::Members do expect(json_response['message']['access_level']).to eq(["should be greater than or equal to Developer inherited membership from group #{parent.name}"]) end - it 'creates the member if group level is lower', :nested_groups do + it 'creates the member if group level is lower' do parent = create(:group) group.update(parent: parent) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 9f9180bc8c9..15d6db42760 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -723,7 +723,7 @@ describe API::MergeRequests do it_behaves_like 'merge requests list' - context 'when have subgroups', :nested_groups do + context 'when have subgroups' do let!(:group) { create(:group, :public) } let!(:subgroup) { create(:group, parent: group) } let!(:project) { create(:project, :public, :repository, creator: user, namespace: subgroup, only_allow_merge_if_pipeline_succeeds: false) } @@ -834,6 +834,31 @@ describe API::MergeRequests do end end + context 'head_pipeline' do + before do + merge_request.update(head_pipeline: create(:ci_pipeline)) + merge_request.project.project_feature.update(builds_access_level: 10) + end + + context 'when user can read the pipeline' do + it 'exposes pipeline information' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) + + expect(json_response).to include('head_pipeline') + end + end + + context 'when user can not read the pipeline' do + let(:guest) { create(:user) } + + it 'does not expose pipeline information' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", guest) + + expect(json_response).not_to include('head_pipeline') + end + end + end + it 'returns the commits behind the target branch when include_diverged_commits_count is present' do allow_any_instance_of(merge_request.class).to receive(:diverged_commits_count).and_return(1) @@ -1546,52 +1571,80 @@ describe API::MergeRequests do end end - describe "PUT /projects/:id/merge_requests/:merge_request_iid/merge_to_ref" do - let(:pipeline) { create(:ci_pipeline_without_jobs) } + describe "GET /projects/:id/merge_requests/:merge_request_iid/merge_ref", :clean_gitlab_redis_shared_state do + before do + merge_request.mark_as_unchecked! + end + + let(:merge_request_iid) { merge_request.iid } + let(:url) do - "/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge_to_ref" + "/projects/#{project.id}/merge_requests/#{merge_request_iid}/merge_ref" end it 'returns the generated ID from the merge service in case of success' do - put api(url, user), params: { merge_commit_message: 'Custom message' } - - commit = project.commit(json_response['commit_id']) + get api(url, user) expect(response).to have_gitlab_http_status(200) - expect(json_response['commit_id']).to be_present - expect(commit.message).to eq('Custom message') + expect(json_response['commit_id']).to eq(merge_request.merge_ref_head.sha) end - it "returns 400 if branch can't be merged" do - merge_request.update!(state: 'merged') + context 'when merge-ref is not synced with merge status' do + before do + merge_request.update!(merge_status: 'cannot_be_merged') + end + + it 'returns 200 if MR can be merged' do + get api(url, user) - put api(url, user) + expect(response).to have_gitlab_http_status(200) + expect(json_response['commit_id']).to eq(merge_request.merge_ref_head.sha) + end - expect(response).to have_gitlab_http_status(400) - expect(json_response['message']) - .to eq("Merge request is not mergeable to #{merge_request.merge_ref_path}") + it 'returns 400 if MR cannot be merged' do + expect_next_instance_of(MergeRequests::MergeToRefService) do |merge_request| + expect(merge_request).to receive(:execute) { { status: :failed } } + end + + get api(url, user) + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq('Merge request is not mergeable') + end end - it 'returns 403 if user has no permissions to merge to the ref' do - user2 = create(:user) - project.add_reporter(user2) + context 'when user has no access to the MR' do + let(:project) { create(:project, :private) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - put api(url, user2) + it 'returns 404' do + project.add_guest(user) - expect(response).to have_gitlab_http_status(403) - expect(json_response['message']).to eq('403 Forbidden') + get api(url, user) + + expect(response).to have_gitlab_http_status(404) + expect(json_response['message']).to eq('404 Not found') + end end - it 'returns 404 for an invalid merge request IID' do - put api("/projects/#{project.id}/merge_requests/12345/merge_to_ref", user) + context 'when invalid merge request IID' do + let(:merge_request_iid) { '12345' } - expect(response).to have_gitlab_http_status(404) + it 'returns 404' do + get api(url, user) + + expect(response).to have_gitlab_http_status(404) + end end - it "returns 404 if the merge request id is used instead of iid" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) + context 'when merge request ID is used instead IID' do + let(:merge_request_iid) { merge_request.id } - expect(response).to have_gitlab_http_status(404) + it 'returns 404' do + get api(url, user) + + expect(response).to have_gitlab_http_status(404) + end end end @@ -1980,6 +2033,9 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(202) expect(RebaseWorker.jobs.size).to eq(1) + + expect(merge_request.reload).to be_rebase_in_progress + expect(json_response['rebase_in_progress']).to be(true) end it 'returns 403 if the user cannot push to the branch' do @@ -1990,6 +2046,16 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(403) end + + it 'returns 409 if a rebase is already in progress' do + Sidekiq::Testing.fake! do + merge_request.rebase_async(user.id) + + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/rebase", user) + end + + expect(response).to have_gitlab_http_status(409) + end end describe 'Time tracking' do diff --git a/spec/requests/api/pages_domains_spec.rb b/spec/requests/api/pages_domains_spec.rb index 3eb68a6abb6..449032b95b7 100644 --- a/spec/requests/api/pages_domains_spec.rb +++ b/spec/requests/api/pages_domains_spec.rb @@ -359,6 +359,14 @@ describe API::PagesDomains do expect(pages_domain_secure.certificate).to eq(params_secure_nokey[:certificate]) end + it 'updates certificate source to user_provided if is changed' do + pages_domain.update!(certificate_source: 'gitlab_provided') + + expect do + put api(route_domain, user), params: params_secure + end.to change { pages_domain.reload.certificate_source }.from('gitlab_provided').to('user_provided') + end + it 'fails to update pages domain adding certificate without key' do put api(route_domain, user), params: params_secure_nokey diff --git a/spec/requests/api/project_clusters_spec.rb b/spec/requests/api/project_clusters_spec.rb index fc0381159dd..a7b919de2ef 100644 --- a/spec/requests/api/project_clusters_spec.rb +++ b/spec/requests/api/project_clusters_spec.rb @@ -6,11 +6,12 @@ describe API::ProjectClusters do include KubernetesHelpers let(:current_user) { create(:user) } - let(:non_member) { create(:user) } - let(:project) { create(:project, :repository) } + let(:developer_user) { create(:user) } + let(:project) { create(:project) } before do project.add_maintainer(current_user) + project.add_developer(developer_user) end describe 'GET /projects/:id/clusters' do @@ -22,10 +23,10 @@ describe API::ProjectClusters do end context 'non-authorized user' do - it 'responds with 404' do - get api("/projects/#{project.id}/clusters", non_member) + it 'responds with 403' do + get api("/projects/#{project.id}/clusters", developer_user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end end @@ -67,10 +68,10 @@ describe API::ProjectClusters do end context 'non-authorized user' do - it 'responds with 404' do - get api("/projects/#{project.id}/clusters/#{cluster_id}", non_member) + it 'responds with 403' do + get api("/projects/#{project.id}/clusters/#{cluster_id}", developer_user) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end end @@ -147,31 +148,7 @@ describe API::ProjectClusters do end end - shared_context 'kubernetes calls stubbed' do - before do - stub_kubeclient_discover(api_url) - stub_kubeclient_get_namespace(api_url, namespace: namespace) - stub_kubeclient_get_service_account(api_url, "#{namespace}-service-account", namespace: namespace) - stub_kubeclient_put_service_account(api_url, "#{namespace}-service-account", namespace: namespace) - - stub_kubeclient_get_secret( - api_url, - { - metadata_name: "#{namespace}-token", - token: Base64.encode64('sample-token'), - namespace: namespace - } - ) - - stub_kubeclient_put_secret(api_url, "#{namespace}-token", namespace: namespace) - stub_kubeclient_get_role_binding(api_url, "gitlab-#{namespace}", namespace: namespace) - stub_kubeclient_put_role_binding(api_url, "gitlab-#{namespace}", namespace: namespace) - end - end - describe 'POST /projects/:id/clusters/user' do - include_context 'kubernetes calls stubbed' - let(:api_url) { 'https://kubernetes.example.com' } let(:namespace) { project.path } let(:authorization_type) { 'rbac' } @@ -195,10 +172,10 @@ describe API::ProjectClusters do end context 'non-authorized user' do - it 'responds with 404' do - post api("/projects/#{project.id}/clusters/user", non_member), params: cluster_params + it 'responds with 403' do + post api("/projects/#{project.id}/clusters/user", developer_user), params: cluster_params - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end end @@ -280,19 +257,27 @@ describe API::ProjectClusters do post api("/projects/#{project.id}/clusters/user", current_user), params: cluster_params end + it 'responds with 400' do + expect(response).to have_gitlab_http_status(400) + + expect(json_response['message']['base'].first).to eq('Instance does not support multiple Kubernetes clusters') + end + end + + context 'non-authorized user' do + before do + post api("/projects/#{project.id}/clusters/user", developer_user), params: cluster_params + end + it 'responds with 403' do expect(response).to have_gitlab_http_status(403) - end - it 'returns an appropriate message' do - expect(json_response['message']).to include('Instance does not support multiple Kubernetes clusters') + expect(json_response['message']).to eq('403 Forbidden') end end end describe 'PUT /projects/:id/clusters/:cluster_id' do - include_context 'kubernetes calls stubbed' - let(:api_url) { 'https://kubernetes.example.com' } let(:namespace) { 'new-namespace' } let(:platform_kubernetes_attributes) { { namespace: namespace } } @@ -316,10 +301,10 @@ describe API::ProjectClusters do end context 'non-authorized user' do - it 'responds with 404' do - put api("/projects/#{project.id}/clusters/#{cluster.id}", non_member), params: update_params + it 'responds with 403' do + put api("/projects/#{project.id}/clusters/#{cluster.id}", developer_user), params: update_params - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end end @@ -351,7 +336,6 @@ describe API::ProjectClusters do it 'does not update cluster attributes' do expect(cluster.domain).not_to eq('new_domain.com') expect(cluster.platform_kubernetes.namespace).not_to eq('invalid_namespace') - expect(cluster.kubernetes_namespace_for(project)).not_to eq('invalid_namespace') end it 'returns validation errors' do @@ -442,10 +426,10 @@ describe API::ProjectClusters do end context 'non-authorized user' do - it 'responds with 404' do - delete api("/projects/#{project.id}/clusters/#{cluster.id}", non_member), params: cluster_params + it 'responds with 403' do + delete api("/projects/#{project.id}/clusters/#{cluster.id}", developer_user), params: cluster_params - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end end diff --git a/spec/requests/api/container_registry_spec.rb b/spec/requests/api/project_container_repositories_spec.rb index ea035a8be4a..f1dc4e6f0b2 100644 --- a/spec/requests/api/container_registry_spec.rb +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' -describe API::ContainerRegistry do +describe API::ProjectContainerRepositories do + include ExclusiveLeaseHelpers + set(:project) { create(:project, :private) } set(:maintainer) { create(:user) } set(:developer) { create(:user) } @@ -10,6 +12,16 @@ describe API::ContainerRegistry do let(:root_repository) { create(:container_repository, :root, project: project) } let(:test_repository) { create(:container_repository, project: project) } + let(:users) do + { + anonymous: nil, + developer: developer, + guest: guest, + maintainer: maintainer, + reporter: reporter + } + end + let(:api_user) { maintainer } before do @@ -25,57 +37,24 @@ describe API::ContainerRegistry do test_repository end - shared_examples 'being disallowed' do |param| - context "for #{param}" do - let(:api_user) { public_send(param) } - - it 'returns access denied' do - subject - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context "for anonymous" do - let(:api_user) { nil } - - it 'returns not found' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - describe 'GET /projects/:id/registry/repositories' do - subject { get api("/projects/#{project.id}/registry/repositories", api_user) } + let(:url) { "/projects/#{project.id}/registry/repositories" } - it_behaves_like 'being disallowed', :guest + subject { get api(url, api_user) } - context 'for reporter' do - let(:api_user) { reporter } + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found - it 'returns a list of repositories' do - subject - - expect(json_response.length).to eq(2) - expect(json_response.map { |repository| repository['id'] }).to contain_exactly( - root_repository.id, test_repository.id) - end - - it 'returns a matching schema' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('registry/repositories') - end + it_behaves_like 'returns repositories for allowed users', :reporter, 'project' do + let(:object) { project } end end describe 'DELETE /projects/:id/registry/repositories/:repository_id' do subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}", api_user) } - it_behaves_like 'being disallowed', :developer + it_behaves_like 'rejected container repository access', :developer, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found context 'for maintainer' do let(:api_user) { maintainer } @@ -94,7 +73,8 @@ describe API::ContainerRegistry do describe 'GET /projects/:id/registry/repositories/:repository_id/tags' do subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user) } - it_behaves_like 'being disallowed', :guest + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found context 'for reporter' do let(:api_user) { reporter } @@ -122,10 +102,13 @@ describe API::ContainerRegistry do describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags' do subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user), params: params } - it_behaves_like 'being disallowed', :developer do + context 'disallowed' do let(:params) do { name_regex: 'v10.*' } end + + it_behaves_like 'rejected container repository access', :developer, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found end context 'for maintainer' do @@ -155,7 +138,10 @@ describe API::ContainerRegistry do older_than: '1 day' } end + let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" } + it 'schedules cleanup of tags repository' do + stub_exclusive_lease(lease_key, timeout: 1.hour) expect(CleanupContainerRepositoryWorker).to receive(:perform_async) .with(maintainer.id, root_repository.id, worker_params) @@ -163,6 +149,22 @@ describe API::ContainerRegistry do expect(response).to have_gitlab_http_status(:accepted) end + + context 'called multiple times in one hour' do + it 'returns 400 with an error message' do + stub_exclusive_lease_taken(lease_key, timeout: 1.hour) + subject + + expect(response).to have_gitlab_http_status(400) + expect(response.body).to include('This request has already been made.') + end + + it 'executes service only for the first time' do + expect(CleanupContainerRepositoryWorker).to receive(:perform_async).once + + 2.times { subject } + end + end end end end @@ -170,7 +172,8 @@ describe API::ContainerRegistry do describe 'GET /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) } - it_behaves_like 'being disallowed', :guest + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found context 'for reporter' do let(:api_user) { reporter } @@ -201,10 +204,11 @@ describe API::ContainerRegistry do describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) } - it_behaves_like 'being disallowed', :developer + it_behaves_like 'rejected container repository access', :reporter, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found - context 'for maintainer' do - let(:api_user) { maintainer } + context 'for developer' do + let(:api_user) { developer } before do stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 5f7d2fa6d9c..1d7ca85cdd2 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -46,8 +46,6 @@ shared_examples 'languages and percentages JSON response' do end describe API::Projects do - include ExternalAuthorizationServiceHelpers - let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } @@ -840,6 +838,28 @@ describe API::Projects do end end + describe 'GET /users/:user_id/starred_projects/' do + before do + user3.update(starred_projects: [project, project2, project3]) + end + + it 'returns error when user not found' do + get api('/users/9999/starred_projects/') + + expect(response).to have_gitlab_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'returns projects filtered by user' do + get api("/users/#{user3.id}/starred_projects/", user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |project| project['id'] }).to contain_exactly(project.id, project2.id, project3.id) + end + end + describe 'POST /projects/user/:id' do it 'creates new project without path but with name and return 201' do expect { post api("/projects/user/#{user.id}", admin), params: { name: 'Foo Project' } }.to change { Project.count }.by(1) @@ -1102,6 +1122,12 @@ describe API::Projects do expect(json_response['wiki_enabled']).to be_present expect(json_response['jobs_enabled']).to be_present expect(json_response['snippets_enabled']).to be_present + expect(json_response['snippets_access_level']).to be_present + expect(json_response['repository_access_level']).to be_present + expect(json_response['issues_access_level']).to be_present + expect(json_response['merge_requests_access_level']).to be_present + expect(json_response['wiki_access_level']).to be_present + expect(json_response['builds_access_level']).to be_present expect(json_response['resolve_outdated_diff_discussions']).to eq(project.resolve_outdated_diff_discussions) expect(json_response['container_registry_enabled']).to be_present expect(json_response['created_at']).to be_present @@ -1353,7 +1379,7 @@ describe API::Projects do end end - context 'nested group project', :nested_groups do + context 'nested group project' do let(:group) { create(:group) } let(:nested_group) { create(:group, parent: group) } let(:project2) { create(:project, group: nested_group) } @@ -1419,39 +1445,6 @@ describe API::Projects do end end end - - context 'with external authorization' do - let(:project) do - create(:project, - namespace: user.namespace, - external_authorization_classification_label: 'the-label') - end - - context 'when the user has access to the project' do - before do - external_service_allow_access(user, project) - end - - it 'includes the label in the response' do - get api("/projects/#{project.id}", user) - - expect(response).to have_gitlab_http_status(200) - expect(json_response['external_authorization_classification_label']).to eq('the-label') - end - end - - context 'when the external service denies access' do - before do - external_service_deny_access(user, project) - end - - it 'returns a 404' do - get api("/projects/#{project.id}", user) - - expect(response).to have_gitlab_http_status(404) - end - end - end end describe 'GET /projects/:id/users' do @@ -1913,6 +1906,34 @@ describe API::Projects do end end + it 'updates builds_access_level' do + project_param = { builds_access_level: 'private' } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(200) + + expect(json_response['builds_access_level']).to eq('private') + end + + it 'updates build_git_strategy' do + project_param = { build_git_strategy: 'clone' } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(200) + + expect(json_response['build_git_strategy']).to eq('clone') + end + + it 'rejects to update build_git_strategy when build_git_strategy is invalid' do + project_param = { build_git_strategy: 'invalid' } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(400) + end + it 'updates merge_method' do project_param = { merge_method: 'ff' } @@ -1946,6 +1967,26 @@ describe API::Projects do '-/system/project/avatar/'\ "#{project3.id}/banana_sample.gif") end + + it 'updates auto_devops_deploy_strategy' do + project_param = { auto_devops_deploy_strategy: 'timed_incremental' } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(200) + + expect(json_response['auto_devops_deploy_strategy']).to eq('timed_incremental') + end + + it 'updates auto_devops_enabled' do + project_param = { auto_devops_enabled: false } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(200) + + expect(json_response['auto_devops_enabled']).to eq(false) + end end context 'when authenticated as project maintainer' do @@ -2007,20 +2048,6 @@ describe API::Projects do expect(response).to have_gitlab_http_status(403) end end - - context 'when updating external classification' do - before do - enable_external_authorization_service_check - end - - it 'updates the classification label' do - put(api("/projects/#{project.id}", user), params: { external_authorization_classification_label: 'new label' }) - - expect(response).to have_gitlab_http_status(200) - - expect(project.reload.external_authorization_classification_label).to eq('new label') - end - end end describe 'POST /projects/:id/archive' do @@ -2143,6 +2170,85 @@ describe API::Projects do end end + describe 'GET /projects/:id/starrers' do + shared_examples_for 'project starrers response' do + it 'returns an array of starrers' do + get api("/projects/#{public_project.id}/starrers", current_user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response[0]['starred_since']).to be_present + expect(json_response[0]['user']).to be_present + end + + it 'returns the proper security headers' do + get api('/projects/1/starrers', current_user) + + expect(response).to include_security_headers + end + end + + let(:public_project) { create(:project, :public) } + let(:private_user) { create(:user, private_profile: true) } + + before do + user.update(starred_projects: [public_project]) + private_user.update(starred_projects: [public_project]) + end + + it 'returns not_found(404) for not existing project' do + get api("/projects/9999999999/starrers", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'public project without user' do + it_behaves_like 'project starrers response' do + let(:current_user) { nil } + end + + it 'returns only starrers with a public profile' do + get api("/projects/#{public_project.id}/starrers", nil) + + user_ids = json_response.map { |s| s['user']['id'] } + expect(user_ids).to include(user.id) + expect(user_ids).not_to include(private_user.id) + end + end + + context 'public project with user with private profile' do + it_behaves_like 'project starrers response' do + let(:current_user) { private_user } + end + + it 'returns current user with a private profile' do + get api("/projects/#{public_project.id}/starrers", private_user) + + user_ids = json_response.map { |s| s['user']['id'] } + expect(user_ids).to include(user.id, private_user.id) + end + end + + context 'private project' do + context 'with unauthorized user' do + it 'returns not_found for existing but unauthorized project' do + get api("/projects/#{project3.id}/starrers", user3) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'without user' do + it 'returns not_found for existing but unauthorized project' do + get api("/projects/#{project3.id}/starrers", nil) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + describe 'GET /projects/:id/languages' do context 'with an authorized user' do it_behaves_like 'languages and percentages JSON response' do @@ -2428,7 +2534,7 @@ describe API::Projects do let(:housekeeping) { Projects::HousekeepingService.new(project) } before do - allow(Projects::HousekeepingService).to receive(:new).with(project).and_return(housekeeping) + allow(Projects::HousekeepingService).to receive(:new).with(project, :gc).and_return(housekeeping) end context 'when authenticated as owner' do diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 8603fa2a73d..206e898381d 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -24,7 +24,7 @@ describe API::Releases do project: project, tag: 'v0.1', author: maintainer, - created_at: 2.days.ago) + released_at: 2.days.ago) end let!(:release_2) do @@ -32,7 +32,7 @@ describe API::Releases do project: project, tag: 'v0.2', author: maintainer, - created_at: 1.day.ago) + released_at: 1.day.ago) end it 'returns 200 HTTP status' do @@ -41,7 +41,7 @@ describe API::Releases do expect(response).to have_gitlab_http_status(:ok) end - it 'returns releases ordered by created_at' do + it 'returns releases ordered by released_at' do get api("/projects/#{project.id}/releases", maintainer) expect(json_response.count).to eq(2) @@ -56,6 +56,26 @@ describe API::Releases do end end + it 'returns an upcoming_release status for a future release' do + tomorrow = Time.now.utc + 1.day + create(:release, project: project, tag: 'v0.1', author: maintainer, released_at: tomorrow) + + get api("/projects/#{project.id}/releases", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.first['upcoming_release']).to eq(true) + end + + it 'returns an upcoming_release status for a past release' do + yesterday = Time.now.utc - 1.day + create(:release, project: project, tag: 'v0.1', author: maintainer, released_at: yesterday) + + get api("/projects/#{project.id}/releases", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.first['upcoming_release']).to eq(false) + end + context 'when tag does not exist in git repository' do let!(:release) { create(:release, project: project, tag: 'v1.1.5') } @@ -316,6 +336,51 @@ describe API::Releases do expect(project.releases.last.description).to eq('Super nice release') end + it 'sets the released_at to the current time if the released_at parameter is not provided' do + now = Time.zone.parse('2015-08-25 06:00:00Z') + Timecop.freeze(now) do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq(now) + end + end + + it 'sets the released_at to the value in the parameters if specified' do + params = { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release', + released_at: '2019-03-20T10:00:00Z' + } + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2019-03-20T10:00:00Z') + end + + it 'assumes the utc timezone for released_at if the timezone is not provided' do + params = { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release', + released_at: '2019-03-25 10:00:00' + } + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2019-03-25T10:00:00Z') + end + + it 'allows specifying a released_at with a local time zone' do + params = { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release', + released_at: '2019-03-25T10:00:00+09:00' + } + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2019-03-25T01:00:00Z') + end + context 'when description is empty' do let(:params) do { @@ -540,6 +605,7 @@ describe API::Releases do project: project, tag: 'v0.1', name: 'New release', + released_at: '2018-03-01T22:00:00Z', description: 'Super nice release') end @@ -560,6 +626,7 @@ describe API::Releases do expect(project.releases.last.tag).to eq('v0.1') expect(project.releases.last.name).to eq('New release') + expect(project.releases.last.released_at).to eq('2018-03-01T22:00:00Z') end it 'matches response schema' do @@ -568,6 +635,14 @@ describe API::Releases do expect(response).to match_response_schema('public_api/v4/release') end + it 'updates released_at' do + params = { released_at: '2015-10-10T05:00:00Z' } + + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2015-10-10T05:00:00Z') + end + context 'when user tries to update sha' do let(:params) { { sha: 'xxx' } } diff --git a/spec/requests/api/resource_label_events_spec.rb b/spec/requests/api/resource_label_events_spec.rb index 37b46eaeb86..25bea627b0c 100644 --- a/spec/requests/api/resource_label_events_spec.rb +++ b/spec/requests/api/resource_label_events_spec.rb @@ -4,55 +4,12 @@ require 'spec_helper' describe API::ResourceLabelEvents do set(:user) { create(:user) } - set(:project) { create(:project, :public, :repository, namespace: user.namespace) } - set(:private_user) { create(:user) } + set(:project) { create(:project, :public, namespace: user.namespace) } before do project.add_developer(user) end - shared_examples 'resource_label_events API' do |parent_type, eventable_type, id_name| - describe "GET /#{parent_type}/:id/#{eventable_type}/:noteable_id/resource_label_events" do - it "returns an array of resource label events" do - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", user) - - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.first['id']).to eq(event.id) - end - - it "returns a 404 error when eventable id not found" do - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/12345/resource_label_events", user) - - expect(response).to have_gitlab_http_status(404) - end - - it "returns 404 when not authorized" do - parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) - - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", private_user) - - expect(response).to have_gitlab_http_status(404) - end - end - - describe "GET /#{parent_type}/:id/#{eventable_type}/:noteable_id/resource_label_events/:event_id" do - it "returns a resource label event by id" do - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/#{event.id}", user) - - expect(response).to have_gitlab_http_status(200) - expect(json_response['id']).to eq(event.id) - end - - it "returns a 404 error if resource label event not found" do - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - end - end - context 'when eventable is an Issue' do let(:issue) { create(:issue, project: project, author: user) } diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 5548e3fd01a..f5ce3a3570e 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -584,6 +584,34 @@ describe API::Runners do end end + context 'when valid order_by is provided' do + context 'when sort order is not specified' do + it 'return jobs in descending order' do + get api("/runners/#{project_runner.id}/jobs?order_by=id", admin) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(2) + expect(json_response.first).to include('id' => job_5.id) + end + end + + context 'when sort order is specified as asc' do + it 'return jobs sorted in ascending order' do + get api("/runners/#{project_runner.id}/jobs?order_by=id&sort=asc", admin) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(2) + expect(json_response.first).to include('id' => job_4.id) + end + end + end + context 'when invalid status is provided' do it 'return 400' do get api("/runners/#{project_runner.id}/jobs?status=non-existing", admin) @@ -591,6 +619,22 @@ describe API::Runners do expect(response).to have_gitlab_http_status(400) end end + + context 'when invalid order_by is provided' do + it 'return 400' do + get api("/runners/#{project_runner.id}/jobs?order_by=non-existing", admin) + + expect(response).to have_gitlab_http_status(400) + end + end + + context 'when invalid sort is provided' do + it 'return 400' do + get api("/runners/#{project_runner.id}/jobs?sort=non-existing", admin) + + expect(response).to have_gitlab_http_status(400) + end + end end context "when runner doesn't exist" do diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 3e0b478abb3..8abdcaa2e0e 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -89,7 +89,7 @@ describe API::Search do it 'returns empty array' do get api('/search', user), params: { scope: 'milestones', search: 'awesome' } - milestones = JSON.parse(response.body) + milestones = json_response expect(milestones).to be_empty end @@ -356,7 +356,7 @@ describe API::Search do it 'returns empty array' do get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } - milestones = JSON.parse(response.body) + milestones = json_response expect(milestones).to be_empty end diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index e260aa21e25..76a70ab6e9e 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -19,13 +19,22 @@ describe API::Services do expect(response).to have_gitlab_http_status(200) current_service = project.services.first - event = current_service.event_names.empty? ? "foo" : current_service.event_names.first - state = current_service[event] || false + events = current_service.event_names.empty? ? ["foo"].freeze : current_service.event_names + query_strings = [] + events.each do |event| + query_strings << "#{event}=#{!current_service[event]}" + end + query_strings = query_strings.join('&') - put api("/projects/#{project.id}/services/#{dashed_service}?#{event}=#{!state}", user), params: service_attrs + put api("/projects/#{project.id}/services/#{dashed_service}?#{query_strings}", user), params: service_attrs expect(response).to have_gitlab_http_status(200) - expect(project.services.first[event]).not_to eq(state) unless event == "foo" + events.each do |event| + next if event == "foo" + + expect(project.services.first[event]).not_to eq(current_service[event]), + "expected #{!current_service[event]} for event #{event} for service #{current_service.title}, got #{current_service[event]}" + end end it "returns if required fields missing" do @@ -70,9 +79,7 @@ describe API::Services do include_context service # inject some properties into the service - before do - initialize_service(service) - end + let!(:initialized_service) { initialize_service(service) } it 'returns authentication error when unauthenticated' do get api("/projects/#{project.id}/services/#{dashed_service}") @@ -93,6 +100,15 @@ describe API::Services do expect(json_response['properties'].keys).to match_array(service_instance.api_field_names) end + it "returns empty hash if properties are empty" do + # deprecated services are not valid for update + initialized_service.update_attribute(:properties, {}) + get api("/projects/#{project.id}/services/#{dashed_service}", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['properties'].keys).to be_empty + end + it "returns error when authenticated but not a project owner" do project.add_developer(user2) get api("/projects/#{project.id}/services/#{dashed_service}", user2) diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 8a60980fe80..590107d5161 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -25,6 +25,9 @@ describe API::Settings, 'Settings' do expect(json_response['ed25519_key_restriction']).to eq(0) expect(json_response['performance_bar_allowed_group_id']).to be_nil expect(json_response['instance_statistics_visibility_private']).to be(false) + expect(json_response['allow_local_requests_from_hooks_and_services']).to be(false) + expect(json_response['allow_local_requests_from_web_hooks_and_services']).to be(false) + expect(json_response['allow_local_requests_from_system_hooks']).to be(true) expect(json_response).not_to have_key('performance_bar_allowed_group_path') expect(json_response).not_to have_key('performance_bar_enabled') end @@ -67,7 +70,9 @@ describe API::Settings, 'Settings' do instance_statistics_visibility_private: true, diff_max_patch_bytes: 150_000, default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE, - local_markdown_version: 3 + local_markdown_version: 3, + allow_local_requests_from_web_hooks_and_services: true, + allow_local_requests_from_system_hooks: false } expect(response).to have_gitlab_http_status(200) @@ -95,6 +100,8 @@ describe API::Settings, 'Settings' do expect(json_response['diff_max_patch_bytes']).to eq(150_000) expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) expect(json_response['local_markdown_version']).to eq(3) + expect(json_response['allow_local_requests_from_web_hooks_and_services']).to eq(true) + expect(json_response['allow_local_requests_from_system_hooks']).to eq(false) end end @@ -117,6 +124,14 @@ describe API::Settings, 'Settings' do expect(json_response['performance_bar_allowed_group_id']).to be_nil end + it 'supports legacy allow_local_requests_from_hooks_and_services' do + put api("/application/settings", admin), + params: { allow_local_requests_from_hooks_and_services: true } + + expect(response).to have_gitlab_http_status(200) + expect(json_response['allow_local_requests_from_hooks_and_services']).to eq(true) + end + context 'external policy classification settings' do let(:settings) do { @@ -129,6 +144,7 @@ describe API::Settings, 'Settings' do external_auth_client_key_pass: "5iveL!fe" } end + let(:attribute_names) { settings.keys.map(&:to_s) } it 'includes the attributes in the API' do @@ -150,6 +166,56 @@ describe API::Settings, 'Settings' do end end + context "snowplow tracking settings" do + let(:settings) do + { + snowplow_collector_hostname: "snowplow.example.com", + snowplow_cookie_domain: ".example.com", + snowplow_enabled: true, + snowplow_site_id: "site_id" + } + end + + let(:attribute_names) { settings.keys.map(&:to_s) } + + it "includes the attributes in the API" do + get api("/application/settings", admin) + + expect(response).to have_gitlab_http_status(200) + attribute_names.each do |attribute| + expect(json_response.keys).to include(attribute) + end + end + + it "allows updating the settings" do + put api("/application/settings", admin), params: settings + + expect(response).to have_gitlab_http_status(200) + settings.each do |attribute, value| + expect(ApplicationSetting.current.public_send(attribute)).to eq(value) + end + end + + context "missing snowplow_collector_hostname value when snowplow_enabled is true" do + it "returns a blank parameter error message" do + put api("/application/settings", admin), params: { snowplow_enabled: true } + + expect(response).to have_gitlab_http_status(400) + expect(json_response["error"]).to eq("snowplow_collector_hostname is missing") + end + + it "handles validation errors" do + put api("/application/settings", admin), params: settings.merge({ + snowplow_collector_hostname: nil + }) + + expect(response).to have_gitlab_http_status(400) + message = json_response["message"] + expect(message["snowplow_collector_hostname"]).to include("can't be blank") + end + end + end + context "missing plantuml_url value when plantuml_enabled is true" do it "returns a blank parameter error message" do put api("/application/settings", admin), params: { plantuml_enabled: true } diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index d898319e709..c4f4a2cb889 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -10,7 +10,7 @@ describe API::Tags do let(:current_user) { nil } before do - project.add_maintainer(user) + project.add_developer(user) end describe 'GET /projects/:id/repository/tags' do diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index f0f01e97f1d..8ea3d16a41f 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -270,34 +270,6 @@ describe API::Triggers do end end - describe 'POST /projects/:id/triggers/:trigger_id/take_ownership' do - context 'authenticated user with valid permissions' do - it 'updates owner' do - post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user) - - expect(response).to have_gitlab_http_status(200) - expect(json_response).to include('owner') - expect(trigger.reload.owner).to eq(user) - end - end - - context 'authenticated user with invalid permissions' do - it 'does not update owner' do - post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user2) - - expect(response).to have_gitlab_http_status(403) - end - end - - context 'unauthenticated user' do - it 'does not update owner' do - post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership") - - expect(response).to have_gitlab_http_status(401) - end - end - end - describe 'DELETE /projects/:id/triggers/:trigger_id' do context 'authenticated user with valid permissions' do it 'deletes trigger' do diff --git a/spec/requests/api/user_counts_spec.rb b/spec/requests/api/user_counts_spec.rb new file mode 100644 index 00000000000..c833bd047e2 --- /dev/null +++ b/spec/requests/api/user_counts_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::UserCounts do + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + + let!(:merge_request) { create(:merge_request, :simple, author: user, assignees: [user], source_project: project, title: "Test") } + + describe 'GET /user_counts' do + context 'when unauthenticated' do + it 'returns authentication error' do + get api('/user_counts') + + expect(response.status).to eq(401) + end + end + + context 'when authenticated' do + it 'returns open counts for current user' do + get api('/user_counts', user) + + expect(response.status).to eq(200) + expect(json_response).to be_a Hash + expect(json_response['merge_requests']).to eq(1) + end + + it 'updates the mr count when a new mr is assigned' do + create(:merge_request, source_project: project, author: user, assignees: [user]) + + get api('/user_counts', user) + + expect(response.status).to eq(200) + expect(json_response).to be_a Hash + expect(json_response['merge_requests']).to eq(2) + end + end + end +end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index bab1520b960..af2bee4563a 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -416,7 +416,6 @@ describe API::Users do expect(response).to have_gitlab_http_status(201) user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq(nil) expect(new_user.admin).to eq(true) expect(new_user.can_create_group).to eq(true) end @@ -435,7 +434,6 @@ describe API::Users do expect(response).to have_gitlab_http_status(201) user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq(nil) expect(new_user.admin).to eq(false) expect(new_user.can_create_group).to eq(false) end @@ -445,12 +443,12 @@ describe API::Users do expect(response).to have_gitlab_http_status(201) user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq(nil) expect(new_user.admin).to eq(false) end it "returns 201 Created on success" do post api("/users", admin), params: attributes_for(:user, projects_limit: 3) + expect(response).to match_response_schema('public_api/v4/user/admin') expect(response).to have_gitlab_http_status(201) end @@ -460,7 +458,6 @@ describe API::Users do user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq nil expect(new_user.external).to be_falsy end @@ -470,7 +467,6 @@ describe API::Users do user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq nil expect(new_user.external).to be_truthy end @@ -482,7 +478,19 @@ describe API::Users do user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq(nil) + expect(new_user.recently_sent_password_reset?).to eq(true) + end + + it "creates user with random password" do + params = attributes_for(:user, force_random_password: true, reset_password: true) + post api('/users', admin), params: params + + expect(response).to have_gitlab_http_status(201) + + user_id = json_response['id'] + new_user = User.find(user_id) + + expect(new_user.valid_password?(params[:password])).to eq(false) expect(new_user.recently_sent_password_reset?).to eq(true) end @@ -636,6 +644,13 @@ describe API::Users do describe "PUT /users/:id" do let!(:admin_user) { create(:admin) } + it "returns 200 OK on success" do + put api("/users/#{user.id}", admin), params: { bio: 'new test bio' } + + expect(response).to match_response_schema('public_api/v4/user/admin') + expect(response).to have_gitlab_http_status(200) + end + it "updates user with new bio" do put api("/users/#{user.id}", admin), params: { bio: 'new test bio' } @@ -738,6 +753,14 @@ describe API::Users do expect(user.reload.private_profile).to eq(true) end + it "updates private profile when nil is given to false" do + admin.update(private_profile: true) + + put api("/users/#{user.id}", admin), params: { private_profile: nil } + + expect(user.reload.private_profile).to eq(false) + end + it "does not update admin status" do put api("/users/#{admin_user.id}", admin), params: { can_create_group: false } diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index 55b1419a004..69f105b71a8 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -106,6 +106,30 @@ describe API::Variables do expect(response).to have_gitlab_http_status(400) end + + it 'creates variable with a specific environment scope' do + expect do + post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'VALUE_2', environment_scope: 'review/*' } + end.to change { project.variables.reload.count }.by(1) + + expect(response).to have_gitlab_http_status(201) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['environment_scope']).to eq('review/*') + end + + it 'allows duplicated variable key given different environment scopes' do + variable = create(:ci_variable, project: project) + + expect do + post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2', environment_scope: 'review/*' } + end.to change { project.variables.reload.count }.by(1) + + expect(response).to have_gitlab_http_status(201) + expect(json_response['key']).to eq(variable.key) + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['environment_scope']).to eq('review/*') + end end context 'authorized user with invalid permissions' do diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 1781759c54b..dc25e4d808e 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -1439,8 +1439,4 @@ describe 'Git LFS API and storage' do post(url, params: params, headers: headers) end - - def json_response - @json_response ||= JSON.parse(response.body) - end end diff --git a/spec/requests/lfs_locks_api_spec.rb b/spec/requests/lfs_locks_api_spec.rb index 5b7b3d2fdd6..11436e5cd0c 100644 --- a/spec/requests/lfs_locks_api_spec.rb +++ b/spec/requests/lfs_locks_api_spec.rb @@ -163,8 +163,4 @@ describe 'Git LFS File Locking API' do def do_get(url, params = nil, headers = nil) get(url, params: (params || {}), headers: (headers || {}).merge('Content-Type' => LfsRequest::CONTENT_TYPE)) end - - def json_response - @json_response ||= JSON.parse(response.body) - end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 86e41cbdf00..025568d8bea 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -104,7 +104,7 @@ describe 'OpenID Connect requests' do expect(json_response).to match(id_token_claims.merge(user_info_claims)) expected_groups = [group1.full_path, group3.full_path] - expected_groups << group4.full_path if Group.supports_nested_objects? + expected_groups << group4.full_path expect(json_response['groups']).to match_array(expected_groups) end diff --git a/spec/requests/projects/cycle_analytics_events_spec.rb b/spec/requests/projects/cycle_analytics_events_spec.rb index 49412b628b3..25390f8a23e 100644 --- a/spec/requests/projects/cycle_analytics_events_spec.rb +++ b/spec/requests/projects/cycle_analytics_events_spec.rb @@ -32,10 +32,10 @@ describe 'cycle analytics events' do it 'lists the plan events' do get project_cycle_analytics_plan_path(project, format: :json) - first_mr_short_sha = project.merge_requests.sort_by_attribute(:created_asc).first.commits.first.short_id + first_issue_iid = project.issues.sort_by_attribute(:created_desc).pluck(:iid).first.to_s expect(json_response['events']).not_to be_empty - expect(json_response['events'].first['short_sha']).to eq(first_mr_short_sha) + expect(json_response['events'].first['iid']).to eq(first_issue_iid) end it 'lists the code events' do diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 89adbc77a7f..478f09a7881 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -102,6 +102,27 @@ describe 'Rack Attack global throttles' do expect_rejection { get(*get_args) } end + + it 'logs RackAttack info into structured logs' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + arguments = { + message: 'Rack_Attack', + env: :throttle, + remote_ip: '127.0.0.1', + request_method: 'GET', + path: get_args.first, + user_id: user.id, + username: user.username + } + + expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once + + expect_rejection { get(*get_args) } + end end context 'when the throttle is disabled' do @@ -189,7 +210,15 @@ describe 'Rack Attack global throttles' do expect(response).to have_http_status 200 end - expect(Gitlab::AuthLogger).to receive(:error).once + arguments = { + message: 'Rack_Attack', + env: :throttle, + remote_ip: '127.0.0.1', + request_method: 'GET', + path: '/users/sign_in' + } + + expect(Gitlab::AuthLogger).to receive(:error).with(arguments) get url_that_does_not_require_authentication end @@ -345,7 +374,17 @@ describe 'Rack Attack global throttles' do expect(response).to have_http_status 200 end - expect(Gitlab::AuthLogger).to receive(:error).once + arguments = { + message: 'Rack_Attack', + env: :throttle, + remote_ip: '127.0.0.1', + request_method: 'GET', + path: '/dashboard/snippets', + user_id: user.id, + username: user.username + } + + expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once get url_that_requires_authentication end diff --git a/spec/requests/request_profiler_spec.rb b/spec/requests/request_profiler_spec.rb index 75b22b1879b..851affbcf88 100644 --- a/spec/requests/request_profiler_spec.rb +++ b/spec/requests/request_profiler_spec.rb @@ -3,13 +3,18 @@ require 'spec_helper' describe 'Request Profiler' do let(:user) { create(:user) } - shared_examples 'profiling a request' do + shared_examples 'profiling a request' do |profile_type, extension| before do allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) allow(RubyProf::Profile).to receive(:profile) do |&blk| blk.call RubyProf::Profile.new end + allow(MemoryProfiler).to receive(:report) do |&blk| + blk.call + MemoryProfiler.start + MemoryProfiler.stop + end end it 'creates a profile of the request' do @@ -18,10 +23,11 @@ describe 'Request Profiler' do path = "/#{project.full_path}" Timecop.freeze(time) do - get path, params: {}, headers: { 'X-Profile-Token' => Gitlab::RequestProfiler.profile_token } + get path, params: {}, headers: { 'X-Profile-Token' => Gitlab::RequestProfiler.profile_token, 'X-Profile-Mode' => profile_type } end - profile_path = "#{Gitlab.config.shared.path}/tmp/requests_profiles/#{path.tr('/', '|')}_#{time.to_i}.html" + profile_type = 'execution' if profile_type.nil? + profile_path = "#{Gitlab.config.shared.path}/tmp/requests_profiles/#{path.tr('/', '|')}_#{time.to_i}_#{profile_type}.#{extension}" expect(File.exist?(profile_path)).to be true end @@ -35,10 +41,14 @@ describe 'Request Profiler' do login_as(user) end - include_examples 'profiling a request' + include_examples 'profiling a request', 'execution', 'html' + include_examples 'profiling a request', nil, 'html' + include_examples 'profiling a request', 'memory', 'txt' end context "when user is not logged-in" do - include_examples 'profiling a request' + include_examples 'profiling a request', 'execution', 'html' + include_examples 'profiling a request', nil, 'html' + include_examples 'profiling a request', 'memory', 'txt' end end |