diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 70f7089b5ed7..94d6136e2eec 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -228,11 +228,11 @@ import type { ColumnConfigurationApi, PaginatedColumnConfigurationListApi, } from 'products/product_analytics/frontend/generated/api.schemas' +import type { SignalReportStateRequestApi } from 'products/signals/frontend/generated/api.schemas' import { SignalReport, SignalReportArtefact, SignalReportArtefactResponse, - SignalReportStateRequest, SignalScoutEmission, SignalScoutEmissionReportLink, SignalScoutRunSummary, @@ -5208,7 +5208,7 @@ const api = { return await new ApiRequest().signalReport(id).withAction('reingest').create() }, // State transitions: suppress (dismiss) or snooze back to potential. Backend: `state` action. - async setState(id: SignalReport['id'], data: SignalReportStateRequest): Promise { + async setState(id: SignalReport['id'], data: SignalReportStateRequestApi): Promise { return await new ApiRequest().signalReport(id).withAction('state').create({ data }) }, // Backend returns a flat `{ [user_uuid]: { name, email } }` map (not paginated). diff --git a/products/customer_analytics/frontend/components/Feed/FeedTabContent.tsx b/products/customer_analytics/frontend/components/Feed/FeedTabContent.tsx index 69e55010be49..5a7b27fdd121 100644 --- a/products/customer_analytics/frontend/components/Feed/FeedTabContent.tsx +++ b/products/customer_analytics/frontend/components/Feed/FeedTabContent.tsx @@ -41,7 +41,7 @@ export function FeedTabContent(): JSX.Element { key={report.id} report={report} backUrl={urls.customerAnalyticsFeed()} - onArchive={(reason, note) => archiveReport(report.id, reason, note)} + onArchive={(dismissal) => archiveReport(report.id, dismissal)} /> ))} diff --git a/products/customer_analytics/frontend/components/Feed/feedLogic.ts b/products/customer_analytics/frontend/components/Feed/feedLogic.ts index 2f771506e243..d43f55786b51 100644 --- a/products/customer_analytics/frontend/components/Feed/feedLogic.ts +++ b/products/customer_analytics/frontend/components/Feed/feedLogic.ts @@ -14,7 +14,7 @@ import { InboxSortField, } from 'products/signals/frontend/inbox/logics/inboxFiltersLogic' import { SignalReport, SignalReportPriority, SignalReportStatus } from 'products/signals/frontend/inbox/types' -import { DismissalReasonValue } from 'products/signals/frontend/inbox/utils/dismissalReasons' +import { DismissalFeedback, suppressDismissalPayload } from 'products/signals/frontend/inbox/utils/dismissalReasons' import type { UserType } from '../../../../../frontend/src/types' @@ -52,17 +52,9 @@ export interface feedLogicValues { export interface feedLogicActions { archiveReport: ( reportId: string, - reason: DismissalReasonValue, - note: string + dismissal: DismissalFeedback ) => { - note: string - reason: - | 'already_fixed' - | 'analysis_wrong' - | 'other' - | 'report_unclear' - | 'wontfix_intentional' - | 'wontfix_irrelevant' + dismissal: DismissalFeedback reportId: string } clearFilters: () => { @@ -156,7 +148,7 @@ export const feedLogic = kea([ toggleScout: (scout: string) => ({ scout }), clearScoutFilter: true, clearFilters: true, - archiveReport: (reportId: string, reason: DismissalReasonValue, note: string) => ({ reportId, reason, note }), + archiveReport: (reportId: string, dismissal: DismissalFeedback) => ({ reportId, dismissal }), }), loaders(({ values }) => ({ reportsResponse: [ @@ -272,12 +264,11 @@ export const feedLogic = kea([ await breakpoint(300) actions.loadReports(null) }, - archiveReport: async ({ reportId, reason, note }) => { + archiveReport: async ({ reportId, dismissal }) => { try { await api.signalReports.setState(reportId, { state: 'suppressed', - dismissal_reason: reason, - ...(note ? { dismissal_note: note } : {}), + ...suppressDismissalPayload(dismissal), }) } catch (error: any) { lemonToast.error(error?.detail || error?.message || 'Failed to archive report') diff --git a/products/desktop/packages/api-client/src/posthog-client.ts b/products/desktop/packages/api-client/src/posthog-client.ts index 24b950452a13..9711c4e1a535 100644 --- a/products/desktop/packages/api-client/src/posthog-client.ts +++ b/products/desktop/packages/api-client/src/posthog-client.ts @@ -4980,6 +4980,8 @@ export class PostHogAPIClient { /** When omitted, the server suppresses without creating a dismissal artefact. */ dismissal_reason?: DismissalReasonOptionValue; dismissal_note?: string; + /** 'owner/repo' the report should have targeted; only allowed with dismissal_reason 'wrong_repo'. */ + corrected_repository?: string; reset_weight?: boolean; error?: string; }, diff --git a/products/desktop/packages/core/src/inbox/bulkActions.ts b/products/desktop/packages/core/src/inbox/bulkActions.ts index 0839bbd9c548..770b192eb959 100644 --- a/products/desktop/packages/core/src/inbox/bulkActions.ts +++ b/products/desktop/packages/core/src/inbox/bulkActions.ts @@ -131,12 +131,15 @@ export function bulkSelectionKey(selection: InboxBulkSelection): string { export interface DismissReportInput { reason: DismissalReasonOptionValue; note: string; + /** 'owner/repo' the reports should have targeted; only set when reason is 'wrong_repo'. */ + correctedRepository?: string | null; } export type SuppressStateRequest = { state: "suppressed"; dismissal_reason?: DismissalReasonOptionValue; dismissal_note?: string; + corrected_repository?: string; }; /** Body for `updateSignalReportState` when suppressing/dismissing. Notes are clamped to 4000 chars. */ @@ -150,6 +153,11 @@ export function buildSuppressRequest( state: "suppressed", dismissal_reason: dismissal.reason, dismissal_note: dismissal.note.slice(0, 4000), + // The API rejects corrected_repository with any other reason, so the gate lives here + // rather than in every caller that builds a DismissReportInput. + ...(dismissal.reason === "wrong_repo" && dismissal.correctedRepository + ? { corrected_repository: dismissal.correctedRepository } + : {}), }; } diff --git a/products/desktop/packages/core/src/inbox/engagement.ts b/products/desktop/packages/core/src/inbox/engagement.ts index c23b86841391..72b184b74bda 100644 --- a/products/desktop/packages/core/src/inbox/engagement.ts +++ b/products/desktop/packages/core/src/inbox/engagement.ts @@ -132,7 +132,11 @@ export interface BuildBulkActionEventsInput { actionType: InboxBulkActionType; surface: InboxReportActionSurface; /** Dismissal metadata, only meaningful for `dismiss`. Note is truncated to 500 chars. */ - dismissal?: { reason?: string; note?: string }; + dismissal?: { + reason?: string; + note?: string; + correctedRepository?: string | null; + }; } /** @@ -168,6 +172,9 @@ export function buildBulkActionEvents( ...(actionType === "dismiss" && dismissal?.note ? { dismissal_note: dismissal.note.slice(0, 500) } : {}), + ...(actionType === "dismiss" && dismissal?.correctedRepository + ? { dismissal_corrected_repository: dismissal.correctedRepository } + : {}), })); } diff --git a/products/desktop/packages/shared/src/analytics-events.ts b/products/desktop/packages/shared/src/analytics-events.ts index ea967e77a67a..5ce5883ec546 100644 --- a/products/desktop/packages/shared/src/analytics-events.ts +++ b/products/desktop/packages/shared/src/analytics-events.ts @@ -774,6 +774,8 @@ export interface InboxReportActionProperties { list_size: number; dismissal_reason?: string; dismissal_note?: string; + // 'owner/repo' correction from a wrong_repo dismissal. + dismissal_corrected_repository?: string; signal_id?: string; signal_source_product?: string; signal_source_type?: string; diff --git a/products/desktop/packages/shared/src/dismissal-reasons.ts b/products/desktop/packages/shared/src/dismissal-reasons.ts index f0c9c4f05e02..ebd9b34d7a1a 100644 --- a/products/desktop/packages/shared/src/dismissal-reasons.ts +++ b/products/desktop/packages/shared/src/dismissal-reasons.ts @@ -16,6 +16,10 @@ export const DISMISSAL_REASON_OPTIONS = [ value: "analysis_wrong", label: "Agent's analysis is wrong", }, + { + value: "wrong_repo", + label: "Agent picked the wrong repository", + }, { value: "wontfix_intentional", label: "Won't fix - intentional behavior", diff --git a/products/desktop/packages/ui/src/features/inbox/components/DismissReportDialog.test.tsx b/products/desktop/packages/ui/src/features/inbox/components/DismissReportDialog.test.tsx new file mode 100644 index 000000000000..a7f68339ce40 --- /dev/null +++ b/products/desktop/packages/ui/src/features/inbox/components/DismissReportDialog.test.tsx @@ -0,0 +1,65 @@ +import type { SignalReport } from "@posthog/shared/types"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { DismissReportDialog } from "./DismissReportDialog"; + +const connectedRepositories = ["posthog/posthog", "posthog/posthog-js"]; + +// Mirrors the real hook's cold-cache behavior: it yields repositories only while enabled +// (the query is gated off when disabled, so the list is empty). Capturing the enabled +// argument is what lets the test assert the fetch is driven by the reason, not the popover. +const useGithubRepositoriesSpy = vi.fn((_search: string, enabled: boolean) => ({ + repositories: enabled ? connectedRepositories : [], + isPending: false, + isFetchingMore: false, + hasMore: false, + loadMore: vi.fn(), +})); + +vi.mock("@posthog/ui/features/integrations/useIntegrations", () => ({ + useIntegrations: vi.fn(), + useGithubRepositories: (search: string, enabled: boolean) => + useGithubRepositoriesSpy(search, enabled), +})); + +vi.mock("@posthog/ui/features/integrations/store", () => ({ + useIntegrationSelectors: () => ({ hasGithubIntegration: true }), +})); + +const report = { title: "Something broke" } as SignalReport; + +describe("DismissReportDialog", () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it("offers an openable repository picker on a cold cache once wrong-repo is chosen", () => { + render( + , + ); + + const wrongRepoRadio = screen + .getByText("Agent picked the wrong repository") + .closest("div") + ?.querySelector("#dismiss-report-dialog-reason-wrong_repo"); + if (!wrongRepoRadio) { + throw new Error("Expected the wrong-repo reason radio to render"); + } + fireEvent.click(wrongRepoRadio); + + // The repositories load on the reason alone, so with the picker still closed it renders + // its openable trigger, not the dead-end disabled "No GitHub repos" button that a + // fetch gated on the popover state would leave the reviewer stuck on. + expect(screen.getByText("Search repositories")).toBeInTheDocument(); + expect(screen.queryByText("No GitHub repos")).not.toBeInTheDocument(); + expect(useGithubRepositoriesSpy).toHaveBeenLastCalledWith("", true); + }); +}); diff --git a/products/desktop/packages/ui/src/features/inbox/components/DismissReportDialog.tsx b/products/desktop/packages/ui/src/features/inbox/components/DismissReportDialog.tsx index 02fabc733ac1..f01988761543 100644 --- a/products/desktop/packages/ui/src/features/inbox/components/DismissReportDialog.tsx +++ b/products/desktop/packages/ui/src/features/inbox/components/DismissReportDialog.tsx @@ -4,10 +4,16 @@ import { isDismissalReasonSnooze, } from "@posthog/shared/dismissalReasons"; import type { SignalReport } from "@posthog/shared/types"; +import { GitHubRepoPicker } from "@posthog/ui/features/folder-picker/GitHubRepoPicker"; import { ExplainedPauseLabel, ExplainedSuppressLabel, } from "@posthog/ui/features/inbox/components/utils/ExplainedDismissOptionLabels"; +import { useIntegrationSelectors } from "@posthog/ui/features/integrations/store"; +import { + useGithubRepositories, + useIntegrations, +} from "@posthog/ui/features/integrations/useIntegrations"; import { Button } from "@posthog/ui/primitives/Button"; import { Dialog, Flex, RadioGroup, Text, TextArea } from "@radix-ui/themes"; import { useEffect, useRef, useState } from "react"; @@ -15,6 +21,8 @@ import { useEffect, useRef, useState } from "react"; export interface DismissReportDialogResult { reason: DismissalReasonOptionValue; note: string; + /** 'owner/repo' the reports should have targeted; only set when reason is 'wrong_repo'. */ + correctedRepository: string | null; } export interface DismissReportDialogProps { @@ -44,10 +52,20 @@ export function DismissReportDialog({ const onOpenChangeRef = useRef(onOpenChange); onOpenChangeRef.current = onOpenChange; + // Quill's combobox portals its popup to document.body, outside Dialog.Content, so Radix + // treats clicks on the popup as "outside the dialog". While the picker is open, every + // dismiss path below must close only the popup — otherwise selecting a repository slams + // the dialog shut and drops the reason and note already entered. + const [isRepoPickerOpen, setIsRepoPickerOpen] = useState(false); + + useEffect(() => { + if (!open) setIsRepoPickerOpen(false); + }, [open]); + // Radix Themes nests Content inside the overlay scroll area, so backdrop clicks // often land on padding/overlay nodes that never reach Content's dismiss layer. useEffect(() => { - if (!open || isSubmitting) return; + if (!open || isSubmitting || isRepoPickerOpen) return; const handlePointerDown = (event: PointerEvent) => { const target = event.target; @@ -67,17 +85,27 @@ export function DismissReportDialog({ document.addEventListener("pointerdown", handlePointerDown, true); return () => document.removeEventListener("pointerdown", handlePointerDown, true); - }, [open, isSubmitting]); + }, [open, isSubmitting, isRepoPickerOpen]); return ( { - if (!isSubmitting) onOpenChange(false); + onPointerDownOutside={(event) => { + // preventDefault, not just skipping onOpenChange: Radix also dismisses a controlled + // dialog through Root's own onOpenChange unless the event is cancelled. + if (isSubmitting || isRepoPickerOpen) { + event.preventDefault(); + return; + } + onOpenChange(false); }} - onEscapeKeyDown={() => { - if (!isSubmitting) onOpenChange(false); + onEscapeKeyDown={(event) => { + if (isSubmitting || isRepoPickerOpen) { + event.preventDefault(); + return; + } + onOpenChange(false); }} > @@ -98,15 +128,38 @@ function DismissReportDialogBody({ isSubmitting, snoozeDisabledReason, onConfirm, + isRepoPickerOpen, + onRepoPickerOpenChange, }: Omit & { selectedCount: number; + /** Owned by the dialog wrapper, which suppresses its dismiss paths while the picker is open. */ + isRepoPickerOpen: boolean; + onRepoPickerOpenChange: (open: boolean) => void; }) { const [reason, setReason] = useState(null); const [note, setNote] = useState(""); + const [correctedRepository, setCorrectedRepository] = useState( + null, + ); + const [repoSearch, setRepoSearch] = useState(""); + + const isWrongRepo = reason === "wrong_repo"; + // Populates the integration store in case no other surface loaded it yet; react-query dedupes. + useIntegrations(); + const { hasGithubIntegration } = useIntegrationSelectors(); + // Enabled on the reason alone, not on isRepoPickerOpen: when the list is empty the picker + // renders a disabled "No GitHub repos" trigger that cannot be opened, so gating the fetch on + // the open state would deadlock on a cold cache (never open -> never fetch -> never open). + const repoPage = useGithubRepositories(repoSearch, isWrongRepo); const handleConfirm = () => { if (!reason) return; - onConfirm({ reason, note: note.trim() }); + onConfirm({ + reason, + note: note.trim(), + // A correction picked and then abandoned for another reason must not ride along. + correctedRepository: isWrongRepo ? correctedRepository : null, + }); }; const alreadyFixedDisabled = snoozeDisabledReason !== null; @@ -160,6 +213,34 @@ function DismissReportDialogBody({ + {isWrongRepo && hasGithubIntegration ? ( +
+ + Which repository should it have been? + + + + Optional. The agent uses your correction when picking repositories + in the future. + +
+ ) : null} +