Merge branch 'reply-by-email' into 'master'

Reply by email

Fixes #1360.

It's far from done, but _it works_.

See merge request !1173

14 files changed, 228 insertions, 87 deletions

diff --git a/app/helpers/icons_helper.rb b/app/helpers/icons_helper.rb
index 30b17a736a7..1cf5b96481a 100644
--- a/app/helpers/icons_helper.rb
+++ b/app/helpers/icons_helper.rb
@@ -20,7 +20,7 @@ module IconsHelper
   end
 
   def boolean_to_icon(value)
-    if value.to_s == "true"
+    if value
       icon('circle', class: 'cgreen')
     else
       icon('power-off', class: 'clgray')
diff --git a/app/mailers/base_mailer.rb b/app/mailers/base_mailer.rb
new file mode 100644
index 00000000000..aedb0889185
--- /dev/null
+++ b/app/mailers/base_mailer.rb
@@ -0,0 +1,32 @@
+class BaseMailer < ActionMailer::Base
+  add_template_helper ApplicationHelper
+  add_template_helper GitlabMarkdownHelper
+
+  attr_accessor :current_user
+  helper_method :current_user, :can?
+
+  default from:     Proc.new { default_sender_address.format }
+  default reply_to: Proc.new { default_reply_to_address.format }
+
+  def self.delay
+    delay_for(2.seconds)
+  end
+
+  def can?
+    Ability.abilities.allowed?(current_user, action, subject)
+  end
+
+  private
+
+  def default_sender_address
+    address = Mail::Address.new(Gitlab.config.gitlab.email_from)
+    address.display_name = Gitlab.config.gitlab.email_display_name
+    address
+  end
+
+  def default_reply_to_address
+    address = Mail::Address.new(Gitlab.config.gitlab.email_reply_to)
+    address.display_name = Gitlab.config.gitlab.email_display_name
+    address
+  end
+end
diff --git a/app/mailers/email_rejection_mailer.rb b/app/mailers/email_rejection_mailer.rb
new file mode 100644
index 00000000000..89aceda82d1
--- /dev/null
+++ b/app/mailers/email_rejection_mailer.rb
@@ -0,0 +1,19 @@
+class EmailRejectionMailer < BaseMailer
+  def rejection(reason, original_raw, can_retry = false)
+    @reason = reason
+    @original_message = Mail::Message.new(original_raw)
+
+    headers = {
+      to: @original_message.from,
+      subject: "[Rejected] #{@original_message.subject}"
+    }
+
+    headers['Message-ID'] = SecureRandom.hex
+    headers['In-Reply-To'] = @original_message.message_id
+    headers['References'] = @original_message.message_id
+
+    headers['Reply-To'] = @original_message.to.first if can_retry
+
+    mail(headers)
+  end
+end
diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb
index 687bac3aa31..2c035fbb70b 100644
--- a/app/mailers/emails/issues.rb
+++ b/app/mailers/emails/issues.rb
@@ -8,6 +8,8 @@ module Emails
                       from: sender(@issue.author_id),
                       to: recipient(recipient_id),
                       subject: subject("#{@issue.title} (##{@issue.iid})"))
+
+      SentNotification.record(@issue, recipient_id, reply_key)
     end
 
     def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id, updated_by_user_id)
@@ -19,6 +21,8 @@ module Emails
                          from: sender(updated_by_user_id),
                          to: recipient(recipient_id),
                          subject: subject("#{@issue.title} (##{@issue.iid})"))
+
+      SentNotification.record(@issue, recipient_id, reply_key)
     end
 
     def closed_issue_email(recipient_id, issue_id, updated_by_user_id)
@@ -30,6 +34,8 @@ module Emails
                          from: sender(updated_by_user_id),
                          to: recipient(recipient_id),
                          subject: subject("#{@issue.title} (##{@issue.iid})"))
+
+      SentNotification.record(@issue, recipient_id, reply_key)
     end
 
     def issue_status_changed_email(recipient_id, issue_id, status, updated_by_user_id)
