Commit de8497ca authored by Stan Hu's avatar Stan Hu

Fix Error 500 when comparing non-existing branches

Closes #2593
parent ac571623
...@@ -16,10 +16,12 @@ class Projects::CompareController < Projects::ApplicationController ...@@ -16,10 +16,12 @@ class Projects::CompareController < Projects::ApplicationController
compare_result = CompareService.new. compare_result = CompareService.new.
execute(@project, head_ref, @project, base_ref) execute(@project, head_ref, @project, base_ref)
@commits = compare_result.commits if compare_result
@diffs = compare_result.diffs @commits = compare_result.commits
@commit = @commits.last @diffs = compare_result.diffs
@line_notes = [] @commit = @commits.last
@line_notes = []
end
end end
def create def create
......
...@@ -4,7 +4,10 @@ require 'securerandom' ...@@ -4,7 +4,10 @@ require 'securerandom'
# and return Gitlab::CompareResult object that responds to commits and diffs # and return Gitlab::CompareResult object that responds to commits and diffs
class CompareService class CompareService
def execute(source_project, source_branch, target_project, target_branch) 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 # If compare with other project we need to fetch ref first
unless target_project == source_project unless target_project == source_project
......
...@@ -22,4 +22,30 @@ describe Projects::CompareController do ...@@ -22,4 +22,30 @@ describe Projects::CompareController do
expect(assigns(:diffs).length).to be >= 1 expect(assigns(:diffs).length).to be >= 1
expect(assigns(:commits).length).to be >= 1 expect(assigns(:commits).length).to be >= 1
end 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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment