use user id for setting the allowlist instead of memberid

Author: shellmayr

src/sentry/api/serializers/models/organization.py

@@ -743,7 +743,7 @@ def serialize(  # type: ignore[override]
             context["replayAccessMembers"] = list(
                 OrganizationMemberReplayAccess.objects.filter(
                     organizationmember__organization=obj
-                ).values_list("organizationmember_id", flat=True)
+                ).values_list("organizationmember__user_id", flat=True)
             )
 
         if has_custom_dynamic_sampling(obj, actor=user):

src/sentry/core/endpoints/organization_details.py

@@ -346,7 +346,7 @@ class OrganizationSerializer(BaseOrganizationSerializer):
         child=serializers.IntegerField(),
         required=False,
         allow_null=True,
-        help_text="List of organization member IDs that have access to replay data. Only modifiable by owners and managers.",
+        help_text="List of user IDs that have access to replay data. Only modifiable by owners and managers.",
     )
 
     def _has_sso_enabled(self):
@@ -597,53 +597,57 @@ def save(self, **kwargs):
             changed_data["hasGranularReplayPermissions"] = f"to {new_value}"
 
         if "replayAccessMembers" in data:
-            member_ids = data["replayAccessMembers"]
-            if member_ids is None:
-                member_ids = []
-            current_member_ids = set(
+            user_ids = data["replayAccessMembers"]
+            if user_ids is None:
+                user_ids = []
+
+            current_user_ids = set(
                 OrganizationMemberReplayAccess.objects.filter(
                     organizationmember__organization=org
-                ).values_list("organizationmember_id", flat=True)
+                ).values_list("organizationmember__user_id", flat=True)
             )
-            new_member_ids = set(member_ids)
+            new_user_ids = set(user_ids)
+
+            to_add = new_user_ids - current_user_ids
+            to_remove = current_user_ids - new_user_ids
 
-            to_add = new_member_ids - current_member_ids
-            to_remove = current_member_ids - new_member_ids
             if to_add:
-                valid_member_ids = set(
-                    OrganizationMember.objects.filter(organization=org, id__in=to_add).values_list(
-                        "id", flat=True
-                    )
+                user_to_member = dict(
+                    OrganizationMember.objects.filter(
+                        organization=org, user_id__in=to_add
+                    ).values_list("user_id", "id")
                 )
-                invalid_member_ids = to_add - valid_member_ids
-                if invalid_member_ids:
+                invalid_user_ids = to_add - set(user_to_member.keys())
+                if invalid_user_ids:
                     raise serializers.ValidationError(
                         {
-                            "replayAccessMembers": f"Invalid organization member IDs: {sorted(invalid_member_ids)}"
+                            "replayAccessMembers": f"Invalid user IDs (not members of this organization): {sorted(invalid_user_ids)}"
                         }
                     )
 
                 OrganizationMemberReplayAccess.objects.bulk_create(
                     [
-                        OrganizationMemberReplayAccess(organizationmember_id=member_id)
-                        for member_id in to_add
+                        OrganizationMemberReplayAccess(
+                            organizationmember_id=user_to_member[user_id]
+                        )
+                        for user_id in to_add
                     ],
                     ignore_conflicts=True,
                 )
 
             if to_remove:
                 OrganizationMemberReplayAccess.objects.filter(
-                    organizationmember__organization=org, organizationmember_id__in=to_remove
+                    organizationmember__organization=org, organizationmember__user_id__in=to_remove
                 ).delete()
 
             if to_add or to_remove:
                 changes = []
                 if to_add:
-                    changes.append(f"added {len(to_add)} member(s)")
+                    changes.append(f"added {len(to_add)} user(s)")
                 if to_remove:
-                    changes.append(f"removed {len(to_remove)} member(s)")
+                    changes.append(f"removed {len(to_remove)} user(s)")
                 changed_data["replayAccessMembers"] = (
-                    f"{' and '.join(changes)} (total: {len(new_member_ids)} member(s) with access)"
+                    f"{' and '.join(changes)} (total: {len(new_user_ids)} user(s) with access)"
                 )
 
         if "openMembership" in data:
@@ -869,7 +873,7 @@ class OrganizationDetailsPutSerializer(serializers.Serializer):
     )
     replayAccessMembers = serializers.ListField(
         child=serializers.IntegerField(),
-        help_text="A list of organization member IDs who have permission to access replay data. Requires the hasGranularReplayPermissions flag to be true to be enforced.",
+        help_text="A list of user IDs who have permission to access replay data. Requires the hasGranularReplayPermissions flag to be true to be enforced.",
         required=False,
         allow_null=True,
     )

