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
| Comment | File | Size | Author |
|---|
Issue fork webform-3593810
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
Comment #2
dalinComment #3
mukeshaddweb commentedAdded 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.
Comment #4
dalin@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...
Comment #5
liam morlandComment #7
mukeshaddweb commentedComment:
Created MR: https://git.drupalcode.org/project/webform/-/merge_requests/877
Comment #8
dalinThanks @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).
Comment #9
erlingx commentedI tested the patch and validation works correctly with an empty custom body. But also agree with @dalin:
Comment #10
liam morlandThis looks like a reasonable approach. It should have a test.
Comment #11
erlingx commentedAdded 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
Comment #12
drupal.ninja03 commentedI 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.
Comment #13
erlingx commentedI Rebased on 6.3.x
The 2 tests for this issue passed successfully.
The pipeline failure is from an unrelated WebformVariantPluginTest - filesystem permission issue.