Merge branch 'dm-submodule-links-nil' into 'master'

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

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

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

changelogs/unreleased/dm-submodule-links-nil.yml

+---
+title: Fix error rendering submodules in MR diffs when there is no .gitmodules
+merge_request: 31162
+author:
+type: fixed

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

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

spec/lib/gitlab/submodule_links_spec.rb

+# 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