Skip to content

ref(api): Refactor Org Serializers to Selectively Include feature flags #71791

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 3, 2024
Merged

ref(api): Refactor Org Serializers to Selectively Include feature flags #71791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77ad87f
refactored endpoint seralizers
iamrajjoshi May 30, 2024
cfec503
fix
iamrajjoshi May 30, 2024
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
15 changes: 8 additions & 7 deletions src/sentry/api/endpoints/organization_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,10 +605,13 @@ def get(self, request: Request, organization) -> Response:
if is_detailed:
serializer = org_serializers.DetailedOrganizationSerializerWithProjectsAndTeams

context = serialize(organization, request.user, serializer(), access=request.access)

if not include_feature_flags:
context.pop("features", None)
context = serialize(
organization,
request.user,
serializer(),
access=request.access,
include_feature_flags=include_feature_flags,
)

return self.respond(context)

Expand Down Expand Up @@ -699,11 +702,9 @@ def put(self, request: Request, organization) -> Response:
request.user,
org_serializers.DetailedOrganizationSerializerWithProjectsAndTeams(),
access=request.access,
include_feature_flags=include_feature_flags,
)

if not include_feature_flags:
context.pop("features", None)

return self.respond(context)
return self.respond(serializer.errors, status=status.HTTP_400_BAD_REQUEST)

Expand Down
76 changes: 45 additions & 31 deletions src/sentry/api/serializers/models/organization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,30 +239,19 @@ def _serialize_auth_providers(
for o in item_list
}

def serialize(
def get_feature_set(
self, obj: Organization, attrs: Mapping[str, Any], user: User, **kwargs: Any
) -> OrganizationSerializerResponse:
) -> set[str]:
from sentry import features
from sentry.features.base import OrganizationFeature

if attrs.get("avatar"):
avatar = {
"avatarType": attrs["avatar"].get_avatar_type_display(),
"avatarUuid": attrs["avatar"].ident if attrs["avatar"].file_id else None,
"avatarUrl": attrs["avatar"].absolute_url(),
}
else:
avatar = {"avatarType": "letter_avatar", "avatarUuid": None, "avatarUrl": None}

status = OrganizationStatus(obj.status)

# Retrieve all registered organization features
org_features = [
feature
for feature in features.all(feature_type=OrganizationFeature).keys()
if feature.startswith(_ORGANIZATION_SCOPE_PREFIX)
]
feature_list = set()
feature_set = set()

with sentry_sdk.start_span(op="features.check", description="check batch features"):
# Check features in batch using the entity handler
Expand All @@ -275,7 +264,7 @@ def serialize(
).items():
if active:
# Remove organization prefix
feature_list.add(feature_name[len(_ORGANIZATION_SCOPE_PREFIX) :])
feature_set.add(feature_name[len(_ORGANIZATION_SCOPE_PREFIX) :])

# This feature_name was found via `batch_has`, don't check again using `has`
org_features.remove(feature_name)
Expand All @@ -285,18 +274,18 @@ def serialize(
for feature_name in org_features:
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) :])
feature_set.add(feature_name[len(_ORGANIZATION_SCOPE_PREFIX) :])

# Do not include the onboarding feature if OrganizationOptions exist
if (
"onboarding" in feature_list
"onboarding" in feature_set
and OrganizationOption.objects.filter(organization=obj).exists()
):
feature_list.remove("onboarding")
feature_set.remove("onboarding")

# Include api-keys feature if they previously had any api-keys
if "api-keys" not in feature_list and attrs["has_api_key"]:
feature_list.add("api-keys")
if "api-keys" not in feature_set and attrs["has_api_key"]:
feature_set.add("api-keys")

