Skip to content

Commit

Permalink
Disabling a flag also dis-allowlists all
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Vinnl committed Aug 7, 2024
1 parent f065b9c commit 0ec33a9
Showing 1 changed file with 6 additions and 3 deletions.
9 changes: 6 additions & 3 deletions src/db/tables/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 0ec33a9

Please sign in to comment.