Skip to content
Merged
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
36 changes: 36 additions & 0 deletions apps/web/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import {
isTerminalRunStatus,
removeTrackedBatch,
removeTrackedRun,
selectStaleTrackedRuns,
STALE_TRACKED_RUN_MS,
subscribeRunTracker,
summarizeBatchStatuses,
} from './lib/run-tracker-store.js'
Expand Down Expand Up @@ -223,6 +225,40 @@ export function RunNotificationObserver() {
removeTrackedRun(run.id)
}

// Give up on tracked runs that have aged out of the capped GET /runs window
// (see STALE_TRACKED_RUN_MS). Such a run never reappears, so the loop above
// never sees it reach a terminal status and its trigger button would wedge
// on "Syncing…"/"Running…" forever. Only reconcile once /runs has actually
// loaded (`data` defined), so a not-yet-loaded list is never misread as
// "absent"; the TTL additionally protects a just-queued run that hasn't hit
// the list yet. Refresh the kind's queries with whatever is current, then
// drop the tracked run so its button re-enables.
if (runsQuery.data !== undefined) {
const presentRunIds = new Set(runs.map((r) => r.id))
const staleRuns = selectStaleTrackedRuns(
Object.values(trackedState.runs),
presentRunIds,
Date.now(),
STALE_TRACKED_RUN_MS,
)
for (const trackedRun of staleRuns) {
const project = projects.find((candidate) => candidate.id === trackedRun.projectId)
if (project) {
invalidateQueriesForRunKind(queryClient, trackedRun.kind, project.name)
}
if (trackedRun.sourceAction !== 'run-all') {
addToast({
title: `${formatTrackedRunKind(trackedRun.kind)} status unavailable`,
detail: resolveProjectLabel(trackedRun.projectId, trackedRun.projectLabel, projects),
tone: 'neutral',
dedupeKey: `run-stale:${trackedRun.runId}`,
dedupeMode: 'replace',
})
}
removeTrackedRun(trackedRun.runId)
}
}

