Skip to content

Content - Fix Meta Title validation racing the field-debounce commit - #4324

Open
geodem127 wants to merge 5 commits into
devfrom
fix/4276-content-creating-new-content-in-a-dataset-model-requires
Open

Content - Fix Meta Title validation racing the field-debounce commit#4324
geodem127 wants to merge 5 commits into
devfrom
fix/4276-content-creating-new-content-in-a-dataset-model-requires

Conversation

@geodem127

@geodem127 geodem127 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

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 (first commit):

  • useDebouncedInput now exposes flush() (wraps 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 already uses to drive fields imperatively) — no new ref-forwarding path needed through Editor.
  • ItemCreate.tsx's save() flushes every registered field, wrapped in flushSync, before validating. flushSync is required: flushing commits the value to the Redux store, but React 18 batches that update, so Meta's validateMetaFields closure would otherwise stay stale for the rest of the synchronous save() call.
  • ItemCreate.tsx now uses validateMetaFields()'s return value to gate the save, matching the pattern already used in ItemEdit.js.

Restoring #2984's intent (second commit): 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/flushSync fix is unaffected and still needed for page/multi-page item auto-population.

Whitespace-only regression (third commit, 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.
  • cypress/e2e/content/content.spec.js — full regression pass against the Field.tsx change (46/46 passing, 5 pending as expected).
  • Manually verified: a dataset item saves with no Meta Title and no asterisk on the label; an existing single-page item's Meta Title cleared to whitespace-only is rejected with a visible "Required Field" error.
  • npx tsc --noEmit — clean.

🤖 Generated with Claude Code

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
#2988. A stale-closure race in ItemCreate.tsx's save callback could also
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
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

QA Review — ✅ PASS

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

  1. ✅ Meta Title is no longer a required field when creating content on a dataset model (Meta/index.tsx drops metaTitle from REQUIRED_FIELDS when model?.type === "dataset", and the MetaTitle input's required prop now follows suit).
  2. createItem's server-side-style guard in content.js mirrors the UI change, skipping the missing-SEO check for dataset (previously only block was exempted).
  3. ✅ Path/parent requirements are also skipped for datasets on save (validateMetaFields deletes pathPart/parentZUID errors when model?.type === "dataset"), consistent with datasets not having a public path.
  4. ✅ Non-dataset models retain the existing required-SEO behavior (metaTitle stays in REQUIRED_FIELDS and required stays true by default) — no regression for standard content models.
  5. ⚠️ The debounce-flush fix (useDebouncedInput.flush, refRegistry flush-on-save in ItemCreate.tsx) addresses a race where a fast save right after typing could validate stale field state — plausible from the code but not observable without running the app.
Suggested Cypress coverage

cypress/e2e/content/meta.spec.js already has a "Does not validate meta description for dataset items" case (line 105) that this PR's scope naturally extends. Add: creating a new dataset item with Meta Title and SEO fields left blank should save successfully with no validation errors surfaced (covers the content.js and Meta/index.tsx changes together). Also add a case creating a new item on a non-dataset (page) model with Meta Title left blank to confirm the existing required-field error still blocks save, guarding against a regression. If feasible, add a case that types into a regular field and immediately clicks Save (no pause) on a dataset item whose Meta Title is auto-populated from that field, to exercise the debounce-flush path before the value would normally commit.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Code Review — ✅ No blockers

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Negative QA

🔵 Console warning: MetaTitle input switches from uncontrolled to controlled on Create save

  • Type: React console warning (no visible UI breakage observed)
  • Steps:
    1. Navigate to a new item form for a dataset-type model (e.g. .../content/6-bea8fcfcb7-qg9cq8/new, "QA Negative Race Dataset").
    2. Click "No, I will improve and edit it myself" (data-cy="ManualMetaFlow") to reveal the manual Meta Title field.
    3. Type a value into the Text field (data-cy="EditorField-text").
    4. Click Create/Save (data-cy="CreateItemSaveButton").
  • Expected: No React warnings about controlled/uncontrolled input transitions; MetaTitle should be controlled consistently across renders.
  • Actual: After save, the console logs: 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 component stack tracing through MetaTitleItemCreate. Reproduced identically on two separate attempts (same stack trace both times).
  • Console/network: Warning stack: at input ... at MetaTitle (main.js) ... at ItemCreate (main.js) .... No corresponding network failure; the item still saves successfully.
  • Screenshot: qa-artifacts/metatitle-uncontrolled-warning.png
Also checked and working correctly
  • Dataset-type models ("QA Negative Race Dataset") correctly allow a blank Meta Title on both Create and Edit flows, per the PR's intent to make Meta Title optional for datasets.
  • Whitespace-only Meta Title (e.g. three spaces) is correctly rejected as a "Required Field" error on a non-dataset model's Create flow, confirming the new isBlank() trim-aware validation works.
  • Double-clicking the Create/Save button does not create duplicate items.
  • Saving a nested "Create & Add New Related Item" dialog does not corrupt or lose the parent item's own unsaved field value, even though the parent stays mounted underneath.

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
Comment thread src/apps/content-editor/src/app/views/ItemCreate/ItemCreate.tsx
Comment thread src/apps/content-editor/src/app/views/ItemCreate/ItemCreate.tsx
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

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

@geodem127 geodem127 self-assigned this Sep 9, 2026
@geodem127 geodem127 added the bug Something isn't working label Sep 9, 2026
…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
@geodem127

Copy link
Copy Markdown
Contributor Author

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.

Comment thread src/apps/content-editor/src/app/views/ItemCreate/ItemCreate.tsx Outdated
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

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

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
@geodem127

Copy link
Copy Markdown
Contributor Author

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.

Comment thread src/apps/content-editor/src/app/views/ItemCreate/ItemCreate.tsx
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Manager UI - Creating New Content in a Dataset Model Requires SEO Information

1 participant