Content - Fix Meta Title validation racing the field-debounce commit by geodem127 · Pull Request #4324 · zesty-io/manager-ui

geodem127 · 2026-09-09T01:29:39Z

Summary

Fixes #4276 ("Creating New Content in a Dataset Model Requires SEO Information") by fixing the actual race condition, and restores the product direction stated in #2984 (Meta Title optional for datasets, like Meta Description already is), rather than making Meta Title required-but-auto-populated everywhere.

Root cause of #4276: Field.tsx debounces every field's onChange commit to the store by 500ms (useDebouncedInput). Editor.js's first-text-field auto-population of Meta Title/Meta Link Text/path part runs inside that same debounced commit. If Save is clicked within 500ms of the last keystroke, the auto-populated value hasn't reached the Redux store yet, so client-side validation sees it as missing and blocks the save — even though the field visibly shows a value on screen. (The existing meta.spec.js test "Does not validate meta description for dataset items" already worked around this exact race with a hardcoded cy.wait(500) before Save.)

A separate, compounding bug: ItemCreate.tsx's save() called metaRef.current.validateMetaFields() but discarded its return value, instead gating the save on possibly-stale SEOErrors state. This let some invalid saves reach the createItem thunk, which returned { err: "VALIDATION_ERROR" } with none of the fields ItemCreate.tsx's error handling recognizes — so the failure was swallowed silently, leaving the user stuck on /new with no visible error.

Fix for the race, revised: an earlier version of this PR fixed the race by flushing every registered field (via refRegistry + flushSync) at save time. Review feedback flagged real problems with that: it could force-commit an unrelated parent page's in-progress edits when a nested "Create & Add New Related Item" dialog saves, flushSync forced a synchronous full re-render on every Save click, and refRegistry's name-only keying meant two same-named fields mounted at once could silently drop the flush. Rather than patch around those, the fix now removes the flush entirely and addresses the race at its source:

Field.tsx takes a new isAutoPopulateSource prop; Editor.js sets it only on the one field (first text/content field) that actually drives Meta Title/Meta Link Text/pathPart auto-population, and only for new items on non-block models.
useDebouncedInput commits that field's value synchronously (delay <= 0) instead of debouncing it, so its own onChange — not Save — is what lands the value in the Redux store. By the time Save is clicked (a separate, later event), the store is already current.
ItemCreate.tsx no longer imports refRegistry or uses flushSync at all; save() just reads validateMetaFields()'s return value directly, matching the pattern already used in ItemEdit.js.
RefHandle's flush method and useDebouncedInput's flush export are removed as dead code now that nothing calls them.

Restoring #2984's intent: an earlier commit on this branch fixed #4276 by making Meta Title required for all types and relying on the auto-population fix above. That contradicts explicit, still-relevant product direction from #2984: Meta Title (like Meta Description already does) should be required only for single/multi-page items, optional for datasets, with the asterisk removed. Restored that — content.js, Meta/index.tsx's REQUIRED_FIELDS, and MetaTitle.tsx's required prop once again treat dataset models as SEO-exempt. The debounce fix is unaffected and still needed for page/multi-page item auto-population.

