feat(flags): support groups and payloads correctly in new /flags se…

…rvice (#26817)

rust/feature-flags/src/api/endpoint.rs

@@ -46,7 +46,6 @@ pub async fn flags(
     let context = RequestContext {
         state,
         ip,
-        meta: meta.0,
         headers,
         body,
     };

rust/feature-flags/src/api/handler.rs

@@ -58,7 +58,6 @@ pub struct FlagsQueryParams {
 pub struct RequestContext {
     pub state: State<router::State>,
     pub ip: IpAddr,
-    pub meta: FlagsQueryParams,
     pub headers: HeaderMap,
     pub body: Bytes,
 }
@@ -82,11 +81,24 @@ pub struct FeatureFlagEvaluationContext {
     hash_key_override: Option<String>,
 }
 
+/// Process a feature flag request and return the evaluated flags
+///
+/// ## Flow
+/// 1. Decodes and validates the request
+/// 2. Extracts and verifies the authentication token
+/// 3. Retrieves team information
+/// 4. Processes person and group properties
+/// 5. Retrieves feature flags
+/// 6. Evaluates flags based on the context
+///
+/// ## Error Handling
+/// - Returns early if any step fails
+/// - Maintains error context through the FlagError enum
+/// - Individual flag evaluation failures don't fail the entire request
 pub async fn process_request(context: RequestContext) -> Result<FlagsResponse, FlagError> {
     let RequestContext {
         state,
         ip,
-        meta: _, // TODO use this
         headers,
         body,
     } = context;
@@ -95,20 +107,24 @@ pub async fn process_request(context: RequestContext) -> Result<FlagsResponse, F
     let token = request
         .extract_and_verify_token(state.redis.clone(), state.reader.clone())
         .await?;
+
     let team = request
         .get_team_from_cache_or_pg(&token, state.redis.clone(), state.reader.clone())
         .await?;
 
     let distinct_id = request.extract_distinct_id()?;
-    let groups = request.groups.clone();
     let team_id = team.id;
     let person_property_overrides = get_person_property_overrides(
         !request.geoip_disable.unwrap_or(false),
         request.person_properties.clone(),
         &ip,
         &state.geoip,
     );
-    let group_property_overrides = request.group_properties.clone();
+
+    let groups = request.groups.clone();
+    let group_property_overrides =
+        process_group_property_overrides(groups.clone(), request.group_properties.clone());
+
     let hash_key_override = request.anon_distinct_id.clone();
 
     let feature_flags_from_cache_or_pg = request
@@ -170,6 +186,39 @@ pub fn get_person_property_overrides(
     }
 }
 
+/// Processes group property overrides by combining existing overrides with group key overrides
+///
+/// When groups are provided in the format {"group_type": "group_key"}, we need to ensure these
+/// are included in the group property overrides with the special "$group_key" property.
+fn process_group_property_overrides(
+    groups: Option<HashMap<String, Value>>,
+    existing_overrides: Option<HashMap<String, HashMap<String, Value>>>,
+) -> Option<HashMap<String, HashMap<String, Value>>> {
+    match groups {
+        Some(groups) => {
+            let group_key_overrides: HashMap<String, HashMap<String, Value>> = groups
+                .into_iter()
+                .map(|(group_type, group_key)| {
+                    let mut properties = existing_overrides
+                        .as_ref()
+                        .and_then(|g| g.get(&group_type))
+                        .cloned()
+                        .unwrap_or_default();
+
+                    properties.insert("$group_key".to_string(), group_key);
+
+                    (group_type, properties)
+                })
+                .collect();
+
+            let mut result = existing_overrides.unwrap_or_default();
+            result.extend(group_key_overrides);
+            Some(result)
+        }
+        None => existing_overrides,
+    }
+}
+
 /// Decode a request into a `FlagRequest`
 /// - Currently only supports JSON requests
 // TODO support all supported content types
@@ -738,4 +787,61 @@ mod tests {
         assert!(!result.error_while_computing_flags);
         assert_eq!(result.feature_flags["test_flag"], FlagValue::Boolean(true));
     }
+
+    #[test]
+    fn test_process_group_property_overrides() {
+        // Test case 1: Both groups and existing overrides
+        let groups = HashMap::from([
+            ("project".to_string(), json!("project_123")),
+            ("organization".to_string(), json!("org_456")),
+        ]);
+
+        let mut existing_overrides = HashMap::new();
+        let mut project_props = HashMap::new();
+        project_props.insert("industry".to_string(), json!("tech"));
+        existing_overrides.insert("project".to_string(), project_props);
+
+        let result =
+            process_group_property_overrides(Some(groups.clone()), Some(existing_overrides));
+
+        assert!(result.is_some());
+        let result = result.unwrap();
+
+        // Check project properties
+        let project_props = result.get("project").expect("Project properties missing");
+        assert_eq!(project_props.get("industry"), Some(&json!("tech")));
+        assert_eq!(project_props.get("$group_key"), Some(&json!("project_123")));
+
+        // Check organization properties
+        let org_props = result
+            .get("organization")
+            .expect("Organization properties missing");
+        assert_eq!(org_props.get("$group_key"), Some(&json!("org_456")));
+
+        // Test case 2: Only groups, no existing overrides
+        let result = process_group_property_overrides(Some(groups.clone()), None);
+
+        assert!(result.is_some());
+        let result = result.unwrap();
+        assert_eq!(result.len(), 2);
+        assert_eq!(
+            result.get("project").unwrap().get("$group_key"),
+            Some(&json!("project_123"))
+        );
+
+        // Test case 3: No groups, only existing overrides
+        let mut existing_overrides = HashMap::new();
+        let mut project_props = HashMap::new();
+        project_props.insert("industry".to_string(), json!("tech"));
+        existing_overrides.insert("project".to_string(), project_props);
+
+        let result = process_group_property_overrides(None, Some(existing_overrides.clone()));
+
+        assert!(result.is_some());
+        assert_eq!(result.unwrap(), existing_overrides);
+
+        // Test case 4: Neither groups nor existing overrides
+        let result = process_group_property_overrides(None, None);
+        assert!(result.is_none());
+    }
 }

rust/feature-flags/src/api/types.rs

@@ -1,4 +1,5 @@
 use serde::{Deserialize, Serialize};
+use serde_json::Value;
 use std::collections::HashMap;
 
 #[derive(Debug, PartialEq, Eq, Deserialize, Serialize)]
@@ -18,4 +19,5 @@ pub enum FlagValue {
 pub struct FlagsResponse {
     pub error_while_computing_flags: bool,
     pub feature_flags: HashMap<String, FlagValue>,
+    pub feature_flag_payloads: HashMap<String, Value>, // flag key -> payload
 }

rust/feature-flags/src/cohort/cohort_operations.rs

@@ -302,8 +302,6 @@ mod tests {
             .find(|c| c.id == main_cohort.id)
             .expect("Failed to find main cohort");
 
-        println!("fetched_main_cohort: {:?}", fetched_main_cohort);
-
         let dependencies = fetched_main_cohort.extract_dependencies().unwrap();
         let expected_dependencies: HashSet<CohortId> =
             [dependent_cohort.id].iter().cloned().collect();