Problem/Motivation

Currently the code always looks for $attachment['filepath']. We can extend this to also check $attachment["filecontent"].

Steps to reproduce

Modules:

webform: 6.2.x with submodule: webform_entity_print_attachment
entity_print: 2.8.0

- Add webform with an Attachment PDF element
- Add an email handler to send the PDF as an attachment.

Proposed resolution

See patch https://www.drupal.org/files/issues/2022-12-06/3261807-51.patch

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#4 symfony_mailer.attach-content.3328847-4.patch2.79 KBadamps

Comments

jungle created an issue. See original summary.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Title: Failed to send email via symfony mailer with Attachment PDF element type in Webform » Failed to send email via symfony mailer with element type: Attachment PDF in Webform
adamps’s picture

adamps commented
Title: Failed to send email via symfony mailer with element type: Attachment PDF in Webform » Support attachements with 'filecontent'
Category: Bug report » Feature request
Issue summary: View changes
Status: Needs review » Needs work

Thanks.

The patch looks like a good start. I think we can simplify quite a lot - only 3 files need changing.

1) I don't think we need WebformEmailBuilder. The requirement may be in other modules, not only Webform. So LegacyEmailBuilder should handle it. We don't need $email->setParam('attachments' which I think is unused.

2) AttachmentEmailAdjuster is a good idea to provide some security/policy to check attachments. However it's not part of this patch instead we should have a separate issue.

3) The attach() method belongs in BaseEmailInterface and BaseEmailTrait alongside the existing attachFromPath().

4) public function attach(string $content, ?string $name = NULL, ?string $mimeType = NULL);
Don't need the '?'. Let's call the parameter $body to match symfony.

adamps’s picture

adamps commented
Status: Needs work » Needs review
StatusFileSize
new symfony_mailer.attach-content.3328847-4.patch2.79 KB
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

@AdamPS, Many thanks for pushing this forward and thanks for the patch.

+++ b/src/LegacyMailerHelper.php
@@ -86,7 +86,12 @@ class LegacyMailerHelper implements LegacyMailerHelperInterface {
+      if (!empty($attachment['filepath'])) {
...
+      elseif (!empty($attachment['filecontent'])) {

One point, is the order matters here? Should!empty($attachment['filecontent']) go first? the original patch did that. I do not really understand the related logic behind it. But Let's take a generic case as an example of reading a file. The file content is read from a file path, and if the file content and file path are both set, later on, if the file content exists, the file path may not be reached.

adamps’s picture

adamps commented

One point, is the order matters here?

I copied the order from swiftmailer module, which seems best for compatibility between the two.

If someone could test this with WebFrom and confirm it works then I will commit. We're not far from a stable release now, and this issue should be in it.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Status: Needs review » Reviewed & tested by the community

Tested manually against a webform (a private one so that I won't post it here.), and the patch works for me. Thanks!

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Title: Support attachements with 'filecontent' » Support attachments with 'filecontent'

Fix typo in title

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

Close #3321877: TypeError: Drupal\symfony_mailer\Email::attachFromPath()?

  • AdamPS committed be26fef0 on 1.x 
    Issue #3328847 by AdamPS, jungle: Support attachments with 'filecontent'
adamps’s picture

adamps commented
Status: Reviewed & tested by the community » Fixed

Thanks

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

Status: Fixed » Closed (fixed)

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