Whitespace-only regression (found by this PR's own negative-QA review): every required-field check here used !value, which treats a string of spaces as truthy. This let a whitespace-only Meta Title through validation on models where it's still required (single/multi-page items), leaving items with no visible title anywhere. Added an isBlank() helper that trims before checking presence, applied everywhere Meta Title/parentZUID/pathPart/dynamic OG-TC fields are validated (content.js's createItem thunk, Meta/index.tsx's live handleOnChange and validateMetaFields). Also fixed handleOnChange's useCallback missing REQUIRED_FIELDS/metaFields from its dependency array now that REQUIRED_FIELDS varies by model type.

No new e2e coverage added — the existing meta.spec.js suite already exercises dataset item creation and covers this path.

Test plan

cypress/e2e/content/meta.spec.js — all 4 existing tests pass, including "Does not validate meta description for dataset items" which creates a dataset item via the same save flow (its hardcoded cy.wait(500) workaround is removed since the race is now fixed at the source).
cypress/e2e/content/content.spec.js — full regression pass against the Field.tsx change (46/46 passing, 5 pending as expected).
Manually verified against the live dev instance across pageset, templateset, and dataset model types: (1) typing into the auto-population field then clicking Save immediately, (2) typing then clearing it then clicking Save immediately, and (3) clicking Save as fast as possible with no input at all. pageset/templateset correctly block a blank/whitespace title with a visible "Required Field" error and never leak one through; dataset saves cleanly in all cases since Meta Title isn't required there. block-type models use a separate "Create Variant" UI (BlockItem.tsx) and don't exercise this code path at all.
npx tsc --noEmit — clean.

🤖 Generated with Claude Code

github-actions · 2026-09-09T01:31:59Z

Acceptance Criteria QA — ✅ PASS — 4/4 code-checkable criteria confirmed

Validates #4276: Manager UI - Creating New Content in a Dataset Model Requires SEO Information

✅ Creating a new dataset item no longer requires manually providing a Meta Title — REQUIRED_FIELDS in Meta/index.tsx skips metaTitle when model?.type === "dataset", MetaTitle receives required={false}, and hasMissingRequiredSEOFields in store/content.js skips the check for dataset models.
✅ Creating a new dataset item no longer requires pathPart/parentZUID — validateMetaFields deletes those error keys for dataset (and block/homepage) models, consistent with datasets never getting a path part.
✅ Non-dataset models still enforce the existing Meta Title/path-part requirements — the dataset exclusions are additive (model?.type !== "dataset"), leaving block's pre-existing exclusion and all other model types unchanged.
✅ The underlying intermittent race (auto-populated Meta Title from the first text/content field not yet committed to the store when Save is clicked) is closed — isAutoPopulateSource forces useDebouncedInput to commit synchronously (delay <= 0) for the field that seeds metaTitle/pathPart, and ItemCreate.save now reads validateMetaFields()'s return value directly instead of relying on the async hasSEOErrors state update.

Suggested Cypress coverage

cypress/e2e/content/meta.spec.js already has "Does not validate meta description for dataset items", updated by this PR to drop its cy.wait(500) — that alone locks in the synchronous-commit fix for the happy path. Worth adding: (1) a case that types into the first text field and clicks "Create Item" back-to-back with no waits at all (simulating a fast user) to directly exercise the race the debounce fix targets; (2) a case creating an item on a non-dataset model with Meta Title left blank, asserting the save is blocked and the existing required-field error still appears, to guard the regression risk in point 3; (3) a case confirming path part/parent fields are not shown or required for a dataset item, complementing the current spec's Meta Title/description coverage; (4) a case typing only whitespace into Meta Title on a non-dataset model, asserting the save is still blocked, to cover the new isBlank whitespace-trim behavior added since the prior review.

github-actions · 2026-09-09T01:34:01Z

Code Review — ✅ No blockers

github-actions · 2026-09-09T01:44:47Z

Adversarial Browser QA

🔴 Rapid double-click on Create-Item Save fires two POSTs, creating duplicate items (or a stray 400)

Type: pre-existing on the touched surface
Steps:

Go to /content/6-bab6b795f8-fbbz95/new (any "QA Negative Test" content item creation page).
Choose "No, I will improve and edit it myself" for the AI meta-data prompt.
Type a unique value into the first Text * field.
Fire two clicks on [data-cy="CreateItemSaveButton"] back-to-back in the same tick (e.g. btn.click(); btn.click();) — this reproduces what a fast real double-click does before React re-renders the disabled state.
Expected: Exactly one POST .../items request, one item created.
Actual: Two POST .../items requests fire. Reproduced twice: once both returned 201, creating two separate content items with the identical title "SyncDblClick NegQA4324 Test2" (visible as two distinct rows in the list); the second time the first returned 201 and the second returned 400, which surfaces as an uncaught console error and "Missing Item" log spam even though the save appeared to succeed and the user was redirected normally. The save() handler in ItemCreate.tsx only calls setSaving(true) (which disables the Save button via isLoading) after its synchronous validation check — there's a window where a second click event, processed before React commits that state update, re-enters save() and issues a second create request. Nothing in this PR's diff adds a submission guard, so this race predates the change, but it lives on the exact save() path the PR modifies (validation timing in ItemCreate.tsx/Meta/index.tsx).
Console/network: Run 1: two POST /v1/content/models/6-bab6b795f8-fbbz95/items both 201. Run 2: one 201, one 400, followed by console errors Failed to load resource: 400 ... /items and Missing Item: new:6-bab6b795f8-fbbz95 / Missing Item: 7-e083a796c0-942m7d.

🔵 Create → Edit transition intermittently throws a React "uncontrolled input becoming controlled" warning on MetaTitle

Type: pre-existing on the touched surface
Steps:

Go to /content/6-8cf1d2d7e4-sqbrdh/new (a "Globals" dataset-type item creation page).
Choose "No, I will improve and edit it myself" for the AI meta-data prompt.
Type a value into "Site Name" (auto-populates Meta Title / Navigation Link Text via the first-text-field auto-populate logic).
Click into the "Meta Title" field, select all, and delete it so it's genuinely blank (0/150).
Click [data-cy="CreateItemSaveButton"].
Expected: Item saves cleanly with no console warnings.
Actual: The item saves successfully (POST .../items returns 201) and the app redirects to the new item's edit view, but that transition throws Warning: A component is changing an uncontrolled input to be controlled. This is likely caused by the value changing from undefined to a defined value... with a stack trace rooted at the MetaTitle component (FieldShell → MetaTitle). Reproduced twice with this exact dataset + blank-Meta-Title recipe, identical warning and stack both times. However, this is not exclusive to the blank/dataset case: the same warning with the same MetaTitle stack frame was also observed once on a non-dataset model ("QA Negative Test") with a non-blank, auto-populated Meta Title, while two other plain baseline saves on that same model produced no warning at all — so the trigger looks like a pre-existing timing race in the ItemCreate → ItemEdit remount (the MetaTitle input's controlled value briefly starts undefined), not something newly introduced by this PR's dataset/required-field logic. Flagging as cosmetic/dev-console-only since it doesn't block the save or corrupt data, but it lands squarely in MetaTitle.tsx, a file this PR modifies.
Console/network: Dataset repro (both runs): POST /v1/content/models/6-8cf1d2d7e4-sqbrdh/items → 201, followed immediately by the warning above on the resulting ItemEdit page load.

