Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove some feature flags #5596

Merged
merged 6 commits into from
Feb 10, 2025
+180 −463
Merged

Remove some feature flags #5596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
12598ed
Remove `ConfirmCancellation` flag
Vinnl Feb 5, 2025
17074cb
Remove `FirstDataBrokerRemovalFixedEmail` flag
Vinnl Feb 5, 2025
e408fe2
Remove `BreachEmailRedesign` flag
Vinnl Feb 6, 2025
f8faeaf
Remove "Redesigned" from breach alert emails
Vinnl Feb 6, 2025
bfb5513
Remove `MonthlyReportFreeUser` flag
Vinnl Feb 6, 2025
c6d185b
Remove unused `UpdatedEmailPreferencesOption` flag
Vinnl Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions locales/en/app.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,6 @@ breach-summary = Breach Summary

##

breach-alert-subject = { -product-name } found your email in a new data breach

# This string is displayed under a large numeral that indicates the total number
# of data breaches that exposed a user’s password. Don’t add $passwords to
# your localization, because it would result in the number showing twice.
Expand Down
11 changes: 1 addition & 10 deletions locales/en/email-strings.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,9 @@ email-breach-summary = Here’s your data breach summary
# Variables:
# $email-address (string) - Email address, bolded
email-breach-detected = Search results for your { $email-address } account have detected that your email may have been exposed. We recommend you act now to resolve this breach.
# Deprecated after the redesigned breach alert email is launched
# Variables:
# $email-address (string) - Email address
email-breach-detected-2 = Search results for your <b>{ $email-address }</b> account have detected that your email may have been exposed. We recommend you act now to resolve this breach.
email-dashboard-cta = Go to Dashboard

## Breach alert

# Deprecated after the redesigned breach alert email is launched
email-spotted-new-breach = We’ve spotted a new data breach

## Redesigned breach alert email
## Breach alert email