@@ -42,6 +48,8 @@ module Emails
                          from: sender(updated_by_user_id),
                          to: recipient(recipient_id),
                          subject: subject("#{@issue.title} (##{@issue.iid})"))
+
+      SentNotification.record(@issue, recipient_id, reply_key)
     end
   end
 end
diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb
index 512a8f7ea6b..7923fb770d0 100644
--- a/app/mailers/emails/merge_requests.rb
+++ b/app/mailers/emails/merge_requests.rb
@@ -10,6 +10,8 @@ module Emails
                       from: sender(@merge_request.author_id),
                       to: recipient(recipient_id),
                       subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
+
+      SentNotification.record(@merge_request, recipient_id, reply_key)
     end
 
     def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id)
@@ -23,6 +25,8 @@ module Emails
                          from: sender(updated_by_user_id),
                          to: recipient(recipient_id),
                          subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
+
+      SentNotification.record(@merge_request, recipient_id, reply_key)
     end
 
     def closed_merge_request_email(recipient_id, merge_request_id, updated_by_user_id)
@@ -36,6 +40,8 @@ module Emails
                          from: sender(updated_by_user_id),
                          to: recipient(recipient_id),
                          subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
+
+      SentNotification.record(@merge_request, recipient_id, reply_key)
     end
 
     def merged_merge_request_email(recipient_id, merge_request_id, updated_by_user_id)
@@ -48,6 +54,8 @@ module Emails
                          from: sender(updated_by_user_id),
                          to: recipient(recipient_id),
                          subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
+
+      SentNotification.record(@merge_request, recipient_id, reply_key)
     end
 
     def merge_request_status_email(recipient_id, merge_request_id, status, updated_by_user_id)
@@ -58,52 +66,12 @@ module Emails
       @target_url = namespace_project_merge_request_url(@project.namespace,
                                                         @project,
                                                         @merge_request)
-      set_reference("merge_request_#{merge_request_id}")
       mail_answer_thread(@merge_request,
                          from: sender(updated_by_user_id),
                          to: recipient(recipient_id),
-                         subject: subject("#{@merge_request.title} (##{@merge_request.iid}) #{@mr_status}"))
-    end
-  end
+                         subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
 
-  # Over rides default behaviour to show source/target
-  # Formats arguments into a String suitable for use as an email subject
-  #
-  # extra - Extra Strings to be inserted into the subject
-  #
-  # Examples
-  #
-  #   >> subject('Lorem ipsum')
-  #   => "GitLab Merge Request | Lorem ipsum"
-  #
-  #   # Automatically inserts Project name:
-  #   Forked MR
-  #   => source project => <Project id: 1, name: "Ruby on Rails", path: "ruby_on_rails", ...>
-  #   => target project => <Project id: 2, name: "My Ror", path: "ruby_on_rails", ...>
-  #   => source branch => source
-  #   => target branch => target
-  #   >> subject('Lorem ipsum')
-  #   => "GitLab Merge Request | Ruby on Rails:source >> My Ror:target | Lorem ipsum "
-  #
-  #   Non Forked MR
-  #   => source project => <Project id: 1, name: "Ruby on Rails", path: "ruby_on_rails", ...>
-  #   => target project => <Project id: 1, name: "Ruby on Rails", path: "ruby_on_rails", ...>
-  #   => source branch => source
-  #   => target branch => target
-  #   >> subject('Lorem ipsum')
-  #   => "GitLab Merge Request | Ruby on Rails | source >> target | Lorem ipsum "
-  #   # Accepts multiple arguments
-  #   >> subject('Lorem ipsum', 'Dolor sit amet')
-  #   => "GitLab Merge Request | Lorem ipsum | Dolor sit amet"
-  def subject(*extra)
-    subject = "Merge Request | "
-    if @merge_request.for_fork?
-      subject << "#{@merge_request.source_project.name_with_namespace}:#{merge_request.source_branch} >> #{@merge_request.target_project.name_with_namespace}:#{merge_request.target_branch}"
-    else
-      subject << "#{@merge_request.source_project.name_with_namespace} | #{merge_request.source_branch} >> #{merge_request.target_branch}"
+      SentNotification.record(@merge_request, recipient_id, reply_key)
     end
