supabase / supabase / 29855736671

74%

push

github

fix(sql-editor): guard removeFavorite against missing snippet like addFavorite (#48111)

## I have read the
[CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md)
file.

YES

## What kind of change does this PR introduce?

Bug fix

## What is the current behavior?

Fixes #48110

In the SQL editor Valtio store, `removeFavorite` guards against a
missing snippet with `if (storeSnippet.snippet)`, which reads `.snippet`
off `undefined` and throws `TypeError: Cannot read properties of
undefined (reading 'snippet')` whenever the id is not loaded in
`sqlEditorState.snippets`. Its counterpart `addFavorite` guards
correctly with `if (storeSnippet)` and no-ops on the same input.

## What is the new behavior?

`removeFavorite` now uses the same `if (storeSnippet)` guard as
`addFavorite`, so un-favoriting an id that is not in the store is a safe
no-op instead of a crash. Behavior for loaded snippets is unchanged.

Since `StateSnippet.snippet` is a required field, the old check was
always true whenever `storeSnippet` existed, so the only real world
difference between the two guards was the crash on the missing case.

I also added a small vitest file covering both methods (favorite set
plus needsSaving queued for loaded snippets, no-op for missing ids). The
missing-id test for `removeFavorite` fails with the exact TypeError
above when run against the old guard, and passes with this fix.

## Additional context

Root cause: `apps/studio/state/sql-editor/sql-editor-state.ts` line 260
(compare `removeFavorite` at lines 258 to 264 with `addFavorite` at
lines 250 to 256).

Gates run locally on top of current master (45ba40eff9): `pnpm
test:prettier`, `pnpm typecheck` (8/8 packages), `pnpm lint
--filter=studio` (0 errors), `pnpm test:studio` (only failure is
`lib/local-storage.test.ts`, which fails identically on clean master),
and `pnpm build --filter=studio`.

quick disclosure: I traced this one down and built the fix and test with
help from ... (continued)

1208 of 1666 branches covered (72.51%)

Branch coverage included in aggregate %.

1779 of 2367 relevant lines covered (75.16%)

241.36 hits per line