diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-03 18:06:11 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-03 18:06:11 +0000 |
commit | 25521def84a6987fe9d4265b560e930bfb32c195 (patch) | |
tree | 711e001ea65f76a9c2eb034c4531bda325af84f3 | |
parent | 9a1c5456747a7b5b218b8b44e4b43396bf7fd705 (diff) | |
download | gitlab-ce-25521def84a6987fe9d4265b560e930bfb32c195.tar.gz |
Add latest changes from gitlab-org/gitlab@master
23 files changed, 349 insertions, 62 deletions
diff --git a/app/assets/javascripts/repository/components/last_commit.vue b/app/assets/javascripts/repository/components/last_commit.vue index e2060d4aeec..1b023f13862 100644 --- a/app/assets/javascripts/repository/components/last_commit.vue +++ b/app/assets/javascripts/repository/components/last_commit.vue @@ -1,5 +1,4 @@ <script> -/* eslint-disable @gitlab/vue-i18n/no-bare-strings */ import { GlTooltipDirective, GlLink, GlButton, GlLoadingIcon } from '@gitlab/ui'; import { sprintf, s__ } from '~/locale'; import Icon from '../../vue_shared/components/icon.vue'; @@ -113,7 +112,7 @@ export default { > {{ commit.author.name }} </gl-link> - authored + {{ s__('LastCommit|authored') }} <timeago-tooltip :time="commit.authoredDate" tooltip-placement="bottom" /> </div> <pre @@ -125,6 +124,7 @@ export default { </pre> </div> <div class="commit-actions flex-row"> + <div v-if="commit.signatureHtml" v-html="commit.signatureHtml"></div> <gl-link v-if="commit.latestPipeline" v-gl-tooltip diff --git a/app/assets/javascripts/repository/queries/pathLastCommit.query.graphql b/app/assets/javascripts/repository/queries/pathLastCommit.query.graphql index 3bdfd979fa4..71c1bf12749 100644 --- a/app/assets/javascripts/repository/queries/pathLastCommit.query.graphql +++ b/app/assets/javascripts/repository/queries/pathLastCommit.query.graphql @@ -13,6 +13,7 @@ query pathLastCommit($projectPath: ID!, $path: String, $ref: String!) { avatarUrl webUrl } + signatureHtml latestPipeline { detailedStatus { detailsPath diff --git a/app/graphql/resolvers/last_commit_resolver.rb b/app/graphql/resolvers/last_commit_resolver.rb new file mode 100644 index 00000000000..7a433d6556f --- /dev/null +++ b/app/graphql/resolvers/last_commit_resolver.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Resolvers + class LastCommitResolver < BaseResolver + type Types::CommitType, null: true + + alias_method :tree, :object + + def resolve(**args) + # Ensure merge commits can be returned by sending nil to Gitaly instead of '/' + path = tree.path == '/' ? nil : tree.path + commit = Gitlab::Git::Commit.last_for_path(tree.repository, tree.sha, path) + + ::Commit.new(commit, tree.repository.project) if commit + end + end +end diff --git a/app/graphql/types/commit_type.rb b/app/graphql/types/commit_type.rb index dd2d81adb8b..fe71791f413 100644 --- a/app/graphql/types/commit_type.rb +++ b/app/graphql/types/commit_type.rb @@ -15,6 +15,8 @@ module Types field :message, type: GraphQL::STRING_TYPE, null: true # rubocop:disable Graphql/Descriptions field :authored_date, type: Types::TimeType, null: true # rubocop:disable Graphql/Descriptions field :web_url, type: GraphQL::STRING_TYPE, null: false # rubocop:disable Graphql/Descriptions + field :signature_html, type: GraphQL::STRING_TYPE, + null: true, calls_gitaly: true, description: 'Rendered html for the commit signature' # models/commit lazy loads the author by email field :author, type: Types::UserType, null: true # rubocop:disable Graphql/Descriptions diff --git a/app/graphql/types/tree/tree_type.rb b/app/graphql/types/tree/tree_type.rb index b967cf3a247..56d544b5fd1 100644 --- a/app/graphql/types/tree/tree_type.rb +++ b/app/graphql/types/tree/tree_type.rb @@ -7,9 +7,9 @@ module Types graphql_name 'Tree' # Complexity 10 as it triggers a Gitaly call on each render - field :last_commit, Types::CommitType, null: true, complexity: 10, calls_gitaly: true, resolve: -> (tree, args, ctx) do # rubocop:disable Graphql/Descriptions - tree.repository.last_commit_for_path(tree.sha, tree.path) - end + field :last_commit, Types::CommitType, + null: true, complexity: 10, calls_gitaly: true, resolver: Resolvers::LastCommitResolver, + description: 'Last commit for the tree' field :trees, Types::Tree::TreeEntryType.connection_type, null: false, resolve: -> (obj, args, ctx) do # rubocop:disable Graphql/Descriptions Gitlab::Graphql::Representation::TreeEntry.decorate(obj.trees, obj.repository) diff --git a/app/presenters/commit_presenter.rb b/app/presenters/commit_presenter.rb index f5b1e45c0e9..94fc8ac8e39 100644 --- a/app/presenters/commit_presenter.rb +++ b/app/presenters/commit_presenter.rb @@ -20,4 +20,15 @@ class CommitPresenter < Gitlab::View::Presenter::Delegated def web_url Gitlab::UrlBuilder.new(commit).url end + + def signature_html + return unless commit.has_signature? + + ApplicationController.renderer.render( + 'projects/commit/_signature', + locals: { signature: commit.signature }, + layout: false, + formats: [:html] + ) + end end diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index dbff2115f50..3e53cb510b0 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -34,8 +34,9 @@ = _('Merge request') - if branch.name != @repository.root_ref - = link_to project_compare_path(@project, @repository.root_ref, branch.name), + = link_to project_compare_index_path(@project, from: @repository.root_ref, to: branch.name), class: "btn btn-default js-onboarding-compare-branches #{'prepend-left-10' unless merge_project}", + method: :post, title: s_('Branches|Compare') do = s_('Branches|Compare') diff --git a/changelogs/unreleased/29284-video-preview-not-working.yml b/changelogs/unreleased/29284-video-preview-not-working.yml new file mode 100644 index 00000000000..1d36b3e0dff --- /dev/null +++ b/changelogs/unreleased/29284-video-preview-not-working.yml @@ -0,0 +1,5 @@ +--- +title: Fix inline rendering of videos for uploads with uppercase file extensions +merge_request: 17924 +author: +type: fixed diff --git a/changelogs/unreleased/9841-geo-unable-to-compare-branches-on-secondary.yml b/changelogs/unreleased/9841-geo-unable-to-compare-branches-on-secondary.yml new file mode 100644 index 00000000000..037e63c4813 --- /dev/null +++ b/changelogs/unreleased/9841-geo-unable-to-compare-branches-on-secondary.yml @@ -0,0 +1,5 @@ +--- +title: Allow users to compare Git revisions on a read-only instance +merge_request: 18038 +author: +type: fixed diff --git a/doc/user/project/pages/custom_domains_ssl_tls_certification/lets_encrypt_integration.md b/doc/user/project/pages/custom_domains_ssl_tls_certification/lets_encrypt_integration.md index ef5466f03c4..c9b504dc6ee 100644 --- a/doc/user/project/pages/custom_domains_ssl_tls_certification/lets_encrypt_integration.md +++ b/doc/user/project/pages/custom_domains_ssl_tls_certification/lets_encrypt_integration.md @@ -57,7 +57,7 @@ Once you've met the requirements, to enable Let's Encrypt integration: 1. Click **Save changes**. Once enabled, GitLab will obtain a LE certificate and add it to the -associated Pages domain. It will be also renewed automatically by GitLab. +associated Pages domain. It also will be renewed automatically by GitLab. > **Notes:** > diff --git a/lib/banzai/filter/video_link_filter.rb b/lib/banzai/filter/video_link_filter.rb index a35b0d7a0b5..0e329339474 100644 --- a/lib/banzai/filter/video_link_filter.rb +++ b/lib/banzai/filter/video_link_filter.rb @@ -8,8 +8,8 @@ module Banzai # a "Download" link in the case the video cannot be played. class VideoLinkFilter < HTML::Pipeline::Filter def call - doc.xpath(query).each do |el| - el.replace(video_node(doc, el)) + doc.xpath('descendant-or-self::img[not(ancestor::a)]').each do |el| + el.replace(video_node(doc, el)) if has_video_extension?(el) end doc @@ -17,22 +17,13 @@ module Banzai private - def query - @query ||= begin - src_query = UploaderHelper::SAFE_VIDEO_EXT.map do |ext| - "'.#{ext}' = substring(@src, string-length(@src) - #{ext.size})" - end + def has_video_extension?(element) + src = element.attr('data-canonical-src').presence || element.attr('src') - if context[:asset_proxy_enabled].present? - src_query.concat( - UploaderHelper::SAFE_VIDEO_EXT.map do |ext| - "'.#{ext}' = substring(@data-canonical-src, string-length(@data-canonical-src) - #{ext.size})" - end - ) - end + return unless src.present? - "descendant-or-self::img[not(ancestor::a) and (#{src_query.join(' or ')})]" - end + src_ext = File.extname(src).sub('.', '').downcase + Gitlab::FileTypeDetection::SAFE_VIDEO_EXT.include?(src_ext) end def video_node(doc, element) diff --git a/lib/gitlab/middleware/read_only/controller.rb b/lib/gitlab/middleware/read_only/controller.rb index 907e031a02e..b18f0eed1fa 100644 --- a/lib/gitlab/middleware/read_only/controller.rb +++ b/lib/gitlab/middleware/read_only/controller.rb @@ -20,6 +20,10 @@ module Gitlab 'projects/lfs_locks_api' => %w{verify create unlock} }.freeze + WHITELISTED_GIT_REVISION_ROUTES = { + 'projects/compare' => %w{create} + }.freeze + GRAPHQL_URL = '/api/graphql' def initialize(app, env) @@ -81,7 +85,7 @@ module Gitlab # Overridden in EE module def whitelisted_routes - grack_route? || internal_route? || lfs_route? || sidekiq_route? || graphql_query? + grack_route? || internal_route? || lfs_route? || compare_git_revisions_route? || sidekiq_route? || graphql_query? end def grack_route? @@ -96,6 +100,13 @@ module Gitlab ReadOnly.internal_routes.any? { |path| request.path.include?(path) } end + def compare_git_revisions_route? + # Calling route_hash may be expensive. Only do it if we think there's a possible match + return false unless request.post? && request.path.end_with?('compare') + + WHITELISTED_GIT_REVISION_ROUTES[route_hash[:controller]]&.include?(route_hash[:action]) + end + def lfs_route? # Calling route_hash may be expensive. Only do it if we think there's a possible match unless request.path.end_with?('/info/lfs/objects/batch', diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e57e4d668a5..75323601017 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9224,6 +9224,9 @@ msgstr "" msgid "Last used on:" msgstr "" +msgid "LastCommit|authored" +msgstr "" + msgid "LastPushEvent|You pushed to" msgstr "" diff --git a/rubocop/cop/line_break_around_conditional_block.rb b/rubocop/cop/line_break_around_conditional_block.rb index 8118b314b63..2523cc162f3 100644 --- a/rubocop/cop/line_break_around_conditional_block.rb +++ b/rubocop/cop/line_break_around_conditional_block.rb @@ -117,7 +117,7 @@ module RuboCop end def block_start?(line) - line.match(/ (do|{)( \|.*?\|)?\s?$/) + line.match(/ (do|{)( \|.*?\|)?\s?(#.+)?\z/) end def end_line?(line) diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb index b35067d0f4d..18e2723ff8e 100644 --- a/spec/features/projects/branches_spec.rb +++ b/spec/features/projects/branches_spec.rb @@ -246,7 +246,6 @@ describe 'Branches' do end expect(page).to have_content 'Commits' - expect(page).to have_link 'Create merge request' end end diff --git a/spec/features/projects/compare_spec.rb b/spec/features/projects/compare_spec.rb index 2674617bcfc..34bde29c8da 100644 --- a/spec/features/projects/compare_spec.rb +++ b/spec/features/projects/compare_spec.rb @@ -12,6 +12,23 @@ describe "Compare", :js do end describe "branches" do + shared_examples 'compares branches' do + it 'compares branches' do + visit project_compare_index_path(project, from: 'master', to: 'master') + + select_using_dropdown 'from', 'feature' + expect(find('.js-compare-from-dropdown .dropdown-toggle-text')).to have_content('feature') + + select_using_dropdown 'to', 'binary-encoding' + expect(find('.js-compare-to-dropdown .dropdown-toggle-text')).to have_content('binary-encoding') + + click_button 'Compare' + + expect(page).to have_content 'Commits' + expect(page).to have_link 'Create merge request' + end + end + it "pre-populates fields" do visit project_compare_index_path(project, from: "master", to: "master") @@ -19,19 +36,14 @@ describe "Compare", :js do expect(find(".js-compare-to-dropdown .dropdown-toggle-text")).to have_content("master") end - it "compares branches" do - visit project_compare_index_path(project, from: "master", to: "master") - - select_using_dropdown "from", "feature" - expect(find(".js-compare-from-dropdown .dropdown-toggle-text")).to have_content("feature") - - select_using_dropdown "to", "binary-encoding" - expect(find(".js-compare-to-dropdown .dropdown-toggle-text")).to have_content("binary-encoding") + it_behaves_like 'compares branches' - click_button "Compare" + context 'on a read-only instance' do + before do + allow(Gitlab::Database).to receive(:read_only?).and_return(true) + end - expect(page).to have_content "Commits" - expect(page).to have_link 'Create merge request' + it_behaves_like 'compares branches' end it 'renders additions info when click unfold diff' do diff --git a/spec/frontend/repository/components/__snapshots__/last_commit_spec.js.snap b/spec/frontend/repository/components/__snapshots__/last_commit_spec.js.snap index cd8372a8800..0e04487cb20 100644 --- a/spec/frontend/repository/components/__snapshots__/last_commit_spec.js.snap +++ b/spec/frontend/repository/components/__snapshots__/last_commit_spec.js.snap @@ -60,6 +60,111 @@ exports[`Repository last commit component renders commit widget 1`] = ` <div class="commit-actions flex-row" > + <!----> + + <gllink-stub + class="js-commit-pipeline" + data-original-title="Commit: failed" + href="https://test.com/pipeline" + title="" + > + <ciicon-stub + aria-label="Commit: failed" + cssclasses="" + size="24" + status="[object Object]" + /> + </gllink-stub> + + <div + class="commit-sha-group d-flex" + > + <div + class="label label-monospace monospace" + > + + 12345678 + + </div> + + <clipboardbutton-stub + cssclass="btn-default" + text="123456789" + title="Copy commit SHA to clipboard" + tooltipplacement="bottom" + /> + </div> + </div> + </div> +</div> +`; + +exports[`Repository last commit component renders the signature HTML as returned by the backend 1`] = ` +<div + class="info-well d-none d-sm-flex project-last-commit commit p-3" +> + <useravatarlink-stub + class="avatar-cell" + imgalt="" + imgcssclasses="" + imgsize="40" + imgsrc="https://test.com" + linkhref="https://test.com/test" + tooltipplacement="top" + tooltiptext="" + username="" + /> + + <div + class="commit-detail flex-list" + > + <div + class="commit-content qa-commit-content" + > + <gllink-stub + class="commit-row-message item-title" + href="https://test.com/commit/123" + > + + Commit title + + </gllink-stub> + + <!----> + + <div + class="committer" + > + <gllink-stub + class="commit-author-link js-user-link" + href="https://test.com/test" + > + + Test + + </gllink-stub> + + authored + + <timeagotooltip-stub + cssclass="" + time="2019-01-01" + tooltipplacement="bottom" + /> + </div> + + <!----> + </div> + + <div + class="commit-actions flex-row" + > + <div> + <button> + Verified + </button> + </div> + <gllink-stub class="js-commit-pipeline" data-original-title="Commit: failed" diff --git a/spec/frontend/repository/components/last_commit_spec.js b/spec/frontend/repository/components/last_commit_spec.js index 14479f3c3a4..01b56d453e6 100644 --- a/spec/frontend/repository/components/last_commit_spec.js +++ b/spec/frontend/repository/components/last_commit_spec.js @@ -107,4 +107,10 @@ describe('Repository last commit component', () => { expect(vm.find('.commit-row-description').isVisible()).toBe(true); expect(vm.find('.text-expander').classes('open')).toBe(true); }); + + it('renders the signature HTML as returned by the backend', () => { + factory(createCommitData({ signatureHtml: '<button>Verified</button>' })); + + expect(vm.element).toMatchSnapshot(); + }); }); diff --git a/spec/graphql/resolvers/last_commit_resolver_spec.rb b/spec/graphql/resolvers/last_commit_resolver_spec.rb new file mode 100644 index 00000000000..15b09b77a10 --- /dev/null +++ b/spec/graphql/resolvers/last_commit_resolver_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Resolvers::LastCommitResolver do + include GraphqlHelpers + + let(:repository) { create(:project, :repository).repository } + let(:tree) { repository.tree(ref, path) } + + let(:commit) { resolve(described_class, obj: tree) } + + describe '#resolve' do + context 'last commit is a merge commit' do + let(:ref) { 'master' } + let(:path) { '/' } + + it 'resolves to the merge commit' do + expect(commit).to eq(repository.commits(ref, limit: 1).last) + end + end + + context 'last commit for a different branch and path' do + let(:ref) { 'fix' } + let(:path) { 'files' } + + it 'resolves commit' do + expect(commit).to eq(repository.commits(ref, path: path, limit: 1).last) + end + end + + context 'last commit does not exist' do + let(:ref) { 'master' } + let(:path) { 'does-not-exist' } + + it 'returns nil' do + expect(commit).to be_nil + end + end + end +end diff --git a/spec/graphql/types/commit_type_spec.rb b/spec/graphql/types/commit_type_spec.rb index 5d8edcf254c..1ff1c97f8db 100644 --- a/spec/graphql/types/commit_type_spec.rb +++ b/spec/graphql/types/commit_type_spec.rb @@ -7,5 +7,10 @@ describe GitlabSchema.types['Commit'] do it { expect(described_class).to require_graphql_authorizations(:download_code) } - it { expect(described_class).to have_graphql_fields(:id, :sha, :title, :description, :message, :authored_date, :author, :web_url, :latest_pipeline) } + it 'contains attributes related to commit' do + expect(described_class).to have_graphql_fields( + :id, :sha, :title, :description, :message, :authored_date, + :author, :web_url, :latest_pipeline, :signature_html + ) + end end diff --git a/spec/lib/banzai/filter/video_link_filter_spec.rb b/spec/lib/banzai/filter/video_link_filter_spec.rb index b5be204d680..332817d6585 100644 --- a/spec/lib/banzai/filter/video_link_filter_spec.rb +++ b/spec/lib/banzai/filter/video_link_filter_spec.rb @@ -12,52 +12,98 @@ describe Banzai::Filter::VideoLinkFilter do end def link_to_image(path) - %(<img src="#{path}" />) + return '<img/>' if path.nil? + + %(<img src="#{path}"/>) end let(:project) { create(:project, :repository) } - context 'when the element src has a video extension' do - UploaderHelper::SAFE_VIDEO_EXT.each do |ext| - it "replaces the image tag 'path/video.#{ext}' with a video tag" do - container = filter(link_to_image("/path/video.#{ext}")).children.first + shared_examples 'a video element' do + let(:image) { link_to_image(src) } - expect(container.name).to eq 'div' - expect(container['class']).to eq 'video-container' + it 'replaces the image tag with a video tag' do + container = filter(image).children.first - video, paragraph = container.children + expect(container.name).to eq 'div' + expect(container['class']).to eq 'video-container' - expect(video.name).to eq 'video' - expect(video['src']).to eq "/path/video.#{ext}" + video, paragraph = container.children - expect(paragraph.name).to eq 'p' + expect(video.name).to eq 'video' + expect(video['src']).to eq src - link = paragraph.children.first + expect(paragraph.name).to eq 'p' - expect(link.name).to eq 'a' - expect(link['href']).to eq "/path/video.#{ext}" - expect(link['target']).to eq '_blank' - end + link = paragraph.children.first + + expect(link.name).to eq 'a' + expect(link['href']).to eq src + expect(link['target']).to eq '_blank' end end - context 'when the element src is an image' do + shared_examples 'an unchanged element' do |ext| it 'leaves the document unchanged' do - element = filter(link_to_image('/path/my_image.jpg')).children.first + element = filter(link_to_image(src)).children.first expect(element.name).to eq 'img' - expect(element['src']).to eq '/path/my_image.jpg' + expect(element['src']).to eq src end end - context 'when asset proxy is enabled' do - it 'uses the correct src' do - stub_asset_proxy_setting(enabled: true) + context 'when the element src has a video extension' do + Gitlab::FileTypeDetection::SAFE_VIDEO_EXT.each do |ext| + it_behaves_like 'a video element' do + let(:src) { "/path/video.#{ext}" } + end + + it_behaves_like 'a video element' do + let(:src) { "/path/video.#{ext.upcase}" } + end + end + end + + context 'when the element has no src attribute' do + let(:src) { nil } + + it_behaves_like 'an unchanged element' + end + + context 'when the element src is an image' do + let(:src) { '/path/my_image.jpg' } + + it_behaves_like 'an unchanged element' + end + + context 'when the element src has an invalid file extension' do + let(:src) { '/path/my_video.somemp4' } + + it_behaves_like 'an unchanged element' + end + + context 'when data-canonical-src is empty' do + let(:image) { %(<img src="#{src}" data-canonical-src=""/>) } + context 'and src is a video' do + let(:src) { '/path/video.mp4' } + + it_behaves_like 'a video element' + end + + context 'and src is an image' do + let(:src) { '/path/my_image.jpg' } + + it_behaves_like 'an unchanged element' + end + end + + context 'when data-canonical-src is set' do + it 'uses the correct src' do proxy_src = 'https://assets.example.com/6d8b63' canonical_src = 'http://example.com/test.mp4' - image = %(<img src="#{proxy_src}" data-canonical-src="#{canonical_src}" />) - container = filter(image, asset_proxy_enabled: true).children.first + image = %(<img src="#{proxy_src}" data-canonical-src="#{canonical_src}"/>) + container = filter(image).children.first expect(container['class']).to eq 'video-container' diff --git a/spec/presenters/commit_presenter_spec.rb b/spec/presenters/commit_presenter_spec.rb index 58179e9a337..bc749acfa3a 100644 --- a/spec/presenters/commit_presenter_spec.rb +++ b/spec/presenters/commit_presenter_spec.rb @@ -55,4 +55,17 @@ describe CommitPresenter do end end end + + describe '#signature_html' do + let(:signature) { 'signature' } + + before do + expect(commit).to receive(:has_signature?).and_return(true) + allow(ApplicationController.renderer).to receive(:render).and_return(signature) + end + + it 'renders html for displaying signature' do + expect(presenter.signature_html).to eq(signature) + end + end end diff --git a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb index 892b393c307..cc933ce12c8 100644 --- a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb +++ b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb @@ -132,6 +132,19 @@ describe RuboCop::Cop::LineBreakAroundConditionalBlock do expect(cop.offenses).to be_empty end + it "doesn't flag violation for #{conditional} preceded by a block definition with a comment" do + source = <<~RUBY + on_block(param_a) do |item| # a short comment + #{conditional} condition + do_something + end + end + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + it "doesn't flag violation for #{conditional} preceded by a block definition using brackets" do source = <<~RUBY on_block(param_a) { |item| |