email-breach-alert-all-subject = New data breach detected
email-breach-alert-all-preview = We’ll guide you through the steps to resolve it.
Expand Down
16 changes: 0 additions & 16 deletions src/app/(proper_react)/(redesign)/(authenticated)/admin/emails/EmailTrigger.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ export const EmailTrigger = (props: Props) => {
const [isSendingSignupReport, setIsSendingSignupReport] = useState(false);
const [isSendingVerification, setIsSendingVerification] = useState(false);
const [isSendingBreachAlert, setIsSendingBreachAlert] = useState(false);
const [isSendingRedesignedBreachAlert, setIsSendingRedesignedBreachAlert] =
useState(false);
const [
isSendingMonthlyActivityFreeOverview,
setIsSendingMonthlyActivityFreeOverview,
Expand Down Expand Up @@ -132,20 +130,6 @@ export const EmailTrigger = (props: Props) => {
>
Breach alert
</Button>
<Button
variant="primary"
isLoading={isSendingRedesignedBreachAlert}
onPress={() => {
setIsSendingRedesignedBreachAlert(true);
void triggerBreachAlert(selectedEmailAddress, {
redesign: true,
}).then(() => {
setIsSendingRedesignedBreachAlert(false);
});
}}
>
Breach alert (redesigned)
</Button>
<Button
variant="primary"
isLoading={firstDataBrokerRemovalFixed}
Expand Down
55 changes: 18 additions & 37 deletions src/app/(proper_react)/(redesign)/(authenticated)/admin/emails/actions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ import {
createRandomHibpListing,
createRandomScanResult,
} from "../../../../../../apiMocks/mockData";
import {
BreachAlertEmail,
RedesignedBreachAlertEmail,
} from "../../../../../../emails/templates/breachAlert/BreachAlertEmail";
import { BreachAlertEmail } from "../../../../../../emails/templates/breachAlert/BreachAlertEmail";
import { SignupReportEmail } from "../../../../../../emails/templates/signupReport/SignupReportEmail";
import { getBreachesForEmail } from "../../../../../../utils/hibp";
import { getSha1 } from "../../../../../../utils/fxa";
Expand Down Expand Up @@ -216,10 +213,7 @@ export async function triggerMonthlyActivityPlus(emailAddress: string) {
);
}

export async function triggerBreachAlert(
emailAddress: string,
options: Partial<{ redesign: boolean }> = {},
) {
export async function triggerBreachAlert(emailAddress: string) {
const session = await getServerSession();
const subscriber = await getAdminSubscriber();
if (!subscriber || !session?.user) {
Expand All @@ -243,35 +237,22 @@ export async function triggerBreachAlert(
countryCode: assumedCountryCode,
});

options.redesign === true
? await send(
emailAddress,
l10n.getString("email-breach-alert-all-subject"),
<RedesignedBreachAlertEmail
subscriber={subscriber}
breach={createRandomHibpListing()}
breachedEmail={emailAddress}
enabledFeatureFlags={["BreachEmailRedesign"]}
utmCampaignId="breach-alert"
l10n={l10n}
dataSummary={
isEligibleForPremium(assumedCountryCode) && !hasPremium(subscriber)
? getDashboardSummary(scanData.results, allSubscriberBreaches)
: undefined
}
/>,
)
: await send(
emailAddress,
l10n.getString("breach-alert-subject"),
<BreachAlertEmail
subscriber={subscriber}
breach={createRandomHibpListing()}
breachedEmail={emailAddress}
utmCampaignId="breach-alert"
l10n={l10n}
/>,
);
await send(
emailAddress,
l10n.getString("email-breach-alert-all-subject"),
<BreachAlertEmail
subscriber={subscriber}
breach={createRandomHibpListing()}
breachedEmail={emailAddress}
utmCampaignId="breach-alert"
l10n={l10n}
dataSummary={
isEligibleForPremium(assumedCountryCode) && !hasPremium(subscriber)
? getDashboardSummary(scanData.results, allSubscriberBreaches)
: undefined
}
/>,
);
}

export async function triggerFirstDataBrokerRemovalFixed(emailAddress: string) {
Expand Down
28 changes: 11 additions & 17 deletions ...(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/SettingsPage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ describe("Settings page", () => {
yearlySubscriptionUrl=""
monthlySubscriptionUrl=""
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={["ConfirmCancellation", "CancellationFlow"]}
enabledFeatureFlags={["CancellationFlow"]}
experimentData={defaultExperimentData["Features"]}
isMonthlySubscriber={true}
data={mockedPlusSubscriberEmailPreferences}
Expand Down Expand Up @@ -1052,7 +1052,7 @@ describe("Settings page", () => {
yearlySubscriptionUrl=""
monthlySubscriptionUrl=""
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={["ConfirmCancellation", "CancellationFlow"]}
enabledFeatureFlags={["CancellationFlow"]}
experimentData={defaultExperimentData["Features"]}
isMonthlySubscriber={true}
data={mockedPlusSubscriberEmailPreferences}
Expand Down Expand Up @@ -1743,7 +1743,6 @@ describe("Settings page", () => {
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={[
"CancellationFlow",
"ConfirmCancellation",
"DiscountCouponNextThreeMonths",
]}
experimentData={defaultExperimentData["Features"]}
Expand Down Expand Up @@ -1833,7 +1832,6 @@ describe("Settings page", () => {
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={[
"CancellationFlow",
"ConfirmCancellation",
"DiscountCouponNextThreeMonths",
]}
experimentData={defaultExperimentData["Features"]}
Expand Down Expand Up @@ -1901,7 +1899,6 @@ describe("Settings page", () => {
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={[
"CancellationFlow",
"ConfirmCancellation",
"DiscountCouponNextThreeMonths",
]}
experimentData={defaultExperimentData["Features"]}
Expand Down Expand Up @@ -2038,7 +2035,7 @@ describe("Settings page", () => {
yearlySubscriptionUrl=""
monthlySubscriptionUrl=""
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={["UpdatedEmailPreferencesOption"]}
enabledFeatureFlags={[]}
experimentData={defaultExperimentData["Features"]}
isMonthlySubscriber={true}
data={mockedPlusSubscriberEmailPreferences}
Expand Down Expand Up @@ -2100,7 +2097,7 @@ describe("Settings page", () => {
yearlySubscriptionUrl=""
monthlySubscriptionUrl=""
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={["UpdatedEmailPreferencesOption"]}
enabledFeatureFlags={[]}
experimentData={defaultExperimentData["Features"]}
isMonthlySubscriber={true}
data={mockedPlusSubscriberEmailPreferences}
Expand Down Expand Up @@ -2142,7 +2139,7 @@ describe("Settings page", () => {
yearlySubscriptionUrl=""
monthlySubscriptionUrl=""
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={["UpdatedEmailPreferencesOption"]}
enabledFeatureFlags={[]}
experimentData={defaultExperimentData["Features"]}
isMonthlySubscriber={true}
data={mockedPlusSubscriberEmailPreferences}
Expand Down Expand Up @@ -2203,7 +2200,7 @@ describe("Settings page", () => {
yearlySubscriptionUrl=""
monthlySubscriptionUrl=""
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={["UpdatedEmailPreferencesOption"]}
enabledFeatureFlags={[]}
experimentData={defaultExperimentData["Features"]}
isMonthlySubscriber={true}
data={mockedPlusSubscriberEmailPreferences}
Expand Down Expand Up @@ -2252,7 +2249,7 @@ describe("Settings page", () => {
yearlySubscriptionUrl=""
monthlySubscriptionUrl=""
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={["UpdatedEmailPreferencesOption"]}
enabledFeatureFlags={[]}
experimentData={defaultExperimentData["Features"]}
isMonthlySubscriber={true}
data={mockedPlusSubscriberEmailPreferences}
Expand Down Expand Up @@ -2311,10 +2308,7 @@ describe("Settings page", () => {
yearlySubscriptionUrl=""
monthlySubscriptionUrl=""
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={[
"UpdatedEmailPreferencesOption",
"MonthlyReportFreeUser",
]}
enabledFeatureFlags={[]}
experimentData={defaultExperimentData["Features"]}
isMonthlySubscriber={false}
data={mockedFreeSubscriberEmailPreferences}
Expand Down Expand Up @@ -2357,7 +2351,7 @@ describe("Settings page", () => {
yearlySubscriptionUrl=""
monthlySubscriptionUrl=""
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={["UpdatedEmailPreferencesOption"]}
enabledFeatureFlags={[]}
experimentData={defaultExperimentData["Features"]}
isMonthlySubscriber={true}
data={mockedPlusSubscriberEmailPreferences}
Expand Down Expand Up @@ -2403,7 +2397,7 @@ describe("Settings page", () => {
yearlySubscriptionUrl=""
monthlySubscriptionUrl=""
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={["UpdatedEmailPreferencesOption"]}
enabledFeatureFlags={[]}
experimentData={defaultExperimentData["Features"]}
isMonthlySubscriber={true}
data={mockedPlusSubscriberEmailPreferences}
Expand Down Expand Up @@ -2459,7 +2453,7 @@ describe("Settings page", () => {
yearlySubscriptionUrl=""
monthlySubscriptionUrl=""
subscriptionBillingAmount={mockedSubscriptionBillingAmount}
enabledFeatureFlags={["UpdatedEmailPreferencesOption"]}
enabledFeatureFlags={[]}
experimentData={defaultExperimentData["Features"]}
isMonthlySubscriber={true}
data={mockedPlusSubscriberEmailPreferences}
Expand Down
5 changes: 0 additions & 5 deletions src/db/tables/featureFlags.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,12 @@ export async function getDeletedFeatureFlags() {
}

export const featureFlagNames = [
"UpdatedEmailPreferencesOption",
"CancellationFlow",
"ConfirmCancellation",
"FirstDataBrokerRemovalFixedEmail",
"DiscountCouponNextThreeMonths",
"LatestScanDateCsatSurvey",
"AutomaticRemovalCsatSurvey",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if we could keep all the feature flags related to the CSAT surveys. We only recently landed a PR that extends the functionality of this CSAT PR #5586.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure: when you say "keep all the feature flags", in the context of this PR you only mean undoing the removal of AutomaticRemovalCsatSurvey, right? I did that in 2cac1d1. Thanks for noticing.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Yes, I specifically mean “keep all the feature flags related to the CSAT surveys”.

"AdditionalRemovalStatuses",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is also a feature flag we would need to keep. We’ve recently disabled it with an issue related to the last opt-out removal. Even though, this issue will be addressed by PR #5591 the flag AdditionalRemovalStatuses, it depends on the opt-out removal to be activated and to work correctly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted this in 82df2e2. Could you create a ticket that specifies when this flag is good to remove?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve created MNTOR-4048 for when we are ready to remove the flag AdditionalRemovalStatuses.

"PetitionBannerCsatSurvey",
"MonthlyReportFreeUser",
"BreachEmailRedesign",
"PromptNoneAuthFlow",
"GA4SubscriptionEvents",
"DataBrokerRemovalTimeEstimateLabel",
Expand Down
42 changes: 4 additions & 38 deletions src/db/tables/subscribers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ import type { Profile } from "next-auth";
import type { EmailAddressRow, SubscriberRow } from "knex/types/tables";
import createDbConnection from "../connect";
import { SerializedSubscriber } from "../../next-auth.js";
import { getFeatureFlagData } from "./featureFlags";
import { getEnvVarsOrThrow } from "../../envVars";
import { parseIso8601Datetime } from "../../utils/parse";
import { logger } from "../../app/functions/server/logging";

const knex = createDbConnection();
const { DELETE_UNVERIFIED_SUBSCRIBERS_TIMER } = getEnvVarsOrThrow([
Expand Down Expand Up @@ -333,12 +331,7 @@ async function deleteResolutionsWithEmail(id: number, email: string) {
async function getPotentialSubscribersWaitingForFirstDataBrokerRemovalFixedEmail(): Promise<
SubscriberRow[]
> {
const flag = await getFeatureFlagData("FirstDataBrokerRemovalFixedEmail");
if (!flag?.is_enabled || !flag?.modified_at) {
return [];
}

let query = knex("subscribers")
const rows = await knex("subscribers")
.select()
// Only send to Plus users...
.whereRaw(
Expand All @@ -350,17 +343,8 @@ async function getPotentialSubscribersWaitingForFirstDataBrokerRemovalFixedEmail
// ...who haven't received the email...
.andWhere("first_broker_removal_email_sent", false)
// ...and signed up after the feature flag `FirstDataBrokerRemovalFixedEmail`
// has been enabled last.
.andWhere("created_at", ">=", flag.modified_at);
Vinnl marked this conversation as resolved.
Show resolved Hide resolved

if (Array.isArray(flag.allow_list) && flag.allow_list.length > 0) {
// If the feature flag has an allowlist, only send to users on that list.
// The `.andWhereIn` alias doesn't exist:
// https://github.com/knex/knex/issues/1881#issuecomment-275433906
query = query.whereIn("primary_email", flag.allow_list);
}

const rows = await query;
// has been enabled (which happened on 2024-07-10).
.andWhere("created_at", ">=", "2024-07-10T00:00:00.000Z");

return rows;
}
Expand Down Expand Up @@ -421,23 +405,12 @@ 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 ??
"2022-01-01T00:00:00.000Z",
);

if (
!flag?.is_enabled &&
!(Array.isArray(flag?.allow_list) && flag.allow_list.length > 0)
) {
logger.info("monthly_free_report_disabled", {
flag: flag,
});
return [];
}

let query = knex("subscribers")
const query = knex("subscribers")
.select<SubscriberRow[]>("subscribers.*")
.select(
knex.raw(
Expand Down Expand Up @@ -507,13 +480,6 @@ async function getFreeSubscribersWaitingForMonthlyEmail(
),
);

if (Array.isArray(flag.allow_list) && flag.allow_list.length > 0) {
// If the feature flag has an allowlist, only send to users on that list.
// The `.andWhereIn` alias doesn't exist:
// https://github.com/knex/knex/issues/1881#issuecomment-275433906
query = query.whereIn("subscribers.primary_email", flag.allow_list);
}

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
Expand Down
Loading
Loading