Also checked and working correctly

Dataset-type model ("Globals") item creation — the Meta panel no longer shows a * on "Meta Title" and it is not required, matching the model?.type !== "dataset" change in Meta/index.tsx. A dataset item genuinely saves with a blank Meta Title (POST .../items returns 201).
A plain (non-rapid) double-click via Playwright's native dblclick() on the same Save button did not reproduce the duplicate-submit issue — only truly back-to-back synchronous clicks did, since the two events land far enough apart for React to disable the button in between.
Whitespace-only Meta Title on a non-dataset model ("QA Negative Test") is correctly caught by isBlank() — the required-field error surfaces and blocks save.
<script> tags and &-containing markup typed into the first Text * field are safely rendered as literal text everywhere they're auto-populated to (Meta Title, Navigation Link Text, SEO preview heading, browser tab title) — no script execution, and pathPart slugification strips the markup into a safe URL segment (e.g. -script-alert1--script-negqa-xss-andamp-test).

github-actions · 2026-09-09T01:52:51Z

Code Review — ✅ No blockers · 🟡 2 warning(s) — see inline comments

geodem127 · 2026-09-09T02:07:09Z

Addressed both review items:

Negative-QA whitespace finding: fixed in 1aae4aa. Added an `isBlank()` helper (trims before checking) used everywhere Meta Title and the other SEO-required fields are validated, so `" "` is now treated the same as empty.
While fixing that, also restored #2984's stated intent that Meta Title should be optional for dataset items (like Meta Description already is), with the asterisk removed — an earlier commit on this branch had made it required-everywhere instead, which the whitespace bug only reproduces on models where Title is still actually required (single/multi-page items). Verified manually: dataset items save fine with no title, and a whitespace-only title on a page item is now blocked with a visible "Required Field" error.

