Allow to use a field for the text part of a multipart message

Look at the display settings of your newsletter content type. You can show/hide CCK field in HTML and plain text messages. This should work for plain text alternatives.

If you could write the code too, the feature has the best cards to get implemented. Talking is silver, code is gold ;)

While using simplenews 6.x-1.x there had been no such choice, so all newsletter issues were written in the main body field. By using the patch from #709234: Simplenews custom plain text version I could create one (!) additional CCK field for the plain text version.

Now, when upgrading to 6.x-2.x I can choose which custom fields to show/hide on plain/html view but I can't set any options for the body field (which contains the HTML part and shouldn't be send when sending as plain text).

Is there possibly an option I missed to disable node->body on plain emails?

Regards,
Paul

Could someone please convert this to a clean patch for 6.x-2.x?

This functionality was requested a few times.

I'm not sure if we should use a CCK field for this...
Instead i think it should be a simple optional text field (in case of specific user permissions).
We also should still have the permission to autocreate the text and provide it as a template (js callback?)

So conversion would be a great start but there's still some work left and decisions to take to include this in simplenews.

I can work on the patch next week-end, but if you think there should be another solution, maybe we'd be better off discussing it first ;)

I tried to combine both this patch as well as #709234: Simplenews custom plain text version. Patch is against current 6.x-2.x (as of 2010-11-22) and still needs work. The solution is still about adding a new CCK field.

Cheers,
Paul

Thanks for the patch. Lot easier to read for me .-) Here's a few thoughts from my first sight.

There are tabs in the patch..

It doesn't make sense to me to choose the plaintext field per newsletter. Much more it is per content type (note there are multiple possible!).

If we add a body field, we really should provide a way (such as js button/callback) to build the plaintext from the node.

Note that you'r making the plaintext simplenews build mode obsolete. We should document this in the UI.

Additionally it seems to me you're ignoring input filters completely for the field. This seems wrong to me. If a filter is chosen it should be applied.
We need cck module to be optional. Make the patch hide the new options in the UI if cck is inactive and also make the processes work cleanly in this case with enough checks.

Other opinions?
(Seems still a lot of work to do.)

You're absolutely right, there's still much work. My patch was just a first draft of a conversion to 2.x and works just fine within a test environment for a client.

I'll just throw in my thoughts:

• In order to make it a general patch, I agree on checking for CCK being enabled or drop CCK dependance completely.
• Since the same content type may be used for several newsletters it's in my opinion plausible to have the plain text field a per newsletter option (one newsletter may require plain text whereas others may be fine with the automatic conversion, hence "none" as plain text field).
• Regarding the per button conversion, I am currently developing a very special script which does just this: convert the (HTML) body text to a plain text version. This is kind of hard to generalize, though, as there are many different opinions of how to convert it. Maybe the automatic simplenews_html_to_text (don't recall the right name right now) could be used and the conversion could be done via AHAH call.
• About the filtering I've had some thoughts, too. For my client it is sufficient to not use any filters as this field shall only contain plaintext, so no HTML allowed. Of course, I am still missing a cleanup call on that field to ensure this behaviour.
• Further, I don't think the plain text build mode is obsolete now. When sending plaintext newsletters only, there will be no HTML part. Since, I do have some government close clients I do know this is still used. Some institutions do not allow HTML emails at all, so internal newsletters are kept to plain text.

In addition, I am working on including the language chooser in the newsletter block, which already works quite well but is still missing an admin UI for choosing to disable this feature because the simplenews forms are meant to be very slim. But that's actually another issue and maybe shouldn't be discussed here.

Cheers,
Paul

You can't hide the plaintext field based on newsletter vocabulary term.
The field will show up in any case. Providing per-newsletter settings will lead to confusion. The node form is per content type so i strongly recommend to define a plaintext body field per nodetype.

We might introduce a permission such as "manually edit plaintext" to prevent non-knowing users from the plaintext topic confusion.

Is someone still working on this?

subscribing.

and asking where this is at... the UI for this for our client is simple:

add a "Body - Plain Text" field to node and send all newsletters as multipart mimemail and let client's mail reader sort it out.

this post sounded promising when i found it but now not so sure what the module is doing. Display settings don't seem to support this. I would guess that we could disable the html field (which is Body; so can't actually do that but could disable Body and make a cck Body field) from going in the plain text email; but we have no need to ever send as plain text... we only will ever send as multipart. So, closest to that i guess is send as html (is this just labeled wrong? is it really JUST html or is it multipart?)

