Commit cfef1e8e authored by Douwe Maan's avatar Douwe Maan Committed by Nick Thomas

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

Without this change, we get a NoMethodError on nil
parent 57aabe16
...@@ -13,6 +13,8 @@ module SubmoduleHelper ...@@ -13,6 +13,8 @@ module SubmoduleHelper
end end
def submodule_links_for_url(submodule_item_id, url, repository) def submodule_links_for_url(submodule_item_id, url, repository)
return [nil, nil] unless url
if url == '.' || url == './' if url == '.' || url == './'
url = File.join(Gitlab.config.gitlab.url, repository.project.full_path) url = File.join(Gitlab.config.gitlab.url, repository.project.full_path)
end end
......
---
title: Fix error rendering submodules in MR diffs when there is no .gitmodules
merge_request: 31162
author:
type: fixed
...@@ -9,7 +9,7 @@ module Gitlab ...@@ -9,7 +9,7 @@ module Gitlab
end end
def for(submodule, sha) 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) SubmoduleHelper.submodule_links_for_url(submodule.id, submodule_url, repository)
end end
...@@ -17,10 +17,15 @@ module Gitlab ...@@ -17,10 +17,15 @@ module Gitlab
attr_reader :repository attr_reader :repository
def submodule_url_for(sha) def submodule_urls_for(sha)
strong_memoize(:"submodule_links_for_#{sha}") do strong_memoize(:"submodule_urls_for_#{sha}") do
repository.submodule_urls_for(sha) repository.submodule_urls_for(sha)
end end
end end
def submodule_url_for(sha, path)
urls = submodule_urls_for(sha)
urls && urls[path]
end
end end
end end
...@@ -229,6 +229,19 @@ describe SubmoduleHelper do ...@@ -229,6 +229,19 @@ describe SubmoduleHelper do
end end
end 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 end
context 'as view helpers in view context' do context 'as view helpers in view context' do
......
# 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
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