github-actions · 2026-09-09T02:11:56Z

Code Review — ✅ No blockers · 🟡 1 warning(s) — see inline comments

geodem127 · 2026-09-09T02:20:30Z

Re: the negative-QA finding about manually-entered Meta Title being clobbered by a later edit to the first field — acknowledging this, leaving it out of scope for this PR.

The bot's own report labels it pre-existing on the touched surface: the root cause is `Editor.js`'s unconditional `SET_ITEM_WEB metaTitle` on every edit to the first field while `isNewItem`, which predates this PR. This PR's flush-on-save does make it reproduce deterministically instead of depending on winning/losing the old debounce race, but the underlying bug — auto-population overwriting a manual edit — is a separate fix (tracking whether the user has manually touched Meta Title so auto-population stops once they have) and out of scope for a PR about the debounce race and restoring #2984's dataset-optional intent.

github-actions · 2026-09-09T02:22:23Z

Code Review — ✅ No blockers · 🟡 1 warning(s) — see inline comments

agalin920 · 2026-09-17T22:31:27Z

  ContentModelField,
 } from "../../../../../../shell/services/types";
 import { SchedulePublish } from "../../../../../../shell/components/SchedulePublish";
+import { refRegistry } from "../../../../../../engine/refRegistry";


Does the bug reproductioned mentioned in the ticket even touch refRegistry? Why is it important to flush this? or what is being done with the registry here?

Good question — turned out we didn't need to. Reworked the fix to not touch refRegistry at all: Field.tsx now skips debouncing entirely for the one field that drives Meta Title auto-population (a new isAutoPopulateSource prop, set by Editor.js only on that field for new items), so the value commits to the store on that field's own onChange instead of being flushed from somewhere else at Save time. ItemCreate.tsx's save() no longer imports or references refRegistry. Details in the updated PR description.

github-actions · 2026-09-21T16:20:29Z

Code Review — 🔴 1 blocker(s) — see inline comments

agalin920 · 2026-09-21T20:59:34Z

@geodem127 this has a code blocker

Meta Title had no dataset exemption anywhere (hardcoded required in MetaTitle.tsx, unconditionally in Meta/index.tsx's REQUIRED_FIELDS, and unconditionally checked in createItem()'s hasMissingRequiredSEOFields), unlike Meta Description which was already made optional for datasets in intermittently force the URL path part to be required for datasets. Dataset items have no URL/page, so neither should ever be required. Refs #4276

The previous commit on this branch worked around #4276 by exempting dataset models from the Meta Title requirement. The actual bug is a race: Field.tsx debounces each field's onChange commit to the store by 500ms, so Editor.js's first-text-field auto-population of Meta Title (which applies to every non-block model, datasets included) doesn't reach the store until 500ms after the last keystroke. A save clicked before then validates against a value that hasn't committed yet. Revert the dataset-only exemption (content.js, Meta/index.tsx, MetaTitle.tsx are back to their pre-#4276-fix state) and fix the race instead: - useDebouncedInput now exposes flush(), wrapping lodash debounce's built-in flush. - Field.tsx adds that flush to the handle it already registers into the engine's refRegistry (the same registry the AI drawer uses), rather than introducing a new ref-forwarding path through Editor. - ItemCreate.tsx's save() flushes every registered field inside flushSync before validating. flushSync is required: flushing commits the value to the store, but React 18 batches that update, so Meta's validateMetaFields closure would otherwise stay stale until a re-render happens after this call already read it. - ItemCreate.tsx also now uses validateMetaFields()'s return value to gate the save (matching ItemEdit.js's existing pattern), instead of discarding it and checking possibly-stale SEOErrors state. - meta.spec.js's dataset test now asserts the created item's metaTitle actually matches what was auto-populated, instead of asserting Meta Title can be left blank. Refs #4276

