yindo/posthog
1
0
Fork 0
mirror of https://github.com/BillyOutlast/posthog.git synced 2026-02-04 03:01:23 +01:00
Code Issues Packages Projects Releases Wiki Activity

fix(flags): evaluate disabled flags and add dependency warnings (#40351)

Browse Source 
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
This commit is contained in:
Gustavo H. Strassburger
2025-11-13 21:19:09 -03:00
committed by GitHub
parent 8425c2bf82
commit 35cc7e06e5
13 changed files with 467 additions and 121 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
frontend/src/scenes/feature-flags/ConfirmationModal.tsx
View File

@@ -11,7 +11,9 @@ interface ConfirmationModalProps {
type: ConfirmationModalType
activeNewValue?: boolean // Only for flag-status type
changes?: string[] // Only for multi-changes type
customMessage?: string // Custom message for multi-changes type
customConfirmationMessage?: string // Custom confirmation message to replace default message
extraMessages?: string[] // Additional messages to display after the main message
featureFlagConfirmationEnabled?: boolean // Whether the team has feature flag confirmation enabled in settings
onConfirm: () => void
}
@@ -24,7 +26,9 @@ export function openConfirmationModal({
type,
activeNewValue,
changes = [],
customMessage,
customConfirmationMessage,
extraMessages,
featureFlagConfirmationEnabled = false,
onConfirm,
}: ConfirmationModalProps): void {
let title: string
@@ -72,6 +76,18 @@ export function openConfirmationModal({
const defaultMessage =
'⚠️ These changes will immediately affect users matching the release conditions. Please ensure you understand the consequences before proceeding.'
const allMessages: string[] = []
if (customConfirmationMessage) {
allMessages.push(customConfirmationMessage)
} else if (featureFlagConfirmationEnabled) {
allMessages.push(defaultMessage)
}
if (extraMessages && extraMessages.length > 0) {
allMessages.push(...extraMessages)
}
description = (
<>
<p>You are about to save the following changes:</p>
@@ -80,7 +96,11 @@ export function openConfirmationModal({
<li key={index}>{change}</li>
))}
</ul>
<p className="mt-4">{customMessage || defaultMessage}</p>
{allMessages.map((message, index) => (
<p key={index} className="mt-4">
{message}
</p>
))}
</>
)
break

27
frontend/src/scenes/feature-flags/FeatureFlag.tsx
View File

@@ -962,9 +962,9 @@ function FeatureFlagRollout({ readOnly }: { readOnly?: boolean }): JSX.Element {
removeVariant,
setMultivariateEnabled,
setFeatureFlag,
saveFeatureFlag,
setRemoteConfigEnabled,
resetEncryptedPayload,
toggleFeatureFlagActive,
} = useActions(featureFlagLogic)
const { addProductIntentForCrossSell } = useActions(teamLogic)
@@ -1052,30 +1052,7 @@ function FeatureFlagRollout({ readOnly }: { readOnly?: boolean }): JSX.Element {
>
<LemonSwitch
onChange={(newValue) => {
LemonDialog.open({
title: `${newValue === true ? 'Enable' : 'Disable'} this flag?`,
description: `This flag will be immediately ${
newValue === true ? 'rolled out to' : 'rolled back from'
} the users matching the release conditions.`,
primaryButton: {
children: 'Confirm',
type: 'primary',
onClick: () => {
const updatedFlag = {
...featureFlag,
active: newValue,
}
setFeatureFlag(updatedFlag)
saveFeatureFlag(updatedFlag)
},
size: 'small',
},
secondaryButton: {
children: 'Cancel',
type: 'tertiary',
size: 'small',
},
})
toggleFeatureFlagActive(newValue)
}}
label={featureFlag.active ? 'Enabled' : 'Disabled'}
disabledReason={

16
frontend/src/scenes/feature-flags/featureFlagConfirmationLogic.ts
View File

@@ -31,12 +31,14 @@ function detectFeatureFlagChanges(
}
// Check for active status changes
let statusChanged = false
if (originalFlag.active !== updatedFlag.active) {
if (updatedFlag.active) {
changes.push('Enable the feature flag')
} else {
changes.push('Disable the feature flag')
}
statusChanged = true
}
// Check for any filter changes (comprehensive detection)
@@ -93,7 +95,7 @@ function detectFeatureFlagChanges(
}
// If we haven't caught the specific change, add a generic message
if (!rolloutChanged && !variantsChanged && !conditionsChanged && !payloadsChanged) {
if (!rolloutChanged && !variantsChanged && !conditionsChanged && !payloadsChanged && !statusChanged) {
changes.push('Feature flag configuration changed')
}
}
@@ -105,12 +107,14 @@ function detectFeatureFlagChanges(
export function checkFeatureFlagConfirmation(
originalFlag: FeatureFlagType | null,
updatedFlag: FeatureFlagType,
confirmationEnabled: boolean,
customMessage: string | undefined,
shouldDisplayConfirmation: boolean,
customConfirmationMessage: string | undefined,
extraMessages: string[] | undefined,
featureFlagConfirmationEnabled: boolean,
onConfirm: () => void
): boolean {
// Check if confirmation is needed
const needsConfirmation = !!updatedFlag.id && confirmationEnabled
const needsConfirmation = !!updatedFlag.id && shouldDisplayConfirmation
if (needsConfirmation) {
const changes = detectFeatureFlagChanges(originalFlag, updatedFlag)
@@ -121,7 +125,9 @@ export function checkFeatureFlagConfirmation(
featureFlag: updatedFlag,
type: 'multi-changes',
changes: changes,
customMessage: customMessage,
customConfirmationMessage: customConfirmationMessage,
extraMessages: extraMessages,
featureFlagConfirmationEnabled: featureFlagConfirmationEnabled,
onConfirm: onConfirm,
})
return true // Confirmation modal shown, don't proceed with save

134
frontend/src/scenes/feature-flags/featureFlagLogic.ts
View File

@@ -1,4 +1,16 @@
import { actions, afterMount, connect, kea, key, listeners, path, props, reducers, selectors } from 'kea'
import {
actions,
afterMount,
connect,
kea,
key,
listeners,
path,
props,
reducers,
selectors,
sharedListeners,
} from 'kea'
import { DeepPartialMap, ValidationErrorType, forms } from 'kea-forms'
import { loaders } from 'kea-loaders'
import { router, urlToAction } from 'kea-router'
@@ -369,6 +381,8 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
) => ({ filters, active, errors, variants, payloads }),
setScheduledChangeOperation: (changeType: ScheduledChangeOperationType) => ({ changeType }),
setAccessDeniedToFeatureFlag: true,
toggleFeatureFlagActive: (active: boolean) => ({ active }),
submitFeatureFlagWithValidation: (featureFlag: Partial<FeatureFlagType>) => ({ featureFlag }),
}),
forms(({ actions, values }) => ({
featureFlag: {
@@ -399,31 +413,8 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
},
}
},
submit: (featureFlag) => {
// Use confirmation logic from dedicated file
const confirmationShown = checkFeatureFlagConfirmation(
values.originalFeatureFlag,
featureFlag,
!!values.currentTeam?.feature_flag_confirmation_enabled,
values.currentTeam?.feature_flag_confirmation_message || undefined,
() => {
// This callback is called when confirmation is completed or not needed
if (featureFlag.id) {
actions.saveFeatureFlag(featureFlag)
} else {
actions.saveFeatureFlag({ ...featureFlag, _create_in_folder: 'Unfiled/Feature Flags' })
}
}
)
// If no confirmation was shown, proceed immediately
if (!confirmationShown) {
if (featureFlag.id) {
actions.saveFeatureFlag(featureFlag)
} else {
actions.saveFeatureFlag({ ...featureFlag, _create_in_folder: 'Unfiled/Feature Flags' })
}
}
submit: async (featureFlag) => {
await actions.submitFeatureFlagWithValidation(featureFlag)
},
},
})),
@@ -688,6 +679,62 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
},
],
}),
sharedListeners(({ values }) => ({
checkDependentFlagsAndConfirm: async (payload: {
originalFlag: FeatureFlagType | null
updatedFlag: Partial<FeatureFlagType>
onConfirm: () => void
}) => {
const { originalFlag, updatedFlag, onConfirm } = payload
// Check if flag is being disabled and has active dependents
const isBeingDisabled = updatedFlag.id && originalFlag?.active === true && updatedFlag.active === false
let dependentFlagsWarning: string | undefined
if (isBeingDisabled) {
try {
const response = await api.create(
`api/projects/${values.currentProjectId}/feature_flags/${updatedFlag.id}/has_active_dependents/`,
{}
)
if (response.has_active_dependents) {
dependentFlagsWarning = response.warning
}
} catch {
lemonToast.error('Failed to check for dependent flags. Please try again.')
return
}
}
const extraMessages: string[] = []
if (dependentFlagsWarning) {
extraMessages.push(dependentFlagsWarning)
}
const featureFlagConfirmationEnabled = !!values.currentTeam?.feature_flag_confirmation_enabled
let customConfirmationMessage: string | undefined
if (featureFlagConfirmationEnabled) {
customConfirmationMessage = values.currentTeam?.feature_flag_confirmation_message
}
const shouldDisplayConfirmation = featureFlagConfirmationEnabled || extraMessages.length > 0
const confirmationShown = checkFeatureFlagConfirmation(
originalFlag,
updatedFlag as FeatureFlagType,
shouldDisplayConfirmation,
customConfirmationMessage,
extraMessages,
featureFlagConfirmationEnabled,
onConfirm
)
// If no confirmation was shown, proceed immediately
if (!confirmationShown) {
onConfirm()
}
},
})),
loaders(({ values, props, actions }) => ({
featureFlag: {
loadFeatureFlag: async () => {
@@ -1073,7 +1120,7 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
},
},
})),
listeners(({ actions, values, props }) => ({
listeners(({ actions, values, props, sharedListeners }) => ({
submitNewDashboardSuccessWithResult: async ({ result }) => {
await api.update(`api/projects/${values.currentProjectId}/feature_flags/${values.featureFlag.id}`, {
analytics_dashboards: [result.id],
@@ -1296,6 +1343,39 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
actions.loadFeatureFlag()
}
},
submitFeatureFlagWithValidation: async ({ featureFlag }, breakpoint, action, previousState) => {
await sharedListeners.checkDependentFlagsAndConfirm(
{
originalFlag: values.originalFeatureFlag,
updatedFlag: featureFlag,
onConfirm: () => {
if (featureFlag.id) {
actions.saveFeatureFlag(featureFlag)
} else {
actions.saveFeatureFlag({ ...featureFlag, _create_in_folder: 'Unfiled/Feature Flags' })
}
},
},
breakpoint,
action as any,
previousState
)
},
toggleFeatureFlagActive: async ({ active }, breakpoint, action, previousState) => {
const updatedFlag = { ...values.featureFlag, active }
await sharedListeners.checkDependentFlagsAndConfirm(
{
originalFlag: values.originalFeatureFlag,
updatedFlag,
onConfirm: () => {
actions.saveFeatureFlag(updatedFlag)
},
},
breakpoint,
action as any,
previousState
)
},
})),
selectors({
sentryErrorCount: [(s) => [s.sentryStats], (stats) => stats.total_count],

45
posthog/api/feature_flag.py
View File

@@ -904,11 +904,11 @@ class FeatureFlagSerializer(
and validated_data[field] != getattr(current_instance, field)
]
def _find_dependent_flags(self, flag_to_delete: FeatureFlag) -> list[FeatureFlag]:
def _find_dependent_flags(self, flag_to_check: FeatureFlag) -> list[FeatureFlag]:
"""Find all active flags that depend on the given flag."""
return list(
FeatureFlag.objects.filter(team=flag_to_delete.team, deleted=False, active=True)
.exclude(id=flag_to_delete.id)
FeatureFlag.objects.filter(team=flag_to_check.team, deleted=False, active=True)
.exclude(id=flag_to_check.id)
.extra(
where=[
"""
@@ -920,7 +920,7 @@ class FeatureFlagSerializer(
)
"""
],
params=[str(flag_to_delete.id)],
params=[str(flag_to_check.id)],
)
.order_by("key")
)
@@ -1400,6 +1400,43 @@ class FeatureFlagViewSet(
return Response({"success": True}, status=200)
@action(methods=["POST"], detail=True)
def has_active_dependents(self, request: request.Request, **kwargs):
"""Check if this flag has other active flags that depend on it."""
feature_flag: FeatureFlag = self.get_object()
# Use the serializer class method to find dependent flags
serializer = self.serializer_class()
dependent_flags = serializer._find_dependent_flags(feature_flag)
has_dependents = len(dependent_flags) > 0
if not has_dependents:
return Response({"has_active_dependents": False, "dependent_flags": []}, status=200)
dependent_flag_data = [
{
"id": flag.id,
"key": flag.key,
"name": flag.name or flag.key,
}
for flag in dependent_flags
]
return Response(
{
"has_active_dependents": True,
"dependent_flags": dependent_flag_data,
"warning": (
f"This feature flag is used by {len(dependent_flags)} other active "
f"{'flag' if len(dependent_flags) == 1 else 'flags'}. "
f"Disabling it will cause {'that flag' if len(dependent_flags) == 1 else 'those flags'} "
f"to evaluate this condition as false."
),
},
status=200,
)
@action(methods=["GET"], detail=False)
def my_flags(self, request: request.Request, **kwargs):
if not request.user.is_authenticated: # for mypy

43
posthog/api/test/test_feature_flag_dependency_deletion.py
View File

@@ -259,3 +259,46 @@ class TestFeatureFlagDependencyDeletion(APIBaseTest):
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
def test_has_active_dependents_with_no_dependencies(self):
"""Test has_active_dependents returns False with 0 dependent flags."""
flag = self.create_flag("standalone_flag")
response = self.client.post(
f"/api/projects/{self.team.id}/feature_flags/{flag.id}/has_active_dependents/",
format="json",
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json()["has_active_dependents"], False)
self.assertEqual(len(response.json()["dependent_flags"]), 0)
def test_has_active_dependents_with_active_dependencies(self):
"""Test has_active_dependents returns True with 1 active dependent flag."""
base_flag = self.create_flag("base_flag")
self.create_flag("dependent_flag", dependencies=[base_flag.id])
response = self.client.post(
f"/api/projects/{self.team.id}/feature_flags/{base_flag.id}/has_active_dependents/",
format="json",
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json()["has_active_dependents"], True)
self.assertEqual(len(response.json()["dependent_flags"]), 1)
def test_has_active_dependents_with_inactive_dependencies(self):
"""Test has_active_dependents returns False when dependent flags are inactive."""
base_flag = self.create_flag("base_flag")
dependent_flag = self.create_flag("dependent_flag", dependencies=[base_flag.id])
dependent_flag.active = False
dependent_flag.save()
response = self.client.post(
f"/api/projects/{self.team.id}/feature_flags/{base_flag.id}/has_active_dependents/",
format="json",
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json()["has_active_dependents"], False)
self.assertEqual(len(response.json()["dependent_flags"]), 0)

1
rust/feature-flags/src/api/types.rs
View File

@@ -441,6 +441,7 @@ impl FromFeatureAndMatch for FlagDetails {
FeatureFlagMatchReason::HoldoutConditionValue => {
Some("Holdout condition value".to_string())
}
FeatureFlagMatchReason::FlagDisabled => Some("Feature flag is disabled".to_string()),
}
}
}

1
rust/feature-flags/src/flags/feature_flag_list.rs
View File

@@ -124,7 +124,6 @@ impl FeatureFlagList {
LEFT JOIN posthog_tag AS tag ON (et.tag_id = tag.id)
WHERE t.project_id = $1
AND f.deleted = false
AND f.active = true
GROUP BY f.id, f.team_id, f.name, f.key, f.filters, f.deleted, f.active,
f.ensure_experience_continuity, f.version, f.evaluation_runtime
"#;

21
rust/feature-flags/src/flags/flag_match_reason.rs
View File

@@ -15,17 +15,20 @@ pub enum FeatureFlagMatchReason {
NoGroupType,
#[strum(serialize = "holdout_condition_value")]
HoldoutConditionValue,
#[strum(serialize = "flag_disabled")]
FlagDisabled,
}
impl FeatureFlagMatchReason {
pub fn score(&self) -> i32 {
match self {
FeatureFlagMatchReason::SuperConditionValue => 5,
FeatureFlagMatchReason::HoldoutConditionValue => 4,
FeatureFlagMatchReason::ConditionMatch => 3,
FeatureFlagMatchReason::NoGroupType => 2,
FeatureFlagMatchReason::OutOfRolloutBound => 1,
FeatureFlagMatchReason::NoConditionMatch => 0,
FeatureFlagMatchReason::SuperConditionValue => 6,
FeatureFlagMatchReason::HoldoutConditionValue => 5,
FeatureFlagMatchReason::ConditionMatch => 4,
FeatureFlagMatchReason::NoGroupType => 3,
FeatureFlagMatchReason::OutOfRolloutBound => 2,
FeatureFlagMatchReason::NoConditionMatch => 1,
FeatureFlagMatchReason::FlagDisabled => 0,
}
}
}
@@ -54,6 +57,7 @@ impl std::fmt::Display for FeatureFlagMatchReason {
FeatureFlagMatchReason::OutOfRolloutBound => "out_of_rollout_bound",
FeatureFlagMatchReason::NoGroupType => "no_group_type",
FeatureFlagMatchReason::HoldoutConditionValue => "holdout_condition_value",
FeatureFlagMatchReason::FlagDisabled => "flag_disabled",
}
)
}
@@ -66,6 +70,7 @@ mod tests {
#[test]
fn test_ordering() {
let reasons = vec![
FeatureFlagMatchReason::FlagDisabled,
FeatureFlagMatchReason::NoConditionMatch,
FeatureFlagMatchReason::OutOfRolloutBound,
FeatureFlagMatchReason::NoGroupType,
@@ -101,5 +106,9 @@ mod tests {
FeatureFlagMatchReason::NoGroupType.to_string(),
"no_group_type"
);
assert_eq!(
FeatureFlagMatchReason::FlagDisabled.to_string(),
"flag_disabled"
);
}
}

13
rust/feature-flags/src/flags/flag_matching.rs
View File

@@ -648,9 +648,7 @@ impl FeatureFlagMatcher {
let flags_to_evaluate = flags
.iter()
.filter(|flag| {
!flag.deleted && !evaluated_flags_map.contains_key(&flag.key) && flag.active
})
.filter(|flag| !flag.deleted && !evaluated_flags_map.contains_key(&flag.key))
.copied()
.collect::<Vec<&FeatureFlag>>();
@@ -807,6 +805,15 @@ impl FeatureFlagMatcher {
property_overrides: Option<HashMap<String, Value>>,
hash_key_overrides: Option<HashMap<String, String>>,
) -> Result<FeatureFlagMatch, FlagError> {
if !flag.active {
return Ok(FeatureFlagMatch {
matches: false,
variant: None,
reason: FeatureFlagMatchReason::FlagDisabled,
condition_index: None,
payload: None,
});
}
// Check if this is a group-based flag with missing group
let hashed_id = self.hashed_identifier(flag, hash_key_overrides.clone())?;
if flag.get_group_type_index().is_some() && hashed_id.is_empty() {

39
rust/feature-flags/src/flags/flag_operations.rs
View File

@@ -861,22 +861,12 @@ mod tests {
"deleted": true
});
let inactive_flag = json!({
"id": 2,
"team_id": team.id,
"name": "Inactive Flag",
"key": "inactive_flag",
"filters": {"groups": []},
"active": false,
"deleted": false
});
// Insert into Redis
insert_flags_for_team_in_redis(
redis_client.clone(),
team.id,
team.project_id(),
Some(json!([deleted_flag, inactive_flag]).to_string()),
Some(json!([deleted_flag]).to_string()),
)
.await
.expect("Failed to insert flags in Redis");
@@ -902,37 +892,13 @@ mod tests {
.await
.expect("Failed to insert deleted flag in Postgres");
context
.insert_flag(
team.id,
Some(FeatureFlagRow {
id: 0,
team_id: team.id,
name: Some("Inactive Flag".to_string()),
key: "inactive_flag".to_string(),
filters: inactive_flag["filters"].clone(),
deleted: false,
active: false,
ensure_experience_continuity: Some(false),
version: Some(1),
evaluation_runtime: Some("all".to_string()),
evaluation_tags: None,
}),
)
.await
.expect("Failed to insert inactive flag in Postgres");
// Fetch and verify from Redis
let redis_flags = get_flags_from_redis(redis_client, team.project_id())
.await
.expect("Failed to fetch flags from Redis");
assert_eq!(redis_flags.flags.len(), 2);
assert_eq!(redis_flags.flags.len(), 1);
assert!(redis_flags.flags.iter().any(|f| f.deleted));
assert!(redis_flags
.flags
.iter()
.any(|f| f.key == "inactive_flag" && !f.active));
// Fetch and verify from Postgres
let pg_flags =
@@ -941,7 +907,6 @@ mod tests {
.expect("Failed to fetch flags from Postgres");
assert_eq!(pg_flags.len(), 0);
assert!(!pg_flags.iter().any(|f| f.deleted)); // no deleted flags
assert!(!pg_flags.iter().any(|f| f.active)); // no inactive flags
}
#[tokio::test]

150
rust/feature-flags/src/flags/test_flag_matching.rs
View File

@@ -61,6 +61,7 @@ mod tests {
"team_id": team.id,
"name": "flag1",
"key": "flag1",
"active": true,
"filters": {
"groups": [
{
@@ -4759,6 +4760,7 @@ mod tests {
"team_id": team.id,
"name": "flag1",
"key": "flag1",
"active": true,
"filters": {
"groups": [
{
@@ -5593,4 +5595,152 @@ mod tests {
);
}
}
#[tokio::test]
async fn test_disabled_flag_evaluates_to_false() {
let context = TestContext::new(None).await;
let cohort_cache = Arc::new(CohortCacheManager::new(
context.non_persons_reader.clone(),
None,
None,
));
let team = context
.insert_new_team(None)
.await
.expect("Failed to insert team in pg");
let flag: FeatureFlag = serde_json::from_value(json!(
{
"id": 1,
"team_id": team.id,
"name": "disabled_flag",
"key": "disabled_flag",
"active": false,
"filters": {
"groups": [
{
"properties": [],
"rollout_percentage": 100
}
]
}
}
))
.unwrap();
let router = context.create_postgres_router();
let matcher = FeatureFlagMatcher::new(
"test_user".to_string(),
team.id,
team.project_id(),
router,
cohort_cache,
None,
None,
);
let match_result = matcher.get_match(&flag, None, None).unwrap();
assert!(!match_result.matches, "Disabled flag should not match");
assert_eq!(
match_result.reason,
FeatureFlagMatchReason::FlagDisabled,
"Reason should be FlagDisabled"
);
assert_eq!(
match_result.variant, None,
"Disabled flag should have no variant"
);
assert_eq!(
match_result.condition_index, None,
"Disabled flag should have no condition index"
);
}
#[tokio::test]
async fn test_flag_depending_on_disabled_flag_evaluates_to_false() {
use crate::utils::test_utils::create_test_flag_that_depends_on_flag;
let context = TestContext::new(None).await;
let cohort_cache = Arc::new(CohortCacheManager::new(
context.non_persons_reader.clone(),
None,
None,
));
let team = context
.insert_new_team(None)
.await
.expect("Failed to insert team in pg");
// Create a disabled base flag
let base_flag: FeatureFlag = serde_json::from_value(json!(
{
"id": 1,
"team_id": team.id,
"name": "base_flag",
"key": "base_flag",
"active": false,
"filters": {
"groups": [
{
"properties": [],
"rollout_percentage": 100
}
]
}
}
))
.unwrap();
// Create a flag that depends on the disabled flag
let dependent_flag = create_test_flag_that_depends_on_flag(
2,
team.id,
"dependent_flag",
1, // depends on flag id 1
FlagValue::Boolean(true),
);
let flags = FeatureFlagList {
flags: vec![base_flag, dependent_flag],
};
let router = context.create_postgres_router();
let mut matcher = FeatureFlagMatcher::new(
"test_user".to_string(),
team.id,
team.project_id(),
router,
cohort_cache,
None,
None,
);
let result = matcher
.evaluate_flags_with_overrides(flags, Default::default(), Uuid::new_v4(), None)
.await;
// Base flag should be disabled
let base_result = result.flags.get("base_flag").unwrap();
assert!(
!base_result.enabled,
"Disabled base flag should not be enabled"
);
assert_eq!(
base_result.reason.code, "flag_disabled",
"Base flag reason should be flag_disabled"
);
// Dependent flag should evaluate to false because the base flag is disabled
let dependent_result = result.flags.get("dependent_flag").unwrap();
assert!(
!dependent_result.enabled,
"Flag depending on disabled flag should not be enabled"
);
assert_ne!(
dependent_result.reason.code, "flag_disabled",
"Dependent flag should not have flag_disabled reason"
);
}
}

72
rust/feature-flags/src/handler/billing.rs
View File

@@ -68,21 +68,18 @@ pub async fn record_usage(
/// Checks if the flag list contains any billable flags.
///
/// Returns true if there are any flags that are NOT survey targeting flags.
/// Survey targeting flags (those starting with "survey-targeting-") are free
/// Returns true if there are any flags that are both active and NOT survey targeting flags.
/// Survey targeting flags (those starting with "survey-targeting-") and disabled flags are free
/// and don't count toward billing.
fn contains_billable_flags(filtered_flags: &FeatureFlagList) -> bool {
filtered_flags
.flags
.iter()
.any(|flag| is_billable_flag(&flag.key))
filtered_flags.flags.iter().any(is_billable_flag)
}
/// Determines if a flag is billable based on its key.
/// Determines if a flag is billable based on its key and active status.
///
/// Returns true for regular feature flags, false for survey targeting flags.
fn is_billable_flag(flag_key: &str) -> bool {
!flag_key.starts_with(SURVEY_TARGETING_FLAG_PREFIX)
/// Returns true for active regular feature flags, false for survey targeting flags or disabled flags.
fn is_billable_flag(flag: &crate::flags::flag_models::FeatureFlag) -> bool {
flag.active && !flag.key.starts_with(SURVEY_TARGETING_FLAG_PREFIX)
}
/// Helper function to determine if usage should be recorded
@@ -187,4 +184,59 @@ mod tests {
// Since we use any(), and the first flag should return true for "!starts_with()", overall result should be true
assert!(should_record_usage(&flag_list));
}
#[test]
fn test_should_record_usage_disabled_flags_not_billable() {
let mut disabled_flag = create_test_flag("regular_flag");
disabled_flag.active = false;
let flag_list = FeatureFlagList {
flags: vec![disabled_flag],
};
// Should NOT record usage when only disabled flags are present
assert!(!should_record_usage(&flag_list));
}
#[test]
fn test_should_record_usage_mixed_active_and_disabled() {
let mut disabled_flag = create_test_flag("disabled_flag");
disabled_flag.active = false;
let active_flag = create_test_flag("active_flag");
let flag_list = FeatureFlagList {
flags: vec![disabled_flag, active_flag],
};
// Should record usage when at least one active, non-survey flag is present
assert!(should_record_usage(&flag_list));
}
#[test]
fn test_should_record_usage_disabled_survey_flag() {
let mut disabled_survey_flag =
create_test_flag(&format!("{SURVEY_TARGETING_FLAG_PREFIX}survey1"));
disabled_survey_flag.active = false;
let flag_list = FeatureFlagList {
flags: vec![disabled_survey_flag],
};
// Should NOT record usage for disabled survey flags
assert!(!should_record_usage(&flag_list));
}
#[test]
fn test_should_record_usage_only_disabled_and_survey_flags() {
let mut disabled_flag = create_test_flag("disabled_flag");
disabled_flag.active = false;
let survey_flag = create_test_flag(&format!("{SURVEY_TARGETING_FLAG_PREFIX}survey1"));
let flag_list = FeatureFlagList {
flags: vec![disabled_flag, survey_flag],
};
// Should NOT record usage when only disabled and survey flags are present
assert!(!should_record_usage(&flag_list));
}
}