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
[Documents] Newsletter: add option to maintain a manual edited plain-text version #4794
Conversation
New panel for plain text input field.
The new panel
Getting the data for the new field on save.
Additional Getter and Setter for plain text part
Icon for the new panel
Adding the plain text part to the message if the plain text field is filled in, otherwise existing pimcore procedure via html2text if installed. Now, either the data of the plain text field or the result of html2text will be added to the message as text/plain part to the email.
Added the link to the new plaintextPanel.js
@@ -137,8 +143,6 @@ public static function sendNewsletterDocumentBasedMail(Mail $mail, SendingParamC | |||
|
|||
if (!empty($mailAddress)) { | |||
$mail->setTo($mailAddress); | |||
// Getting bounces |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why removing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary, as the SMTP Server will set it automatically set the return path when finally sending the email.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You added this in #4349 ... can't hurt to leave it as it is, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it was me. :-)
But on what I read in the meantime, I would recommend to leave it out, because some mailserver seems to reject or identify the email as spam, when return path is wrongly set. At the end if we leave it in, it won't hurt the sending process, !maybe! some emails won't receive the customer.
@@ -115,6 +119,8 @@ public static function prepareMail( | |||
|
|||
$mail->setBodyHtml($contentHTML); | |||
$mail->setBodyText($contentText); | |||
// Adds the plain text part to the message, that it becomes a multipart email | |||
$mail->addPart($contentText, 'text/plain'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? See:
Line 567 in d688a67
$this->addPart($bodyTextRendered, 'text/plain'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 567 is part of the send function, the newsletter is send via sendWithoutRendering function:
Pimcore::getEventDispatcher()->dispatch(DocumentEvents::NEWSLETTER_PRE_SEND, $event);
$mail->sendWithoutRendering($mailer);
Pimcore::getEventDispatcher()->dispatch(DocumentEvents::NEWSLETTER_POST_SEND, $event);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point 👍😊
@@ -52,6 +52,8 @@ | |||
"send_newsletter": "Newsletter JETZT versenden", | |||
"object_filter": "Objekt Filter", | |||
"add_newsletter": "Newsletter hinzufügen", | |||
"plain_text": "Klartext", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Languages other than EN are managed here:
Please revert this change and add the translation in POEditor once this PR is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where exactly? Link missing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, will do
Spam filters like SpamAssassin rate multipart emails higher than just html-email. Having the text part is worth one score point. The existing version just sends html-emails, also with the detour via html2text. (See comment in code).
Following database change has to be done first: (I don't know how to send the db-update in a PR)
ALTER TABLE
intranet
.documents_newsletter
ADD COLUMN
plaintext
LONGTEXT NULL DEFAULT NULL AFTERsendingMode
;Best regards
Lexi