Skip to content

Commit 06bfbde

Browse files
cemms1cemms1
committed
refactor and improve readability + styling of email HTML
1 parent c50a223 commit 06bfbde

File tree

5 files changed

+112
-132
lines changed

5 files changed

+112
-132
lines changed

‎ab-testing/notification-lambda/src/index.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1+
import { activeABtests } from "@guardian/ab-testing-config";
12
import { sendEmail } from "./lib/email.ts";
2-
import { getExpiringAbTestsGroupedByOwner } from "./lib/groupExpiryChecksByEmail.ts";
3+
import { getExpiringAbTestsGroupedByOwner } from "./lib/expiringTestsByOwner.ts";
34

45
export const handler = async (): Promise<void> => {
5-
const abTestsByOwner = getExpiringAbTestsGroupedByOwner();
6+
const expiringAbTestsByOwner =
7+
getExpiringAbTestsGroupedByOwner(activeABtests);
68

79
await Promise.all(
8-
Object.entries(abTestsByOwner).map(([owner, abTests]) =>
10+
Object.entries(expiringAbTestsByOwner).map(([owner, abTests]) =>
911
sendEmail(owner, abTests),
1012
),
1113
);

‎ab-testing/notification-lambda/src/lib/checkExpiry.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export type ABExpiryChecks = {
66
expired: ABTest[];
77
};
88

9-
export const testExpiresWithinDays = (days: number) => (test: ABTest) => {
9+
export const expiresWithinDays = (days: number) => (test: ABTest) => {
1010
const oneDay = 1000 * 60 * 60 * 24; // a day in millis
1111
const daysInMillis = days * oneDay;
1212
return new Date(test.expirationDate) < new Date(Date.now() + daysInMillis);
@@ -16,19 +16,19 @@ export const checkExpiry = (tests: ABTest[]) => {
1616
return tests.reduce(
1717
(acc: ABExpiryChecks, test: ABTest) => {
1818
// Has the test expired?
19-
if (testExpiresWithinDays(0)(test)) {
19+
if (expiresWithinDays(0)(test)) {
2020
acc.expired.push(test);
2121
return acc;
2222
}
2323

2424
// Is the test expiring within the next day?
25-
if (testExpiresWithinDays(1)(test)) {
25+
if (expiresWithinDays(1)(test)) {
2626
acc.within1Day.push(test);
2727
return acc;
2828
}
2929

3030
// Is the test expiring within the next two days?
31-
if (testExpiresWithinDays(2)(test)) {
31+
if (expiresWithinDays(2)(test)) {
3232
acc.within2Days.push(test);
3333
return acc;
3434
}

‎ab-testing/notification-lambda/src/lib/email.ts‎

Lines changed: 90 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -2,116 +2,103 @@ import { SendEmailCommand } from "@aws-sdk/client-ses";
22
import { type ABExpiryChecks } from "./checkExpiry.ts";
33
import { sesClient } from "./sesClient.ts";
44

5-
const getEmailBodyHtml = (abTestExpiryChecks: ABExpiryChecks): string => {
5+
const getEmailBodyHtml = (expiryChecks: ABExpiryChecks): string => {
66
const tableConfig: Record<
77
keyof ABExpiryChecks,
88
{ title: string; accentColour: string }
99
> = {
1010
expired: {
11-
title: "‼️ Expired",
12-
accentColour: "orangered",
11+
title: "Expired",
12+
accentColour: "firebrick",
1313
},
1414
within1Day: {
15-
title: "⚠️ Expiring today @ 23:59",
16-
accentColour: "firebrick",
15+
title: "Expiring today at 23:59",
16+
accentColour: "chocolate",
1717
},
1818
within2Days: {
19-
title: "⏰ Expires tomorrow @ 23:59",
20-
accentColour: "chocolate",
19+
title: "Expires tomorrow at 23:59",
20+
accentColour: "peru",
2121
},
2222
};
2323

2424
const getTestsTableHtml = (group: keyof typeof tableConfig): string => {
25-
const config = tableConfig[group];
25+
const { accentColour, title } = tableConfig[group];
26+
2627
return `
27-
<h2 style="margin-bottom: 0.5rem; font-size: 1.2rem; color: ${
28-
config.accentColour
29-
};">
30-
${config.title}
31-
</h2>
32-
<table style="border-spacing: 4px; padding: 6px; border: 1px dashed ${
33-
config.accentColour
34-
};">
35-
<thead style>
36-
<th>Test name</th>
37-
<th>Expiry date</th>
38-
<th>Owners</th>
39-
</thead>
40-
<tbody style="text-align: center;">
41-
${abTestExpiryChecks[group]
42-
.map((test) => {
43-
return `<tr style="padding: 10px;">
44-
<td><strong>${test.name}</strong></td>
45-
<td>${test.expirationDate}</td>
46-
<td>${test.owners.join("<br/>")}</td>
47-
</tr>`;
48-
})
49-
.join("<br />")}
50-
</tbody>
51-
</table>`;
28+
<h2 style="margin-bottom: 0.5rem; font-size: 1.2rem; color: ${accentColour};">
29+
${title}
30+
</h2>
31+
<table style="width: 100%; border-spacing: 4px; padding: 6px; border: 1px dashed ${accentColour};">
32+
<thead style="text-align: left;">
33+
<th style="width: 30%; border-bottom: 1px solid rgba(0,0,0,0.3);">Name</th>
34+
<th style="width: 10%; border-bottom: 1px solid rgba(0,0,0,0.3);">Expiry</th>
35+
<th style="width: 30%; border-bottom: 1px solid rgba(0,0,0,0.3);">Owners</th>
36+
<th style="width: 30%; border-bottom: 1px solid rgba(0,0,0,0.3);">Description</th>
37+
</thead>
38+
<tbody style="text-align: left;">
39+
${expiryChecks[group]
40+
.map((test) => {
41+
return `
42+
<tr>
43+
<td><strong>${test.name}</strong></td>
44+
<td>${test.expirationDate}</td>
45+
<td>${test.owners.join("<br/>")}</td>
46+
<td>${test.description}</td>
47+
</tr>`;
48+
})
49+
.join("")}
50+
</tbody>
51+
</table>
52+
`;
5253
};
5354

54-
return `
55-
<div style="margin:auto; margin:10px; font-family: sans-serif;">
55+
return `<div style="margin:auto; font-family: sans-serif;">
5656
<h1 style="font-size: 1.4rem">AB Tests Expiry Reminder</h1>
57-
58-
${abTestExpiryChecks["expired"].length ? getTestsTableHtml("expired") : ""}
59-
60-
${
61-
abTestExpiryChecks["within1Day"].length
62-
? getTestsTableHtml("within1Day")
63-
: ""
64-
}
65-
66-
${
67-
abTestExpiryChecks["within2Days"].length
68-
? getTestsTableHtml("within2Days")
69-
: ""
70-
}
71-
72-
<br></br>
73-
74-
If you are not ready to remove a test yet but are happy to leave it expired for now, please turn it <strong>OFF</strong> in <a href="https://github.com/guardian/dotcom-rendering/blob/main/ab-testing/config/abTests.ts">the code</a>.
75-
76-
<br></br>
77-
<br></br>
78-
79-
<em>See https://frontend.gutools.co.uk/analytics/ab-testing for more details</em>
80-
</div>
81-
`;
57+
${expiryChecks["expired"].length ? getTestsTableHtml("expired") : ""}
58+
${expiryChecks["within1Day"].length ? getTestsTableHtml("within1Day") : ""}
59+
${expiryChecks["within2Days"].length ? getTestsTableHtml("within2Days") : ""}
60+
<br>
61+
<p>
62+
If you are not ready to remove a test yet but are happy to leave it expired for now, please turn it <strong>OFF</strong>
63+
in <a href="https://github.com/guardian/dotcom-rendering/blob/main/ab-testing/config/abTests.ts">the code</a>.
64+
</p>
65+
<br>
66+
<p>
67+
<em>See https://frontend.gutools.co.uk/analytics/ab-testing for more details</em>
68+
</p>
69+
</div>`;
8270
};
8371

84-
const getEmailBodyPlainText = (abTestsByExpiryDate: ABExpiryChecks): string => {
85-
return `
86-
AB Tests Expiry Reminder
72+
const getEmailBodyPlainText = (expiryChecks: ABExpiryChecks): string => {
73+
return `AB Tests Expiry Reminder
8774
8875
Expired:
89-
${abTestsByExpiryDate.expired
76+
${expiryChecks.expired
9077
.map(
91-
(test) =>
92-
`${test.name} expired ${
93-
test.expirationDate
94-
}. Owners: ${test.owners.join(", ")}`,
78+
({ name, expirationDate, owners }) =>
79+
`${name} expired ${expirationDate}. Owners: ${owners.join(
80+
", ",
81+
)}`,
9582
)
9683
.join("\n")}
9784
9885
Expiring today (at 23:59):
99-
${abTestsByExpiryDate.within1Day
86+
${expiryChecks.within1Day
10087
.map(
101-
(test) =>
102-
`${test.name} expired ${
103-
test.expirationDate
104-
}. Owners: ${test.owners.join(", ")}`,
88+
({ name, expirationDate, owners }) =>
89+
`${name} expires ${expirationDate} at 00:00. Owners: ${owners.join(
90+
", ",
91+
)}`,
10592
)
10693
.join("\n")}
10794
10895
Expiring tomorrow (at 23:59):
109-
${abTestsByExpiryDate.within2Days
96+
${expiryChecks.within2Days
11097
.map(
111-
(test) =>
112-
`${test.name} expired ${
113-
test.expirationDate
114-
}. Owners: ${test.owners.join(", ")}`,
98+
({ name, expirationDate, owners }) =>
99+
`${name} expires ${expirationDate} at 00:00. Owners: ${owners.join(
100+
", ",
101+
)}`,
115102
)
116103
.join("\n")}
117104
@@ -121,43 +108,37 @@ const getEmailBodyPlainText = (abTestsByExpiryDate: ABExpiryChecks): string => {
121108
`;
122109
};
123110

124-
export const getEmailCommand = (
125-
recipient: string,
126-
abTestsByExpiryDate: ABExpiryChecks,
127-
) =>
128-
new SendEmailCommand({
129-
// Verified email domain in AWS
130-
Source: `AB Testing <notifications@${process.env.EMAIL_DOMAIN}>`,
131-
Destination: {
132-
ToAddresses: [recipient],
133-
},
134-
Message: {
135-
Subject: {
136-
Data: "Expiring AB Tests",
137-
Charset: "UTF-8",
138-
},
139-
Body: {
140-
Html: {
141-
Data: getEmailBodyHtml(abTestsByExpiryDate),
142-
Charset: "UTF-8",
143-
},
144-
Text: {
145-
Data: getEmailBodyPlainText(abTestsByExpiryDate),
146-
Charset: "UTF-8",
147-
},
148-
},
149-
},
150-
});
151-
152111
export const sendEmail = async (
153112
recipient: string,
154113
expiringAbTests: ABExpiryChecks,
155114
): Promise<void> => {
156-
const emailCommand = getEmailCommand(recipient, expiringAbTests);
157-
158115
try {
159116
await sesClient
160-
.send(emailCommand)
117+
.send(
118+
new SendEmailCommand({
119+
// Verified email domain in AWS. Ensure to update in CDK if changing
120+
Source: `notifications@${process.env.EMAIL_DOMAIN}`,
121+
Destination: {
122+
ToAddresses: [recipient],
123+
},
124+
Message: {
125+
Subject: {
126+
Data: "Expiring AB Tests",
127+
Charset: "UTF-8",
128+
},
129+
Body: {
130+
Html: {
131+
Data: getEmailBodyHtml(expiringAbTests),
132+
Charset: "UTF-8",
133+
},
134+
Text: {
135+
Data: getEmailBodyPlainText(expiringAbTests),
136+
Charset: "UTF-8",
137+
},
138+
},
139+
},
140+
}),
141+
)
161142
.then(() =>
162143
console.log(
163144
`Sent AB test expiry reminder email to ${recipient}`,

ab-testing/notification-lambda/src/lib/groupExpiryChecksByEmail.ts renamed to ab-testing/notification-lambda/src/lib/expiringTestsByOwner.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,30 @@
1-
import { activeABtests } from "@guardian/ab-testing-config";
1+
import type { ABTest } from "@guardian/ab-testing-config";
22
import {
33
type ABExpiryChecks,
44
checkExpiry,
5-
testExpiresWithinDays,
5+
expiresWithinDays,
66
} from "./checkExpiry.ts";
77

88
/**
99
* Returns an object containing AB test owners and a grouped version of
1010
* their tests which are near or past expiry
1111
*/
12-
export const getExpiringAbTestsGroupedByOwner = (): Record<
13-
string,
14-
ABExpiryChecks
15-
> => {
16-
const expiringSoon = activeABtests.filter(testExpiresWithinDays(2));
12+
export const getExpiringAbTestsGroupedByOwner = (
13+
abTests: ABTest[],
14+
): Record<string, ABExpiryChecks> => {
15+
// Filter list of AB tests expiring within the next two days
16+
const expiringSoon = abTests.filter(expiresWithinDays(2));
1717

18-
/** Gets unique list of AB test owners who have tests that are about to expire */
18+
// Finds AB test owners who have tests that are about to expire
1919
const ownersWithExpiringTests = new Set(
2020
expiringSoon.flatMap((test) => test.owners),
2121
);
2222

23+
// Returns expiring, or expired, AB tests grouped by owner
2324
return Array.from(ownersWithExpiringTests).reduce(
2425
(testsByOwner: Record<string, ABExpiryChecks>, owner: string) => {
2526
// find tests owned by owner
26-
const testsForOwner = activeABtests.filter((test) =>
27+
const testsForOwner = abTests.filter((test) =>
2728
test.owners.includes(owner),
2829
);
2930
// group into expiry categories

‎ab-testing/notification-lambda/src/run.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,8 @@ import type { CloudFormationCustomResourceEvent, Context } from "aws-lambda";
1414
import { handler } from "./index.ts";
1515

1616
const stage = process.env.STAGE ?? "CODE";
17-
const emailDomain =
18-
process.env.EMAIL_DOMAIN ??
19-
`[email protected]`;
2017

21-
process.env.EMAIL_DOMAIN = emailDomain;
18+
process.env.EMAIL_DOMAIN = `abtesting.code.dev-gutools.co.uk`;
2219

2320
// Create a mock CloudFormation event
2421
const mockEvent: CloudFormationCustomResourceEvent = {
@@ -55,9 +52,8 @@ const mockContext: Context = {
5552
succeed: () => {},
5653
};
5754

58-
console.log(
59-
`Running lambda handler locally with STAGE=${stage} and EMAIL_DOMAIN=${emailDomain}\n`,
60-
);
55+
console.log(`Running lambda handler locally with STAGE=${stage}\n`);
56+
console.log(`EMAIL_DOMAIN=${process.env.EMAIL_DOMAIN}\n`);
6157

6258
// Run the handler
6359
// Cast to unknown then to the correct function type to work around typing issues

0 commit comments

Comments
 (0)