Gate activity reads on backfill completion

Author: shashjar

src/sentry/api/helpers/group_index/update.py

@@ -33,6 +33,7 @@
     resolve_action_source,
 )
 from sentry.issues.action_log.types import MergeIntoOtherAction
+from sentry.issues.derived.gate import should_serve_action_log_activity
 from sentry.issues.grouptype import GroupCategory
 from sentry.issues.ignored import handle_archived_until_escalating, handle_ignored
 from sentry.issues.merge import MergedGroup, handle_merge
@@ -783,9 +784,7 @@ def prepare_response(
         if len(group_list) == 1:
             if res_type in (GroupResolution.Type.in_next_release, GroupResolution.Type.in_release):
                 group = group_list[0]
-                if features.has(
-                    "projects:issue-action-log-activity", group.project, actor=acting_user
-                ):
+                if should_serve_action_log_activity(group.project, acting_user):
                     action_log = GroupActionLogEntry.objects.get_actions_for_group(
                         group, ACTIVITIES_COUNT - 1
                     )

src/sentry/issues/derived/gate.py

@@ -1,6 +1,10 @@
+from django.contrib.auth.models import AnonymousUser
+
 from sentry import features
 from sentry.models.options.project_option import ProjectOption
 from sentry.models.project import Project
+from sentry.users.models.user import User
+from sentry.users.services.user.model import RpcUser
 
 # ProjectOption key whose value tracks group action-log backfill state:
 # true means complete, false means pending, and a missing option means not yet scheduled.
@@ -14,8 +18,19 @@ def is_backfilled(project: Project) -> bool:
 
 
 def derived_should_be_correct(project: Project) -> bool:
-    """The project is backfilled and writing actions, so derived data should cover its full history.
+    """
+    The project is backfilled and writing actions, so derived data should cover its full history.
 
     It may still be wrong, but checking it and surfacing it should be reasonable.
     """
     return features.has("projects:issue-action-log-write-to-db", project) and is_backfilled(project)
+
+
+def should_serve_action_log_activity(
+    project: Project,
+    actor: User | RpcUser | AnonymousUser | None = None,
+) -> bool:
+    """Whether the action log can back this project's Activity-shaped responses."""
+    return features.has(
+        "projects:issue-action-log-activity", project, actor=actor
+    ) and derived_should_be_correct(project)

src/sentry/issues/endpoints/group_activities.py

@@ -3,13 +3,13 @@
 from rest_framework.request import Request
 from rest_framework.response import Response
 
-from sentry import features
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import cell_silo_endpoint
 from sentry.api.helpers.deprecation import deprecated
 from sentry.api.serializers import serialize
 from sentry.api.serializers.models.groupactionlogentry import serialize_first_seen_entry
 from sentry.constants import CELL_API_DEPRECATION_DATE
+from sentry.issues.derived.gate import should_serve_action_log_activity
 from sentry.issues.endpoints.bases.group import GroupEndpoint
 from sentry.issues.models.groupactionlogentry import GroupActionLogEntry
 from sentry.models.activity import Activity
@@ -33,7 +33,7 @@ def get(self, request: Request, group: Group) -> Response:
         """
         Retrieve all the Activities for a Group
         """
-        if features.has("projects:issue-action-log-activity", group.project, actor=request.user):
+        if should_serve_action_log_activity(group.project, request.user):
             action_log = GroupActionLogEntry.objects.get_actions_for_group(group, 99)
             if action_log:
                 serialized = serialize(action_log, request.user)

src/sentry/issues/endpoints/group_details.py

@@ -54,7 +54,7 @@
     get_issue_tsdb_group_model,
 )
 from sentry.issues.derived.check import check_status_consistency
-from sentry.issues.derived.gate import derived_should_be_correct
+from sentry.issues.derived.gate import derived_should_be_correct, should_serve_action_log_activity
 from sentry.issues.endpoints.bases.group import GroupEndpoint
 from sentry.issues.escalating.escalating_group_forecast import EscalatingGroupForecast
 from sentry.issues.models.groupactionlogentry import GroupActionLogEntry
@@ -368,9 +368,7 @@ def get(self, request: Request, group: Group) -> Response[GroupDetailsResponse]:
                 }
             )
 
-            if features.has(
-                "projects:issue-action-log-activity", group.project, actor=request.user
-            ):
+            if should_serve_action_log_activity(group.project, request.user):
                 action_log = GroupActionLogEntry.objects.get_actions_for_group(group, 99)
                 if action_log:
                     # swap action log data in under the activity name

src/sentry/issues/endpoints/group_notes.py

@@ -8,7 +8,6 @@
 from rest_framework.request import Request
 from rest_framework.response import Response
 
