Remove unused feature flags #2136

Vinnl · 2022-06-28T18:01:37Z

New feature description

This removes "old" feature flags, to be replaced by Waffle-based ones.

Note that this does enable the hitherto gated feature
generateCustomAliasSubdomain (a popover when clicking your
subdomain explaining how to create a custom mask).
I'll also ask Tony and Eduardo to confirm that this is OK. (Edit: confirmed!)

Additionally, it makes the interview recruitment banner gated by the new-style
feature flags, which doesn't have a flag for that feature yet. That
means it will in effect be disabled for now - I'm verifying that this is OK.
If not, I'll have to learn how to add it. (Edit: also verified that that's OK!)

Screenshot (if applicable)

Not applicable.

How to test

Nothing should have changed, other than that this popup is now present, and the CSAT survey is restored:

Checklist

l10n changes have been submitted to the l10n repository, if any.
All acceptance criteria are met.
I've added or updated relevant docs in the docs/ directory.
All UI revisions follow the coding standards, and use Protocol tokens where applicable (see /frontend/src/styles/tokens.scss).
Commits in this PR are minimal and have descriptive commit messages.

Vinnl · 2022-06-28T18:02:13Z

emails/models.py

@@ -70,14 +70,12 @@ def valid_available_subdomain(subdomain, *args, **kwargs):


 def default_server_storage():
-    return datetime.now(timezone.utc) > settings.PREMIUM_RELEASE_DATE
+    return True


I kept this since a migration (specifically 0029_profile_add_deleted_metric_and_changeserver_storage_default.py for the emails app) pointed to this function as the one to generate a default value. Not sure if there's a better way to do that.

Suggestion: Add a comment like "This historical function is referenced in migration 0029_profile_add_deleted_metric_and_changeserver_storage_default"

Good idea: a12cf6b

Vinnl · 2022-06-28T18:02:47Z

docs/frontend-architecture.md

@@ -67,15 +67,6 @@ environment variables to the outside world. You can extend this endpoint in
 using the `useRuntimeData` hook; you can find this in
 `/frontend/src/hooks/api/runtimeData.ts`.

-## Add a feature flag


IIRC Se Yeon is writing docs on how feature flagging with Waffle will be done, so I didn't include replacement docs for this.

Vinnl · 2022-06-28T18:07:10Z

frontend/src/functions/waffle.ts

@@ -4,7 +4,7 @@ export type RuntimeDataWithWaffle = RuntimeData & {
  WAFFLE_FLAGS: RuntimeData["WAFFLE_FLAGS"];
 };

