diff options
-rw-r--r-- | app/helpers/submodule_helper.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/dm-submodule-links-nil.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/submodule_links.rb | 11 | ||||
-rw-r--r-- | spec/helpers/submodule_helper_spec.rb | 13 | ||||
-rw-r--r-- | spec/lib/gitlab/submodule_links_spec.rb | 47 |
5 files changed, 75 insertions, 3 deletions
diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb index 9a281065b90..e683e2959d1 100644 --- a/app/helpers/submodule_helper.rb +++ b/app/helpers/submodule_helper.rb @@ -13,6 +13,8 @@ module SubmoduleHelper end def submodule_links_for_url(submodule_item_id, url, repository) + return [nil, nil] unless url + if url == '.' || url == './' url = File.join(Gitlab.config.gitlab.url, repository.project.full_path) end diff --git a/changelogs/unreleased/dm-submodule-links-nil.yml b/changelogs/unreleased/dm-submodule-links-nil.yml new file mode 100644 index 00000000000..c09ca41d01d --- /dev/null +++ b/changelogs/unreleased/dm-submodule-links-nil.yml @@ -0,0 +1,5 @@ +--- +title: Fix error rendering submodules in MR diffs when there is no .gitmodules +merge_request: 31162 +author: +type: fixed diff --git a/lib/gitlab/submodule_links.rb b/lib/gitlab/submodule_links.rb index a6c0369d864..18fd604a3b0 100644 --- a/lib/gitlab/submodule_links.rb +++ b/lib/gitlab/submodule_links.rb @@ -9,7 +9,7 @@ module Gitlab end def for(submodule, sha) - submodule_url = submodule_url_for(sha)[submodule.path] + submodule_url = submodule_url_for(sha, submodule.path) SubmoduleHelper.submodule_links_for_url(submodule.id, submodule_url, repository) end @@ -17,10 +17,15 @@ module Gitlab attr_reader :repository - def submodule_url_for(sha) - strong_memoize(:"submodule_links_for_#{sha}") do + def submodule_urls_for(sha) + strong_memoize(:"submodule_urls_for_#{sha}") do repository.submodule_urls_for(sha) end end + + def submodule_url_for(sha, path) + urls = submodule_urls_for(sha) + urls && urls[path] + end end end diff --git a/spec/helpers/submodule_helper_spec.rb b/spec/helpers/submodule_helper_spec.rb index b922b910c89..ab4ef899119 100644 --- a/spec/helpers/submodule_helper_spec.rb +++ b/spec/helpers/submodule_helper_spec.rb @@ -229,6 +229,19 @@ describe SubmoduleHelper do end end end + + context 'unknown submodule' do + before do + # When there is no `.gitmodules` file, or if `.gitmodules` does not + # know the submodule at the specified path, + # `Repository#submodule_url_for` returns `nil` + stub_url(nil) + end + + it 'returns no links' do + expect(subject).to eq([nil, nil]) + end + end end context 'as view helpers in view context' do diff --git a/spec/lib/gitlab/submodule_links_spec.rb b/spec/lib/gitlab/submodule_links_spec.rb new file mode 100644 index 00000000000..a84602cd07d --- /dev/null +++ b/spec/lib/gitlab/submodule_links_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::SubmoduleLinks do + let(:submodule_item) { double(id: 'hash', path: 'gitlab-ce') } + let(:repo) { double } + let(:links) { described_class.new(repo) } + + describe '#for' do + subject { links.for(submodule_item, 'ref') } + + context 'when there is no .gitmodules file' do + before do + stub_urls(nil) + end + + it 'returns no links' do + expect(subject).to eq([nil, nil]) + end + end + + context 'when the submodule is unknown' do + before do + stub_urls({ 'path' => 'url' }) + end + + it 'returns no links' do + expect(subject).to eq([nil, nil]) + end + end + + context 'when the submodule is known' do + before do + stub_urls({ 'gitlab-ce' => 'git@gitlab.com:gitlab-org/gitlab-ce.git' }) + end + + it 'returns links' do + expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash']) + end + end + end + + def stub_urls(urls) + allow(repo).to receive(:submodule_urls_for).and_return(urls) + end +end |