-from sentry import features
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import cell_silo_endpoint
 from sentry.api.helpers.deprecation import deprecated
@@ -25,6 +24,7 @@
     GroupActionActor,
     GroupActionType,
 )
+from sentry.issues.derived.gate import should_serve_action_log_activity
 from sentry.issues.endpoints.bases.group import GroupEndpoint
 from sentry.issues.models.groupactionlogentry import GroupActionLogEntry
 from sentry.models.activity import Activity
@@ -58,7 +58,7 @@ class GroupNotesEndpoint(GroupEndpoint):
         url_names=["sentry-api-0-group-notes"],
     )
     def get(self, request: Request, group: Group) -> Response:
-        if features.has("projects:issue-action-log-activity", group.project, actor=request.user):
+        if should_serve_action_log_activity(group.project, request.user):
             edit_entries = GroupActionLogEntry.objects.filter(
                 group_id=group.id, type=GroupActionType.COMMENT_EDIT.value
             ).order_by("-date_added", "-id")
@@ -187,7 +187,7 @@ def post(self, request: Request, group: Group) -> Response:
             sender="post",
         )
 
-        if features.has("projects:issue-action-log-activity", group.project, actor=request.user):
+        if should_serve_action_log_activity(group.project, request.user):
             entry = GroupActionLogEntry.objects.filter(
                 group_id=group.id,
                 idempotency_key=activity_action_idempotency_key(activity),

src/sentry/issues/endpoints/group_notes_details.py

@@ -6,7 +6,6 @@
 from rest_framework.request import Request
 from rest_framework.response import Response
 
-from sentry import features
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import cell_silo_endpoint
 from sentry.api.exceptions import ResourceDoesNotExist
@@ -24,6 +23,7 @@
     resolve_action_source,
 )
 from sentry.issues.action_log.types import CommentDeleteAction, CommentEditAction
+from sentry.issues.derived.gate import should_serve_action_log_activity
 from sentry.issues.endpoints.bases.group import GroupEndpoint
 from sentry.issues.models.groupactionlogentry import GroupActionLogEntry
 from sentry.models.activity import Activity
@@ -80,8 +80,8 @@ def delete(self, request: Request, group: Group, note_id: str) -> Response:
             group_id=group.id,
             idempotency_key=activity_action_idempotency_key(note),
         ).first()
-        if original_comment_log_action is None and features.has(
-            "projects:issue-action-log-activity", group.project, actor=request.user
+        if original_comment_log_action is None and should_serve_action_log_activity(
+            group.project, request.user
         ):
             raise ResourceDoesNotExist
 
@@ -164,8 +164,8 @@ def put(self, request: Request, group: Group, note_id: str) -> Response:
                 group_id=group.id,
                 idempotency_key=activity_action_idempotency_key(note),
             ).first()
-            if original_comment_log_action is None and features.has(
-                "projects:issue-action-log-activity", group.project, actor=request.user
+            if original_comment_log_action is None and should_serve_action_log_activity(
+                group.project, request.user
             ):
                 raise ResourceDoesNotExist
 
@@ -208,9 +208,7 @@ def put(self, request: Request, group: Group, note_id: str) -> Response:
                 sender="put",
             )
 
-            if features.has(
-                "projects:issue-action-log-activity", group.project, actor=request.user
-            ):
+            if should_serve_action_log_activity(group.project, request.user):
                 if original_comment_log_action is not None:
                     # editing a note doesn't update its COMMENT entry (instead it
                     # appends a separate COMMENT_EDIT entry), so patch in the fresh

tests/sentry/api/helpers/test_group_index.py

@@ -34,6 +34,7 @@
 from sentry.grouping.grouptype import ErrorGroupType
 from sentry.issues.action_log import ActionSource, GroupActionActor, action_context_scope
 from sentry.issues.action_log.types import GroupActionType, GroupActorType
+from sentry.issues.derived.gate import GROUP_ACTION_LOG_BACKFILL_COMPLETED_OPTION
 from sentry.issues.issue_search import parse_search_query
 from sentry.issues.models.groupactionlogentry import GroupActionLogEntry
 from sentry.models.activity import Activity
@@ -736,6 +737,7 @@ def test_resolve_in_next_release_ignores_archived_releases(self) -> None:
     def test_resolve_in_next_release_activity_from_action_log(self) -> None:
         self.create_release(project=self.project, version="test@1.0.0.0")
         group = self.create_group(status=GroupStatus.UNRESOLVED)
