Treat a whitespace-only place description as no change #138

Merged

rob merged 1 commit from task-194-unchanged-form-discard-prompt into main

2026-08-12 08:19:36 +00:00

rob commented

2026-08-12 07:56:55 +00:00

Owner

Closes task 194. Diagnosed before fixing, by testing rather than reading — the ticket's own lead confirmed, the other two disproved:

Reproduced: PlaceForm.NormalizeDescription used IsNullOrEmpty, not IsNullOrWhiteSpace like the repository's own. Typing only spaces into a description read as dirty even against a form that opened with none — in both add mode and edit mode (stored null and stored legacy "" both checked). Fixed by matching the repository's own threshold, so whatever would not actually persist as a description does not count as a change.
"Save, then dismiss": not reproducible. PlaceForm has exactly one host, PlaceFormPanel, whose HandleSavedAsync closes unconditionally after every successful save — there is no code path today where a saved form stays open to be dismissed from. Checked PlaceForm.IsDirty directly instead, simulating what a future host that stayed open would do (re-render with Place updated to the saved values): already correct, and now held by a committed test.
Group selector in edit mode: not applicable. Editing renders no group control at all (group_id cannot change through this form) — already proven by an existing test, nothing to fix.
"Type then undo exactly" and "touch nothing": already covered by existing tests, all passing before this change.

Mutation-tested: reverted the threshold change, the three whitespace tests reddened; reverted nothing for the save-then-dismiss test (the code was already correct), so mutated IsDirty itself to compare against a frozen name instead of live Place — reddened exactly that test, restored.

Avoided PlaceFormPanel.razor/.razor.cs/.css entirely — PR #137 is mid-flight there.

No ADR — a one-line threshold fix reversing an explicitly documented prior decision, argued in the code comment it replaces; not a shape-of-the-system decision.

1799 total / 1789 passing / 10 skipped (4 WebUI repro tests, 6 E2E — no browser in this environment). dotnet format --verify-no-changes clean, coverage ratchet holds.

Closes task 194. Diagnosed before fixing, by testing rather than reading — the ticket's own lead confirmed, the other two disproved: - **Reproduced**: `PlaceForm.NormalizeDescription` used `IsNullOrEmpty`, not `IsNullOrWhiteSpace` like the repository's own. Typing only spaces into a description read as dirty even against a form that opened with none — in both add mode and edit mode (stored `null` and stored legacy `""` both checked). Fixed by matching the repository's own threshold, so whatever would not actually persist as a description does not count as a change. - **"Save, then dismiss": not reproducible.** `PlaceForm` has exactly one host, `PlaceFormPanel`, whose `HandleSavedAsync` closes unconditionally after every successful save — there is no code path today where a saved form stays open to be dismissed from. Checked `PlaceForm.IsDirty` directly instead, simulating what a future host that stayed open would do (re-render with `Place` updated to the saved values): already correct, and now held by a committed test. - **Group selector in edit mode: not applicable.** Editing renders no group control at all (`group_id` cannot change through this form) — already proven by an existing test, nothing to fix. - **"Type then undo exactly" and "touch nothing": already covered** by existing tests, all passing before this change. Mutation-tested: reverted the threshold change, the three whitespace tests reddened; reverted nothing for the save-then-dismiss test (the code was already correct), so mutated `IsDirty` itself to compare against a frozen name instead of live `Place` — reddened exactly that test, restored. Avoided `PlaceFormPanel.razor`/`.razor.cs`/`.css` entirely — PR #137 is mid-flight there. No ADR — a one-line threshold fix reversing an explicitly documented prior decision, argued in the code comment it replaces; not a shape-of-the-system decision. 1799 total / 1789 passing / 10 skipped (4 WebUI repro tests, 6 E2E — no browser in this environment). `dotnet format --verify-no-changes` clean, coverage ratchet holds.

rob added 1 commit

2026-08-12 07:56:55 +00:00

Treat a whitespace-only place description as no change

CI / build (pull_request) Successful in 3m10s

Details

CI / container-images (pull_request) Has been skipped

Details

CI / e2e (pull_request) Successful in 1m41s

Details

0f472f0077

rob reviewed

2026-08-12 08:19:10 +00:00

rob left a comment

Verdict: mergeable

Doc-comment check: IsDirty's remarks and NormalizeDescription's summary both restate the PR #126-era distinction and explain why it's now collapsed rather than silently dropping it. ADR-0112's "independent by design" language is about rule ownership (fixing IsDirty doesn't touch what a save writes), not a promise the two thresholds stay different forever — still literally true, no metadata update needed.
Mutation-tested NormalizeDescription myself (IsNullOrWhiteSpace→IsNullOrEmpty): the three named whitespace tests reddened with the expected IsDirty assertion failure, restored, green again.
Verified both disproven leads against the code directly: HandleSavedAsync calls StartClosingAsync() unconditionally, no branch leaves a saved form open; PlaceForm.razor's group <select> is wrapped in @if (!IsEdit), confirmed no control renders in edit mode, matching the existing test.
Verified end-to-end through PlaceFormPanel with a scratch bUnit test (typed whitespace, clicked the backdrop, confirmed no .place-form-panel-discard-confirm and OnClose fired) — not committed, but proves the fix holds through the real dismiss path, not just the predicate.
Full solution run: 1799 total / 1789 passing / 10 skipped, matches the PR body exactly. dotnet format --verify-no-changes clean. Diff confirmed limited to PlaceForm.razor.cs and its tests — no PlaceFormPanel files touched.
CI (#587) independently confirmed green against head 0f472f0.

Verdict: mergeable - Doc-comment check: `IsDirty`'s remarks and `NormalizeDescription`'s summary both restate the PR #126-era distinction and explain why it's now collapsed rather than silently dropping it. ADR-0112's "independent by design" language is about rule *ownership* (fixing `IsDirty` doesn't touch what a save writes), not a promise the two thresholds stay different forever — still literally true, no metadata update needed. - Mutation-tested `NormalizeDescription` myself (`IsNullOrWhiteSpace`→`IsNullOrEmpty`): the three named whitespace tests reddened with the expected `IsDirty` assertion failure, restored, green again. - Verified both disproven leads against the code directly: `HandleSavedAsync` calls `StartClosingAsync()` unconditionally, no branch leaves a saved form open; `PlaceForm.razor`'s group `<select>` is wrapped in `@if (!IsEdit)`, confirmed no control renders in edit mode, matching the existing test. - Verified end-to-end through `PlaceFormPanel` with a scratch bUnit test (typed whitespace, clicked the backdrop, confirmed no `.place-form-panel-discard-confirm` and `OnClose` fired) — not committed, but proves the fix holds through the real dismiss path, not just the predicate. - Full solution run: 1799 total / 1789 passing / 10 skipped, matches the PR body exactly. `dotnet format --verify-no-changes` clean. Diff confirmed limited to `PlaceForm.razor.cs` and its tests — no `PlaceFormPanel` files touched. - CI (#587) independently confirmed green against head `0f472f0`.

rob merged commit 485658358c into main

2026-08-12 08:19:36 +00:00

rob deleted branch task-194-unchanged-form-discard-prompt

2026-08-12 08:19:36 +00:00

rob referenced this pull request from a commit

2026-08-12 08:19:37 +00:00

Treat a whitespace-only place description as no change (#138)