Text-only email templates

Medvezo commented 5 years ago

Problem

By default Devise comes with html templates. The problem with that is that some spam catching systems may consider that as a negative signal and route email to spam folder. That is worrisome because we are talking about transactional email. Think of all extra customer support emails that will generate in the long run.

Solution

Ship with text-only templates for the next major release (5.0.0).

References

tegon commented 5 years ago

Hi @Medvezo, thanks for bringing this up.

I agree with you that is important to include text versions of the emails, but do you think we need to send only them, without the HTML versions?

By reading the references you pointed out, all of them say that it is safe to send both, as long as the text and links are similar to HTML version. I've also worked on applications that did something similiar in the past and it worked well.

Thoughts?

JanBussieck commented 5 years ago

I was also going to bring this up, I solved this by overwriting the default devise templates. Sending both works well and only sending html email might be linked to being sent to gmails notorious promotion tab :)

I could start work on this, drawing on what we have done so far and open a PR if you think this is interesting to the broader user base.

tegon commented 5 years ago

@JanBussieck That would be great!

colinross commented 5 years ago

If we supplied the text formats alongside the html (in app/views/devise/mailer) wouldn't this automagically send multi-part via ActionMailer?

ref: https://guides.rubyonrails.org/action_mailer_basics.html#sending-multipart-emails

You could then essentially configure which format (or both by default) get sent via action mailer's config options. It wouldn't be specific to devise, but I would expect the domain requirement would be consistent anyways, right?

default config.action_mailer.default_options = { parts_order: ["text/plain", "text/enriched", "text/html"] } text-only config.action_mailer.default_options = { parts_order: ["text/plain"] } html-only config.action_mailer.default_options = { parts_order: ["text/html"] }

Edit: For backwards compatibility, we could also override the devise_mail call to only include html format as a default (current behavior) by appending parts_order: ["text/html"] in the options passed off inheaders_for`.

ref: https://github.com/plataformatec/devise/blob/master/lib/devise/mailers/helpers.rb#L31

tegon commented 5 years ago

@colinross

If we supplied the text formats alongside the html (in app/views/devise/mailer) wouldn't this automagically send multi-part via ActionMailer?

I think so 👍

colinross commented 5 years ago

I've put a bit of time into this and wanted to report back a bit on my findings so far.

If the goal is just to send multipart emails, it is a small code change (just adding the templates). Where things get messy is dealing with backwards compatibility and not wanting to release a change that essentially locks people into a multipart-emails as a requirement. I think this needs to be an opt-in/explicit configuration feature. There are way too many deployments of devise with very customized emails to just rollout this change otherwise, imho.

Due to the way that rails (actionmailer specifically) handles formats (similar to other types of views) if the formats exist, they will be used. This means that if we add them to the gem-based views ( app/views/devise/mailer) they get picked up implicitly and are used. This also means that if the deployment is using custom mailers, they may end up with a mix of custom-html and gem-supplied plain text. We can't make assumptions the contents are rationally similar. I've worked on projects that used password reset emails as the 'welcome' email since the accounts were automatically created either on demand or before hand in non-public applications for instance.

So, what is the only 'safe' way of rolling this out? We could either do nothing (just beef up the documentation around mailer views saying 'if you want multipart, make a confirmation_instructions.text.erb view template). Alternatively, we could only have the plain text views populated via the views rake task, but not in the gem's view path.

Other than that, I think we have painted ourselves into a box a bit as far as a graceful upgrade path here.

More technically, we loose a lot of control in preventing formats from going out since devise_mail accepts a block. This isn't bad persay (devise is meant to be highly customizable), but it does mean that we can either support custom blocks, or we can support configuration of what formats go out (not solely based on the formats available in the view_paths due to the above issues). You control the format by what formats get called in the block. Please open my eyes, but i think ruby runs into void concerning some concept of a super block. We also run into issues when trying to use the new multipart emails with scoped views.

I want to chew on this a bit more with some concrete tests to see what happens in the various scenarios, but overall – I don't think we are ready to implement this 'as a small change' and as such, I want to see if there is a better approach, maybe even shipping a Devise::Mailer vs Devise::MultipartMailer that would allow for breaking changes and a more graceful upgrade path.

colinross commented 5 years ago

I circled back a bit, it seems that even if you override the parts_order and omit plain text, it is still included, confirming for me a bit that parts_order is really just about order and isn't an array of the parts to include.

I know this is a bit to get through, but before moving forward, I'd want a bit more buy-in from the group and team as to the direction they'd like to see this move.

In the meantime (time permitting) I was going to dig more into the ActionMailer::Collector part of the mail generation to see if I can find a legit way to better control the formats produced.

heartcombo / devise