Skip to content

Commit

Permalink
Revert "feat(api): Team creation endpoint changes w/ fixes (#48724)" (#…
Browse files Browse the repository at this point in the history 
…48953)

Reverts #48724 in favor of creating a new endpoint as detailed in the [`Project Creation for All Tech Specs`](https://www.notion.so/sentry/24Q2-Project-Creation-for-All-5ed58b658f28464b8582252836de5394)
  • Loading branch information
@schew2381
schew2381 committed May 11, 2023
1 parent 821f0a2 commit 6d9101b
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 229 deletions.
130 changes: 49 additions & 81 deletions src/sentry/api/endpoints/organization_teams.py
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
from django.db import IntegrityError, transaction
from django.db.models import Q
from django.utils.translation import ugettext_lazy as _
from rest_framework import serializers
from rest_framework.exceptions import PermissionDenied
from rest_framework import serializers, status
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.serializers import ValidationError

from sentry import audit_log, features
from sentry import audit_log
from sentry.api.base import region_silo_endpoint
from sentry.api.bases.organization import OrganizationEndpoint, OrganizationPermission
from sentry.api.exceptions import ConflictError, ResourceDoesNotExist
from sentry.api.paginator import OffsetPaginator
from sentry.api.serializers import serialize
from sentry.api.serializers.models.team import TeamSerializer
Expand All @@ -23,16 +20,17 @@
)
from sentry.search.utils import tokenize_query
from sentry.signals import team_created
from sentry.utils.snowflake import MaxSnowflakeRetryError

CONFLICTING_SLUG_ERROR = "A team with this slug already exists."


# OrganizationPermission + team:write
class OrganizationTeamsPermission(OrganizationPermission):
scope_map = {
"GET": ["org:read", "org:write"],
"POST": ["org:write", "team:write"],
"GET": ["org:read", "org:write", "org:admin"],
"POST": ["org:write", "org:admin", "team:write"],
"PUT": ["org:write", "org:admin", "team:write"],
"DELETE": ["org:admin", "team:write"],
}


Expand All @@ -51,11 +49,10 @@ class TeamPostSerializer(serializers.Serializer):
},
)
idp_provisioned = serializers.BooleanField(required=False, default=False)
set_team_admin = serializers.BooleanField(required=False, default=False)

def validate(self, attrs):
if not (attrs.get("name") or attrs.get("slug")):
raise ValidationError("Name or slug is required")
raise serializers.ValidationError("Name or slug is required")
return attrs