for (const batch of Object.values(trackedState.batches)) {
const summary = summarizeBatchStatuses(batch.runIds, runsById)
if (!summary.finished) continue
Expand Down
46 changes: 38 additions & 8 deletions apps/web/src/components/project/GbpSection.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useState } from 'react'
import { useEffect, useState, useSyncExternalStore } from 'react'
import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'
import {
getApiV1ProjectsByNameGoogleConnectionsOptions,
Expand All @@ -15,6 +15,7 @@ import {
classifyGbpMetric,
GBP_CONVERSION_METRICS,
GBP_REACH_METRICS,
RunKinds,
} from '@ainyc/canonry-contracts'

import { Button } from '../ui/button.js'
Expand Down Expand Up @@ -43,8 +44,9 @@ import {
} from '../shared/ChartPrimitives.js'
import { fetchInsights, heyClient } from '../../api.js'
import { addToast } from '../../lib/toast-store.js'
import { getRunTrackerState, subscribeRunTracker, trackRun } from '../../lib/run-tracker-store.js'

export function GbpSection({ projectName }: { projectName: string }) {
export function GbpSection({ projectName, projectId }: { projectName: string; projectId: string }) {
const queryClient = useQueryClient()
// `null` = all tracked locations (aggregate). A locationName scopes every read
// to one location. A single tracked location reads 1:1 with no selector.
Expand Down Expand Up @@ -92,15 +94,43 @@ export function GbpSection({ projectName }: { projectName: string }) {
})
}

// Errors surface through the global error toast (no skipGlobalErrorToast meta).
// GBP sync is an async background run: POST /gbp/sync queues a `gbp-sync` run
// and returns immediately. Hand the run to the global RunNotificationObserver
// (via trackRun) so it polls to completion, invalidates the GBP queries once
// the data is actually refreshed, and emits the completion toast — rather than
// invalidating eagerly here (which refetches the same stale data while the run
// is still in flight) and re-enabling the button the instant the POST returns.
// Errors on the POST itself still surface through the global error toast.
const syncMutation = useMutation({
...postApiV1ProjectsByNameGbpSyncMutation(),
onSuccess: () => {
addToast('Google Business Profile sync started.', 'positive')
invalidateGbp()
onSuccess: (data) => {
trackRun({
id: data.runId,
projectId,
kind: RunKinds['gbp-sync'],
projectLabel: projectName,
sourceAction: 'gbp-sync',
})
addToast({
title: 'Business Profile sync started',
detail: `${projectName} will refresh when the sync completes.`,
tone: 'neutral',
dedupeKey: `gbp-sync:${projectName}`,
dedupeMode: 'replace',
})
},
})

// Keep the Sync button disabled until the queued run reaches a terminal
// status. `syncMutation.isPending` only covers the brief POST; the tracked
// run (cleared by RunNotificationObserver on completion) covers the
// background sync that follows.
const trackerState = useSyncExternalStore(subscribeRunTracker, getRunTrackerState, getRunTrackerState)
const gbpSyncInFlight = Object.values(trackerState.runs).some(
(run) => run.kind === RunKinds['gbp-sync'] && run.projectId === projectId,
)
const isSyncing = syncMutation.isPending || gbpSyncInFlight

const selectionMutation = useMutation({
...putApiV1ProjectsByNameGbpLocationsByLocationNameSelectionMutation(),
onSuccess: () => invalidateGbp(),
Expand Down Expand Up @@ -176,10 +206,10 @@ export function GbpSection({ projectName }: { projectName: string }) {
type="button"
variant="outline"
size="sm"
disabled={syncMutation.isPending}
disabled={isSyncing}
onClick={() => syncMutation.mutate({ client: heyClient, path: { name: projectName }, body: {} })}
>
{syncMutation.isPending ? 'Syncing…' : 'Sync'}
{isSyncing ? 'Syncing…' : 'Sync'}
</Button>
</div>
</div>
Expand Down
40 changes: 39 additions & 1 deletion apps/web/src/lib/run-tracker-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,21 @@ export type TrackedRunSourceAction =
| 'setup-launch'
| 'run-all'
| 'gsc-sync'
| 'gbp-sync'
| 'discover-sitemaps'
| 'inspect-sitemap'

export interface TrackedRun {
runId: string
projectId: string
projectLabel?: string
kind: string
kind: ApiRun['kind']
sourceAction: TrackedRunSourceAction
lastAnnouncedStatus: string
/** Epoch ms when the run was first tracked. Used to give up on a run that has
* aged out of the capped /runs window (see `selectStaleTrackedRuns`).
* Optional: rows persisted before this field existed hydrate without it. */
trackedAt?: number
}

export interface TrackedBatch {
Expand Down Expand Up @@ -96,6 +101,8 @@ export function trackRun(run: Pick<ApiRun, 'id' | 'projectId' | 'kind'> & {
projectLabel?: string
sourceAction: TrackedRunSourceAction
lastAnnouncedStatus?: string
/** Override the tracked-at stamp (tests); defaults to now. */
trackedAt?: number
}) {
const snapshot = getRunTrackerState()
updateState({
Expand All @@ -109,6 +116,7 @@ export function trackRun(run: Pick<ApiRun, 'id' | 'projectId' | 'kind'> & {
kind: run.kind,
sourceAction: run.sourceAction,
lastAnnouncedStatus: run.lastAnnouncedStatus ?? 'queued',
trackedAt: run.trackedAt ?? Date.now(),
},
},
})
Expand Down Expand Up @@ -172,6 +180,36 @@ export function isTerminalRunStatus(status: string) {
return status === 'completed' || status === 'partial' || status === 'failed' || status === 'cancelled'
}

/**
* How long a tracked run may be ABSENT from the GET /runs response before the
* observer gives up on it. GET /runs is capped (default 500 rows / a since
* window), so a tracked run can age out of that window — cron syncs alone can
* fill it in under an hour — after which the observer never sees it reach a
* terminal status and its trigger button would wedge on "Syncing…"/"Running…"
* forever. Absence is only ever the aged-out case (an in-flight run is still in
* the window), so this is purely a backstop; a page reload clears it sooner.
*/
export const STALE_TRACKED_RUN_MS = 10 * 60_000

/**
* Pure give-up rule: return the tracked runs that have aged out of the /runs
* window and should be cleared. A run still PRESENT in `presentRunIds` is
* never stale (the observer is still watching it); a run ABSENT for at least
* `staleMs` (relative to `now`) has aged out. A run with no `trackedAt` (a row
* persisted before this field existed) is treated as infinitely old, so any
* pre-existing wedged run is cleared on the next poll.
*/
export function selectStaleTrackedRuns(
trackedRuns: TrackedRun[],
presentRunIds: Set<string>,
now: number,
staleMs: number,
): TrackedRun[] {
return trackedRuns.filter(
(run) => !presentRunIds.has(run.runId) && now - (run.trackedAt ?? 0) >= staleMs,
)
}

export function summarizeBatchStatuses(runIds: string[], runsById: Record<string, ApiRun | undefined>) {
let completed = 0
let partial = 0
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/pages/ProjectPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2227,7 +2227,7 @@ function ProjectPageContent({
) : tab === 'local' ? (
// Local presence (Google Business Profile + Places). GbpSection
// self-gates on the connection and renders its own empty state.
<GbpSection projectName={model.project.name} />
<GbpSection projectName={model.project.name} projectId={model.project.id} />
) : (
<SearchConsoleSection projectName={model.project.name} />
)}
Expand Down
75 changes: 72 additions & 3 deletions apps/web/test/gbp-section.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react'
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { afterEach, expect, onTestFinished, test, vi } from 'vitest'
import { afterEach, beforeEach, expect, onTestFinished, test, vi } from 'vitest'
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'

// Recharts needs a sized container (jsdom has none); stub it like the other
Expand All @@ -21,16 +21,31 @@ vi.mock('recharts', () => {
}
})

import { act } from '@testing-library/react'

import { GbpSection } from '../src/components/project/GbpSection.js'
import { getRunTrackerState, removeTrackedRun, resetRunTracker } from '../src/lib/run-tracker-store.js'
import { mockFetch, jsonResponse } from './mock-fetch.js'

afterEach(cleanup)
// The run tracker is a module-global store backed by sessionStorage; reset it
// around every test so a queued gbp-sync run from one test never leaks into the
// button state of the next.
beforeEach(() => {
resetRunTracker()
window.sessionStorage.clear()
})

afterEach(() => {
cleanup()
resetRunTracker()
window.sessionStorage.clear()
})

function renderGbpSection() {
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } })
return render(
<QueryClientProvider client={queryClient}>
<GbpSection projectName="test-project" />
<GbpSection projectName="test-project" projectId="p1" />
</QueryClientProvider>,
)
}
Expand Down Expand Up @@ -299,3 +314,57 @@ test('shows a connect empty-state when the project has no GBP connection', async
await waitFor(() => expect(screen.getByText(/No Google Business Profile is connected/)).toBeTruthy())
expect(screen.getByText('Google Business Profile')).toBeTruthy()
})

test('Sync queues a gbp-sync run, stays disabled, and hands off to the run tracker', async () => {
const restoreFetch = mockFetch((url, init) => {
const urlPath = url.split('?')[0]!
if (urlPath.endsWith('/projects/test-project/gbp/sync') && init?.method === 'POST') {
// POST /gbp/sync queues an async run and returns immediately.
return jsonResponse({ runId: 'gbp-run-1', status: 'running' })
}
if (urlPath.endsWith('/projects/test-project/google/connections')) return jsonResponse([gbpConnection])
if (urlPath.endsWith('/projects/test-project/gbp/summary')) return jsonResponse(emptySummary())
if (urlPath.endsWith('/projects/test-project/gbp/locations')) {
return jsonResponse({ locations: [makeLocation({})], totalDiscovered: 1, totalSelected: 1 })
}
if (urlPath.endsWith('/projects/test-project/gbp/keywords')) return jsonResponse({ keywords: [], total: 0, thresholdedPct: 0 })
if (urlPath.endsWith('/projects/test-project/gbp/places')) return jsonResponse({ places: [], total: 0 })
if (urlPath.endsWith('/projects/test-project/insights')) return jsonResponse([])
throw new Error(`Unexpected fetch: ${url} ${init?.method ?? ''}`)
})
onTestFinished(restoreFetch)

renderGbpSection()

// The Sync action is enabled once the connection resolves.
const syncButton = await screen.findByRole('button', { name: 'Sync' })
expect(syncButton.hasAttribute('disabled')).toBe(false)

fireEvent.click(syncButton)

// The button stays disabled and reads "Syncing…" while the queued run is in
// flight — it does not snap back to "Sync" the instant the POST returns. (The
// global RunNotificationObserver, not mounted here, clears the tracked run on
// completion, which is what re-enables it in the app.)
await waitFor(() => expect(screen.getByRole('button', { name: /Syncing/ })).toBeTruthy())
expect(screen.getByRole('button', { name: /Syncing/ }).hasAttribute('disabled')).toBe(true)

// The async run is handed to the global run tracker (which polls it to
// completion and then invalidates the GBP queries) rather than invalidating
// eagerly here while the data is still stale.
const tracked = Object.values(getRunTrackerState().runs)
expect(tracked).toHaveLength(1)
expect(tracked[0]).toMatchObject({
runId: 'gbp-run-1',
projectId: 'p1',
kind: 'gbp-sync',
sourceAction: 'gbp-sync',
})

// When the run reaches a terminal status the observer drops it from the
// tracker (RunNotificationObserver → removeTrackedRun). Simulate that and
// assert the Sync button re-enables — i.e. it stays disabled *until the sync
// is complete*, not just for the POST.
act(() => removeTrackedRun('gbp-run-1'))
await waitFor(() => expect(screen.getByRole('button', { name: 'Sync' }).hasAttribute('disabled')).toBe(false))
})
7 changes: 7 additions & 0 deletions apps/web/test/run-invalidations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,16 @@ test('invalidates GA operations for ga-sync runs', () => {
expect(predicateMatches('getApiV1ProjectsByNameGaStatus')).toBe(true)
})

test('invalidates GBP operations for gbp-sync runs', () => {
invalidateQueriesForRunKind(queryClient, RunKinds['gbp-sync'], 'demo')
expect(predicateMatches('getApiV1ProjectsByNameGbpSummary')).toBe(true)
expect(predicateMatches('getApiV1ProjectsByNameGbpKeywords')).toBe(true)
})

test('does not invalidate domain-scoped operations for answer-visibility runs', () => {
invalidateQueriesForRunKind(queryClient, RunKinds['answer-visibility'], 'demo')
expect(predicateMatches('getApiV1ProjectsByNameGoogleGscCoverage')).toBe(false)
expect(predicateMatches('getApiV1ProjectsByNameBingCoverage')).toBe(false)
expect(predicateMatches('getApiV1ProjectsByNameGaTraffic')).toBe(false)
expect(predicateMatches('getApiV1ProjectsByNameGbpSummary')).toBe(false)
})
38 changes: 38 additions & 0 deletions apps/web/test/run-notifications.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -480,3 +480,41 @@ test('maps RUN_IN_PROGRESS errors to one caution toast with an extended timer',
expect(runInProgressToasts[0]?.detail).toContain('Citypoint Dental NYC already has an active run')
})
})

