-
Notifications
You must be signed in to change notification settings - Fork 44
feat(inbox): track INBOX_REPORT_ACTION for bulk dismiss/snooze/delete/reingest #2750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import type { | ||
| InboxReportActionProperties, | ||
| InboxReportActionSurface, | ||
| InboxViewedProperties, | ||
| } from "@posthog/shared/analytics-events"; | ||
| import type { SignalReport } from "@posthog/shared/domain-types"; | ||
|
|
@@ -119,6 +120,56 @@ export function resolveActionProperties( | |
| return { rank, list_size: listSize, priority, actionability }; | ||
| } | ||
|
|
||
| /** Bulk-capable report actions fired from the selection toolbar / dismiss flows. */ | ||
| export type InboxBulkActionType = Extract< | ||
| InboxReportActionProperties["action_type"], | ||
| "dismiss" | "snooze" | "delete" | "reingest" | ||
| >; | ||
|
|
||
| export interface BuildBulkActionEventsInput { | ||
| /** Reports the action actually succeeded for (one event is built per report). */ | ||
| reports: SignalReport[]; | ||
| actionType: InboxBulkActionType; | ||
| surface: InboxReportActionSurface; | ||
| /** Dismissal metadata, only meaningful for `dismiss`. Note is truncated to 500 chars. */ | ||
| dismissal?: { reason?: string; note?: string }; | ||
| } | ||
|
|
||
| /** | ||
| * Build `INBOX_REPORT_ACTION` payloads for a bulk (or single-report) dismiss / | ||
| * snooze / delete / reingest. Pure so it can be unit-tested and reused across | ||
| * the toolbar, the per-row dismiss action, and detail-screen dismiss. | ||
| * | ||
| * `is_bulk` / `bulk_size` carry the grouping; `rank` / `list_size` are left at 0 | ||
| * because these flows act on a selection, not a positional list slot. | ||
| */ | ||
| export function buildBulkActionEvents( | ||
| input: BuildBulkActionEventsInput, | ||
| ): InboxReportActionProperties[] { | ||
| const { reports, actionType, surface, dismissal } = input; | ||
| const bulkSize = reports.length; | ||
| const isBulk = bulkSize > 1; | ||
|
Comment on lines
+150
to
+151
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a user selects multiple reports and only one succeeds, Useful? React with 👍 / 👎. |
||
| return reports.map((report) => ({ | ||
| report_id: report.id, | ||
| report_title: report.title ?? null, | ||
| report_age_hours: reportAgeHours(report.created_at), | ||
| priority: report.priority ?? null, | ||
| actionability: report.actionability ?? null, | ||
| action_type: actionType, | ||
| surface, | ||
| is_bulk: isBulk, | ||
| bulk_size: bulkSize, | ||
| rank: 0, | ||
| list_size: 0, | ||
| ...(actionType === "dismiss" && dismissal?.reason | ||
| ? { dismissal_reason: dismissal.reason } | ||
| : {}), | ||
| ...(actionType === "dismiss" && dismissal?.note | ||
| ? { dismissal_note: dismissal.note.slice(0, 500) } | ||
| : {}), | ||
| })); | ||
| } | ||
|
|
||
| export interface InboxViewedFilterState { | ||
| sourceProductFilter: string[]; | ||
| priorityFilter: string[]; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,16 @@ | ||
| import { | ||
| buildBulkActionEvents, | ||
| type InboxBulkActionType, | ||
| } from "@posthog/core/inbox/engagement"; | ||
| import { inboxStatusLabel } from "@posthog/core/inbox/reportPresentation"; | ||
| import type { InboxReportActionSurface } from "@posthog/shared/analytics-events"; | ||
| import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events"; | ||
| import type { SignalReport } from "@posthog/shared/types"; | ||
| import type { DismissReportDialogResult } from "@posthog/ui/features/inbox/components/DismissReportDialog"; | ||
| import { reportKeys } from "@posthog/ui/features/inbox/hooks/useInboxReports"; | ||
| import { useInboxReportSelectionStore } from "@posthog/ui/features/inbox/stores/inboxReportSelectionStore"; | ||
| import { useAuthenticatedMutation } from "@posthog/ui/hooks/useAuthenticatedMutation"; | ||
| import { track } from "@posthog/ui/shell/analytics"; | ||
| import { useQueryClient } from "@tanstack/react-query"; | ||
| import { useCallback, useMemo } from "react"; | ||
| import { toast } from "sonner"; | ||
|
|
@@ -187,6 +194,7 @@ export function buildSuppressDisabledReasonMap( | |
| export function useInboxBulkActions( | ||
| reports: SignalReport[], | ||
| selection: InboxBulkSelection, | ||
| surface: InboxReportActionSurface = "toolbar", | ||
| ) { | ||
| const queryClient = useQueryClient(); | ||
| const clearSelection = useInboxReportSelectionStore( | ||
|
|
@@ -196,6 +204,38 @@ export function useInboxBulkActions( | |
| (state) => state.removeFromSelection, | ||
| ); | ||
|
|
||
| /** | ||
| * Emit one `INBOX_REPORT_ACTION` per report the action succeeded for. Resolves | ||
| * report metadata from the pre-mutation list (the query is invalidated right | ||
| * after, dropping the affected reports), and skips firing when nothing landed. | ||
| */ | ||
| const trackBulkAction = useCallback( | ||
| ( | ||
| actionType: InboxBulkActionType, | ||
| result: BulkActionResult, | ||
| dismissal?: DismissReportDialogResult, | ||
| ) => { | ||
| if (result.successCount === 0) return; | ||
| const byId = new Map(reports.map((report) => [report.id, report])); | ||
| const succeeded = result.succeededIds | ||
| .map((id) => byId.get(id)) | ||
| .filter((report): report is SignalReport => report !== undefined); | ||
|
Comment on lines
+219
to
+222
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because the inbox list polls every 3 seconds, a longer bulk mutation can succeed for some reports and then refetch them out of Useful? React with 👍 / 👎. |
||
| if (succeeded.length === 0) return; | ||
| const events = buildBulkActionEvents({ | ||
| reports: succeeded, | ||
| actionType, | ||
| surface, | ||
| dismissal: dismissal | ||
| ? { reason: dismissal.reason, note: dismissal.note } | ||
| : undefined, | ||
| }); | ||
| for (const event of events) { | ||
| track(ANALYTICS_EVENTS.INBOX_REPORT_ACTION, event); | ||
| } | ||
| }, | ||
| [reports, surface], | ||
| ); | ||
|
|
||
| /** | ||
| * Reflect a bulk-action result in the selection: drop succeeded ids so the | ||
| * user can retry the failed subset; keep everything if nothing succeeded so | ||
|
|
@@ -247,7 +287,8 @@ export function useInboxBulkActions( | |
| ); | ||
| }, | ||
| { | ||
| onSuccess: async (result) => { | ||
| onSuccess: async (result, variables) => { | ||
| trackBulkAction("dismiss", result, variables.dismissal); | ||
| await invalidateInboxQueries(); | ||
| applyBulkResultToSelection(result); | ||
|
|
||
|
|
@@ -274,6 +315,7 @@ export function useInboxBulkActions( | |
| ), | ||
| { | ||
| onSuccess: async (result) => { | ||
| trackBulkAction("snooze", result); | ||
| await invalidateInboxQueries(); | ||
| applyBulkResultToSelection(result); | ||
|
|
||
|
|
@@ -297,6 +339,7 @@ export function useInboxBulkActions( | |
| ), | ||
| { | ||
| onSuccess: async (result) => { | ||
| trackBulkAction("delete", result); | ||
| await invalidateInboxQueries(); | ||
| applyBulkResultToSelection(result); | ||
|
|
||
|
|
@@ -320,6 +363,7 @@ export function useInboxBulkActions( | |
| ), | ||
| { | ||
| onSuccess: async (result) => { | ||
| trackBulkAction("reingest", result); | ||
| await invalidateInboxQueries(); | ||
| applyBulkResultToSelection(result); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The third test conflates "dismiss attaches metadata and truncates the note" with "non-dismiss action type drops the metadata" into a single
it(), even though the rest of the file already usesit.eachfor exactly this pattern (see thehas_active_filterstable below). Splitting these into a parameterised row-per-action-type table would make it immediately obvious which action types should and shouldn't carry dismissal fields, and makes it trivial to adddelete/reingestcoverage later.Context Used: Do not attempt to comment on incorrect alphabetica... (source)
Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!