Expand Down Expand Up @@ -145,85 +142,56 @@ def post(self, request: Request, organization, **kwargs) -> Response:
:pparam string organization_slug: the slug of the organization the
team should be created for.
:qparam string name: the optional name of the team.
:qparam string slug: the optional slug for this team. If not provided it will be auto
generated from the name.
:qparam bool set_team_admin: If this is true, the user is added to the as a Team Admin
instead of regular member
:param string name: the optional name of the team.
:param string slug: the optional slug for this team. If
not provided it will be auto generated from the
name.
:auth: required
"""
serializer = TeamPostSerializer(data=request.data)

if not serializer.is_valid():
raise ValidationError(serializer.errors)
if serializer.is_valid():
result = serializer.validated_data

result = serializer.validated_data
set_team_admin = result.get("set_team_admin")

if set_team_admin:
if not features.has("organizations:team-roles", organization) or not features.has(
"organizations:team-project-creation-all", organization
):
raise ResourceDoesNotExist(
detail="You do not have permission to join a new team as a team admin"
)
if not self.should_add_creator_to_team(request):
raise ValidationError(
{"detail": "You do not have permission to join a new team as a Team Admin"},
)
try:
with transaction.atomic():
team = Team.objects.create(
name=result.get("name") or result["slug"],
slug=result.get("slug"),
idp_provisioned=result.get("idp_provisioned", False),
organization=organization,
)
except (IntegrityError, MaxSnowflakeRetryError):
raise ConflictError(
{
"non_field_errors": [CONFLICTING_SLUG_ERROR],
"detail": CONFLICTING_SLUG_ERROR,
}
)
else:
team_created.send_robust(
organization=organization,
user=request.user,
team=team,
sender=self.__class__,
)

if self.should_add_creator_to_team(request):
cleanup_needed = True
try:
with transaction.atomic():
team = Team.objects.create(
name=result.get("name") or result["slug"],
slug=result.get("slug"),
idp_provisioned=result.get("idp_provisioned", False),
organization=organization,
)
except IntegrityError:
return Response(
{
"non_field_errors": [CONFLICTING_SLUG_ERROR],
"detail": CONFLICTING_SLUG_ERROR,
},
status=409,
)
else:
team_created.send_robust(
organization=organization, user=request.user, team=team, sender=self.__class__
)
if self.should_add_creator_to_team(request):
try:
member = OrganizationMember.objects.get(
user_id=request.user.id, organization=organization
)
OrganizationMemberTeam.objects.create(
team=team,
organizationmember=member,
role="admin" if set_team_admin else None,
)
cleanup_needed = False
except OrganizationMember.DoesNotExist:
if set_team_admin:
raise PermissionDenied(
detail="You must be a member of the organization to join a new team as a Team Admin"
)
finally:
if set_team_admin and cleanup_needed:
team.delete()
except OrganizationMember.DoesNotExist:
pass
else:
OrganizationMemberTeam.objects.create(team=team, organizationmember=member)

self.create_audit_entry(
request=request,
organization=organization,
target_object=team.id,
event=audit_log.get_event_id("TEAM_ADD"),
data=team.get_audit_log_data(),
)
return Response(
serialize(team, request.user, self.team_serializer_for_post()),
status=201,
)
self.create_audit_entry(
request=request,
organization=organization,
target_object=team.id,
event=audit_log.get_event_id("TEAM_ADD"),
data=team.get_audit_log_data(),
)
return Response(
serialize(team, request.user, self.team_serializer_for_post()),
status=201,
)
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
2 changes: 0 additions & 2 deletions src/sentry/conf/server.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1484,8 +1484,6 @@ def SOCIAL_AUTH_DEFAULT_USERNAME():
"organizations:team-insights": True,
# Enable u2f verification on superuser form
"organizations:u2f-superuser-form": False,
# Enable project creation for all
"organizations:team-project-creation-all": False,
# Enable setting team-level roles and receiving permissions from them
"organizations:team-roles": False,
# Enable team member role provisioning through scim
Expand Down
1 change: 0 additions & 1 deletion src/sentry/features/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@
default_manager.add("organizations:starfish-view", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
default_manager.add("organizations:streamline-targeting-context", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
default_manager.add("organizations:symbol-sources", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add('organizations:team-project-creation-all', OrganizationFeature, FeatureHandlerStrategy.REMOTE)
default_manager.add("organizations:team-roles", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("organizations:transaction-name-normalize", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("organizations:transaction-name-mark-scrubbed-as-sanitized", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
Expand Down
146 changes: 1 addition & 145 deletions tests/sentry/api/endpoints/test_organization_teams.py
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
from functools import cached_property
from unittest.mock import patch

import pytest
from django.urls import reverse

from sentry.api.endpoints.organization_teams import OrganizationTeamsEndpoint
from sentry.models import OrganizationMember, OrganizationMemberTeam, ProjectTeam, Team
from sentry.testutils import APITestCase
from sentry.testutils.helpers.features import with_feature
from sentry.testutils.silo import region_silo_test
from sentry.types.integrations import get_provider_string

Expand Down Expand Up @@ -233,151 +229,11 @@ def test_duplicate(self):
self.get_success_response(
self.organization.slug, name="hello world", slug="foobar", status_code=201
)
response = self.get_error_response(
self.get_error_response(
self.organization.slug, name="hello world", slug="foobar", status_code=409
)
assert response.data == {
"non_field_errors": ["A team with this slug already exists."],
"detail": "A team with this slug already exists.",
}

def test_name_too_long(self):
self.get_error_response(
self.organization.slug, name="x" * 65, slug="xxxxxxx", status_code=400
)

def test_org_member_does_not_exist_passes(self):
prior_team_count = Team.objects.count()

# Multiple calls are made to OrganizationMember.objects.get, so in order to only raise
# OrganizationMember.DoesNotExist for the correct call, we set a reference to the actual
# function then call the reference unless the organization matches the test case
get_reference = OrganizationMember.objects.get

def get_callthrough(*args, **kwargs):
if self.organization in kwargs.values():
raise OrganizationMember.DoesNotExist
return get_reference(*args, **kwargs)

with patch.object(OrganizationMember.objects, "get", side_effect=get_callthrough):
resp = self.get_success_response(
self.organization.slug, name="hello world", slug="foobar", status_code=201
)
team = Team.objects.get(id=resp.data["id"])
assert team.name == "hello world"
assert team.slug == "foobar"
assert not team.idp_provisioned
assert team.organization == self.organization

member = OrganizationMember.objects.get(user=self.user, organization=self.organization)

assert not OrganizationMemberTeam.objects.filter(
organizationmember=member, team=team, is_active=True
).exists()
assert Team.objects.count() == prior_team_count + 1

@with_feature(["organizations:team-roles", "organizations:team-project-creation-all"])
def test_valid_team_admin(self):
prior_team_count = Team.objects.count()
resp = self.get_success_response(
self.organization.slug,
name="hello world",
slug="foobar",
set_team_admin=True,
status_code=201,
)

team = Team.objects.get(id=resp.data["id"])
assert team.name == "hello world"
assert team.slug == "foobar"
assert not team.idp_provisioned
assert team.organization == self.organization

member = OrganizationMember.objects.get(user=self.user, organization=self.organization)

assert OrganizationMemberTeam.objects.filter(
organizationmember=member, team=team, is_active=True, role="admin"
).exists()
assert Team.objects.count() == prior_team_count + 1

def test_team_admin_missing_team_roles_flag(self):
response = self.get_error_response(
self.organization.slug,
name="hello world",
slug="foobar",
set_team_admin=True,
status_code=404,
)
assert response.data == {
"detail": "You do not have permission to join a new team as a team admin"
}

@with_feature("organizations:team-roles")
def test_team_admin_missing_project_creation_all_flag(self):
response = self.get_error_response(
self.organization.slug,
name="hello world",
slug="foobar",
set_team_admin=True,
status_code=404,
)
assert response.data == {
"detail": "You do not have permission to join a new team as a team admin"
}

@with_feature(["organizations:team-roles", "organizations:team-project-creation-all"])
@patch.object(OrganizationTeamsEndpoint, "should_add_creator_to_team", return_value=False)
def test_team_admin_not_authenticated(self, mock_creator_check):
response = self.get_error_response(
self.organization.slug,
name="hello world",
slug="foobar",
set_team_admin=True,
status_code=400,
)
assert response.data == {
"detail": "You do not have permission to join a new team as a Team Admin"
}
mock_creator_check.assert_called_once()

@with_feature(["organizations:team-roles", "organizations:team-project-creation-all"])
def test_team_admin_member_does_not_exist(self):
prior_team_count = Team.objects.count()

# Multiple calls are made to OrganizationMember.objects.get, so in order to only raise
# OrganizationMember.DoesNotExist for the correct call, we set a reference to the actual
# function then call the reference unless the organization matches the test case
get_reference = OrganizationMember.objects.get

def get_callthrough(*args, **kwargs):
if self.organization in kwargs.values():
raise OrganizationMember.DoesNotExist
return get_reference(*args, **kwargs)

with patch.object(OrganizationMember.objects, "get", side_effect=get_callthrough):
response = self.get_error_response(
self.organization.slug,
name="hello world",
slug="foobar",
set_team_admin=True,
status_code=403,
)
assert response.data == {
"detail": "You must be a member of the organization to join a new team as a Team Admin",
}
assert Team.objects.count() == prior_team_count

@with_feature(["organizations:team-roles", "organizations:team-project-creation-all"])
@patch.object(OrganizationMemberTeam.objects, "create", side_effect=Exception("test"))
def test_team_admin_org_member_team_create_generically_fails(self, mock_create):
prior_team_count = Team.objects.count()
with pytest.raises(Exception):
self.get_error_response(
self.organization.slug,
name="hello world",
slug="foobar",
set_team_admin=True,
status_code=400,
)
mock_create.assert_called_once()
assert Team.objects.count() == prior_team_count

0 comments on commit 6d9101b

Please sign in to comment.