Skip to content

G
gitlab-ce
Commit 26ac53b3 authored by Douwe Maan's avatar Douwe Maan
Browse files
Options

Ensure SubmoduleHelper works outside view context

parent 65b16f98
Hide whitespace changes
Inline Side-by-side
Showing with 58 additions and 40 deletions
app/helpers/submodule_helper.rb
View file @ 26ac53b3
......@@ -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
changelogs/unreleased/dm-submodule-helper-routing.yml 0 → 100644
View file @ 26ac53b3
---
title: Fix bug that caused diffs not to show on MRs with changes to submodules
merge_request:
author:
type: fixed
spec/helpers/submodule_helper_spec.rb
View file @ 26ac53b3
......@@ -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
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7