feat(flags): support groups and payloads correctly in new /flags service
#26817
Conversation
…on-tests-for-new-flags
| @@ -46,7 +46,6 @@ pub async fn flags( | |||
| let context = RequestContext { | |||
| state, | |||
| ip, | |||
| meta: meta.0, | |||
There was a problem hiding this comment.
I never use this value in the request logic, so I can omit it.
| @@ -58,7 +58,6 @@ pub struct FlagsQueryParams { | |||
| pub struct RequestContext { | |||
| pub state: State<router::State>, | |||
| pub ip: IpAddr, | |||
| pub meta: FlagsQueryParams, | |||
There was a problem hiding this comment.
see above comment.
| /// ## 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> { |
There was a problem hiding this comment.
this needed a comment since it's an entrypoint method
| pub async fn process_request(context: RequestContext) -> Result<FlagsResponse, FlagError> { | ||
| let RequestContext { | ||
| state, | ||
| ip, | ||
| meta: _, // TODO use this |
There was a problem hiding this comment.
lol I don't actually use this anywhere outside of recording request metadata, so bye bye meta!
| .cloned() | ||
| .unwrap_or_default(); | ||
|
|
||
| properties.insert("$group_key".to_string(), group_key); |
There was a problem hiding this comment.
I'm replicating the work done here: https://github.com/PostHog/posthog/blob/master/posthog/models/feature_flag/flag_matching.py#L1108-L1119
| @@ -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() { | |||
There was a problem hiding this comment.
wanted to make sure I'm correctly overriding the group values with any passed in property overrides.
| feature_flags_map.insert(flag.key.clone(), flag_value); | ||
|
|
||
| if let Some(payload) = flag_match.payload { | ||
| feature_flag_payloads_map.insert(flag.key.clone(), payload); | ||
| } |
There was a problem hiding this comment.
insert the payloads to the response.
|
|
||
| let groups = request.groups.clone(); | ||
| let group_property_overrides = | ||
| process_group_property_overrides(groups.clone(), request.group_properties.clone()); |
There was a problem hiding this comment.
note, this means every time we have a flags request with groups, there will be a group_property_override that looks like (e.g.)
Some({"project": {"$group_key": String("test_company_id")}})
Problem
When I was locally testing the new
/flagsservice, I realized I'd missed 2 things:This PR makes those changes, as well as a few other cleanups around the codebase.