also, don't really care about the ahah convert to text and fill plain text field.. kind of a cool feature; but not a must have by a long shot - whereas not being able to have users read text or html makes snews unusable for us.

I'll do some testing to see if i can sort out what is there - which despite display setting options; i dont think there is any support at the moment for handling the real issue here (can we define what the plaintext version looks like).

another option of course (other than multipart) is to do what Notifications does; which allows user to select the format they want and then snews will send the email with text fields to text users and the email again with html fields to html users. But multipart seems like a better solution.

i can do the code to "fix" this; just not sure yet what is there or is required/preferred - maybe the patches here do the right thing?

looking at patch from #13, looks like the right way to go; but patch fails on 6.x-2.0-alpha2 rev

i'll apply manually and see how it works.

yes, i agree that defining the field per node is overkill. should be per type.

no luck yet with manually patching as current code is missing part in simplenews.module which are being patched. looking deeper.

attached patch takes the per type approach.

it allows designating a cck field in the node as the plain text version of the newsletter to be added to the mimemail mulitpart msg.

i still have a few issues/questions with what i did here:

i mostly tried to follow the logic that the earlier (per node) patch was using but either i followed it wrong; or the logic is just wrong.

1. i think the text_alternative parameter should be set to either the ptext cck field if it exists and is not empty or to simplenews_html_to_text(body field) - does that seem correct. currently it is only set to the field designated as the plain text field.

2. i am not too sure the mail format selector makes any sense, why would we want to ever send plain text and should html not really say multipart? but maybe i am still missing something here.

i did a quick test (selecting html mail format, which is really multipart) and sure enough (once i found a text mail reader, because setting Outlook to read as plain text simply converts the html part of the multipart to text) then i do see my cck plain field as the msg and if i read in an html mail reader i get the body field in the mail. But, issue with plain text version is the footer is lost - why is that?

btw - the patch applies to latest -dev version of simplenews.

ok, since it is drupal_mail that tacks on the footer to the body i think best is for snews to do the same for alternative text (i.e. plain text part of multipart). so this patch adds this and with this i now get this:

node type edit form: http://screencast.com/t/64gW836trIVv
newsletter issue edit: http://screencast.com/t/cX6FdDjn
plain text result email: http://screencast.com/t/MRscROTuu7
html version of email: http://screencast.com/t/evFYFTblJUL

@todo:

- still not sure if my cck field is undefined or empty i should be putting simplenews_html_to_text(body field) in as text_alternative - but maybe no need as the mail client will just do its best with the html version

- i guess cck plain textfield should have a token replace performed - yes, obvious.. so will add patch with that in a minute.

patch to match with #22, but still i see issues..

i think text alternative field is only valid for mimemail (is this true?) so it should likely be wrapped in a if mimemail exists.

yes, i need to insert cleaned body if my custom cck field is empty... or i get nothing in plain text mail client.

i still think might be some issues with the order i do things for token handling.. but maybe someone else can take a look. this works pretty good now for my needs.

We have discussed this need here. In order to have a clean setup, we decided to
- Use a field to (optionally) store plaintext node replacement
- Define a widget for this textfield to allow automated plaintext creation as a template
-- We still stay with simplenews view modes plaintext and html
-- The plaintext view mode is used for plaintext suggestion
- When sending out, the optional plaintext field overrides the plaintext variant.

Changing assignment. :-)

@Berdir

Here's the patch. I diffed it from you commit f986e02c747c0b2742a02f6feb539f1da37880d3 "Issue #805114 by Berdir: Added missing multi signup block template." cause i was not able to diff from origin/7.x-1.x .

I could not load the JS in simplenews_plaintext using #attached. It's loaded in a preprocess_page hook at the moment which is obviously wrong. But it was the only way i could get the JS to run and i needed the JS to see a result to develop other parts of the module.

Please review/improve.

Patch:

