diff options
| -rw-r--r-- | app/views/projects/show.html.haml | 4 | ||||
| -rw-r--r-- | changelogs/unreleased/sh-fix-commit-signatures-error.yml | 5 | ||||
| -rw-r--r-- | changelogs/unreleased/test-usage-ping-in-timeout-case.yml | 5 | ||||
| -rw-r--r-- | doc/development/code_review.md | 2 | ||||
| -rw-r--r-- | doc/user/project/integrations/webhooks.md | 13 | ||||
| -rw-r--r-- | lib/gitlab/hook_data/base_builder.rb | 11 | ||||
| -rw-r--r-- | spec/features/projects_spec.rb | 17 | ||||
| -rw-r--r-- | spec/lib/gitlab/hook_data/base_builder_spec.rb | 129 | ||||
| -rw-r--r-- | spec/lib/gitlab/hook_data/issue_builder_spec.rb | 5 | ||||
| -rw-r--r-- | spec/lib/gitlab/hook_data/merge_request_builder_spec.rb | 7 |
10 files changed, 140 insertions, 58 deletions
diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index aba289c790f..283031b06da 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -8,8 +8,8 @@ = render partial: 'flash_messages', locals: { project: @project } -- if @project.repository_exists? && !@project.empty_repo? - - signatures_path = namespace_project_signatures_path(namespace_id: @project.namespace.full_path, project_id: @project.path, id: @project.default_branch) +- if !@project.empty_repo? && can?(current_user, :download_code, @project) + - signatures_path = project_signatures_path(@project, @project.default_branch) .js-signature-container{ data: { 'signatures-path': signatures_path } } %div{ class: [container_class, ("limit-container-width" unless fluid_layout)] } diff --git a/changelogs/unreleased/sh-fix-commit-signatures-error.yml b/changelogs/unreleased/sh-fix-commit-signatures-error.yml new file mode 100644 index 00000000000..e2ea0e5857e --- /dev/null +++ b/changelogs/unreleased/sh-fix-commit-signatures-error.yml @@ -0,0 +1,5 @@ +--- +title: Fix commit signature error when project is disabled +merge_request: 22344 +author: +type: fixed diff --git a/changelogs/unreleased/test-usage-ping-in-timeout-case.yml b/changelogs/unreleased/test-usage-ping-in-timeout-case.yml new file mode 100644 index 00000000000..daee98765ac --- /dev/null +++ b/changelogs/unreleased/test-usage-ping-in-timeout-case.yml @@ -0,0 +1,5 @@ +--- +title: Fix auto-corrected upload URLs in webhooks +merge_request: 22361 +author: +type: fixed diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 87437dd7720..4bbcdc6329f 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -109,7 +109,7 @@ It is responsibility of the author of a merge request that the merge request is ### List of merge requests ready for review -Developers who have capacity can regularly check the list of [merge requests to review](https://gitlab.com/groups/gitlab-org/-/merge_requests?scope=all&utf8=%E2%9C%93&state=opened&label_name%5B%5D=ready%20for%20review&assignee_id=0) and assign any merge request they want to review. +Developers who have capacity can regularly check the list of [merge requests to review](https://gitlab.com/groups/gitlab-org/-/merge_requests?scope=all&utf8=%E2%9C%93&state=opened&label_name%5B%5D=ready%20for%20review) and assign any merge request they want to review. ### Reviewing code diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index 7d12cd8f7c2..7c63967c829 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -7,7 +7,7 @@ > - the `project.http_url` key is deprecated in favor of the `project.git_http_url` key > > **Note:** -> Starting from GitLab 11.1, the logs of web hooks are automatically removed after +> Starting from GitLab 11.1, the logs of webhooks are automatically removed after > one month. > > **Note:** @@ -73,8 +73,8 @@ Below are described the supported events. Triggered when you push to the repository except when pushing tags. -> **Note:** When more than 20 commits are pushed at once, the `commits` web hook - attribute will only contain the first 20 for performance reasons. Loading +> **Note:** When more than 20 commits are pushed at once, the `commits` webhook + attribute will only contain the first 20 for performance reasons. Loading detailed commit data is expensive. Note that despite only 20 commits being present in the `commits` attribute, the `total_commits_count` attribute will contain the actual total. @@ -1157,10 +1157,11 @@ its description: ``` It will appear in the webhook body as the below (assuming that GitLab is -installed at gitlab.example.com): +installed at gitlab.example.com, and the project is at +example-group/example-project): ```markdown - + ``` This will not rewrite URLs that already are pointing to HTTP, HTTPS, or @@ -1190,7 +1191,7 @@ From this page, you can repeat delivery with the same data by clicking `Resend R > **Note:** If URL or secret token of the webhook were updated, data will be delivered to the new address. -### Receiving duplicate or multiple web hook requests triggered by one event +### Receiving duplicate or multiple webhook requests triggered by one event When GitLab sends a webhook it expects a response in 10 seconds (set default value). If it does not receive one, it'll retry the webhook. If the endpoint doesn't send its HTTP response within those 10 seconds, GitLab may decide the hook failed and retry it. diff --git a/lib/gitlab/hook_data/base_builder.rb b/lib/gitlab/hook_data/base_builder.rb index 4ffca356b29..1a91301e8be 100644 --- a/lib/gitlab/hook_data/base_builder.rb +++ b/lib/gitlab/hook_data/base_builder.rb @@ -25,6 +25,7 @@ module Gitlab markdown_text.gsub(MARKDOWN_SIMPLE_IMAGE) do if $~[:image] url = $~[:url] + url = "#{uploads_prefix}#{url}" if url.start_with?('/uploads') url = "/#{url}" unless url.start_with?('/') "![#{$~[:title]}](#{Gitlab.config.gitlab.url}#{url})" @@ -33,6 +34,16 @@ module Gitlab end end end + + def uploads_prefix + project&.full_path || '' + end + + def project + return unless object.respond_to?(:project) + + object.project + end end end end diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index 8e310f38a8c..fb766addb31 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -193,6 +193,23 @@ describe 'Project' do end end + describe 'when the project repository is disabled', :js do + let(:user) { create(:user) } + let(:project) { create(:project, :repository_disabled, :repository, namespace: user.namespace) } + + before do + sign_in(user) + project.add_maintainer(user) + visit project_path(project) + end + + it 'does not show an error' do + wait_for_requests + + expect(page).not_to have_selector('.flash-alert') + end + end + describe 'removal', :js do let(:user) { create(:user) } let(:project) { create(:project, namespace: user.namespace) } diff --git a/spec/lib/gitlab/hook_data/base_builder_spec.rb b/spec/lib/gitlab/hook_data/base_builder_spec.rb index a921dd766c3..e3c5ee3b905 100644 --- a/spec/lib/gitlab/hook_data/base_builder_spec.rb +++ b/spec/lib/gitlab/hook_data/base_builder_spec.rb @@ -8,57 +8,94 @@ describe Gitlab::HookData::BaseBuilder do end end - subject { subclass.new(nil) } - using RSpec::Parameterized::TableSyntax - where do - { - 'relative image URL' => { - input: '', - output: "" - }, - 'HTTP URL' => { - input: '', - output: '' - }, - 'HTTPS URL' => { - input: '', - output: '' - }, - 'protocol-relative URL' => { - input: '', - output: '' - }, - 'URL reference by title' => { - input: "![foo]\n\n[foo]: foo.png", - output: "![foo]\n\n[foo]: foo.png" - }, - 'URL reference by label' => { - input: "![][foo]\n\n[foo]: foo.png", - output: "![][foo]\n\n[foo]: foo.png" - }, - 'in Markdown inline code block' => { - input: '``', - output: "``" - }, - 'in HTML tag on the same line' => { - input: '<p></p>', - output: "<p></p>" - }, - 'in Markdown multi-line code block' => { - input: "```\n\n```", - output: "```\n\n```" - }, - 'in HTML tag on different lines' => { - input: "<p>\n\n</p>", - output: "<p>\n\n</p>" + context 'with an upload prefix specified' do + let(:project_with_path) { double(full_path: 'baz/bar') } + let(:object_with_project) { double(project: project_with_path) } + subject { subclass.new(object_with_project) } + + where do + { + 'relative image URL' => { + input: '', + output: "" + }, + 'absolute upload URL' => { + input: '', + output: "" + }, + 'absolute non-upload URL' => { + input: '', + output: "" + } } - } + end + + with_them do + it { expect(subject.absolute_image_urls(input)).to eq(output) } + end end - with_them do - it { expect(subject.absolute_image_urls(input)).to eq(output) } + context 'without an upload prefix specified' do + subject { subclass.new(nil) } + + where do + { + 'relative image URL' => { + input: '', + output: "" + }, + 'absolute upload URL' => { + input: '', + output: "" + }, + 'absolute non-upload URL' => { + input: '', + output: "" + }, + 'HTTP URL' => { + input: '', + output: '' + }, + 'HTTPS URL' => { + input: '', + output: '' + }, + 'protocol-relative URL' => { + input: '', + output: '' + }, + 'URL reference by title' => { + input: "![foo]\n\n[foo]: foo.png", + output: "![foo]\n\n[foo]: foo.png" + }, + 'URL reference by label' => { + input: "![][foo]\n\n[foo]: foo.png", + output: "![][foo]\n\n[foo]: foo.png" + }, + 'in Markdown inline code block' => { + input: '``', + output: "``" + }, + 'in HTML tag on the same line' => { + input: '<p></p>', + output: "<p></p>" + }, + 'in Markdown multi-line code block' => { + input: "```\n\n```", + output: "```\n\n```" + }, + 'in HTML tag on different lines' => { + input: "<p>\n\n</p>", + output: "<p>\n\n</p>" + } + } + end + + with_them do + it { expect(subject.absolute_image_urls(input)).to eq(output) } + end end end end diff --git a/spec/lib/gitlab/hook_data/issue_builder_spec.rb b/spec/lib/gitlab/hook_data/issue_builder_spec.rb index 60093474f8a..f066c0e3813 100644 --- a/spec/lib/gitlab/hook_data/issue_builder_spec.rb +++ b/spec/lib/gitlab/hook_data/issue_builder_spec.rb @@ -46,7 +46,10 @@ describe Gitlab::HookData::IssueBuilder do let(:builder) { described_class.new(issue_with_description) } it 'sets the image to use an absolute URL' do - expect(data[:description]).to eq("test") + expected_path = "#{issue_with_description.project.path_with_namespace}/uploads/abc/Issue_Image.png" + + expect(data[:description]) + .to eq("test") end end end diff --git a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb index dd586af6118..9ce697adbba 100644 --- a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb +++ b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb @@ -58,11 +58,14 @@ describe Gitlab::HookData::MergeRequestBuilder do end context 'when the MR has an image in the description' do - let(:mr_with_description) { create(:merge_request, description: 'test') } + let(:mr_with_description) { create(:merge_request, description: 'test') } let(:builder) { described_class.new(mr_with_description) } it 'sets the image to use an absolute URL' do - expect(data[:description]).to eq("test") + expected_path = "#{mr_with_description.project.path_with_namespace}/uploads/abc/MR_Image.png" + + expect(data[:description]) + .to eq("test") end end end |