-export function flagIsActive(
+export function isFlagActive(


I renamed this to be aligned with other function names (i.e. start with a verb, is if it returns a boolean) because I couldn't find this function at first, but since we don't have an explicit guideline for that anyway, I'm happy to drop this commit.

Personally, this makes sense to me. +1 from me.

jwhitlock

Thanks @Vinnl! I love cleaning up old code!

I've got some suggestions, and we need the migration for the updated model.

jwhitlock · 2022-06-28T22:00:11Z

emails/models.py

@@ -70,14 +70,12 @@ def valid_available_subdomain(subdomain, *args, **kwargs):


 def default_server_storage():
-    return datetime.now(timezone.utc) > settings.PREMIUM_RELEASE_DATE
+    return True


Suggestion: Add a comment like "This historical function is referenced in migration 0029_profile_add_deleted_metric_and_changeserver_storage_default"

jwhitlock · 2022-06-28T22:02:17Z

emails/models.py

@@ -102,7 +100,7 @@ class Profile(models.Model):
        db_index=True,
        validators=[valid_available_subdomain],
    )
-    server_storage = models.BooleanField(default=default_server_storage)
+    server_storage = models.BooleanField(default=True)


This will require a migration, please run ./manage.py makemigrations; black . and add the new / changed files

Oh, thanks, I did not know that! a1cd4e6

jwhitlock · 2022-06-28T22:03:52Z

emails/models.py

@@ -303,7 +301,7 @@ def emails_replied(self):
    @property
    def joined_before_premium_release(self):
        date_created = self.user.date_joined
-        return date_created < settings.PREMIUM_RELEASE_DATE
+        return date_created < datetime.fromisoformat('2021-10-22 17:00:00+00:00')


Was this the actual premium release date? I wouldn't mind getting that correct, although it may screw up tests.

I believe so, since that was the value of this var in .env-dist: https://github.com/mozilla/fx-private-relay/pull/2136/files#diff-9b75aa53740e3b569fdd203e0b882eaadadc8c03b5c840e648f37a5b2ca7b622L36

There were a couple of tests that overrode the env var to make sure they preceded or followed the given date, but I went over the (non-removed) ones and verified that the dates were still correct relative to this static date.

netlify · 2022-06-28T23:14:39Z

✅ Deploy Preview for fx-relay-demo canceled.

Name	Link
🔨 Latest commit	`56d945f`
🔍 Latest deploy log	https://app.netlify.com/sites/fx-relay-demo/deploys/62bb8bc6ef3f510008f38790

lloan

LGTM.

I reviewed the React items. Leaving the Python code up to BE team.

jwhitlock

Thanks @Vinnl, looks good to me!

We're replacing them with a more sophisticated mechanism that does not require recompiling the front-end, and allows gradually enabling features for specific groups of users. Note that this does enable the hitherto gated feature `generateCustomAliasSubdomain` (a popover when clicking your subdomain explaining how to create a custom mask). Additionally, it makes the interview recruitment banner gated by the new-style feature flags, which doesn't have a flag for that feature yet. That means it will in effect be disabled for now.

This makes it consistent with other functions in that it starts with a verb, and specifically with `is` because it returns a boolean.

Vinnl added 🛑 Do Not Merge Do not merge this PR, even if approved. Review: XS Code review time: up to 30min labels Jun 28, 2022

Vinnl requested review from jwhitlock, maxxcrawford, lloan and say-yawn June 28, 2022 18:01

Vinnl self-assigned this Jun 28, 2022

Vinnl commented Jun 28, 2022

View reviewed changes

Vinnl removed the 🛑 Do Not Merge Do not merge this PR, even if approved. label Jun 28, 2022

jwhitlock requested changes Jun 28, 2022

View reviewed changes

Vinnl force-pushed the remove-unused-feature-flags branch from a12cf6b to 56d945f Compare June 28, 2022 23:16

Vinnl requested a review from jwhitlock June 28, 2022 23:16

lloan approved these changes Jun 29, 2022

View reviewed changes

jwhitlock approved these changes Jun 29, 2022

View reviewed changes

lloan mentioned this pull request Jun 29, 2022

Revise flagIsActive arguments to use runtimeData that is already included in the component #2120

Merged

Vinnl added 4 commits June 29, 2022 23:28

Remove PREMIUM_RELEASE_DATE feature flag

99baf1d

Remove unused flag DOMAIN_REGISTRATION_MODAL

d424700

Rename flagIsActive to isFlagActive

2807c8f

This makes it consistent with other functions in that it starts with a verb, and specifically with `is` because it returns a boolean.

Vinnl force-pushed the remove-unused-feature-flags branch from 56d945f to 2807c8f Compare June 29, 2022 21:29

Vinnl merged commit b3c2a40 into main Jun 29, 2022

Vinnl deleted the remove-unused-feature-flags branch June 29, 2022 21:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove unused feature flags #2136

Remove unused feature flags #2136

Vinnl commented Jun 28, 2022 •

edited

Loading

Vinnl Jun 28, 2022

jwhitlock Jun 28, 2022

Vinnl Jun 28, 2022

Vinnl Jun 28, 2022

Vinnl Jun 28, 2022 •

edited

Loading

lloan Jun 29, 2022

jwhitlock left a comment

jwhitlock Jun 28, 2022

jwhitlock Jun 28, 2022

Vinnl Jun 28, 2022

jwhitlock Jun 28, 2022

Vinnl Jun 28, 2022

netlify bot commented Jun 28, 2022 •

edited

Loading

lloan left a comment

jwhitlock left a comment

Remove unused feature flags #2136

Remove unused feature flags #2136

Conversation

Vinnl commented Jun 28, 2022 • edited Loading

New feature description

Screenshot (if applicable)

How to test

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vinnl Jun 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwhitlock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Jun 28, 2022 • edited Loading

✅ Deploy Preview for fx-relay-demo canceled.

lloan left a comment

Choose a reason for hiding this comment

jwhitlock left a comment

Choose a reason for hiding this comment

Vinnl commented Jun 28, 2022 •

edited

Loading

Vinnl Jun 28, 2022 •

edited

Loading

netlify bot commented Jun 28, 2022 •

edited

Loading