Skip to content

Commit

Permalink
Only collect scan & breach data when needed
Browse files Browse the repository at this point in the history
When processing tens of thousands of breach notifications, we might
theoretically be hammering the database by running a bunch of
queries for each of those notifications. It's currently hard to
test whether that load is actually excessive, but to be sure, this
refactors it to only load that data if it is absolutely necessary.
(Which currently is just for the free US user who has run a scan,
where we display the number of unfixed results.)
  • Loading branch information
Vinnl committed Sep 17, 2024
1 parent 76eda09 commit bffd2a6
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import { getSha1 } from "../../../../../../utils/fxa";
import { getBreaches } from "../../../../../functions/server/getBreaches";
import { getSignupLocaleCountry } from "../../../../../../emails/functions/getSignupLocaleCountry";
import { refreshStoredScanResults } from "../../../../../functions/server/refreshStoredScanResults";
import { hasPremium } from "../../../../../functions/universal/user";
import { isEligibleForPremium } from "../../../../../functions/universal/premium";

async function getAdminSubscriber(): Promise<SubscriberRow | null> {
const session = await getServerSession();
Expand Down Expand Up @@ -197,11 +199,14 @@ export async function triggerBreachAlert(
subscriber={subscriber}
breach={createRandomHibpListing()}
breachedEmail={emailAddress}
allSubscriberBreaches={allSubscriberBreaches}
scanData={scanData}
enabledFeatureFlags={["BreachEmailRedesign"]}
utmCampaignId="breach-alert"
l10n={l10n}
dataSummary={
isEligibleForPremium(assumedCountryCode) && hasPremium(subscriber)
? getDashboardSummary(scanData.results, allSubscriberBreaches)
: undefined
}
/>,
)
: await send(
Expand Down
46 changes: 10 additions & 36 deletions src/emails/templates/breachAlert/BreachAlertEmail.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import type { Meta, StoryObj } from "@storybook/react";
import { FC } from "react";
import type { SubscriberRow, OnerepScanRow } from "knex/types/tables";
import type { SubscriberRow } from "knex/types/tables";
import {
RedesignedBreachAlertEmailProps,
RedesignedBreachAlertEmail,
Expand All @@ -16,6 +16,7 @@ import {
createRandomHibpListing,
createRandomScanResult,
} from "../../../apiMocks/mockData";
import { getDashboardSummary } from "../../../app/functions/server/dashboard";

const meta: Meta<FC<RedesignedBreachAlertEmailProps>> = {
title: "Emails/Breach alert",
Expand All @@ -28,10 +29,6 @@ const meta: Meta<FC<RedesignedBreachAlertEmailProps>> = {
l10n: getL10n("en"),
utmCampaignId: "breach-alert",
enabledFeatureFlags: [],
scanData: {
scan: null,
results: [],
},
subscriber: {
fxa_profile_json: {
locale: "en-US",
Expand Down Expand Up @@ -72,36 +69,20 @@ export const RedesignedBreachAlertEmailUsFreeNoScanStory: Story = {
args: {
breach: createRandomHibpListing(),
breachedEmail: "[email protected]",
scanData: {
scan: null,
results: [],
},
enabledFeatureFlags: ["BreachEmailRedesign"],
},
};

const mockedScan: OnerepScanRow = {
created_at: new Date(1998, 2, 31),
updated_at: new Date(1998, 2, 31),
id: 0,
onerep_profile_id: 0,
onerep_scan_id: 0,
onerep_scan_reason: "initial",
onerep_scan_status: "finished",
};
export const RedesignedBreachAlertEmailUsFreeWithScanStory: Story = {
name: "Breach alert/US free, scan has run",
args: {
breach: createRandomHibpListing(),
breachedEmail: "[email protected]",
allSubscriberBreaches: Array.from({ length: 5 }, () =>
createRandomBreach(),
),
scanData: {
scan: mockedScan,
results: Array.from({ length: 5 }, () => createRandomScanResult()),
},
enabledFeatureFlags: ["BreachEmailRedesign"],
dataSummary: getDashboardSummary(
Array.from({ length: 5 }, () => createRandomScanResult()),
Array.from({ length: 5 }, () => createRandomBreach()),
),
},
};

Expand All @@ -110,10 +91,6 @@ export const RedesignedBreachAlertEmailUsPlusNoScanStory: Story = {
args: {
breach: createRandomHibpListing(),
breachedEmail: "[email protected]",
scanData: {
scan: null,
results: [],
},
subscriber: {
fxa_profile_json: {
subscriptions: ["monitor"],
Expand All @@ -128,18 +105,15 @@ export const RedesignedBreachAlertEmailUsPlusWithScanStory: Story = {
args: {
breach: createRandomHibpListing(),
breachedEmail: "[email protected]",
allSubscriberBreaches: Array.from({ length: 5 }, () =>
createRandomBreach(),
),
scanData: {
scan: mockedScan,
results: Array.from({ length: 5 }, () => createRandomScanResult()),
},
subscriber: {
fxa_profile_json: {
subscriptions: ["monitor"],
},
} as SubscriberRow,
enabledFeatureFlags: ["BreachEmailRedesign"],
dataSummary: getDashboardSummary(
Array.from({ length: 5 }, () => createRandomScanResult()),
Array.from({ length: 5 }, () => createRandomBreach()),
),
},
};
30 changes: 17 additions & 13 deletions src/emails/templates/breachAlert/BreachAlertEmail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,9 @@ import { EmailHero } from "../EmailHero";
import { getLocale } from "../../../app/functions/universal/getLocale";
import { isEligibleForPremium } from "../../../app/functions/universal/premium";
import { hasPremium } from "../../../app/functions/universal/user";
import type { LatestOnerepScanData } from "../../../db/tables/onerep_scans";
import type { SubscriberBreach } from "../../../utils/subscriberBreaches";
import { getSignupLocaleCountry } from "../../functions/getSignupLocaleCountry";
import { getPremiumSubscriptionUrl } from "../../../app/functions/server/getPremiumSubscriptionInfo";
import { getDashboardSummary } from "../../../app/functions/server/dashboard";
import { DashboardSummary } from "../../../app/functions/server/dashboard";
import { ResolutionRelevantBreachDataTypes } from "../../../app/functions/universal/breach";

export type Props = {
Expand Down Expand Up @@ -75,8 +73,18 @@ export type RedesignedBreachAlertEmailProps = {
breachedEmail: string;
utmCampaignId: string;
subscriber: SubscriberRow;
scanData: LatestOnerepScanData;
allSubscriberBreaches: SubscriberBreach[];
/**
* We need to run a bunch of queries to collect this data,
* so it's optional; however, make sure to pass it in for
* free users who are eligible for Plus (i.e. in the US),
* who have run a scan — those are the ones we show a
* <DataPointCount> for at the moment. We also use the
* presence of this property to determine whether free
* US users have run a scan. If, in the future, we assume
* we can do the same for Plus users, that might break, but
* that's a risk we'll have to take for now:
*/
dataSummary?: DashboardSummary;
enabledFeatureFlags: FeatureFlagName[];
};

Expand Down Expand Up @@ -104,7 +112,7 @@ export const RedesignedBreachAlertEmail = (
if (hasPremium(props.subscriber)) {
utmCampaignId = "breach-alert-plus";
} else {
if (props.scanData.scan === null) {
if (typeof props.dataSummary === "undefined") {
utmCampaignId = "breach-alert-free-us-no-scan";
} else {
utmCampaignId = "breach-alert-free-us-scanned";
Expand Down Expand Up @@ -252,7 +260,7 @@ export const RedesignedBreachAlertEmail = (
/>
{isEligibleForPremium(assumedCountryCode) &&
!hasPremium(props.subscriber) &&
(props.scanData.scan === null ? (
(typeof props.dataSummary === "undefined" ? (
<Banner
heading={l10n.getString(
"email-breach-alert-plus-scan-banner-heading",
Expand Down Expand Up @@ -328,7 +336,7 @@ const Banner = (props: {
const DataPointCount = (
props: RedesignedBreachAlertEmailProps & { utmContentSuffix: string },
) => {
if (props.scanData.scan === null) {
if (typeof props.dataSummary === "undefined") {
return null;
}
if (hasPremium(props.subscriber)) {
Expand All @@ -338,12 +346,8 @@ const DataPointCount = (
}

const l10n = props.l10n;
const dataSummary = getDashboardSummary(
props.scanData.results,
props.allSubscriberBreaches,
);
const sumOfUnresolvedDataPoints =
dataSummary.unresolvedSanitizedDataPoints.reduce(
props.dataSummary.unresolvedSanitizedDataPoints.reduce(
(total, dataPointSummary) => {
return total + Object.values(dataPointSummary)[0];
},
Expand Down
34 changes: 25 additions & 9 deletions src/scripts/cronjobs/emailBreachAlerts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ import { getLatestOnerepScanResults } from "../../db/tables/onerep_scans";
import { getSubscriberBreaches } from "../../app/functions/server/getSubscriberBreaches";
import { getSignupLocaleCountry } from "../../emails/functions/getSignupLocaleCountry";
import { refreshStoredScanResults } from "../../app/functions/server/refreshStoredScanResults";
import { isEligibleForPremium } from "../../app/functions/universal/premium";
import { hasPremium } from "../../app/functions/universal/user";
import {
DashboardSummary,
getDashboardSummary,
} from "../../app/functions/server/dashboard";

const SENTRY_SLUG = "cron-breach-alerts";

Expand Down Expand Up @@ -288,13 +294,24 @@ export async function poll(
if (typeof recipient.onerep_profile_id === "number") {
await refreshStoredScanResults(recipient.onerep_profile_id);
}
const scanData = await getLatestOnerepScanResults(
recipient.onerep_profile_id,
);
const allSubscriberBreaches = await getSubscriberBreaches({
fxaUid: recipient.fxa_uid,
countryCode: assumedCountryCode,
});

let dataSummary: DashboardSummary | undefined;
if (
isEligibleForPremium(assumedCountryCode) &&
!hasPremium(recipient)
) {
const scanData = await getLatestOnerepScanResults(
recipient.onerep_profile_id,
);
const allSubscriberBreaches = await getSubscriberBreaches({
fxaUid: recipient.fxa_uid,
countryCode: assumedCountryCode,
});
dataSummary = getDashboardSummary(
scanData.results,
allSubscriberBreaches,
);
}

const subject = l10n.getString(
"email-breach-alert-all-subject",
Expand All @@ -308,11 +325,10 @@ export async function poll(
l10n={l10n}
breach={breachAlert}
breachedEmail={breachedEmail}
allSubscriberBreaches={allSubscriberBreaches}
utmCampaignId={utmCampaignId}
enabledFeatureFlags={enabledFeatureFlags}
subscriber={recipient}
scanData={scanData}
dataSummary={dataSummary}
/>,
),
);
Expand Down

0 comments on commit bffd2a6

Please sign in to comment.