Skip to content

chore(org member invite): changes to organization member to support organization member invite changes #88867

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.

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 6 commits into from
Apr 7, 2025
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
7 changes: 7 additions & 0 deletions src/sentry/api/bases/organizationmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from sentry.db.models.fields.bounded import BoundedAutoField
from sentry.models.organization import Organization
from sentry.models.organizationmember import InviteStatus, OrganizationMember
from sentry.models.organizationmemberinvite import OrganizationMemberInvite
from sentry.organizations.services.organization.model import (
RpcOrganization,
RpcUserOrganizationContext,
Expand Down Expand Up @@ -124,4 +125,10 @@ def _get_member(
if invite_status:
kwargs["invite_status"] = invite_status.value

om_id = kwargs.get("id")
if isinstance(om_id, int):
invite = OrganizationMemberInvite.objects.filter(organization_member_id=om_id).first()
if invite is not None:
raise ResourceDoesNotExist

return OrganizationMember.objects.filter(**kwargs).get()
1 change: 0 additions & 1 deletion src/sentry/api/endpoints/organization_member/details.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,6 @@ def delete(
"""
Remove an organization member.
"""

# with superuser read write separation, superuser read cannot hit this endpoint
# so we can keep this as is_active_superuser
if request.user.is_authenticated and not is_active_superuser(request):
Expand Down
21 changes: 15 additions & 6 deletions src/sentry/api/endpoints/organization_member/index.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from django.conf import settings
from django.db import router, transaction
from django.db.models import F, Q
from django.db.models import Exists, F, OuterRef, Q
from drf_spectacular.utils import extend_schema, extend_schema_serializer
from rest_framework import serializers
from rest_framework.request import Request
Expand Down Expand Up @@ -28,6 +28,7 @@
from sentry.auth.authenticators import available_authenticators
from sentry.integrations.models.external_actor import ExternalActor
from sentry.models.organizationmember import InviteStatus, OrganizationMember
from sentry.models.organizationmemberinvite import OrganizationMemberInvite
from sentry.models.team import Team, TeamStatus
from sentry.roles import organization_roles, team_roles
from sentry.search.utils import tokenize_query
Expand Down Expand Up @@ -183,11 +184,19 @@ def get(self, request: Request, organization) -> Response:

Response includes pending invites that are approved by organization owners or managers but waiting to be accepted by the invitee.
"""
queryset = OrganizationMember.objects.filter(
Q(user_is_active=True, user_id__isnull=False) | Q(user_id__isnull=True),
organization=organization,
invite_status=InviteStatus.APPROVED.value,
).order_by("id")
queryset = (
OrganizationMember.objects.filter(
Q(user_is_active=True, user_id__isnull=False) | Q(user_id__isnull=True),
organization=organization,
invite_status=InviteStatus.APPROVED.value,
)
.filter(
~Exists(
OrganizationMemberInvite.objects.filter(organization_member_id=OuterRef("id"))
)
)
.order_by("id")
)

query = request.GET.get("query")
if query:
Expand Down
5 changes: 5 additions & 0 deletions src/sentry/models/organizationmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
RpcOrganizationMemberMappingUpdate,
organizationmember_mapping_service,
)
from sentry.models.organizationmemberinvite import OrganizationMemberInvite
from sentry.models.team import TeamStatus
from sentry.roles import organization_roles
from sentry.roles.manager import OrganizationRole
Expand Down Expand Up @@ -249,6 +250,10 @@ class Meta:
__repr__ = sane_repr("organization_id", "user_id", "email", "role")

def save(self, *args, **kwargs):
if self.id is not None:
invite = OrganizationMemberInvite.objects.filter(organization_member_id=self.id).first()
assert invite is None, "Cannot save placeholder organization member for an invited user"

with outbox_context(transaction.atomic(using=router.db_for_write(OrganizationMember))):
if self.token and not self.token_expires_at:
self.refresh_expires_at()
Expand Down
21 changes: 21 additions & 0 deletions tests/sentry/api/endpoints/test_organization_member_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,13 @@ def test_does_not_include_secondary_emails(self):
assert "emails" not in response.data["user"]
assert "emails" not in response.data.get("serializedUser", {})

def test_does_not_serialize_placeholder_member(self):
invite = self.create_member_invite(organization=self.organization)
placeholder_om = invite.organization_member

response = self.get_error_response(self.organization.slug, placeholder_om.id)
assert response.data["detail"] == "The requested resource does not exist"


class UpdateOrganizationMemberTest(OrganizationMemberTestBase, HybridCloudTestMixin):
method = "put"
Expand Down Expand Up @@ -895,6 +902,13 @@ def test_on_role_change_not_called_when_reinviting(self, mock_on_role_change):

mock_on_role_change.assert_not_called()

def test_cannot_edit_placeholder_member(self):
invite = self.create_member_invite(organization=self.organization)
placeholder_om = invite.organization_member

response = self.get_error_response(self.organization.slug, placeholder_om.id, role="member")
assert response.data["detail"] == "The requested resource does not exist"


class DeleteOrganizationMemberTest(OrganizationMemberTestBase):
method = "delete"
Expand Down Expand Up @@ -1201,6 +1215,13 @@ def test_invitations_dont_get_deleted_on_invite_detetion(self):
assert members_and_invites_count_after == members_and_invites_count_before - 1
assert not OrganizationMember.objects.filter(inviter_id=manager_user.id).exists()

def test_cannot_delete_placeholder_member(self):
invite = self.create_member_invite(organization=self.organization)
placeholder_om = invite.organization_member

response = self.get_error_response(self.organization.slug, placeholder_om.id)
assert response.data["detail"] == "The requested resource does not exist"


class ResetOrganizationMember2faTest(APITestCase):
def setUp(self):
Expand Down
13 changes: 13 additions & 0 deletions tests/sentry/api/endpoints/test_organization_member_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,19 @@ def test_does_not_include_secondary_emails(self):
assert all("email" not in team for team in member_data.get("teams", []))
assert all("email" not in role for role in member_data.get("teamRoles", []))

def test_does_not_include_placeholder_org_members(self):
# do not list the placeholder org members created for OrganizationMemberInvite objects
user = self.create_user("real@member.com")
self.create_member(organization=self.organization, user=user)
invite = self.create_member_invite(organization=self.organization)
placeholder_member = invite.organization_member

response = self.get_success_response(self.organization.slug)

# check that the placeholder org member is not serialized
member_ids = [m["id"] for m in response.data]
assert str(placeholder_member.id) not in member_ids


class OrganizationMemberPermissionRoleTest(OrganizationMemberListTestBase, HybridCloudTestMixin):
method = "post"
Expand Down
5 changes: 4 additions & 1 deletion tests/sentry/backup/test_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -2241,12 +2241,15 @@ def test_organization_member_inviter_id(self, expected_models: list[type[Model]]
# they will not be imported if we only filter down to `test-org`. The desired outcome is
# that the inviter is nulled out.
for org_member in OrganizationMember.objects.filter(organization=org):
if OrganizationMemberInvite.objects.filter(organization_member=org_member).exists():
# placeholder organization member for invited member, skip
continue
if not org_member.inviter_id:
org_member.inviter_id = admin.id
org_member.save()
assert (
OrganizationMember.objects.filter(organization=org.id, inviter_id__isnull=False).count()
== 6
== 4
)

with tempfile.TemporaryDirectory() as tmp_dir:
Expand Down
10 changes: 10 additions & 0 deletions tests/sentry/models/test_organizationmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -553,3 +553,13 @@ def test_cannot_demote_last_owner(self):

member.role = "manager"
member.save()

def test_cannot_edit_placeholder_org_member(self):
omi = self.create_member_invite()
placeholder_om = omi.organization_member

placeholder_om.role = "manager"
with pytest.raises(
AssertionError, match="Cannot save placeholder organization member for an invited user"
):
placeholder_om.save()
Loading