PresetService.updatePreset silently drops the edit or deletes the wrong preset
- Dominant language
- Scala
- Stars
- 314
- Forks
- 187
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 214
Description
### Describe the bug
`PresetService.updatePreset` locates the preset to replace with lodash `indexOf`, which compares by reference. The presets it searches were just produced by `JSON.parse`, so `indexOf` never matches and always returns `-1`:
```ts
} else if (contains(presets, replacementPreset)) {
presets.splice(indexOf(presets, originalPreset), 1);
} else {
presets[indexOf(presets, originalPreset)] = replacementPreset;
}
```
`contains` compares with `isEqual`, so the guard above passes and execution reaches these two lines — then `-1` makes both of them do the wrong thing:
- `presets[-1] = replacementPreset` sets a non-index property on the array, so the edit is silently discarded and the stored list is written back unchanged.
- `presets.splice(-1, 1)` removes the **last** element, so the wrong preset is deleted.
The sibling method `updateOrCreatePreset` directly below already has the fix, with a comment naming this exact cause:
```ts
// deep-equality index: presets are freshly JSON-parsed, so reference-based indexOf would miss
presets.splice(presets.findIndex(preset => isEqual(preset, originalPreset)), 1);
```
so `updatePreset` appears to have been missed when that one was corrected.
### To Reproduce
With `["v1","v2","v3"]` stored for an operator type:
1. `updatePreset(type, target, v2, v2Edited)` → the saved list is still `["v1","v2","v3"]`; the edit is lost.
2. With `["v1","v2"]` stored, `updatePreset(type, target, v1, v2)` → the saved list is `["v1"]`; `v2` was deleted instead of `v1`.
### Expected behavior
1. `["v1","v2-edited","v3"]`.
2. `["v2"]` — replacing a preset with one that already exists merges the two by dropping the original.
### Additional context
Both lines are currently unhit, which is why the defect has gone unnoticed; they are among the gaps listed in #7777. Fix is to use the same deep-equality `findIndex` as `updateOrCreatePreset`.
Contributor guide
Assessment
This issue has not been assessed yet.