tests/sentry/api/serializers/test_organization.py

@@ -212,7 +212,7 @@ def test_replay_access_members_serialized(self) -> None:
             serializer = DetailedOrganizationSerializer()
             result = serialize(organization, user, serializer, access=acc)
 
-            assert set(result["replayAccessMembers"]) == {member1.id, member2.id}
+            assert set(result["replayAccessMembers"]) == {member1.user_id, member2.user_id}
 
     def test_replay_access_members_empty_when_none_set(self) -> None:
         user = self.create_user()

tests/sentry/core/endpoints/test_organization_details.py

@@ -1542,7 +1542,7 @@ def test_replay_access_members_add(self) -> None:
         with assume_test_silo_mode_of(AuditLogEntry):
             AuditLogEntry.objects.filter(organization_id=self.organization.id).delete()
 
-        data = {"replayAccessMembers": [member1.id, member2.id]}
+        data = {"replayAccessMembers": [member1.user_id, member2.user_id]}
         with outbox_runner():
             self.get_success_response(self.organization.slug, **data)
 
@@ -1555,8 +1555,8 @@ def test_replay_access_members_add(self) -> None:
 
         with assume_test_silo_mode_of(AuditLogEntry):
             log = AuditLogEntry.objects.get(organization_id=self.organization.id)
-        assert "added 2 member(s)" in log.data["replayAccessMembers"]
-        assert "total: 2 member(s)" in log.data["replayAccessMembers"]
+        assert "added 2 user(s)" in log.data["replayAccessMembers"]
+        assert "total: 2 user(s)" in log.data["replayAccessMembers"]
 
     @with_feature("organizations:granular-replay-permissions")
     def test_replay_access_members_remove(self) -> None:
@@ -1571,7 +1571,7 @@ def test_replay_access_members_remove(self) -> None:
         with assume_test_silo_mode_of(AuditLogEntry):
             AuditLogEntry.objects.filter(organization_id=self.organization.id).delete()
 
-        data = {"replayAccessMembers": [member1.id]}
+        data = {"replayAccessMembers": [member1.user_id]}
         with outbox_runner():
             self.get_success_response(self.organization.slug, **data)
 
@@ -1584,8 +1584,8 @@ def test_replay_access_members_remove(self) -> None:
 
         with assume_test_silo_mode_of(AuditLogEntry):
             log = AuditLogEntry.objects.get(organization_id=self.organization.id)
-        assert "removed 1 member(s)" in log.data["replayAccessMembers"]
-        assert "total: 1 member(s)" in log.data["replayAccessMembers"]
+        assert "removed 1 user(s)" in log.data["replayAccessMembers"]
+        assert "total: 1 user(s)" in log.data["replayAccessMembers"]
 
     @with_feature("organizations:granular-replay-permissions")
     def test_replay_access_members_add_and_remove(self) -> None:
@@ -1602,7 +1602,7 @@ def test_replay_access_members_add_and_remove(self) -> None:
         with assume_test_silo_mode_of(AuditLogEntry):
             AuditLogEntry.objects.filter(organization_id=self.organization.id).delete()
 
-        data = {"replayAccessMembers": [member2.id, member3.id]}
+        data = {"replayAccessMembers": [member2.user_id, member3.user_id]}
         with outbox_runner():
             self.get_success_response(self.organization.slug, **data)
 
