diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/helpers/emails_helper.rb | 25 | ||||
| -rw-r--r-- | app/views/notify/_note_message.html.haml | 2 | ||||
| -rw-r--r-- | app/views/notify/new_issue_email.html.haml | 2 | ||||
| -rw-r--r-- | app/views/notify/new_merge_request_email.html.haml | 2 | ||||
| -rw-r--r-- | spec/mailers/notify_spec.rb | 82 |
6 files changed, 108 insertions, 6 deletions
diff --git a/CHANGELOG b/CHANGELOG index 3b263a7d4fa..e1e22102ed5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.9.0 (unreleased) + - Fix broken email images (Hannes Rosenögger) - Fix mass SQL statements on initial push (Hannes Rosenögger) - Add tag push notifications and normalize HipChat and Slack messages to be consistent (Stan Hu) - Add comment notification events to HipChat and Slack services (Stan Hu) diff --git a/app/helpers/emails_helper.rb b/app/helpers/emails_helper.rb index 92cc9c426b8..08476f8516e 100644 --- a/app/helpers/emails_helper.rb +++ b/app/helpers/emails_helper.rb @@ -1,3 +1,6 @@ +require 'html/pipeline' +require 'html/pipeline/gitlab' + module EmailsHelper # Google Actions @@ -39,4 +42,26 @@ module EmailsHelper lexer = Rugments::Lexers::Diff.new raw formatter.format(lexer.lex(diffcontent)) end + + def replace_image_links_with_base64(text, project) + # Used pipelines in GitLab: + # GitlabEmailImageFilter - replaces images that have been uploaded as attachments with inline images in emails. + # + # see https://gitlab.com/gitlab-org/html-pipeline-gitlab for more filters + filters = [ + HTML::Pipeline::Gitlab::GitlabEmailImageFilter + ] + + context = { + base_url: File.join(Gitlab.config.gitlab.url, project.path_with_namespace, 'uploads'), + upload_path: File.join(Rails.root, 'public', 'uploads', project.path_with_namespace), + } + + pipeline = HTML::Pipeline::Gitlab.new(filters).pipeline + + result = pipeline.call(text, context) + text = result[:output].to_html(save_with: 0) + + text.html_safe + end end diff --git a/app/views/notify/_note_message.html.haml b/app/views/notify/_note_message.html.haml index 5272dfa0ede..778a78acf56 100644 --- a/app/views/notify/_note_message.html.haml +++ b/app/views/notify/_note_message.html.haml @@ -1,2 +1,2 @@ %div - = markdown(@note.note) + = replace_image_links_with_base64(markdown(@note.note), @note.project) diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml index f2f8eee18c4..03cbee94608 100644 --- a/app/views/notify/new_issue_email.html.haml +++ b/app/views/notify/new_issue_email.html.haml @@ -1,5 +1,5 @@ -if @issue.description - = markdown(@issue.description) + = replace_image_links_with_base64(markdown(@issue.description), @issue.project) - if @issue.assignee_id.present? %p diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index f02d5111b22..729a7bb505d 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -6,4 +6,4 @@ Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name} -if @merge_request.description - = markdown(@merge_request.description) + = replace_image_links_with_base64(markdown(@merge_request.description), @merge_request.project) diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index b3c507ccbe2..e3a3b542358 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -183,6 +183,13 @@ describe Notify do context 'for issues' do let(:issue) { create(:issue, author: current_user, assignee: assignee, project: project) } let(:issue_with_description) { create(:issue, author: current_user, assignee: assignee, project: project, description: Faker::Lorem.sentence) } + let(:issue_with_image) do + create(:issue, + author: current_user, + assignee: assignee, + project: project, + description: "") + end describe 'that are new' do subject { Notify.new_issue_email(issue.assignee_id, issue.id) } @@ -207,6 +214,22 @@ describe Notify do end end + describe 'that contain images' do + let(:png) { File.read("#{Rails.root}/spec/fixtures/dk.png") } + let(:png_encoded) { Base64::encode64(png) } + + before :each do + file_path = File.join(Rails.root, 'public', 'uploads', issue_with_image.project.path_with_namespace, '12345/test.jpg') + allow(File).to receive(:file?).with(file_path).and_return(true) + allow(File).to receive(:read).with(file_path).and_return(png) + end + + subject { Notify.new_issue_email(issue_with_image.assignee_id, issue_with_image.id) } + it 'replaces attached images with inline images' do + is_expected.to have_body_text URI.encode(png_encoded) + end + end + describe 'that have been reassigned' do subject { Notify.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id, current_user) } @@ -271,6 +294,14 @@ describe Notify do let(:merge_author) { create(:user) } let(:merge_request) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project) } let(:merge_request_with_description) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project, description: Faker::Lorem.sentence) } + let(:merge_request_with_image) do + create(:merge_request, + author: current_user, + assignee: assignee, + source_project: project, + target_project: project, + description: "") + end describe 'that are new' do subject { Notify.new_merge_request_email(merge_request.assignee_id, merge_request.id) } @@ -307,6 +338,22 @@ describe Notify do end end + describe 'that are new and contain contain images in the description' do + let(:png) {File.read("#{Rails.root}/spec/fixtures/dk.png")} + let(:png_encoded) { Base64::encode64(png) } + + before :each do + file_path = File.join(Rails.root, 'public', 'uploads', merge_request_with_image.project.path_with_namespace, '/12345/test.jpg') + allow(File).to receive(:file?).with(file_path).and_return(true) + allow(File).to receive(:read).with(file_path).and_return(png) + end + + subject { Notify.new_merge_request_email(merge_request_with_image.assignee_id, merge_request_with_image.id) } + it 'replaces attached images with inline images' do + is_expected.to have_body_text URI.encode(png_encoded) + end + end + describe 'that are reassigned' do subject { Notify.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) } @@ -415,9 +462,12 @@ describe Notify do describe 'project access changed' do let(:project) { create(:project) } let(:user) { create(:user) } - let(:project_member) { create(:project_member, - project: project, - user: user) } + let(:project_member) do + create(:project_member, + project: project, + user: user) + end + subject { Notify.project_access_granted_email(project_member.id) } it_behaves_like 'an email sent from GitLab' @@ -457,6 +507,32 @@ describe Notify do end end + describe 'on a commit that contains an image' do + let(:commit) { project.repository.commit } + let(:note_with_image) do + create(:note, + project: project, + author: note_author, + note: "") + end + + let(:png) {File.read("#{Rails.root}/spec/fixtures/dk.png")} + let(:png_encoded) { Base64::encode64(png) } + + before :each do + file_path = File.join(Rails.root, 'public', 'uploads', note_with_image.project.path_with_namespace, '12345/test.jpg') + allow(File).to receive(:file?).with(file_path).and_return(true) + allow(File).to receive(:read).with(file_path).and_return(png) + allow(Note).to receive(:find).with(note_with_image.id).and_return(note_with_image) + allow(note_with_image).to receive(:noteable).and_return(commit) + end + + subject { Notify.note_commit_email(recipient.id, note_with_image.id) } + it 'replaces attached images with inline images' do + is_expected.to have_body_text URI.encode(png_encoded) + end + end + describe 'on a commit' do let(:commit) { project.repository.commit } |