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.

Sign up for GitHub

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

Jump to bottom

Allow to resend e-mails when transport fails #54

Merged
merged 34 commits into from Oct 14, 2023
Merged

Allow to resend e-mails when transport fails #54

merged 34 commits into from Oct 14, 2023

Conversation

schneider-felix
Copy link
Contributor

With this PR E-Mails are stored, when the transport fails as well. They are flagged and highlighted in the Administration.
image
image
This is useful in case the Mail Transport was incorrectly configured. In that case the user can simply check the settings and try again later.

This PR only supports en-GB and de-DE.

@shyim
Copy link
Member

shyim commented Sep 12, 2023

Really nice feature idea! :)

tinect
tinect requested changes Sep 12, 2023
View reviewed changes
src/Services/MailSender.php Outdated Show resolved Hide resolved
schneider-felix and others added 3 commits September 13, 2023 00:47
@schneider-felix @tinect
Fix german translation for transportFailed alert
40c7975 
Co-authored-by: tinect <s.koenig@tinect.de>
@schneider-felix @tinect
Fix english translation for mail delivery failed alert
250cf35 
Co-authored-by: tinect <s.koenig@tinect.de>
@schneider-felix @tinect
Improve readability of SaveMail in MailSender
257ddaa 
Co-authored-by: tinect <s.koenig@tinect.de>
@tinect
Copy link
Member

tinect commented Sep 12, 2023 •
edited

Idea from discussion:
we could keep track of resent mails in a dedicated column source_mail_id which needs to filled when resending mails.

What should be checked: Does tracking the success of sending mails catch sending mails through the messenger? https://developer.shopware.com/docs/guides/hosting/infrastructure/message-queue.html#sending-mails-over-the-message-queue

@schneider-felix
Copy link
Contributor Author

Improved the overall UX in regards to resending e-mails.
image
Mails now contain a resend history to keep track of associated Entries in the Mail Archive. Furthermore we now send a notification when the resend is successful or fails.

Maybe have a look @tinect

I'll check if Mails sent using the Queue are saved as well, but I believe they should.

tinect
tinect requested changes Sep 14, 2023
View reviewed changes
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.

I love this implement!!! 😍
I added a few feedbacks.

src/Resources/app/administration/src/module/frosh-mail-archive/snippet/en-GB.json Outdated Show resolved Hide resolved
src/Controller/Api/MailArchiveController.php Show resolved Hide resolved
src/Services/MailSender.php Outdated Show resolved Hide resolved
...e/frosh-mail-archive/component/frosh-mail-resend-history/frosh-mail-resend-history.html.twig Outdated Show resolved Hide resolved
...e/frosh-mail-archive/component/frosh-mail-resend-history/frosh-mail-resend-history.html.twig Outdated Show resolved Hide resolved
src/Services/MailSender.php Outdated Show resolved Hide resolved
src/Services/MailSender.php Outdated Show resolved Hide resolved
src/Services/MailSender.php Outdated Show resolved Hide resolved
schneider-felix and others added 8 commits September 14, 2023 15:47
@schneider-felix @tinect
Update src/Resources/app/administration/src/module/frosh-mail-archive…
5fc6b74 
…/snippet/en-GB.json

Co-authored-by: tinect <s.koenig@tinect.de>
@schneider-felix @tinect
Disable Link for current mail id
81e9731 
Co-authored-by: tinect <s.koenig@tinect.de>
@schneider-felix @tinect
Disable link for current mail
429c5e3 
Co-authored-by: tinect <s.koenig@tinect.de>
@schneider-felix @tinect
Codestyle src/Services/MailSender.php
14fe20d 
Co-authored-by: tinect <s.koenig@tinect.de>
@schneider-felix @tinect
Codestyle src/Services/MailSender.php
b331406 
Co-authored-by: tinect <s.koenig@tinect.de>
@schneider-felix @tinect
Codestyle src/Services/MailSender.php
a839bfe 
Co-authored-by: tinect <s.koenig@tinect.de>
@wirduzen-felix
Change MailSender import
c7bdb4d
@wirduzen-felix
Symfony 6.2 compatibility
8090503
@schneider-felix
Copy link
Contributor Author

I love this implement!!! 😍 I added a few feedbacks.

Thank you very much for your feedback :)

@schneider-felix
Copy link
Contributor Author

I finally had time to work on this PR again. I would love to hear your feedback.

@tinect
Copy link
Member

tinect commented Oct 14, 2023

Would you please Migrate to MessageId to be used as id for mailArchive? then we don't need this additional field and save some logic.

tinect
tinect requested changes Oct 14, 2023
View reviewed changes
src/Subscriber/MailTransportSubscriber.php Outdated Show resolved Hide resolved
src/Subscriber/MailTransportSubscriber.php Outdated Show resolved Hide resolved
src/Services/MailSender.php Outdated Show resolved Hide resolved
@schneider-felix @tinect
Update src/Subscriber/MailTransportSubscriber.php
9622708 
Co-authored-by: tinect <s.koenig@tinect.de>
@tinect tinect requested a review from shyim October 14, 2023 21:40
@schneider-felix schneider-felix mentioned this pull request Oct 14, 2023
Closed
@tinect tinect merged commit 1a8fd31 into FriendsOfShopware:main Oct 14, 2023
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tinect tinect tinect approved these changes

@shyim shyim shyim approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@schneider-felix @shyim @tinect @wirduzen-felix