diff options
Diffstat (limited to 'spec/controllers/projects/compare_controller_spec.rb')
| -rw-r--r-- | spec/controllers/projects/compare_controller_spec.rb | 243 |
1 files changed, 165 insertions, 78 deletions
diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 6aa4bfe235b..80a6d3960cd 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -3,8 +3,21 @@ require 'spec_helper' RSpec.describe Projects::CompareController do - let(:project) { create(:project, :repository) } - let(:user) { create(:user) } + include ProjectForksHelper + + using RSpec::Parameterized::TableSyntax + + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:user) { create(:user) } + + let(:private_fork) { fork_project(project, nil, repository: true).tap { |fork| fork.update!(visibility: 'private') } } + let(:public_fork) do + fork_project(project, nil, repository: true).tap do |fork| + fork.update!(visibility: 'public') + # Create a reference that only exists in this project + fork.repository.create_ref('refs/heads/improve/awesome', 'refs/heads/improve/more-awesome') + end + end before do sign_in(user) @@ -32,18 +45,20 @@ RSpec.describe Projects::CompareController do { namespace_id: project.namespace, project_id: project, - from: source_ref, - to: target_ref, + from_project_id: from_project_id, + from: from_ref, + to: to_ref, w: whitespace } end let(:whitespace) { nil } - context 'when the refs exist' do + context 'when the refs exist in the same project' do context 'when we set the white space param' do - let(:source_ref) { "08f22f25" } - let(:target_ref) { "66eceea0" } + let(:from_project_id) { nil } + let(:from_ref) { '08f22f25' } + let(:to_ref) { '66eceea0' } let(:whitespace) { 1 } it 'shows some diffs with ignore whitespace change option' do @@ -60,8 +75,9 @@ RSpec.describe Projects::CompareController do end context 'when we do not set the white space param' do - let(:source_ref) { "improve%2Fawesome" } - let(:target_ref) { "feature" } + let(:from_project_id) { nil } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { 'feature' } let(:whitespace) { nil } it 'sets the diffs and commits ivars' do @@ -74,9 +90,40 @@ RSpec.describe Projects::CompareController do end end + context 'when the refs exist in different projects that the user can see' do + let(:from_project_id) { public_fork.id } + let(:from_ref) { 'improve%2Fmore-awesome' } + let(:to_ref) { 'feature' } + let(:whitespace) { nil } + + it 'shows the diff' do + show_request + + expect(response).to be_successful + expect(assigns(:diffs).diff_files.first).not_to be_nil + expect(assigns(:commits).length).to be >= 1 + end + end + + context 'when the refs exist in different projects but the user cannot see' do + let(:from_project_id) { private_fork.id } + let(:from_ref) { 'improve%2Fmore-awesome' } + let(:to_ref) { 'feature' } + let(:whitespace) { nil } + + it 'does not show the diff' do + show_request + + expect(response).to be_successful + expect(assigns(:diffs)).to be_empty + expect(assigns(:commits)).to be_empty + end + end + context 'when the source ref does not exist' do - let(:source_ref) { 'non-existent-source-ref' } - let(:target_ref) { "feature" } + let(:from_project_id) { nil } + let(:from_ref) { 'non-existent-source-ref' } + let(:to_ref) { 'feature' } it 'sets empty diff and commit ivars' do show_request @@ -88,8 +135,9 @@ RSpec.describe Projects::CompareController do end context 'when the target ref does not exist' do - let(:target_ref) { 'non-existent-target-ref' } - let(:source_ref) { "improve%2Fawesome" } + let(:from_project_id) { nil } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { 'non-existent-target-ref' } it 'sets empty diff and commit ivars' do show_request @@ -101,8 +149,9 @@ RSpec.describe Projects::CompareController do end context 'when the target ref is invalid' do - let(:target_ref) { "master%' AND 2554=4423 AND '%'='" } - let(:source_ref) { "improve%2Fawesome" } + let(:from_project_id) { nil } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { "master%' AND 2554=4423 AND '%'='" } it 'shows a flash message and redirects' do show_request @@ -113,8 +162,9 @@ RSpec.describe Projects::CompareController do end context 'when the source ref is invalid' do - let(:source_ref) { "master%' AND 2554=4423 AND '%'='" } - let(:target_ref) { "improve%2Fawesome" } + let(:from_project_id) { nil } + let(:from_ref) { "master%' AND 2554=4423 AND '%'='" } + let(:to_ref) { 'improve%2Fawesome' } it 'shows a flash message and redirects' do show_request @@ -126,24 +176,33 @@ RSpec.describe Projects::CompareController do end describe 'GET diff_for_path' do - def diff_for_path(extra_params = {}) - params = { + subject(:diff_for_path_request) { get :diff_for_path, params: request_params } + + let(:request_params) do + { + from_project_id: from_project_id, + from: from_ref, + to: to_ref, namespace_id: project.namespace, - project_id: project + project_id: project, + old_path: old_path, + new_path: new_path } - - get :diff_for_path, params: params.merge(extra_params) end let(:existing_path) { 'files/ruby/feature.rb' } - let(:source_ref) { "improve%2Fawesome" } - let(:target_ref) { "feature" } - context 'when the source and target refs exist' do + let(:from_project_id) { nil } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { 'feature' } + let(:old_path) { existing_path } + let(:new_path) { existing_path } + + context 'when the source and target refs exist in the same project' do context 'when the user has access target the project' do context 'when the path exists in the diff' do it 'disables diff notes' do - diff_for_path(from: source_ref, to: target_ref, old_path: existing_path, new_path: existing_path) + diff_for_path_request expect(assigns(:diff_notes_disabled)).to be_truthy end @@ -154,16 +213,17 @@ RSpec.describe Projects::CompareController do meth.call(diffs) end - diff_for_path(from: source_ref, to: target_ref, old_path: existing_path, new_path: existing_path) + diff_for_path_request end end context 'when the path does not exist in the diff' do - before do - diff_for_path(from: source_ref, to: target_ref, old_path: existing_path.succ, new_path: existing_path.succ) - end + let(:old_path) { existing_path.succ } + let(:new_path) { existing_path.succ } it 'returns a 404' do + diff_for_path_request + expect(response).to have_gitlab_http_status(:not_found) end end @@ -172,31 +232,56 @@ RSpec.describe Projects::CompareController do context 'when the user does not have access target the project' do before do project.team.truncate - diff_for_path(from: source_ref, to: target_ref, old_path: existing_path, new_path: existing_path) end it 'returns a 404' do + diff_for_path_request + expect(response).to have_gitlab_http_status(:not_found) end end end - context 'when the source ref does not exist' do - before do - diff_for_path(from: source_ref.succ, to: target_ref, old_path: existing_path, new_path: existing_path) + context 'when the source and target refs exist in different projects and the user can see' do + let(:from_project_id) { public_fork.id } + let(:from_ref) { 'improve%2Fmore-awesome' } + + it 'shows the diff for that path' do + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs| + expect(diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) + meth.call(diffs) + end + + diff_for_path_request + end + end + + context 'when the source and target refs exist in different projects and the user cannot see' do + let(:from_project_id) { private_fork.id } + + it 'does not show the diff for that path' do + diff_for_path_request + + expect(response).to have_gitlab_http_status(:not_found) end + end + + context 'when the source ref does not exist' do + let(:from_ref) { 'this-ref-does-not-exist' } it 'returns a 404' do + diff_for_path_request + expect(response).to have_gitlab_http_status(:not_found) end end context 'when the target ref does not exist' do - before do - diff_for_path(from: source_ref, to: target_ref.succ, old_path: existing_path, new_path: existing_path) - end + let(:to_ref) { 'this-ref-does-not-exist' } it 'returns a 404' do + diff_for_path_request + expect(response).to have_gitlab_http_status(:not_found) end end @@ -209,53 +294,54 @@ RSpec.describe Projects::CompareController do { namespace_id: project.namespace, project_id: project, - from: source_ref, - to: target_ref + from_project_id: from_project_id, + from: from_ref, + to: to_ref } end context 'when sending valid params' do - let(:source_ref) { "improve%2Fawesome" } - let(:target_ref) { "feature" } + let(:from_ref) { 'awesome%2Ffeature' } + let(:to_ref) { 'feature' } - it 'redirects back to show' do - create_request - - expect(response).to redirect_to(project_compare_path(project, to: target_ref, from: source_ref)) - end - end + context 'without a from_project_id' do + let(:from_project_id) { nil } - context 'when sending invalid params' do - context 'when the source ref is empty and target ref is set' do - let(:source_ref) { '' } - let(:target_ref) { 'master' } - - it 'redirects back to index and preserves the target ref' do + it 'redirects to the show page' do create_request - expect(response).to redirect_to(project_compare_index_path(project, to: target_ref)) + expect(response).to redirect_to(project_compare_path(project, from: from_ref, to: to_ref)) end end - context 'when the target ref is empty and source ref is set' do - let(:source_ref) { 'master' } - let(:target_ref) { '' } + context 'with a from_project_id' do + let(:from_project_id) { 'something or another' } - it 'redirects back to index and preserves source ref' do + it 'redirects to the show page without interpreting from_project_id' do create_request - expect(response).to redirect_to(project_compare_index_path(project, from: source_ref)) + expect(response).to redirect_to(project_compare_path(project, from: from_ref, to: to_ref, from_project_id: from_project_id)) end end + end + + context 'when sending invalid params' do + where(:from_ref, :to_ref, :from_project_id, :expected_redirect_params) do + '' | '' | '' | {} + 'main' | '' | '' | { from: 'main' } + '' | 'main' | '' | { to: 'main' } + '' | '' | '1' | { from_project_id: 1 } + 'main' | '' | '1' | { from: 'main', from_project_id: 1 } + '' | 'main' | '1' | { to: 'main', from_project_id: 1 } + end - context 'when the target and source ref are empty' do - let(:source_ref) { '' } - let(:target_ref) { '' } + with_them do + let(:expected_redirect) { project_compare_index_path(project, expected_redirect_params) } - it 'redirects back to index' do + it 'redirects back to the index' do create_request - expect(response).to redirect_to(namespace_project_compare_index_path) + expect(response).to redirect_to(expected_redirect) end end end @@ -268,15 +354,15 @@ RSpec.describe Projects::CompareController do { namespace_id: project.namespace, project_id: project, - from: source_ref, - to: target_ref, + from: from_ref, + to: to_ref, format: :json } end context 'when the source and target refs exist' do - let(:source_ref) { "improve%2Fawesome" } - let(:target_ref) { "feature" } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { 'feature' } context 'when the user has access to the project' do render_views @@ -285,14 +371,14 @@ RSpec.describe Projects::CompareController do let(:non_signature_commit) { build(:commit, project: project, safe_message: "message", sha: 'non_signature_commit') } before do - escaped_source_ref = Addressable::URI.unescape(source_ref) - escaped_target_ref = Addressable::URI.unescape(target_ref) + escaped_from_ref = Addressable::URI.unescape(from_ref) + escaped_to_ref = Addressable::URI.unescape(to_ref) - compare_service = CompareService.new(project, escaped_target_ref) - compare = compare_service.execute(project, escaped_source_ref) + compare_service = CompareService.new(project, escaped_to_ref) + compare = compare_service.execute(project, escaped_from_ref) - expect(CompareService).to receive(:new).with(project, escaped_target_ref).and_return(compare_service) - expect(compare_service).to receive(:execute).with(project, escaped_source_ref).and_return(compare) + expect(CompareService).to receive(:new).with(project, escaped_to_ref).and_return(compare_service) + expect(compare_service).to receive(:execute).with(project, escaped_from_ref).and_return(compare) expect(compare).to receive(:commits).and_return([signature_commit, non_signature_commit]) expect(non_signature_commit).to receive(:has_signature?).and_return(false) @@ -313,6 +399,7 @@ RSpec.describe Projects::CompareController do context 'when the user does not have access to the project' do before do project.team.truncate + project.update!(visibility: 'private') end it 'returns a 404' do @@ -324,8 +411,8 @@ RSpec.describe Projects::CompareController do end context 'when the source ref does not exist' do - let(:source_ref) { 'non-existent-ref-source' } - let(:target_ref) { "feature" } + let(:from_ref) { 'non-existent-ref-source' } + let(:to_ref) { 'feature' } it 'returns no signatures' do signatures_request @@ -336,8 +423,8 @@ RSpec.describe Projects::CompareController do end context 'when the target ref does not exist' do - let(:target_ref) { 'non-existent-ref-target' } - let(:source_ref) { "improve%2Fawesome" } + let(:from_ref) { 'improve%2Fawesome' } + let(:to_ref) { 'non-existent-ref-target' } it 'returns no signatures' do signatures_request |