mirror of
https://github.com/supabase/supabase.git
synced 2026-10-09 03:15:06 +03:00
## What
PR 4 of a stacked refactor of the SQL editor snippet/folder state. It
pulls the persistence logic out of the store into an injectable
mechanism, and replaces the folder `'new-folder'` id sentinel with an
explicit lifecycle — plus a concurrency bug fix that surfaced along the
way.
### Save mechanism (`sql-editor-save.ts`)
`createSaveMechanism({ state, upsertContent, createSQLSnippetFolder,
updateSQLSnippetFolder, invalidate, notify, debounceMs })` → `{
saveSnippet, createFolder, updateFolder }`. The store's subscribe now
dispatches to it; *when* to save still lives in the subscribe (the
scheduler/provider move is PR 5). Per-id debounce cache lives in the
factory closure (no module-global leak).
- **`saveSnippet`** reads the live store snippet, guards
`isLoadedSnippet` so a content-less snippet can **never PUT an empty
body** (directly unit-tested), then builds the payload + drives status
transitions + gated invalidation.
- **`toast` is injected** as a `Notifier` (new generic DI contract in
`lib/notifier.ts`) — the mechanism no longer imports sonner.
- **create vs rename are two named-arg functions**, not an `isNew`
branch; rollback is deterministic per operation instead of matching on
`error.message` text.
- **caught errors are `unknown`**, narrowed via the existing
`getErrorMessage` util with a generic fallback — no `any`.
### Folder lifecycle (replaces the `NEW_FOLDER_ID` sentinel)
- **`FolderStatus`** enum (`new_editing | new_saving | editing | saving
| idle`) collapses the persistence and progress axes into one enum —
same pattern as `SnippetStatus` — with `isNewFolder` / `isFolderEditing`
/ `isFolderSaving` predicates. Tagging a folder as new/persisted is now
an explicit field, not an id convention.
- New placeholders get a **unique local id** (`crypto.randomUUID`);
`NEW_FOLDER_ID` is deleted, which also lifts the accidental
one-unsaved-folder-at-a-time limit.
### Bug fix: folder-rename rollback race
The shared `lastUpdatedFolderName` field let two in-flight renames
clobber each other's rollback target (and a shared `finally` could wipe
it). Replaced by a **per-folder `previousName`** on
`StateSnippetFolder`, so concurrent renames of different folders are
isolated. A new test runs two failing renames concurrently and asserts
each restores its own previous name.
## Tests
`sql-editor-save.test.ts` (mechanism — fakes + fake timers, incl.
content-less no-PUT and concurrent-rename isolation) and
folder-lifecycle predicate tests. `pnpm --filter studio typecheck`
clean; 82 state/sql-editor unit tests pass.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Improved SQL editor folder handling with clearer create, rename, and
save states.
* Added a more consistent notification flow for successful and failed
save actions.
* **Bug Fixes**
* Improved rollback handling when folder renames fail, helping restore
the previous name reliably.
* Updated save behavior to better protect against duplicate or
out-of-order updates.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
174 lines
5.4 KiB
TypeScript
174 lines
5.4 KiB
TypeScript
import { describe, expect, it } from 'vitest'
|
|
|
|
import {
|
|
folderStatusOnSaveStart,
|
|
isFolderEditing,
|
|
isFolderSaving,
|
|
isNewFolder,
|
|
isSaveFailed,
|
|
isSaving,
|
|
statusOnSaveError,
|
|
statusOnSaveStart,
|
|
statusOnSaveSuccess,
|
|
wasNeverPersisted,
|
|
type FolderStatus,
|
|
} from './sql-editor-lifecycle'
|
|
import type { SnippetStatus } from '@/data/content/snippet-status'
|
|
|
|
const NEVER_PERSISTED: Array<SnippetStatus> = ['new', 'new_saving', 'new_save_failed']
|
|
const PERSISTED: Array<SnippetStatus> = ['saved', 'unsaved', 'saving', 'save_failed']
|
|
|
|
describe('wasNeverPersisted', () => {
|
|
it.each(NEVER_PERSISTED)('is true for never-persisted status %s', (status) => {
|
|
expect(wasNeverPersisted(status)).toBe(true)
|
|
})
|
|
|
|
it.each(PERSISTED)('is false for persisted status %s', (status) => {
|
|
expect(wasNeverPersisted(status)).toBe(false)
|
|
})
|
|
|
|
it('treats an absent status as persisted', () => {
|
|
expect(wasNeverPersisted(undefined)).toBe(false)
|
|
})
|
|
})
|
|
|
|
describe('isSaving', () => {
|
|
it('is true only while a save is in flight (new or re-save)', () => {
|
|
expect(isSaving('new_saving')).toBe(true)
|
|
expect(isSaving('saving')).toBe(true)
|
|
})
|
|
|
|
it.each(['new', 'new_save_failed', 'saved', 'unsaved', 'save_failed', undefined] as const)(
|
|
'is false for %s',
|
|
(status) => {
|
|
expect(isSaving(status)).toBe(false)
|
|
}
|
|
)
|
|
})
|
|
|
|
describe('isSaveFailed', () => {
|
|
it('is true only after a failed save (new or re-save)', () => {
|
|
expect(isSaveFailed('new_save_failed')).toBe(true)
|
|
expect(isSaveFailed('save_failed')).toBe(true)
|
|
})
|
|
|
|
it.each(['new', 'new_saving', 'saved', 'unsaved', 'saving', undefined] as const)(
|
|
'is false for %s',
|
|
(status) => {
|
|
expect(isSaveFailed(status)).toBe(false)
|
|
}
|
|
)
|
|
})
|
|
|
|
describe('statusOnSaveStart', () => {
|
|
it('keeps never-persisted snippets in the new family', () => {
|
|
expect(statusOnSaveStart('new')).toBe('new_saving')
|
|
expect(statusOnSaveStart('new_save_failed')).toBe('new_saving')
|
|
})
|
|
|
|
it('moves persisted snippets to saving', () => {
|
|
expect(statusOnSaveStart('saved')).toBe('saving')
|
|
expect(statusOnSaveStart('unsaved')).toBe('saving')
|
|
expect(statusOnSaveStart('save_failed')).toBe('saving')
|
|
expect(statusOnSaveStart(undefined)).toBe('saving')
|
|
})
|
|
})
|
|
|
|
describe('statusOnSaveSuccess', () => {
|
|
it('always resolves to saved', () => {
|
|
expect(statusOnSaveSuccess()).toBe('saved')
|
|
})
|
|
})
|
|
|
|
describe('statusOnSaveError', () => {
|
|
it('keeps never-persisted snippets in the new family', () => {
|
|
expect(statusOnSaveError('new_saving')).toBe('new_save_failed')
|
|
expect(statusOnSaveError('new')).toBe('new_save_failed')
|
|
})
|
|
|
|
it('moves persisted snippets to save_failed', () => {
|
|
expect(statusOnSaveError('saving')).toBe('save_failed')
|
|
expect(statusOnSaveError('saved')).toBe('save_failed')
|
|
expect(statusOnSaveError(undefined)).toBe('save_failed')
|
|
})
|
|
})
|
|
|
|
describe('lifecycle round trips', () => {
|
|
it('new snippet: first save succeeds then a re-save succeeds', () => {
|
|
let status: SnippetStatus = 'new'
|
|
status = statusOnSaveStart(status)
|
|
expect(status).toBe('new_saving')
|
|
status = statusOnSaveSuccess()
|
|
expect(status).toBe('saved')
|
|
// re-save
|
|
status = statusOnSaveStart(status)
|
|
expect(status).toBe('saving')
|
|
status = statusOnSaveSuccess()
|
|
expect(status).toBe('saved')
|
|
})
|
|
|
|
it('new snippet: first save fails, retry succeeds (stays never-persisted until success)', () => {
|
|
let status: SnippetStatus = 'new'
|
|
status = statusOnSaveStart(status)
|
|
status = statusOnSaveError(status)
|
|
expect(status).toBe('new_save_failed')
|
|
expect(wasNeverPersisted(status)).toBe(true)
|
|
// retry
|
|
status = statusOnSaveStart(status)
|
|
expect(status).toBe('new_saving')
|
|
status = statusOnSaveSuccess()
|
|
expect(status).toBe('saved')
|
|
expect(wasNeverPersisted(status)).toBe(false)
|
|
})
|
|
})
|
|
|
|
const NEW_FOLDER: Array<FolderStatus> = ['new_editing', 'new_saving']
|
|
const PERSISTED_FOLDER: Array<FolderStatus> = ['editing', 'saving', 'idle']
|
|
|
|
describe('isNewFolder', () => {
|
|
it.each(NEW_FOLDER)('is true for not-yet-persisted folder status %s', (status) => {
|
|
expect(isNewFolder(status)).toBe(true)
|
|
})
|
|
|
|
it.each(PERSISTED_FOLDER)('is false for persisted folder status %s', (status) => {
|
|
expect(isNewFolder(status)).toBe(false)
|
|
})
|
|
|
|
it('treats an absent status as not-new', () => {
|
|
expect(isNewFolder(undefined)).toBe(false)
|
|
})
|
|
})
|
|
|
|
describe('isFolderEditing', () => {
|
|
it('is true only while the name is being edited inline (new or persisted)', () => {
|
|
expect(isFolderEditing('new_editing')).toBe(true)
|
|
expect(isFolderEditing('editing')).toBe(true)
|
|
})
|
|
|
|
it.each(['new_saving', 'saving', 'idle', undefined] as const)('is false for %s', (status) => {
|
|
expect(isFolderEditing(status)).toBe(false)
|
|
})
|
|
})
|
|
|
|
describe('isFolderSaving', () => {
|
|
it('is true only while a create/rename is in flight (new or persisted)', () => {
|
|
expect(isFolderSaving('new_saving')).toBe(true)
|
|
expect(isFolderSaving('saving')).toBe(true)
|
|
})
|
|
|
|
it.each(['new_editing', 'editing', 'idle', undefined] as const)('is false for %s', (status) => {
|
|
expect(isFolderSaving(status)).toBe(false)
|
|
})
|
|
})
|
|
|
|
describe('folderStatusOnSaveStart', () => {
|
|
it('keeps a new folder in the new family', () => {
|
|
expect(folderStatusOnSaveStart('new_editing')).toBe('new_saving')
|
|
})
|
|
|
|
it('moves a persisted folder to saving', () => {
|
|
expect(folderStatusOnSaveStart('editing')).toBe('saving')
|
|
expect(folderStatusOnSaveStart('idle')).toBe('saving')
|
|
})
|
|
})
|