2012-11-24 23:04:13 +00:00
|
|
|
require 'spec_helper'
|
|
|
|
|
2013-06-23 17:25:06 +00:00
|
|
|
describe Projects::MergeRequestsController do
|
2017-09-29 08:04:50 +00:00
|
|
|
include ProjectForksHelper
|
|
|
|
|
2017-08-01 18:51:52 +00:00
|
|
|
let(:project) { create(:project, :repository) }
|
Use CTEs for nested groups and authorizations
This commit introduces the usage of Common Table Expressions (CTEs) to
efficiently retrieve nested group hierarchies, without having to rely on
the "routes" table (which is an _incredibly_ inefficient way of getting
the data). This requires a patch to ActiveRecord (found in the added
initializer) to work properly as ActiveRecord doesn't support WITH
statements properly out of the box.
Unfortunately MySQL provides no efficient way of getting nested groups.
For example, the old routes setup could easily take 5-10 seconds
depending on the amount of "routes" in a database. Providing vastly
different logic for both MySQL and PostgreSQL will negatively impact the
development process. Because of this the various nested groups related
methods return empty relations when used in combination with MySQL.
For project authorizations the logic is split up into two classes:
* Gitlab::ProjectAuthorizations::WithNestedGroups
* Gitlab::ProjectAuthorizations::WithoutNestedGroups
Both classes get the fresh project authorizations (= as they should be
in the "project_authorizations" table), including nested groups if
PostgreSQL is used. The logic of these two classes is quite different
apart from their public interface. This complicates development a bit,
but unfortunately there is no way around this.
This commit also introduces Gitlab::GroupHierarchy. This class can be
used to get the ancestors and descendants of a base relation, or both by
using a UNION. This in turn is used by methods such as:
* Namespace#ancestors
* Namespace#descendants
* User#all_expanded_groups
Again this class relies on CTEs and thus only works on PostgreSQL. The
Namespace methods will return an empty relation when MySQL is used,
while User#all_expanded_groups will return only the groups a user is a
direct member of.
Performance wise the impact is quite large. For example, on GitLab.com
Namespace#descendants used to take around 580 ms to retrieve data for a
particular user. Using CTEs we are able to reduce this down to roughly 1
millisecond, returning the exact same data.
== On The Fly Refreshing
Refreshing of authorizations on the fly (= when
users.authorized_projects_populated was not set) is removed with this
commit. This simplifies the code, and ensures any queries used for
authorizations are not mutated because they are executed in a Rails
scope (e.g. Project.visible_to_user).
This commit includes a migration to schedule refreshing authorizations
for all users, ensuring all of them have their authorizations in place.
Said migration schedules users in batches of 5000, with 5 minutes
between every batch to smear the load around a bit.
== Spec Changes
This commit also introduces some changes to various specs. For example,
some specs for ProjectTeam assumed that creating a personal project
would _not_ lead to the owner having access, which is incorrect. Because
we also no longer refresh authorizations on the fly for new users some
code had to be added to the "empty_project" factory. This chunk of code
ensures that the owner's permissions are refreshed after creating the
project, something that is normally done in Projects::CreateService.
2017-04-24 15:19:22 +00:00
|
|
|
let(:user) { project.owner }
|
2014-08-06 06:07:11 +00:00
|
|
|
let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
|
2016-07-27 16:54:04 +00:00
|
|
|
let(:merge_request_with_conflicts) do
|
2016-08-05 11:15:06 +00:00
|
|
|
create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project) do |mr|
|
2016-07-27 16:54:04 +00:00
|
|
|
mr.mark_as_unmergeable
|
|
|
|
end
|
|
|
|
end
|
2012-11-24 23:04:13 +00:00
|
|
|
|
|
|
|
before do
|
|
|
|
sign_in(user)
|
|
|
|
end
|
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
describe 'GET commit_change_content' do
|
|
|
|
it 'renders commit_change_content template' do
|
|
|
|
get :commit_change_content,
|
|
|
|
namespace_id: project.namespace.to_param,
|
|
|
|
project_id: project,
|
|
|
|
id: merge_request.iid,
|
|
|
|
format: 'html'
|
|
|
|
|
|
|
|
expect(response).to render_template('_commit_change_content')
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2016-11-11 00:04:28 +00:00
|
|
|
shared_examples "loads labels" do |action|
|
|
|
|
it "loads labels into the @labels variable" do
|
|
|
|
get action,
|
|
|
|
namespace_id: project.namespace.to_param,
|
2017-02-23 23:55:01 +00:00
|
|
|
project_id: project,
|
2016-11-11 00:04:28 +00:00
|
|
|
id: merge_request.iid,
|
|
|
|
format: 'html'
|
|
|
|
expect(assigns(:labels)).not_to be_nil
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2016-07-08 17:11:47 +00:00
|
|
|
describe "GET show" do
|
2017-05-09 04:15:34 +00:00
|
|
|
def go(extra_params = {})
|
|
|
|
params = {
|
|
|
|
namespace_id: project.namespace.to_param,
|
|
|
|
project_id: project,
|
|
|
|
id: merge_request.iid
|
|
|
|
}
|
2012-11-24 23:04:13 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
get :show, params.merge(extra_params)
|
|
|
|
end
|
2012-11-24 23:04:13 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
it_behaves_like "loads labels", :show
|
2016-11-11 00:04:28 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
describe 'as html' do
|
|
|
|
it "renders merge request page" do
|
|
|
|
go(format: :html)
|
2012-11-24 23:04:13 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(response).to be_success
|
2012-11-24 23:04:13 +00:00
|
|
|
end
|
2017-08-29 13:46:40 +00:00
|
|
|
|
|
|
|
context "loads notes" do
|
|
|
|
let(:first_contributor) { create(:user) }
|
|
|
|
let(:contributor) { create(:user) }
|
|
|
|
let(:merge_request) { create(:merge_request, author: first_contributor, target_project: project, source_project: project) }
|
|
|
|
let(:contributor_merge_request) { create(:merge_request, :merged, author: contributor, target_project: project, source_project: project) }
|
|
|
|
# the order here is important
|
|
|
|
# as the controller reloads these from DB, references doesn't correspond after
|
|
|
|
let!(:first_contributor_note) { create(:note, author: first_contributor, noteable: merge_request, project: project) }
|
|
|
|
let!(:contributor_note) { create(:note, author: contributor, noteable: merge_request, project: project) }
|
|
|
|
let!(:owner_note) { create(:note, author: user, noteable: merge_request, project: project) }
|
|
|
|
|
|
|
|
it "with special_role FIRST_TIME_CONTRIBUTOR" do
|
|
|
|
go(format: :html)
|
|
|
|
|
|
|
|
notes = assigns(:notes)
|
|
|
|
expect(notes).to match(a_collection_containing_exactly(an_object_having_attributes(special_role: Note::SpecialRole::FIRST_TIME_CONTRIBUTOR),
|
|
|
|
an_object_having_attributes(special_role: nil),
|
|
|
|
an_object_having_attributes(special_role: nil)
|
|
|
|
))
|
|
|
|
end
|
|
|
|
end
|
2017-05-09 04:15:34 +00:00
|
|
|
end
|
2012-11-24 23:04:13 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
describe 'as json' do
|
2017-10-31 16:15:03 +00:00
|
|
|
context 'with basic serializer param' do
|
2017-05-09 04:15:34 +00:00
|
|
|
it 'renders basic MR entity as json' do
|
2017-10-31 16:15:03 +00:00
|
|
|
go(serializer: 'basic', format: :json)
|
2012-11-24 23:04:13 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(response).to match_response_schema('entities/merge_request_basic')
|
|
|
|
end
|
2012-11-24 23:04:13 +00:00
|
|
|
end
|
|
|
|
|
2017-10-31 16:15:03 +00:00
|
|
|
context 'without basic serializer param' do
|
2017-05-09 04:15:34 +00:00
|
|
|
it 'renders the merge request in the json format' do
|
|
|
|
go(format: :json)
|
2012-11-24 23:04:13 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(response).to match_response_schema('entities/merge_request')
|
|
|
|
end
|
2012-11-24 23:04:13 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
describe "as diff" do
|
2016-05-12 18:50:49 +00:00
|
|
|
it "triggers workhorse to serve the request" do
|
2017-05-09 04:15:34 +00:00
|
|
|
go(format: :diff)
|
2012-11-24 23:04:13 +00:00
|
|
|
|
2016-06-08 12:30:15 +00:00
|
|
|
expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-diff:")
|
2012-11-24 23:04:13 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
describe "as patch" do
|
2016-06-10 12:57:50 +00:00
|
|
|
it 'triggers workhorse to serve the request' do
|
2017-05-09 04:15:34 +00:00
|
|
|
go(format: :patch)
|
2012-11-24 23:04:13 +00:00
|
|
|
|
2016-07-03 21:01:13 +00:00
|
|
|
expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-format-patch:")
|
2012-11-24 23:04:13 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2015-04-13 05:27:45 +00:00
|
|
|
|
2016-07-08 17:11:47 +00:00
|
|
|
describe 'GET index' do
|
2017-08-01 16:55:57 +00:00
|
|
|
let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
|
2017-01-23 20:40:25 +00:00
|
|
|
|
2016-12-20 18:52:09 +00:00
|
|
|
def get_merge_requests(page = nil)
|
2016-02-18 02:15:13 +00:00
|
|
|
get :index,
|
|
|
|
namespace_id: project.namespace.to_param,
|
2017-02-23 23:55:01 +00:00
|
|
|
project_id: project,
|
2016-12-20 18:52:09 +00:00
|
|
|
state: 'opened', page: page.to_param
|
|
|
|
end
|
|
|
|
|
2017-01-23 20:40:25 +00:00
|
|
|
it_behaves_like "issuables list meta-data", :merge_request
|
|
|
|
|
2016-12-20 18:52:09 +00:00
|
|
|
context 'when page param' do
|
2016-12-21 15:02:05 +00:00
|
|
|
let(:last_page) { project.merge_requests.page().total_pages }
|
|
|
|
let!(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
|
|
|
|
|
2016-12-20 18:52:09 +00:00
|
|
|
it 'redirects to last_page if page number is larger than number of pages' do
|
|
|
|
get_merge_requests(last_page + 1)
|
|
|
|
|
2016-12-21 15:02:05 +00:00
|
|
|
expect(response).to redirect_to(namespace_project_merge_requests_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope]))
|
2016-12-20 18:52:09 +00:00
|
|
|
end
|
|
|
|
|
|
|
|
it 'redirects to specified page' do
|
|
|
|
get_merge_requests(last_page)
|
|
|
|
|
|
|
|
expect(assigns(:merge_requests).current_page).to eq(last_page)
|
2017-10-19 18:28:19 +00:00
|
|
|
expect(response).to have_gitlab_http_status(200)
|
2016-12-20 18:52:09 +00:00
|
|
|
end
|
2017-04-05 22:52:19 +00:00
|
|
|
|
|
|
|
it 'does not redirect to external sites when provided a host field' do
|
|
|
|
external_host = "www.example.com"
|
|
|
|
get :index,
|
|
|
|
namespace_id: project.namespace.to_param,
|
|
|
|
project_id: project,
|
|
|
|
state: 'opened',
|
|
|
|
page: (last_page + 1).to_param,
|
|
|
|
host: external_host
|
|
|
|
|
|
|
|
expect(response).to redirect_to(namespace_project_merge_requests_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope]))
|
|
|
|
end
|
2016-02-18 02:15:13 +00:00
|
|
|
end
|
|
|
|
|
|
|
|
context 'when filtering by opened state' do
|
|
|
|
context 'with opened merge requests' do
|
2016-07-25 18:16:19 +00:00
|
|
|
it 'lists those merge requests' do
|
2017-08-01 16:55:57 +00:00
|
|
|
expect(merge_request).to be_persisted
|
|
|
|
|
2016-02-18 02:15:13 +00:00
|
|
|
get_merge_requests
|
|
|
|
|
|
|
|
expect(assigns(:merge_requests)).to include(merge_request)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'with reopened merge requests' do
|
|
|
|
before do
|
|
|
|
merge_request.close!
|
|
|
|
merge_request.reopen!
|
|
|
|
end
|
|
|
|
|
2016-07-25 18:16:19 +00:00
|
|
|
it 'lists those merge requests' do
|
2016-02-18 02:15:13 +00:00
|
|
|
get_merge_requests
|
|
|
|
|
|
|
|
expect(assigns(:merge_requests)).to include(merge_request)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2016-07-08 17:11:47 +00:00
|
|
|
describe 'PUT update' do
|
2017-11-01 17:35:14 +00:00
|
|
|
def update_merge_request(mr_params, additional_params = {})
|
|
|
|
params = {
|
|
|
|
namespace_id: project.namespace,
|
|
|
|
project_id: project,
|
|
|
|
id: merge_request.iid,
|
|
|
|
merge_request: mr_params
|
|
|
|
}.merge(additional_params)
|
|
|
|
|
|
|
|
put :update, params
|
|
|
|
end
|
|
|
|
|
2017-03-18 04:23:15 +00:00
|
|
|
context 'changing the assignee' do
|
|
|
|
it 'limits the attributes exposed on the assignee' do
|
|
|
|
assignee = create(:user)
|
|
|
|
project.add_developer(assignee)
|
|
|
|
|
2017-11-01 17:35:14 +00:00
|
|
|
update_merge_request({ assignee_id: assignee.id }, format: :json)
|
2017-03-18 04:23:15 +00:00
|
|
|
body = JSON.parse(response.body)
|
|
|
|
|
|
|
|
expect(body['assignee'].keys)
|
|
|
|
.to match_array(%w(name username avatar_url))
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2017-11-01 17:35:14 +00:00
|
|
|
context 'when user does not have access to update issue' do
|
|
|
|
before do
|
|
|
|
reporter = create(:user)
|
|
|
|
project.add_reporter(reporter)
|
|
|
|
sign_in(reporter)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'responds with 404' do
|
|
|
|
update_merge_request(title: 'New title')
|
|
|
|
|
|
|
|
expect(response).to have_http_status(:not_found)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2016-04-11 19:21:32 +00:00
|
|
|
context 'there is no source project' do
|
2017-08-01 18:51:52 +00:00
|
|
|
let(:project) { create(:project, :repository) }
|
2017-09-29 08:04:50 +00:00
|
|
|
let(:forked_project) { fork_project_with_submodules(project) }
|
|
|
|
let!(:merge_request) { create(:merge_request, source_project: forked_project, source_branch: 'add-submodule-version-bump', target_branch: 'master', target_project: project) }
|
2016-04-11 19:21:32 +00:00
|
|
|
|
|
|
|
before do
|
2017-09-29 08:04:50 +00:00
|
|
|
forked_project.destroy
|
2016-04-11 19:21:32 +00:00
|
|
|
end
|
|
|
|
|
|
|
|
it 'closes MR without errors' do
|
2017-11-01 17:35:14 +00:00
|
|
|
update_merge_request(state_event: 'close')
|
2016-04-11 19:21:32 +00:00
|
|
|
|
|
|
|
expect(response).to redirect_to([merge_request.target_project.namespace.becomes(Namespace), merge_request.target_project, merge_request])
|
|
|
|
expect(merge_request.reload.closed?).to be_truthy
|
|
|
|
end
|
2016-07-26 11:57:43 +00:00
|
|
|
|
2016-08-10 13:36:30 +00:00
|
|
|
it 'allows editing of a closed merge request' do
|
2016-07-26 11:57:43 +00:00
|
|
|
merge_request.close!
|
|
|
|
|
2017-11-01 17:35:14 +00:00
|
|
|
update_merge_request(title: 'New title')
|
2016-07-26 11:57:43 +00:00
|
|
|
|
|
|
|
expect(response).to redirect_to([merge_request.target_project.namespace.becomes(Namespace), merge_request.target_project, merge_request])
|
|
|
|
expect(merge_request.reload.title).to eq 'New title'
|
|
|
|
end
|
|
|
|
|
2016-08-10 13:36:30 +00:00
|
|
|
it 'does not allow to update target branch closed merge request' do
|
2016-07-26 11:57:43 +00:00
|
|
|
merge_request.close!
|
|
|
|
|
2017-11-01 17:35:14 +00:00
|
|
|
update_merge_request(target_branch: 'new_branch')
|
2016-07-26 11:57:43 +00:00
|
|
|
|
|
|
|
expect { merge_request.reload.target_branch }.not_to change { merge_request.target_branch }
|
|
|
|
end
|
2017-01-16 18:43:03 +00:00
|
|
|
|
|
|
|
it_behaves_like 'update invalid issuable', MergeRequest
|
2016-04-11 19:21:32 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2016-07-08 17:11:47 +00:00
|
|
|
describe 'POST merge' do
|
2016-06-01 12:25:44 +00:00
|
|
|
let(:base_params) do
|
|
|
|
{
|
2017-02-23 23:55:01 +00:00
|
|
|
namespace_id: project.namespace,
|
|
|
|
project_id: project,
|
2016-06-01 12:25:44 +00:00
|
|
|
id: merge_request.iid,
|
2017-05-09 04:15:34 +00:00
|
|
|
format: 'json'
|
2016-06-01 12:25:44 +00:00
|
|
|
}
|
|
|
|
end
|
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
context 'when user cannot access' do
|
Use CTEs for nested groups and authorizations
This commit introduces the usage of Common Table Expressions (CTEs) to
efficiently retrieve nested group hierarchies, without having to rely on
the "routes" table (which is an _incredibly_ inefficient way of getting
the data). This requires a patch to ActiveRecord (found in the added
initializer) to work properly as ActiveRecord doesn't support WITH
statements properly out of the box.
Unfortunately MySQL provides no efficient way of getting nested groups.
For example, the old routes setup could easily take 5-10 seconds
depending on the amount of "routes" in a database. Providing vastly
different logic for both MySQL and PostgreSQL will negatively impact the
development process. Because of this the various nested groups related
methods return empty relations when used in combination with MySQL.
For project authorizations the logic is split up into two classes:
* Gitlab::ProjectAuthorizations::WithNestedGroups
* Gitlab::ProjectAuthorizations::WithoutNestedGroups
Both classes get the fresh project authorizations (= as they should be
in the "project_authorizations" table), including nested groups if
PostgreSQL is used. The logic of these two classes is quite different
apart from their public interface. This complicates development a bit,
but unfortunately there is no way around this.
This commit also introduces Gitlab::GroupHierarchy. This class can be
used to get the ancestors and descendants of a base relation, or both by
using a UNION. This in turn is used by methods such as:
* Namespace#ancestors
* Namespace#descendants
* User#all_expanded_groups
Again this class relies on CTEs and thus only works on PostgreSQL. The
Namespace methods will return an empty relation when MySQL is used,
while User#all_expanded_groups will return only the groups a user is a
direct member of.
Performance wise the impact is quite large. For example, on GitLab.com
Namespace#descendants used to take around 580 ms to retrieve data for a
particular user. Using CTEs we are able to reduce this down to roughly 1
millisecond, returning the exact same data.
== On The Fly Refreshing
Refreshing of authorizations on the fly (= when
users.authorized_projects_populated was not set) is removed with this
commit. This simplifies the code, and ensures any queries used for
authorizations are not mutated because they are executed in a Rails
scope (e.g. Project.visible_to_user).
This commit includes a migration to schedule refreshing authorizations
for all users, ensuring all of them have their authorizations in place.
Said migration schedules users in batches of 5000, with 5 minutes
between every batch to smear the load around a bit.
== Spec Changes
This commit also introduces some changes to various specs. For example,
some specs for ProjectTeam assumed that creating a personal project
would _not_ lead to the owner having access, which is incorrect. Because
we also no longer refresh authorizations on the fly for new users some
code had to be added to the "empty_project" factory. This chunk of code
ensures that the owner's permissions are refreshed after creating the
project, something that is normally done in Projects::CreateService.
2017-04-24 15:19:22 +00:00
|
|
|
let(:user) { create(:user) }
|
|
|
|
|
2016-06-01 12:25:44 +00:00
|
|
|
before do
|
2017-05-09 04:15:34 +00:00
|
|
|
project.add_reporter(user)
|
|
|
|
xhr :post, :merge, base_params
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
it 'returns 404' do
|
2017-10-19 18:28:19 +00:00
|
|
|
expect(response).to have_gitlab_http_status(404)
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when the merge request is not mergeable' do
|
|
|
|
before do
|
|
|
|
merge_request.update_attributes(title: "WIP: #{merge_request.title}")
|
|
|
|
|
|
|
|
post :merge, base_params
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns :failed' do
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(json_response).to eq('status' => 'failed')
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when the sha parameter does not match the source SHA' do
|
2017-06-14 18:18:56 +00:00
|
|
|
before do
|
|
|
|
post :merge, base_params.merge(sha: 'foo')
|
|
|
|
end
|
2016-06-01 12:25:44 +00:00
|
|
|
|
|
|
|
it 'returns :sha_mismatch' do
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(json_response).to eq('status' => 'sha_mismatch')
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when the sha parameter matches the source SHA' do
|
|
|
|
def merge_with_sha
|
2016-06-20 16:48:04 +00:00
|
|
|
post :merge, base_params.merge(sha: merge_request.diff_head_sha)
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns :success' do
|
|
|
|
merge_with_sha
|
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(json_response).to eq('status' => 'success')
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
|
|
|
|
|
|
|
it 'starts the merge immediately' do
|
|
|
|
expect(MergeWorker).to receive(:perform_async).with(merge_request.id, anything, anything)
|
|
|
|
|
|
|
|
merge_with_sha
|
|
|
|
end
|
|
|
|
|
2017-02-17 13:56:13 +00:00
|
|
|
context 'when the pipeline succeeds is passed' do
|
|
|
|
def merge_when_pipeline_succeeds
|
|
|
|
post :merge, base_params.merge(sha: merge_request.diff_head_sha, merge_when_pipeline_succeeds: '1')
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
|
|
|
|
|
|
|
before do
|
2017-05-19 20:51:07 +00:00
|
|
|
create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch, head_pipeline_of: merge_request)
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
|
|
|
|
2017-02-17 13:56:13 +00:00
|
|
|
it 'returns :merge_when_pipeline_succeeds' do
|
|
|
|
merge_when_pipeline_succeeds
|
2016-06-01 12:25:44 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(json_response).to eq('status' => 'merge_when_pipeline_succeeds')
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
|
|
|
|
2017-02-17 13:56:13 +00:00
|
|
|
it 'sets the MR to merge when the pipeline succeeds' do
|
|
|
|
service = double(:merge_when_pipeline_succeeds_service)
|
2016-06-01 12:25:44 +00:00
|
|
|
|
2017-02-22 22:54:59 +00:00
|
|
|
expect(MergeRequests::MergeWhenPipelineSucceedsService)
|
|
|
|
.to receive(:new).with(project, anything, anything)
|
|
|
|
.and_return(service)
|
2016-06-01 12:25:44 +00:00
|
|
|
expect(service).to receive(:execute).with(merge_request)
|
|
|
|
|
2017-02-17 13:56:13 +00:00
|
|
|
merge_when_pipeline_succeeds
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
2016-06-24 16:13:56 +00:00
|
|
|
|
2017-02-17 13:56:13 +00:00
|
|
|
context 'when project.only_allow_merge_if_pipeline_succeeds? is true' do
|
2016-06-24 16:13:56 +00:00
|
|
|
before do
|
2017-02-17 13:56:13 +00:00
|
|
|
project.update_column(:only_allow_merge_if_pipeline_succeeds, true)
|
2016-06-24 16:13:56 +00:00
|
|
|
end
|
|
|
|
|
2017-02-17 13:56:13 +00:00
|
|
|
it 'returns :merge_when_pipeline_succeeds' do
|
|
|
|
merge_when_pipeline_succeeds
|
2016-06-24 16:13:56 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(json_response).to eq('status' => 'merge_when_pipeline_succeeds')
|
2016-06-24 16:13:56 +00:00
|
|
|
end
|
|
|
|
end
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
2016-09-16 11:02:42 +00:00
|
|
|
|
2016-10-26 17:19:17 +00:00
|
|
|
describe 'only_allow_merge_if_all_discussions_are_resolved? setting' do
|
|
|
|
let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project, author: user) }
|
|
|
|
|
|
|
|
context 'when enabled' do
|
|
|
|
before do
|
|
|
|
project.update_column(:only_allow_merge_if_all_discussions_are_resolved, true)
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'with unresolved discussion' do
|
|
|
|
before do
|
|
|
|
expect(merge_request).not_to be_discussions_resolved
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns :failed' do
|
|
|
|
merge_with_sha
|
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(json_response).to eq('status' => 'failed')
|
2016-10-26 17:19:17 +00:00
|
|
|
end
|
|
|
|
end
|
2016-09-16 11:02:42 +00:00
|
|
|
|
2016-10-26 17:19:17 +00:00
|
|
|
context 'with all discussions resolved' do
|
|
|
|
before do
|
|
|
|
merge_request.discussions.each { |d| d.resolve!(user) }
|
|
|
|
expect(merge_request).to be_discussions_resolved
|
|
|
|
end
|
2016-09-16 11:02:42 +00:00
|
|
|
|
2016-10-26 17:19:17 +00:00
|
|
|
it 'returns :success' do
|
|
|
|
merge_with_sha
|
2016-09-16 11:02:42 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(json_response).to eq('status' => 'success')
|
2016-10-26 17:19:17 +00:00
|
|
|
end
|
2016-09-16 11:02:42 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2016-10-26 17:19:17 +00:00
|
|
|
context 'when disabled' do
|
|
|
|
before do
|
|
|
|
project.update_column(:only_allow_merge_if_all_discussions_are_resolved, false)
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'with unresolved discussion' do
|
|
|
|
before do
|
|
|
|
expect(merge_request).not_to be_discussions_resolved
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns :success' do
|
|
|
|
merge_with_sha
|
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(json_response).to eq('status' => 'success')
|
2016-10-26 17:19:17 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'with all discussions resolved' do
|
|
|
|
before do
|
|
|
|
merge_request.discussions.each { |d| d.resolve!(user) }
|
|
|
|
expect(merge_request).to be_discussions_resolved
|
|
|
|
end
|
2016-09-16 11:02:42 +00:00
|
|
|
|
2016-10-26 17:19:17 +00:00
|
|
|
it 'returns :success' do
|
|
|
|
merge_with_sha
|
2016-09-16 11:02:42 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
expect(json_response).to eq('status' => 'success')
|
2016-10-26 17:19:17 +00:00
|
|
|
end
|
2016-09-16 11:02:42 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2016-06-01 12:25:44 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2016-07-08 17:11:47 +00:00
|
|
|
describe "DELETE destroy" do
|
Use CTEs for nested groups and authorizations
This commit introduces the usage of Common Table Expressions (CTEs) to
efficiently retrieve nested group hierarchies, without having to rely on
the "routes" table (which is an _incredibly_ inefficient way of getting
the data). This requires a patch to ActiveRecord (found in the added
initializer) to work properly as ActiveRecord doesn't support WITH
statements properly out of the box.
Unfortunately MySQL provides no efficient way of getting nested groups.
For example, the old routes setup could easily take 5-10 seconds
depending on the amount of "routes" in a database. Providing vastly
different logic for both MySQL and PostgreSQL will negatively impact the
development process. Because of this the various nested groups related
methods return empty relations when used in combination with MySQL.
For project authorizations the logic is split up into two classes:
* Gitlab::ProjectAuthorizations::WithNestedGroups
* Gitlab::ProjectAuthorizations::WithoutNestedGroups
Both classes get the fresh project authorizations (= as they should be
in the "project_authorizations" table), including nested groups if
PostgreSQL is used. The logic of these two classes is quite different
apart from their public interface. This complicates development a bit,
but unfortunately there is no way around this.
This commit also introduces Gitlab::GroupHierarchy. This class can be
used to get the ancestors and descendants of a base relation, or both by
using a UNION. This in turn is used by methods such as:
* Namespace#ancestors
* Namespace#descendants
* User#all_expanded_groups
Again this class relies on CTEs and thus only works on PostgreSQL. The
Namespace methods will return an empty relation when MySQL is used,
while User#all_expanded_groups will return only the groups a user is a
direct member of.
Performance wise the impact is quite large. For example, on GitLab.com
Namespace#descendants used to take around 580 ms to retrieve data for a
particular user. Using CTEs we are able to reduce this down to roughly 1
millisecond, returning the exact same data.
== On The Fly Refreshing
Refreshing of authorizations on the fly (= when
users.authorized_projects_populated was not set) is removed with this
commit. This simplifies the code, and ensures any queries used for
authorizations are not mutated because they are executed in a Rails
scope (e.g. Project.visible_to_user).
This commit includes a migration to schedule refreshing authorizations
for all users, ensuring all of them have their authorizations in place.
Said migration schedules users in batches of 5000, with 5 minutes
between every batch to smear the load around a bit.
== Spec Changes
This commit also introduces some changes to various specs. For example,
some specs for ProjectTeam assumed that creating a personal project
would _not_ lead to the owner having access, which is incorrect. Because
we also no longer refresh authorizations on the fly for new users some
code had to be added to the "empty_project" factory. This chunk of code
ensures that the owner's permissions are refreshed after creating the
project, something that is normally done in Projects::CreateService.
2017-04-24 15:19:22 +00:00
|
|
|
let(:user) { create(:user) }
|
|
|
|
|
2016-03-21 13:12:52 +00:00
|
|
|
it "denies access to users unless they're admin or project owner" do
|
2017-02-23 23:55:01 +00:00
|
|
|
delete :destroy, namespace_id: project.namespace, project_id: project, id: merge_request.iid
|
2016-02-26 08:55:43 +00:00
|
|
|
|
2017-10-19 18:28:19 +00:00
|
|
|
expect(response).to have_gitlab_http_status(404)
|
2016-02-26 08:55:43 +00:00
|
|
|
end
|
|
|
|
|
2016-03-21 13:12:52 +00:00
|
|
|
context "when the user is owner" do
|
|
|
|
let(:owner) { create(:user) }
|
|
|
|
let(:namespace) { create(:namespace, owner: owner) }
|
2017-08-01 18:51:52 +00:00
|
|
|
let(:project) { create(:project, :repository, namespace: namespace) }
|
2016-03-21 13:12:52 +00:00
|
|
|
|
2017-06-14 18:18:56 +00:00
|
|
|
before do
|
|
|
|
sign_in owner
|
|
|
|
end
|
2016-02-26 08:55:43 +00:00
|
|
|
|
2016-03-21 13:12:52 +00:00
|
|
|
it "deletes the merge request" do
|
2017-02-23 23:55:01 +00:00
|
|
|
delete :destroy, namespace_id: project.namespace, project_id: project, id: merge_request.iid
|
2016-02-26 08:55:43 +00:00
|
|
|
|
2017-10-19 18:28:19 +00:00
|
|
|
expect(response).to have_gitlab_http_status(302)
|
2017-07-24 12:21:16 +00:00
|
|
|
expect(controller).to set_flash[:notice].to(/The merge request was successfully deleted\./)
|
2016-02-26 08:55:43 +00:00
|
|
|
end
|
2016-09-01 22:12:05 +00:00
|
|
|
|
|
|
|
it 'delegates the update of the todos count cache to TodoService' do
|
2017-11-02 14:51:42 +00:00
|
|
|
expect_any_instance_of(TodoService).to receive(:destroy_issuable).with(merge_request, owner).once
|
2016-09-01 22:12:05 +00:00
|
|
|
|
2017-02-23 23:55:01 +00:00
|
|
|
delete :destroy, namespace_id: project.namespace, project_id: project, id: merge_request.iid
|
2016-09-01 22:12:05 +00:00
|
|
|
end
|
2016-02-26 08:55:43 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2016-07-08 21:50:06 +00:00
|
|
|
describe 'GET commits' do
|
2015-06-17 20:12:28 +00:00
|
|
|
def go(format: 'html')
|
2015-06-23 05:24:39 +00:00
|
|
|
get :commits,
|
|
|
|
namespace_id: project.namespace.to_param,
|
2017-02-23 23:55:01 +00:00
|
|
|
project_id: project,
|
2015-06-23 05:24:39 +00:00
|
|
|
id: merge_request.iid,
|
|
|
|
format: format
|
2015-06-17 20:12:28 +00:00
|
|
|
end
|
|
|
|
|
2017-06-13 22:12:31 +00:00
|
|
|
it 'renders the commits template to a string' do
|
|
|
|
go format: 'json'
|
2015-06-17 20:12:28 +00:00
|
|
|
|
2017-06-13 22:12:31 +00:00
|
|
|
expect(response).to render_template('projects/merge_requests/_commits')
|
|
|
|
expect(json_response).to have_key('html')
|
2015-04-13 05:27:45 +00:00
|
|
|
end
|
|
|
|
end
|
2016-07-27 11:42:18 +00:00
|
|
|
|
2016-11-11 00:04:28 +00:00
|
|
|
describe 'GET pipelines' do
|
2017-01-27 13:45:56 +00:00
|
|
|
before do
|
|
|
|
create(:ci_pipeline, project: merge_request.source_project,
|
|
|
|
ref: merge_request.source_branch,
|
|
|
|
sha: merge_request.diff_head_sha)
|
2016-07-29 13:51:11 +00:00
|
|
|
|
2017-06-13 22:12:31 +00:00
|
|
|
get :pipelines,
|
|
|
|
namespace_id: project.namespace.to_param,
|
|
|
|
project_id: project,
|
|
|
|
id: merge_request.iid,
|
|
|
|
format: :json
|
2016-07-29 13:51:11 +00:00
|
|
|
end
|
|
|
|
|
2017-06-13 22:12:31 +00:00
|
|
|
it 'responds with serialized pipelines' do
|
2017-07-14 15:52:54 +00:00
|
|
|
expect(json_response['pipelines']).not_to be_empty
|
|
|
|
expect(json_response['count']['all']).to eq 1
|
2016-07-27 11:42:18 +00:00
|
|
|
end
|
|
|
|
end
|
2016-07-27 16:54:04 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
describe 'POST remove_wip' do
|
|
|
|
before do
|
2016-09-08 09:18:41 +00:00
|
|
|
merge_request.title = merge_request.wip_title
|
|
|
|
merge_request.save
|
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
xhr :post, :remove_wip,
|
|
|
|
namespace_id: merge_request.project.namespace.to_param,
|
|
|
|
project_id: merge_request.project,
|
|
|
|
id: merge_request.iid,
|
|
|
|
format: :json
|
|
|
|
end
|
2016-09-08 09:18:41 +00:00
|
|
|
|
2017-05-09 04:15:34 +00:00
|
|
|
it 'removes the wip status' do
|
2016-09-08 09:18:41 +00:00
|
|
|
expect(merge_request.reload.title).to eq(merge_request.wipless_title)
|
2016-10-05 12:57:57 +00:00
|
|
|
end
|
2017-05-09 04:15:34 +00:00
|
|
|
|
|
|
|
it 'renders MergeRequest as JSON' do
|
|
|
|
expect(json_response.keys).to include('id', 'iid', 'description')
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
describe 'POST cancel_merge_when_pipeline_succeeds' do
|
|
|
|
subject do
|
|
|
|
xhr :post, :cancel_merge_when_pipeline_succeeds,
|
|
|
|
namespace_id: merge_request.project.namespace.to_param,
|
|
|
|
project_id: merge_request.project,
|
|
|
|
id: merge_request.iid,
|
|
|
|
format: :json
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'calls MergeRequests::MergeWhenPipelineSucceedsService' do
|
|
|
|
mwps_service = double
|
|
|
|
|
|
|
|
allow(MergeRequests::MergeWhenPipelineSucceedsService)
|
|
|
|
.to receive(:new)
|
|
|
|
.and_return(mwps_service)
|
|
|
|
|
|
|
|
expect(mwps_service).to receive(:cancel).with(merge_request)
|
|
|
|
|
|
|
|
subject
|
|
|
|
end
|
|
|
|
|
2017-10-19 18:28:19 +00:00
|
|
|
it { is_expected.to have_gitlab_http_status(:success) }
|
2017-05-09 04:15:34 +00:00
|
|
|
|
|
|
|
it 'renders MergeRequest as JSON' do
|
|
|
|
subject
|
|
|
|
|
|
|
|
expect(json_response.keys).to include('id', 'iid', 'description')
|
|
|
|
end
|
2016-09-01 12:59:10 +00:00
|
|
|
end
|
|
|
|
|
2016-08-08 22:30:01 +00:00
|
|
|
describe 'POST assign_related_issues' do
|
|
|
|
let(:issue1) { create(:issue, project: project) }
|
|
|
|
let(:issue2) { create(:issue, project: project) }
|
|
|
|
|
|
|
|
def post_assign_issues
|
|
|
|
merge_request.update!(description: "Closes #{issue1.to_reference} and #{issue2.to_reference}",
|
|
|
|
author: user,
|
|
|
|
source_branch: 'feature',
|
|
|
|
target_branch: 'master')
|
|
|
|
|
|
|
|
post :assign_related_issues,
|
|
|
|
namespace_id: project.namespace.to_param,
|
2017-02-23 23:55:01 +00:00
|
|
|
project_id: project,
|
2016-08-08 22:30:01 +00:00
|
|
|
id: merge_request.iid
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'shows a flash message on success' do
|
|
|
|
post_assign_issues
|
|
|
|
|
|
|
|
expect(flash[:notice]).to eq '2 issues have been assigned to you'
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'correctly pluralizes flash message on success' do
|
2017-05-04 12:11:15 +00:00
|
|
|
issue2.assignees = [user]
|
2016-08-08 22:30:01 +00:00
|
|
|
|
|
|
|
post_assign_issues
|
|
|
|
|
|
|
|
expect(flash[:notice]).to eq '1 issue has been assigned to you'
|
|
|
|
end
|
2016-10-07 16:16:42 +00:00
|
|
|
|
|
|
|
it 'calls MergeRequests::AssignIssuesService' do
|
2017-06-21 13:48:12 +00:00
|
|
|
expect(MergeRequests::AssignIssuesService).to receive(:new)
|
|
|
|
.with(project, user, merge_request: merge_request)
|
|
|
|
.and_return(double(execute: { count: 1 }))
|
2016-10-07 16:16:42 +00:00
|
|
|
|
|
|
|
post_assign_issues
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'is skipped when not signed in' do
|
|
|
|
project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
|
|
|
|
sign_out(:user)
|
|
|
|
|
|
|
|
expect(MergeRequests::AssignIssuesService).not_to receive(:new)
|
|
|
|
|
|
|
|
post_assign_issues
|
|
|
|
end
|
2016-08-08 22:30:01 +00:00
|
|
|
end
|
2016-10-13 12:23:18 +00:00
|
|
|
|
|
|
|
describe 'GET ci_environments_status' do
|
2016-10-19 12:49:09 +00:00
|
|
|
context 'the environment is from a forked project' do
|
2017-09-29 08:04:50 +00:00
|
|
|
let!(:forked) { fork_project(project, user, repository: true) }
|
2016-10-13 12:23:18 +00:00
|
|
|
let!(:environment) { create(:environment, project: forked) }
|
|
|
|
let!(:deployment) { create(:deployment, environment: environment, sha: forked.commit.id, ref: 'master') }
|
|
|
|
let(:admin) { create(:admin) }
|
|
|
|
|
|
|
|
let(:merge_request) do
|
|
|
|
create(:merge_request, source_project: forked, target_project: project)
|
|
|
|
end
|
|
|
|
|
|
|
|
before do
|
|
|
|
get :ci_environments_status,
|
|
|
|
namespace_id: merge_request.project.namespace.to_param,
|
2017-02-23 23:55:01 +00:00
|
|
|
project_id: merge_request.project,
|
2016-10-13 12:23:18 +00:00
|
|
|
id: merge_request.iid, format: 'json'
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'links to the environment on that project' do
|
2017-07-20 09:34:09 +00:00
|
|
|
expect(json_response.first['url']).to match /#{forked.full_path}/
|
2016-10-13 12:23:18 +00:00
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2016-12-21 10:32:37 +00:00
|
|
|
|
2017-03-17 06:58:12 +00:00
|
|
|
describe 'GET pipeline_status.json' do
|
2017-03-21 13:21:13 +00:00
|
|
|
context 'when head_pipeline exists' do
|
|
|
|
let!(:pipeline) do
|
2017-03-06 12:01:18 +00:00
|
|
|
create(:ci_pipeline, project: merge_request.source_project,
|
2017-03-06 15:42:39 +00:00
|
|
|
ref: merge_request.source_branch,
|
2017-05-19 20:51:07 +00:00
|
|
|
sha: merge_request.diff_head_sha,
|
|
|
|
head_pipeline_of: merge_request)
|
2017-03-06 12:01:18 +00:00
|
|
|
end
|
|
|
|
|
2017-03-21 13:21:13 +00:00
|
|
|
let(:status) { pipeline.detailed_status(double('user')) }
|
|
|
|
|
2017-03-28 20:04:14 +00:00
|
|
|
before do
|
|
|
|
get_pipeline_status
|
|
|
|
end
|
2017-03-21 13:21:13 +00:00
|
|
|
|
|
|
|
it 'return a detailed head_pipeline status in json' do
|
2017-10-19 18:28:19 +00:00
|
|
|
expect(response).to have_gitlab_http_status(:ok)
|
2017-03-11 14:30:25 +00:00
|
|
|
expect(json_response['text']).to eq status.text
|
|
|
|
expect(json_response['label']).to eq status.label
|
|
|
|
expect(json_response['icon']).to eq status.icon
|
2017-10-03 14:47:56 +00:00
|
|
|
expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.ico"
|
2017-03-06 12:01:18 +00:00
|
|
|
end
|
|
|
|
end
|
2017-03-21 13:21:13 +00:00
|
|
|
|
|
|
|
context 'when head_pipeline does not exist' do
|
2017-06-14 18:18:56 +00:00
|
|
|
before do
|
|
|
|
get_pipeline_status
|
|
|
|
end
|
2017-03-21 13:21:13 +00:00
|
|
|
|
|
|
|
it 'return empty' do
|
2017-10-19 18:28:19 +00:00
|
|
|
expect(response).to have_gitlab_http_status(:ok)
|
2017-03-21 13:21:13 +00:00
|
|
|
expect(json_response).to be_empty
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
def get_pipeline_status
|
|
|
|
get :pipeline_status, namespace_id: project.namespace,
|
|
|
|
project_id: project,
|
|
|
|
id: merge_request.iid,
|
|
|
|
format: :json
|
|
|
|
end
|
2017-03-06 12:01:18 +00:00
|
|
|
end
|
2012-11-24 23:04:13 +00:00
|
|
|
end
|