diff --git a/web/OffscaleServerPicker.tsx b/web/OffscaleServerPicker.tsx index 9f430d0..1f52e03 100644 --- a/web/OffscaleServerPicker.tsx +++ b/web/OffscaleServerPicker.tsx @@ -1,6 +1,6 @@ import { Check, Network, Plus } from 'lucide-react'; import { Link } from 'react-router'; -import { offscaleSectionPath } from './shared'; +import { offscaleRegisterPath } from './shared'; import { useOffscaleServers } from './useOffscaleServers'; // Top-left panel of the /offscale workspace: which server everything else acts on. @@ -45,9 +45,13 @@ export const OffscaleServerPicker = () => { ))} + {/* Goes to the Servers section AND opens the form. A link to the section alone was a no-op from the + place you are most likely to be standing: `/offscale` and `/offscale/servers` render the same + view, so the navigation was invisible and the only visible result was a second button with the + same label. */} {servers.length === 0 && !isLoading && ( diff --git a/web/ServersView.test.tsx b/web/ServersView.test.tsx new file mode 100644 index 0000000..05e8b31 --- /dev/null +++ b/web/ServersView.test.tsx @@ -0,0 +1,84 @@ +import type { ReactNode } from 'react'; +import { afterEach, describe, expect, test } from 'bun:test'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { MemoryRouter, useLocation } from 'react-router'; +import { render, screen, waitFor } from '@testing-library/react'; +import { ServersView } from './ServersView'; +import { offscaleRegisterPath } from './shared'; + +/** + * Whether "Register a server" opens the form — from every place that offers it. + * + * The bug this pins was reported as "I click register a server and nothing happens", and nothing was + * broken: three controls carry that label and the one in the server picker was a plain link to the Servers + * section. `/offscale` and `/offscale/servers` render the same view, so from the section you were already + * on it changed the URL and nothing else — a control that looks like the primary action and is a no-op. + * + * So the test is about the link's DESTINATION doing something, which is why it renders the view at that + * destination rather than clicking the picker: the picker lives in another panel, and what has to be true + * is that arriving there opens the form. + * + * `fetch` is stubbed rather than `mock.module('hooks/useClient')` — Bun has one module registry per run, so + * that mock is process-wide and permanent, and other files already own it. + */ +const realFetch = globalThis.fetch; +afterEach(() => { + globalThis.fetch = realFetch; +}); + +const stubServers = (servers: unknown[] = []) => { + globalThis.fetch = (async () => + new Response(JSON.stringify({ servers }), { + status: 200, + headers: { 'content-type': 'application/json' }, + })) as unknown as typeof fetch; +}; + +/** Reports the live URL, so "the flag was consumed" is an assertion rather than an inference. */ +const Search = () => {useLocation().search}; + +const Wrap = ({ at, children }: { at: string; children: ReactNode }) => ( + + + {children} + + + +); + +const mountAt = (at: string) => + render( + + + , + ); + +const formHeading = () => screen.queryByText('Register a Headscale server'); +const search = () => screen.getByTestId('search').textContent; + +describe('ServersView registration form', () => { + test('the picker link lands with the form already open', async () => { + stubServers([]); + mountAt(offscaleRegisterPath()); + + await waitFor(() => expect(formHeading()).not.toBeNull()); + }); + + test('the flag is consumed, so clicking the picker link again still works', async () => { + stubServers([]); + mountAt(offscaleRegisterPath()); + + // Left in the URL, a second click on the picker would be a navigation to the URL already showing — + // which is the original bug, restored. + await waitFor(() => expect(search()).toBe('')); + }); + + test('without the flag the form stays closed', async () => { + stubServers([]); + mountAt('/offscale/servers'); + + // The empty state has to have rendered before "no form" means anything. + await waitFor(() => expect(screen.queryByText('No Headscale servers yet')).not.toBeNull()); + expect(formHeading()).toBeNull(); + }); +}); diff --git a/web/ServersView.tsx b/web/ServersView.tsx index e65e4fc..a0d5880 100644 --- a/web/ServersView.tsx +++ b/web/ServersView.tsx @@ -1,7 +1,8 @@ import { useEffect, useRef, useState } from 'react'; +import { useSearchParams } from 'react-router'; import { Plus, Loader2, Server, Check, Activity, Pencil, Trash2 } from 'lucide-react'; import type { OffscaleServer, OffscaleHealth } from './shared'; -import { MIN_OFFSCALE_VERSION } from './shared'; +import { MIN_OFFSCALE_VERSION, OFFSCALE_REGISTER_PARAM } from './shared'; import { useOffscaleServers, useOffscaleHealth, offscaleErrorMessage } from './useOffscaleServers'; import { Card, SectionHeader, Button, Dot, Badge, ErrorNote } from './Cards'; import { ServerForm } from './ServerForm'; @@ -136,11 +137,29 @@ export const ServersView = () => { const healthProbe = useOffscaleHealth(); const [formFor, setFormFor] = useState<'new' | OffscaleServer | null>(null); + const [searchParams, setSearchParams] = useSearchParams(); const [health, setHealth] = useState>({}); // Several probes are in flight at once now, so this is a set of ids rather than the one id it used to be. const [testingIds, setTestingIds] = useState([]); const [actionError, setActionError] = useState(null); + /* + The URL may ask for the form: the server picker links here with `?register=1` so its "Register a server" + opens the thing rather than landing you on a page that offers the same thing again. + + The flag is CONSUMED — read once, then stripped with `replace`. Leaving it in place would break the + control a second time: close the form, click the picker again, and the URL would already say + `?register=1`, so there would be no navigation and nothing would happen. Stripping it also means a + reload does not reopen a form you dismissed. + */ + useEffect(() => { + if (searchParams.get(OFFSCALE_REGISTER_PARAM) !== '1') return; + setFormFor('new'); + const next = new URLSearchParams(searchParams); + next.delete(OFFSCALE_REGISTER_PARAM); + setSearchParams(next, { replace: true }); + }, [searchParams, setSearchParams]); + const test = async (id: number) => { setTestingIds((prev) => (prev.includes(id) ? prev : [...prev, id])); setActionError(null); diff --git a/web/shared.ts b/web/shared.ts index 00b4c49..9f03422 100644 --- a/web/shared.ts +++ b/web/shared.ts @@ -26,6 +26,20 @@ export const isOffscaleSection = (value: string | undefined): value is OffscaleS /** The one place the section URL is spelled, so the nav, the guard and any deep link cannot drift apart. */ export const offscaleSectionPath = (id: OffscaleSectionId) => `/offscale/${id}`; +/** + * A request, in the URL, to open the registration form. + * + * The server picker's "Register a server" used to be a plain link to the Servers section — which renders + * identically at `/offscale` and `/offscale/servers`, so clicking it changed the URL and nothing else and + * read as a dead control. Three things on that screen carry this label; this is what makes the first one + * do what the other two do. + * + * It is a one-shot flag rather than addressable state: ServersView consumes it and strips it, so closing + * the form and clicking again works, and a reload does not reopen a form you dismissed. + */ +export const OFFSCALE_REGISTER_PARAM = 'register'; +export const offscaleRegisterPath = () => `${offscaleSectionPath('servers')}?${OFFSCALE_REGISTER_PARAM}=1`; + /** A registered Headscale server. The API key is never included — it stays encrypted in Postgres. */ export type OffscaleServer = { id: number;