-    subject << " | " + extra.join(' | ') if extra.present?
-    subject
   end
-
 end
diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb
index ff251209e01..63d4aca61af 100644
--- a/app/mailers/emails/notes.rb
+++ b/app/mailers/emails/notes.rb
@@ -11,6 +11,8 @@ module Emails
                          from: sender(@note.author_id),
                          to: recipient(recipient_id),
                          subject: subject("#{@commit.title} (#{@commit.short_id})"))
+
+      SentNotification.record(@commit, recipient_id, reply_key)
     end
 
     def note_issue_email(recipient_id, note_id)
@@ -24,6 +26,8 @@ module Emails
                          from: sender(@note.author_id),
                          to: recipient(recipient_id),
                          subject: subject("#{@issue.title} (##{@issue.iid})"))
+
+      SentNotification.record(@issue, recipient_id, reply_key)
     end
 
     def note_merge_request_email(recipient_id, note_id)
@@ -38,6 +42,8 @@ module Emails
                          from: sender(@note.author_id),
                          to: recipient(recipient_id),
                          subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
+
+      SentNotification.record(@merge_request, recipient_id, reply_key)
     end
   end
 end
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 79fb48b00d3..5717c89e61d 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -1,4 +1,4 @@
-class Notify < ActionMailer::Base
+class Notify < BaseMailer
   include ActionDispatch::Routing::PolymorphicRoutes
 
   include Emails::Issues
@@ -8,22 +8,9 @@ class Notify < ActionMailer::Base
   include Emails::Profile
   include Emails::Groups
 
-  add_template_helper ApplicationHelper
-  add_template_helper GitlabMarkdownHelper
   add_template_helper MergeRequestsHelper
   add_template_helper EmailsHelper
 
