Skip to content

Commit

Permalink
Fix event type filtering involving feature flags
Browse files Browse the repository at this point in the history
The previous logic was incorrectly filtering out event types with no
feature flag set.
  • Loading branch information
svix-andor committed Jan 20, 2023
1 parent 862fd19 commit 72dcbf9
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 44 deletions.
10 changes: 8 additions & 2 deletions server/svix-server/src/v1/endpoints/event_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,10 @@ async fn list_event_types(
}

if let permissions::AllowedFeatureFlags::Some(flags) = feature_flags {
query = query.filter(eventtype::Column::FeatureFlag.is_in(flags.into_iter()));
let condition = sea_orm::Condition::any()
.add(eventtype::Column::FeatureFlag.is_in(flags.into_iter()))
.add(eventtype::Column::FeatureFlag.is_null());
query = query.filter(condition);
}

Ok(Json(EventTypeOut::list_response_no_prev(
Expand Down Expand Up @@ -280,7 +283,10 @@ async fn get_event_type(
) -> Result<Json<EventTypeOut>> {
let mut query = eventtype::Entity::secure_find_by_name(org_id, evtype_name);
if let permissions::AllowedFeatureFlags::Some(flags) = feature_flags {
query = query.filter(eventtype::Column::FeatureFlag.is_in(flags.into_iter()));
let condition = sea_orm::Condition::any()
.add(eventtype::Column::FeatureFlag.is_in(flags.into_iter()))
.add(eventtype::Column::FeatureFlag.is_null());
query = query.filter(condition);
}
let evtype = ctx!(query.one(db).await)?.ok_or_else(|| HttpError::not_found(None, None))?;

Expand Down
87 changes: 45 additions & 42 deletions server/svix-server/tests/e2e_event_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ async fn test_event_type_feature_flags() {
.post(
"api/v1/event-type/",
EventTypeIn {
name: EventTypeName("foo-event".to_owned()),
name: EventTypeName("event-type-with-flag".to_owned()),
description: "test-event-description".to_owned(),
deleted: false,
schemas: None,
Expand All @@ -228,6 +228,21 @@ async fn test_event_type_feature_flags() {
.await
.unwrap();

let _: EventTypeOut = client
.post(
"api/v1/event-type/",
EventTypeIn {
name: EventTypeName("no-flag-event".to_owned()),
description: "test-event-description".to_owned(),
deleted: false,
schemas: None,
feature_flag: None,
},
StatusCode::CREATED,
)
.await
.unwrap();

let app: ApplicationId = client
.post::<_, ApplicationOut>(
"api/v1/app/",
Expand All @@ -238,50 +253,38 @@ async fn test_event_type_feature_flags() {
.unwrap()
.id;

// Client with no feature flags set
let client1 = app_portal_access(&client, &app, FeatureFlagSet::default()).await;
// Client with a different set of feature flags than needed
let client2 = app_portal_access(&client, &app, other_features).await;
// Client with the right flag, plus extras
let client3 = app_portal_access(&client, &app, union.clone()).await;
// Client with only the right flag
let client4 = app_portal_access(&client, &app, features.clone()).await;

// Clients which don't have the right flag shouldn't see it
let list: ListResponse<EventTypeOut> = client1
.get("api/v1/event-type/", StatusCode::OK)
.await
.unwrap();
assert_eq!(list.data.len(), 0);
let path = format!("api/v1/event-type/{}/", et.name);
let _: IgnoredResponse = client1.get(&path, StatusCode::NOT_FOUND).await.unwrap();

let list: ListResponse<EventTypeOut> = client2
.get("api/v1/event-type/", StatusCode::OK)
.await
.unwrap();
assert_eq!(list.data.len(), 0);
let path = format!("api/v1/event-type/{}/", et.name);
let _: IgnoredResponse = client2.get(&path, StatusCode::NOT_FOUND).await.unwrap();

// Clients with the right flags set should see the event type
let list: ListResponse<EventTypeOut> = client3
.get("api/v1/event-type/", StatusCode::OK)
.await
.unwrap();
assert_eq!(list.data.len(), 1);
assert!(list.data.contains(&et));
let got_et: EventTypeOut = client3.get(&path, StatusCode::OK).await.unwrap();
assert_eq!(et, got_et);
for (flag_set, should_see) in [
(FeatureFlagSet::default(), false),
(other_features, false),
(union.clone(), true),
(features.clone(), true),
] {
let client = app_portal_access(&client, &app, flag_set).await;

let list: ListResponse<EventTypeOut> = client4
.get("api/v1/event-type/", StatusCode::OK)
.await
.unwrap();
assert_eq!(list.data.len(), 1);
assert!(list.data.contains(&et));
let got_et: EventTypeOut = client3.get(&path, StatusCode::OK).await.unwrap();
assert_eq!(et, got_et);
let list: ListResponse<EventTypeOut> = client
.get("api/v1/event-type/", StatusCode::OK)
.await
.unwrap();

if should_see {
// If the client is expected to see both event types it should be able to retrieve it
let got_et: EventTypeOut = client.get(&path, StatusCode::OK).await.unwrap();
assert_eq!(et, got_et);

// ... and see it in the list.
assert_eq!(list.data.len(), 2);
assert!(list.data.contains(&et));
} else {
// If the client is not supposed to see it it shouldn't be able to rertieve it
let _: IgnoredResponse = client.get(&path, StatusCode::NOT_FOUND).await.unwrap();

// ... and it shouldn't be in the list.
assert_eq!(list.data.len(), 1);
assert!(!list.data.contains(&et));
};
}
}

#[tokio::test]
Expand Down

0 comments on commit 72dcbf9

Please sign in to comment.