From e571b943f515d48b6e6c7fd1d750f9e969201fe9 Mon Sep 17 00:00:00 2001 From: Pavlo Shylo Date: Tue, 14 Jul 2026 16:00:28 +0100 Subject: [PATCH] fix(tickets): restore dashboard-activity events for remote actions & resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three ticket-detail activity events were broken: - open_remote_shell_ticket_detail / open_remote_control_ticket_detail stopped firing (the tracking relied on the core ActionsMenu invoking onClick on an href row — a fragile implementation detail). - resolve_ticket_ticket_detail never fired: the RESOLVE_TICKET subtype was defined but wired to a handleResolve that never existed. Resolve actually happens through the inline status changer (and the Edit form), neither of which tracked anything. Fixes: - withActivityTracking now drops the item href and drives navigation itself (trackDashboardActivity + router.push) inside onClick, forcing the reliable button-row path whose onClick always fires. Windows Remote Shell submenu leaves are handled by the existing recursion. - Resolve is tracked optimistically on click (not on mutation success) at every in-detail resolve entry point: the status changer and the Edit Ticket form. A new isResolvedStatusId() predicate (keyed on status kind, so custom-named resolved statuses count) is the single source of truth shared by both. Board drag-to-Resolved is intentionally NOT tracked (different surface — would need its own subtype). Docs (dashboard-activity-events.md) updated to match. --- docs/dashboard-activity-events.md | 34 +++++++++----- .../components/ticket-details-view.tsx | 47 +++++++++++++++---- .../tickets/hooks/use-create-ticket-form.ts | 8 ++++ .../(app)/tickets/utils/is-resolved-status.ts | 20 ++++++++ 4 files changed, 87 insertions(+), 22 deletions(-) create mode 100644 src/app/(app)/tickets/utils/is-resolved-status.ts diff --git a/docs/dashboard-activity-events.md b/docs/dashboard-activity-events.md index cc89d75..e466506 100644 --- a/docs/dashboard-activity-events.md +++ b/docs/dashboard-activity-events.md @@ -53,8 +53,13 @@ centralized in `EVENT_SUBTYPE`, so renaming later is a one‑file change. ## Fire‑timing policy - **Navigation/intent buttons** (no meaningful success state) → fire on click. -- **Mutation buttons** (send/approve/reject/resolve/start) → fire on **success** - only, so failed/cancelled attempts don't pollute activation metrics. +- **Mutation buttons** (send/approve/reject/start) → fire on **success** only, so + failed/cancelled attempts don't pollute activation metrics. +- **Resolve ticket** → fire **optimistically on click** (when a RESOLVED-kind + status is chosen), *not* on success. A resolve can happen via two mutations + (the status changer and the Edit form), and success callbacks proved + unreliable to observe; losing one event on a rare failed transition is + acceptable, missing real resolves is not. - **Skip onboarding** → fire when the walkthrough is dismissed. ## Tracked events @@ -67,9 +72,9 @@ centralized in `EVENT_SUBTYPE`, so renaming later is a one‑file change. | 4 | Send Mingo message | `SEND_MINGO_MESSAGE` | `send_mingo_message_mingo` | mingo | `mingo/page.tsx` → `handleSendMessage` | send **success** (`success === true`, both draft & existing branches) | | 5 | Approve Mingo command | `APPROVE_MINGO_COMMAND` | `approve_mingo_command_mingo` | mingo | `mingo/hooks/use-mingo-dialog-selection.ts` → `handleApprove` | approve mutation **success** | | 6 | Reject Mingo command *(added per request)* | `REJECT_MINGO_COMMAND` | `reject_mingo_command_mingo` | mingo | `mingo/hooks/use-mingo-dialog-selection.ts` → `handleReject` | reject mutation **success** | -| 7 | Open Remote Shell | `OPEN_REMOTE_SHELL` | `open_remote_shell_ticket_detail` | ticket detail | `tickets/components/ticket-details-view.tsx` → `menuActions` memo wraps the `remoteShell` item (and its Windows submenu leaves) with an `onClick` | menu item click | -| 8 | Open Remote Control | `OPEN_REMOTE_CONTROL` | `open_remote_control_ticket_detail` | ticket detail | `tickets/components/ticket-details-view.tsx` → `menuActions` memo wraps the `remoteControl` item with an `onClick` | menu item click | -| 9 | Resolve ticket | `RESOLVE_TICKET` | `resolve_ticket_ticket_detail` | ticket detail | `tickets/components/ticket-details-view.tsx` → `handleResolve` | resolve **success** (`nextStatus` truthy) | +| 7 | Open Remote Shell | `OPEN_REMOTE_SHELL` | `open_remote_shell_ticket_detail` | ticket detail | `tickets/components/ticket-details-view.tsx` → `menuActions` memo wraps the `remoteShell` item (and its Windows submenu leaves) via `withActivityTracking` | menu item click | +| 8 | Open Remote Control | `OPEN_REMOTE_CONTROL` | `open_remote_control_ticket_detail` | ticket detail | `tickets/components/ticket-details-view.tsx` → `menuActions` memo wraps the `remoteControl` item via `withActivityTracking` | menu item click | +| 9 | Resolve ticket | `RESOLVE_TICKET` | `resolve_ticket_ticket_detail` | ticket detail | `handleTransition` (detail-view status changer) **and** `useCreateTicketForm.handleSave` (Edit Ticket form), both via `isResolvedStatusId` | click, when target status kind is RESOLVED (optimistic, before the mutation) | | 10 | Start Direct Chat | `START_DIRECT_CHAT` | `start_direct_chat_ticket_detail` | ticket detail | `tickets/hooks/use-direct-chat.ts` → `createDialogMutation.onSuccess` | dialog create **success** | > The Approve/Reject buttons themselves and the onboarding buttons render inside @@ -81,13 +86,18 @@ centralized in `EVENT_SUBTYPE`, so renaming later is a one‑file change. ### Remote Shell / Remote Control nuance `buildDeviceMenuItems` (`devices/utils/device-menu-items.tsx`) returns -`ActionsMenuItem`s that navigate via `href`. `ActionsMenuItem` supports `href` -**and** `onClick` together, so in the `menuActions` memo we shallow‑wrap the -built `remoteShell` / `remoteControl` items adding an `onClick` that calls -`trackDashboardActivity(...)` while leaving `href` untouched. On Windows, -`remoteShell` is a `type: 'submenu'` whose leaf children carry the `href`; the -wrapper maps those leaves too so the event fires on the actual shell choice. -`iconAction` (open‑in‑new‑tab secondary button) is left as‑is. +`ActionsMenuItem`s that navigate via `href`, which the core `ActionsMenu` +renders as a ``. Whether a link row also invokes the item's `onClick` is a +core‑lib implementation detail we must **not** let the analytics silently depend +on — it regressed once and dropped these two events entirely. So +`withActivityTracking` (in `ticket-details-view.tsx`) **drops `href`** and drives +navigation itself: the wrapped `onClick` calls `trackDashboardActivity(...)` then +`router.push(href)`. That forces the reliable button‑row path whose `onClick` +always fires. On Windows, `remoteShell` is a `type: 'submenu'` whose leaf +children carry the `href`; the wrapper recurses into those leaves so the event +fires on the actual shell choice. Trade‑off: the row is no longer a native anchor, +so cmd/middle‑click "open in new tab" on the row is gone (the ticket menu has no +`iconAction` new‑tab button anyway). ## Non‑functional requirements diff --git a/src/app/(app)/tickets/components/ticket-details-view.tsx b/src/app/(app)/tickets/components/ticket-details-view.tsx index 9ad91f3..976e085 100644 --- a/src/app/(app)/tickets/components/ticket-details-view.tsx +++ b/src/app/(app)/tickets/components/ticket-details-view.tsx @@ -87,8 +87,10 @@ import { useAddTicketNote, useDeleteTicketNote, useUpdateTicketNote } from '../h import { useAssigneeOptions } from '../hooks/use-ticket-options'; import { useTicketStatus } from '../hooks/use-ticket-status'; import { useTransitionTicket } from '../hooks/use-transition-ticket'; +import { useTicketStatusesQuery } from '../statuses/hooks/use-ticket-statuses-query'; import { useTicketDetailsStore } from '../stores/ticket-details-store'; import type { ClientDialogOwner, Dialog, DialogOwner } from '../types/dialog.types'; +import { isResolvedStatusId } from '../utils/is-resolved-status'; import { ticketsQueryKeys } from '../utils/query-keys'; import { TICKET_STATUS_KIND } from '../utils/ticket-statistics'; import { TicketAttachmentsSection } from './ticket-attachments-section'; @@ -103,20 +105,32 @@ interface TicketDetailsViewProps { /** * Wrap a device-menu item so opening it also fires a dashboard-activity event. - * `href` navigation is preserved. For a submenu parent the click only expands - * the submenu, so tracking is attached to the leaf items that actually - * navigate, not the parent. + * + * The built device items navigate via `href`, which the core `ActionsMenu` + * renders as a ``. Whether a link row also invokes the item's `onClick` + * is a core-lib implementation detail we must not let the analytics silently + * depend on (it regressed once, dropping open_remote_shell/open_remote_control + * events entirely). So instead of attaching an `onClick` alongside `href`, we + * drop `href` and drive navigation ourselves inside `onClick` (track + + * `navigate`). That forces the reliable button-row path whose `onClick` always + * fires. For a submenu parent the click only expands the submenu, so we recurse + * into the leaf items that actually navigate, not the parent. */ -function withActivityTracking(item: ActionsMenuItem, subtype: EventSubtype): ActionsMenuItem { +function withActivityTracking( + item: ActionsMenuItem, + subtype: EventSubtype, + navigate: (href: string) => void, +): ActionsMenuItem { if (item.submenu && item.submenu.length > 0) { - return { ...item, submenu: item.submenu.map(child => withActivityTracking(child, subtype)) }; + return { ...item, submenu: item.submenu.map(child => withActivityTracking(child, subtype, navigate)) }; } - const originalOnClick = item.onClick; + const { href, onClick: originalOnClick, ...rest } = item; return { - ...item, + ...rest, onClick: () => { trackDashboardActivity(subtype); originalOnClick?.(); + if (href) navigate(href); }, }; } @@ -252,6 +266,9 @@ export function TicketDetailsView({ ticketId }: TicketDetailsViewProps) { const { activate, archive, isUpdating } = useTicketStatus(); const transitionTicket = useTransitionTicket(); + // Target-status kinds, to recognize a resolve at click time (availableTransitions + // carries no `kind`). Cached/shared with the board & table, so no extra fetch. + const { data: statusesData } = useTicketStatusesQuery(); const { handleApproveRequest, handleRejectRequest } = useApprovalRequests(); // Time tracker lives in a global host provider (mounted when the feature flag @@ -407,9 +424,17 @@ export function TicketDetailsView({ ticketId }: TicketDetailsViewProps) { const handleTransition = useCallback( (toStatusId: string) => { if (!dialog || transitionTicket.isPending) return; + // Resolve is the inline status changer moving the ticket into a + // RESOLVED-kind status — there is no dedicated "resolve" button. Track + // optimistically on click (like the other activity events): losing one + // event on a failed transition is fine; missing real resolves because a + // success callback didn't land is not. + if (isResolvedStatusId(toStatusId, statusesData?.snapshot)) { + trackDashboardActivity(EVENT_SUBTYPE.RESOLVE_TICKET); + } transitionTicket.mutate({ ticketId, toStatusId }); }, - [dialog, ticketId, transitionTicket], + [dialog, ticketId, transitionTicket, statusesData], ); const handleApprovalAction = useCallback( @@ -542,8 +567,10 @@ export function TicketDetailsView({ ticketId }: TicketDetailsViewProps) { if (deviceDetails || isDeviceLoading) { infoItems.push(deviceMenuItems.deviceDetails, deviceMenuItems.deviceLogs); remoteItems.push( - withActivityTracking(deviceMenuItems.remoteShell, EVENT_SUBTYPE.OPEN_REMOTE_SHELL), - withActivityTracking(deviceMenuItems.remoteControl, EVENT_SUBTYPE.OPEN_REMOTE_CONTROL), + withActivityTracking(deviceMenuItems.remoteShell, EVENT_SUBTYPE.OPEN_REMOTE_SHELL, href => router.push(href)), + withActivityTracking(deviceMenuItems.remoteControl, EVENT_SUBTYPE.OPEN_REMOTE_CONTROL, href => + router.push(href), + ), deviceMenuItems.manageFiles, deviceMenuItems.runScript, ); diff --git a/src/app/(app)/tickets/hooks/use-create-ticket-form.ts b/src/app/(app)/tickets/hooks/use-create-ticket-form.ts index 5bc3c52..b5e6c00 100644 --- a/src/app/(app)/tickets/hooks/use-create-ticket-form.ts +++ b/src/app/(app)/tickets/hooks/use-create-ticket-form.ts @@ -5,6 +5,7 @@ import { useQuery } from '@tanstack/react-query'; import { useEffect } from 'react'; import { useForm } from 'react-hook-form'; import { useApplyAssignmentsDiff, useAssignedItems } from '@/components/assignments'; +import { EVENT_SUBTYPE, trackDashboardActivity } from '@/lib/analytics'; import { apiClient } from '@/lib/api-client'; import { API_ENDPOINTS, CREATION_SOURCE } from '../constants'; import { GET_TICKET_QUERY } from '../queries/ticket-queries'; @@ -13,6 +14,7 @@ import { type CreateTicketFormData, createTicketSchema } from '../types/create-t import type { Ticket } from '../types/ticket.types'; import type { GraphQlResponse } from '../utils/graphql'; import { extractGraphQlData } from '../utils/graphql'; +import { isResolvedStatusId } from '../utils/is-resolved-status'; import { ticketsQueryKeys } from '../utils/query-keys'; import { resolveCurrentStatus } from '../utils/resolve-current-status'; import { useCreateTicket } from './use-create-ticket'; @@ -119,6 +121,12 @@ export function useCreateTicketForm({ ticketId }: UseCreateTicketFormOptions = { // Transition first: updateTicket's onSuccess navigates away, so a failed // transition afterwards would strand the user on the next page mid-error. if (data.statusId && data.statusId !== currentStatus?.id) { + // Editing a ticket into a RESOLVED-kind status is also a "resolve". + // Track optimistically before the mutation, same as the detail-view + // status changer (see isResolvedStatusId). + if (isResolvedStatusId(data.statusId, statusesQuery.data?.snapshot)) { + trackDashboardActivity(EVENT_SUBTYPE.RESOLVE_TICKET); + } await transitionTicketMutation.mutateAsync({ ticketId, toStatusId: data.statusId }); } diff --git a/src/app/(app)/tickets/utils/is-resolved-status.ts b/src/app/(app)/tickets/utils/is-resolved-status.ts new file mode 100644 index 0000000..65fa6b5 --- /dev/null +++ b/src/app/(app)/tickets/utils/is-resolved-status.ts @@ -0,0 +1,20 @@ +import type { TicketStatusDefinition } from '../statuses/types/ticket-statuses.types'; +import { TICKET_STATUS_KIND } from './ticket-statistics'; + +/** + * True when `statusId` refers to a RESOLVED-kind status in `snapshot`. + * + * Single source of truth for "is this transition/edit resolving the ticket?", + * used to fire the `resolve_ticket_ticket_detail` activity event from every + * resolve entry point (the detail-view status changer and the Edit Ticket form). + * Keyed on `kind`, not a hardcoded id/name, so custom-named resolved statuses + * ("Done", "Closed", …) are recognized. `availableTransitions` carries no + * `kind`, so callers pass the full statuses `snapshot`. + */ +export function isResolvedStatusId( + statusId: string | null | undefined, + snapshot: TicketStatusDefinition[] | undefined, +): boolean { + if (!statusId || !snapshot) return false; + return snapshot.find(s => s.id === statusId)?.kind === TICKET_STATUS_KIND.RESOLVED; +}