Merge branch 'dm-submodule-links-nil' into 'master'12-1-auto-deploy-20190721

Fix error rendering submodules in MR diffs when there is no .gitmodules

See merge request gitlab-org/gitlab-ce!31162

(cherry picked from commit 55f99e930e1c147ec191a234ff4881ea7e70ea61)

cfef1e8e Fix error rendering submodules in MR diffs when there is no .gitmodules

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