Fix sending emails when storing invoices in S3

[michaelKaefer]

Fixes:

Handling "Sylius\InvoicingPlugin\Event\OrderPaymentPaid" failed: Handling "Sylius\InvoicingPlugin\Command\SendInvoiceEmail" failed: Unable to open file for reading [/.../shop/private/invoices//2021_07_000000001.pdf]

Reason: when storing invoices on S3 the directory /private/invoices does not exist, so it is necassary to readd the TemporaryFilesystem and use it for sendending the invoice emal.

[GSadee]

Hello @michaelKaefer, first of all, sorry for not answering for a long time. Probably I'm not 100% aware of the problem and it is more complex than I think, but isn't it enough to create a proper directory (/private/invoices in this case) on your server to fix the issue? We've introduced the functionality to save PDF files partially to avoid the necessity of generating the temporary file before sending mails.

[michaelKaefer]

@GSadee I guess this should work too, I did not try. But it means to force everbody who uses S3 (or similar) to create and never delete a directory ./private/invoices which will be empty most of the time and serves exactly the purpose of the /tmp directory to hold temporary files. In my opinion reintroducing the old TemporaryFilesystem seems cleaner if there is no downside on it.

[Zales0123]

Hi @michaelKaefer 🖖 As far as I see, the invoices path is parametrized, so it should be fairly easy to change it if it's needed. Instead of these changes, maybe we could extract the hardcoded "%kernel.project_dir%/private/invoices/" to env variable (e.g. SYLIUS_INVOICING_INVOICES_PATH)? It would allow to make it even easier to store invoices in desired directory, while not changing in the code itself 🚀

[michaelKaefer]

@Zales0123 I am not sure if that is a good solution:

• With S3 the invoice is never saved to the filesystem of the application server, so it does not seem to make sense to configure a SYLIUS_INVOICING_INVOICES_PATH for the application server. The only time when the invoice is temporarily saved to the application server is when sending it in an email, afterwards it can be deleted anytime, so this should be some temporary folder like /tmp I guess.
• More important: I think if Sylius removes swiftmailer/swiftmailer in favour of the newer symfony/mailer then there is no more need for SYLIUS_INVOICING_INVOICES_PATH and also my fix could be removed again because then it is no more necessary to temporarily save the file to the application server at all.

[Zales0123]

Hello @michaelKaefer and sorry one more time for the silence 🖖 However, I'm still not convinced should we change it the way you propose :/ I see your point, but I believe it's a fix/change done on the wrong repository. The Invoicing plugin just uses the SyliusMailerBundle, which is responsible for sending emails, with everything that comes with it. There is already a PR opened which, as I understand from your previous message, would fix this problem. So I say we should push this PR forward - and I can say it's a priority for me to release a new mailer bundle with the Symfony mailer support. Then, we can bump the requirement for SyliusMailerBundle here or in Sylius itself and everything should be fine... am I right? 😄 Cheers 🚀

[michaelKaefer]

@Zales0123 Thank you for your answer.

• If it should here or in SyliusMailerBundle I really don't know. For me it would make sense to fix it here - just because it was like this before: Remove unneeded TemporaryFilesystem service #240
• Yes, I think Add an adapter to support the Symfony Mailer component and deprecate the Swiftmailer integration SyliusMailerBundle#102 should fix it as well (but to mention it: the PR just deprecates swiftmailer, it does not remove it yet)

[mbabker]

IMO, this PR is mostly correct. The Sylius Mailer API implicitly requires the $attachments parameter to be a list of file paths, using any other type of data structure requires a custom implementation of the adapter to break the implicit API contract (i.e. before switching to the Symfony Mailer component, we were using a custom Swiftmailer adapter and our app was passing Swift_Mime_Attachment objects through the API). Where this probably would fall apart is if you're using the Messenger component in an async config and the attachment file is deleted before Messenger processes the email message.

[michaelKaefer]

@mbabker I forgot about that but I checked it and it should not be a problem when using an async Messenger config: everytime the message is handled the invoice file is generated if it cannot be found.

[gabrielmustiere]

Hi @GSadee and @michaelKaefer, do you have any news about this problem or this PR, I have the same problem with a storage of invoices on S3, I tried to leave the folder private/invoices on the server but nothing helps.