@@ -1615,9 +1615,9 @@ def test_replay_access_members_add_and_remove(self) -> None:
 
         with assume_test_silo_mode_of(AuditLogEntry):
             log = AuditLogEntry.objects.get(organization_id=self.organization.id)
-        assert "added 2 member(s)" in log.data["replayAccessMembers"]
-        assert "removed 1 member(s)" in log.data["replayAccessMembers"]
-        assert "total: 2 member(s)" in log.data["replayAccessMembers"]
+        assert "added 2 user(s)" in log.data["replayAccessMembers"]
+        assert "removed 1 user(s)" in log.data["replayAccessMembers"]
+        assert "total: 2 user(s)" in log.data["replayAccessMembers"]
 
     @with_feature("organizations:granular-replay-permissions")
     def test_replay_access_members_clear_all(self) -> None:
@@ -1639,14 +1639,14 @@ def test_replay_access_members_clear_all(self) -> None:
 
         with assume_test_silo_mode_of(AuditLogEntry):
             log = AuditLogEntry.objects.get(organization_id=self.organization.id)
-        assert "removed 1 member(s)" in log.data["replayAccessMembers"]
-        assert "total: 0 member(s)" in log.data["replayAccessMembers"]
+        assert "removed 1 user(s)" in log.data["replayAccessMembers"]
+        assert "total: 0 user(s)" in log.data["replayAccessMembers"]
 
     def test_replay_access_members_requires_feature(self) -> None:
         member1 = self.create_member(
             organization=self.organization, user=self.create_user(), role="member"
         )
-        data = {"replayAccessMembers": [member1.id]}
+        data = {"replayAccessMembers": [member1.user_id]}
         self.get_error_response(self.organization.slug, **data, status_code=404)
 
     @with_feature("organizations:granular-replay-permissions")
@@ -1660,11 +1660,11 @@ def test_replay_access_members_requires_admin_scope(self) -> None:
         other_member = self.create_member(
             organization=self.organization, user=self.create_user(), role="member"
         )
-        data = {"replayAccessMembers": [other_member.id]}
+        data = {"replayAccessMembers": [other_member.user_id]}
         self.get_error_response(self.organization.slug, **data, status_code=403)
 
     @with_feature("organizations:granular-replay-permissions")
-    def test_replay_access_members_invalid_member_ids(self) -> None:
+    def test_replay_access_members_invalid_user_ids(self) -> None:
         nonexistent_id = 999999999
         data = {"replayAccessMembers": [nonexistent_id]}
         response = self.get_error_response(self.organization.slug, **data, status_code=400)
@@ -1677,22 +1677,22 @@ def test_replay_access_members_from_other_organization(self) -> None:
         other_org_member = self.create_member(
             organization=other_org, user=self.create_user(), role="member"
         )
-        data = {"replayAccessMembers": [other_org_member.id]}
+        data = {"replayAccessMembers": [other_org_member.user_id]}
         response = self.get_error_response(self.organization.slug, **data, status_code=400)
         assert "replayAccessMembers" in response.data
-        assert str(other_org_member.id) in response.data["replayAccessMembers"]
+        assert str(other_org_member.user_id) in response.data["replayAccessMembers"]
 
     @with_feature("organizations:granular-replay-permissions")
     def test_replay_access_members_mixed_valid_and_invalid(self) -> None:
         valid_member = self.create_member(
             organization=self.organization, user=self.create_user(), role="member"
         )
         nonexistent_id = 999999999
-        data = {"replayAccessMembers": [valid_member.id, nonexistent_id]}
+        data = {"replayAccessMembers": [valid_member.user_id, nonexistent_id]}
         response = self.get_error_response(self.organization.slug, **data, status_code=400)
         assert "replayAccessMembers" in response.data
         assert str(nonexistent_id) in response.data["replayAccessMembers"]
-        assert str(valid_member.id) not in response.data["replayAccessMembers"]
+        assert str(valid_member.user_id) not in response.data["replayAccessMembers"]
 
         access_count = OrganizationMemberReplayAccess.objects.filter(
             organizationmember__organization=self.organization