Problem/Motivation

In the Drupal 7 version of this module, there is a checkbox called "Exclude empty components" when creating an email handler. In the Drupal 8 version this is no longer offered and empty components are just ignored when sending the email.

Proposed resolution

I suggest adding a checkbox called 'Show empty elements" when defining the email handler that will therefore include empty values in the email output.

A workaround can be to create a fully custom email template where you specify label + [webform-submission:values:key], but this is far from ideal.

Remaining tasks

The patch will need to be reviewed and it'll be ideal to include tests as well.

User interface changes

There is a new checkbox in the settings area when defining an email handler.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fjgarlin created an issue. See original summary.

fjgarlin’s picture

Issue summary: View changes
fjgarlin’s picture

Status: Active » Needs review
FileSize
2.91 KB

This patch adds the desired checkbox and displays empty values when creating the email template, so we just need to use: [webform-submission:values].

Status: Needs review » Needs work

The last submitted patch, 3: option_to_include_empty-2888717-3.patch, failed testing. View results

fjgarlin’s picture

Status: Needs work » Needs review
FileSize
4.75 KB

Updated patch with missing configuration.

Status: Needs review » Needs work

The last submitted patch, 5: option_to_include_empty-2888717-5.patch, failed testing. View results

jrockowitz’s picture

@fjgarlin The feature and patch makes sense to me.

Please include a simple test in \Drupal\webform\Tests\Handler\WebformHandlerEmailAdvancedTest that verifies empty elements are included.

fjgarlin’s picture

Hi @jrockowitz,

I'm not very experienced with the testing procedure so it took me a while to figure out, thanks for suggesting though. I added simple test in the patch, run the tests on my D8 installation and everything went green.

New patch re-uploaded.

fjgarlin’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 8: option_to_include_empty-2888717-8.patch, failed testing. View results

fjgarlin’s picture

Status: Needs work » Needs review
FileSize
10.66 KB

Previous patch used git add -N new_file and the git diff assumed that it was an existing file.
For this one I used git add and then git diff --cached. I hope it applies this time.

Status: Needs review » Needs work

The last submitted patch, 11: option_to_include_empty-2888717-11.patch, failed testing. View results

jrockowitz’s picture

@fjgarlin I think there are two issues here.

  • The remote post handler should always include empty values. This should be the default hard-coded behavior.
  • The email handlers could optionally include empty values. This is going to be very challenging because conditional logic would create empty values that should probably not be emailed.

Some minor tweaks...

  • We should change 'show_empty_elements' to 'include_empty' because the settings we are altering is 'excluded_elements'.
  • Latest dev has a new 'ignore_access' settings checkbox, the 'include_empty' settings should be appear after this checkbox.
jrockowitz’s picture

I am going to work on making this feature also applicable to a previews elements.

  • jrockowitz committed 33f2231 on 2888717-exclude-empty
    Issue #2888717 by fjgarlin: Option to include empty components in [...
  • jrockowitz committed d9697c5 on 2888717-exclude-empty
    Issue #2888717 by fjgarlin: Option to include empty components in [...

  • jrockowitz committed 1fe41f7 on 2888717-exclude-empty
    Issue #2888717 by fjgarlin: Option to include empty components in [...
jrockowitz’s picture

Status: Needs work » Needs review
FileSize
116.87 KB

@fjgarlin I took a similar approach but went with a different variable name (exclude_empty).

Status: Needs review » Needs work

The last submitted patch, 17: option_to_include_empty-2888717-17.patch, failed testing. View results

  • jrockowitz committed e382ec6 on 2888717-exclude-empty
    Issue #2888717 by fjgarlin: Option to include empty components in [...
jrockowitz’s picture

Status: Needs work » Needs review
FileSize
117.51 KB

  • jrockowitz committed 495eb6d on 8.x-5.x
    Issue #2888717 by fjgarlin, jrockowitz: Option to include empty...
jrockowitz’s picture

Status: Needs review » Fixed

I committed that patch. Please download the latest dev release to review.

fjgarlin’s picture

Hi @jrockowitz, that's great! Thanks for picking this up and improve. Will download and test.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.