Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/sentry/api/serializers/models/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ def serialize(self, obj, attrs, user):
]
feature_list = set()

# Check features in batch using the entity handler
batch_features = features.batch_has(org_features, actor=user, organization=obj)

# batch_has has found some features
Expand All @@ -135,8 +136,9 @@ def serialize(self, obj, attrs, user):
# This feature_name was found via `batch_has`, don't check again using `has`
org_features.remove(feature_name)

# Remaining features should not be checked via the entity handler
for feature_name in org_features:
if features.has(feature_name, obj, actor=user):
if features.has(feature_name, obj, actor=user, skip_entity=True):
# Remove the organization scope prefix
feature_list.add(feature_name[len(_ORGANIZATION_SCOPE_PREFIX) :])

Expand Down
4 changes: 2 additions & 2 deletions src/sentry/features/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def __call__(self, feature: "Feature", actor: "User") -> Optional[bool]:
return self.has(feature, actor)

@abc.abstractmethod
def has(self, feature: "Feature", actor: "User") -> bool:
def has(self, feature: "Feature", actor: "User", skip_entity: Optional[bool] = False) -> bool:
raise NotImplementedError

def has_for_batch(self, batch: "FeatureCheckBatch") -> Mapping["Project", bool]:
Expand Down Expand Up @@ -54,7 +54,7 @@ def _check_for_batch(
) -> bool:
raise NotImplementedError

def has(self, feature: "Feature", actor: "User") -> bool:
def has(self, feature: "Feature", actor: "User", skip_entity: Optional[bool] = False) -> bool:
return self._check_for_batch(feature.name, feature.get_organization(), actor)

def has_for_batch(self, batch: "FeatureCheckBatch") -> Mapping["Project", bool]:
Expand Down
6 changes: 4 additions & 2 deletions src/sentry/features/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,9 @@ def add_entity_handler(self, handler: "FeatureHandler") -> None:
"""
self._entity_handler = handler

def has(self, name: str, *args: Any, **kwargs: Any) -> bool:
def has(
self, name: str, *args: Any, skip_entity: Optional[bool] = False, **kwargs: Any
) -> bool:
"""
Determine if a feature is enabled. If a handler returns None, then the next
mechanism is used for feature checking.
Expand Down Expand Up @@ -213,7 +215,7 @@ def has(self, name: str, *args: Any, **kwargs: Any) -> bool:
if rv is not None:
return rv

if self._entity_handler:
if self._entity_handler and not skip_entity:
rv = self._entity_handler.has(feature, actor)
if rv is not None:
return rv
Expand Down
10 changes: 9 additions & 1 deletion tests/sentry/features/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ class MockBatchHandler(features.BatchFeatureHandler):
features = frozenset(["auth:register", "organizations:feature", "projects:feature"])

def has(
self, feature: Feature, actor: User
self,
feature: Feature,
actor: User,
skip_entity: Optional[bool] = False,
) -> Union[Optional[bool], Mapping[str, Optional[bool]]]:
return {feature.name: True}

Expand Down Expand Up @@ -117,6 +120,11 @@ def test_entity_handler(self):
assert len(entity_handler.has.mock_calls) == 1
assert len(registered_handler.mock_calls) == 1

# The feature isn't registered, but lets skip the entity_handler
manager.has("organizations:unregistered-feature", test_org, skip_entity=True)
assert len(entity_handler.has.mock_calls) == 1
assert len(registered_handler.mock_calls) == 1

# The entity_handler doesn't have a response for this feature either, so settings should be checked instead
entity_handler.has.return_value = None
settings.SENTRY_FEATURES["organizations:settings-feature"] = "test"
Expand Down