Repository navigation
✅ test: Pin Add Entry Flow for Array Config Collections - #110
dustinhealy wants to merge 1 commit into
Conversation
Adding an entry to an array-object collection prepends an empty entry at index 0, while per-entry edits flow through indexed paths like modelSpecs.list.0. Before the applyConfigEdit indexed-merge branch landed, typing into the new entry deleted the pending whole-array edit and left only the bare indexed edit, which then resolved against the baseline array. The new entity overwrote the first existing entry in the UI and in the save payload, and for a previously-unset array the save shipped an indexed fieldPath the backend could not apply, so the toast reported success while nothing persisted. The unit tests for applyConfigEdit cover the merge in isolation but nothing exercised the component flow end to end. These tests drive the real SingleFieldRenderer, ArrayObjectField, and ObjectEntryCard through ConfigPage-equivalent state handling (applyConfigEdit, mergeIndexedArrayEdits, buildSavePayload) for both reported scenarios: typing into a newly added modelSpecs entry must keep the existing entries, and a new entry in a previously-unset endpoints.azureOpenAI.groups array must reach the save payload as the full array path. Both fail when the indexed-merge branch is removed and pass on main.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Thanks @dustinhealy for pinning this down and for checking both flows on v1.0.0 and on main. Since then the configuration editor was rewritten (#161, #162, #166, #180). The renderer and merge helpers these tests exercise no longer exist, and main has its own coverage for the same flows:
I also re-checked #105 and #44 on a main build. Existing specs are kept when one is added, and Azure groups either save correctly or are shown read-only with the reason. Closing this as obsolete. If you see a regression in either flow, please open an issue with the steps. |
Summary
Adding an entry to an array-object collection prepends an empty entry at index 0 while per-entry edits flow through indexed paths like
modelSpecs.list.0. Before theapplyConfigEditindexed-merge branch landed in #92, typing into the new entry deleted the pending whole-array edit and left only the bare indexed edit, which then resolved against the baseline array: the new entity overwrote the first existing entry in the UI and in the save payload, and for a previously-unset array the save shipped an indexed fieldPath the backend could not apply as an array, so the toast reported success while nothing persisted. Both reported repros run on v1.0.0, which predates #92; the flows pass on main (verified end to end in the running app for bothmodelSpecs.listandendpoints.azureOpenAI.groups).#92 covers the merge with unit tests on
applyConfigEdit, but nothing exercised the component flow end to end. This adds integration tests that drive the realSingleFieldRenderer,ArrayObjectField, andObjectEntryCardthrough ConfigPage-equivalent state handling (applyConfigEdit,mergeIndexedArrayEdits,buildSavePayload) for both scenarios: typing into a newly addedmodelSpecsentry must keep the existing entries, and a new entry in a previously-unsetendpoints.azureOpenAI.groupsarray must reach the save payload as the full array path.Note for deployments: v1.0.0 is the latest release and does not contain #92, so both issues remain reproducible there until a new release is cut.
Fixes #44
Fixes #105
Change Type
Testing
Both new tests fail with the
applyConfigEditindexed-merge branch removed and pass on main, pinning the #92 behavior against regression. Both issue flows were also verified live in the running app: on a v1.0.0 checkout the modelSpecs add-entry drops the first existing entry and the azure groups add reports success without persisting; on this branch's base both flows behave correctly. Local gates: fullvitestsuite passes (801 tests),eslintclean,tsc --noEmitclean.Checklist