Skip to content

Commit 53aa6c0

Browse files
EMMLynchEMMLynch
authored
Merge pull request #6602 from mozilla/MNTOR-5264
MNTOR-5264: add active user filter to subscriber queries
2 parents d959aa3 + 62a54d1 commit 53aa6c0

5 files changed

Lines changed: 186 additions & 27 deletions

File tree

‎package-lock.json‎

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/db/models/BreachNotificationSubscriber.integration.ts‎

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ import { getBreachNotificationSubscribersByHashes } from "./BreachNotificationSu
1010
import type { SubscriberRow } from "knex/types/tables";
1111

1212
describe("BreachNotificationSubscriber", () => {
13-
const subscriber = seeds.subscribers({ primary_verified: true });
13+
const recentSession = new Date();
14+
const subscriber = seeds.subscribers({
15+
primary_verified: true,
16+
fxa_session_expiry: recentSession,
17+
});
1418
const chaffSubs = Array.from(Array(10).keys()).map((_) =>
1519
seeds.subscribers(),
1620
);
@@ -111,6 +115,7 @@ describe("BreachNotificationSubscriber", () => {
111115
seeds.subscribers({
112116
primary_verified: true,
113117
all_emails_to_primary: true,
118+
fxa_session_expiry: recentSession,
114119
}),
115120
)
116121
.returning("*")
@@ -142,6 +147,7 @@ describe("BreachNotificationSubscriber", () => {
142147
seeds.subscribers({
143148
primary_verified: true,
144149
all_emails_to_primary: false,
150+
fxa_session_expiry: recentSession,
145151
}),
146152
)
147153
.returning("*")
@@ -166,4 +172,113 @@ describe("BreachNotificationSubscriber", () => {
166172
]),
167173
);
168174
});
175+
176+
it("excludes subscribers with null fxa_session_expiry", async () => {
177+
const inactiveSub = (
178+
await conn("subscribers")
179+
.insert(
180+
seeds.subscribers({
181+
primary_verified: true,
182+
fxa_session_expiry: null,
183+
}),
184+
)
185+
.returning("*")
186+
)[0];
187+
const hashes = [inactiveSub.primary_sha1];
188+
const actual = await getBreachNotificationSubscribersByHashes(hashes);
189+
expect(actual.length).toEqual(0);
190+
});
191+
192+
it("excludes subscribers with fxa_session_expiry older than 1 year", async () => {
193+
const twoYearsAgo = new Date(Date.now() - 2 * 365.25 * 24 * 60 * 60 * 1000);
194+
const expiredSub = (
195+
await conn("subscribers")
196+
.insert(
197+
seeds.subscribers({
198+
primary_verified: true,
199+
fxa_session_expiry: twoYearsAgo,
200+
}),
201+
)
202+
.returning("*")
203+
)[0];
204+
const hashes = [expiredSub.primary_sha1];
205+
const actual = await getBreachNotificationSubscribersByHashes(hashes);
206+
expect(actual.length).toEqual(0);
207+
});
208+
209+
it("includes subscribers with fxa_session_expiry within the last year", async () => {
210+
const sixMonthsAgo = new Date(
211+
Date.now() - 0.5 * 365.25 * 24 * 60 * 60 * 1000,
212+
);
213+
const activeSub = (
214+
await conn("subscribers")
215+
.insert(
216+
seeds.subscribers({
217+
primary_verified: true,
218+
fxa_session_expiry: sixMonthsAgo,
219+
}),
220+
)
221+
.returning("*")
222+
)[0];
223+
const hashes = [activeSub.primary_sha1];
224+
const actual = await getBreachNotificationSubscribersByHashes(hashes);
225+
expect(actual.length).toEqual(1);
226+
expect(actual[0]).toEqual(
227+
expect.objectContaining({
228+
subscriber_id: activeSub.id,
229+
breached_email: activeSub.primary_email,
230+
}),
231+
);
232+
});
233+
234+
it("excludes inactive subscribers via secondary email path", async () => {
235+
const inactiveSub = (
236+
await conn("subscribers")
237+
.insert(
238+
seeds.subscribers({
239+
primary_verified: true,
240+
fxa_session_expiry: null,
241+
}),
242+
)
243+
.returning("*")
244+
)[0];
245+
const insertedEmails = await conn("email_addresses")
246+
.insert([seeds.emails(inactiveSub.id, { verified: true })])
247+
.returning("*");
248+
const hashes = [insertedEmails[0].sha1];
249+
const actual = await getBreachNotificationSubscribersByHashes(hashes);
250+
expect(actual.length).toEqual(0);
251+
});
252+
253+
it("returns only active subscribers when mixed with inactive ones", async () => {
254+
const activeSub = (
255+
await conn("subscribers")
256+
.insert(
257+
seeds.subscribers({
258+
primary_verified: true,
259+
fxa_session_expiry: recentSession,
260+
}),
261+
)
262+
.returning("*")
263+
)[0];
264+
const inactiveSub = (
265+
await conn("subscribers")
266+
.insert(
267+
seeds.subscribers({
268+
primary_verified: true,
269+
fxa_session_expiry: null,
270+
}),
271+
)
272+
.returning("*")
273+
)[0];
274+
const hashes = [activeSub.primary_sha1, inactiveSub.primary_sha1];
275+
const actual = await getBreachNotificationSubscribersByHashes(hashes);
276+
expect(actual.length).toEqual(1);
277+
expect(actual[0]).toEqual(
278+
expect.objectContaining({
279+
subscriber_id: activeSub.id,
280+
breached_email: activeSub.primary_email,
281+
}),
282+
);
283+
});
169284
});

