apache / apache/texera

PresetService.updatePreset silently drops the edit or deletes the wrong preset

Open
#7,795 1 comment 0 reactions 1 assignee Claimed by @mengw15 View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.