Skip to content

PROD-2542: validate content against the target model before sending it; harden batch result alignment - #234

Open
5PK wants to merge 3 commits into
mainfrom
feature/PROD-2542-required-field-validation
Open

5PK wants to merge 3 commits into
mainfrom
feature/PROD-2542-required-field-validation

Conversation

@5PK

@5PK 5PK commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Problem

Content items missing a value for a Required field are sent to the Management API anyway, and the database rejects them (Column 'ftdTitle' does not allow nulls. on the 2026-09-18 and 09-28 runs). Neither --preflight nor the run's own reporting flags these items ahead of time. Batch results are also matched to their source items purely by array position, with no length or ID check. logBatchErrors could already name the wrong item, because it took a position from the failed list instead of from the batch.

Change

Validation before the API call (always on, no new flags)

  • New validate-required-fields.ts checks each item against the target model's field definitions.
    • Fails when a Required field is missing or null. This applies to Text, LongText, HTML, Integer, Decimal, Date, DropdownList and Boolean.
    • Warns only on "", text longer than the field's Length, dropdown values not in Choices, and on Link, Image, File, Gallery, CustomField and linked-content fields. We haven't confirmed the API rejects nulls for those.
    • Field names and the Required setting are matched case-insensitively.
  • The target model comes from a new run-scoped effectiveTargetModels registry, which the model pusher fills after a save. The target cache isn't rewritten after a save in the same run, so the registry is checked first, then the target cache, then the source model.
  • Invalid items are removed in filterContentItemsForProcessing, before any batch is built.

Reporting

  • Real runs: a skipped item appears in the console log, the ERROR SUMMARY and --jsonSummary failures, with the model, fields, contentID and locale. This covers both the linked-content pass and the normal pass. The item is registered as failed straight away.
  • --preflight: a new invalid action, shown as a warning. It isn't counted as a create or update, and the exit code doesn't change.
  • Cascade: the "Unresolved content reference(s)" message now names the skipped child and why. The failed-content registry is keyed by locale + contentID. In preflight, pages note "N component(s) would be dropped".

Batch alignment hardening

  • prepareContentPayloads returns a single list of {source, payload} pairs, so payloads and source items can't drift apart.
  • No API call is made for an empty batch. When a batch throws, only the prepared items are marked failed, so items aren't counted twice.
  • extractBatchResultsImpl: if the number of results differs from the number sent, every item fails as "unverifiable batch result alignment". For updates, a returned itemID that differs from the contentID sent fails that item.
  • logBatchErrors finds the payload through batchItemId. If it can't, it prints "unknown" instead of guessing.

Testing

  • npx jest: 119 suites / 2,350 tests pass (main: 116 / 2,256). tsc --noEmit is clean.
  • New hermetic scenario 17-required-field-missing-in-batch: item 2 of 4 is invalid, and the other three map to the correct target IDs.
  • Unit tests cover:
    • the validator rules
    • filter routing
    • the model lookup order
    • an all-invalid batch, which makes no API call
    • the error path, with no double counting
    • a length mismatch and an ID mismatch
    • logBatchErrors naming the right item
    • the JSON summary
    • the cascade message
    • locale-scoped lookups
  • Live read-only preflight 067c2568-us2 → e711e981-us2: 3 create / 5,843 skip / 24 conflict / 8 invalid. The invalid items are 7 DepositBanner items, including 12942 (ftdTitle, lowBalanceTitle, …), plus einstants_einstantallgames (sectionTitle). The exit code was 1 because of the 24 conflicts already on that pair, not the invalid items.

Known limits

  • The ID cross-check is skipped for an --overwrite recreate of a deleted target, where the server can assign a new ID.
  • Page batches get the length check but not the ID cross-check.
  • PROD-2552 (the verify command) contains a copy of validate-required-fields.ts with the same interface. This branch's copy should win at merge.

Closes PROD-2542.

🤖 Generated with Claude Code

5PK and others added 3 commits October 7, 2026 15:45
…tive target models

- failedContentRegistry is keyed by locale + contentID, so an es-us failure no
  longer labels the en-us module that references the same source contentID.
  getFailedContent now takes the locale; process-page passes it.
- New effectiveTargetModels registry in state (cleared per push run and by
  resetState). model-pusher registers the saved model after a successful
  create/update (the payload when the response has no fields), and the source
  model for planned creates/updates under --preflight. The target
  models/<id>.json cache isn't rewritten after saveModel, so it is stale for
  the rest of the run.
- Preflight page entries note "N component(s) would be dropped" when modules
  point at content registered as failed in that locale.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
New pure validator (util/validate-required-fields.ts): a Required field whose
key is absent (matched case-insensitively) or null fails the item for scalar
types (Text, LongText, HTML, Integer, Decimal, Date, DropdownList, Boolean) -
the DB rejects these ("Column 'ftdTitle' does not allow nulls."). Attachments,
galleries, linked content, Link and custom fields only warn. Empty values,
Text over its Length and DropdownList values outside Choices ("Label|value"
lines) only warn.

filterContentItemsForProcessing validates every item it would create/update
against the effective target model (run registry -> target cache -> source
model -> none) and routes failures to a new itemsToFail list, so they never
reach a batch. Warnings are logged and the item proceeds.

- Real runs: content-pusher folds itemsToFail into totalFailed and the failure
  details (with model + missing fields), so they reach the console/log, the
  ERROR SUMMARY and --jsonSummary, for both the linked and normal passes; each
  is registered as failed content immediately.
- Preflight: recorded under a new "invalid" action (counted separately, shown
  with its reason, never a create/update, no exit-code change); JSON summary
  preflight totals/phases gain an additive "invalid" count.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…n cascades

- prepareContentPayloads returns one paired {source, payload} list; the
  payloads sent and the sources results are matched against both derive from
  it. An empty prepared list skips the API call. If the save/poll throws, only
  the prepared items are failed (prep failures were already counted, and
  results already recorded are not re-failed).
- extractBatchResults: batch.items must have one entry per item sent, or every
  item fails with "unverifiable batch result alignment" instead of guessing.
  For updates of a target item known to exist, a returned itemID that differs
  from the contentID sent fails that item.
- logBatchErrors resolves a failed item via batchItemId -> position in
  batch.items -> payload (the old fallback used the index within the failed
  list and could name the wrong item); unresolvable -> "unknown".
- "Unresolved content reference(s)" now appends "(child '<ref>' skipped:
  <reason>)" when the referenced child failed earlier in the same locale.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@5PK
5PK requested a review from DerekAgility October 13, 2026 15:01
@5PK
5PK marked this pull request as ready for review October 13, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants