Skip to content

Commit

Permalink
MNTOR-3857 - move locale filtering from TS to SQL for monthly free ac…
Browse files Browse the repository at this point in the history
…tivity report (#5431)

* move locale filtering from TS to SQL for monthly free activity report
  • Loading branch information
rhelmer authored Dec 18, 2024
1 parent 0b7c3dd commit bcc3b7a
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 18 deletions.
44 changes: 32 additions & 12 deletions src/db/tables/subscribers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<SubscriberRow[]> {
const flag = await getFeatureFlagData("MonthlyReportFreeUser");
const accountCutOffDate = parseIso8601Datetime(
process.env.MONTHLY_ACTIVITY_FREE_EMAIL_ACCOUNT_CUTOFF_DATE ??
Expand All @@ -438,6 +439,24 @@ async function getFreeSubscribersWaitingForMonthlyEmail(): Promise<

let query = knex("subscribers")
.select<SubscriberRow[]>("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",
Expand Down Expand Up @@ -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 */

Expand Down
10 changes: 4 additions & 6 deletions src/scripts/cronjobs/monthlyActivityFree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit bcc3b7a

Please sign in to comment.