-  attr_accessor :current_user
-  helper_method :current_user, :can?
-
-  default from: Proc.new { default_sender_address.format }
-  default reply_to: Gitlab.config.gitlab.email_reply_to
-
-  # Just send email with 2 seconds delay
-  def self.delay
-    delay_for(2.seconds)
-  end
-
   def test_email(recipient_email, subject, body)
     mail(to: recipient_email,
          subject: subject,
@@ -48,13 +35,6 @@ class Notify < ActionMailer::Base
 
   private
 
-  # The default email address to send emails from
-  def default_sender_address
-    address = Mail::Address.new(Gitlab.config.gitlab.email_from)
-    address.display_name = Gitlab.config.gitlab.email_display_name
-    address
-  end
-
   def can_send_from_user_email?(sender)
     sender_domain = sender.email.split("@").last
     self.class.allowed_email_domains.include?(sender_domain)
@@ -85,14 +65,6 @@ class Notify < ActionMailer::Base
     @current_user.notification_email
   end
 
-  # Set the References header field
-  #
-  # local_part - The local part of the referenced message ID
-  #
-  def set_reference(local_part)
-    headers["References"] = "<#{local_part}@#{Gitlab.config.gitlab.host}>"
-  end
-
   # Formats arguments into a String suitable for use as an email subject
   #
   # extra - Extra Strings to be inserted into the subject
@@ -126,14 +98,37 @@ class Notify < ActionMailer::Base
     "<#{model_name}_#{model.id}@#{Gitlab.config.gitlab.host}>"
   end
 
+  def mail_thread(model, headers = {})
+    if @project
+      headers['X-GitLab-Project'] = @project.name 
+      headers['X-GitLab-Project-Id'] = @project.id
+      headers['X-GitLab-Project-Path'] = @project.path_with_namespace
+    end
+
+    headers["X-GitLab-#{model.class.name}-ID"] = model.id
+
+    if reply_key
+      headers['X-GitLab-Reply-Key'] = reply_key
+
+      address = Mail::Address.new(Gitlab::ReplyByEmail.reply_address(reply_key))
+      address.display_name = @project.name_with_namespace
+
+      headers['Reply-To'] = address
+
+      @reply_by_email = true
+    end
+
+    mail(headers)
+  end
+
   # Send an email that starts a new conversation thread,
   # with headers suitable for grouping by thread in email clients.
   #
   # See: mail_answer_thread
-  def mail_new_thread(model, headers = {}, &block)
+  def mail_new_thread(model, headers = {})
     headers['Message-ID'] = message_id(model)
-    headers['X-GitLab-Project'] = "#{@project.name} | " if @project
-    mail(headers, &block)
+
+    mail_thread(model, headers)
   end
 
   # Send an email that responds to an existing conversation thread,
@@ -144,19 +139,17 @@ class Notify < ActionMailer::Base
   #  * have a subject that begin by 'Re: '
   #  * have a 'In-Reply-To' or 'References' header that references the original 'Message-ID'
   #
-  def mail_answer_thread(model, headers = {}, &block)
+  def mail_answer_thread(model, headers = {})
+    headers['Message-ID'] = SecureRandom.hex
     headers['In-Reply-To'] = message_id(model)
     headers['References'] = message_id(model)
-    headers['X-GitLab-Project'] = "#{@project.name} | " if @project
 
-    if headers[:subject]
-      headers[:subject].prepend('Re: ')
-    end
+    headers[:subject].prepend('Re: ') if headers[:subject]
 
-    mail(headers, &block)
+    mail_thread(model, headers)
   end
 
-  def can?
-    Ability.abilities.allowed?(user, action, subject)
+  def reply_key
+    @reply_key ||= Gitlab::ReplyByEmail.reply_key
   end
 end
diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb
new file mode 100644
index 00000000000..460ca40be3f
--- /dev/null
+++ b/app/models/sent_notification.rb
@@ -0,0 +1,50 @@
+class SentNotification < ActiveRecord::Base
+  belongs_to :project
+  belongs_to :noteable, polymorphic: true
+  belongs_to :recipient, class_name: "User"
+
+  validate :project, :recipient, :reply_key, presence: true
+  validate :reply_key, uniqueness: true
+
+  validates :noteable_id, presence: true, unless: :for_commit?
+  validates :commit_id, presence: true, if: :for_commit?
+
+  class << self
+    def for(reply_key)
+      find_by(reply_key: reply_key)
+    end
+
+    def record(noteable, recipient_id, reply_key)
+      return unless reply_key
+
+      noteable_id = nil
+      commit_id = nil
+      if noteable.is_a?(Commit)
+        commit_id = noteable.id
+      else
+        noteable_id = noteable.id
+      end
+
+      create(
+        project:        noteable.project,
+        noteable_type:  noteable.class.name,
+        noteable_id:    noteable_id,
+        commit_id:      commit_id,
+        recipient_id:   recipient_id,
+        reply_key:      reply_key
+      )
+    end
+  end
+
+  def for_commit?
+    noteable_type == "Commit"
+  end
+
+  def noteable
+    if for_commit?
+      project.commit(commit_id) rescue nil
+    else
+      super
+    end
+  end
+end
diff --git a/app/services/projects/upload_service.rb b/app/services/projects/upload_service.rb
index 992a7a7a1dc..279550d6f4a 100644
--- a/app/services/projects/upload_service.rb
+++ b/app/services/projects/upload_service.rb
@@ -13,9 +13,9 @@ module Projects
       filename = uploader.image? ? uploader.file.basename : uploader.file.filename
 
       {
-        'alt'       => filename,
-        'url'       => uploader.secure_url,
-        'is_image'  => uploader.image?
+        alt:      filename,
+        url:      uploader.secure_url,
+        is_image: uploader.image?
       }
     end
 
diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml
index 3732ff847b9..54191aadda6 100644
--- a/app/views/admin/dashboard/index.html.haml
+++ b/app/views/admin/dashboard/index.html.haml
@@ -55,6 +55,10 @@
         OmniAuth
         %span.light.pull-right
           = boolean_to_icon Gitlab.config.omniauth.enabled
+      %p
+        Reply by email
+        %span.light.pull-right
+          = boolean_to_icon Gitlab::ReplyByEmail.enabled?
     .col-md-4
       %h4
         Components
diff --git a/app/views/email_rejection_mailer/rejection.html.haml b/app/views/email_rejection_mailer/rejection.html.haml
new file mode 100644
index 00000000000..7f7d841fe21
--- /dev/null
+++ b/app/views/email_rejection_mailer/rejection.html.haml
@@ -0,0 +1,4 @@
+%p
+  Unfortunately, your email message to GitLab could not be processed.
+
+= markdown @reason
diff --git a/app/views/email_rejection_mailer/rejection.text.haml b/app/views/email_rejection_mailer/rejection.text.haml
new file mode 100644
index 00000000000..6693e6f90e8
--- /dev/null
+++ b/app/views/email_rejection_mailer/rejection.text.haml
@@ -0,0 +1,4 @@
+Unfortunately, your email message to GitLab could not be processed.
+
+
+= @reason
diff --git a/app/views/layouts/notify.html.haml b/app/views/layouts/notify.html.haml
index c8662a15adb..ec209c38eed 100644
--- a/app/views/layouts/notify.html.haml
+++ b/app/views/layouts/notify.html.haml
@@ -36,7 +36,11 @@
         &mdash;
         %br
         - if @target_url
-          #{link_to "View it on GitLab", @target_url}
+          - if @reply_by_email
+            Reply to this email directly or
+            #{link_to "view it on GitLab", @target_url}.
+          - else
+            #{link_to "View it on GitLab", @target_url}
           = email_action @target_url
         - if @project && !@disable_footer
           You're receiving this notification because you are a member of the #{link_to_unless @target_url, @project.name_with_namespace, namespace_project_url(@project.namespace, @project)} project team.
diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb
new file mode 100644
index 00000000000..a588a1f45ee
--- /dev/null
+++ b/app/workers/email_receiver_worker.rb
@@ -0,0 +1,49 @@
+class EmailReceiverWorker
+  include Sidekiq::Worker
+
+  sidekiq_options queue: :incoming_email
+
+  def perform(raw)
+    return unless Gitlab::ReplyByEmail.enabled?
+
+    begin
+      Gitlab::Email::Receiver.new(raw).execute
+    rescue => e
+      handle_failure(raw, e)
+    end
+  end
+
+  private
+
+  def handle_failure(raw, e)
+    Rails.logger.warn("Email can not be processed: #{e}\n\n#{raw}")
+
+    can_retry = false
+    reason = nil
+
+    case e
+    when Gitlab::Email::Receiver::SentNotificationNotFoundError
+      reason = "We couldn't figure out what the email is in reply to. Please create your comment through the web interface."
+    when Gitlab::Email::Receiver::EmptyEmailError
+      can_retry = true
+      reason = "It appears that the email is blank. Make sure your reply is at the top of the email, we can't process inline replies."
+    when Gitlab::Email::Receiver::AutoGeneratedEmailError
+      reason = "The email was marked as 'auto generated', which we can't accept. Please create your comment through the web interface."
+    when Gitlab::Email::Receiver::UserNotFoundError
+      reason = "We couldn't figure out what user corresponds to the email. Please create your comment through the web interface."
+    when Gitlab::Email::Receiver::UserBlockedError
+      reason = "Your account has been blocked. If you believe this is in error, contact a staff member."
+    when Gitlab::Email::Receiver::UserNotAuthorizedError
+      reason = "You are not allowed to respond to the thread you are replying to. If you believe this is in error, contact a staff member."
+    when Gitlab::Email::Receiver::NoteableNotFoundError
+      reason = "The thread you are replying to no longer exists, perhaps it was deleted? If you believe this is in error, contact a staff member."
+    when Gitlab::Email::Receiver::InvalidNoteError
+      can_retry = true
+      reason = e.message
+    else
+      return
+    end
+
+    EmailRejectionMailer.delay.rejection(reason, raw, can_retry)
+  end
+end