Browse Source

fix(features): Fix only first project getting flags without entity handler (#69679)

Resolves https://github.com/getsentry/sentry/issues/57162

Resolves project flags only being provided to the first project when
serializing project flags. This was because of the use of `filter()`
which upon a second iteration, returns empty.


**New test that fails on master**

<img width="702" alt="image"
src="https://github.com/getsentry/sentry/assets/35509934/c5a06be3-fe96-46bc-9b92-89f47808521c">

**Flag test on discard feature**

<img width="801" alt="image"
src="https://github.com/getsentry/sentry/assets/35509934/cc431adb-2cd5-42e8-b521-4e15f599ec7b">
Leander Rodrigues 10 months ago
parent
commit
b80bd029cd
2 changed files with 13 additions and 2 deletions
  1. 2 2
      src/sentry/features/manager.py
  2. 11 0
      tests/sentry/features/test_manager.py

+ 2 - 2
src/sentry/features/manager.py

@@ -110,7 +110,7 @@ class RegisteredFeatureManager:
 
                 batch = FeatureCheckBatch(self, name, organization, remaining, actor)
                 handler_result = handler.has_for_batch(batch)
-                for (obj, flag) in handler_result.items():
+                for obj, flag in handler_result.items():
                     if flag is not None:
                         remaining.remove(obj)
                         result[obj] = flag
@@ -286,7 +286,7 @@ class FeatureManager(RegisteredFeatureManager):
             )
         else:
             # Fall back to default handler if no entity handler available.
-            project_features = filter(lambda name: name.startswith("projects:"), feature_names)
+            project_features = [name for name in feature_names if name.startswith("projects:")]
             if projects and project_features:
                 results: MutableMapping[str, Mapping[str, bool]] = {}
                 for project in projects:

+ 11 - 0
tests/sentry/features/test_manager.py

@@ -254,6 +254,17 @@ class FeatureManagerTest(TestCase):
         assert ret is not None
         assert ret[f"project:{self.project.id}"]["projects:feature"]
 
+    def test_batch_has_no_entity_multiple_projects(self):
+        manager = features.FeatureManager()
+        manager.add("projects:feature", ProjectFeature)
+        manager.add_handler(MockBatchHandler())
+        projects = [self.project, self.create_project()]
+
+        result = manager.batch_has(["projects:feature"], actor=self.user, projects=projects)
+        assert result is not None
+        for project in projects:
+            assert result[f"project:{project.id}"]["projects:feature"]
+
     def test_has(self):
         manager = features.FeatureManager()
         manager.add("auth:register")