diff options
author | Hannes Rosenögger <123haynes@gmail.com> | 2015-03-04 12:58:40 +0100 |
---|---|---|
committer | Hannes Rosenögger <123haynes@gmail.com> | 2015-03-12 09:26:06 +0100 |
commit | 3c7e0f45c2d9c242bf45d153bb73e96ce7525a06 (patch) | |
tree | a905c2a07656dcb1e287b4bdf47b3b118bc36017 | |
parent | 3f823068e1f6e3e88d6631de60d9aaf9ecd5e6f9 (diff) | |
download | gitlab-ce-3c7e0f45c2d9c242bf45d153bb73e96ce7525a06.tar.gz |
replace images in emails with inline images
This adds the functionality of replacing all images that were uploaded
to gitlab with inline images(base64) in emails.
This change fixes the broken images in emails that 7.8 introduced
-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: "![test](#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/12345/test.jpg)") + 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: "![test](#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/12345/test.jpg)") + 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: "![test](#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/12345/test.jpg)") + 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 } |