diff --git a/.github/workflows/cd-apps.yml b/.github/workflows/cd-apps.yml index 3fd046c..701416c 100644 --- a/.github/workflows/cd-apps.yml +++ b/.github/workflows/cd-apps.yml @@ -106,6 +106,9 @@ jobs: # Pull and deploy app containers docker compose --env-file app.env pull journal planner + # Hand-written data migrations (idempotent) run BEFORE drizzle-kit + # push so unique-key reshapes can collapse duplicate rows first. + docker compose --env-file app.env run --rm journal node --experimental-strip-types /app/packages/db/src/migrate-data.ts docker compose --env-file app.env run --rm journal npx drizzle-kit push --config /app/packages/db/drizzle.config.ts --force # --remove-orphans cleans up containers whose service was deleted # from the compose file, matching cd-infra's behaviour. diff --git a/apps/journal/app/lib/sync/providers/wahoo.test.ts b/apps/journal/app/lib/sync/providers/wahoo.test.ts index 25d90e7..ebc7de9 100644 --- a/apps/journal/app/lib/sync/providers/wahoo.test.ts +++ b/apps/journal/app/lib/sync/providers/wahoo.test.ts @@ -122,6 +122,7 @@ describe("wahooProvider.pushRoute", () => { it.each([ [401, "token_expired"], [403, "scope_missing"], + [404, "not_found"], [422, "validation"], [429, "rate_limit"], [500, "generic"], @@ -149,6 +150,81 @@ describe("wahooProvider.pushRoute", () => { }); }); +describe("wahooProvider.updateRoute", () => { + const tokens = { + accessToken: "test-token", + refreshToken: "refresh", + expiresAt: new Date(Date.now() + 3600_000), + }; + const fit = new Uint8Array([0x01, 0x02, 0x03]); + const basePayload = { + fit, + externalId: "route:abc", + providerUpdatedAt: new Date("2026-04-30T12:00:00Z"), + name: "Test route", + description: "A test", + startLat: 52.52, + startLng: 13.405, + distance: 12345, + ascent: 200, + }; + + let fetchMock: ReturnType; + + beforeEach(() => { + fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("PUTs to /v1/routes/:id and keeps the same remote id", async () => { + fetchMock.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ id: 9876 }), + }); + + const result = await wahooProvider.updateRoute!(tokens, "9876", basePayload); + + expect(result.remoteId).toBe("9876"); + expect(fetchMock).toHaveBeenCalledOnce(); + const [url, init] = fetchMock.mock.calls[0]!; + expect(url).toBe("https://api.wahooligan.com/v1/routes/9876"); + expect(init.method).toBe("PUT"); + const body = new URLSearchParams(init.body as string); + expect(body.get("route[external_id]")).toBe("route:abc"); + expect(body.get("route[file]")).toBe( + `data:application/vnd.fit;base64,${Buffer.from(fit).toString("base64")}`, + ); + }); + + it("falls back to the targeted remote id when PUT returns 204 (no body)", async () => { + fetchMock.mockResolvedValueOnce({ + ok: true, + status: 204, + json: async () => null, + }); + const result = await wahooProvider.updateRoute!(tokens, "555", basePayload); + expect(result.remoteId).toBe("555"); + }); + + it("throws PushError(not_found) on 404 so the pipeline can fall back to POST", async () => { + fetchMock.mockResolvedValueOnce({ + ok: false, + status: 404, + text: async () => "gone", + }); + await expect(wahooProvider.updateRoute!(tokens, "missing", basePayload)).rejects.toMatchObject({ + name: "PushError", + code: "not_found", + status: 404, + }); + }); +}); + describe("wahooProvider.exchangeCode", () => { let fetchMock: ReturnType; beforeEach(() => { diff --git a/apps/journal/app/lib/sync/providers/wahoo.ts b/apps/journal/app/lib/sync/providers/wahoo.ts index 7f7ce64..d6d27cc 100644 --- a/apps/journal/app/lib/sync/providers/wahoo.ts +++ b/apps/journal/app/lib/sync/providers/wahoo.ts @@ -17,6 +17,65 @@ const WAHOO_AUTH = "https://api.wahooligan.com/oauth"; const clientId = () => process.env.WAHOO_CLIENT_ID ?? ""; const clientSecret = () => process.env.WAHOO_CLIENT_SECRET ?? ""; +function buildRouteFormBody(payload: PushRoutePayload): URLSearchParams { + // Wahoo expects route[file] as a data URI, not raw base64. Sending plain + // base64 results in a route record where file.url is null and the Wahoo + // app shows the route in the list (with metadata) but renders no track. + const fitDataUri = `data:application/vnd.fit;base64,${Buffer.from(payload.fit).toString("base64")}`; + const body = new URLSearchParams({ + "route[external_id]": payload.externalId, + "route[provider_updated_at]": payload.providerUpdatedAt.toISOString(), + "route[name]": payload.name, + "route[workout_type_family_id]": "0", + "route[start_lat]": payload.startLat.toString(), + "route[start_lng]": payload.startLng.toString(), + "route[distance]": payload.distance.toString(), + "route[ascent]": payload.ascent.toString(), + "route[file]": fitDataUri, + }); + if (payload.description) body.set("route[description]", payload.description); + if (payload.filename) body.set("route[filename]", payload.filename); + return body; +} + +async function wahooRouteRequest( + tokens: TokenSet, + method: "POST" | "PUT", + url: string, + payload: PushRoutePayload, + opts: { fallbackRemoteId?: string } = {}, +): Promise { + const resp = await fetch(url, { + method, + headers: { + Authorization: `Bearer ${tokens.accessToken}`, + "Content-Type": "application/x-www-form-urlencoded", + }, + body: buildRouteFormBody(payload).toString(), + }); + + if (resp.ok) { + // PUT may respond 200/204 without a body. The remote id is unchanged in + // that case, so fall back to the one we just targeted. + let remoteId: string | undefined; + if (resp.status !== 204) { + const data = (await resp.json().catch(() => null)) as { id?: number | string } | null; + remoteId = data?.id?.toString(); + } + remoteId ??= opts.fallbackRemoteId; + if (!remoteId) throw new PushError("generic", "Wahoo response missing route id", resp.status); + return { remoteId }; + } + + const text = await resp.text().catch(() => ""); + if (resp.status === 401) throw new PushError("token_expired", text || "Unauthorized", 401); + if (resp.status === 403) throw new PushError("scope_missing", text || "Forbidden", 403); + if (resp.status === 404) throw new PushError("not_found", text || "Not Found", 404); + if (resp.status === 422) throw new PushError("validation", text || "Validation failed", 422); + if (resp.status === 429) throw new PushError("rate_limit", text || "Rate limited", 429); + throw new PushError("generic", `Wahoo route ${method} failed: ${resp.status} ${text}`, resp.status); +} + export const wahooProvider: SyncProvider = { id: "wahoo", name: "Wahoo", @@ -181,46 +240,21 @@ export const wahooProvider: SyncProvider = { }, async pushRoute(tokens: TokenSet, payload: PushRoutePayload): Promise { - // Wahoo expects route[file] as a data URI, not raw base64. Sending plain - // base64 results in a route record where file.url is null and the Wahoo - // app shows the route in the list (with metadata) but renders no track. - const fitDataUri = `data:application/vnd.fit;base64,${Buffer.from(payload.fit).toString("base64")}`; - const body = new URLSearchParams({ - "route[external_id]": payload.externalId, - "route[provider_updated_at]": payload.providerUpdatedAt.toISOString(), - "route[name]": payload.name, - "route[workout_type_family_id]": "0", - "route[start_lat]": payload.startLat.toString(), - "route[start_lng]": payload.startLng.toString(), - "route[distance]": payload.distance.toString(), - "route[ascent]": payload.ascent.toString(), - "route[file]": fitDataUri, - }); - if (payload.description) body.set("route[description]", payload.description); - if (payload.filename) body.set("route[filename]", payload.filename); + return wahooRouteRequest(tokens, "POST", `${WAHOO_API}/v1/routes`, payload); + }, - const resp = await fetch(`${WAHOO_API}/v1/routes`, { - method: "POST", - headers: { - Authorization: `Bearer ${tokens.accessToken}`, - "Content-Type": "application/x-www-form-urlencoded", - }, - body: body.toString(), - }); - - if (resp.ok) { - const data = (await resp.json()) as { id?: number | string }; - const remoteId = data.id?.toString(); - if (!remoteId) throw new PushError("generic", "Wahoo response missing route id", resp.status); - return { remoteId }; - } - - const text = await resp.text().catch(() => ""); - if (resp.status === 401) throw new PushError("token_expired", text || "Unauthorized", 401); - if (resp.status === 403) throw new PushError("scope_missing", text || "Forbidden", 403); - if (resp.status === 422) throw new PushError("validation", text || "Validation failed", 422); - if (resp.status === 429) throw new PushError("rate_limit", text || "Rate limited", 429); - throw new PushError("generic", `Wahoo route push failed: ${resp.status} ${text}`, resp.status); + async updateRoute( + tokens: TokenSet, + remoteId: string, + payload: PushRoutePayload, + ): Promise { + return wahooRouteRequest( + tokens, + "PUT", + `${WAHOO_API}/v1/routes/${encodeURIComponent(remoteId)}`, + payload, + { fallbackRemoteId: remoteId }, + ); }, async revoke(tokens: TokenSet): Promise { diff --git a/apps/journal/app/lib/sync/pushes.server.test.ts b/apps/journal/app/lib/sync/pushes.server.test.ts index 2f6fc3c..208904d 100644 --- a/apps/journal/app/lib/sync/pushes.server.test.ts +++ b/apps/journal/app/lib/sync/pushes.server.test.ts @@ -91,6 +91,7 @@ function makeProvider(over: Record = {}) { name: "Wahoo", scopes: ["routes_write"], pushRoute: vi.fn(), + updateRoute: vi.fn(), refreshToken: vi.fn(), ...over, }; @@ -112,18 +113,23 @@ describe("pushRouteToProvider", () => { expect(out).toMatchObject({ status: "success", remoteId: "wahoo-9" }); expect(provider.pushRoute).toHaveBeenCalledOnce(); const [, payload] = provider.pushRoute.mock.calls[0]!; - expect(payload.externalId).toBe("route:r1:v2"); + expect(payload.externalId).toBe("route:r1"); expect(payload.startLat).toBeCloseTo(52.52, 4); expect(mockInsert).toHaveBeenCalledOnce(); expect(mockUpdate).not.toHaveBeenCalled(); }); - it("short-circuits when the version is already pushed", async () => { + it("short-circuits when the current version is already pushed", async () => { mockGetRoute.mockResolvedValue(baseRoute); mockGetConnection.mockResolvedValue(baseConnection); queueSelectResults( [{ version: 1, gpx: SHORT_GPX }], - [{ id: "p1", pushedAt: new Date("2026-04-01T00:00:00Z"), remoteId: "wahoo-7" }], + [{ + id: "p1", + pushedAt: new Date("2026-04-01T00:00:00Z"), + remoteId: "wahoo-7", + lastPushedVersion: 1, + }], ); const provider = makeProvider(); @@ -134,6 +140,64 @@ describe("pushRouteToProvider", () => { expect(out).toMatchObject({ status: "success", remoteId: "wahoo-7" }); expect(provider.pushRoute).not.toHaveBeenCalled(); + expect(provider.updateRoute).not.toHaveBeenCalled(); + }); + + it("PUTs against the existing remote id when the local version has advanced", async () => { + mockGetRoute.mockResolvedValue(baseRoute); + mockGetConnection.mockResolvedValue(baseConnection); + queueSelectResults( + [{ version: 2, gpx: SHORT_GPX }], + [{ + id: "p1", + pushedAt: new Date("2026-04-01T00:00:00Z"), + remoteId: "wahoo-7", + lastPushedVersion: 1, + }], + ); + + const provider = makeProvider(); + provider.updateRoute.mockResolvedValue({ remoteId: "wahoo-7" }); + mockGetProvider.mockReturnValue(provider); + + const { pushRouteToProvider } = await importPipeline(); + const out = await pushRouteToProvider({ userId: "u1", providerId: "wahoo", routeId: "r1" }); + + expect(out).toMatchObject({ status: "success", remoteId: "wahoo-7" }); + expect(provider.updateRoute).toHaveBeenCalledOnce(); + expect(provider.pushRoute).not.toHaveBeenCalled(); + // Existing row updated in place — exactly one row exists afterwards. + expect(mockUpdate).toHaveBeenCalledOnce(); + expect(mockInsert).not.toHaveBeenCalled(); + }); + + it("falls back to POST when PUT returns not_found", async () => { + mockGetRoute.mockResolvedValue(baseRoute); + mockGetConnection.mockResolvedValue(baseConnection); + queueSelectResults( + [{ version: 2, gpx: SHORT_GPX }], + [{ + id: "p1", + pushedAt: new Date("2026-04-01T00:00:00Z"), + remoteId: "wahoo-stale", + lastPushedVersion: 1, + }], + ); + + const provider = makeProvider(); + const { PushError } = await import("./types.ts"); + provider.updateRoute.mockRejectedValueOnce(new PushError("not_found", "gone", 404)); + provider.pushRoute.mockResolvedValueOnce({ remoteId: "wahoo-fresh" }); + mockGetProvider.mockReturnValue(provider); + + const { pushRouteToProvider } = await importPipeline(); + const out = await pushRouteToProvider({ userId: "u1", providerId: "wahoo", routeId: "r1" }); + + expect(out).toMatchObject({ status: "success", remoteId: "wahoo-fresh" }); + expect(provider.updateRoute).toHaveBeenCalledOnce(); + expect(provider.pushRoute).toHaveBeenCalledOnce(); + expect(mockUpdate).toHaveBeenCalledOnce(); + expect(mockInsert).not.toHaveBeenCalled(); }); it("retries a failed prior attempt and updates the existing row", async () => { diff --git a/apps/journal/app/lib/sync/pushes.server.ts b/apps/journal/app/lib/sync/pushes.server.ts index 3100a51..b4ee9f4 100644 --- a/apps/journal/app/lib/sync/pushes.server.ts +++ b/apps/journal/app/lib/sync/pushes.server.ts @@ -58,7 +58,9 @@ export async function pushRouteToProvider(opts: PushRouteOptions): Promise { + if (existingRow?.remoteId && provider.updateRoute) { + try { + return await provider.updateRoute(toks, existingRow.remoteId, payload); + } catch (e) { + if (e instanceof PushError && e.code === "not_found") { + return provider.pushRoute(toks, payload); + } + throw e; + } + } + return provider.pushRoute(toks, payload); + }; + try { let result; try { - result = await provider.pushRoute(tokens, payload); + result = await callProvider(tokens); } catch (e) { if (e instanceof PushError && e.code === "token_expired") { // 401 mid-call — refresh once and retry. tokens = await provider.refreshToken(tokens.refreshToken); await updateTokens(connection.id, tokens); - result = await provider.pushRoute(tokens, payload); + result = await callProvider(tokens); } else { throw e; } @@ -163,7 +187,11 @@ export async function pushRouteToProvider(opts: PushRouteOptions): Promise:v` for trails.cool. */ + /** Stable per route — `route:` for trails.cool. */ externalId: string; providerUpdatedAt: Date; name: string; @@ -56,6 +56,7 @@ export type PushErrorCode = | "token_expired" | "validation" | "rate_limit" + | "not_found" | "generic"; export type OAuthErrorCode = "too_many_tokens" | "generic"; @@ -102,6 +103,16 @@ export interface SyncProvider { /** Optional: providers that can accept routes implement this. UI hides the action when undefined. */ pushRoute?: (tokens: TokenSet, payload: PushRoutePayload) => Promise; + /** + * Optional: update an already-pushed route in place. Throws PushError("not_found") + * if the remote route no longer exists; callers should fall back to pushRoute. + */ + updateRoute?: ( + tokens: TokenSet, + remoteId: string, + payload: PushRoutePayload, + ) => Promise; + /** Optional: revoke the given access token at the provider. Best-effort — failures should be swallowed by callers. */ revoke?: (tokens: TokenSet) => Promise; } diff --git a/apps/journal/app/routes/routes.$id.tsx b/apps/journal/app/routes/routes.$id.tsx index 1683de9..3b9cb84 100644 --- a/apps/journal/app/routes/routes.$id.tsx +++ b/apps/journal/app/routes/routes.$id.tsx @@ -2,7 +2,7 @@ import { useState, useCallback } from "react"; import { data, redirect } from "react-router"; import { useTranslation } from "react-i18next"; import type { Route } from "./+types/routes.$id"; -import { and, desc, eq } from "drizzle-orm"; +import { and, eq } from "drizzle-orm"; import { canView, getSessionUser } from "~/lib/auth.server"; import { getRoute, getRouteWithVersions, deleteRoute, updateRoute } from "~/lib/routes.server"; import { getDb } from "~/lib/db"; @@ -44,20 +44,32 @@ export async function loader({ params, request }: Route.LoaderArgs) { const currentVersion = route.versions[0]?.version ?? 1; - // Wahoo push state — only meaningful for the owner. Includes the latest - // sync_pushes row for the current version (if any) and whether the user - // has a Wahoo connection with the routes_write scope. + // Wahoo push state — only meaningful for the owner. The single + // sync_pushes row per (user, route, wahoo) carries `lastPushedVersion`, + // which we compare against the current local version to render one of + // three states: matches, local newer, or last attempt failed. let wahooPush: | { canPush: boolean; needsReauth: boolean; - latest: { pushedAt: string | null; remoteId: string | null; error: string | null } | null; + currentVersion: number; + latest: { + pushedAt: string | null; + remoteId: string | null; + lastPushedVersion: number | null; + error: string | null; + } | null; } | null = null; if (isOwner && user && !!route.gpx) { const connection = await getConnection(user.id, "wahoo"); let latest: - | { pushedAt: string | null; remoteId: string | null; error: string | null } + | { + pushedAt: string | null; + remoteId: string | null; + lastPushedVersion: number | null; + error: string | null; + } | null = null; if (connection) { const db = getDb(); @@ -68,16 +80,15 @@ export async function loader({ params, request }: Route.LoaderArgs) { and( eq(syncPushes.userId, user.id), eq(syncPushes.routeId, route.id), - eq(syncPushes.routeVersion, currentVersion), eq(syncPushes.provider, "wahoo"), ), ) - .orderBy(desc(syncPushes.createdAt)) .limit(1); latest = row ? { pushedAt: row.pushedAt ? row.pushedAt.toISOString() : null, remoteId: row.remoteId, + lastPushedVersion: row.lastPushedVersion, error: row.error, } : null; @@ -85,6 +96,7 @@ export async function loader({ params, request }: Route.LoaderArgs) { wahooPush = { canPush: !!connection, needsReauth: !!connection && !connection.grantedScopes.includes("routes_write"), + currentVersion, latest, }; } @@ -237,33 +249,49 @@ export default function RouteDetailPage({ loaderData }: Route.ComponentProps) { {t("routes.exportGpx")} )} - {wahooPush?.canPush && ( - wahooPush.latest?.pushedAt ? ( - - {t("routes.sentToWahoo", { - date: new Date(wahooPush.latest.pushedAt).toLocaleDateString(i18n.language), - })} - - ) : ( + {wahooPush?.canPush && (() => { + const latest = wahooPush.latest; + const matches = + latest?.pushedAt && + latest.lastPushedVersion === wahooPush.currentVersion; + const localNewer = + latest?.pushedAt && + latest.lastPushedVersion != null && + latest.lastPushedVersion < wahooPush.currentVersion; + if (matches) { + return ( + + {t("routes.sentToWahoo", { + date: new Date(latest!.pushedAt!).toLocaleDateString(i18n.language), + })} + + ); + } + return (
+ {localNewer && ( + + {t("routes.onWahooNewer", { n: latest!.lastPushedVersion })} + + )} - {wahooPush.latest?.error && ( + {latest?.error && ( - {t("routes.sendToWahooFailed", { error: wahooPush.latest.error })} + {t("routes.sendToWahooFailed", { error: latest.error })} )}
- ) - )} + ); + })()} )} diff --git a/openspec/changes/wahoo-route-update/tasks.md b/openspec/changes/wahoo-route-update/tasks.md index 6acc67b..1174a1e 100644 --- a/openspec/changes/wahoo-route-update/tasks.md +++ b/openspec/changes/wahoo-route-update/tasks.md @@ -1,33 +1,33 @@ ## 1. Schema migration -- [ ] 1.1 Add `last_pushed_version` (integer, nullable) to `sync_pushes` in the Drizzle schema. -- [ ] 1.2 Write a Drizzle migration that backfills `last_pushed_version` from `route_version` for every existing row. -- [ ] 1.3 Extend the migration to collapse rows: per `(user_id, route_id, provider)` group, keep the row with the highest `route_version` and delete the rest. If the highest-versioned row failed, keep it (it's still the canonical one under the new key). -- [ ] 1.4 Drop the unique constraint that includes `route_version`; add a unique index on `(user_id, route_id, provider)`. -- [ ] 1.5 Drop the `route_version` column. -- [ ] 1.6 Run the migration locally against a copy of prod data (or a synthesized dataset that has multiple versions) and verify counts. +- [x] 1.1 Add `last_pushed_version` (integer, nullable) to `sync_pushes` in the Drizzle schema. +- [x] 1.2 Write a Drizzle migration that backfills `last_pushed_version` from `route_version` for every existing row. +- [x] 1.3 Extend the migration to collapse rows: per `(user_id, route_id, provider)` group, keep the row with the highest `route_version` and delete the rest. If the highest-versioned row failed, keep it (it's still the canonical one under the new key). +- [x] 1.4 Drop the unique constraint that includes `route_version`; add a unique index on `(user_id, route_id, provider)`. +- [x] 1.5 Drop the `route_version` column. +- [x] 1.6 Run the migration locally against a copy of prod data (or a synthesized dataset that has multiple versions) and verify counts. ## 2. Provider plumbing -- [ ] 2.1 Extend `wahooProvider.pushRoute` (or add a sibling `updateRoute`) so the wahoo provider exposes a way to PUT against an existing remote id; share the body construction with the POST path. -- [ ] 2.2 Treat a 404 response from PUT as a signal to fall back to POST and return the new `remote_id` to the caller. -- [ ] 2.3 Update the wahoo provider's unit tests: a successful PUT, a 404 PUT that falls back to a successful POST, and the existing POST/error matrix. +- [x] 2.1 Extend `wahooProvider.pushRoute` (or add a sibling `updateRoute`) so the wahoo provider exposes a way to PUT against an existing remote id; share the body construction with the POST path. +- [x] 2.2 Treat a 404 response from PUT as a signal to fall back to POST and return the new `remote_id` to the caller. +- [x] 2.3 Update the wahoo provider's unit tests: a successful PUT, a 404 PUT that falls back to a successful POST, and the existing POST/error matrix. ## 3. Push pipeline -- [ ] 3.1 In `pushes.server.ts`, look up `sync_pushes` by `(user_id, route_id, provider)` instead of `(…, route_version, …)`. -- [ ] 3.2 If the row has a non-null `remote_id`, call the provider's update path; otherwise call the create path. -- [ ] 3.3 On success, upsert the row with `last_pushed_version` set to the just-pushed local version, `pushed_at = now()`, `error = null`. -- [ ] 3.4 Change `external_id` construction to `route:` (drop the `:v` suffix). -- [ ] 3.5 Update existing pushes.server tests to assert PUT is used on the second push and that exactly one row exists afterwards. +- [x] 3.1 In `pushes.server.ts`, look up `sync_pushes` by `(user_id, route_id, provider)` instead of `(…, route_version, …)`. +- [x] 3.2 If the row has a non-null `remote_id`, call the provider's update path; otherwise call the create path. +- [x] 3.3 On success, upsert the row with `last_pushed_version` set to the just-pushed local version, `pushed_at = now()`, `error = null`. +- [x] 3.4 Change `external_id` construction to `route:` (drop the `:v` suffix). +- [x] 3.5 Update existing pushes.server tests to assert PUT is used on the second push and that exactly one row exists afterwards. ## 4. UI -- [ ] 4.1 Update the route detail page status block to compare `last_pushed_version` with the local route's current version and render the three states (matches, local newer, failed). -- [ ] 4.2 i18n: add "On Wahoo (v{{n}}) — local version is newer" and "Send updated version" strings (en + de). +- [x] 4.1 Update the route detail page status block to compare `last_pushed_version` with the local route's current version and render the three states (matches, local newer, failed). +- [x] 4.2 i18n: add "On Wahoo (v{{n}}) — local version is newer" and "Send updated version" strings (en + de). - [ ] 4.3 E2E test: connect Wahoo (mocked), push v1, edit the route to v2, push again, assert the request was a PUT to the same remote id and the UI returns to "Sent to Wahoo on ". ## 5. Spec & docs -- [ ] 5.1 Verify `openspec validate wahoo-route-update` passes. +- [x] 5.1 Verify `openspec validate wahoo-route-update` passes. - [ ] 5.2 After merge, run `/opsx:archive` to fold the delta into `openspec/specs/wahoo-route-push/spec.md`. diff --git a/package.json b/package.json index d85c3b8..389d2ff 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,7 @@ "test:e2e:ui": "playwright test --ui", "typecheck": "turbo typecheck", "db:push": "drizzle-kit push --config packages/db/drizzle.config.ts --force", + "db:migrate-data": "tsx packages/db/src/migrate-data.ts", "db:studio": "drizzle-kit studio --config packages/db/drizzle.config.ts", "dev:services": "docker compose -f docker-compose.dev.yml up -d", "dev:full": "./scripts/dev.sh", diff --git a/packages/db/migrations/0001_wahoo_route_update.sql b/packages/db/migrations/0001_wahoo_route_update.sql new file mode 100644 index 0000000..9b783b1 --- /dev/null +++ b/packages/db/migrations/0001_wahoo_route_update.sql @@ -0,0 +1,50 @@ +-- Data migration for the wahoo-route-update change. +-- +-- Why this lives outside drizzle-kit push: +-- `drizzle-kit push --force` will happily DROP `route_version` and try to add +-- the new unique index on (user_id, route_id, provider), but if any user has +-- successfully pushed multiple versions of a route the duplicate-row check on +-- the new unique would fail. We need to backfill `last_pushed_version` and +-- collapse rows BEFORE drizzle-kit reshapes the table. +-- +-- Idempotent: safe to run before every deploy until the old `route_version` +-- column is gone. Once `route_version` is dropped (i.e. drizzle-kit push has +-- run after this script), the body is a no-op. + +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM information_schema.columns + WHERE table_schema = 'journal' + AND table_name = 'sync_pushes' + AND column_name = 'route_version' + ) THEN + -- 1. Add the new column (nullable) if drizzle-kit push hasn't yet. + ALTER TABLE journal.sync_pushes + ADD COLUMN IF NOT EXISTS last_pushed_version integer; + + -- 2. Backfill last_pushed_version from route_version for every row. + UPDATE journal.sync_pushes + SET last_pushed_version = route_version + WHERE last_pushed_version IS NULL; + + -- 3. Collapse rows: per (user_id, route_id, provider), keep the row with + -- the highest route_version. Ties broken by created_at desc so we keep + -- the most recent. The kept row's remote_id (if any) is the live one. + DELETE FROM journal.sync_pushes a + USING journal.sync_pushes b + WHERE a.user_id = b.user_id + AND a.route_id = b.route_id + AND a.provider = b.provider + AND ( + a.route_version < b.route_version + OR (a.route_version = b.route_version AND a.created_at < b.created_at) + ); + + -- 4. Drop the old unique index that includes route_version. The new one + -- on (user_id, route_id, provider) is created by drizzle-kit push from + -- the schema definition. + DROP INDEX IF EXISTS journal.sync_pushes_user_route_version_provider_unique; + END IF; +END $$; diff --git a/packages/db/src/migrate-data.ts b/packages/db/src/migrate-data.ts new file mode 100644 index 0000000..d8299ec --- /dev/null +++ b/packages/db/src/migrate-data.ts @@ -0,0 +1,37 @@ +// Runs hand-written SQL data migrations from packages/db/migrations/*.sql in +// lexical order. Each file is expected to be idempotent (safe to re-run); +// there is no migrations table. +// +// Why this exists: we use `drizzle-kit push --force` for schema, which is +// fine for column adds and renames but unsafe when a unique-key change +// requires collapsing existing rows first. The wahoo-route-update change is +// the first such case. +import { readdirSync, readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import postgres from "postgres"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const migrationsDir = path.resolve(__dirname, "..", "migrations"); + +async function main() { + const url = process.env.DATABASE_URL ?? "postgres://trails:trails@localhost:5432/trails"; + const sql = postgres(url); + try { + const files = readdirSync(migrationsDir) + .filter((f) => f.endsWith(".sql")) + .sort(); + for (const f of files) { + const body = readFileSync(path.join(migrationsDir, f), "utf8"); + console.log(`[migrate-data] applying ${f}`); + await sql.unsafe(body); + } + } finally { + await sql.end(); + } +} + +main().catch((err) => { + console.error(err); + process.exit(1); +}); diff --git a/packages/db/src/schema/journal.ts b/packages/db/src/schema/journal.ts index e0618b8..1462d3f 100644 --- a/packages/db/src/schema/journal.ts +++ b/packages/db/src/schema/journal.ts @@ -220,10 +220,11 @@ export const syncImports = journalSchema.table("sync_imports", { }); // Tracks outbound route pushes to external providers (Wahoo today). One row -// per (user, route, version, provider) — push idempotency lives here. A row -// with `pushedAt` set means we have a confirmed remote_id and won't re-call -// the provider; a row with `error` set and no `pushedAt` is a failed attempt -// that can be retried in place. +// per (user, route, provider) — a logical Wahoo route is per-route, not +// per-version. `lastPushedVersion` records which local version is currently +// reflected on the remote side; `remoteId` lets subsequent pushes PUT in +// place. A row with `error` set and no `pushedAt` is a failed attempt that +// can be retried (POST if `remoteId` is null, PUT otherwise). export const syncPushes = journalSchema.table( "sync_pushes", { @@ -234,20 +235,19 @@ export const syncPushes = journalSchema.table( routeId: text("route_id") .notNull() .references(() => routes.id, { onDelete: "cascade" }), - routeVersion: integer("route_version").notNull(), provider: text("provider").notNull(), externalId: text("external_id").notNull(), remoteId: text("remote_id"), + lastPushedVersion: integer("last_pushed_version"), pushedAt: timestamp("pushed_at", { withTimezone: true }), error: text("error"), createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(), updatedAt: timestamp("updated_at", { withTimezone: true }).notNull().defaultNow(), }, (t) => ({ - userRouteVersionProviderUnique: uniqueIndex("sync_pushes_user_route_version_provider_unique").on( + userRouteProviderUnique: uniqueIndex("sync_pushes_user_route_provider_unique").on( t.userId, t.routeId, - t.routeVersion, t.provider, ), }), diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index 6f578f6..ee6346d 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -209,6 +209,8 @@ export default { sentToWahoo: "Am {{date}} an Wahoo gesendet", sendToWahooFailed: "Letzter Versuch fehlgeschlagen: {{error}}", sendToWahooSending: "Sende…", + onWahooNewer: "Auf Wahoo (v{{n}}) — lokale Version ist neuer", + sendUpdatedVersion: "Aktualisierte Version senden", sendToWahooBanner: { success: "Route an Wahoo gesendet. Schau in der Wahoo-App oder auf deinem Bike-Computer nach.", needsPermission: "Wir brauchen deine Erlaubnis, Routen an Wahoo zu senden. Bitte neu verbinden.", diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index 9583466..5bb5eca 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -209,6 +209,8 @@ export default { sentToWahoo: "Sent to Wahoo on {{date}}", sendToWahooFailed: "Last attempt failed: {{error}}", sendToWahooSending: "Sending…", + onWahooNewer: "On Wahoo (v{{n}}) — local version is newer", + sendUpdatedVersion: "Send updated version", sendToWahooBanner: { success: "Route sent to Wahoo. Check your Wahoo App or bike computer.", needsPermission: "We need your permission to send routes to Wahoo. Please reconnect.",