From f76eac56b9d7d4ae61010cddcca68682824b2239 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 18 Aug 2015 15:46:36 -0700 Subject: [PATCH] Reply by email POC --- .gitignore | 1 + Gemfile | 2 + Gemfile.lock | 8 +++ Procfile | 3 +- app/mailers/emails/issues.rb | 8 +++ app/mailers/emails/merge_requests.rb | 11 +++- app/mailers/emails/notes.rb | 6 +++ app/mailers/notify.rb | 54 ++++++++++++------- app/models/sent_notification.rb | 29 ++++++++++ app/workers/email_receiver_worker.rb | 22 ++++++++ config/gitlab.yml.example | 5 ++ config/initializers/1_settings.rb | 6 +++ config/mail_room.yml.example | 14 +++++ .../20150818213832_add_sent_notifications.rb | 13 +++++ db/schema.rb | 13 ++++- lib/gitlab/email_receiver.rb | 50 +++++++++++++++++ 16 files changed, 224 insertions(+), 21 deletions(-) create mode 100644 app/models/sent_notification.rb create mode 100644 app/workers/email_receiver_worker.rb create mode 100644 config/mail_room.yml.example create mode 100644 db/migrate/20150818213832_add_sent_notifications.rb create mode 100644 lib/gitlab/email_receiver.rb diff --git a/.gitignore b/.gitignore index 3e30fb8cf77..8a68bb3e4f0 100644 --- a/.gitignore +++ b/.gitignore @@ -25,6 +25,7 @@ config/initializers/rack_attack.rb config/initializers/smtp_settings.rb config/resque.yml config/unicorn.rb +config/mail_room.yml coverage/* db/*.sqlite3 db/*.sqlite3-journal diff --git a/Gemfile b/Gemfile index 8f65a274baa..e3f76671f5b 100644 --- a/Gemfile +++ b/Gemfile @@ -272,3 +272,5 @@ end gem "newrelic_rpm" gem 'octokit', '3.7.0' + +gem "mail_room", github: "DouweM/mail_room", branch: "sidekiq" diff --git a/Gemfile.lock b/Gemfile.lock index f0c661fa9c5..3aca2c8890b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,10 @@ +GIT + remote: git://github.com/DouweM/mail_room.git + revision: e1795b807f492533ad40afcb80abf870f1baddb5 + branch: sidekiq + specs: + mail_room (0.3.1) + GEM remote: https://rubygems.org/ specs: @@ -805,6 +812,7 @@ DEPENDENCIES jquery-ui-rails kaminari (~> 0.15.1) letter_opener + mail_room! minitest (~> 5.3.0) mousetrap-rails mysql2 diff --git a/Procfile b/Procfile index 799b92729fa..51154f150d1 100644 --- a/Procfile +++ b/Procfile @@ -1,2 +1,3 @@ web: bundle exec unicorn_rails -p ${PORT:="3000"} -E ${RAILS_ENV:="development"} -c ${UNICORN_CONFIG:="config/unicorn.rb"} -worker: bundle exec sidekiq -q post_receive -q mailer -q archive_repo -q system_hook -q project_web_hook -q gitlab_shell -q common -q default +worker: bundle exec sidekiq -q post_receive -q mailer -q archive_repo -q system_hook -q project_web_hook -q gitlab_shell -q incoming_email -q common -q default +mail_room: bundle exec mail_room -c config/mail_room.yml diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb index 687bac3aa31..c8b7775c328 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})")) + + sent_notification!(@issue, recipient_id) 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})")) + + sent_notification!(@issue, recipient_id) 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})")) + + sent_notification!(@issue, recipient_id) 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})")) + + sent_notification!(@issue, recipient_id) end end end diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb index 512a8f7ea6b..7a685c04ebc 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})")) + + sent_notification!(@merge_request, recipient_id) 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})")) + + sent_notification!(@merge_request, recipient_id) 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})")) + + sent_notification!(@merge_request, recipient_id) 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})")) + + sent_notification!(@merge_request, recipient_id) end def merge_request_status_email(recipient_id, merge_request_id, status, updated_by_user_id) @@ -58,11 +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}")) + + sent_notification!(@merge_request, recipient_id) end end diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb index ff251209e01..2612074a5ff 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})")) + + sent_notification!(@commit, recipient_id) 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})")) + + sent_notification!(@issue, recipient_id) 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})")) + + sent_notification!(@merge_request, recipient_id) end end end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 79fb48b00d3..44df3d6407d 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -46,6 +46,17 @@ class Notify < ActionMailer::Base allowed_domains end + def sent_notification!(noteable, recipient_id) + return unless reply_key + + SentNotification.create( + project: noteable.project, + noteable: noteable, + recipient_id: recipient_id, + reply_key: reply_key + ) + end + private # The default email address to send emails from @@ -85,14 +96,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 @@ -130,10 +133,23 @@ class Notify < ActionMailer::Base # 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 = {}) + 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 + headers['Message-ID'] = message_id(model) - headers['X-GitLab-Project'] = "#{@project.name} | " if @project - mail(headers, &block) + + if reply_key + headers['X-GitLab-Reply-Key'] = reply_key + headers['Reply-To'] = Gitlab.config.reply_by_email.address.gsub('%{reply_key}', reply_key) + end + + mail(headers) end # Send an email that responds to an existing conversation thread, @@ -144,19 +160,21 @@ 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 - - mail(headers, &block) + mail_new_thread(model, headers) end def can? Ability.abilities.allowed?(user, action, subject) end + + def reply_key + return nil unless Gitlab.config.reply_by_email.enabled + + @reply_key ||= SecureRandom.hex(16) + end end diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb new file mode 100644 index 00000000000..a3d24669b52 --- /dev/null +++ b/app/models/sent_notification.rb @@ -0,0 +1,29 @@ +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, if: ->(n) { n.noteable_type.present? && n.noteable_type != 'Commit' } + validates :commit_id, presence: true, if: ->(n) { n.noteable_type == 'Commit' } + + def self.for(reply_key) + find_by(reply_key: reply_key) + end + + def for_commit? + noteable_type == "Commit" + end + + def noteable + if for_commit? + project.commit(commit_id) + else + super + end + rescue + nil + end +end diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb new file mode 100644 index 00000000000..e44a430f6bc --- /dev/null +++ b/app/workers/email_receiver_worker.rb @@ -0,0 +1,22 @@ +class EmailReceiverWorker + include Sidekiq::Worker + + sidekiq_options queue: :incoming_email + + def perform(raw) + return unless Gitlab.config.reply_by_email.enabled + + # begin + Gitlab::EmailReceiver.new(raw).process + # rescue => e + # handle_failure(raw, e) + # end + end + + private + + def handle_failure(raw, e) + # TODO: Handle better. + Rails.logger.warn("Email can not be processed: #{e}\n\n#{raw}") + end +end diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 56770335ddc..e78429b29a3 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -94,6 +94,11 @@ production: &base # The default is 'tmp/repositories' relative to the root of the Rails app. # repository_downloads_path: tmp/repositories + ## Reply by email + reply_by_email: + enabled: false + address: "replies+%{reply_key}@gitlab.example.com" + ## Gravatar ## For Libravatar see: http://doc.gitlab.com/ce/customization/libravatar.html gravatar: diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 026c1a5792c..9e83660e103 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -149,6 +149,12 @@ Settings.gitlab.default_projects_features['visibility_level'] = Settings.send Settings.gitlab['repository_downloads_path'] = File.absolute_path(Settings.gitlab['repository_downloads_path'] || 'tmp/repositories', Rails.root) Settings.gitlab['restricted_signup_domains'] ||= [] +# +# Reply by email +# +Settings['reply_by_email'] ||= Settingslogic.new({}) +Settings.reply_by_email['enabled'] = false if Settings.gravatar['enabled'].nil? + # # Gravatar # diff --git a/config/mail_room.yml.example b/config/mail_room.yml.example new file mode 100644 index 00000000000..a7a6a25362e --- /dev/null +++ b/config/mail_room.yml.example @@ -0,0 +1,14 @@ +:mailboxes: + # - + # :host: "imap.gmail.com" + # :port: 993 + # :ssl: true + # :email: "replies@gitlab.example.com" + # :password: "password" + # :name: "inbox" + # :delivery_method: sidekiq + # :delivery_options: + # :redis_url: redis://localhost:6379 + # :namespace: resque:gitlab + # :queue: incoming_email + # :worker: EmailReceiverWorker diff --git a/db/migrate/20150818213832_add_sent_notifications.rb b/db/migrate/20150818213832_add_sent_notifications.rb new file mode 100644 index 00000000000..43e8d6a1a82 --- /dev/null +++ b/db/migrate/20150818213832_add_sent_notifications.rb @@ -0,0 +1,13 @@ +class AddSentNotifications < ActiveRecord::Migration + def change + create_table :sent_notifications do |t| + t.references :project + t.references :noteable, polymorphic: true + t.references :recipient + t.string :commit_id + t.string :reply_key, null: false + end + + add_index :sent_notifications, :reply_key, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 6e919f2883b..0827e810f18 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150806104937) do +ActiveRecord::Schema.define(version: 20150818213832) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -404,6 +404,17 @@ ActiveRecord::Schema.define(version: 20150806104937) do add_index "protected_branches", ["project_id"], name: "index_protected_branches_on_project_id", using: :btree + create_table "sent_notifications", force: true do |t| + t.integer "project_id" + t.integer "noteable_id" + t.string "noteable_type" + t.integer "recipient_id" + t.string "commit_id" + t.string "reply_key", null: false + end + + add_index "sent_notifications", ["reply_key"], name: "index_sent_notifications_on_reply_key", unique: true, using: :btree + create_table "services", force: true do |t| t.string "type" t.string "title" diff --git a/lib/gitlab/email_receiver.rb b/lib/gitlab/email_receiver.rb new file mode 100644 index 00000000000..a9f67bb5da0 --- /dev/null +++ b/lib/gitlab/email_receiver.rb @@ -0,0 +1,50 @@ +module Gitlab + class EmailReceiver + def initialize(raw) + @raw = raw + end + + def message + @message ||= Mail::Message.new(@raw) + end + + def process + return unless message && sent_notification + + Notes::CreateService.new( + sent_notification.project, + sent_notification.recipient, + note: message.text_part.to_s, + noteable_type: sent_notification.noteable_type, + noteable_id: sent_notification.noteable_id, + commit_id: sent_notification.commit_id + ).execute + end + + private + + def reply_key + address = Gitlab.config.reply_by_email.address + return nil unless address + + regex = Regexp.escape(address) + regex = regex.gsub(Regexp.escape('%{reply_key}'), "(.*)") + regex = Regexp.new(regex) + + address = message.to.find { |address| address =~ regex } + return nil unless address + + match = address.match(regex) + + return nil unless match && match[1].present? + + match[1] + end + + def sent_notification + return nil unless reply_key + + SentNotification.for(reply_key) + end + end +end