Skip to content

Commit 3c229d1

Browse files
authored
fix(inbox): show true tab counts and hide zero counts (#2714)
1 parent 6e1d3f5 commit 3c229d1

12 files changed

Lines changed: 142 additions & 64 deletions

File tree

packages/api-client/src/posthog-client.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2737,6 +2737,12 @@ export class PostHogAPIClient {
27372737
if (params?.priority) {
27382738
url.searchParams.set("priority", params.priority);
27392739
}
2740+
if (params?.has_implementation_pr != null) {
2741+
url.searchParams.set(
2742+
"has_implementation_pr",
2743+
String(params.has_implementation_pr),
2744+
);
2745+
}
27402746

27412747
const response = await this.api.fetcher.fetch({
27422748
method: "get",

packages/core/src/inbox/reportFiltering.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,6 @@ function normalizeReviewerId(value: string): string {
1919
return value.trim();
2020
}
2121

22-
/**
23-
* Reports that are surfaced to the current user as needing review: ready,
24-
* immediately actionable, and addressed to them. Used for both the sidebar
25-
* red badge count and the inbox toolbar "up for review" byline so the two
26-
* numbers always agree.
27-
*/
28-
export function isReportUpForReview(report: SignalReport): boolean {
29-
return (
30-
report.status === "ready" &&
31-
report.is_suggested_reviewer === true &&
32-
report.actionability === "immediately_actionable"
33-
);
34-
}
35-
3622
export function filterReportsBySearch(
3723
reports: SignalReport[],
3824
query: string,

packages/shared/src/domain-types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,11 @@ export interface SignalReportsQueryParams {
505505
suggested_reviewers?: string;
506506
/** Comma-separated `P0`–`P4` priorities — only returns reports with one of these priorities. */
507507
priority?: string;
508+
/**
509+
* Filter by whether a shipped implementation pull request exists. `true` keeps only PR
510+
* reports, `false` only non-PR reports. Pair with `limit: 1` to count PR reports cheaply.
511+
*/
512+
has_implementation_pr?: boolean;
508513
}
509514

510515
/** Values match `SignalReportTask.Relationship` on the PostHog API. */

packages/ui/src/features/home/components/HomeView.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,11 @@ function Section({ title, count, icon, children }: SectionProps) {
286286
>
287287
{icon}
288288
<Text className="font-semibold text-[12px] text-gray-12">{title}</Text>
289-
<Text className="rounded-full bg-(--gray-a3) px-1.5 py-px font-medium text-(--gray-11) text-[10.5px] tabular-nums">
290-
{count}
291-
</Text>
289+
{count > 0 && (
290+
<Text className="rounded-full bg-(--gray-a3) px-1.5 py-px font-medium text-(--gray-11) text-[10.5px] tabular-nums">
291+
{count}
292+
</Text>
293+
)}
292294
</Flex>
293295
{children}
294296
</Box>

packages/ui/src/features/inbox/components/InboxReportListTab.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ interface InboxReportListTabProps {
7373
reports: SignalReport[];
7474
children: ReactNode;
7575
}>;
76+
/**
77+
* Fetch a server-filtered PR-only list instead of the broad pipeline list, so
78+
* the tab body comes from the same source as the Pull-requests count (a PR
79+
* past the broad list's first page would otherwise not render).
80+
*/
81+
pullRequestsOnly?: boolean;
7682
}
7783

7884
/**
@@ -89,8 +95,11 @@ export function InboxReportListTab({
8995
searchPlaceholder,
9096
emptyState,
9197
CardListWrapper,
98+
pullRequestsOnly = false,
9299
}: InboxReportListTabProps) {
93-
const { scopedReports, allReports, isLoading } = useInboxAllReports();
100+
const { scopedReports, allReports, isLoading } = useInboxAllReports({
101+
pullRequestsOnly,
102+
});
94103
const scope = useInboxReviewerScopeStore((s) => s.scope);
95104
const [dismissReport, setDismissReport] = useState<SignalReport | null>(null);
96105

packages/ui/src/features/inbox/components/InboxTabBar.tsx

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,17 @@ export function InboxTabBar({ counts }: InboxTabBarProps) {
4949
<span className="font-medium text-[13px]">
5050
{INBOX_TAB_LABEL[key]}
5151
</span>
52-
<span
53-
className={
54-
isActive
55-
? "text-[12px] text-gray-11 tabular-nums"
56-
: "text-[12px] text-gray-10 tabular-nums"
57-
}
58-
>
59-
{counts[key]}
60-
</span>
52+
{counts[key] > 0 && (
53+
<span
54+
className={
55+
isActive
56+
? "text-[12px] text-gray-11 tabular-nums"
57+
: "text-[12px] text-gray-10 tabular-nums"
58+
}
59+
>
60+
{counts[key]}
61+
</span>
62+
)}
6163
</TabsTrigger>
6264
);
6365
})}

packages/ui/src/features/inbox/components/PullRequestsTab.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export function PullRequestsTab() {
1414
return (
1515
<InboxReportListTab
1616
predicate={isPullRequestReport}
17+
pullRequestsOnly
1718
Card={PullRequestCard}
1819
CardListWrapper={PullRequestsBatchProvider}
1920
searchPlaceholder="Search pull requests…"

packages/ui/src/features/inbox/components/RunsTab.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,11 @@ function RunsSection({
187187
<Text className="font-semibold text-[13px] text-gray-12">
188188
{title}
189189
</Text>
190-
<Text className="text-[12px] text-gray-10 tabular-nums">{count}</Text>
190+
{count > 0 && (
191+
<Text className="text-[12px] text-gray-10 tabular-nums">
192+
{count}
193+
</Text>
194+
)}
191195
{isLive && count > 0 && (
192196
<span
193197
className="inline-flex h-1.5 w-1.5 animate-pulse rounded-full bg-(--blue-9)"

packages/ui/src/features/inbox/hooks/useInboxAllReports.ts

Lines changed: 71 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,18 @@ import {
99
import {
1010
computeInboxTabCounts,
1111
INBOX_SCOPE_FOR_YOU,
12+
isExcludedFromInbox,
13+
isPullRequestReport,
14+
isReportTabReport,
1215
matchesReviewerScope,
1316
parseTeammateInboxScope,
1417
} from "@posthog/core/inbox/reportMembership";
1518
import { useOptionalAuthenticatedClient } from "@posthog/ui/features/auth/authClient";
1619
import { useCurrentUser } from "@posthog/ui/features/auth/useCurrentUser";
17-
import { useInboxReportsInfinite } from "@posthog/ui/features/inbox/hooks/useInboxReports";
20+
import {
21+
useInboxReports,
22+
useInboxReportsInfinite,
23+
} from "@posthog/ui/features/inbox/hooks/useInboxReports";
1824
import { useInboxReviewerScopeStore } from "@posthog/ui/features/inbox/stores/inboxReviewerScopeStore";
1925
import { useInboxSignalsFilterStore } from "@posthog/ui/features/inbox/stores/inboxSignalsFilterStore";
2026
import { useMemo } from "react";
@@ -36,9 +42,15 @@ const EMPTY_FILTER_ARRAY: never[] = [];
3642
export function useInboxAllReports(options?: {
3743
ignoreScope?: boolean;
3844
ignoreFilters?: boolean;
45+
pullRequestsOnly?: boolean;
3946
}) {
4047
const ignoreScope = options?.ignoreScope ?? false;
4148
const ignoreFilters = options?.ignoreFilters ?? false;
49+
// The Pull requests tab fetches a server-filtered list (reports that have a
50+
// shipped PR) so its list body comes from the same source as its count — a PR
51+
// sitting past the broad list's first page no longer renders an empty tab
52+
// under a positive badge.
53+
const pullRequestsOnly = options?.pullRequestsOnly ?? false;
4254
const scope = useInboxReviewerScopeStore((s) => s.scope);
4355
const searchQuery = useInboxSignalsFilterStore((s) =>
4456
ignoreFilters ? "" : s.searchQuery,
@@ -69,6 +81,7 @@ export function useInboxAllReports(options?: {
6981
const query = useInboxReportsInfinite(
7082
{
7183
status: INBOX_PIPELINE_STATUS_FILTER,
84+
has_implementation_pr: pullRequestsOnly ? true : undefined,
7285
ordering: buildSignalReportListOrdering(sortField, sortDirection),
7386
source_product:
7487
sourceProductFilter.length > 0
@@ -90,6 +103,36 @@ export function useInboxAllReports(options?: {
90103
},
91104
);
92105

106+
// True count of pull-request reports for the active scope. The infinite list
107+
// only holds the first page(s), so deriving pulls from loaded reports caps at
108+
// the page size and depends on ordering (a PR can sit past page 1). A cheap
109+
// `limit: 1` count query with the server-side `has_implementation_pr` filter
110+
// returns the real total regardless of page size.
111+
const pullRequestCountQuery = useInboxReports(
112+
{
113+
status: INBOX_PIPELINE_STATUS_FILTER,
114+
has_implementation_pr: true,
115+
// Mirror the list query's active filters so the badge matches the tab
116+
// body. These are empty when `ignoreFilters` is set (sidebar usage), so
117+
// the count stays scope-only there.
118+
source_product:
119+
sourceProductFilter.length > 0
120+
? sourceProductFilter.join(",")
121+
: undefined,
122+
priority: buildPriorityFilterParam(priorityFilter),
123+
suggested_reviewers: reviewerUuid
124+
? buildSuggestedReviewerFilterParam([reviewerUuid])
125+
: undefined,
126+
limit: 1,
127+
},
128+
{
129+
enabled: !isForYou || reviewerUuid != null,
130+
refetchInterval: INBOX_REFETCH_INTERVAL_MS,
131+
refetchIntervalInBackground: false,
132+
},
133+
);
134+
const pullRequestTotal = pullRequestCountQuery.data?.count ?? 0;
135+
93136
const scopedReports = useMemo(() => {
94137
const byScope = ignoreScope
95138
? query.allReports
@@ -99,10 +142,33 @@ export function useInboxAllReports(options?: {
99142
: byScope;
100143
}, [query.allReports, scope, searchQuery, ignoreScope]);
101144

102-
const counts = useMemo(
103-
() => computeInboxTabCounts(query.allReports, scope),
104-
[query.allReports, scope],
105-
);
145+
const counts = useMemo(() => {
146+
const loaded = computeInboxTabCounts(query.allReports, scope);
147+
// The list is an infinite query that only holds the pages loaded so far
148+
// (100 per page), so the loaded-derived Reports count caps at the page size
149+
// and reads as a misleading "100". Reports is the dominant bucket, so derive
150+
// its true size from the backend total `count` (unaffected by the page cap)
151+
// minus the non-report items the total also includes. PRs use the true
152+
// `pullRequestTotal` (also a real backend count), so PRs sitting past the
153+
// loaded page don't inflate Reports. Runs/failed without a PR stay
154+
// loaded-derived — small, newest-first, and an accepted approximation.
155+
const loadedOtherNonReport = query.allReports.filter(
156+
(r) =>
157+
matchesReviewerScope(r, scope) &&
158+
!isExcludedFromInbox(r) &&
159+
!isReportTabReport(r) &&
160+
!isPullRequestReport(r),
161+
).length;
162+
return {
163+
...loaded,
164+
// True backend counts, unaffected by the list's page-size cap.
165+
pulls: pullRequestTotal,
166+
reports: Math.max(
167+
0,
168+
query.totalCount - pullRequestTotal - loadedOtherNonReport,
169+
),
170+
};
171+
}, [query.allReports, query.totalCount, scope, pullRequestTotal]);
106172

107173
return {
108174
...query,

packages/ui/src/features/inbox/hooks/useReportOpenTracker.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,20 @@ export function useReportOpenTracker(
5050
report: SignalReport,
5151
tab: InboxDetailTab,
5252
): void {
53-
// The Pull requests / Reports tabs render the scoped+filtered list; the Runs
54-
// tab is project-wide. Mount only the query matching the originating tab so
55-
// rank is relative to the rows the user actually saw (and so a non-run detail
56-
// doesn't start the unused project-wide poll alongside the scoped one).
53+
// Mount only the query matching the originating tab so rank is relative to the
54+
// rows the user actually saw (and so a non-run detail doesn't start the unused
55+
// project-wide poll alongside the scoped one):
56+
// - Runs tab is project-wide (ignoreScope/ignoreFilters).
57+
// - Pull requests tab renders the server-filtered PR-only list, so mirror
58+
// that here — otherwise a PR past the broad list's first page would get
59+
// rank -1 and a list_size from the broad list, not the rows shown.
60+
// - Reports tab renders the scoped+filtered broad list.
5761
const { scopedReports } = useInboxAllReports(
58-
tab === "runs" ? { ignoreScope: true, ignoreFilters: true } : undefined,
62+
tab === "runs"
63+
? { ignoreScope: true, ignoreFilters: true }
64+
: tab === "pulls"
65+
? { pullRequestsOnly: true }
66+
: undefined,
5967
);
6068
const visible = inboxDetailTabReports(tab, scopedReports);
6169

0 commit comments

Comments
 (0)