‎src/db/models/BreachNotificationSubscriber.ts‎

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import { EmailAddressRow, SubscriberRow } from "knex/types/tables";
66
import { getEmailAddressesByHashes } from "../tables/emailAddresses";
77
import { getSubscribersByHashes } from "../tables/subscribers";
88

9+
const ACTIVE_USER_MAX_AGE_MS = 365.25 * 24 * 60 * 60 * 1000; // 1 year
10+
911
// Covered by integration tests but report is not combined
1012
// TODO: MNTOR-5117
1113
/* c8 ignore start */
@@ -23,32 +25,37 @@ export type BreachNotificationSubscriber = {
2325
export async function getBreachNotificationSubscribersByHashes(
2426
hashes: string[],
2527
): Promise<BreachNotificationSubscriber[]> {
28+
const activeFilter = { activeWithinMs: ACTIVE_USER_MAX_AGE_MS };
2629
// 2 sources of email address - the subscribers table and
2730
// email address table (containing additional emails)
2831
// First query if the primary email has been breached
29-
const primary = (await getSubscribersByHashes(hashes)).map((row) => ({
30-
subscriber_id: row.id,
31-
all_emails_to_primary: row.all_emails_to_primary,
32-
primary_email: row.primary_email,
33-
breached_email: row.primary_email,
34-
notification_email: row.primary_email,
35-
signup_language: row.signup_language,
36-
fxa_profile_json: row.fxa_profile_json,
37-
fxa_uid: row.fxa_uid,
38-
}));
32+
const primary = (await getSubscribersByHashes(hashes, activeFilter)).map(
33+
(row) => ({
34+
subscriber_id: row.id,
35+
all_emails_to_primary: row.all_emails_to_primary,
36+
primary_email: row.primary_email,
37+
breached_email: row.primary_email,
38+
notification_email: row.primary_email,
39+
signup_language: row.signup_language,
40+
fxa_profile_json: row.fxa_profile_json,
41+
fxa_uid: row.fxa_uid,
42+
}),
43+
);
3944
// Second for secondary emails (joined to subscriber record)
40-
const secondary = (await getEmailAddressesByHashes(hashes)).map((row) => ({
41-
subscriber_id: row.subscriber_id,
42-
all_emails_to_primary: row.all_emails_to_primary,
43-
primary_email: row.primary_email,
44-
breached_email: row.email,
45-
notification_email: row.all_emails_to_primary
46-
? row.primary_email
47-
: row.email,
48-
signup_language: row.signup_language,
49-
fxa_profile_json: row.fxa_profile_json,
50-
fxa_uid: row.fxa_uid,
51-
}));
45+
const secondary = (await getEmailAddressesByHashes(hashes, activeFilter)).map(
46+
(row) => ({
47+
subscriber_id: row.subscriber_id,
48+
all_emails_to_primary: row.all_emails_to_primary,
49+
primary_email: row.primary_email,
50+
breached_email: row.email,
51+
notification_email: row.all_emails_to_primary
52+
? row.primary_email
53+
: row.email,
54+
signup_language: row.signup_language,
55+
fxa_profile_json: row.fxa_profile_json,
56+
fxa_uid: row.fxa_uid,
57+
}),
58+
);
5259
return primary.concat(secondary);
5360
}
5461
/* c8 ignore stop */

‎src/db/tables/emailAddresses.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,11 +328,22 @@ async function removeOneSecondaryEmail(emailId: number, subscriberId: number) {
328328

329329
// Not covered by tests; mostly side-effects. See test-coverage.md#mock-heavy
330330
/* c8 ignore start */
331-
async function getEmailAddressesByHashes(hashes: string[]) {
332-
return await knex("email_addresses")
331+
async function getEmailAddressesByHashes(
332+
hashes: string[],
333+
options?: { activeWithinMs?: number },
334+
) {
335+
const query = knex("email_addresses")
333336
.join("subscribers", "email_addresses.subscriber_id", "=", "subscribers.id")
334337
.whereIn("email_addresses.sha1", hashes)
335338
.andWhere("email_addresses.verified", "=", true);
339+
if (options?.activeWithinMs !== undefined) {
340+
query.andWhere(
341+
"subscribers.fxa_session_expiry",
342+
">",
343+
new Date(Date.now() - options.activeWithinMs),
344+
);
345+
}
346+
return await query;
336347
}
337348
/* c8 ignore stop */
338349

‎src/db/tables/subscribers.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,21 @@ const knex = createDbConnection();
1616

1717
// Not covered by tests; mostly side-effects. See test-coverage.md#mock-heavy
1818
/* c8 ignore start */
19-
async function getSubscribersByHashes(hashes: string[]) {
20-
return await knex("subscribers")
19+
async function getSubscribersByHashes(
20+
hashes: string[],
21+
options?: { activeWithinMs?: number },
22+
) {
23+
const query = knex("subscribers")
2124
.whereIn("primary_sha1", hashes)
2225
.andWhere("primary_verified", "=", true);
26+
if (options?.activeWithinMs !== undefined) {
27+
query.andWhere(
28+
"fxa_session_expiry",
29+
">",
30+
new Date(Date.now() - options.activeWithinMs),
31+
);
32+
}
33+
return await query;
2334
}
2435
/* c8 ignore stop */
2536

0 commit comments

Comments
 (0)