summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/helpers/submodule_helper.rb12
-rw-r--r--changelogs/unreleased/dm-submodule-helper-routing.yml5
-rw-r--r--spec/helpers/submodule_helper_spec.rb81
3 files changed, 58 insertions, 40 deletions
diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb
index 35e04b0ced3..9a281065b90 100644
--- a/app/helpers/submodule_helper.rb
+++ b/app/helpers/submodule_helper.rb
@@ -34,8 +34,8 @@ module SubmoduleHelper
project.sub!(/\.git\z/, '')
if self_url?(url, namespace, project)
- [namespace_project_path(namespace, project),
- namespace_project_tree_path(namespace, project, submodule_item_id)]
+ [url_helpers.namespace_project_path(namespace, project),
+ url_helpers.namespace_project_tree_path(namespace, project, submodule_item_id)]
elsif relative_self_url?(url)
relative_self_links(url, submodule_item_id, repository.project)
elsif github_dot_com_url?(url)
@@ -99,8 +99,8 @@ module SubmoduleHelper
begin
[
- namespace_project_path(target_namespace_path, submodule_base),
- namespace_project_tree_path(target_namespace_path, submodule_base, commit)
+ url_helpers.namespace_project_path(target_namespace_path, submodule_base),
+ url_helpers.namespace_project_tree_path(target_namespace_path, submodule_base, commit)
]
rescue ActionController::UrlGenerationError
[nil, nil]
@@ -118,4 +118,8 @@ module SubmoduleHelper
rescue URI::InvalidURIError
nil
end
+
+ def url_helpers
+ Gitlab::Routing.url_helpers
+ end
end
diff --git a/changelogs/unreleased/dm-submodule-helper-routing.yml b/changelogs/unreleased/dm-submodule-helper-routing.yml
new file mode 100644
index 00000000000..779d4d167df
--- /dev/null
+++ b/changelogs/unreleased/dm-submodule-helper-routing.yml
@@ -0,0 +1,5 @@
+---
+title: Fix bug that caused diffs not to show on MRs with changes to submodules
+merge_request:
+author:
+type: fixed
diff --git a/spec/helpers/submodule_helper_spec.rb b/spec/helpers/submodule_helper_spec.rb
index ea48c69e0ae..b922b910c89 100644
--- a/spec/helpers/submodule_helper_spec.rb
+++ b/spec/helpers/submodule_helper_spec.rb
@@ -3,15 +3,11 @@ require 'spec_helper'
describe SubmoduleHelper do
include RepoHelpers
- describe 'submodule links' do
- let(:submodule_item) { double(id: 'hash', path: 'rack') }
- let(:config) { Gitlab.config.gitlab }
- let(:repo) { double }
-
- before do
- self.instance_variable_set(:@repository, repo)
- end
+ let(:submodule_item) { double(id: 'hash', path: 'rack') }
+ let(:config) { Gitlab.config.gitlab }
+ let(:repo) { double }
+ shared_examples 'submodule_links' do
context 'submodule on self' do
before do
allow(Gitlab.config.gitlab).to receive(:protocol).and_return('http') # set this just to be sure
@@ -21,28 +17,28 @@ describe SubmoduleHelper do
allow(Gitlab.config.gitlab_shell).to receive(:ssh_port).and_return(22) # set this just to be sure
allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix))
stub_url([config.user, '@', config.host, ':gitlab-org/gitlab-ce.git'].join(''))
- expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
+ expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
end
it 'detects ssh on non-standard port' do
allow(Gitlab.config.gitlab_shell).to receive(:ssh_port).and_return(2222)
allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix))
stub_url(['ssh://', config.user, '@', config.host, ':2222/gitlab-org/gitlab-ce.git'].join(''))
- expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
+ expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
end
it 'detects http on standard port' do
allow(Gitlab.config.gitlab).to receive(:port).and_return(80)
allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
stub_url(['http://', config.host, '/gitlab-org/gitlab-ce.git'].join(''))
- expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
+ expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
end
it 'detects http on non-standard port' do
allow(Gitlab.config.gitlab).to receive(:port).and_return(3000)
allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
stub_url(['http://', config.host, ':3000/gitlab-org/gitlab-ce.git'].join(''))
- expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
+ expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
end
it 'works with relative_url_root' do
@@ -50,7 +46,7 @@ describe SubmoduleHelper do
allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root')
allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
stub_url(['http://', config.host, '/gitlab/root/gitlab-org/gitlab-ce.git'].join(''))
- expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
+ expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash')])
end
it 'works with subgroups' do
@@ -58,34 +54,34 @@ describe SubmoduleHelper do
allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root')
allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
stub_url(['http://', config.host, '/gitlab/root/gitlab-org/sub/gitlab-ce.git'].join(''))
- expect(submodule_links(submodule_item)).to eq([namespace_project_path('gitlab-org/sub', 'gitlab-ce'), namespace_project_tree_path('gitlab-org/sub', 'gitlab-ce', 'hash')])
+ expect(subject).to eq([namespace_project_path('gitlab-org/sub', 'gitlab-ce'), namespace_project_tree_path('gitlab-org/sub', 'gitlab-ce', 'hash')])
end
end
context 'submodule on github.com' do
it 'detects ssh' do
stub_url('git@github.com:gitlab-org/gitlab-ce.git')
- expect(submodule_links(submodule_item)).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
+ expect(subject).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
end
it 'detects http' do
stub_url('http://github.com/gitlab-org/gitlab-ce.git')
- expect(submodule_links(submodule_item)).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
+ expect(subject).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
end
it 'detects https' do
stub_url('https://github.com/gitlab-org/gitlab-ce.git')
- expect(submodule_links(submodule_item)).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
+ expect(subject).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
end
it 'handles urls with no .git on the end' do
stub_url('http://github.com/gitlab-org/gitlab-ce')
- expect(submodule_links(submodule_item)).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
+ expect(subject).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash'])
end
it 'returns original with non-standard url' do
stub_url('http://github.com/another/gitlab-org/gitlab-ce.git')
- expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil])
+ expect(subject).to eq([repo.submodule_url_for, nil])
end
end
@@ -97,39 +93,39 @@ describe SubmoduleHelper do
allow(repo).to receive(:project).and_return(project)
stub_url('./')
- expect(submodule_links(submodule_item)).to eq(["/master-project/#{project.path}", "/master-project/#{project.path}/tree/hash"])
+ expect(subject).to eq(["/master-project/#{project.path}", "/master-project/#{project.path}/tree/hash"])
end
end
context 'submodule on gitlab.com' do
it 'detects ssh' do
stub_url('git@gitlab.com:gitlab-org/gitlab-ce.git')
- expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
+ expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
end
it 'detects http' do
stub_url('http://gitlab.com/gitlab-org/gitlab-ce.git')
- expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
+ expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
end
it 'detects https' do
stub_url('https://gitlab.com/gitlab-org/gitlab-ce.git')
- expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
+ expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
end
it 'handles urls with no .git on the end' do
stub_url('http://gitlab.com/gitlab-org/gitlab-ce')
- expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
+ expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
end
it 'handles urls with trailing whitespace' do
stub_url('http://gitlab.com/gitlab-org/gitlab-ce.git ')
- expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
+ expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash'])
end
it 'returns original with non-standard url' do
stub_url('http://gitlab.com/another/gitlab-org/gitlab-ce.git')
- expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil])
+ expect(subject).to eq([repo.submodule_url_for, nil])
end
end
@@ -137,27 +133,25 @@ describe SubmoduleHelper do
it 'sanitizes unsupported protocols' do
stub_url('javascript:alert("XSS");')
- expect(helper.submodule_links(submodule_item)).to eq([nil, nil])
+ expect(subject).to eq([nil, nil])
end
it 'sanitizes unsupported protocols disguised as a repository URL' do
stub_url('javascript:alert("XSS");foo/bar.git')
- expect(helper.submodule_links(submodule_item)).to eq([nil, nil])
+ expect(subject).to eq([nil, nil])
end
it 'sanitizes invalid URL with extended ASCII' do
stub_url('é')
- expect(helper.submodule_links(submodule_item)).to eq([nil, nil])
+ expect(subject).to eq([nil, nil])
end
it 'returns original' do
stub_url('http://mygitserver.com/gitlab-org/gitlab-ce')
- expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil])
- stub_url('http://mygitserver.com/gitlab-org/gitlab-ce.git')
- expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil])
+ expect(subject).to eq([repo.submodule_url_for, nil])
end
end
@@ -168,8 +162,7 @@ describe SubmoduleHelper do
def expect_relative_link_to_resolve_to(relative_path, expected_path)
allow(repo).to receive(:submodule_url_for).and_return(relative_path)
-
- result = submodule_links(submodule_item)
+ result = subject
expect(result).to eq([expected_path, "#{expected_path}/tree/#{submodule_item.id}"])
end
@@ -190,7 +183,7 @@ describe SubmoduleHelper do
it 'returns nil' do
allow(repo).to receive(:submodule_url_for).and_return('../../test.git')
- result = submodule_links(submodule_item)
+ result = subject
expect(result).to eq([nil, nil])
end
@@ -200,7 +193,7 @@ describe SubmoduleHelper do
it 'returns nil because it is not possible to have repo nested under another repo' do
allow(repo).to receive(:submodule_url_for).and_return('./test.git')
- result = submodule_links(submodule_item)
+ result = subject
expect(result).to eq([nil, nil])
end
@@ -238,6 +231,22 @@ describe SubmoduleHelper do
end
end
+ context 'as view helpers in view context' do
+ subject { helper.submodule_links(submodule_item) }
+
+ before do
+ self.instance_variable_set(:@repository, repo)
+ end
+
+ it_behaves_like 'submodule_links'
+ end
+
+ context 'as stand-alone module' do
+ subject { described_class.submodule_links(submodule_item, nil, repo) }
+
+ it_behaves_like 'submodule_links'
+ end
+
def stub_url(url)
allow(repo).to receive(:submodule_url_for).and_return(url)
end
View Lorry Controller Status