Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Attachments show up correctly again now #92

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

schneider-felix
Copy link
Member

This PR fixes #88

Because of the way Shopware handles attachments, the attachments are not resolved before the MailTransport has run. That's why we need to save attachments in the Subscriber.

While I was at it I also improved the UI of attachments.
image

@schneider-felix schneider-felix force-pushed the fix-attachments branch 2 times, most recently from 3ac50a1 to 8f9a9fb Compare October 6, 2024 19:28
@flkasper
Copy link
Contributor

flkasper commented Oct 7, 2024

@schneider-felix It would be desirable if the change were also integrated in 2.x. Shopware 6.5 is still used quite frequently.

@schneider-felix
Copy link
Member Author

@schneider-felix It would be desirable if the change were also integrated in 2.x. Shopware 6.5 is still used quite frequently.

@flkasper Sure, I will backport this to the 2.x branch, after @tinect or @shyim had the chance to review this change.

Copy link
Member

@tinect tinect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to handle that none of the Events are called when the message is not sent by queue?
The result keeps, that no attachment would be saved.

On short check, I could not find a better way. What do you think?

@schneider-felix
Copy link
Member Author

schneider-felix commented Oct 9, 2024

This might also solve #93

The BCC recipient set in the MailSender should be considered with this change.

https://github.com/shopware/shopware/blob/trunk/src/Core/Content/Mail/Service/MailSender.php#L49

@schneider-felix schneider-felix linked an issue Oct 9, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BCC and CC adresses Attachments always zero
4 participants