Skip to content

G
gitlab-ce
Commit 3e7d5910 authored by Phaiax's avatar Phaiax Committed by Phil Hughes
Browse files
Options

Add link to compare changes intoduced by a git submodule update

parent 2b9e609f
Show whitespace changes
Inline Side-by-side
Showing with 383 additions and 91 deletions
app/assets/javascripts/diffs/components/diff_file_header.vue
View file @ 3e7d5910
......@@ -2,7 +2,14 @@
/* eslint-disable vue/no-v-html */
import { escape } from 'lodash';
import { mapActions, mapGetters } from 'vuex';
import { GlDeprecatedButton, GlTooltipDirective, GlLoadingIcon, GlIcon } from '@gitlab/ui';
import {
GlDeprecatedButton,
GlTooltipDirective,
GlSafeHtmlDirective,
GlLoadingIcon,
GlIcon,
GlButton,
} from '@gitlab/ui';
import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
import FileIcon from '~/vue_shared/components/file_icon.vue';
import { truncateSha } from '~/lib/utils/text_utility';
......@@ -21,9 +28,11 @@ export default {
GlIcon,
FileIcon,
DiffStats,
GlButton,
},
directives: {
GlTooltip: GlTooltipDirective,
SafeHtml: GlSafeHtmlDirective,
},
props: {
discussionPath: {
......@@ -77,6 +86,21 @@ export default {
return this.discussionPath;
},
submoduleDiffCompareLinkText() {
if (this.diffFile.submodule_compare) {
const truncatedOldSha = escape(truncateSha(this.diffFile.submodule_compare.old_sha));
const truncatedNewSha = escape(truncateSha(this.diffFile.submodule_compare.new_sha));
return sprintf(
s__('Compare %{oldCommitId}...%{newCommitId}'),
{
oldCommitId: `<span class="commit-sha">${truncatedOldSha}</span>`,
newCommitId: `<span class="commit-sha">${truncatedNewSha}</span>`,
},
false,
);
}
return null;
},
filePath() {
if (this.diffFile.submodule) {
return `${this.diffFile.file_path} @ ${truncateSha(this.diffFile.blob.id)}`;
......@@ -307,5 +331,18 @@ export default {
</a>
</div>
</div>
<div
v-if="diffFile.submodule_compare"
class="file-actions d-none d-sm-flex align-items-center flex-wrap"
>
<gl-button
v-gl-tooltip.hover
v-safe-html="submoduleDiffCompareLinkText"
class="submodule-compare"
:title="s__('Compare submodule commit revisions')"
:href="diffFile.submodule_compare.url"
/>
</div>
</div>
</template>
app/helpers/diff_helper.rb
View file @ 3e7d5910
......@@ -100,20 +100,43 @@ module DiffHelper
end
def submodule_link(blob, ref, repository = @repository)
project_url, tree_url = submodule_links(blob, ref, repository)
commit_id = if tree_url.nil?
Commit.truncate_sha(blob.id)
else
link_to Commit.truncate_sha(blob.id), tree_url
end
urls = submodule_links(blob, ref, repository)
folder_name = truncate(blob.name, length: 40)
folder_name = link_to(folder_name, urls.web) if urls&.web
commit_id = Commit.truncate_sha(blob.id)
commit_id = link_to(commit_id, urls.tree) if urls&.tree
[
content_tag(:span, link_to(truncate(blob.name, length: 40), project_url)),
content_tag(:span, folder_name),
'@',
content_tag(:span, commit_id, class: 'commit-sha')
].join(' ').html_safe
end
def submodule_diff_compare_link(diff_file)
compare_url = submodule_links(diff_file.blob, diff_file.content_sha, diff_file.repository, diff_file)&.compare
link = ""
if compare_url
link_text = [
_('Compare'),
' ',
content_tag(:span, Commit.truncate_sha(diff_file.old_blob.id), class: 'commit-sha'),
'...',
content_tag(:span, Commit.truncate_sha(diff_file.blob.id), class: 'commit-sha')
].join('').html_safe
tooltip = _('Compare submodule commit revisions')
link = content_tag(:span, link_to(link_text, compare_url, class: 'btn has-tooltip', title: tooltip), class: 'submodule-compare')
end
link
end
def diff_file_blob_raw_url(diff_file, only_path: false)
project_raw_url(@project, tree_join(diff_file.content_sha, diff_file.file_path), only_path: only_path)
end
......
app/helpers/submodule_helper.rb
View file @ 3e7d5910
......@@ -6,12 +6,12 @@ module SubmoduleHelper
VALID_SUBMODULE_PROTOCOLS = %w[http https git ssh].freeze
# links to files listing for submodule if submodule is a project on this server
def submodule_links(submodule_item, ref = nil, repository = @repository)
repository.submodule_links.for(submodule_item, ref)
def submodule_links(submodule_item, ref = nil, repository = @repository, diff_file = nil)
repository.submodule_links.for(submodule_item, ref, diff_file)
end
def submodule_links_for_url(submodule_item_id, url, repository)
return [nil, nil] unless url
def submodule_links_for_url(submodule_item_id, url, repository, old_submodule_item_id = nil)
return [nil, nil, nil] unless url
if url == '.' || url == './'
url = File.join(Gitlab.config.gitlab.url, repository.project.full_path)
......@@ -34,21 +34,24 @@ module SubmoduleHelper
project.sub!(/\.git\z/, '')
if self_url?(url, namespace, project)
[url_helpers.namespace_project_path(namespace, project),
url_helpers.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),
(url_helpers.namespace_project_compare_path(namespace, project, to: submodule_item_id, from: old_submodule_item_id) if old_submodule_item_id)
]
elsif relative_self_url?(url)
relative_self_links(url, submodule_item_id, repository.project)
relative_self_links(url, submodule_item_id, old_submodule_item_id, repository.project)
elsif gist_github_dot_com_url?(url)
gist_github_com_tree_links(namespace, project, submodule_item_id)
elsif github_dot_com_url?(url)
github_com_tree_links(namespace, project, submodule_item_id)
github_com_tree_links(namespace, project, submodule_item_id, old_submodule_item_id)
elsif gitlab_dot_com_url?(url)
gitlab_com_tree_links(namespace, project, submodule_item_id)
gitlab_com_tree_links(namespace, project, submodule_item_id, old_submodule_item_id)
else
[sanitize_submodule_url(url), nil]
[sanitize_submodule_url(url), nil, nil]
end
else
[sanitize_submodule_url(url), nil]
[sanitize_submodule_url(url), nil, nil]
end
end
......@@ -79,22 +82,30 @@ module SubmoduleHelper
url.start_with?('../', './')
end
def gitlab_com_tree_links(namespace, project, commit)
def gitlab_com_tree_links(namespace, project, commit, old_commit)
base = ['https://gitlab.com/', namespace, '/', project].join('')
[base, [base, '/-/tree/', commit].join('')]
[
base,
[base, '/-/tree/', commit].join(''),
([base, '/-/compare/', old_commit, '...', commit].join('') if old_commit)
]
end
def gist_github_com_tree_links(namespace, project, commit)
base = ['https://gist.github.com/', namespace, '/', project].join('')
[base, [base, commit].join('/')]
[base, [base, commit].join('/'), nil]
end
def github_com_tree_links(namespace, project, commit)
def github_com_tree_links(namespace, project, commit, old_commit)
base = ['https://github.com/', namespace, '/', project].join('')
[base, [base, '/tree/', commit].join('')]
[
base,
[base, '/tree/', commit].join(''),
([base, '/compare/', old_commit, '...', commit].join('') if old_commit)
]
end
def relative_self_links(relative_path, commit, project)
def relative_self_links(relative_path, commit, old_commit, project)
relative_path = relative_path.rstrip
absolute_project_path = "/" + project.full_path
......@@ -107,7 +118,7 @@ module SubmoduleHelper
target_namespace_path = File.dirname(submodule_project_path)
if target_namespace_path == '/' || target_namespace_path.start_with?(absolute_project_path)
return [nil, nil]
return [nil, nil, nil]
end
target_namespace_path.sub!(%r{^/}, '')
......@@ -116,10 +127,11 @@ module SubmoduleHelper
begin
[
url_helpers.namespace_project_path(target_namespace_path, submodule_base),
url_helpers.namespace_project_tree_path(target_namespace_path, submodule_base, commit)
url_helpers.namespace_project_tree_path(target_namespace_path, submodule_base, commit),
(url_helpers.namespace_project_compare_path(target_namespace_path, submodule_base, to: commit, from: old_commit) if old_commit)
]
rescue ActionController::UrlGenerationError
[nil, nil]
[nil, nil, nil]
end
end
......
app/serializers/diff_file_base_entity.rb
View file @ 3e7d5910
......@@ -12,11 +12,23 @@ class DiffFileBaseEntity < Grape::Entity
expose :submodule?, as: :submodule
expose :submodule_link do |diff_file, options|
memoized_submodule_links(diff_file, options).first
memoized_submodule_links(diff_file, options)&.web
end
expose :submodule_tree_url do |diff_file|
memoized_submodule_links(diff_file, options).last
memoized_submodule_links(diff_file, options)&.tree
end
expose :submodule_compare do |diff_file|
url = memoized_submodule_links(diff_file, options)&.compare
next unless url
{
url: url,
old_sha: diff_file.old_blob&.id,
new_sha: diff_file.blob&.id
}
end
expose :edit_path, if: -> (_, options) { options[:merge_request] } do |diff_file|
......@@ -96,11 +108,9 @@ class DiffFileBaseEntity < Grape::Entity
def memoized_submodule_links(diff_file, options)
strong_memoize(:submodule_links) do
if diff_file.submodule?
options[:submodule_links].for(diff_file.blob, diff_file.content_sha)
else
[]
end
next unless diff_file.submodule?
options[:submodule_links].for(diff_file.blob, diff_file.content_sha, diff_file)
end
end
......
app/views/projects/diffs/_file.html.haml
View file @ 3e7d5910
......@@ -9,6 +9,10 @@
.file-header-content
= render "projects/diffs/file_header", diff_file: diff_file, url: "##{file_hash}"
- if diff_file.submodule?
.file-actions.d-none.d-sm-block
= submodule_diff_compare_link(diff_file)
- unless diff_file.submodule?
- blob = diff_file.blob
.file-actions.d-none.d-sm-block
......
changelogs/unreleased/compare-link-git-submodule-update.yml 0 → 100644
View file @ 3e7d5910
---
title: Add link to compare changes intoduced by a git submodule update
merge_request: 37740
author: Daniel Seemer @Phaiax
type: added
lib/gitlab/graphql/representation/submodule_tree_entry.rb
View file @ 3e7d5910
......@@ -24,11 +24,11 @@ module Gitlab
end
def web_url
@submodule_links.first
@submodule_links&.web
end
def tree_url
@submodule_links.last
@submodule_links&.tree
end
end
end
......
lib/gitlab/submodule_links.rb
View file @ 3e7d5910
......@@ -4,14 +4,18 @@ module Gitlab
class SubmoduleLinks
include Gitlab::Utils::StrongMemoize
Urls = Struct.new(:web, :tree, :compare)
def initialize(repository)
@repository = repository
@cache_store = {}
end
def for(submodule, sha)
def for(submodule, sha, diff_file = nil)
submodule_url = submodule_url_for(sha, submodule.path)
SubmoduleHelper.submodule_links_for_url(submodule.id, submodule_url, repository)
old_submodule_id = old_submodule_id(submodule_url, diff_file)
urls = SubmoduleHelper.submodule_links_for_url(submodule.id, submodule_url, repository, old_submodule_id)
Urls.new(*urls) if urls.any?
end
private
......@@ -29,5 +33,15 @@ module Gitlab
urls = submodule_urls_for(sha)
urls && urls[path]
end
def old_submodule_id(submodule_url, diff_file)
return unless diff_file&.old_blob && diff_file&.old_content_sha
# if the submodule url has changed from old_sha to sha, a compare link does not make sense
#
old_submodule_url = submodule_url_for(diff_file.old_content_sha, diff_file.old_blob.path)
diff_file.old_blob.id if old_submodule_url == submodule_url
end
end
end
locale/gitlab.pot
View file @ 3e7d5910
......@@ -6325,6 +6325,9 @@ msgstr ""
msgid "Compare"
msgstr ""
msgid "Compare %{oldCommitId}...%{newCommitId}"
msgstr ""
msgid "Compare Git revisions"
msgstr ""
......@@ -6340,6 +6343,9 @@ msgstr ""
msgid "Compare changes with the merge request target branch"
msgstr ""
msgid "Compare submodule commit revisions"
msgstr ""
msgid "Compare with previous version"
msgstr ""
......
spec/helpers/submodule_helper_spec.rb
View file @ 3e7d5910
......@@ -24,7 +24,11 @@ RSpec.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.ssh_user, '@', config.host, ':gitlab-org/gitlab-foss.git'].join(''))
expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')])
aggregate_failures do
expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss'))
expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash'))
expect(subject.compare).to be_nil
end
end
it 'detects ssh on standard port without a username' do
......@@ -32,14 +36,22 @@ RSpec.describe SubmoduleHelper do
allow(Gitlab.config.gitlab_shell).to receive(:ssh_user).and_return('')
allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix))
stub_url([config.host, ':gitlab-org/gitlab-foss.git'].join(''))
expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')])
aggregate_failures do
expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss'))
expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash'))
expect(subject.compare).to be_nil
end
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.ssh_user, '@', config.host, ':2222/gitlab-org/gitlab-foss.git'].join(''))
expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')])
aggregate_failures do
expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss'))
expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash'))
expect(subject.compare).to be_nil
end
end
it 'detects ssh on non-standard port without a username' do
......@@ -47,21 +59,33 @@ RSpec.describe SubmoduleHelper do
allow(Gitlab.config.gitlab_shell).to receive(:ssh_user).and_return('')
allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix))
stub_url(['ssh://', config.host, ':2222/gitlab-org/gitlab-foss.git'].join(''))
expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')])
aggregate_failures do
expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss'))
expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash'))
expect(subject.compare).to be_nil
end
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-foss.git'].join(''))
expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')])
aggregate_failures do
expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss'))
expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash'))
expect(subject.compare).to be_nil
end
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-foss.git'].join(''))
expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')])
aggregate_failures do
expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss'))
expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash'))
expect(subject.compare).to be_nil
end
end
it 'works with relative_url_root' do
......@@ -69,7 +93,11 @@ RSpec.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-foss.git'].join(''))
expect(subject).to eq([namespace_project_path('gitlab-org', 'gitlab-foss'), namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash')])
aggregate_failures do
expect(subject.web).to eq(namespace_project_path('gitlab-org', 'gitlab-foss'))
expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org', 'gitlab-foss', 'hash'))
expect(subject.compare).to be_nil
end
end
it 'works with subgroups' do
......@@ -77,61 +105,105 @@ RSpec.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-foss.git'].join(''))
expect(subject).to eq([namespace_project_path('gitlab-org/sub', 'gitlab-foss'), namespace_project_tree_path('gitlab-org/sub', 'gitlab-foss', 'hash')])
aggregate_failures do
expect(subject.web).to eq(namespace_project_path('gitlab-org/sub', 'gitlab-foss'))
expect(subject.tree).to eq(namespace_project_tree_path('gitlab-org/sub', 'gitlab-foss', 'hash'))
expect(subject.compare).to be_nil
end
end
end
context 'submodule on gist.github.com' do
it 'detects ssh' do
stub_url('git@gist.github.com:gitlab-org/gitlab-foss.git')
is_expected.to eq(['https://gist.github.com/gitlab-org/gitlab-foss', 'https://gist.github.com/gitlab-org/gitlab-foss/hash'])
aggregate_failures do
expect(subject.web).to eq('https://gist.github.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gist.github.com/gitlab-org/gitlab-foss/hash')
expect(subject.compare).to be_nil
end
end
it 'detects http' do
stub_url('http://gist.github.com/gitlab-org/gitlab-foss.git')
is_expected.to eq(['https://gist.github.com/gitlab-org/gitlab-foss', 'https://gist.github.com/gitlab-org/gitlab-foss/hash'])
aggregate_failures do
expect(subject.web).to eq('https://gist.github.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gist.github.com/gitlab-org/gitlab-foss/hash')
expect(subject.compare).to be_nil
end
end
it 'detects https' do
stub_url('https://gist.github.com/gitlab-org/gitlab-foss.git')
is_expected.to eq(['https://gist.github.com/gitlab-org/gitlab-foss', 'https://gist.github.com/gitlab-org/gitlab-foss/hash'])
aggregate_failures do
expect(subject.web).to eq('https://gist.github.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gist.github.com/gitlab-org/gitlab-foss/hash')
expect(subject.compare).to be_nil
end
end
it 'handles urls with no .git on the end' do
stub_url('http://gist.github.com/gitlab-org/gitlab-foss')
is_expected.to eq(['https://gist.github.com/gitlab-org/gitlab-foss', 'https://gist.github.com/gitlab-org/gitlab-foss/hash'])
aggregate_failures do
expect(subject.web).to eq('https://gist.github.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gist.github.com/gitlab-org/gitlab-foss/hash')
expect(subject.compare).to be_nil
end
end
it 'returns original with non-standard url' do
stub_url('http://gist.github.com/another/gitlab-org/gitlab-foss.git')
is_expected.to eq([repo.submodule_url_for, nil])
aggregate_failures do
expect(subject.web).to eq(repo.submodule_url_for)
expect(subject.tree).to be_nil
expect(subject.compare).to be_nil
end
end
end
context 'submodule on github.com' do
it 'detects ssh' do
stub_url('git@github.com:gitlab-org/gitlab-foss.git')
expect(subject).to eq(['https://github.com/gitlab-org/gitlab-foss', 'https://github.com/gitlab-org/gitlab-foss/tree/hash'])
aggregate_failures do
expect(subject.web).to eq('https://github.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://github.com/gitlab-org/gitlab-foss/tree/hash')
expect(subject.compare).to be_nil
end
end
it 'detects http' do
stub_url('http://github.com/gitlab-org/gitlab-foss.git')
expect(subject).to eq(['https://github.com/gitlab-org/gitlab-foss', 'https://github.com/gitlab-org/gitlab-foss/tree/hash'])
aggregate_failures do
expect(subject.web).to eq('https://github.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://github.com/gitlab-org/gitlab-foss/tree/hash')
expect(subject.compare).to be_nil
end
end
it 'detects https' do
stub_url('https://github.com/gitlab-org/gitlab-foss.git')
expect(subject).to eq(['https://github.com/gitlab-org/gitlab-foss', 'https://github.com/gitlab-org/gitlab-foss/tree/hash'])
aggregate_failures do
expect(subject.web).to eq('https://github.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://github.com/gitlab-org/gitlab-foss/tree/hash')
expect(subject.compare).to be_nil
end
end
it 'handles urls with no .git on the end' do
stub_url('http://github.com/gitlab-org/gitlab-foss')
expect(subject).to eq(['https://github.com/gitlab-org/gitlab-foss', 'https://github.com/gitlab-org/gitlab-foss/tree/hash'])
aggregate_failures do
expect(subject.web).to eq('https://github.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://github.com/gitlab-org/gitlab-foss/tree/hash')
expect(subject.compare).to be_nil
end
end
it 'returns original with non-standard url' do
stub_url('http://github.com/another/gitlab-org/gitlab-foss.git')
expect(subject).to eq([repo.submodule_url_for, nil])
aggregate_failures do
expect(subject.web).to eq(repo.submodule_url_for)
expect(subject.tree).to be_nil
expect(subject.compare).to be_nil
end
end
end
......@@ -143,39 +215,67 @@ RSpec.describe SubmoduleHelper do
allow(repo).to receive(:project).and_return(project)
stub_url('./')
expect(subject).to eq(["/master-project/#{project.path}", "/master-project/#{project.path}/-/tree/hash"])
aggregate_failures do
expect(subject.web).to eq("/master-project/#{project.path}")
expect(subject.tree).to eq("/master-project/#{project.path}/-/tree/hash")
expect(subject.compare).to be_nil
end
end
end
context 'submodule on gitlab.com' do
it 'detects ssh' do
stub_url('git@gitlab.com:gitlab-org/gitlab-foss.git')
expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash'])
aggregate_failures do
expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash')
expect(subject.compare).to be_nil
end
end
it 'detects http' do
stub_url('http://gitlab.com/gitlab-org/gitlab-foss.git')
expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash'])
aggregate_failures do
expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash')
expect(subject.compare).to be_nil
end
end
it 'detects https' do
stub_url('https://gitlab.com/gitlab-org/gitlab-foss.git')
expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash'])
aggregate_failures do
expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash')
expect(subject.compare).to be_nil
end
end
it 'handles urls with no .git on the end' do
stub_url('http://gitlab.com/gitlab-org/gitlab-foss')
expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash'])
aggregate_failures do
expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash')
expect(subject.compare).to be_nil
end
end
it 'handles urls with trailing whitespace' do
stub_url('http://gitlab.com/gitlab-org/gitlab-foss.git ')
expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash'])
aggregate_failures do
expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash')
expect(subject.compare).to be_nil
end
end
it 'returns original with non-standard url' do
stub_url('http://gitlab.com/another/gitlab-org/gitlab-foss.git')
expect(subject).to eq([repo.submodule_url_for, nil])
aggregate_failures do
expect(subject.web).to eq(repo.submodule_url_for)
expect(subject.tree).to be_nil
expect(subject.compare).to be_nil
end
end
end
......@@ -183,25 +283,29 @@ RSpec.describe SubmoduleHelper do
it 'sanitizes unsupported protocols' do
stub_url('javascript:alert("XSS");')
expect(subject).to eq([nil, nil])
expect(subject).to be_nil
end
it 'sanitizes unsupported protocols disguised as a repository URL' do
stub_url('javascript:alert("XSS");foo/bar.git')
expect(subject).to eq([nil, nil])
expect(subject).to be_nil
end
it 'sanitizes invalid URL with extended ASCII' do
stub_url('é')
expect(subject).to eq([nil, nil])
expect(subject).to be_nil
end
it 'returns original' do
stub_url('http://mygitserver.com/gitlab-org/gitlab-foss')
expect(subject).to eq([repo.submodule_url_for, nil])
aggregate_failures do
expect(subject.web).to eq(repo.submodule_url_for)
expect(subject.tree).to be_nil
expect(subject.compare).to be_nil
end
end
end
......@@ -214,7 +318,11 @@ RSpec.describe SubmoduleHelper do
stub_url(relative_path)
result = subject
expect(result).to eq([expected_path, "#{expected_path}/-/tree/#{submodule_item.id}"])
aggregate_failures do
expect(result.web).to eq(expected_path)
expect(result.tree).to eq("#{expected_path}/-/tree/#{submodule_item.id}")
expect(result.compare).to be_nil
end
end
it 'handles project under same group' do
......@@ -235,7 +343,7 @@ RSpec.describe SubmoduleHelper do
result = subject
expect(result).to eq([nil, nil])
expect(result).to be_nil
end
end
......@@ -245,7 +353,7 @@ RSpec.describe SubmoduleHelper do
result = subject
expect(result).to eq([nil, nil])
expect(result).to be_nil
end
end
......@@ -289,7 +397,7 @@ RSpec.describe SubmoduleHelper do
end
it 'returns no links' do
expect(subject).to eq([nil, nil])
expect(subject).to be_nil
end
end
end
......
spec/lib/gitlab/submodule_links_spec.rb
View file @ 3e7d5910
......@@ -18,7 +18,7 @@ RSpec.describe Gitlab::SubmoduleLinks do
end
it 'returns no links' do
expect(subject).to eq([nil, nil])
expect(subject).to be_nil
end
end
......@@ -28,17 +28,28 @@ RSpec.describe Gitlab::SubmoduleLinks do
end
it 'returns no links' do
expect(subject).to eq([nil, nil])
expect(subject).to be_nil
end
end
context 'when the submodule is known' do
before do
stub_urls({ 'gitlab-foss' => 'git@gitlab.com:gitlab-org/gitlab-foss.git' })
gitlab_foss = 'git@gitlab.com:gitlab-org/gitlab-foss.git'
stub_urls({
'ref' => { 'gitlab-foss' => gitlab_foss },
'other_ref' => { 'gitlab-foss' => gitlab_foss },
'signed-commits' => { 'gitlab-foss' => gitlab_foss },
'special_ref' => { 'gitlab-foss' => 'git@OTHER.com:gitlab-org/gitlab-foss.git' }
})
end
it 'returns links and caches the by ref' do
expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash'])
aggregate_failures do
expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash')
expect(subject.compare).to be_nil
end
cache_store = links.instance_variable_get("@cache_store")
......@@ -49,13 +60,46 @@ RSpec.describe Gitlab::SubmoduleLinks do
let(:ref) { 'signed-commits' }
it 'returns links' do
expect(subject).to eq(['https://gitlab.com/gitlab-org/gitlab-foss', 'https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash'])
aggregate_failures do
expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash')
expect(subject.compare).to be_nil
end
end
end
context 'and the diff information is available' do
let(:old_ref) { 'other_ref' }
let(:diff_file) { double(old_blob: double(id: 'old-hash', path: 'gitlab-foss'), old_content_sha: old_ref) }
subject { links.for(submodule_item, ref, diff_file) }
it 'the returned links include the compare link' do
aggregate_failures do
expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash')
expect(subject.compare).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/compare/old-hash...hash')
end
end
def stub_urls(urls)
allow(repo).to receive(:submodule_urls_for).and_return(urls)
context 'but the submodule url has changed' do
let(:old_ref) { 'special_ref' }
it 'the returned links do not include the compare link' do
aggregate_failures do
expect(subject.web).to eq('https://gitlab.com/gitlab-org/gitlab-foss')
expect(subject.tree).to eq('https://gitlab.com/gitlab-org/gitlab-foss/-/tree/hash')
expect(subject.compare).to be_nil
end
end
end
end
end
end
def stub_urls(urls_by_ref)
allow(repo).to receive(:submodule_urls_for) do |ref|
urls_by_ref[ref] if urls_by_ref
end
end
end
spec/serializers/diff_file_base_entity_spec.rb
View file @ 3e7d5910
......@@ -7,22 +7,51 @@ RSpec.describe DiffFileBaseEntity do
let(:repository) { project.repository }
let(:entity) { described_class.new(diff_file, options).as_json }
context 'diff for a changed submodule' do
let(:commit_sha_with_changed_submodule) do
"cfe32cf61b73a0d5e9f13e774abde7ff789b1660"
end
let(:commit) { project.commit(commit_sha_with_changed_submodule) }
context 'submodule information for a' do
let(:commit_sha) { "" }
let(:commit) { project.commit(commit_sha) }
let(:options) { { request: {}, submodule_links: Gitlab::SubmoduleLinks.new(repository) } }
let(:diff_file) { commit.diffs.diff_files.to_a.last }
it do
context 'newly added submodule' do
let(:commit_sha) { "cfe32cf61b73a0d5e9f13e774abde7ff789b1660" }
it 'says it is a submodule and contains links' do
expect(entity[:submodule]).to eq(true)
expect(entity[:submodule_link]).to eq("https://github.com/randx/six")
expect(entity[:submodule_tree_url]).to eq(
"https://github.com/randx/six/tree/409f37c4f05865e4fb208c771485f211a22c4c2d"
)
end
it 'has no compare url because the submodule was newly added' do
expect(entity[:submodule_compare]).to eq(nil)
end
end
context 'changed submodule' do
# Test repo does not contain a commit that changes a submodule, so we have create such a commit here
let(:commit_sha) { repository.update_submodule(project.members[0].user, "six", "c6bc3aa2ee76cadaf0cbd325067c55450997632e", message: "Go one commit back", branch: "master") }
it 'contains a link to compare the changes' do
expect(entity[:submodule_compare]).to eq(
{
url: "https://github.com/randx/six/compare/409f37c4f05865e4fb208c771485f211a22c4c2d...c6bc3aa2ee76cadaf0cbd325067c55450997632e",
old_sha: "409f37c4f05865e4fb208c771485f211a22c4c2d",
new_sha: "c6bc3aa2ee76cadaf0cbd325067c55450997632e"
}
)
end
end
context 'normal file (no submodule)' do
let(:commit_sha) { '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' }
it 'sets submodule to false' do
expect(entity[:submodule]).to eq(false)
expect(entity[:submodule_compare]).to eq(nil)
end
end
end
context 'contains raw sizes for the blob' do
......
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