Fix attachments using the wrong URLs in e-mails

Prior to https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29889,
only the project context were set for the Markdown renderer. For a note
on an issuable, the group context was set to `nil` because
`note.noteable.try(:group)` attempted to get the issuable's group, which
doesn't exist.

To make group notifications work, now both the project and group context
are set. The context gets passed to `RelativeLinkFilter`, which
previously assumed that it wasn't possible to have both a group and a
project in the Markdown context. However, if a group were defined, it
would take precedence, and the URL rendered for uploads would be
`/group/-/uploads` instead of `/group/project/uploads/`. This led to
404s in e-mails.

However, now that we have both project and group in the context, we
render the Markdown giving priority to the project context if is set.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/63910
This commit is contained in:
Stan Hu 2019-06-28 18:08:35 -07:00
parent df717efaab
commit 0e341a6e58
3 changed files with 45 additions and 3 deletions

View File

@ -0,0 +1,5 @@
---
title: Fix attachments using the wrong URLs in e-mails
merge_request: 30197
author:
type: fixed

View File

@ -56,10 +56,10 @@ module Banzai
def process_link_to_upload_attr(html_attr)
path_parts = [Addressable::URI.unescape(html_attr.value)]
if group
path_parts.unshift(relative_url_root, 'groups', group.full_path, '-')
elsif project
if project
path_parts.unshift(relative_url_root, project.full_path)
elsif group
path_parts.unshift(relative_url_root, 'groups', group.full_path, '-')
else
path_parts.unshift(relative_url_root)
end

View File

@ -50,6 +50,43 @@ describe MarkupHelper do
expect(markdown(actual, project: second_project)).to match(expected)
end
end
describe 'uploads' do
let(:text) { "![ImageTest](/uploads/test.png)" }
let(:group) { create(:group) }
subject { helper.markdown(text) }
describe 'inside a project' do
it 'renders uploads relative to project' do
expect(subject).to include("#{project.full_path}/uploads/test.png")
end
end
describe 'inside a group' do
before do
helper.instance_variable_set(:@group, group)
helper.instance_variable_set(:@project, nil)
end
it 'renders uploads relative to the group' do
expect(subject).to include("#{group.full_path}/-/uploads/test.png")
end
end
describe "with a group in the context" do
let(:project_in_group) { create(:project, group: group) }
before do
helper.instance_variable_set(:@group, group)
helper.instance_variable_set(:@project, project_in_group)
end
it 'renders uploads relative to project' do
expect(subject).to include("#{project_in_group.path_with_namespace}/uploads/test.png")
end
end
end
end
describe '#markdown_field' do