No new e2e coverage needed for this fix; the existing meta.spec.js suite (including "Does not validate meta description for dataset items", which creates a dataset item the same way) already exercises the save path and passes with the debounce-flush fix in place. Refs #4276

…ly required fields Per #2984, Meta Title (like Meta Description already does) should be required only for single/multi-page items and optional for datasets, with the asterisk removed on the label. Restore that behavior: it was dropped when #4276 was originally "fixed" by requiring Meta Title but relying on auto-population, which took the wrong approach per that stated product intent. The debounce/flushSync fix from the previous commit is unaffected and still needed for pathPart/Meta Title auto-population on page and multi-page items. Also fixes a regression found by the PR's negative-QA review: whitespace-only text (e.g. " ") passed every required-field check here since `!value` treats a non-empty string of spaces as truthy. A dataset item's Meta Title bypassed this by not being required, but the same bug independently affects Meta Title on single/multi-page items, where it stays required. Added an isBlank() helper that trims before checking, used everywhere these fields are validated (content.js's createItem thunk, and Meta/index.tsx's live handleOnChange and validateMetaFields checks). Also fixed handleOnChange's useCallback missing REQUIRED_FIELDS and metaFields in its dependency array, which now varies by model type. Refs #4276, #2984

Addresses two review comments on 1aae4aa's flushSync block: - refRegistry is a single app-wide registry keyed by field name, not by item. "Create & Add New Related Item" (RelationalFieldBase -> CreateNewItemDialog) portals a full nested ContentEditor/ItemCreate on top of a still-mounted parent page, so saving the nested dialog was flushing every registered field app-wide -- including any debounced edit still in progress on the parent's fields, force- committing it early. - flushSync forcing a synchronous re-render of every mounted field on every Save click was flagged as a jank risk; scoping the flush to just this model's fields bounds that re-render to what Save was already about to touch, rather than the whole app. Filter to entries whose registered contentModelZUID matches this ItemCreate's own modelZUID before flushing. This resolves the reported cross-model case (the common one for relational fields); a nested dialog creating an item of the *same* model as the parent is a narrower remaining edge case that would need itemZUID-level scoping in refRegistry itself to fully close. Refs #4276

… on save Replaces the flushSync-over-refRegistry save-time flush with a targeted fix: Field.tsx now takes an isAutoPopulateSource prop, set by Editor.js only on the one field (first text/content field) that drives Meta Title auto-population on new items, and useDebouncedInput commits that field synchronously (delay=0) instead of debouncing it. The race is fixed at the point the value is typed rather than patched around at save time, so ItemCreate.tsx no longer needs refRegistry, flushSync, or a per-model filter at all — addressing review concerns about flushing unrelated in-flight fields, the refRegistry name-collision risk, and the unclear justification for touching refRegistry in the first place. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

github-actions · 2026-09-21T22:34:05Z

Localization Reviewer — ✅ No blockers

🔴 Blocking

None

🟡 Advisory

None

github-actions · 2026-09-21T22:35:47Z

Code Review — ✅ No blockers · 🟡 1 warning(s) — see inline comments

…taset-model-requires

github-actions · 2026-09-22T01:04:35Z

Localization Reviewer — ✅ No blockers

🔴 Blocking

None

🟡 Advisory

None

github-actions · 2026-09-22T01:06:46Z

Code Review — ✅ No blockers · 🟡 1 warning(s) — see inline comments

…taset-model-requires

github-actions · 2026-09-22T15:19:09Z

Localization Reviewer — ✅ No blockers

🔴 Blocking

None