diff --git a/CHANGELOG b/CHANGELOG
index 3b263a7d4faf74f7f25a256c600dc6207f02e0e1..e1e22102ed536d62084c45486cf2e7f4394a8a51 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 92cc9c426b89177dba22bdd3b6d81705894cd7bb..08476f8516e8265af3ef0f1c374b732de0301863 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 5272dfa0edec4ce0e6fc0ca80b9dbd254b806f03..778a78acf56b83ee70d7392d18dc013b243b977a 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 f2f8eee18c49cd18b374ba822a38db2610bc443f..03cbee94608285950a47d037a6e33e4a783c970b 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 f02d5111b22c333ed48c0c3c36041e6f86da5431..729a7bb505d3ae8829121bc25fbc341edf994316 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 b3c507ccbe2aee13c2c2f073c5f3cf0b073c78c4..e3a3b542358e238de5fb649045c9a7193a867287 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 }