diff --git a/src/db/tables/subscribers.ts b/src/db/tables/subscribers.ts index cf34931053..a20a061ad9 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 9439dbce2b..309d9769f8 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) {