+        self.project.update_option(GROUP_ACTION_LOG_BACKFILL_COMPLETED_OPTION, True)
         GroupActionLogEntry.objects.create(
             group_id=group.id,
             project_id=group.project_id,
@@ -751,24 +753,34 @@ def test_resolve_in_next_release_activity_from_action_log(self) -> None:
         request = _wrap_request(http_request, data={"status": "resolvedInNextRelease"})
 
         group_list = get_group_list(self.organization.id, [self.project], request.GET.getlist("id"))
-        with self.feature("projects:issue-action-log-activity"):
+        with self.feature(
+            ["projects:issue-action-log-write-to-db", "projects:issue-action-log-activity"]
+        ):
             response = update_groups(request, group_list)
 
         activity = response.data["activity"]
-        assert [entry["type"] for entry in activity] == ["set_resolved", "first_seen"]
+        # the manually logged RESOLVE exists only in GALE, so its presence means
+        # the action log was served rather than Activity
+        assert "set_resolved" in [entry["type"] for entry in activity]
         assert activity[-1]["id"] == "0"
 
-    def test_resolve_in_next_release_no_activity_without_action_log(self) -> None:
+    def test_resolve_in_next_release_no_activity_when_action_log_is_empty(self) -> None:
+        # A gated project can still read an empty log: the GALE write for this
+        # resolve goes through an outbox that may not have drained yet.
         self.create_release(project=self.project, version="test@1.0.0.0")
         group = self.create_group(status=GroupStatus.UNRESOLVED)
+        self.project.update_option(GROUP_ACTION_LOG_BACKFILL_COMPLETED_OPTION, True)
 
         http_request = self.make_request(user=self.user, method="GET")
         http_request.GET = QueryDict(query_string=f"id={group.id}")
         request = _wrap_request(http_request, data={"status": "resolvedInNextRelease"})
 
         group_list = get_group_list(self.organization.id, [self.project], request.GET.getlist("id"))
         with (
-            self.feature("projects:issue-action-log-activity"),
+            self.feature(
+                ["projects:issue-action-log-write-to-db", "projects:issue-action-log-activity"]
+            ),
+            patch.object(GroupActionLogEntry.objects, "get_actions_for_group", return_value=[]),
             self.assertLogs("sentry.api.helpers.group_index.update", level="INFO") as logs,
         ):
             response = update_groups(request, group_list)
@@ -778,7 +790,35 @@ def test_resolve_in_next_release_no_activity_without_action_log(self) -> None:
         )
         assert response is not None
         assert "activity" not in response.data
-        assert GroupActionLogEntry.objects.filter(group_id=group.id).count() == 0
+
+    def test_resolve_in_next_release_ignores_action_log_when_not_backfilled(self) -> None:
+        # The log covers only part of this project's history, so serving it would
+        # silently drop everything that predates the rollout. Fall back to Activity.
+        self.create_release(project=self.project, version="test@1.0.0.0")
+        group = self.create_group(status=GroupStatus.UNRESOLVED)
+        GroupActionLogEntry.objects.create(
+            group_id=group.id,
+            project_id=group.project_id,
+            type=GroupActionType.COMMENT.value,
+            actor_type=GroupActorType.USER.value,
+            actor_id=self.user.id,
+            source="web",
+            data={"comment_id": 123, "text": "hello world"},
+        )
+
+        http_request = self.make_request(user=self.user, method="GET")
+        http_request.GET = QueryDict(query_string=f"id={group.id}")
+        request = _wrap_request(http_request, data={"status": "resolvedInNextRelease"})
+
+        group_list = get_group_list(self.organization.id, [self.project], request.GET.getlist("id"))
+        with self.feature(
+            ["projects:issue-action-log-write-to-db", "projects:issue-action-log-activity"]
+        ):
+            response = update_groups(request, group_list)
+
+        # the COMMENT only exists in the log, so its absence means Activity was served
+        activity = response.data["activity"]
+        assert "note" not in [entry["type"] for entry in activity]
 
 
 class MergeGroupsTest(TestCase):

tests/sentry/issues/endpoints/test_group_activities.py

@@ -1,4 +1,5 @@
 from sentry.issues.action_log.types import ActionSource, GroupActionType, GroupActorType
+from sentry.issues.derived.gate import GROUP_ACTION_LOG_BACKFILL_COMPLETED_OPTION
 from sentry.issues.models.groupactionlogentry import GroupActionLogEntry
 from sentry.models.activity import Activity
 from sentry.models.group import GroupStatus
@@ -44,9 +45,10 @@ def test_endpoint_with_group_activities(self) -> None:
         assert "activity" in response.data
         assert len(response.data["activity"]) == 5
 
-    @with_feature("projects:issue-action-log-activity")
+    @with_feature(["projects:issue-action-log-write-to-db", "projects:issue-action-log-activity"])
     def test_endpoint_with_group_action_log_entries(self) -> None:
         group = self.create_group(status=GroupStatus.UNRESOLVED)
