Problem/Motivation

Steps to reproduce

1. Create a webform
2. Set up an email handler, set the "Body" type to "Custom Body", but clear all the default text from the WYSIWYG field. Save
3. Submit the webform.

There's a fatal PHP error

Drupal\Core\Entity\EntityStorageException: "$value" must have at least 1 characters. Got: 0 in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 817 of /code/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

Proposed resolution

Technically, this is a problem with SendGrid module (#3593813: Fatal PHP error if something tries to send an email with an empty body), but I think Webform should also make the body required if there's a custom body.

Remaining tasks

Add something (#states ? constraint?) to ensure that if there's a custom body, that the body is not empty.

User interface changes

An error is thrown if it's set to a custom body, but the body is empty.

API changes

none

Data model changes

none

Issue fork webform-3593810

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

dalin created an issue. See original summary.

dalin’s picture

mukeshaddweb’s picture

Status: Active » Needs review
StatusFileSize
new740 bytes

Added server-side validation in validateConfigurationForm() to prevent saving an empty custom body in the email handler.
When "Body" is set to "Custom body" and the editor is cleared, $values['body'] becomes an empty string which is silently saved.
On webform submission the email handler fires with an empty body, causing a fatal EntityStorageException.
The fix adds an empty check after assigning the custom body value and sets a form error, preventing the invalid configuration from
being saved in the first place.

dalin’s picture

Status: Needs review » Needs work

@mukeshaddweb

Rather than adding a patch file, can you open a merge request with your change?
https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr...

liam morland’s picture

Issue tags: +Needs merge request

mukeshaddweb’s picture

Status: Needs work » Needs review
dalin’s picture

Issue tags: -Needs merge request

Thanks @mukeshaddweb

This LGTM,
But the maintainers will need to comment on whether this is the correct approach, or if they prefer something different (like a Constraint).

erlingx’s picture

I tested the patch and validation works correctly with an empty custom body. But also agree with @dalin:

But the maintainers will need to comment on whether this is the correct approach, or if they prefer something different (like a Constraint).

liam morland’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

This looks like a reasonable approach. It should have a test.

erlingx’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

Added test coverage for empty custom body validation (text and HTML).

Tests verify that:
- Validation error is shown when custom body is empty
- Handler configuration is not saved with empty body

drupal.ninja03’s picture

Status: Needs review » Needs work

I reviewed MR !877.

The approach looks reasonable: it adds validation for the email handler custom body field and includes functional test coverage for empty plain-text and HTML custom body values.

However, I do not think this is ready for RTBC yet because the MR overview still shows merge/pipeline issues:

- The MR is blocked because fast-forward merge is not possible.
- The source branch is 4 commits behind the target branch and needs a rebase.
- The test summary shows 452 failed tests out of 2138.
- Code Quality reports 73 new findings.

Leaving as Needs work until the branch is rebased and the pipeline/test results are clean or explained.

erlingx’s picture

Status: Needs work » Needs review

I Rebased on 6.3.x
The 2 tests for this issue passed successfully.
The pipeline failure is from an unrelated WebformVariantPluginTest - filesystem permission issue.