test('clears a tracked sync run that has aged out of the /runs window so its button cannot wedge', async () => {
const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
const url = input instanceof Request ? input.url : String(input)
// The tracked run has aged out of the capped window: GET /runs no longer returns it.
if (url.includes('/api/v1/runs')) return jsonResponse([])
if (url.includes('/api/v1/projects')) return jsonResponse([makeProject()])
throw new Error(`Unexpected fetch: ${url}`)
})
vi.stubGlobal('fetch', fetchMock)

// A gbp-sync run tracked long ago (epoch) — absent from /runs and past the TTL.
trackRun({
id: 'gbp_run_aged',
projectId: 'proj_1',
kind: 'gbp-sync',
projectLabel: 'Citypoint Dental NYC',
sourceAction: 'gbp-sync',
lastAnnouncedStatus: 'queued',
trackedAt: 0,
})
expect(getRunTrackerState().runs.gbp_run_aged).toBeTruthy()

const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity } } })
queryClient.setQueryData(runsCacheKey, [])
queryClient.setQueryData(projectsCacheKey, [makeProject()])
render(
<QueryClientProvider client={queryClient}>
<RunNotificationObserver />
</QueryClientProvider>,
)

// The observer gives up on the aged-out run instead of tracking it forever: it
// drops from the tracker (which re-enables the Sync button) and surfaces a
// neutral "status unavailable" toast.
await waitFor(() => expect(getRunTrackerState().runs.gbp_run_aged).toBeUndefined())
expect(getToasts().some((toast) => toast.title === 'Business Profile sync status unavailable')).toBe(true)
})
Loading
Loading