mirror of
https://github.com/supabase/supabase.git
synced 2026-06-29 03:50:30 -04:00
create-pull-request/patch
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Charis
|
5cb81123ae |
refactor(studio): move SQL editor save trigger into a scheduler + provider (5/9) (#47316)
## What
PR 5 of a stacked refactor. Moves *when to save* out of a module-load
`subscribe` and into an injectable **scheduler** armed by a headless
**provider**, splits the save queue, and adds an unsaved-close warning.
### Scheduler (`sql-editor-save-scheduler.ts`)
`createSaveScheduler({ state, saveMechanism, notify, getSaveMode })`
owns the save *policy*:
- **auto** mode drains the dirty snippet queue as edits land; **manual**
mode (the seam for a future opt-in; defaults to `auto`) leaves snippets
queued until `requestSave`. Folder saves always drain.
- `start()` returns an unsubscribe; `requestSave(id)` is the
explicit-save entry.
### Provider (`sql-editor-save-coordinator.tsx`)
Headless `SqlEditorSaveCoordinatorProvider` instantiates the mechanism
(invalidation via the **React Query client from context**, not the
global `getQueryClient`) + scheduler, `start()`s it in an effect
(start/stop with the provider), and exposes `requestSave` via
`useSqlEditorSaveCoordinator()`. Mounted in `ProjectContext` (under the
app's QueryClientProvider). Cmd+S and the SavingIndicator Retry now go
through `requestSave`.
### Queue split
`needsSaving` (snippets) and `pendingFolderSaves` (folders) are separate
queues, drained independently — the old snippet-vs-folder `if/else` is
gone.
### Unsaved-close warning
A `beforeunload` guard triggers the browser's native "Leave site?"
prompt while any snippet's `status !== 'saved'` (failed / in-flight /
never-saved).
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Improved SQL editor saving with a centralized save flow, including
automatic/manual save handling and immediate “Save Query” requests.
* Added unsaved-change detection so the app can warn before closing or
reloading when edits are still pending.
* **Bug Fixes**
* Retry actions now use the updated save flow for more reliable
re-saving.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Charis
|
16526bd6bf |
refactor(studio): extract SQL editor save mechanism + model folder lifecycle (4/9) (#47276)
## 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 -->
|
||
|
Charis
|
d5653f1f92 |
refactor(studio): unify snippet save + persistence into SnippetStatus (3/9) (#47251)
## What PR 3 of a stacked refactor of the SQL editor snippet state. Replaces the two overlapping pieces of snippet lifecycle state — the `savingStates` map (`IDLE|UPDATING|UPDATING_FAILED`) and the `isNotSavedInDatabaseYet` boolean — with a single `SnippetStatus` enum. ## Status is attached at the data layer (never absent) - `SnippetStatus` + `SnippetWithContent` now live in `data/content`. The snippet queries attach `status: 'saved'` via a typed `withSavedStatus()` helper, and `upsertContent` returns `SnippetWithContent` so move/rename responses carry status too. - A SQL-typed `getSqlSnippetById`/`useSqlSnippetByIdQuery` returns `SnippetWithContent` (the generic `useContentIdQuery` stays for Reports, which use it). `[id].tsx` loads content with **no casting**. - `'new'` is attached on local creation (`createSqlSnippetSkeletonV2`). ## Behavior Behavior-preserving for the existing auto-save flow (faithful mapping of both old fields, including the replication-lag swallow). One incidental fix: the read-only/saving indicator now also covers a brand-new snippet's first save (previously only re-saves of persisted snippets had distinct saving/failed states in some paths). ## Tests New `sql-editor-lifecycle.test.ts` (29 tests) covering every predicate and transition; existing rules tests updated. `pnpm --filter studio typecheck` clean; 52 state/sql-editor unit tests pass. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Refactor** * Restructured SQL snippet persistence tracking, replacing boolean flags with a comprehensive status system for clearer visibility into save progress. * Enhanced saving indicator UI to reflect accurate snippet save states. * **Tests** * Added test coverage for snippet persistence state transitions and lifecycle scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Charis
|
e1e2498db0 |
refactor(studio): extract SQL editor domain rules into pure module (2/9) (#47204)
## What PR 2 of a stacked refactor of the SQL editor snippet state. **Stacked on #47203 (PR 1)** — review/merge that first. Extracts scattered business rules + the upsert-payload builder into a new **pure** module `apps/studio/state/sql-editor/sql-editor-rules.ts` (no Valtio, React, toast, or runtime data-layer imports): - `canEditSnippet` — read-only rule (shared snippet you don't own), was inline in `MonacoEditor` `disableEdit` - `isSnippetOwner` — owner check, was inline in `ReadOnlyBadge` / `SavingIndicator` - `validateMoveToFolder` — 'shared snippet cannot be within a folder', was a buried `toast.error` - `buildUpsertPayload` — the PUT /content payload, was an inline object literal (all `??` defaults preserved) - `isLoadedSnippet` — type guard (see below) ## Bug fix: no more empty-content saves (and no non-null assertion) The old payload builder used `{ ...content!, content_id: id }`. Tracing that `!` upstream surfaced a real bug: **favoriting a snippet from the sidebar that had never been opened** enqueued a save with no loaded content, producing a PUT with an empty content body (rejected by API). The requirement that a persisted snippet has loaded content is now enforced **at the type level** rather than by a runtime assertion or comment: - `buildUpsertPayload` accepts only a `LoadedSnippet` (content non-nullable) — the `!` is gone. - the save subscriber crosses that boundary via the `isLoadedSnippet` type guard. - the sidebar favorite toggle loads content first (mirroring `onSelectDuplicate` / the share modals), narrowing the fetched union content to the SQL variant via its discriminant — **no type cast**. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved consistency in read-only behavior and ownership checks across the SQL editor by centralizing permission logic. * Fixed favorite toggle to ensure snippet content is fully loaded before persisting changes. * **Refactor** * Centralized SQL snippet permission rules and validation logic into a dedicated helper module. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Charis
|
2b24065c7b |
refactor(studio): relocate SQL editor store into state/sql-editor/ with facade (1/9) (#47203)
## What PR 1 of a stacked refactor that re-layers the SQL editor snippet state (`apps/studio/state/sql-editor-v2.ts`). This first PR is a **pure structural move with zero behavior change** — no consumer files are touched. - Relocates the Valtio store body into `apps/studio/state/sql-editor/sql-editor-state.ts` - Extracts the type declarations into `apps/studio/state/sql-editor/types.ts` - Adds `apps/studio/state/sql-editor/index.ts` as the public surface - Keeps the old `apps/studio/state/sql-editor-v2.ts` path as a thin re-export **facade**, so all existing importers keep working unchanged ## How to read the diff `sql-editor/sql-editor-state.ts` (~507 lines) is the **verbatim relocation** of the former `sql-editor-v2.ts` body — not new code. Git does not show it as a rename because the old path is intentionally retained as the facade. The only genuinely new lines are `types.ts` (20), `index.ts` (8), and the facade itself (8). ## Why The store has accreted four tangled responsibilities (snippet/folder CRUD, query results, persistence, Assistant diff). The stack incrementally splits these into pure rules, a persistent store, a session store, and an injectable save mechanism whose trigger is a swappable policy (setting up a future auto→manual save migration). Each PR stays ≤300–400 non-test lines and behavior-preserving. ## Verification - `pnpm --filter studio typecheck` passes (only pre-existing unrelated module-resolution errors remain). - Lint passes (no new errors). - No consumer imports changed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Restructured SQL editor state management into a modular architecture with improved separation of concerns and enhanced code organization. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |