diff options
| -rw-r--r-- | app/controllers/projects/compare_controller.rb | 10 | ||||
| -rw-r--r-- | app/services/compare_service.rb | 5 | ||||
| -rw-r--r-- | spec/controllers/projects/compare_controller_spec.rb | 26 |
3 files changed, 36 insertions, 5 deletions
diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index d9b3adae95b..d15004f93a6 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -16,10 +16,12 @@ class Projects::CompareController < Projects::ApplicationController compare_result = CompareService.new. execute(@project, head_ref, @project, base_ref) - @commits = compare_result.commits - @diffs = compare_result.diffs - @commit = @commits.last - @line_notes = [] + if compare_result + @commits = compare_result.commits + @diffs = compare_result.diffs + @commit = @commits.last + @line_notes = [] + end end def create diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb index 70f642baaaa..bfe6a3dc4be 100644 --- a/app/services/compare_service.rb +++ b/app/services/compare_service.rb @@ -4,7 +4,10 @@ require 'securerandom' # and return Gitlab::CompareResult object that responds to commits and diffs class CompareService def execute(source_project, source_branch, target_project, target_branch) - source_sha = source_project.commit(source_branch).sha + source_commit = source_project.commit(source_branch) + return unless source_commit + + source_sha = source_commit.sha # If compare with other project we need to fetch ref first unless target_project == source_project diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index b643b354073..2a447248b70 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -22,4 +22,30 @@ describe Projects::CompareController do expect(assigns(:diffs).length).to be >= 1 expect(assigns(:commits).length).to be >= 1 end + + describe 'non-existent refs' do + it 'invalid source ref' do + get(:show, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + from: 'non-existent', + to: ref_to) + + expect(response).to be_success + expect(assigns(:diffs)).to eq([]) + expect(assigns(:commits)).to eq([]) + end + + it 'invalid target ref' do + get(:show, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + from: ref_from, + to: 'non-existent') + + expect(response).to be_success + expect(assigns(:diffs)).to eq(nil) + expect(assigns(:commits)).to eq(nil) + end + end end |