fix(investigations): Resolve who applied a request on read

Look the applier up with the usual user serializer, so names and avatars stay current and no user data is copied into the request, and list the newest report's requests first.

Author: wedamija

src/sentry/investigations/endpoints/serializers/action_request.py

@@ -12,9 +12,11 @@
     InvestigationActionRequestAnchorType,
     InvestigationBlock,
 )
-from sentry.users.api.serializers.user import SerializedAvatarFields
+from sentry.users.api.serializers.user import UserSerializerResponse
 from sentry.users.models.user import User
 from sentry.users.services.user.model import RpcUser
+from sentry.users.services.user.serial import serialize_generic_user
+from sentry.users.services.user.service import user_service
 
 
 class InvestigationActionRequestAnchorResponse(TypedDict):
@@ -27,15 +29,6 @@ class InvestigationActionRequestAnchorResponse(TypedDict):
     blockId: str | None
 
 
-class InvestigationActionRequestActorResponse(TypedDict):
-    id: str
-    name: str
-    username: str
-    email: str
-    avatarUrl: str
-    avatar: SerializedAvatarFields
-
-
 class InvestigationActionRequestSerializerResponse(TypedDict):
     """
     A step Seer suggests. `primary` marks the recommended next step after a run, and `anchor`
@@ -55,7 +48,7 @@ class InvestigationActionRequestSerializerResponse(TypedDict):
     state: str
     reason: str | None
     action: dict[str, Any] | None
-    appliedBy: InvestigationActionRequestActorResponse | None
+    appliedBy: UserSerializerResponse | None
     appliedDetails: dict[str, Any] | None
     dateApplied: datetime | None
     error: dict[str, Any] | None
@@ -94,13 +87,25 @@ def get_attrs(
         **kwargs: Any,
     ) -> MutableMapping[InvestigationActionRequest, dict[str, Any]]:
         anchored_blocks = self._anchored_block_ids(item_list)
+        applier_ids = {item.applied_by_id for item in item_list if item.applied_by_id is not None}
+        appliers = (
+            {
+                applier["id"]: applier
+                for applier in user_service.serialize_many(
+                    filter={"user_ids": list(applier_ids)}, as_user=serialize_generic_user(user)
+                )
+            }
+            if applier_ids
+            else {}
+        )
         return {
             item: {
                 "block_id": (
                     anchored_blocks.get((item.investigation_id, item.anchor_id or ""))
                     if item.anchor_type == InvestigationActionRequestAnchorType.BLOCK
                     else None
-                )
+                ),
+                "applied_by": appliers.get(str(item.applied_by_id)),
             }
             for item in item_list
         }
@@ -129,14 +134,8 @@ def serialize(
             "state": obj.state,
             "reason": obj.reason,
             "action": obj.action_payload,
-            "appliedBy": (
-                (obj.applied_details or {}).get("actor") if obj.applied_by_id is not None else None
-            ),
-            "appliedDetails": (
-                {key: value for key, value in obj.applied_details.items() if key != "actor"}
-                if obj.applied_details is not None
-                else None
-            ),
+            "appliedBy": attrs["applied_by"],
+            "appliedDetails": obj.applied_details,
             "dateApplied": obj.date_applied,
             "error": obj.error,
             "dateCreated": obj.date_added,

src/sentry/investigations/endpoints/serializers/investigation.py

@@ -117,7 +117,8 @@ class InvestigationDetailsSerializerResponse(InvestigationSerializerResponse):
     """
     Full notebook with ordered active blocks, parameter definitions and saved values, project
     IDs, and source context. source.snapshot is present only for source-backed investigations.
-    actionRequests has Seer's live action requests, with the primary one first.
+    actionRequests has Seer's action requests that aren't superseded, newest report first, with
+    that report's primary at the top.
     """
 
     template: InvestigationTemplateSerializerResponse | None
@@ -242,7 +243,7 @@ def _action_requests_by_investigation(
         action_requests = list(
             InvestigationActionRequest.objects.filter(investigation__in=item_list)
             .exclude(state=InvestigationActionRequestState.SUPERSEDED)
-            .order_by("-primary", "-report_revision", "id")
+            .order_by("-report_revision", "-primary", "id")
         )
         serialized = serialize(
             action_requests,

tests/sentry/investigations/endpoints/serializers/test_investigation.py

@@ -374,26 +374,32 @@ def test_serializes_requests_without_a_known_action(self) -> None:
             {"kind": "seer_task", "prompt": "Add spans"},
         ]
 
-    def test_orders_the_primary_first_and_omits_superseded_requests(self) -> None:
+    def test_orders_the_newest_report_first_and_omits_superseded_requests(self) -> None:
+        old_primary = self.add_request(
+            "old-primary",
+            report_revision=0,
+            primary=True,
+            state=InvestigationActionRequestState.WITHDRAWN,
+        )
         older = self.add_request("older", report_revision=0)
         self.add_request("superseded", state=InvestigationActionRequestState.SUPERSEDED)
-        second = self.add_request("second", report_revision=1)
-        first = self.add_request("first", report_revision=0, primary=True)
+        newer = self.add_request("newer", report_revision=1)
+        primary = self.add_request("primary", report_revision=1, primary=True)
 
         assert [row["id"] for row in self.serialize_requests()] == [
-            str(first.id),
-            str(second.id),
+            str(primary.id),
+            str(newer.id),
+            str(old_primary.id),
             str(older.id),
         ]
 
     def test_serializes_who_applied_it_failures_and_withdrawals(self) -> None:
         now = timezone.now()
-        actor = {"id": str(self.user.id), "name": "Ada Lovelace"}
         self.add_request(
             state=InvestigationActionRequestState.APPLIED,
             applied_by_id=self.user.id,
             date_applied=now,
-            applied_details={"provider": "datadog", "rerun": None, "actor": actor},
+            applied_details={"provider": "datadog", "rerun": None},
         )
         self.add_request(
             "failed",
@@ -408,7 +414,8 @@ def test_serializes_who_applied_it_failures_and_withdrawals(self) -> None:
 
         applied, failed, withdrawn = self.serialize_requests()
 
-        assert applied["appliedBy"] == actor
+        assert applied["appliedBy"] is not None
+        assert applied["appliedBy"]["id"] == str(self.user.id)
         assert applied["appliedDetails"] == {"provider": "datadog", "rerun": None}
         assert applied["dateApplied"] == now
         assert failed["state"] == "failed"
@@ -418,11 +425,11 @@ def test_serializes_who_applied_it_failures_and_withdrawals(self) -> None:
         }
         assert (withdrawn["state"], withdrawn["reason"]) == ("withdrawn", "The spikes are real.")
 
-    def test_hides_the_stored_applier_once_the_user_is_gone(self) -> None:
+    def test_shows_no_applier_once_the_user_is_gone(self) -> None:
         self.add_request(
             state=InvestigationActionRequestState.APPLIED,
             applied_by_id=None,
-            applied_details={"provider": "datadog", "rerun": None, "actor": {"id": "1"}},
+            applied_details={"provider": "datadog", "rerun": None},
         )
 
         (applied,) = self.serialize_requests()