diff --git a/src/sentry/issues/action_log/types.py b/src/sentry/issues/action_log/types.py index 19d3e941c6e9..7f326bba32a1 100644 --- a/src/sentry/issues/action_log/types.py +++ b/src/sentry/issues/action_log/types.py @@ -76,6 +76,7 @@ class GroupActionType(IntEnum): ROOT_CAUSE_IDENTIFIED = 26 AUTOFIX_CODING_COMPLETE = 27 PULL_REQUEST_CLOSED = 29 + RECONCILE_STATUS = 30 # Certain GroupActions are mirrors of Activity records. # (See ACTIVITY_TYPE_TO_GROUP_ACTION_TYPE for the mapping.) @@ -618,3 +619,19 @@ class RelatedPullRequestClosedAction(GroupAction): @classmethod def get_type(cls) -> GroupActionType: return GroupActionType.PULL_REQUEST_CLOSED + + +class ReconcileStatusAction(GroupAction): + """Force-set the derived status to a known-correct value. + + Used when out-of-log information (e.g. the Group model) disagrees with + the derived status computed from the action log. + """ + + # Must stay in sync with IssueStatus in sentry.issues.derived.features. + status: Literal["open", "closed"] + reason: Optional[str] = None + + @classmethod + def get_type(cls) -> GroupActionType: + return GroupActionType.RECONCILE_STATUS diff --git a/src/sentry/issues/derived/aggregators.py b/src/sentry/issues/derived/aggregators.py index 1d77966972aa..c4fef2ce319d 100644 --- a/src/sentry/issues/derived/aggregators.py +++ b/src/sentry/issues/derived/aggregators.py @@ -1,4 +1,4 @@ -from sentry.issues.action_log.types import GroupActionType +from sentry.issues.action_log.types import GroupActionType, ReconcileStatusAction from sentry.issues.derived.features import ( LAST_PROGRESSED_AT, PROGRESS, @@ -22,31 +22,43 @@ def track_views(state: StateView, entry: GroupActionLogEntry) -> AggregatorResul return emit(VIEW_COUNT.value(state[VIEW_COUNT] + 1)) -@aggregator( - (STATUS,), - scope=( +_RESOLVES: frozenset[GroupActionType] = frozenset( + { GroupActionType.RESOLVE, - GroupActionType.UNRESOLVE, GroupActionType.RESOLVED_IN_PULL_REQUEST, GroupActionType.ARCHIVE, + } +) + +_REOPENS: frozenset[GroupActionType] = frozenset( + { + GroupActionType.UNRESOLVE, GroupActionType.SET_REGRESSED, + } +) + + +@aggregator( + (STATUS,), + scope=( + *_RESOLVES, + *_REOPENS, + GroupActionType.RECONCILE_STATUS, ), ) def track_status(state: StateView, entry: GroupActionLogEntry) -> AggregatorResult: current = state[STATUS] - resolves = ( - GroupActionType.RESOLVE.value, - GroupActionType.RESOLVED_IN_PULL_REQUEST.value, - GroupActionType.ARCHIVE.value, - ) - reopens = ( - GroupActionType.UNRESOLVE.value, - GroupActionType.SET_REGRESSED.value, - ) - if entry.type in resolves and current == IssueStatus.OPEN: + + if entry.type == GroupActionType.RECONCILE_STATUS: + action = ReconcileStatusAction(**entry.data) + new_status = IssueStatus(action.status) + if new_status != current: + return emit(STATUS.value(new_status)) + elif entry.type in _RESOLVES and current == IssueStatus.OPEN: return emit(STATUS.value(IssueStatus.CLOSED)) - if entry.type in reopens and current == IssueStatus.CLOSED: + elif entry.type in _REOPENS and current == IssueStatus.CLOSED: return emit(STATUS.value(IssueStatus.OPEN)) + return None diff --git a/src/sentry/issues/derived/framework.py b/src/sentry/issues/derived/framework.py index d98773e4c64f..0ba327cdead9 100644 --- a/src/sentry/issues/derived/framework.py +++ b/src/sentry/issues/derived/framework.py @@ -262,6 +262,8 @@ def emit(*entries: FeatureEntry) -> AggregatorResult: >>> return emit(VIEW_COUNT.value(5), STATUS.value(IssueStatus.CLOSED)) """ + if not entries: + return None return StateUpdate(dict(entries)) diff --git a/tests/sentry/issues/derived/test_aggregators.py b/tests/sentry/issues/derived/test_aggregators.py index 8568462fb533..8033ac6f398e 100644 --- a/tests/sentry/issues/derived/test_aggregators.py +++ b/tests/sentry/issues/derived/test_aggregators.py @@ -9,11 +9,11 @@ from dataclasses import dataclass, field from datetime import UTC, datetime -from typing import Any +from typing import Any, get_args, get_type_hints import pytest -from sentry.issues.action_log.types import GroupActionType, GroupActorType +from sentry.issues.action_log.types import GroupActionType, GroupActorType, ReconcileStatusAction from sentry.issues.derived.aggregators import AGGREGATORS from sentry.issues.derived.features import ( LAST_PROGRESSED_AT, @@ -68,6 +68,14 @@ def _resolved_pr_data(pr_id: int) -> dict[str, object]: return {"pull_request": pr_id} +def _reconcile_entry(status: IssueStatus) -> FakeEntry: + action = ReconcileStatusAction(status=status.value) + return FakeEntry( + type=GroupActionType.RECONCILE_STATUS, + data=action.dict(), + ) + + # --------------------------------------------------------------------------- # track_views # --------------------------------------------------------------------------- @@ -239,6 +247,111 @@ def test_resolved_in_pr_when_already_closed_is_noop() -> None: ) +class TestReconcileStatus: + def test_literal_matches_issue_status(self) -> None: + literal_values = set(get_args(get_type_hints(ReconcileStatusAction)["status"])) + enum_values = {s.value for s in IssueStatus} + assert literal_values == enum_values + + def test_action_roundtrips(self) -> None: + action = ReconcileStatusAction( + status=IssueStatus.CLOSED.value, reason="group model disagrees" + ) + assert action.status == "closed" + assert action.reason == "group model disagrees" + assert IssueStatus(action.status) == IssueStatus.CLOSED + # reason survives serialization round-trip through dict + restored = ReconcileStatusAction(**action.dict()) + assert restored.reason == "group model disagrees" + + def test_overrides_status(self) -> None: + assert ( + _run_for_feature( + STATUS, + [ + FakeEntry(type=GroupActionType.RESOLVE), + FakeEntry(type=GroupActionType.UNRESOLVE), + _reconcile_entry(IssueStatus.CLOSED), + ], + ) + == IssueStatus.CLOSED + ) + + def test_from_initial(self) -> None: + assert ( + _run_for_feature( + STATUS, + [_reconcile_entry(IssueStatus.CLOSED)], + ) + == IssueStatus.CLOSED + ) + + def test_reopens_closed(self) -> None: + assert ( + _run_for_feature( + STATUS, + [ + FakeEntry(type=GroupActionType.RESOLVE), + _reconcile_entry(IssueStatus.OPEN), + ], + ) + == IssueStatus.OPEN + ) + + def test_same_value_is_noop(self) -> None: + assert ( + _run_for_feature( + STATUS, + [_reconcile_entry(IssueStatus.OPEN)], + ) + == IssueStatus.OPEN + ) + + def test_normal_actions_continue_after(self) -> None: + assert ( + _run_for_feature( + STATUS, + [ + _reconcile_entry(IssueStatus.CLOSED), + FakeEntry(type=GroupActionType.UNRESOLVE), + ], + ) + == IssueStatus.OPEN + ) + + def test_to_closed_nulls_progress(self) -> None: + p = _pipeline() + state = p.run( + [ + FakeEntry(type=GroupActionType.ASSIGN), + _reconcile_entry(IssueStatus.CLOSED), + ] + ) + assert state[STATUS] == IssueStatus.CLOSED + assert state[PROGRESS] is None + + def test_updates_last_progressed_at(self) -> None: + p = _pipeline() + state = p.run( + [ + FakeEntry(type=GroupActionType.ASSIGN), + _reconcile_entry(IssueStatus.CLOSED), + ] + ) + assert state[LAST_PROGRESSED_AT] is not None + + def test_to_open_resets_progress(self) -> None: + p = _pipeline() + state = p.run( + [ + FakeEntry(type=GroupActionType.RESOLVE), + _reconcile_entry(IssueStatus.OPEN), + ] + ) + assert state[STATUS] == IssueStatus.OPEN + assert state[PROGRESS] == IssueProgressState.IDENTIFIED + + # --------------------------------------------------------------------------- # track_progress # ---------------------------------------------------------------------------