feat(org member invite): OrganizationMemberInviteDetails DELETE endpoint #88587

mifu67 · 2025-04-02T18:43:32Z

Delete an OrganizationMemberInvite object from the database. This is used to delete invites and reject invite requests.

mifu67 · 2025-04-02T18:44:49Z

src/sentry/api/endpoints/organization_member_invite/details.py

+        audit_data = invited_member.get_audit_log_data()
+        event_name = "INVITE_REMOVE" if invited_member.invite_approved else "INVITE_REQUEST_REMOVE"
+        with transaction.atomic(router.db_for_write(OrganizationMemberInvite)):
+            invited_member.delete()


I think I unfortunately need to move this to a method on the model to support rejecting an invite request via slack ;_;

codecov · 2025-04-02T19:15:02Z

Codecov Report

Attention: Patch coverage is 96.80000% with 4 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...pi/endpoints/organization_member_invite/details.py	90.47%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #88587      +/-   ##
==========================================
+ Coverage   81.79%   82.93%   +1.13%     
==========================================
  Files       10204    10206       +2     
  Lines      575357   575882     +525     
  Branches    22662    22662              
==========================================
+ Hits       470626   477619    +6993     
+ Misses     103802    97334    -6468     
  Partials      929      929

src/sentry/api/endpoints/organization_member_invite/details.py

cathteng · 2025-04-14T21:44:23Z

src/sentry/api/endpoints/organization_member_invite/details.py

+                if not request.access.has_scope("member:admin"):
+                    if not organization.flags.disable_member_invite and request.access.has_scope(
+                        "member:invite"
+                    ):


can we pull these out into their own variables? then checking them is a bit easier to grok

src/sentry/api/endpoints/organization_member_invite/details.py

leedongwei

Code quality issues

leedongwei · 2025-04-29T00:37:41Z

src/sentry/api/endpoints/organization_member_invite/details.py

+        if request.user.is_authenticated and not is_active_superuser(request):
+            try:
+                acting_member = OrganizationMember.objects.get(
+                    organization=organization, user_id=request.user.id
+                )
+            except OrganizationMember.DoesNotExist:
+                return Response({"detail": ERR_INSUFFICIENT_ROLE}, status=400)
+
+            has_member_admin_scope = request.access.has_scope("member:admin")
+            if not has_member_admin_scope:
+                can_invite_members = request.access.has_scope("member:invite")
+                if can_invite_members:
+                    return self._handle_deletion_by_member(
+                        request, organization, invited_member, acting_member
+                    )
+                return Response({"detail": ERR_INSUFFICIENT_SCOPE}, status=400)
+            else:
+                can_manage = roles.can_manage(acting_member.role, invited_member.role)


I think you can move this further down the method, and do all the low-hanging fruit checks and return early from there. It will make this section a lot less nested.

leedongwei · 2025-04-29T00:38:26Z

src/sentry/api/endpoints/organization_member_invite/details.py

+            has_member_admin_scope = request.access.has_scope("member:admin")
+            if not has_member_admin_scope:
+                can_invite_members = request.access.has_scope("member:invite")
+                if can_invite_members:
+                    return self._handle_deletion_by_member(
+                        request, organization, invited_member, acting_member
+                    )


Is this more readable?

has_member_admin_scope = request.access.has_scope("member:admin") can_invite_members = request.access.has_scope("member:invite") if not has_member_admin_scope or can_invite_members return self._handle_deletion_by_member( request, organization, invited_member, acting_member )

This isn't the same logic, because users with org:admin have member:invite. We would enter the if statement erroneously.

I'll make it a little cleaner.

cathteng · 2025-04-22T23:13:53Z

tests/sentry/api/endpoints/test_organization_member_invite_details.py

+        """
+        Members can remove invites that they sent
+        """
+        self.login_as(self.regular_user)


how is this working if the user is not a member of the org?