+        group.project.update_option(GROUP_ACTION_LOG_BACKFILL_COMPLETED_OPTION, True)
 
         for i in range(0, 4):
             GroupActionLogEntry.objects.create(

tests/sentry/issues/endpoints/test_group_details.py

@@ -17,6 +17,7 @@
     ActionSource,
     GroupActionActor,
     GroupActionType,
+    GroupActorType,
     ReconcileStatusAction,
 )
 from sentry.issues.constants import cache_key_for_issue_view
@@ -134,6 +135,7 @@ def test_no_releases(self) -> None:
     @with_feature(["projects:issue-action-log-write-to-db", "projects:issue-action-log-activity"])
     def test_group_action_log_entry(self) -> None:
         group = self.create_group()
+        group.project.update_option(GROUP_ACTION_LOG_BACKFILL_COMPLETED_OPTION, True)
 
         # activity dual writes to GALE. use action context scope to attribute it to the user rather than system
         data = {"assignee": str(self.user.id)}
@@ -173,6 +175,7 @@ def test_group_action_log_entry(self) -> None:
     def test_group_action_log_comment_is_addressable(self) -> None:
         self.login_as(user=self.user)
         group = self.create_group()
+        group.project.update_option(GROUP_ACTION_LOG_BACKFILL_COMPLETED_OPTION, True)
 
         comments_url = f"/api/0/issues/{group.id}/comments/"
         response = self.client.post(comments_url, format="json", data={"text": "original"})
@@ -205,6 +208,64 @@ def test_group_action_log_comment_is_addressable(self) -> None:
         response = self.client.delete(f"{comments_url}{note_id}/", format="json")
         assert response.status_code == 204, response.status_code
 
+    @with_feature(["projects:issue-action-log-write-to-db", "projects:issue-action-log-activity"])
+    def test_group_action_log_ignored_when_not_backfilled(self) -> None:
+        # The log covers only part of this project's history, so serving it would
+        # silently drop everything that predates the rollout. Fall back to Activity.
+        self.login_as(user=self.user)
+        group = self.create_group()
+        self.create_group_action_log_entry(
+            group=group,
+            type=GroupActionType.COMMENT,
+            actor_type=GroupActorType.USER,
+            actor_id=self.user.id,
+            data={"comment_id": 123, "text": "hello world"},
+        )
+
+        url = f"/api/0/organizations/{group.organization.slug}/issues/{group.id}/"
+        response = self.client.get(url, format="json")
+        assert response.status_code == 200, response.content
+
+        # the COMMENT only exists in the log, so its absence means Activity was served
+        assert [item["type"] for item in response.data["activity"]] == ["first_seen"]
+
+    @with_feature(["projects:issue-action-log-write-to-db", "projects:issue-action-log-activity"])
+    def test_group_action_log_served_when_backfilled(self) -> None:
+        self.login_as(user=self.user)
+        group = self.create_group()
+        group.project.update_option(GROUP_ACTION_LOG_BACKFILL_COMPLETED_OPTION, True)
+        self.create_group_action_log_entry(
+            group=group,
+            type=GroupActionType.COMMENT,
+            actor_type=GroupActorType.USER,
+            actor_id=self.user.id,
+            data={"comment_id": 123, "text": "hello world"},
+        )
+
+        url = f"/api/0/organizations/{group.organization.slug}/issues/{group.id}/"
+        response = self.client.get(url, format="json")
+        assert response.status_code == 200, response.content
+
+        assert [item["type"] for item in response.data["activity"]] == ["note", "first_seen"]
+
+    def test_group_action_log_ignored_when_flag_off(self) -> None:
+        self.login_as(user=self.user)
+        group = self.create_group()
+        group.project.update_option(GROUP_ACTION_LOG_BACKFILL_COMPLETED_OPTION, True)
+        self.create_group_action_log_entry(
+            group=group,
+            type=GroupActionType.COMMENT,
+            actor_type=GroupActorType.USER,
+            actor_id=self.user.id,
+            data={"comment_id": 123, "text": "hello world"},
+        )
+
+        url = f"/api/0/organizations/{group.organization.slug}/issues/{group.id}/"
+        response = self.client.get(url, format="json")
+        assert response.status_code == 200, response.content
+
+        assert [item["type"] for item in response.data["activity"]] == ["first_seen"]
+
     def test_pending_delete_pending_merge_excluded(self) -> None:
         group1 = self.create_group(status=GroupStatus.PENDING_DELETION)
         group2 = self.create_group(status=GroupStatus.DELETION_IN_PROGRESS)