Fix event type filtering involving feature flags

server/svix-server/src/db/models/eventtype.rs

@@ -4,7 +4,9 @@
 use std::collections::HashMap;
 
 use crate::{
-    core::types::{BaseId, EventTypeId, EventTypeName, FeatureFlag, OrganizationId},
+    core::types::{
+        BaseId, EventTypeId, EventTypeName, FeatureFlag, FeatureFlagSet, OrganizationId,
+    },
     json_wrapper,
 };
 use chrono::Utc;
@@ -64,6 +66,14 @@ impl Entity {
     pub fn secure_find_by_name(org_id: OrganizationId, name: EventTypeName) -> Select<Entity> {
         Self::secure_find(org_id).filter(Column::Name.eq(name))
     }
+
+    pub fn filter_feature_flags(query: Select<Self>, flags: FeatureFlagSet) -> Select<Self> {
+        query.filter(
+            sea_orm::Condition::any()
+                .add(Column::FeatureFlag.is_in(flags.into_iter()))
+                .add(Column::FeatureFlag.is_null()),
+        )
+    }
 }
 
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Default, JsonSchema)]

server/svix-server/src/v1/endpoints/event_type.rs

@@ -215,7 +215,7 @@ async fn list_event_types(
     }
 
     if let permissions::AllowedFeatureFlags::Some(flags) = feature_flags {
-        query = query.filter(eventtype::Column::FeatureFlag.is_in(flags.into_iter()));
+        query = eventtype::Entity::filter_feature_flags(query, flags);
     }
 
     Ok(Json(EventTypeOut::list_response_no_prev(
@@ -280,7 +280,7 @@ 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()));
+        query = eventtype::Entity::filter_feature_flags(query, flags);
     }
     let evtype = ctx!(query.one(db).await)?.ok_or_else(|| HttpError::not_found(None, None))?;
 

server/svix-server/tests/e2e_event_type.rs

@@ -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,
@@ -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/",
@@ -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 retrieve 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]