Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
  • This project
    • Loading...
  • Sign in / Register
GitLab Community Edition
GitLab Community Edition
  • Overview
    • Overview
    • Details
    • Activity
    • Cycle Analytics
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
    • Locked Files
  • Issues 10,409
    • Issues 10,409
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
  • Merge Requests 553
    • Merge Requests 553
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Charts
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • GitLab.org
  • GitLab Community EditionGitLab Community Edition
  • Merge Requests
  • !1173

Merged
Opened Aug 18, 2015 by Douwe Maan@DouweM 6 of 8 tasks completed6/8 tasks
  • Report abuse
Report abuse

Reply by email

Fixes #1360 (closed).

It's far from done, but it works.

  • Add attachments
  • Reply with error message
  • Documentation for source
  • Update init scripts
  • Verify updated init scripts actually work
  • Tests
  • Integration with omnibus
  • Documentation for omnibus

cc @dzaporozhets @rspeicher

  • Discussion 48
  • Commits 53
  • Pipelines 40
  • Changes
{{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved
  • Douwe Maan @DouweM

    mentioned in issue #1360 (closed)

    Aug 18, 2015

    mentioned in issue #1360 (closed)

    mentioned in issue #1360
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • a428838d - Update mail_room
    Aug 18, 2015

    Added 1 commit:

    • a428838d - Update mail_room
    Added 1 commit: * a428838d - Update mail_room
    Toggle commit list
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on an old version of the diff Aug 18, 2015
    app/models/sent_notification.rb 0 → 100644
    Unable to load the diff.
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 18, 2015
      Owner

      Can we re-use the for_commit? method in place of the proc?

      Can we re-use the `for_commit`? method in place of the proc?
    Please register or sign in to reply
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on an old version of the diff Aug 18, 2015
    app/models/sent_notification.rb 0 → 100644
    Unable to load the diff.
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 18, 2015
      Owner

      Find out what we're rescuing from here.

      Find out what we're rescuing from here.
    Please register or sign in to reply
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on an old version of the diff Aug 18, 2015
    app/mailers/notify.rb
    Unable to load the diff.
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 18, 2015
      Owner

      Think about extracting all reply key-related replacing/extracting into its own class.

      Think about extracting all reply key-related replacing/extracting into its own class.
    Please register or sign in to reply
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on an old version of the diff Aug 18, 2015
    config/initializers/1_settings.rb
    Unable to load the diff.
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 18, 2015
      Owner

      Note from yourself: Settings.reply_by_email

      Note from yourself: `Settings.reply_by_email`
    Please register or sign in to reply
  • Douwe Maan @DouweM

    Added 1 commit:

    • 8906caba - Changes and stuff.
    Aug 19, 2015

    Added 1 commit:

    • 8906caba - Changes and stuff.
    Added 1 commit: * 8906caba - Changes and stuff.
    Toggle commit list
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on commit 8906caba Aug 19, 2015
    Last updated by Douwe Maan Aug 19, 2015
    lib/gitlab/email_html_cleaner.rb 0 → 100644
    1 # Taken mostly from Discourse's Email::HtmlCleaner
    2 module Gitlab
    3 # HtmlCleaner cleans up the extremely dirty HTML that many email clients
    4 # generate by stripping out any excess divs or spans, removing styling in
    5 # the process (which also makes the html more suitable to be parsed as
    6 # Markdown).
    7 class EmailHtmlCleaner
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 19, 2015
      Owner

      Candidate for a filter?

      Candidate for a filter? ![](http://memedad.com/memes/713297.jpg)
    • Douwe Maan @DouweM commented Aug 19, 2015
      Master

      Heh, possibly. I haven't actually read the code so I have no idea. Will check it out tomorrow!

      Edited Aug 19, 2015
      Heh, possibly. I haven't actually read the code so I have no idea. Will check it out tomorrow!
    • Robert Schilling @razer6 commented Aug 19, 2015
      Master

      Maybe this filter can be reused.

      Edited Aug 19, 2015
      Maybe [this filter](https://github.com/jch/html-pipeline/blob/master/lib/html/pipeline/email_reply_filter.rb) can be reused.
    • Douwe Maan @DouweM commented Aug 19, 2015
      Master

      @razer6 I'll check that out, thanks.

      @razer6 I'll check that out, thanks.
    Please register or sign in to reply
  • Jeroen van Baarsen @jvanbaarsen commented Aug 19, 2015

    @DouweM Awesome work!

    @DouweM Awesome work!
  • Jeroen van Baarsen
    @jvanbaarsen started a discussion on an old version of the diff Aug 19, 2015
    Last updated by Drew Blessing Aug 19, 2015
    config/mail_room.yml.example 0 → 100644
    Unable to load the diff.
    • Jeroen van Baarsen @jvanbaarsen commented Aug 19, 2015

      Does it have to be a gmail address? Or is it also possible to use an inhouse imap server? (Maybe a stupid obvious question, but still good to get it answered :D )

      Does it have to be a gmail address? Or is it also possible to use an inhouse imap server? (Maybe a stupid obvious question, but still good to get it answered :D )
    • Drew Blessing @dblessing commented Aug 19, 2015
      Master

      I wondered the same when I saw the reply email address example was using plus addressing. That would be a limiting factor in which mail systems could be used.

      I wondered the same when I saw the reply email address example was using plus addressing. That would be a limiting factor in which mail systems could be used.
    • Douwe Maan @DouweM commented Aug 19, 2015
      Master

      I used to think the same thing, that plus addressing was Gmail only, but fortunately, it's not! It's supported by Yahoo, iCloud, Outlook.com, as well as in-house Postfix installations: https://en.wikipedia.org/wiki/Email_address#Sub-addressing

      I used to think the same thing, that plus addressing was Gmail only, but fortunately, it's not! It's supported by Yahoo, iCloud, Outlook.com, as well as in-house Postfix installations: https://en.wikipedia.org/wiki/Email_address#Sub-addressing
    • Drew Blessing @dblessing commented Aug 19, 2015
      Master

      One major system that doesn't seem to support it is Exchange. At least AFAICT.

      One major system that doesn't seem to support it is Exchange. At least AFAICT.
    • Douwe Maan @DouweM commented Aug 19, 2015
      Master

      Another option would be to use a catch all email like %{reply_key}@replies.gitlab.example.com. I'm assuming Exchange supports something like that?

      Otherwise, the admin would need to install a separate Postfix mail server for this specific purpose.

      Another option would be to use a catch all email like `%{reply_key}@replies.gitlab.example.com`. I'm assuming Exchange supports something like that? Otherwise, the admin would need to install a separate Postfix mail server for this specific purpose.
    • Drew Blessing @dblessing commented Aug 19, 2015
      Master

      Great question - I'm not sure.

      Great question - I'm not sure.
    Please register or sign in to reply
  • Douwe Maan @DouweM

    Added 6 commits:

    • 73eef57d - Make MR notification email subject more standard.
    • 4f34d363 - Correctly set Message-ID for comment notifications.
    • 6b31827a - Include display name with reply to address.
    • 01c5eced - Add "Reply to this email directly" to notification footer.
    • ee1eb294 - Turn reply-by-email attachments into uploads.
    • 170aa3b4 - Update mail_room.
    Aug 19, 2015

    Added 6 commits:

    • 73eef57d - Make MR notification email subject more standard.
    • 4f34d363 - Correctly set Message-ID for comment notifications.
    • 6b31827a - Include display name with reply to address.
    • 01c5eced - Add "Reply to this email directly" to notification footer.
    • ee1eb294 - Turn reply-by-email attachments into uploads.
    • 170aa3b4 - Update mail_room.
    Added 6 commits: * 73eef57d - Make MR notification email subject more standard. * 4f34d363 - Correctly set Message-ID for comment notifications. * 6b31827a - Include display name with reply to address. * 01c5eced - Add "Reply to this email directly" to notification footer. * ee1eb294 - Turn reply-by-email attachments into uploads. * 170aa3b4 - Update mail_room.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 76dbafba - Send a rejection email when the incoming email couldn't be processed.
    Aug 19, 2015

    Added 1 commit:

    • 76dbafba - Send a rejection email when the incoming email couldn't be processed.
    Added 1 commit: * 76dbafba - Send a rejection email when the incoming email couldn't be processed.
    Toggle commit list
  • Douwe Maan @DouweM commented Aug 19, 2015
    Master

    We're pretty much done implementation wise, pending your review. Next up are tests and documentation.

    We're pretty much done implementation wise, pending your review. Next up are tests and documentation.
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on an old version of the diff Aug 19, 2015
    Last updated by Douwe Maan Aug 19, 2015
    Procfile
    Unable to load the diff.
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 19, 2015
      Owner

      To check: If this feature is disabled, what does this job do? Exit early? Continually poll for nothing?

      To check: If this feature is disabled, what does this job do? Exit early? Continually poll for nothing?
    • Douwe Maan @DouweM commented Aug 19, 2015
      Master

      Looks like it quits early: https://github.com/tpitale/mail_room/blob/master/lib%2Fmail_room%2Fcoordinator.rb#L21

      Note from the readme:

      It is important to properly configure MailRoom when using -q in conjunction with a process manager like upstart/god/monit/etc, or to configure the manager to not restart a process infinitely. MailRoom will stop running immediately if it has no settings for mailboxes.

      I have no idea how this will behave with runit which is used in omnibus.

      cc @marin

      Looks like it quits early: https://github.com/tpitale/mail_room/blob/master/lib%2Fmail_room%2Fcoordinator.rb#L21 Note from the readme: > It is important to properly configure MailRoom when using -q in conjunction with a process manager like upstart/god/monit/etc, or to configure the manager to not restart a process infinitely. MailRoom will stop running immediately if it has no settings for mailboxes. I have no idea how this will behave with `runit` which is used in omnibus. cc @marin
    • Douwe Maan @DouweM commented Aug 19, 2015
      Master

      I commented here: https://github.com/tpitale/mail_room/commit/2de3ffe4af24dee8b6288ff58410dd2e598dc95a#commitcomment-12792653

      I commented here: https://github.com/tpitale/mail_room/commit/2de3ffe4af24dee8b6288ff58410dd2e598dc95a#commitcomment-12792653
    Please register or sign in to reply
  • Douwe Maan @DouweM

    Added 1 commit:

    • 03b54f1f - Fix responding to commit notes
    Aug 19, 2015

    Added 1 commit:

    • 03b54f1f - Fix responding to commit notes
    Added 1 commit: * 03b54f1f - Fix responding to commit notes
    Toggle commit list
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on an old version of the diff Aug 19, 2015
    Last updated by Douwe Maan Aug 19, 2015
    app/mailers/email_rejection_mailer.rb 0 → 100644
    Unable to load the diff.
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 19, 2015
      Owner

      Notify handles these two differently:

        default from: Proc.new { default_sender_address.format }
        default reply_to: Gitlab.config.gitlab.email_reply_to
      
        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

      Maybe we should have a base mailer that configures these and has the current_user and can? helpers and whatever else we need to share, that these can both inherit from?

      Edit: Nevermind about current_user/can, their implementations are different for these two mailers.

      Edited Aug 19, 2015 by 🏝 Robert Speicher 🏝
      `Notify` handles these two differently: ```ruby default from: Proc.new { default_sender_address.format } default reply_to: Gitlab.config.gitlab.email_reply_to 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 ``` Maybe we should have a base mailer that configures these and has the `current_user` and `can?` helpers and whatever else we need to share, that these can both inherit from? Edit: Nevermind about `current_user`/`can`, their implementations are different for these two mailers.
    • Douwe Maan @DouweM commented Aug 19, 2015
      Master

      Sounds reasonable. Will do

      Sounds reasonable. Will do
    Please register or sign in to reply
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on an old version of the diff Aug 19, 2015
    Last updated by Douwe Maan Aug 19, 2015
    app/mailers/notify.rb
    Unable to load the diff.
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 19, 2015
      Owner

      I'm unsure about this method belonging in Notify. On the one hand, it's great that each individual [whatever]_email method can just call this method directly, but on the other I think it falls outside the scope of the mailer.

      Maybe we should make a class method on SentNotification and then the methods can call SentNotification.whatever(...) instead of this method.

      I'm unsure about this method belonging in `Notify`. On the one hand, it's great that each individual `[whatever]_email` method can just call this method directly, but on the other I think it falls outside the scope of the mailer. Maybe we should make a class method on `SentNotification` and then the methods can call `SentNotification.whatever(...)` instead of this method.
    • Douwe Maan @DouweM commented Aug 19, 2015
      Master

      Makes sense.

      Makes sense.
    Please register or sign in to reply
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on an old version of the diff Aug 19, 2015
    Last updated by Douwe Maan Aug 19, 2015
    app/views/email_rejection_mailer/rejection.html.haml 0 → 100644
    Unable to load the diff.
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 19, 2015
      Owner

      Why the gfm here?

      Why the `gfm` here?
    • Douwe Maan @DouweM commented Aug 19, 2015
      Master

      Because the reason will have a list of error messages if the note couldn't be posted because of validation errors.

      Because the reason will have a list of error messages if the note couldn't be posted because of validation errors.
    Please register or sign in to reply
  • Douwe Maan @DouweM

    Added 2 commits:

    • a8a861ae - Add BaseMailer to house shared mailer functionality.
    • 992dbbd9 - Move sent_notification! out of Notify.
    Aug 19, 2015

    Added 2 commits:

    • a8a861ae - Add BaseMailer to house shared mailer functionality.
    • 992dbbd9 - Move sent_notification! out of Notify.
    Added 2 commits: * a8a861ae - Add BaseMailer to house shared mailer functionality. * 992dbbd9 - Move `sent_notification!` out of Notify.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • ed6122d7 - Markdown, not GFM.
    Aug 19, 2015

    Added 1 commit:

    • ed6122d7 - Markdown, not GFM.
    Added 1 commit: * ed6122d7 - Markdown, not GFM.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 4e33127b - We'll get the

      for free.

    Aug 19, 2015

    Added 1 commit:

    • 4e33127b - We'll get the

      for free.

    Added 1 commit: * 4e33127b - We'll get the <p> for free.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 0bc94198 - Fix BaseMailer.
    Aug 19, 2015

    Added 1 commit:

    • 0bc94198 - Fix BaseMailer.
    Added 1 commit: * 0bc94198 - Fix BaseMailer.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 3 commits:

    • 089aa2de - Add Reply by email feature to admin dashboard.
    • f3b57ce6 - Update init scripts.
    • 34026c97 - Add documentation.
    Aug 19, 2015

    Added 3 commits:

    • 089aa2de - Add Reply by email feature to admin dashboard.
    • f3b57ce6 - Update init scripts.
    • 34026c97 - Add documentation.
    Added 3 commits: * 089aa2de - Add Reply by email feature to admin dashboard. * f3b57ce6 - Update init scripts. * 34026c97 - Add documentation.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • c50e5e68 - Fix bin/mail_room.
    Aug 19, 2015

    Added 1 commit:

    • c50e5e68 - Fix bin/mail_room.
    Added 1 commit: * c50e5e68 - Fix bin/mail_room.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 1202875d - Fix lib/support/init.d/gitlab.
    Aug 19, 2015

    Added 1 commit:

    • 1202875d - Fix lib/support/init.d/gitlab.
    Added 1 commit: * 1202875d - Fix lib/support/init.d/gitlab.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • eacdd508 - Fix Basemailer#can?
    Aug 19, 2015

    Added 1 commit:

    • eacdd508 - Fix Basemailer#can?
    Added 1 commit: * eacdd508 - Fix Basemailer#can?
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 41fdd20c - Test Gitlab::ReplyByEmail.
    Aug 19, 2015

    Added 1 commit:

    • 41fdd20c - Test Gitlab::ReplyByEmail.
    Added 1 commit: * 41fdd20c - Test Gitlab::ReplyByEmail.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 2 commits:

    • 9501495a - Fix init.d script.
    • 19a049ec - Make documentation clearer.
    Aug 20, 2015

    Added 2 commits:

    • 9501495a - Fix init.d script.
    • 19a049ec - Make documentation clearer.
    Added 2 commits: * 9501495a - Fix init.d script. * 19a049ec - Make documentation clearer.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 83081f16 - Start on tests.
    Aug 20, 2015

    Added 1 commit:

    • 83081f16 - Start on tests.
    Added 1 commit: * 83081f16 - Start on tests.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 3 commits:

    • 3ff9d5c6 - Use canonical version of mail_room.
    • e9972efc - Extract ReplyParser and AttachmentUploader from Receiver.
    • 0b401f2e - Fix a couple of whoopsy daisies.
    Aug 20, 2015

    Added 3 commits:

    • 3ff9d5c6 - Use canonical version of mail_room.
    • e9972efc - Extract ReplyParser and AttachmentUploader from Receiver.
    • 0b401f2e - Fix a couple of whoopsy daisies.
    Added 3 commits: * 3ff9d5c6 - Use canonical version of mail_room. * e9972efc - Extract ReplyParser and AttachmentUploader from Receiver. * 0b401f2e - Fix a couple of whoopsy daisies.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 2 commits:

    • 2f78b5e8 - Make error class names more consistent.
    • 991c9f6f - Test Email::AttachmentUploader.
    Aug 20, 2015

    Added 2 commits:

    • 2f78b5e8 - Make error class names more consistent.
    • 991c9f6f - Test Email::AttachmentUploader.
    Added 2 commits: * 2f78b5e8 - Make error class names more consistent. * 991c9f6f - Test Email::AttachmentUploader.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • e44936f3 - Test EmailReceiverWorker.
    Aug 20, 2015

    Added 1 commit:

    • e44936f3 - Test EmailReceiverWorker.
    Added 1 commit: * e44936f3 - Test EmailReceiverWorker.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 8ec5fb13 - Test Gitlab::Email::Receiver.
    Aug 20, 2015

    Added 1 commit:

    • 8ec5fb13 - Test Gitlab::Email::Receiver.
    Added 1 commit: * 8ec5fb13 - Test Gitlab::Email::Receiver.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 48 commits:

    • 8ec5fb13...8819007c - 47 commits from branch master
    • 3d51a6d4 - Merge branch 'master' into reply-by-email
    Aug 20, 2015

    Added 48 commits:

    • 8ec5fb13...8819007c - 47 commits from branch master
    • 3d51a6d4 - Merge branch 'master' into reply-by-email
    Added 48 commits: * 8ec5fb13...8819007c - 47 commits from branch `master` * 3d51a6d4 - Merge branch 'master' into reply-by-email
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 2b5a2b8f - Removed unused fixtures.
    Aug 20, 2015

    Added 1 commit:

    • 2b5a2b8f - Removed unused fixtures.
    Added 1 commit: * 2b5a2b8f - Removed unused fixtures.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • e1eb09dd - Remove more unused fixtures.
    Aug 20, 2015

    Added 1 commit:

    • e1eb09dd - Remove more unused fixtures.
    Added 1 commit: * e1eb09dd - Remove more unused fixtures.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • ac268674 - Update spec.
    Aug 20, 2015

    Added 1 commit:

    • ac268674 - Update spec.
    Added 1 commit: * ac268674 - Update spec.
    Toggle commit list
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on an old version of the diff Aug 20, 2015
    spec/lib/gitlab/email/attachment_uploader_spec.rb 0 → 100644
    Unable to load the diff.
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 20, 2015
      Owner

      Let's add a FixtureHelper module in spec/support/fixtures.rb to house this.

      Let's add a `FixtureHelper` module in `spec/support/fixtures.rb` to house this.
    Please register or sign in to reply
  • Douwe Maan @DouweM

    Added 1 commit:

    • 54b04f1c - Add fixture_file helper.
    Aug 20, 2015

    Added 1 commit:

    • 54b04f1c - Add fixture_file helper.
    Added 1 commit: * 54b04f1c - Add fixture_file helper.
    Toggle commit list
  • Douwe Maan @DouweM commented Aug 20, 2015
    Master

    @marin Could you see what the impact of this would be to omnibus? We'd need to add the mail_room process if reply by email is enabled, as well as reply_by_email config options (enabled/address, imap host/port/ssl/email/password). You can see the doc for source installations at https://gitlab.com/gitlab-org/gitlab-ce/blob/reply-by-email/doc/reply_by_email/README.md.

    @marin Could you see what the impact of this would be to omnibus? We'd need to add the `mail_room` process if reply by email is enabled, as well as reply_by_email config options (enabled/address, imap host/port/ssl/email/password). You can see the doc for source installations at https://gitlab.com/gitlab-org/gitlab-ce/blob/reply-by-email/doc/reply_by_email/README.md.
  • Douwe Maan @DouweM

    Added 1 commit:

    • 6110e175 - Use heredocs.
    Aug 20, 2015

    Added 1 commit:

    • 6110e175 - Use heredocs.
    Added 1 commit: * 6110e175 - Use heredocs.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 48e25a01 - Add stub_reply_by_email_setting helper.
    Aug 20, 2015

    Added 1 commit:

    • 48e25a01 - Add stub_reply_by_email_setting helper.
    Added 1 commit: * 48e25a01 - Add stub_reply_by_email_setting helper.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 2 commits:

    • 123af785 - Add gitlab:reply_by_email:check rake task.
    • 02b7be4a - Remove weirdness.
    Aug 20, 2015

    Added 2 commits:

    • 123af785 - Add gitlab:reply_by_email:check rake task.
    • 02b7be4a - Remove weirdness.
    Added 2 commits: * 123af785 - Add gitlab:reply_by_email:check rake task. * 02b7be4a - Remove weirdness.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 41ed60c2 - Fix MailRoom running check.
    Aug 20, 2015

    Added 1 commit:

    • 41ed60c2 - Fix MailRoom running check.
    Added 1 commit: * 41ed60c2 - Fix MailRoom running check.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • f26c2905 - Fix indentation
    Aug 20, 2015

    Added 1 commit:

    • f26c2905 - Fix indentation
    Added 1 commit: * f26c2905 - Fix indentation
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 99ef8c81 - Fix markdown specs.
    Aug 20, 2015

    Added 1 commit:

    • 99ef8c81 - Fix markdown specs.
    Added 1 commit: * 99ef8c81 - Fix markdown specs.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • afb765ad - Fix markdown specs again. Apparently development and test behave differently.
    Aug 20, 2015

    Added 1 commit:

    • afb765ad - Fix markdown specs again. Apparently development and test behave differently.
    Added 1 commit: * afb765ad - Fix markdown specs again. Apparently development and test behave differently.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 3d141d15 - Fix spec by removing global state.
    Aug 20, 2015

    Added 1 commit:

    • 3d141d15 - Fix spec by removing global state.
    Added 1 commit: * 3d141d15 - Fix spec by removing global state.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 35224d5e - Memoize address_regex locally.
    Aug 20, 2015

    Added 1 commit:

    • 35224d5e - Memoize address_regex locally.
    Added 1 commit: * 35224d5e - Memoize address_regex locally.
    Toggle commit list
  • Marin Jankovski @marin commented Aug 21, 2015
    Master

    @DouweM The impact on omnibus-gitlab is not big(new config and templates but no structure changes), when this is ready for merge open an issue at omnibus-gitlab so I can add in the support.

    Edited Aug 21, 2015 by Marin Jankovski
    @DouweM The impact on omnibus-gitlab is not big(new config and templates but no structure changes), when this is ready for merge open an issue at omnibus-gitlab so I can add in the support.
  • Douwe Maan @DouweM

    Added 1 commit:

    • 69708dab - Block blocked users from replying to threads by email.
    Aug 21, 2015

    Added 1 commit:

    • 69708dab - Block blocked users from replying to threads by email.
    Added 1 commit: * 69708dab - Block blocked users from replying to threads by email.
    Toggle commit list
  • Douwe Maan @DouweM

    Reassigned to @DouweM

    Aug 21, 2015

    Reassigned to @DouweM

    Reassigned to @DouweM
    Toggle commit list
  • Douwe Maan @DouweM

    Reassigned to @rspeicher

    Aug 21, 2015

    Reassigned to @rspeicher

    Reassigned to @rspeicher
    Toggle commit list
  • Douwe Maan @DouweM commented Aug 21, 2015
    Master

    @rspeicher Ready for the final review? :)

    @rspeicher Ready for the final review? :)
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on an old version of the diff Aug 21, 2015
    Last updated by Douwe Maan Aug 21, 2015
    doc/reply_by_email/README.md 0 → 100644
    Unable to load the diff.
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 21, 2015
      Owner

      I think anywhere in our documentation where we say what we're about to do and then follow it with a command, the explanatory line should always end in a colon. You did that for the steps above, let's add it to steps 6 and 7.

      I think anywhere in our documentation where we say what we're about to do and then follow it with a command, the explanatory line should *always* end in a colon. You did that for the steps above, let's add it to steps 6 and 7.
    • Douwe Maan @DouweM commented Aug 21, 2015
      Master

      Agreed

      Agreed
    Please register or sign in to reply
  • 🏝 Robert Speicher 🏝
    @rspeicher started a discussion on an old version of the diff Aug 21, 2015
    Last updated by Douwe Maan Aug 21, 2015
    doc/reply_by_email/README.md 0 → 100644
    Unable to load the diff.
    • 🏝 Robert Speicher 🏝 @rspeicher commented Aug 21, 2015
      Owner

      Should we break this out into its own ### Development section below Omnibus?

      Should we break this out into its own `### Development` section below Omnibus?
    • Douwe Maan @DouweM commented Aug 21, 2015
      Master

      I'll add a new section. The init.d stuff isn't required in development either.

      I'll add a new section. The init.d stuff isn't required in development either.
    Please register or sign in to reply
  • Douwe Maan @DouweM

    Added 1 commit:

    • 50baa1fd - Add development section to doc.
    Aug 21, 2015

    Added 1 commit:

    • 50baa1fd - Add development section to doc.
    Added 1 commit: * 50baa1fd - Add development section to doc.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 03666553 - Fix check task for development.
    Aug 21, 2015

    Added 1 commit:

    • 03666553 - Fix check task for development.
    Added 1 commit: * 03666553 - Fix check task for development.
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 3abb356d - Add development documentation for real.
    Aug 21, 2015

    Added 1 commit:

    • 3abb356d - Add development documentation for real.
    Added 1 commit: * 3abb356d - Add development documentation for real.
    Toggle commit list
  • Douwe Maan @DouweM

    Title changed from WIP: Reply by email to Reply by email

    Aug 21, 2015

    Title changed from WIP: Reply by email to Reply by email

    Title changed from **WIP: Reply by email** to **Reply by email**
    Toggle commit list
  • Douwe Maan @DouweM

    Added 1 commit:

    • 15fc7bd6 - No HTML-only email please
    Aug 21, 2015

    Added 1 commit:

    • 15fc7bd6 - No HTML-only email please
    Added 1 commit: * 15fc7bd6 - No HTML-only email please
    Toggle commit list
  • 🏝 Robert Speicher 🏝 @rspeicher

    Status changed to merged

    Aug 22, 2015

    Status changed to merged

    Status changed to merged
    Toggle commit list
  • 🏝 Robert Speicher 🏝 @rspeicher

    mentioned in commit f0bdf7f8

    Aug 22, 2015

    mentioned in commit f0bdf7f8

    mentioned in commit f0bdf7f8102405f272a42c04c1fa70dae7365854
    Toggle commit list
  • Douwe Maan @DouweM

    mentioned in issue omnibus-gitlab#732 (closed)

    Aug 22, 2015

    mentioned in issue omnibus-gitlab#732 (closed)

    mentioned in issue gitlab-org/omnibus-gitlab#732
    Toggle commit list
  • Patricio Cano @patricio commented Aug 22, 2015

    Great work @DouweM!!

    Great work @DouweM!!
  • Robert Schilling @razer6 commented Aug 22, 2015
    Master

    Awesome 📧

    Awesome :email:
  • Jacob Vosmaer
    @jacobvosmaer started a discussion on the diff Aug 25, 2015
    Last updated by Douwe Maan Aug 25, 2015
    doc/reply_by_email/README.md 0 → 100644
    62 :delivery_options:
    63 # The URL to the Redis server used by Sidekiq. Should match the URL in config/resque.yml.
    64 :redis_url: redis://localhost:6379
    65 # Always "resque:gitlab".
    66 :namespace: resque:gitlab
    67 # Always "incoming_email".
    68 :queue: incoming_email
    69 # Always "EmailReceiverWorker"
    70 :worker: EmailReceiverWorker
    71 ```
    72
    73
    74 4. Find `lib/support/init.d/gitlab.default.example` and copy it to `/etc/default/gitlab`:
    75
    76 ```sh
    77 sudo cp lib/support/init.d/gitlab.default.example /etc/default/gitlab
    • Jacob Vosmaer @jacobvosmaer commented Aug 25, 2015

      @DouweM this is a bad idea. People put customizations in /etc/default/gitlab, we should not be telling them to clobber that file.

      When this got deployed to dev last night, we broke Git HTTP access (and hence ci.gitlab.org cc @rspeicher ) because we set the GITLAB_GRACK_AUTH_ONLY variable in that file.

      cc @dzaporozhets

      @DouweM this is a bad idea. People put customizations in /etc/default/gitlab, we should not be telling them to clobber that file. When this got deployed to dev last night, we broke Git HTTP access (and hence ci.gitlab.org cc @rspeicher ) because we set the GITLAB_GRACK_AUTH_ONLY variable in that file. cc @dzaporozhets
    • Douwe Maan @DouweM commented Aug 25, 2015
      Master

      @jacobvosmaer Ah, good point. I didn't realize that people may already have that file. I'll update the doc to say they should edit it to add mail_room_enabled=true instead if it already exists.

      @jacobvosmaer Ah, good point. I didn't realize that people may already have that file. I'll update the doc to say they should edit it to add `mail_room_enabled=true` instead if it already exists.
    • Douwe Maan @DouweM commented Aug 25, 2015
      Master

      @jacobvosmaer Done in f00fdc71, part of !1193 (merged).

      @jacobvosmaer Done in f00fdc71e9ea4e227b405dbfa671bf9c6f94736d, part of !1193.
    Please register or sign in to reply
  • Jacob Vosmaer @jacobvosmaer commented Aug 27, 2015

    What happened to the omnibus integration? This got merged with two 'todo checkboxes' unticked.

    Screen_Shot_2015-08-27_at_10.51.32

    Edit: ah I see the reference to omnibus-gitlab#732 (closed) now in the fine print above. Never mind :)

    Edited Aug 27, 2015 by Jacob Vosmaer
    What happened to the omnibus integration? This got merged with two 'todo checkboxes' unticked. ![Screen_Shot_2015-08-27_at_10.51.32](https://gitlab.com/gitlab-org/gitlab-ce/uploads/396691ee3ed078b2e2fc56b65a3ee88f/Screen_Shot_2015-08-27_at_10.51.32.png) Edit: ah I see the reference to gitlab-org/omnibus-gitlab#732 now in the fine print above. Never mind :)
  • Marin Jankovski @marin commented Aug 27, 2015
    Master

    @jacobvosmaer Thanks for asking. @DouweM Created an issue in omnibus-gitlab/issues!732 and MR was merged yesterday with this. I pushed a documentation commit to gitlab-ce/merge_requests!1193

    @jacobvosmaer Thanks for asking. @DouweM Created an issue in omnibus-gitlab/issues!732 and MR was merged yesterday with this. I pushed a documentation commit to gitlab-ce/merge_requests!1193
  • Douwe Maan @DouweM commented Aug 27, 2015
    Master

    @jacobvosmaer Also see !1173 (comment 1935363), which I posted a day before this MR was merged.

    Edited Aug 27, 2015 by Douwe Maan
    @jacobvosmaer Also see https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1173#note_1935363, which I posted a day before this MR was merged.
  • Stan Hu
    @stanhu started a discussion on commit f76eac56 Sep 08, 2015
    Last updated by Douwe Maan Sep 09, 2015
    app/workers/email_receiver_worker.rb 0 → 100644
    5
    6 def perform(raw)
    7 return unless Gitlab.config.reply_by_email.enabled
    8
    9 # begin
    10 Gitlab::EmailReceiver.new(raw).process
    11 # rescue => e
    12 # handle_failure(raw, e)
    13 # end
    14 end
    15
    16 private
    17
    18 def handle_failure(raw, e)
    19 # TODO: Handle better.
    20 Rails.logger.warn("Email can not be processed: #{e}\n\n#{raw}")
    • Stan Hu @stanhu commented Sep 08, 2015
      Owner

      Just came across this. Given this is a Sidekiq worker, should this just be logger.warn instead of Rails.logger?

      Just came across this. Given this is a Sidekiq worker, should this just be `logger.warn` instead of `Rails.logger`?
    • Douwe Maan @DouweM commented Sep 09, 2015
      Master

      @stanhu Does that make a difference?

      @stanhu Does that make a difference?
    Please register or sign in to reply
  • Stan Hu
    @stanhu started a discussion on commit f76eac56 Oct 01, 2015
    • Stan Hu @stanhu

      mentioned in merge request !1480 (merged)

      Oct 01, 2015

      mentioned in merge request !1480 (merged)

      mentioned in merge request !1480
      Toggle commit list
    Please register or sign in to reply
  • Lin Jen-Shin
    @godfat started a discussion on commit 76dbafba Mar 31, 2016
    Last updated by Douwe Maan Apr 04, 2016
    app/workers/email_receiver_worker.rb
    30 when Gitlab::EmailReceiver::AutoGeneratedEmailError
    31 reason = "The email was marked as 'auto generated', which we can't accept. Please create your comment through the web interface."
    32 when Gitlab::EmailReceiver::UserNotFoundError
    33 reason = "We couldn't figure out what user corresponds to the email. Please create your comment through the web interface."
    34 when Gitlab::EmailReceiver::UserNotAuthorizedError
    35 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."
    36 when Gitlab::EmailReceiver::NoteableNotFoundError
    37 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."
    38 when Gitlab::EmailReceiver::InvalidNote
    39 can_retry = true
    40 reason = e.message
    41 else
    42 return
    43 end
    44
    45 EmailRejectionMailer.delay.rejection(reason, raw, can_retry)
    • Lin Jen-Shin @godfat commented Mar 31, 2016
      Master

      @DouweM I think it would never reach here because in the case there's an else clause which just returns from the method. Was this expected?

      @DouweM I think it would never reach here because in the `case` there's an `else` clause which just returns from the method. Was this expected?
    • Douwe Maan @DouweM commented Apr 01, 2016
      Master

      @godfat When e is any of those errors, we don't reach else and return right?

      @godfat When `e` is any of those errors, we don't reach `else` and `return` right?
    • Lin Jen-Shin @godfat commented Apr 01, 2016
      Master

      Oh, you're right. I was confused. I didn't use return for a long while. (because functional programming :P)

      Oh, you're right. I was confused. I didn't use return for a long while. (because functional programming :P)
    • Lin Jen-Shin @godfat commented Apr 01, 2016
      Master

      So personally I'll omit that else clause and write:

      EmailRejectionMailer.delay.rejection(reason, raw, can_retry) if reason
      So personally I'll omit that else clause and write: ``` EmailRejectionMailer.delay.rejection(reason, raw, can_retry) if reason ```
    • Douwe Maan @DouweM commented Apr 04, 2016
      Master

      @godfat 👍

      @godfat :+1:
    Please register or sign in to reply
  • Lin Jen-Shin
    @godfat started a discussion on commit 76dbafba May 24, 2016
    Last updated by Lin Jen-Shin Sep 14, 2016
    • Lin Jen-Shin @godfat

      mentioned in commit godfat/gitlab-ce@cc69bd07

      May 24, 2016

      mentioned in commit godfat/gitlab-ce@cc69bd07

      mentioned in commit godfat/gitlab-ce@cc69bd07e7b0f7b8f81fcff0f1e464e4aa36a583
      Toggle commit list
    • Lin Jen-Shin @godfat

      Mentioned in commit ubudzisz/gitlab-ce@cc69bd07

      Sep 14, 2016

      Mentioned in commit ubudzisz/gitlab-ce@cc69bd07

      Mentioned in commit ubudzisz/gitlab-ce@cc69bd07e7b0f7b8f81fcff0f1e464e4aa36a583
      Toggle commit list
    Please register or sign in to reply
  • Dmitriy Zaporozhets @dzaporozhets

    mentioned in commit winniehell/gitlab-ce@4bebdc09

    Jul 07, 2016

    mentioned in commit winniehell/gitlab-ce@4bebdc09

    mentioned in commit winniehell/gitlab-ce@4bebdc09463e29d26eac0117e0e3b45a9448c600
    Toggle commit list
  • Dmitriy Zaporozhets @dzaporozhets

    Mentioned in commit winniehell/gitlab-ce@4bebdc09

    Nov 09, 2016

    Mentioned in commit winniehell/gitlab-ce@4bebdc09

    Mentioned in commit winniehell/gitlab-ce@4bebdc09463e29d26eac0117e0e3b45a9448c600
    Toggle commit list
  • Toon Claes @toon

    mentioned in issue #36054 (closed)

    Aug 07, 2017

    mentioned in issue #36054 (closed)

    mentioned in issue #36054
    Toggle commit list
  • Write
  • Preview
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment
🏝 Robert Speicher 🏝
Assignee
🏝 Robert Speicher 🏝 @rspeicher
Assign to
8.0
Milestone
8.0
Assign milestone
Time tracking
0
Labels
None
Assign labels
  • View project labels
Reference: gitlab-org/gitlab-ce!1173