Commit Graph

3 Commits

Author SHA1 Message Date
Charis
fa5eb17277 feat(studio): discriminated snippet union + source-aware writes (#48313)
Stacked on #48305.

## What

PR 3 of the stacked SQL-editor query-source series (Database vs Logs).
Stacked on the PR 2 branch `charislam/log-sql-content-shape`.

Turns `SnippetWithContent` into a discriminated union on `type` and
makes all snippet writes source-aware:

- `data/content/sql-folders-query.ts`: `SnippetWithContent` is now `{
type: 'sql'; content?: SqlSnippets.Content } | { type: 'log_sql';
content?: LogSqlSnippets.Content } | { type: 'report'; content?: never
}`. `report` is kept (the content endpoints' wire type carries it) but
has no SQL content — its body is `Dashboards.Content`, loaded through
the separate `Content` union.
- `setSql` brands per type (`untrustedLogSql` vs `untrustedSql`).
- `buildUpsertPayload` persists `snippet.type` (no longer hardcoded
`'sql'`).
- `createSqlSnippetSkeletonV2({ source })` emits the matching type +
content shape with the `as any` cast removed.
- New `components/interfaces/SQLEditor/querySource.ts`:
`SqlSnippetSource` + `getSnippetSource`.
- `seedSnippet` test helper gains a `source` arg.
- New `remapWireSnippet` boundary helper in `content-remap.ts`
concentrates the single wire->domain assertion, so `content-id-query` /
`content-upsert-mutation` call sites are cast-free (no `as unknown as`).
- Collateral: query result types aligned to the union; `updateSnippet`
no longer accepts `type` (source is immutable); db-only editor read
paths narrow away `log_sql`.

## Why

Impossible-states-impossible typing: a snippet's brand follows its
content type, so logs SQL and database SQL can never cross execution
paths. No behavior change for existing database snippets.

## Testing

- \`pnpm typecheck\` — clean
- \`pnpm --filter studio run lint:ratchet\` — no new warnings
- \`pnpm test:studio\` (data/content, SQLEditor, state/sql-editor) —
passing, including new tests for \`getSnippetSource\`, source-aware
\`setSql\`, type-aware \`buildUpsertPayload\`, and both skeleton shapes.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added source-aware creation for SQL editor snippets, including
log-based SQL snippets.
* Introduced backend source mapping so log snippets are treated as
log_sql.
* **Bug Fixes**
* Improved SQL retrieval/prettification so log snippets no longer use
the wrong fallback content.
* Ensured log snippets are sanitized and preserve correct type, content,
identifiers, and statuses during save/upsert flows.
* **Tests**
* Expanded unit and integration coverage for log snippet creation,
source mapping, editing, prettification, and upsert payloads.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-07-28 12:28:36 -04:00
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 -->
2026-06-24 08:56:39 -04:00
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 -->
2026-06-23 09:42:49 -04:00