Allow messages sent to be text/html instead of text/plain

See also http://drupal.stackexchange.com/questions/199762/formatting-email-sent-w..., for some more ideas that we should consider

* Have a contact-message template that's empty by default, with per-bundle variants. #2270883: Automatically add theme hook suggestions for all entity types would help with the second part.
* Offer a per-entity type theme suggestion for field templates and have a version for contact messages that results in better markup for HTML mails, possibly one for mail and one for web output?

Currently working on this issue. Uploaded necessary code to allow messages to be sent in HTML format, but still need to figure out how to allow this on a per form basis.

1. index 0000000..4111ce6
   --- /dev/null
   
   --- /dev/null
   +++ b/config/install/contact_storage.settings.yml
   
   +++ b/config/install/contact_storage.settings.yml
   @@ -0,0 +1 @@
   
   @@ -0,0 +1 @@
   +send_html: true
   

   Don't forget to remove this again when adding the per-form setting.

2. +++ b/contact_storage.module
   @@ -202,3 +207,43 @@ function contact_storage_entity_extra_field_info() {
   +/**
   + * Implements hook_theme().
   + */
   +function contact_storage_theme() {
   +  return array(
   +    'contact_storage_mail' => array(
   +      'variables' => array(
   +        'message' => array(),
   +      ),
   +    ),
   ...
   +    $message['params']['theme'] = 'contact_storage_mail';
   

   This feature is specific to swiftmailer.module.

   We will need to document this somehwere. Also add a description in the UI to the checkbox that explains that a module like Swiftmailer (as link) is necessary to send HTML mails.

   Also, we should consider to rely more on the suggestions feature of swiftmailer, which now supports swiftmailer__contact/swiftmailer--contact.html.twig. If you do that, you should not have to specify theme here.

   We could also, instead of unsetting #theme, actually provide one, including suggestions that can then be used to do per-form theming of the inner elements.

3. +++ b/contact_storage.module
   @@ -202,3 +207,43 @@ function contact_storage_entity_extra_field_info() {
   +  // If enabled, ensure that contact mails are sent as plain text.
   +  if (\Drupal::config('contact_storage.settings')->get('send_html') && $message['module'] == 'contact' && isset($message['params']['contact_message'])) {
   +    // Enforce that we are sending mails as HTML, and tell Swiftmailer to
   

   this will then also need to be updated to check the form setting.

   @larowlan: I'm not 100% convinced yet about the per-form setting, actually. Are you sure that's a useful use case?

   In my case, I want this to be enabled for *all* contact forms of my site, and I might have a lot.

4. +++ b/src/ContactMessageViewBuilder.php
   @@ -0,0 +1,31 @@
   +    // The message fields are individually rendered into email templates, so
   +    // the entity has no template itself.
   +    unset($build['#theme']);
   

   That would mean we would change this part no longer unset it.

Thanks for the feedback. @Bambell already started to work on doing the per-form thing, we will have to undo that again. What you learned there will be of use in other contact_storage tasks, so that is fine :)

Yes, what we need instead then is a contact storage settings page, probably on http://d8/admin/structure/contact/settings, with a local task.

Yes, we can add some comments. note that the table/html structure there is 1:1 taken from the default swiftmailer template, we're just adding the CSS. We should explain that in the comments.

I have added a settings page to allow toggling send_html on and off. Also created an event subscriber for the onSave event of the settings form, as suggested by @Berdir. I also did the other changes suggested by @larowlan. I did not use swiftmailer's suggestions feature and did not make changes for per-form theming, as I'm not sure if that's still desired, after the 3 last comments. This will need reviewing and I can add a test to verify if send_html is true/false after checking/unchecking the checkbox, but I don't see how this feature can be further tested...

For future reference, here's the patch I wrote to enable per form HTML messages (allow_messages_sent_to-2680731-10.patch).

Remember to set issues to needs review when uploading a patch (even if it is not complete. that will trigger testbot and it will run the existing tests).

1. +++ b/config/install/contact_storage.settings.yml
   +++ b/config/install/contact_storage.settings.yml
   @@ -0,0 +1 @@
   
   @@ -0,0 +1 @@
   +send_html: true
   diff --git a/contact_storage.links.task.yml b/contact_storage.links.task.yml
   

   now we need config schema for this as well.

   The option should also be disabled by default, because drupal by default can't send HTML.

2. +++ b/contact_storage.module
   @@ -202,3 +207,43 @@ function contact_storage_entity_extra_field_info() {
   +    // Enforce that we are sending mails as HTML, and tell Swiftmailer to
   +    // generate a plain text version.
   +    $message['headers']['Content-Type'] = 'text/html';
   +    $message['params']['theme'] = 'contact_storage_mail';
   +    $message['params']['convert'] = TRUE;
   

   Even without swiftmailer, we can test these properties. Have a look at how the core contact.module uses drupalGetMail() in its tests to get "sent" mails. We can assert that the body there contains the expected HTML, and that the content type header is set/not set as expected.

   I think we should do the template change. It will make it easier for users to have more specific templates.

3. +++ b/contact_storage.module
   @@ -202,3 +207,43 @@ function contact_storage_entity_extra_field_info() {
   + */
   +function template_preprocess_contact_storage_mail(&$variables) {
   +  $variables['subject'] = $variables['message']['subject'];
   +  $variables['body'] = $variables['message']['body'];
   +}
   

   the nice thing about is that we can remove this then.

4. +++ b/contact_storage.services.yml
   @@ -0,0 +1,5 @@
   +services:
   +  contact_storage.settings_form_save:
   +        class: \Drupal\contact_storage\EventSubscriber\ContactStorageSettingsFormSave
   +        tags:
   +          - { name: event_subscriber }
   

   should have two space indendation only, redirect.module (where we copied this from is incorrectly formatted)

5. +++ b/src/ContactMessageViewBuilder.php
   @@ -0,0 +1,31 @@
   +    // The message fields are individually rendered into email templates, so
   +    // the entity has no template itself.
   +    unset($build['#theme']);
   

   as suggested, I think we should open a follow-up issue to actually have a template for this, including per-form suggestions. Then this part of the code would go away as well.

6. +++ b/src/EventSubscriber/ContactStorageSettingsFormSave.php
   @@ -0,0 +1,38 @@
   +    // Changing the Redirect settings means that any cached page might
   +    // result in a different response, so we need to invalidate them all.
   

   leftover redirect.module comment.

Fixed the issues @Berdir pointed out and added test coverage (only for the header !). Will figure out how to define a per form template in another issue.

For reference, here's the follow-up issue for the theming : #2722501: Per-form theming of the inner elements of messages.

+++ b/src/Tests/ContactStorageTest.php
@@ -134,6 +155,10 @@ class ContactStorageTest extends ContactStorageTestBase {
+    // Check that this new message is now in HTML format.
+    $captured_emails = $this->drupalGetMails();
+    $this->assertTrue(strpos($captured_emails[1]['headers']['Content-Type'], 'text/html') !== FALSE);

It should be possible to check the additional options that we set as well.

Body is a bit tricky. As discussed, the problem is that the mail collector extends from PhpMail and converts HTML.

See also the very related issue, which would make our job here easier: #2223967: Do not decode a contact message twice. That adds a test mail collector that does not remove HTML. We could copy that. We should also try to move that core issue forward, then we can remove parts of what we're doing here.

Made some changes to use Swiftmailer's suggestions feature, however @Berdir, I believe that template preprocessing (on Contact Storage's end) is still required (not to use message['body'] inside the template file). I also added a basid README.txt file...

+++ b/README.txt
@@ -0,0 +1,53 @@
+tim-e

This should be me now. ;-)

This should be me now. ;-)

Yep, I have updated the name in the README.txt file.

the entity has no template itself

please point to todo https://www.drupal.org/node/2270883

please point to todo

Added a todo pointing to this issue (2270883).

Thanx but that should be

// @todo Remove when commited http
...

Take examples from core, @todo should said what it about

I updated the @todo.

+++ b/src/ContactMessageViewBuilder.php
@@ -0,0 +1,32 @@
+    // The message fields are individually rendered into email templates, so
+    // the entity has no template itself.
+    // @todo Remove when commited http
+    unset($build['#theme']);
+    return $build;

The core issue isn't related to *this*. It will not solve this, it just adds suggestions *if* the template exists.

It is however related to the follow-up that we created: #2722501: Per-form theming of the inner elements of messages. Because there, we will add the suggestions.

What you want to do here is add a @todo for our own issue, something like this:

@todo Remove this when providing a template in https://www.drupal.org/node/2722501.

And in that issue, we will add a suggestions hook and in that suggestions hook, we can add a todo on the core issue.

1. +++ b/README.txt
   @@ -0,0 +1,53 @@
   +If you want to be able to send messages in HTML format, the Swiftmailer module
   +is required. To install it, follow the same instructions as above, using the
   +following module :
   +
   +https://www.drupal.org/project/swiftmailer
   

   Having the generic things is nice too, but I would like to have step-by-step instructions in a separate HTML chapter including:

   1. Enabling Swiftmailer
   2. Recommended settings (respect content type checkbox enabled, not forcing HTML)
   3. Enable Mailsystem and configuring it to use Swiftmailer (mention that it can be set either just for con
   4. Enable the HTML sending checkbox in the Contact Storage settings page.
   5. How to customize theming (that contact_storage provides a default template, how it is called).

   Also include the path to the various configuration forms.

2. +++ b/src/ContactMessageViewBuilder.php
   @@ -0,0 +1,32 @@
   +    // @todo Remove this when providing a template in https://www.drupal.org/node/2722501.
   

   over 80 characters. link needs to be on a separate line. indendation after @todo is two spaces.

3. +++ b/src/EventSubscriber/ContactStorageSettingsFormSave.php
   @@ -0,0 +1,38 @@
   + * A subscriber invalidating the entity type definition cache when the settings
   + * page is saved.
   + */
   +class ContactStorageSettingsFormSave implements EventSubscriberInterface {
   +
   +  /**
   +   * Invalidate the entity type definition cache whenever the settings are
   +   * modified.
   

   per coding standards, those should also be below 80 characters.

4. +++ b/src/EventSubscriber/ContactStorageSettingsFormSave.php
   @@ -0,0 +1,38 @@
   +    // Changing the Redirect settings means that any cached page might
   +    // result in a different response, so we need to invalidate them all.
   

   still mentiones redirect. since we already document twice what we do, I think we can just remove this comment.

I've done those changes, except #3, as the longest comment is precisely 80 characters, which I believe is acceptable?

[...] lines of code should not be longer than 80 characters

I assume this is inclusively..

The patch needed a small re-roll due to the last commit. Note that I left a static route for the settings tab / page (and thus re-created the YAML file). I'm not too sure if an entity route provider can be used for that purpose...

Don't forget to provide interdiffs, this is hard to review otherwise.

+++ b/README.txt
@@ -0,0 +1,86 @@
+2. Choose PHP as the transport type.
+    In "Configuration" -> "Swift Mailer", in the "Transport" tab, select "PHP"
+    under "Transport Types".

this is not a requirement, any transport method works. I think we can leave this part out. Or rewrite to just say that you need to configure whatever makes sense there for your site.

I removed that line, should be good now. I did not upload an interdiff because the last patch wouldn't apply cleanly. Perhaps I should have uploaded the re-rolled patch first and then interdiff with that one ..?

You don't have to actually upload a rerolled patch, but yes, try to do that locally, then make changes, so that you can provide an interdiff.

Looks pretty good to me now, the only thing I'm not sure about is that we need the preprocess function.

This looks OK to me.

The only question I am not sure about and that we might need to clarify better in the documentation is the relationship/integration with swiftmailer:

* Not sure how other modules (like smpt) implement the HTML stuff, setting the content type will likely not affect them in the same way.
* The template will only work with Swiftmailer.

Maybe the UI checkbox should say "This has only been tested with the Swiftmailer module, other modules might not work out of the box and will not use the provided default template." instead of the current text?