From bcc3b7a2317be8b98d35045e74def9f717b9dfc0 Mon Sep 17 00:00:00 2001 From: Robert Helmer Date: Wed, 18 Dec 2024 15:25:55 -0800 Subject: [PATCH] MNTOR-3857 - move locale filtering from TS to SQL for monthly free activity report (#5431) * move locale filtering from TS to SQL for monthly free activity report --- src/db/tables/subscribers.ts | 44 ++++++++++++++------ src/scripts/cronjobs/monthlyActivityFree.tsx | 10 ++--- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/db/tables/subscribers.ts b/src/db/tables/subscribers.ts index cf349310536..a20a061ad96 100644 --- a/src/db/tables/subscribers.ts +++ b/src/db/tables/subscribers.ts @@ -417,9 +417,10 @@ async function getPlusSubscribersWaitingForMonthlyEmail( // Not covered by tests; mostly side-effects. See test-coverage.md#mock-heavy /* c8 ignore start */ -async function getFreeSubscribersWaitingForMonthlyEmail(): Promise< - SubscriberRow[] -> { +async function getFreeSubscribersWaitingForMonthlyEmail( + batchSize: number, + countryCodes: string[], +): Promise { const flag = await getFeatureFlagData("MonthlyReportFreeUser"); const accountCutOffDate = parseIso8601Datetime( process.env.MONTHLY_ACTIVITY_FREE_EMAIL_ACCOUNT_CUTOFF_DATE ?? @@ -438,6 +439,24 @@ async function getFreeSubscribersWaitingForMonthlyEmail(): Promise< let query = knex("subscribers") .select("subscribers.*") + .select( + knex.raw( + `CASE + WHEN (fxa_profile_json->>'locale') ~ ',' THEN + CASE + WHEN split_part(fxa_profile_json->>'locale', ',', 1) ~ '-' THEN + split_part(split_part(fxa_profile_json->>'locale', ',', 1), '-', 2) -- Extract country code from first part + ELSE + split_part(fxa_profile_json->>'locale', ',', 1) -- Fallback to the language code + END + WHEN (fxa_profile_json->>'locale') ~ '-' THEN + split_part(fxa_profile_json->>'locale', '-', 2) -- Extract country code if present + ELSE + fxa_profile_json->>'locale' -- Fallback to the language code + END AS country_code`, + ), + ) + .leftJoin( "subscriber_email_preferences", "subscribers.id", @@ -494,16 +513,17 @@ async function getFreeSubscribersWaitingForMonthlyEmail(): Promise< // https://github.com/knex/knex/issues/1881#issuecomment-275433906 query = query.whereIn("subscribers.primary_email", flag.allow_list); } - // One thing to note as being absent from this query: a LIMIT clause. - // The reason for this is that we want to filter out people who had - // a language other than `en-US` set when signing up, but to do so, - // we need to parse the `accept-language` field, which we can only - // do after we have the query results. Thus, if we were to limit - // the result of this query, we would at some point end up filtering - // every returned row, and then never moving on to the rows after that. - const rows = await query; - return rows; + const wrappedQuery = knex + // @ts-ignore TODO MNTOR-3890 Move away from this approach and simplify query. + .from({ base_query: query }) // Use the existing query as a subquery + .select("*") + .whereIn("country_code", countryCodes) + .limit(batchSize); + + const rows = await wrappedQuery; + + return rows as SubscriberRow[]; } /* c8 ignore stop */ diff --git a/src/scripts/cronjobs/monthlyActivityFree.tsx b/src/scripts/cronjobs/monthlyActivityFree.tsx index 9439dbce2bf..309d9769f8e 100644 --- a/src/scripts/cronjobs/monthlyActivityFree.tsx +++ b/src/scripts/cronjobs/monthlyActivityFree.tsx @@ -28,12 +28,10 @@ async function run() { const batchSize = MONTHLY_ACTIVITY_FREE_EMAIL_BATCH_SIZE; logger.info(`Getting free subscribers with batch size: ${batchSize}`); - const subscribersToEmail = (await getFreeSubscribersWaitingForMonthlyEmail()) - .filter((subscriber) => { - const assumedCountryCode = getSignupLocaleCountry(subscriber); - return assumedCountryCode === "us"; - }) - .slice(0, batchSize); + const subscribersToEmail = await getFreeSubscribersWaitingForMonthlyEmail( + batchSize, + ["US"], + ); await initEmail(); for (const subscriber of subscribersToEmail) {