mirror of
https://github.com/TryGhost/Ghost.git
synced 2024-12-24 03:14:03 +03:00
🐛 Fixed large mailgun recipient data (#15638)
fixes https://github.com/TryGhost/Team/issues/2096 When generating the recipient data for emails, the email clicks implementation is resulting in a recipient variable being added called replacement_xxx once for each link containing the same UUID. This generates a lot of unnecessary data overhead for emails, and it turns out that mailgun has a 25MB message limit. We wouldn't have come close if we only included the uuid once.
This commit is contained in:
parent
9a2fcba68a
commit
d1e6870740
@ -8,7 +8,7 @@ module.exports = (model, frame) => {
|
||||
const replacements = mega.postEmailSerializer.parseReplacements(jsonModel);
|
||||
replacements.forEach((replacement) => {
|
||||
jsonModel[replacement.format] = jsonModel[replacement.format].replace(
|
||||
replacement.match,
|
||||
replacement.regexp,
|
||||
replacement.fallback || ''
|
||||
);
|
||||
});
|
||||
|
@ -11,7 +11,6 @@ const debug = require('@tryghost/debug')('mega');
|
||||
const postEmailSerializer = require('../mega/post-email-serializer');
|
||||
const configService = require('../../../shared/config');
|
||||
const settingsCache = require('../../../shared/settings-cache');
|
||||
const labs = require('../../../shared/labs');
|
||||
|
||||
const messages = {
|
||||
error: 'The email service received an error from mailgun and was unable to send.'
|
||||
@ -223,6 +222,10 @@ module.exports = {
|
||||
const startTime = Date.now();
|
||||
debug(`sending message to ${recipients.length} recipients`);
|
||||
|
||||
// Update email content for this segment before searching replacements
|
||||
emailData = postEmailSerializer.renderEmailForSegment(emailData, memberSegment);
|
||||
|
||||
// Check all the used replacements in this email
|
||||
const replacements = postEmailSerializer.parseReplacements(emailData);
|
||||
|
||||
// collate static and dynamic data for each recipient ready for provider
|
||||
@ -235,11 +238,6 @@ module.exports = {
|
||||
unsubscribe_url: postEmailSerializer.createUnsubscribeUrl(recipient.member_uuid, {newsletterUuid})
|
||||
};
|
||||
|
||||
if (labs.isSet('audienceFeedback')) {
|
||||
// create unique urls for every recipient (for example, for feedback buttons)
|
||||
emailData = postEmailSerializer.createUserLinks(emailData, recipient.member_uuid);
|
||||
}
|
||||
|
||||
// computed properties on recipients - TODO: better way of handling these
|
||||
recipient.member_first_name = (recipient.member_name || '').split(' ')[0];
|
||||
|
||||
@ -251,8 +249,6 @@ module.exports = {
|
||||
recipientData[recipient.member_email] = data;
|
||||
});
|
||||
|
||||
emailData = postEmailSerializer.renderEmailForSegment(emailData, memberSegment);
|
||||
|
||||
try {
|
||||
const response = await mailgunClient.send(emailData, recipientData, replacements);
|
||||
debug(`sent message (${Date.now() - startTime}ms)`);
|
||||
|
@ -32,7 +32,7 @@ class EmailPreview {
|
||||
|
||||
replacements.forEach((replacement) => {
|
||||
emailContent[replacement.format] = emailContent[replacement.format].replace(
|
||||
replacement.match,
|
||||
replacement.regexp,
|
||||
replacement.fallback || ''
|
||||
);
|
||||
});
|
||||
|
@ -17,6 +17,7 @@ const linkReplacer = require('@tryghost/link-replacer');
|
||||
const linkTracking = require('../link-tracking');
|
||||
const memberAttribution = require('../member-attribution');
|
||||
const feedbackButtons = require('./feedback-buttons');
|
||||
const labs = require('../../../shared/labs');
|
||||
|
||||
const ALLOWED_REPLACEMENTS = ['first_name', 'uuid'];
|
||||
|
||||
@ -109,20 +110,16 @@ const PostEmailSerializer = {
|
||||
},
|
||||
|
||||
/**
|
||||
* createUserLinks
|
||||
* replaceFeedbackLinks
|
||||
*
|
||||
* Generate personalised links for each user
|
||||
* Replace the button template links with real links
|
||||
*
|
||||
* @param {string} memberUuid member uuid
|
||||
* @param {Object} email
|
||||
* @param {string} html
|
||||
* @param {string} postId (will be url encoded)
|
||||
* @param {string} memberUuid member uuid to use in the URL (will be url encoded)
|
||||
*/
|
||||
createUserLinks(email, memberUuid) {
|
||||
const result = {...email};
|
||||
|
||||
result.html = feedbackButtons.generateLinks(result.post.id, memberUuid, result.html);
|
||||
result.plaintext = htmlToPlaintext.email(result.html);
|
||||
|
||||
return result;
|
||||
replaceFeedbackLinks(html, postId, memberUuid) {
|
||||
return feedbackButtons.generateLinks(postId, memberUuid, html);
|
||||
},
|
||||
|
||||
// NOTE: serialization is needed to make sure we do post transformations such as image URL transformation from relative to absolute
|
||||
@ -181,12 +178,21 @@ const PostEmailSerializer = {
|
||||
const EMAIL_REPLACEMENT_REGEX = /%%(\{.*?\})%%/g;
|
||||
const REPLACEMENT_STRING_REGEX = /\{(?<recipientProperty>\w*?)(?:,? *(?:"|")(?<fallback>.*?)(?:"|"))?\}/;
|
||||
|
||||
function escapeRegExp(string) {
|
||||
return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
}
|
||||
|
||||
const replacements = [];
|
||||
|
||||
['html', 'plaintext'].forEach((format) => {
|
||||
let result;
|
||||
while ((result = EMAIL_REPLACEMENT_REGEX.exec(email[format])) !== null) {
|
||||
const [replacementMatch, replacementStr] = result;
|
||||
|
||||
// Did we already found this match and added it to the replacements array?
|
||||
if (replacements.find(r => r.match === replacementMatch && r.format === format)) {
|
||||
continue;
|
||||
}
|
||||
const match = replacementStr.match(REPLACEMENT_STRING_REGEX);
|
||||
|
||||
if (match) {
|
||||
@ -199,6 +205,7 @@ const PostEmailSerializer = {
|
||||
format,
|
||||
id,
|
||||
match: replacementMatch,
|
||||
regexp: new RegExp(escapeRegExp(replacementMatch), 'g'),
|
||||
recipientProperty: `member_${recipientProperty}`,
|
||||
fallback
|
||||
});
|
||||
@ -395,6 +402,14 @@ const PostEmailSerializer = {
|
||||
});
|
||||
}
|
||||
|
||||
// Add buttons
|
||||
if (labs.isSet('audienceFeedback')) {
|
||||
// create unique urls for every recipient (for example, for feedback buttons)
|
||||
// Note, we need to use a different member uuid in the links because `%%{uuid}%%` would get escaped by the URL object when set as a search param
|
||||
const urlSafeToken = '--' + new Date().getTime() + 'url-safe-uuid--';
|
||||
result.html = this.replaceFeedbackLinks(result.html, post.id, urlSafeToken).replace(new RegExp(urlSafeToken, 'g'), '%%{uuid}%%');
|
||||
}
|
||||
|
||||
// Clean up any unknown replacements strings to get our final content
|
||||
const {html, plaintext} = this.normalizeReplacementStrings(result);
|
||||
const data = {
|
||||
@ -520,7 +535,6 @@ module.exports = {
|
||||
serialize: PostEmailSerializer.serialize.bind(PostEmailSerializer),
|
||||
createUnsubscribeUrl: PostEmailSerializer.createUnsubscribeUrl.bind(PostEmailSerializer),
|
||||
createPostSignupUrl: PostEmailSerializer.createPostSignupUrl.bind(PostEmailSerializer),
|
||||
createUserLinks: PostEmailSerializer.createUserLinks.bind(PostEmailSerializer),
|
||||
renderEmailForSegment: PostEmailSerializer.renderEmailForSegment.bind(PostEmailSerializer),
|
||||
parseReplacements: PostEmailSerializer.parseReplacements.bind(PostEmailSerializer),
|
||||
// Export for tests
|
||||
|
@ -158,13 +158,13 @@ describe('MEGA', function () {
|
||||
// Do the actual replacements for the first member, so we don't have to worry about them anymore
|
||||
replacements.forEach((replacement) => {
|
||||
emailData[replacement.format] = emailData[replacement.format].replace(
|
||||
replacement.match,
|
||||
replacement.regexp,
|
||||
recipient[replacement.id]
|
||||
);
|
||||
|
||||
// Also force Mailgun format
|
||||
emailData[replacement.format] = emailData[replacement.format].replace(
|
||||
`%recipient.${replacement.id}%`,
|
||||
new RegExp(`%recipient.${replacement.id}%`, 'g'),
|
||||
recipient[replacement.id]
|
||||
);
|
||||
});
|
||||
@ -190,6 +190,9 @@ describe('MEGA', function () {
|
||||
// Check if the link is a tracked link
|
||||
assert(href.includes('?m=' + memberUuid), href + ' is not tracked');
|
||||
|
||||
// Check if this link is also present in the plaintext version (with the right replacements)
|
||||
assert(emailData.plaintext.includes(href), href + ' is not present in the plaintext version');
|
||||
|
||||
if (!firstLink) {
|
||||
firstLink = new URL(href);
|
||||
}
|
||||
|
@ -34,6 +34,46 @@ describe('Post Email Serializer', function () {
|
||||
assert.equal(replaced[1].recipientProperty, 'member_first_name');
|
||||
});
|
||||
|
||||
it('reuses the same replacement pattern when used multiple times', function () {
|
||||
const html = '<html>Hey %%{first_name}%%, what is up? Just repeating %%{first_name}%%</html>';
|
||||
const plaintext = 'Hey %%{first_name}%%, what is up? Just repeating %%{first_name}%%';
|
||||
|
||||
const replaced = parseReplacements({
|
||||
html,
|
||||
plaintext
|
||||
});
|
||||
|
||||
assert.equal(replaced.length, 2);
|
||||
assert.equal(replaced[0].format, 'html');
|
||||
assert.equal(replaced[0].recipientProperty, 'member_first_name');
|
||||
|
||||
assert.equal(replaced[1].format, 'plaintext');
|
||||
assert.equal(replaced[1].recipientProperty, 'member_first_name');
|
||||
});
|
||||
|
||||
it('creates multiple replacement pattern for valid format and value', function () {
|
||||
const html = '<html>Hey %%{first_name}%%, %%{uuid}%% %%{first_name}%% %%{uuid}%%</html>';
|
||||
const plaintext = 'Hey %%{first_name}%%, %%{uuid}%% %%{first_name}%% %%{uuid}%%';
|
||||
|
||||
const replaced = parseReplacements({
|
||||
html,
|
||||
plaintext
|
||||
});
|
||||
|
||||
assert.equal(replaced.length, 4);
|
||||
assert.equal(replaced[0].format, 'html');
|
||||
assert.equal(replaced[0].recipientProperty, 'member_first_name');
|
||||
|
||||
assert.equal(replaced[1].format, 'html');
|
||||
assert.equal(replaced[1].recipientProperty, 'member_uuid');
|
||||
|
||||
assert.equal(replaced[2].format, 'plaintext');
|
||||
assert.equal(replaced[2].recipientProperty, 'member_first_name');
|
||||
|
||||
assert.equal(replaced[3].format, 'plaintext');
|
||||
assert.equal(replaced[3].recipientProperty, 'member_uuid');
|
||||
});
|
||||
|
||||
it('does not create replacements for unsupported variable names', function () {
|
||||
const html = '<html>Hey %%{last_name}%%, what is up?</html>';
|
||||
const plaintext = 'Hey %%{age}%%, what is up?';
|
||||
|
@ -51,7 +51,7 @@ module.exports = class MailgunClient {
|
||||
// update content to use Mailgun variable syntax for replacements
|
||||
replacements.forEach((replacement) => {
|
||||
messageContent[replacement.format] = messageContent[replacement.format].replace(
|
||||
replacement.match,
|
||||
replacement.regexp,
|
||||
`%recipient.${replacement.id}%`
|
||||
);
|
||||
});
|
||||
|
Loading…
Reference in New Issue
Block a user