- Add STEP_FIELDS constant mapping each step to its required fields - Extract trigger() from useForm and call it on Next click - Only advance to next step when trigger() returns true, blocking silent failures where invalid data could reach the Review step - Add CreateServerPage.test.tsx with 8 tests covering step navigation, validation blocking, happy path, and submit mutation - Update CLAUDE.md: mark /servers/new Complete, remove resolved bug - Mark implementation plan as completed
147 lines
4.9 KiB
Markdown
147 lines
4.9 KiB
Markdown
# Implementation Plan: Fix Create Server Submit Button
|
||
|
||
> **Status: IMPLEMENTED** — All 3 code changes applied, 8 tests added, 128/128 passing.
|
||
> Implemented: 2026-04-17
|
||
|
||
## Task Type
|
||
- [x] Frontend only
|
||
|
||
## Problem Analysis
|
||
|
||
**Root Cause** (`frontend/src/pages/CreateServerPage.tsx:266-273`):
|
||
|
||
The "Next" button advances steps unconditionally:
|
||
```tsx
|
||
onClick={() => setStep(step + 1)}
|
||
```
|
||
|
||
Users can proceed through steps 0→1→2→3 with invalid data in steps 0–2. On step 3, `handleSubmit(onSubmit)` runs Zod validation across the whole schema — if any required field (name, exe_path, game_port, game_type) is empty or malformed, validation fails and `onSubmit` is **never called**. There is no error shown because RHF simply blocks submission silently when in an invalid state.
|
||
|
||
The step 3 error block (lines 229-237) only shows `errors` that RHF knows about — but RHF only populates `errors` after a submit attempt, not after just navigating to a step. So the Review step also appears clean even when data is bad.
|
||
|
||
## Technical Solution
|
||
|
||
Extract `trigger` from `useForm` and call it per-step before advancing.
|
||
`trigger(fields)` runs Zod validation for the given fields only, populates `formState.errors`,
|
||
and returns `true` if all pass. Only advance to the next step when it returns `true`.
|
||
|
||
### Per-step field mapping
|
||
|
||
| Step | Label | Fields to validate |
|
||
|------|-------|-------------------|
|
||
| 0 | Game Type | `game_type` |
|
||
| 1 | Server Info | `name`, `exe_path`, `game_port`, `rcon_port` |
|
||
| 2 | Options | `max_restarts` |
|
||
| 3 | Review | (submit — no additional trigger needed) |
|
||
|
||
## Implementation Steps
|
||
|
||
### Step 1 — Extract `trigger` from `useForm`
|
||
|
||
**File:** `frontend/src/pages/CreateServerPage.tsx:38-56`
|
||
|
||
Change:
|
||
```tsx
|
||
const {
|
||
register,
|
||
handleSubmit,
|
||
watch,
|
||
setValue,
|
||
formState: { errors },
|
||
} = useForm<CreateServerForm>({ ... });
|
||
```
|
||
|
||
To:
|
||
```tsx
|
||
const {
|
||
register,
|
||
handleSubmit,
|
||
watch,
|
||
setValue,
|
||
trigger,
|
||
formState: { errors },
|
||
} = useForm<CreateServerForm>({ ... });
|
||
```
|
||
|
||
### Step 2 — Add per-step field mapping constant
|
||
|
||
Add after `const STEPS = ["Game Type", "Server Info", "Options", "Review"];`:
|
||
|
||
```tsx
|
||
const STEP_FIELDS: Array<(keyof CreateServerForm)[]> = [
|
||
["game_type"], // step 0
|
||
["name", "exe_path", "game_port", "rcon_port"], // step 1
|
||
["max_restarts"], // step 2
|
||
[], // step 3 (review — submit handles it)
|
||
];
|
||
```
|
||
|
||
### Step 3 — Replace Next button onClick with validated handler
|
||
|
||
**File:** `frontend/src/pages/CreateServerPage.tsx:266-274`
|
||
|
||
Change:
|
||
```tsx
|
||
<button
|
||
type="button"
|
||
onClick={() => setStep(step + 1)}
|
||
className="btn-primary flex items-center gap-1.5"
|
||
>
|
||
Next
|
||
<ChevronRight size={16} />
|
||
</button>
|
||
```
|
||
|
||
To:
|
||
```tsx
|
||
<button
|
||
type="button"
|
||
onClick={async () => {
|
||
const valid = await trigger(STEP_FIELDS[step]);
|
||
if (valid) setStep(step + 1);
|
||
}}
|
||
className="btn-primary flex items-center gap-1.5"
|
||
>
|
||
Next
|
||
<ChevronRight size={16} />
|
||
</button>
|
||
```
|
||
|
||
### Step 4 — Add CreateServerPage unit tests
|
||
|
||
**File:** `frontend/src/__tests__/CreateServerPage.test.tsx` (new file)
|
||
|
||
Tests to cover:
|
||
|
||
1. **Renders step 0 by default** — Game Type selector visible
|
||
2. **Next on step 0 advances to step 1** — server info fields visible
|
||
3. **Next on step 1 with empty name blocks advance** — stays on step 1, error message shown
|
||
4. **Next on step 1 with empty exe_path blocks advance** — stays on step 1, error shown
|
||
5. **Next on step 1 with valid data advances to step 2** — options fields visible
|
||
6. **Back button on step 1 returns to step 0**
|
||
7. **Full happy path reaches Review step** — review table visible
|
||
8. **Submit fires createServer mutation on valid data**
|
||
|
||
Test setup: mock `useCreateServer`, `useGamesList`, `useAuthStore` (admin), `useUIStore`.
|
||
|
||
## Key Files
|
||
|
||
| File | Operation | Description |
|
||
|------|-----------|-------------|
|
||
| `frontend/src/pages/CreateServerPage.tsx:38-56` | Modify | Add `trigger` to useForm destructure |
|
||
| `frontend/src/pages/CreateServerPage.tsx:27` | Modify | Add `STEP_FIELDS` constant after `STEPS` |
|
||
| `frontend/src/pages/CreateServerPage.tsx:266-274` | Modify | Replace Next onClick with async trigger-guarded handler |
|
||
| `frontend/src/__tests__/CreateServerPage.test.tsx` | Create | Unit tests for wizard validation |
|
||
|
||
## Risks and Mitigation
|
||
|
||
| Risk | Mitigation |
|
||
|------|------------|
|
||
| `trigger` for `rcon_port` (nullable optional) might block valid empty input | `rcon_port` schema is `.nullable().optional()` so empty/null passes — no issue |
|
||
| `max_restarts` has `valueAsNumber` — NaN on empty input | Schema has `.min(0).max(20).optional()` with coerce; default is 3 so this won't be empty in practice |
|
||
| Async onClick on a `type="button"` | Safe — button is not `type="submit"`, no double-submit risk |
|
||
|
||
## SESSION_ID
|
||
- CODEX_SESSION: N/A (codeagent-wrapper not available)
|
||
- GEMINI_SESSION: N/A (codeagent-wrapper not available)
|