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
-
-
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 -
Owner
-
Master
Heh, possibly. I haven't actually read the code so I have no idea. Will check it out tomorrow!
-
Master
-
-
@DouweM Awesome work!
-
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 -
Master
We're pretty much done implementation wise, pending your review. Next up are tests and documentation.
-
Added 48 commits:
-
8ec5fb13...8819007c - 47 commits from branch
master
- 3d51a6d4 - Merge branch 'master' into reply-by-email
Toggle commit list -
8ec5fb13...8819007c - 47 commits from branch
-
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. -
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.
-
-
Master
@rspeicher Ready for the final review? :)
-
Title changed from WIP: Reply by email to Reply by email
Toggle commit list -
Status changed to merged
Toggle commit list -
-
-
Master
Awesome
📧 -
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 -
@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.
-
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. -
Master
@jacobvosmaer Done in f00fdc71, part of !1193 (merged).
-
-
What happened to the omnibus integration? This got merged with two 'todo checkboxes' unticked.
Edit: ah I see the reference to omnibus-gitlab#732 (closed) now in the fine print above. Never mind :)
-
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
-
Master
@jacobvosmaer Also see !1173 (comment 1935363), which I posted a day before this MR was merged.
-
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}") -
-
-
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) -
Master
Oh, you're right. I was confused. I didn't use return for a long while. (because functional programming :P)
-
Master
So personally I'll omit that else clause and write:
EmailRejectionMailer.delay.rejection(reason, raw, can_retry) if reason
-
-
-
-
-