# Organization flag features (not provided through the features module)
options_as_features = OrganizationOption.objects.filter(
Expand All @@ -305,19 +294,39 @@ def serialize(
for option in options_as_features:
for option_feature, option_function in ORGANIZATION_OPTIONS_AS_FEATURES[option.key]:
if option_function(option):
feature_list.add(option_feature)
feature_set.add(option_feature)

if getattr(obj.flags, "allow_joinleave"):
feature_list.add("open-membership")
feature_set.add("open-membership")
if not getattr(obj.flags, "disable_shared_issues"):
feature_list.add("shared-issues")
feature_set.add("shared-issues")
request = env.request
if request and is_using_customer_domain(request):
# If the current request is using a customer domain, then we activate the feature for this organization.
feature_list.add("customer-domains")
feature_set.add("customer-domains")

if "dynamic-sampling" not in feature_list and "mep-rollout-flag" in feature_list:
feature_list.remove("mep-rollout-flag")
if "dynamic-sampling" not in feature_set and "mep-rollout-flag" in feature_set:
feature_set.remove("mep-rollout-flag")

return feature_set

def serialize(
self, obj: Organization, attrs: Mapping[str, Any], user: User, **kwargs: Any
) -> OrganizationSerializerResponse:
from sentry import features

if attrs.get("avatar"):
avatar = {
"avatarType": attrs["avatar"].get_avatar_type_display(),
"avatarUuid": attrs["avatar"].ident if attrs["avatar"].file_id else None,
"avatarUrl": attrs["avatar"].absolute_url(),
}
else:
avatar = {"avatarType": "letter_avatar", "avatarUuid": None, "avatarUrl": None}

status = OrganizationStatus(obj.status)

include_feature_flags = kwargs.get("include_feature_flags", True)

has_auth_provider = attrs.get("auth_provider", None) is not None

Expand All @@ -334,14 +343,16 @@ def serialize(
and obj.flags.require_email_verification
),
"avatar": avatar,
"features": feature_list,
"links": {
"organizationUrl": generate_organization_url(obj.slug),
"regionUrl": generate_region_url(),
},
"hasAuthProvider": has_auth_provider,
}

if include_feature_flags:
context["features"] = self.get_feature_set(obj, attrs, user, **kwargs)

if "access" in kwargs:
context["access"] = kwargs["access"].scopes
tasks_to_serialize = list(onboarding_tasks.fetch_onboarding_tasks(obj, user))
Expand Down Expand Up @@ -446,17 +457,20 @@ def get_attrs(
return super().get_attrs(item_list, user)

def serialize( # type: ignore[explicit-override, override]
self, obj: Organization, attrs: Mapping[str, Any], user: User, access: Access
self, obj: Organization, attrs: Mapping[str, Any], user: User, access: Access, **kwargs: Any
) -> DetailedOrganizationSerializerResponse:
# TODO: rectify access argument overriding parent if we want to remove above type ignore

from sentry import experiments

experiment_assignments = experiments.all(org=obj, actor=user)
include_feature_flags = kwargs.get("include_feature_flags", True)
Copy link
Collaborator Author

@iamrajjoshi iamrajjoshi May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should include the flag if the kwarg isn't passed in.


context = cast(
DetailedOrganizationSerializerResponse,
super().serialize(obj, attrs, user, access=access),
super().serialize(
obj, attrs, user, access=access, include_feature_flags=include_feature_flags
),
)
max_rate = quotas.get_maximum_quota(obj)
context["experiments"] = experiment_assignments
Expand Down Expand Up @@ -647,7 +661,7 @@ def _team_list(self, organization: Organization, access: Access) -> list[Team]:
return team_list

def serialize( # type: ignore[explicit-override, override]
self, obj: Organization, attrs: Mapping[str, Any], user: User, access: Access
self, obj: Organization, attrs: Mapping[str, Any], user: User, access: Access, **kwargs: Any
) -> DetailedOrganizationSerializerWithProjectsAndTeamsResponse:
from sentry.api.serializers.models.project import (
LATEST_DEPLOYS_KEY,
Expand All @@ -657,7 +671,7 @@ def serialize( # type: ignore[explicit-override, override]

context = cast(
DetailedOrganizationSerializerWithProjectsAndTeamsResponse,
super().serialize(obj, attrs, user, access),
super().serialize(obj, attrs, user, access, **kwargs),
)

team_list = self._team_list(obj, access)
Expand Down