From 0ec33a95ea01e71c92c1ee83ba9c3c8a98629b51 Mon Sep 17 00:00:00 2001 From: Vincent Date: Thu, 1 Aug 2024 14:44:48 +0200 Subject: [PATCH] Disabling a flag also dis-allowlists all I actually think the previous approach makes more sense, but it wasn't the behaviour that we intended - the allowlists were intended to be a restriction on the `enabled: true` property (i.e. clearing the allowlist enabled it for everyone), rather than a loosening of `enabled: false`. This is also why we have the `ignoreAllowlist` option, to allow the use of feature flags on public pages where we do not have a specific user. We might still want to consciously choose the previous behaviour (yes please!), but then we should replace the `ignoreAllowlist` option by a `disableAllowlist` option that only removes the ability to expose the flag to specific users. --- src/db/tables/featureFlags.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/db/tables/featureFlags.ts b/src/db/tables/featureFlags.ts index 03495f7ae6b..0ab566dfda0 100644 --- a/src/db/tables/featureFlags.ts +++ b/src/db/tables/featureFlags.ts @@ -67,9 +67,12 @@ export async function getEnabledFeatureFlags( .and.where("is_enabled", true); if (!options.ignoreAllowlist) { - query = query.and - .whereRaw("ARRAY_LENGTH(allow_list, 1) IS NULL") - .orWhereRaw("? = ANY(allow_list)", options.email); + query = query.andWhere( + (whereBuilder) => + void whereBuilder + .whereRaw("ARRAY_LENGTH(allow_list, 1) IS NULL") + .orWhereRaw("? = ANY(allow_list)", options.email), + ); } const enabledFlagNames = await query;