Repository navigation
FIX: Keep stored scenario preset list params intact on unrelated edits - #3781
Draft
Mr.X (MRX-72) wants to merge 1 commit into
Draft
Mr.X (MRX-72) wants to merge 1 commit into
Mr.X (MRX-72) wants to merge 1 commit into
Conversation
The preset editor renders a stored list as comma-joined text and splits it on save, so an element containing a comma became several. Keep the stored value for params the user didn't edit.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Editing only the description of a scenario preset could change its list parameters. The editor shows a stored list as comma-joined text and splits it on commas when it saves. So goals
['Hello, world!', 'Second goal']were saved as['Hello', 'world!', 'Second goal']. That changes the preset's behavior, and the backend validators accept both versions, so nothing catches it.In
presetScenarioParams, for a param the preset already stores, I now compare the built value with what the form produces from the stored value with no edits. If they match, the user didn't touch the field and the stored value is kept as-is. If the user edits the field, it is parsed as before.I did not change the shared comma parsing in
parameterForm.ts. The text input is comma-separated, so a comma inside an element is ambiguous once someone edits that field. Handling it there would need a different input, which seemed out of scope here.Fixes #3754
Tests and Documentation
Added a case to
scenarioPresetForm.test.ts. A description-only edit keeps['Hello, world!', 'Second goal'], and typinga, binto the field still gives['a', 'b']. The test fails without the change.Ran
jestonScenarioPresetsandParameters(184 passing), plustsc --noEmitand eslint onScenarioPresets. I haven't run the full jest suite.No docs changed. No notebook or JupyText changes.