i think you might need to update MemberInvitePermission's scope_map for DELETE

cathteng · 2025-04-22T23:14:42Z

src/sentry/api/endpoints/organization_member_invite/details.py

+                    return self._handle_deletion_by_member(
+                        request, organization, invited_member, acting_member
+                    )
+                return Response({"detail": ERR_INSUFFICIENT_SCOPE}, status=400)


should these be responding with 403 if you're not allowed to perform this action?

cathteng · 2025-04-29T22:50:26Z

src/sentry/api/endpoints/organization_member_invite/details.py

+        # prevents superuser read from deleting an invite or invite request
+        elif not request.access.has_scope("member:admin"):
+            return Response({"detail": ERR_INSUFFICIENT_SCOPE}, status=400)


rather than preventing superuser read (since they can't hit the endpoint anyway), i think this is preventing unauthenticated users from hitting this endpoint

cathteng · 2025-04-29T22:55:25Z

src/sentry/api/endpoints/organization_member_invite/details.py

-        raise NotImplementedError
+        # 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):


maybe check the user is authenticated separately and early return? they should always be authenticated to proceed

cathteng · 2025-04-29T22:59:16Z

src/sentry/api/endpoints/organization_member_invite/details.py

@@ -140,7 +145,90 @@ def put(

        return Response(serialize(invited_member, request.user), status=200)

+    def _remove_invite_from_db(


this is almost the same as reject_invite_request on the OrganizationMemberInvite model. could they be collapsed into a single function?

getsantry · 2025-06-06T07:00:14Z

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 2, 2025

mifu67 commented Apr 2, 2025

View reviewed changes

vercel bot deployed to Preview April 7, 2025 19:52 View deployment

mifu67 commented Apr 7, 2025

View reviewed changes

src/sentry/api/endpoints/organization_member_invite/details.py Outdated Show resolved Hide resolved

mifu67 marked this pull request as ready for review April 8, 2025 16:38

mifu67 requested a review from cathteng April 8, 2025 16:38

vercel bot deployed to Preview April 8, 2025 16:45 View deployment

cathteng reviewed Apr 14, 2025

View reviewed changes

src/sentry/api/endpoints/organization_member_invite/details.py Outdated Show resolved Hide resolved

Base automatically changed from mifu67/member-invite/omi-details-put to master April 15, 2025 23:04

mifu67 added 4 commits April 17, 2025 10:04

first pass + simple test

02b148e

tests (need to fix audit log stuff)

4ca2a29

fix tests

42f862e

repair rebase

d71b8b1

mifu67 force-pushed the mifu67/member-invite/omi-details-delete branch from 62029b8 to d71b8b1 Compare April 17, 2025 17:14

vercel bot deployed to Preview April 17, 2025 17:20 View deployment

PR comments

fcb1187

mifu67 requested a review from cathteng April 17, 2025 18:41

vercel bot deployed to Preview April 17, 2025 18:48 View deployment

mifu67 requested a review from leedongwei April 28, 2025 16:08

leedongwei reviewed Apr 29, 2025

View reviewed changes

cathteng reviewed Apr 29, 2025

View reviewed changes

getsantry bot added the Stale label Jun 6, 2025

mifu67 added WIP and removed Stale labels Jun 6, 2025

		@@ -140,7 +145,90 @@ def put(

		return Response(serialize(invited_member, request.user), status=200)

		def _remove_invite_from_db(

Uh oh!

feat(org member invite): OrganizationMemberInviteDetails DELETE endpoint #88587

Are you sure you want to change the base?

feat(org member invite): OrganizationMemberInviteDetails DELETE endpoint #88587

Uh oh!

Conversation

mifu67 commented Apr 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leedongwei left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leedongwei Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

getsantry bot commented Jun 6, 2025

Uh oh!

Uh oh!

codecov bot commented Apr 2, 2025 •

edited

Loading

leedongwei Apr 29, 2025 •

edited

Loading