Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions packages/core/src/inbox/reportFiltering.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { SignalReport, SignalReportPriority } from "@posthog/shared/types";
import { describe, expect, it } from "vitest";
import {
buildArchiveListOrdering,
buildPriorityFilterParam,
buildSignalReportListOrdering,
buildSuggestedReviewerFilterParam,
Expand Down Expand Up @@ -113,6 +114,22 @@ describe("buildSignalReportListOrdering", () => {
});
});

describe("buildArchiveListOrdering", () => {
it.each([
["updated_at", "desc", "-updated_at"],
["updated_at", "asc", "updated_at"],
["created_at", "desc", "-created_at"],
] as const)("orders by %s (%s) only", (field, direction, expected) => {
expect(buildArchiveListOrdering(field, direction)).toBe(expected);
});

it("does not prefix with status, so suppressed and resolved interleave by time", () => {
expect(buildArchiveListOrdering("updated_at", "desc")).not.toContain(
"status",
);
});
});

describe("buildSuggestedReviewerFilterParam", () => {
it("returns undefined for an empty array", () => {
expect(buildSuggestedReviewerFilterParam([])).toBeUndefined();
Expand Down
30 changes: 23 additions & 7 deletions packages/core/src/inbox/reportFiltering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ export const INBOX_PIPELINE_STATUS_FILTER =
"potential,candidate,in_progress,ready,pending_input,failed";

/**
* Status filter for the Archive tab. `suppressed` is the only archived status:
* it is the single state the archive action sets, and the only not-in-inbox
* state worth restoring. `deleted` is permanent and stripped server-side; snooze
* is a temporary `snoozed_until` timestamp, not a status, and auto-returns. See
* `isDismissedReport` for the full rationale. Suppressed reports are excluded
* from the main pipeline query, so the Archive tab fetches them explicitly.
* Status filter for the Archive tab — the two terminal, not-in-inbox states:
* `suppressed` (the user archived it; restorable) and `resolved` (its
* implementation PR merged; terminal, shown for reference only). `deleted` is
* permanent and stripped server-side; snooze is a temporary `snoozed_until`
* timestamp, not a status, and auto-returns. See `isDismissedReport` for the
* full rationale. Both states are excluded from the main pipeline query, so the
* Archive tab fetches them explicitly.
*/
export const INBOX_DISMISSED_STATUS_FILTER = "suppressed";
export const INBOX_DISMISSED_STATUS_FILTER = "suppressed,resolved";
Comment thread
andrewm4894 marked this conversation as resolved.
Comment thread
andrewm4894 marked this conversation as resolved.

/**
* Status filter for the Pull requests tab's list and count. Only `ready` PRs —
Expand Down Expand Up @@ -78,6 +79,21 @@ export function buildSignalReportListOrdering(
return `status,${fieldKey}`;
}

/**
* Ordering for the Archive tab, which lists two terminal statuses
* (`suppressed` + `resolved`). Unlike the pipeline ordering above, it must NOT
* prefix with `status`: that would group one terminal state ahead of the other
* before applying the time sort, burying recent completions behind older items
* from the sibling status. Sort purely by the selected field so the list is
* globally newest-changed-first across both states.
*/
export function buildArchiveListOrdering(
field: SignalReportOrderingField,
direction: "asc" | "desc",
): string {
return direction === "desc" ? `-${field}` : field;
}

export function buildSuggestedReviewerFilterParam(
reviewerIds: string[],
): string | undefined {
Expand Down
14 changes: 10 additions & 4 deletions packages/core/src/inbox/reportMembership.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,12 @@ function fakeReport(overrides: Partial<SignalReport> = {}): SignalReport {
}

describe("isDismissedReport", () => {
it("matches only suppressed reports", () => {
expect(isDismissedReport(fakeReport({ status: "suppressed" }))).toBe(true);
});
it.each(["suppressed", "resolved"] as const)(
"matches %s reports",
(status) => {
expect(isDismissedReport(fakeReport({ status }))).toBe(true);
},
);

it.each([
"potential",
Expand Down Expand Up @@ -158,10 +161,13 @@ describe("tabFilters", () => {
});

describe("isExcludedFromInbox", () => {
it("returns true for suppressed and deleted", () => {
it("returns true for suppressed, resolved and deleted", () => {
expect(isExcludedFromInbox(fakeReport({ status: "suppressed" }))).toBe(
true,
);
expect(isExcludedFromInbox(fakeReport({ status: "resolved" }))).toBe(
true,
);
expect(isExcludedFromInbox(fakeReport({ status: "deleted" }))).toBe(true);
});

Expand Down
28 changes: 16 additions & 12 deletions packages/core/src/inbox/reportMembership.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import type { SignalReport } from "@posthog/shared/types";

/**
* Statuses that are out of the inbox entirely (user-suppressed or removed).
* Statuses that are out of the inbox entirely (user-suppressed, resolved, or
* removed). `resolved` is terminal — its implementation PR merged — so it drops
* out of the live inbox and is surfaced only in the Archive tab for reference.
* `failed` is NOT in here: failed runs surface in the Runs tab's Recently
* finished section so the user can see what went wrong. Other tabs filter
* them out via their own predicates.
*/
export const INBOX_EXCLUDED_STATUSES = new Set<SignalReport["status"]>([
"suppressed",
"resolved",
"deleted",
]);

Expand All @@ -16,19 +19,20 @@ export function isExcludedFromInbox(report: SignalReport): boolean {
}

/**
* Archive tab membership. `suppressed` is the only status that represents "the
* user archived this out of the inbox" — there is no separate `dismissed` /
* `resolved` status in the enum (see `SignalReportStatus`), the archive action
* sets `suppressed`. The other not-in-inbox states are deliberately excluded:
* `deleted` is permanent (gone, never restorable, stripped server-side), and
* snooze is not a status at all — it is a temporary `snoozed_until` timestamp
* on an otherwise-active report that auto-returns to the inbox when it elapses,
* so it doesn't belong in a manual restore list. Archived reports are fetched
* by a dedicated query (the main pipeline query excludes them), so this
* predicate is applied to that separate list.
* Archive tab membership — the two terminal, not-in-inbox states. `suppressed`
* is "the user archived this out of the inbox" (the archive action sets it; it
* is restorable). `resolved` is "the implementation PR merged" — terminal, set
* server-side, shown for reference only and not restorable. The other
* not-in-inbox states are deliberately excluded: `deleted` is permanent (gone,
* never restorable, stripped server-side), and snooze is not a status at all —
* it is a temporary `snoozed_until` timestamp on an otherwise-active report that
* auto-returns to the inbox when it elapses, so it doesn't belong in a manual
* restore list. Archived reports are fetched by a dedicated query (the main
* pipeline query excludes them), so this predicate is applied to that separate
* list.
*/
export function isDismissedReport(report: SignalReport): boolean {
return report.status === "suppressed";
return report.status === "suppressed" || report.status === "resolved";
}

export type InboxScope = "for-you" | "entire-project" | `teammate:${string}`;
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/inbox/reportPresentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ export function inboxStatusLabel(status: SignalReportStatus): string {
switch (status) {
case "ready":
return "Ready";
case "resolved":
return "Resolved";
case "pending_input":
return "Needs input";
case "in_progress":
Expand All @@ -73,6 +75,8 @@ export function inboxStatusAccentCss(status: SignalReportStatus): string {
switch (status) {
case "ready":
return "var(--green-9)";
case "resolved":
return "var(--green-9)";
case "pending_input":
return "var(--violet-9)";
case "in_progress":
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/inbox/statusLabels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ export function inboxStatusLabel(status: SignalReportStatus): string {
switch (status) {
case "ready":
return "Ready";
case "resolved":
return "Resolved";
case "pending_input":
return "Needs input";
case "in_progress":
Expand Down
1 change: 1 addition & 0 deletions packages/shared/src/signal-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export type SignalReportStatus =
| "candidate"
| "in_progress"
| "ready"
| "resolved"
| "failed"
| "pending_input"
| "suppressed"
Expand Down
17 changes: 10 additions & 7 deletions packages/ui/src/features/inbox/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,20 @@ Inbox has four tabs and one reviewer-scope control:
| Pull requests | `/code/inbox/pulls` | Reports with `implementation_pr_url` set |
| Reports | `/code/inbox/reports` | Reports without a PR and not currently running |
| Runs | `/code/inbox/runs` | Reports that are still in progress or waiting on input |
| Archive | `/code/inbox/dismissed` | Reports the user archived/suppressed (`status === "suppressed"`) |
| Archive | `/code/inbox/dismissed` | Terminal reports: archived/suppressed (`status === "suppressed"`) and resolved-by-merged-PR (`status === "resolved"`) |

Detail pages live under the same tab: `/code/inbox/<tab>/$reportId`.

The Archive tab (route `/code/inbox/dismissed`, user-facing label "Archive") is
the exception: suppressed reports are excluded from the
main pipeline query, so the tab fetches them with a dedicated `status=suppressed`
query (`useInboxDismissedReports`). Its detail view (`DismissedReportDetail`) is
read-only — summary + evidence + a single Restore action, no triage affordances —
and depends on the backend serving suppressed reports on the `retrieve`/`signals`
read paths (PostHog/posthog#64019). Restore uses `useInboxRestoreReport`, which
the exception: it holds the two terminal, not-in-inbox states — `suppressed`
(user-archived) and `resolved` (implementation PR merged) — both excluded from
the main pipeline query, so the tab fetches them with a dedicated
`status=suppressed,resolved` query (`useInboxDismissedReports`). Its detail view
(`DismissedReportDetail`) is read-only — summary + evidence, no triage
affordances — and depends on the backend serving these reports on the
`retrieve`/`signals` read paths (PostHog/posthog#64019). Suppressed cards offer a
single Restore action; resolved cards are reference-only (terminal, no restore),
badged "Resolved". Restore uses `useInboxRestoreReport`, which
reuses the `state` action's `potential` ("reopen") transition — the only reopen
path the backend exposes. The reviewer scope control is hidden on this tab since
the archive list is not scoped, and the tab carries no count badge. The
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,17 @@ interface DismissedReportDetailProps {
}

/**
* Detail view for an archived (suppressed) report. Read-only re-read of what the
* report was — summary + evidence — with a single Restore action. No triage
* affordances (archive, discuss, create PR, reviewers): the report is out of the
* pipeline until it's restored.
* Detail view for a terminal report shown in the Archive tab. Read-only re-read
* of what the report was — summary + evidence — with no triage affordances
* (archive, discuss, create PR, reviewers): the report is out of the pipeline.
*
* The gate keeps reports on the route that matches their status: a no-longer-
* suppressed report opened here (stale URL or restored elsewhere) is redirected
* to its current home, so this content only ever renders a suppressed report.
* Suppressed (user-archived) reports offer a single Restore action. Resolved
* (implementation PR merged) reports are reference-only — resolving is terminal,
* so there's nothing to restore and no Restore button is shown.
*
* The gate keeps reports on the route that matches their status: a report that
* is no longer terminal opened here (stale URL or restored elsewhere) is
* redirected to its current home.
*/
export function DismissedReportDetail({
reportId,
Expand All @@ -46,6 +49,9 @@ export function DismissedReportDetail({
}

function DismissedReportDetailContent({ report }: { report: SignalReport }) {
// Resolved reports are terminal (their PR already merged) — nothing to
// restore, so only suppressed reports get a Restore action.
const canRestore = report.status === "suppressed";
return (
<InboxDetailFrame
report={report}
Expand All @@ -55,7 +61,7 @@ function DismissedReportDetailContent({ report }: { report: SignalReport }) {
showDismiss={false}
primaryAction={
<>
<RestoreReportButton report={report} />
{canRestore && <RestoreReportButton report={report} />}
<Button
type="button"
variant="outline"
Expand Down
10 changes: 6 additions & 4 deletions packages/ui/src/features/inbox/components/DismissedTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ import { useInboxRestoreReport } from "@posthog/ui/features/inbox/hooks/useInbox
import { Flex } from "@radix-ui/themes";

/**
* Archive tab: reports the user has archived (suppressed) from the inbox,
* newest first. Each card can be restored back into the pipeline, or opened in
* Archive tab: terminal reports, newest first — ones the user archived
* (suppressed, restorable back into the pipeline) and ones resolved by a merged
* implementation PR (shown for reference only, not restorable). Each card opens
* a read-only detail view (summary + evidence) — no triage affordances.
*/
export function DismissedTab() {
Expand All @@ -40,8 +41,9 @@ export function DismissedTab() {
</EmptyMedia>
<EmptyTitle>No archived reports</EmptyTitle>
<EmptyDescription>
Reports you archive from your inbox show up here. You can restore
any of them back to the inbox.
Reports you archive from your inbox show up here, and you can
restore any of them. Resolved reports (their pull request merged)
also appear here for reference.
</EmptyDescription>
</EmptyHeader>
</Empty>
Expand Down
27 changes: 16 additions & 11 deletions packages/ui/src/features/inbox/components/InboxReportDetailGate.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
isDismissedReport,
isPullRequestReport,
isReportTabReport,
} from "@posthog/core/inbox/reportMembership";
Expand Down Expand Up @@ -70,21 +71,25 @@ export function InboxReportDetailGate({

// Keep the report on the route that matches its status. A status↔route mismatch
// happens when a URL goes stale — browser history, a bookmark, a copied deep
// link, or a status change in another session. A suppressed report reached via a
// /pulls, /reports, or /runs URL would otherwise render that tab's full triage
// actions (archive, discuss, create PR) on an out-of-pipeline report; a restored
// report reached via /dismissed would offer Restore and silently re-queue it
// (READY/RESOLVED → POTENTIAL is an allowed server-side transition). Redirect
// across that dismissed↔pipeline boundary, gated on a settled fetch so we act on
// the confirmed status rather than a pre-change cache snapshot (the detail query
// forces a fresh fetch on mount via `initialDataUpdatedAt: 0`).
// link, or a status change in another session. An archived report (suppressed or
// resolved) reached via a /pulls, /reports, or /runs URL would otherwise render
// that tab's full triage actions (archive, discuss, create PR) on an
// out-of-pipeline report; a restored report reached via /dismissed would offer
// Restore and silently re-queue it (READY/RESOLVED → POTENTIAL is an allowed
// server-side transition). Redirect across that dismissed↔pipeline boundary,
// gated on a settled fetch so we act on the confirmed status rather than a
// pre-change cache snapshot (the detail query forces a fresh fetch on mount via
// `initialDataUpdatedAt: 0`). Both terminal states belong on the Archive route,
// so resolved cards keep their reference-only detail view instead of being
// bounced to Runs.
const onDismissedRoute = backTo === "/code/inbox/dismissed";
const isSuppressed = resolvedReport?.status === "suppressed";
const isArchived =
resolvedReport != null && isDismissedReport(resolvedReport);
Comment thread
andrewm4894 marked this conversation as resolved.
let redirectTo: InboxDetailRoute | null = null;
if (resolvedReport && !isFetching) {
if (isSuppressed && !onDismissedRoute) {
if (isArchived && !onDismissedRoute) {
redirectTo = "/code/inbox/dismissed/$reportId";
} else if (!isSuppressed && onDismissedRoute) {
} else if (!isArchived && onDismissedRoute) {
redirectTo = nonSuppressedDetailRoute(resolvedReport);
}
}
Expand Down
Loading