diff --git a/apps/mobile/src/features/pull-requests/pullRequestEnvironmentFallback.test.ts b/apps/mobile/src/features/pull-requests/pullRequestEnvironmentFallback.test.ts index 3ed5fe6b1835f..808bfa1b485ec 100644 --- a/apps/mobile/src/features/pull-requests/pullRequestEnvironmentFallback.test.ts +++ b/apps/mobile/src/features/pull-requests/pullRequestEnvironmentFallback.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from "vite-plus/test"; import type { EnvironmentProject } from "@t3tools/client-runtime/state/models"; +import type { EnvironmentConnectionPhase } from "@t3tools/client-runtime/connection"; +import { EnvironmentId } from "@t3tools/contracts"; import { pullRequestFallbackEnvironment } from "./pullRequestEnvironmentFallback"; const project = (environmentId: string, id: string, canonicalKey: string) => @@ -20,24 +22,35 @@ const target = { repository: "pingdotgg/t3code", host: "github.com", }; +const environment = ( + environmentId: string, + connectionState: EnvironmentConnectionPhase = "connected", + isEnabled = true, +) => ({ environmentId: EnvironmentId.make(environmentId), connectionState, isEnabled }); describe("pullRequestFallbackEnvironment", () => { it("moves to a connected copy of the repository when the original is down", () => { expect( - pullRequestFallbackEnvironment(target, projects, new Set(["desktop", "server"])), + pullRequestFallbackEnvironment(target, projects, [ + environment("desktop"), + environment("server"), + ]), ).toEqual({ environmentId: "server", projectId: "t3-copy" }); }); it("stays put while the original is connected or nothing else holds the repository", () => { - expect(pullRequestFallbackEnvironment(target, projects, new Set(["laptop", "server"]))).toBe( - null, - ); - expect(pullRequestFallbackEnvironment(target, projects, new Set(["desktop"]))).toBe(null); + expect( + pullRequestFallbackEnvironment(target, projects, [ + environment("laptop"), + environment("server"), + ]), + ).toBe(null); + expect(pullRequestFallbackEnvironment(target, projects, [environment("desktop")])).toBe(null); }); it("derives the host from the original project when the route has none", () => { const { host: _host, ...hostless } = target; - expect(pullRequestFallbackEnvironment(hostless, projects, new Set(["server"]))).toEqual({ + expect(pullRequestFallbackEnvironment(hostless, projects, [environment("server")])).toEqual({ environmentId: "server", projectId: "t3-copy", }); diff --git a/apps/mobile/src/features/pull-requests/pullRequestEnvironmentFallback.ts b/apps/mobile/src/features/pull-requests/pullRequestEnvironmentFallback.ts index 7c1d234f92577..bfa31edfea756 100644 --- a/apps/mobile/src/features/pull-requests/pullRequestEnvironmentFallback.ts +++ b/apps/mobile/src/features/pull-requests/pullRequestEnvironmentFallback.ts @@ -1,4 +1,5 @@ import type { EnvironmentProject } from "@t3tools/client-runtime/state/models"; +import type { EnvironmentConnectionSummary } from "@t3tools/client-runtime/state/presentation"; export type PullRequestEnvironmentTarget = { readonly environmentId: string; @@ -10,9 +11,28 @@ export type PullRequestEnvironmentTarget = { export function pullRequestFallbackEnvironment( target: PullRequestEnvironmentTarget, projects: ReadonlyArray, - connectedEnvironmentIds: ReadonlySet, + environments: ReadonlyArray< + Pick + >, ) { - if (connectedEnvironmentIds.has(target.environmentId)) return null; + const requested = environments.find( + (environment) => environment.environmentId === target.environmentId, + ); + // Pending connections retain the requested target until an explicit failure, even before + // its project hydrates. Available includes the period before connection startup. + if ( + requested?.isEnabled && + (requested.connectionState === "connected" || + requested.connectionState === "available" || + requested.connectionState === "connecting" || + requested.connectionState === "reconnecting") + ) + return null; + const connectedEnvironmentIds = new Set( + environments + .filter((environment) => environment.isEnabled && environment.connectionState === "connected") + .map((environment) => environment.environmentId), + ); const own = projects.find( (project) => project.environmentId === target.environmentId && project.id === target.projectId, ); diff --git a/apps/mobile/src/features/pull-requests/usePullRequestEnvironmentFallback.test.tsx b/apps/mobile/src/features/pull-requests/usePullRequestEnvironmentFallback.test.tsx new file mode 100644 index 0000000000000..1500f429bb606 --- /dev/null +++ b/apps/mobile/src/features/pull-requests/usePullRequestEnvironmentFallback.test.tsx @@ -0,0 +1,143 @@ +import type { EnvironmentConnectionPhase } from "@t3tools/client-runtime/connection"; +import type { EnvironmentProject } from "@t3tools/client-runtime/state/models"; +import { EnvironmentId } from "@t3tools/contracts"; +import { act, useEffect, useState } from "react"; +import { createRoot } from "react-dom/client"; +import { expect, it, vi } from "vite-plus/test"; + +import type { WorkspaceEnvironment } from "../../state/workspaceModel"; +import type { PullRequestEnvironmentTarget } from "./pullRequestEnvironmentFallback"; + +const mocks = vi.hoisted(() => ({ + environments: [] as ReadonlyArray, + isReady: true, + projects: [] as ReadonlyArray, + navigation: { setParams: vi.fn<(params: Partial) => void>() }, +})); + +vi.mock("@react-navigation/native", () => ({ useNavigation: () => mocks.navigation })); +vi.mock("../../state/entities", () => ({ useProjects: () => mocks.projects })); +vi.mock("../../state/workspace", () => ({ + useWorkspaceEnvironments: () => mocks.environments, + useConnectionsReady: () => mocks.isReady, +})); + +import { usePullRequestEnvironmentFallback } from "./usePullRequestEnvironmentFallback"; + +function environment( + environmentId: string, + connectionState: EnvironmentConnectionPhase, + isEnabled = true, +): WorkspaceEnvironment { + return { + environmentId: EnvironmentId.make(environmentId), + environmentLabel: environmentId, + displayUrl: "", + isRelayManaged: false, + isEnabled, + connectionState, + connectionError: null, + connectionErrorTraceId: null, + }; +} + +function setup() { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + mocks.isReady = true; + mocks.environments = [environment("requested", "connecting"), environment("copy", "connected")]; + mocks.projects = [ + { environmentId: "requested", id: "original" }, + { environmentId: "copy", id: "copy-project" }, + ].map( + (project) => + ({ + ...project, + repositoryIdentity: { canonicalKey: "github.com/owner/repo" }, + }) as unknown as EnvironmentProject, + ); + const container = document.createElement("div"); + const root = createRoot(container); + mocks.navigation.setParams = vi.fn(); + function Probe() { + const [params, updateParams] = useState({ + environmentId: "requested", + projectId: "original", + repository: "owner/repo", + host: "github.com", + }); + useEffect(() => { + mocks.navigation.setParams.mockImplementation((next) => + updateParams((previous) => ({ ...previous, ...next })), + ); + }, []); + usePullRequestEnvironmentFallback(params); + return {`${params.environmentId}/${params.projectId}`}; + } + return { + render() { + act(() => root.render()); + }, + get target() { + return container.textContent; + }, + [Symbol.dispose]() { + act(() => root.unmount()); + }, + }; +} + +it("keeps the requested target while the catalog initializes or its enabled connection is pending", () => { + using screen = setup(); + mocks.isReady = false; + mocks.environments = [environment("copy", "connected")]; + screen.render(); + expect(screen.target).toBe("requested/original"); + expect(mocks.navigation.setParams).not.toHaveBeenCalled(); + + mocks.isReady = true; + for (const phase of ["available", "connecting", "reconnecting", "connected"] as const) { + mocks.environments = [environment("requested", phase), environment("copy", "connected")]; + screen.render(); + expect(screen.target).toBe("requested/original"); + expect(mocks.navigation.setParams).not.toHaveBeenCalled(); + } +}); + +it("falls back to a connected repository copy when the requested environment cannot connect", () => { + for (const state of ["missing", "disabled", "offline", "error", "unsupported"] as const) { + using screen = setup(); + mocks.environments = [ + ...(state === "missing" + ? [] + : [ + environment( + "requested", + state === "disabled" ? "available" : state, + state !== "disabled", + ), + ]), + environment("copy", "connected"), + ]; + screen.render(); + expect(screen.target).toBe("copy/copy-project"); + expect(mocks.navigation.setParams).toHaveBeenCalledExactlyOnceWith({ + environmentId: "copy", + projectId: "copy-project", + }); + } +}); + +it("does not replace a connecting target with a connected repository copy before its project hydrates", () => { + using screen = setup(); + const projects = mocks.projects; + mocks.projects = projects.filter((project) => project.environmentId !== "requested"); + screen.render(); + expect(screen.target).toBe("requested/original"); + expect(mocks.navigation.setParams).not.toHaveBeenCalled(); + + mocks.projects = projects; + mocks.environments = [environment("requested", "connected"), environment("copy", "connected")]; + screen.render(); + expect(screen.target).toBe("requested/original"); + expect(mocks.navigation.setParams).not.toHaveBeenCalled(); +}); diff --git a/apps/mobile/src/features/pull-requests/usePullRequestEnvironmentFallback.ts b/apps/mobile/src/features/pull-requests/usePullRequestEnvironmentFallback.ts index 7fb13669642f0..11a3e667fb32b 100644 --- a/apps/mobile/src/features/pull-requests/usePullRequestEnvironmentFallback.ts +++ b/apps/mobile/src/features/pull-requests/usePullRequestEnvironmentFallback.ts @@ -2,7 +2,7 @@ import { useNavigation } from "@react-navigation/native"; import { useEffect, useMemo } from "react"; import { useProjects } from "../../state/entities"; -import { useWorkspaceEnvironments } from "../../state/workspace"; +import { useConnectionsReady, useWorkspaceEnvironments } from "../../state/workspace"; import { pullRequestFallbackEnvironment, type PullRequestEnvironmentTarget, @@ -11,13 +11,11 @@ import { export function usePullRequestEnvironmentFallback(params: PullRequestEnvironmentTarget) { const navigation = useNavigation(); const projects = useProjects(); - const connected = useWorkspaceEnvironments() - .filter((environment) => environment.connectionState === "connected") - .map((environment) => String(environment.environmentId)) - .join("\n"); + const isReady = useConnectionsReady(); + const environments = useWorkspaceEnvironments(); const fallback = useMemo( - () => pullRequestFallbackEnvironment(params, projects, new Set(connected.split("\n"))), - [params, projects, connected], + () => (isReady ? pullRequestFallbackEnvironment(params, projects, environments) : null), + [params, projects, environments, isReady], ); useEffect(() => { if (fallback) navigation.setParams(fallback); diff --git a/apps/mobile/vite.config.ts b/apps/mobile/vite.config.ts index 3c4a8bf400012..69728880a79b7 100644 --- a/apps/mobile/vite.config.ts +++ b/apps/mobile/vite.config.ts @@ -2,6 +2,7 @@ import { defineConfig } from "vite-plus"; import "vite-plus/test/config"; const reactTests = [ + "src/features/pull-requests/usePullRequestEnvironmentFallback.test.tsx", "src/lib/filePreview.test.ts", "src/features/diffs/nativeReviewDiffSurface.test.tsx", "src/features/pull-requests/usePullRequestDiff.test.tsx",