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.

CommentFileSizeAuthor
#20 option_to_include_empty-2888717-20.patch117.51 KBjrockowitz
#17 option_to_include_empty-2888717-17.patch116.87 KBjrockowitz
#11 option_to_include_empty-2888717-11.patch10.66 KBfjgarlin
#8 option_to_include_empty-2888717-8.patch10.73 KBfjgarlin
#5 option_to_include_empty-2888717-5.patch4.75 KBfjgarlin
#3 option_to_include_empty-2888717-3.patch2.91 KBfjgarlin
Screen Shot 2017-06-23 at 10.00.04.png98.79 KBfjgarlin

Comments

fjgarlin created an issue. See original summary.

fjgarlin’s picture

fjgarlin commented
Issue summary: View changes
fjgarlin’s picture

fjgarlin commented
Status: Active » Needs review
StatusFileSize
new option_to_include_empty-2888717-3.patch2.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

fjgarlin commented
Status: Needs work » Needs review
StatusFileSize
new option_to_include_empty-2888717-5.patch4.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

jrockowitz commented

@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

fjgarlin commented
StatusFileSize
new option_to_include_empty-2888717-8.patch10.73 KB

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

fjgarlin commented
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

fjgarlin commented
Status: Needs work » Needs review
StatusFileSize
new option_to_include_empty-2888717-11.patch10.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

jrockowitz commented

@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

jrockowitz commented

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

jrockowitz commented
Status: Needs work » Needs review
StatusFileSize
new option_to_include_empty-2888717-17.patch116.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

jrockowitz commented
Status: Needs work » Needs review
StatusFileSize
new option_to_include_empty-2888717-20.patch117.51 KB

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

jrockowitz commented
Status: Needs review » Fixed

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

fjgarlin’s picture

fjgarlin commented

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.