Uploading the patch without the # to see if it applies. Will do a review in the next post.

And here is the same patch against the most recent 7.x-1.x.

I had a very weird issue when trying to apply the patch with git apply, only patch worked.

Both a review and thinking out loud about some parts below... We'll see next week who has time to continue working on this one.

Maybe I'll find time this weekend to start working on the cache issue, might make this a bit easier to handle, although it would require quite a re-roll probably.

+++ b/includes/simplenews.mail.inc
@@ -195,7 +195,7 @@ function simplenews_mail_mail($msgbase, $key = 'node') {
     // @todo Create the plaintext portion of the message, we don't have $message['body'] here.
-    // $params['plaintext'] = $params['plain'] ? $message['body'] : simplenews_html_to_text($message['body'], TRUE);
+    //$params['plaintext'] = $params['plain'] ? $message['body'] : simplenews_html_to_text($message['body'], TRUE);
     // @todo Get the attachments. Upload module no longer exists for Drupal 7.

Just a space changed and even the wrong way round...

+++ b/simplenews.install
@@ -341,6 +341,16 @@ function simplenews_install() {
+/*
+ * Enable plaintext widget module if there are any
+ * HTML mail transmission modules available.
+ */
+function simplenews_enable() {
+  if (simplenews_html_transmitter_modules()) {
+    module_enable(array('simplenews_plaintext'));
+  }
+}
+

Not convinced about this. If the plaintext module is an optional feature in a sub-module, why automatically enable it?

+++ b/simplenews.module
@@ -662,13 +662,23 @@ function simplenews_get_category_field($bundle_name) {
  * Add checkbox to the content type form to use the content type as newsletter.
+ * Add Selectbox for plaintext field selection.
  */
 function simplenews_form_node_type_form_alter(&$form, $form_state) {
-  // Add option to use content type as simplenews newsletter.
-  $form['workflow']['simplenews_content_type'] = array(
+
+  $form['simplenews'] = array(
+    '#type' => 'fieldset',
+    '#title' => 'Newsletter',
+    '#collapsible' => TRUE,
+    '#collapsed' => FALSE,
+    '#group' => 'additional_settings',
+  );
+  ¶
+  // Add option to use content type as simplenews newsletter.  ¶
+  $form['simplenews']['simplenews_content_type'] = array(

Not sure about adding another fieldset. It made sense when we had a second option, but adding a fieldset for just a single option is a but too much I think.

Maybe open a different issue where it can be discussed separatly if you like this change.

+++ b/simplenews.module
@@ -1956,24 +1965,17 @@ function simplenews_build_node_mail(&$message, $params) {
+    /*
+    $plain_build = simplenews_build_message_body('plain');
+    $plain_body = theme('simplenews_newsletter_body', array('build' => $plain_build, 'category' => $category, 'language' => $message['language']));
+    $messages[$nid][$langcode]['plain_body'] = $plain_body;
+    */

This part is commented out, what's it supposed to do?

+++ b/simplenews.module
@@ -2008,21 +2010,66 @@ function simplenews_build_node_mail(&$message, $params) {
-  if ($category->format == 'plain') {
+  if ($category->format == 'plain' && !trim($context['node']->{$field_plaintext}['und'][0]['value'])) {
     module_load_include('inc', 'simplenews', 'includes/simplenews.mail');
     $message['body']['body'] = simplenews_html_to_text($message['body']['body'], $category->hyperlinks);
   }
+  else if ($category->format == 'plain') {

field_get_items() should be used here instead of the hardcoded 'und'.

+++ b/simplenews.module
@@ -2008,21 +2010,66 @@ function simplenews_build_node_mail(&$message, $params) {
+  ¶
+  /************ ORIG ********************
+  // Build message body
+  // @todo move this code to a function. We need to do this twice for HTML and Plain text Alternative.
+  // @todo restore the format selection.
+  //$build = node_view($node, 'email_' . $category->format);
+  // Supported view modes: 'email_plain', 'email_html', 'email_textalt'
+  $build = node_view($node, 'email_plain');
+  // @todo Use simplenews_newsletter_body as #theme?
+  unset($build['#theme']);
+  foreach (element_children($build) as $child) {
+    $build[$child]['#theme'] = 'simplenews_field';
+  }
+  /********** /ORIG ***********************/
+}
+
+// END move this code to a function.

Looks like this part can be removed.

+++ b/simplenews.module
@@ -2966,6 +3019,12 @@ function template_preprocess_simplenews_newsletter_body(&$variables) {
+  if ($variables['category']->format == 'html') {
+    // hide the plaintext field for html mails since the plaintext part has it's own theme function
+    $field_plaintext = simplenews_get_plaintext_field_name($variables['build']['#bundle']);
+    hide($variables['build'][$field_plaintext]);
+  }

It would probably be nice to make that function return an array of fields and then loop over them.

+++ b/simplenews.module
@@ -3030,6 +3089,21 @@ function theme_simplenews_newsletter_footer($variables) {
+/**
+* Theme the newsletter message subject and alternative plaintext body. ¶
+*/

Quite a few trailing whitespaces. I suggest to configure your editor to remove them. Netbeans has an option to only do it on changed lines, not sure about Eclipse.

+++ b/simplenews.module
@@ -3030,6 +3089,21 @@ function theme_simplenews_newsletter_footer($variables) {
+function theme_simplenews_newsletter_plaintext_body($variables) {
+  // In an override of this function, $text can be loaded with other field from the node.
+  $text  = $variables['node']->title . "\n\n";
+  if ($variables['plain_field'] == 'body') {
+    $text .= $variables['node']->body['und'][0]['value'] . "\n\n";
+  } ¶
+  else {
+    $plain = $variables['node']->$variables['plain_field'];
+    $text .= $plain['und'][0]['value'] . "\n\n";
+  }
+  return $text;
+}

The body special case is probably a left-over from D6 where body wasn't a field. It's quite pointless now. Also, this should probably use field_view_field or something like that. Given that, not sure if we actually need a theme function anymore.

+++ b/simplenews.module
@@ -3345,3 +3419,39 @@ function simplenews_require_double_opt_in($tid, $account) {
+
+/*
+ * Function either checks if a compatible transmitter
+ * module exists or gets all possible transmitter modules.
+ */

First line of a function docblock should be a single line and shorter.

Maybe something like "Checks if a html mail transmitter module is enabled."

Should also contain @return, something like:
@return
TRUE if html mails can be sent, FALSE if not.

+++ b/simplenews.module
@@ -3345,3 +3419,39 @@ function simplenews_require_double_opt_in($tid, $account) {
+function simplenews_html_transmitter_modules() {

maybe rename the function to something like "simplenews_html_mails_supported()" or something like that?

+++ b/simplenews.module
@@ -3345,3 +3419,39 @@ function simplenews_require_double_opt_in($tid, $account) {
+ * Note: At the moment only one (the first) plaintext field ¶
+ * will be detected. ¶

Yeah, what I said above :)

+++ b/simplenews.module
@@ -3345,3 +3419,39 @@ function simplenews_require_double_opt_in($tid, $account) {
+      // check if field has a plaintext_textarea widget
+      if ($info['widget']['type'] != 'plaintext_textarea') continue;
+      else return $info['field_name'];

Always use curly braces for conditions.

+++ b/simplenews_plaintext/simplenews_plaintext.info
@@ -0,0 +1,6 @@
+;files[] = text.test
\ No newline at end of file

Missing newline at end of file, the commented out files declaration should be removed or tests added :)

+++ b/simplenews_plaintext/simplenews_plaintext.js
@@ -0,0 +1,9 @@
+(function ($) {
+
+	// Defines an ajax command to replace content inside ¶
+	// the plaintext field after serverside processing
+	Drupal.ajax.prototype.commands.replacecontent = function (ajax, response, status) {
+	  $('#' + response.trigger).parent().find('textarea').val(response.plaintext);
+	}
+

Tabs.

+++ b/simplenews_plaintext/simplenews_plaintext.module
@@ -0,0 +1,205 @@
+ * @file ¶
+ * ¶
+ * Defines a text widget for content types that are used as simplenews newsletters.
+ * This can only be used by newsletters that are sent as html mails and use a specified
+ * field for their plaintext content. The widget consists of a textarea and an additional
+ * 'process' button which allows a serverside conversion of the typed text into plaintext.
+ * ¶
+ * Notes
+ * Don't use "Filtered text" with the plaintext field.

Comments should be wrapped at 80 characters.

+++ b/simplenews_plaintext/simplenews_plaintext.module
@@ -0,0 +1,205 @@
+  return array(
+    'plaintext_textarea' => array(
+      'label' => t('Textarea for Plaintext (multiple rows)'),
+      'field types' => array('text_long'),

Would probably be good to use the module name in the widget name for proper namespacing.

+++ b/simplenews_plaintext/simplenews_plaintext.module
@@ -0,0 +1,205 @@
+  ¶
+  /* This doesn't work, had to load the js using hook_preprocess_page() (see code below)
+  $jspath = drupal_get_path('module', 'simplenews_plaintext') . '/simplenews_plaintext.js';
+  $form['#attached']['js'] = array($jspath); || drupal_add_js($jspath, 'file');
+  */
+  ¶

Will need to look into this.

+++ b/simplenews_plaintext/simplenews_plaintext.module
@@ -0,0 +1,205 @@
+    '#value' => t('Process'), // @todo maybe give the label a better text

Maybe "Generated plain text" or "Fetch generated plain text"?

+++ b/simplenews_plaintext/simplenews_plaintext.module
@@ -0,0 +1,205 @@
+      'progress' => array(
+        'type' => 'bar',
+        'message' => t('Please wait while your text is being processed on the server.')
+      ),

bar just measures the time until the sending is finished, which is e.g. useful when uploading large files. I doubt that text is actually even visible long enough to read it under normal instances.

+++ b/simplenews_plaintext/simplenews_plaintext.module
@@ -0,0 +1,205 @@
+  ¶
+  // gather plaintext values from relevant fields
+  /*
+  // the way miro wanted it to be done, unfortunately ¶
+  // links like 'edit' or 'delete' get rendered too this way
+  $data = node_view($node, 'email_plain');
+  $text = render($data);
+  */
+  ¶
+  // code below pulls the textdata out of $form_state directly
+  $text = '';
+  $field_types = array('text', 'text_long', 'text_with_summary');
+  foreach($form_state['field'] as $field_name => $field) {
+    // skip empty fields
+    if (!isset($form_state['values'][$field_name]['und'][0]['value']) || empty($form_state['values'][$field_name]['und'][0]['value'])) continue;
+    if (in_array($field['und']['field']['type'], $field_types)
+        && $field['und']['instance']['widget']['type'] != 'plaintext_textarea') {
+      $label = $field['und']['instance']['label'];
+      if (count($form_state['values'][$field_name]['und']) > 1) {
+        $text .= $label . '<br />';
+        foreach($form_state['values'][$field_name]['und'] as $key => $val) {
+          $text .= '<p>' . t('Value') . ' ' . ($key + 1) . '<br />' . $val['value'] . '</p>';
+        }
+      }
+      else {
+        $value = $form_state['values'][$field_name]['und'][0]['value'];
+        if ($field['und']['instance']['display']['email_plain']['label'] != 'hidden') {
+          $text .= '<p>' . $label . '<br />' . $value . '</p>';
+        }
+        else {
+          $text .= '<p>' . $value  . '</p>';
+        }
+      }
+    }
+  }

Will need to check this. Would be nice if node_view() would work, the code below at least needs to be reworked to use the correct language..

+++ b/simplenews_plaintext/simplenews_plaintext.module
@@ -0,0 +1,205 @@
+  // convert html values of fields to plaintext
+  module_load_include('inc', 'simplenews', 'includes/simplenews.mail');
+  $plaintext = simplenews_html_to_text($text);
+
+  // send the plaintext value to a client side js command

Comments should start with a upper case and end with a "."

Assigning to Berdir. Can you please make a suggestion?
We possibly postpone this to AFTER beta / stable.

Promoting this to 8.x

People learned to live without it... and one might claim this feature is far from critical...
However, bad quality alternative fallback mails are bad and in D8 we tried to push newsletter quality.

Previewing the alternative text and content awareness of what a user sends (or recipients might see) is still of the goal list.

Setting to active as i guess we need to start over again.