fix(studio): DOM-nesting hydration errors, ghost deleted-snippet nav, and migrations query 400s (#47667)

App-level fixes that reproduce on BOTH the Next and TanStack builds —
split out of #47657 (which stays TanStack-only) for reviewability. All
were found by a full-site click-through of the dashboard.

## Invalid HTML nesting (React 19 "will cause a hydration error" console
errors)

- **FormLayout description rendered in a `<p>`**
(`packages/ui-patterns`): consumers pass arbitrary JSX (the RowEditor's
`created_at` timezone note passes a `<div>` with `<p>`s) →
`<p>`-in-`<p>` / `<div>`-in-`<p>`. Container is now a `<div>` with
identical classes (Tailwind preflight makes them render the same).
- **Switch toggles nested inside Tooltip trigger buttons**
(button-in-button) in ColumnEditor ("Allow Nullable" + "Is Unique"),
ExtensionRow, and PublicationsTableItem → repo-standard `TooltipTrigger
asChild` + `<div>` wrapper.
- **Saved log queries rendered a `<div>` directly inside `<tbody>`**
(`/logs/explorer/saved`) → rows are now proper `<tr><td colSpan>`
wrappers; the component itself is untouched (it's valid in its sidebar
usage).
- **Nested anchors in observability metric cards**: a card-level
`<Link>` wrapped MetricCard's "More information" `<Link>` (identical
URLs) → the chevron affordance renders as a `<span>` when no `href` is
passed; clicks bubble to the card link, tooltips preserved.
Design-system standalone usage unaffected.
- **`objectFit="cover"` passed to modern `next/image`** on the featured
integration card (unknown-prop warning) — the className already had
`object-cover`; prop dropped.

## Ghost dead-snippet after deletion

Deleting the active SQL snippet left its id in `useDashboardHistory`
(`history.sql`), so the "SQL Editor" nav item navigated to
`/sql/<deleted-id>` — content fetch 404s, no editor pane renders, and a
phantom tab reappears. Fixed both ends: delete flows now purge dashboard
history (and the tabs store clears a stale `previewTabId`), and
`/sql/[id]` treats a snippet 404 as "clean up + `router.replace` to
`/sql/new` + toast" instead of rendering the dead state. Unit tests for
the store/history cleanup.

## `pg-meta` migrations query 400s on every project load

`ActivityStats` on project home runs the migrations list query, whose
SQL was a bare `select * from supabase_migrations.schema_migrations` —
that table only exists once a migration has run, so every other project
logged a failed `?key=migrations` request on every load (visible in
production consoles too). The SQL is now guarded with `to_regclass` +
`query_to_xml` (same pattern as the advisor lints' `storage.buckets`
guard), returning zero rows instead of erroring; legacy version-only
tables still work. Tested against real dockerized Postgres (absent
table, populated ordering, special chars, legacy schema) + MSW hook
tests.

Found and verified via /test-supabase-local (browser click-through +
console audit on both builds).

## To test

Console must stay free of React DOM-nesting errors ("cannot be a
descendant of" / "cannot contain a nested") on each surface:

1. Table editor → Insert row panel (`created_at` field renders its
timezone note) and Edit column panel ("Allow Nullable"/"Is Unique"
tooltips still hover).
2. `/database/extensions` and `/database/publications` → toggle switches
render, tooltips hover.
3. `/logs/explorer/saved` (with ≥1 saved query) → rows render full-width
inside the table, hover shows Actions.
4. `/observability` → no nested-anchor error on load; card body click
and the chevron both navigate; label help-icons still show tooltips.
5. `/integrations` → no `objectFit` unknown-prop warning; featured card
images still cover.
6. **Ghost snippet**: open a SQL snippet → delete it via the sidebar →
click the "SQL Editor" nav item → lands on `/sql/new` (no phantom tab,
no 404 content fetch). Direct-load `/sql/<random-uuid>` → toast +
redirect to `/sql/new`.
7. **Migrations 400**: load project home with a project that has never
run a migration → the `pg-meta/<ref>/query?key=migrations` request
returns **200** with `[]` (previously a 400 on every load). Database →
Migrations still lists real migrations when they exist.


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

## Summary by CodeRabbit

* **Bug Fixes**
* Deleted SQL snippets are fully removed from dashboard history and
stale editor/tab state; users are redirected with a toast.
  * Closing preview tabs no longer leaves stale references.
* Improved toggle/tooltip/dialog interactions to avoid broken UI,
including metric headers showing tooltips even without direct links.
* Migrations display safely when migration tables/relations are missing.

* **UI Improvements**
* Refreshed layout for saved queries, form descriptions, and integration
imagery.

* **Tests**
* Added coverage for snippet history cleanup, tab removal, migrations
SQL behavior, and query edge cases.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->


---

### Review feedback: `query_to_xml` breaks on Multigres (Ivan)

The defensive migrations query (added here to stop the `?key=migrations`
400 when the table doesn't exist yet) originally guarded with
`query_to_xml`, which is forbidden through Multigres's pooler (MUL-736 /
PSQL-1318). Rewritten without `query_to_xml`/`xmltable` using the
splinter#170 pattern: a PL/pgSQL `do` block guarded by `to_regclass`
(PL/pgSQL defers planning, so a missing table never errors) stashes the
rows into a transaction-local GUC via `set_config`, and a trailing
`select` reads them back with `jsonb_array_elements`. Verified that
postgres-meta sends the whole SQL as one simple-query string → single
implicit transaction → the local GUC survives to the `select` and
doesn't leak into the pooled connection. 6/6 dockerized-Postgres tests
(absent table → `[]`, populated/ordered/special-chars, legacy
version-only table, full pg-meta-shaped multi-statement string, GUC
non-leakage).

Note (out of scope, pre-existing):
`packages/pg-meta/src/sql/studio/advisor/lints.ts` still uses
`query_to_xml` — a separate pre-existing Multigres risk that should get
its own splinter-pattern sync.

---------

Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com>
Co-authored-by: Joshen Lim <joshenlimek@gmail.com>
Co-authored-by: Saxon Fletcher <saxonafletcher@gmail.com>
This commit is contained in:
authored and GitHub committed 2026-07-08 12:32:11 +08:00
1 parent b3c98c11f8
commit 9af6e65df4
24 files changed
+592 -93

No files matched your search

@@ -807,6 +807,7 @@ export const UsersV2 = () => {
renderRow(id, props) {
return (
<Row
key={id}
{...props}
onClick={() => {
const user = users.find((u) => u.id === id)
@@ -158,14 +158,17 @@ export const ExtensionRow = ({ extension }: ExtensionRowProps) => {
<Loader2 className="animate-spin" size={16} />
) : (
<Tooltip>
<TooltipTrigger>
<Switch
disabled={disabled}
checked={isOn}
onCheckedChange={() =>
isOn ? setIsDisableModalOpen(true) : setShowConfirmEnableModal(true)
}
/>
<TooltipTrigger asChild>
<div>
<Switch
aria-label="Toggle extension"
disabled={disabled}
checked={isOn}
onCheckedChange={() =>
isOn ? setIsDisableModalOpen(true) : setShowConfirmEnableModal(true)
}
/>
</div>
</TooltipTrigger>
{disabled && (
<TooltipContent side="bottom">
@@ -25,7 +25,7 @@ export const PublicationsTableItem = ({
const isProtected = protectedSchemas.map((x) => x.name).includes(table.schema)
const [checked, setChecked] = useState(
selectedPublication.tables?.find((x: any) => x.id == table.id) != undefined
selectedPublication.tables?.find((x) => x.id == table.id) != undefined
)
const { can: canUpdatePublications } = useAsyncCheckPermissions(
@@ -42,14 +42,12 @@ export const PublicationsTableItem = ({
setChecked(!checked)
const publicationTables = publication?.tables ?? []
const exists = publicationTables.some((x: any) => x.id == table.id)
const exists = publicationTables.some((x) => x.id == table.id)
const tables = !exists
? [`${table.schema}.${table.name}`].concat(
publicationTables.map((t: any) => `${t.schema}.${t.name}`)
publicationTables.map((t) => `${t.schema}.${t.name}`)
)
: publicationTables
.filter((x: any) => x.id != table.id)
.map((x: any) => `${x.schema}.${x.name}`)
: publicationTables.filter((x) => x.id != table.id).map((x) => `${x.schema}.${x.name}`)
updatePublications(
{
@@ -87,13 +85,16 @@ export const PublicationsTableItem = ({
</Badge>
) : (
<Tooltip>
<TooltipTrigger>
<Switch
size="small"
disabled={!canUpdatePublications || isPending || isProtected}
checked={checked}
onClick={() => toggleReplicationForTable(table, selectedPublication)}
/>
<TooltipTrigger asChild>
<div>
<Switch
size="small"
aria-label={`Toggle replication for ${table.name}`}
disabled={!canUpdatePublications || isPending || isProtected}
checked={checked}
onClick={() => toggleReplicationForTable(table, selectedPublication)}
/>
</div>
</TooltipTrigger>
{isProtected && (
<TooltipContent side="bottom" className="w-64 text-center">
@@ -51,6 +51,7 @@ export const CreateRolePanel = ({ visible, onClose }: CreateRolePanelProps) => {
const form = useForm<z.infer<typeof FormSchema>>({
resolver: zodResolver(FormSchema),
defaultValues: initialValues,
})
const { mutate: createDatabaseRole, isPending: isCreating } = useDatabaseRoleCreateMutation({
@@ -38,8 +38,9 @@ import {
import { CodeBlock } from 'ui-patterns/CodeBlock'
import { TimestampInfo } from 'ui-patterns/TimestampInfo'
import { type CronTableColumn } from './CronJobs.constants'
import { useDatabaseCronJobRunCommandMutation } from '@/data/database-cron-jobs/database-cron-job-run-mutation'
import { CronJob } from '@/data/database-cron-jobs/database-cron-jobs-infinite-query'
import { type CronJob } from '@/data/database-cron-jobs/database-cron-jobs-infinite-query'
import { useDatabaseCronJobToggleMutation } from '@/data/database-cron-jobs/database-cron-jobs-toggle-mutation'
import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject'
@@ -75,8 +76,8 @@ const getNextRun = (schedule: string, lastRun?: string) => {
}
interface CronJobTableCellProps {
col: any
row: any
col: CronTableColumn
row: CronJob
onSelectEdit: (job: CronJob) => void
onSelectDelete: (job: CronJob) => void
}
@@ -92,19 +93,20 @@ export const CronJobTableCell = ({
const [showToggleModal, setShowToggleModal] = useState(false)
const value = row?.[col.id]
const value = row?.[col.id as keyof typeof row]
const { jobid, schedule, latest_run, status, active, jobname } = row
const formattedValue =
const formattedValue = (
col.id === 'jobname' && !jobname
? 'No name provided'
: col.id === 'lastest_run'
? !!value
? dayjs(value).valueOf()
? dayjs(value as string).valueOf()
: undefined
: col.id === 'next_run'
? getNextRun(schedule, latest_run)
: value
) as string
const hasValue = col.id === 'next_run' ? !!formattedValue : col.id in row
@@ -148,6 +150,7 @@ export const CronJobTableCell = ({
<DropdownMenuTrigger asChild>
<Button
variant="text"
aria-label="More actions"
loading={isRunning}
className="h-6 w-6"
icon={<MoreVertical />}
@@ -203,13 +206,16 @@ export const CronJobTableCell = ({
if (col.id === 'active') {
return (
<Dialog open={showToggleModal} onOpenChange={setShowToggleModal}>
<DialogTrigger className="flex items-center" onClick={(e) => e.stopPropagation()}>
<Switch
id={`cron-job-active-${jobid}`}
size="medium"
disabled={isToggling}
checked={active}
/>
<DialogTrigger asChild onClick={(e) => e.stopPropagation()}>
<div className="flex items-center">
<Switch
id={`cron-job-active-${jobid}`}
aria-label={`${active ? 'Disable' : 'Enable'} cron job`}
size="medium"
disabled={isToggling}
checked={active}
/>
</div>
</DialogTrigger>
<DialogContent
onClick={(e) => e.stopPropagation()}
@@ -49,7 +49,7 @@ export type HTTPHeader = { name: string; value: string }
export type HTTPParameter = { name: string; value: string }
type CronTableColumn = {
export type CronTableColumn = {
id: string
name: string
width?: number
@@ -60,7 +60,6 @@ export const IntegrationCard = ({
src={image}
alt={`${name} integration`}
className="w-full h-full object-cover"
objectFit="cover"
/>
) : (
<div className="w-12 h-12 text-foreground relative">
@@ -152,10 +152,7 @@ export const DatabaseInfrastructureSection = ({
className="block group"
>
<MetricCard isLoading={slowQueriesLoading}>
<MetricCardHeader
href={`/project/${projectRef}/observability/query-performance?totalTimeFilter=${encodeURIComponent(JSON.stringify({ operator: '>', value: 1000 }))}`}
linkTooltip="Go to query performance"
>
<MetricCardHeader linkTooltip="Go to query performance">
<MetricCardLabel tooltip="Queries with total execution time (execution time + planning time) greater than 1000ms. High values may indicate query optimization opportunities">
Slow Queries
</MetricCardLabel>
@@ -168,7 +165,7 @@ export const DatabaseInfrastructureSection = ({
<Link href={databaseReportUrl} className="block group">
<MetricCard isLoading={infraLoading}>
<MetricCardHeader href={databaseReportUrl} linkTooltip="Go to database report">
<MetricCardHeader linkTooltip="Go to database report">
<MetricCardLabel tooltip="Highest concurrent database connections observed in the selected window, against the connection limit. Monitor to avoid connection exhaustion.">
Peak Connections
</MetricCardLabel>
@@ -189,7 +186,7 @@ export const DatabaseInfrastructureSection = ({
<Link href={databaseReportUrl} className="block group">
<MetricCard isLoading={infraLoading}>
<MetricCardHeader href={databaseReportUrl} linkTooltip="Go to database report">
<MetricCardHeader linkTooltip="Go to database report">
<MetricCardLabel tooltip="Disk usage percentage of total disk space used">
Disk Usage
</MetricCardLabel>
@@ -208,7 +205,7 @@ export const DatabaseInfrastructureSection = ({
<Link href={databaseReportUrl} className="block group">
<MetricCard isLoading={infraLoading}>
<MetricCardHeader href={databaseReportUrl} linkTooltip="Go to database report">
<MetricCardHeader linkTooltip="Go to database report">
<MetricCardLabel tooltip="Disk I/O consumption percentage. High values may indicate disk bottlenecks">
Disk IO
</MetricCardLabel>
@@ -227,7 +224,7 @@ export const DatabaseInfrastructureSection = ({
<Link href={databaseReportUrl} className="block group">
<MetricCard isLoading={infraLoading}>
<MetricCardHeader href={databaseReportUrl} linkTooltip="Go to database report">
<MetricCardHeader linkTooltip="Go to database report">
<MetricCardLabel tooltip="RAM usage percentage. Sustained high usage may indicate memory pressure">
Memory
</MetricCardLabel>
@@ -246,7 +243,7 @@ export const DatabaseInfrastructureSection = ({
<Link href={databaseReportUrl} className="block group">
<MetricCard isLoading={infraLoading}>
<MetricCardHeader href={databaseReportUrl} linkTooltip="Go to database report">
<MetricCardHeader linkTooltip="Go to database report">
<MetricCardLabel tooltip="CPU usage percentage. High values may suggest CPU-intensive queries or workloads">
CPU
</MetricCardLabel>
@@ -160,7 +160,7 @@ export const ExitSurveyModal = ({ visible, projects, onClose }: ExitSurveyModalP
id="message"
name="message"
value={message}
onChange={(event: any) => setMessage(event.target.value)}
onChange={(event) => setMessage(event.target.value)}
rows={3}
/>
</div>
@@ -434,6 +434,7 @@ export const ColumnEditor = ({
>
<Switch
id="isPrimaryKey"
aria-label="Toggle primary key"
checked={columnFields?.isPrimaryKey ?? false}
onCheckedChange={() =>
onUpdateField({
@@ -446,23 +447,27 @@ export const ColumnEditor = ({
</FormItemLayout>
<Tooltip>
<TooltipTrigger>
<FormItemLayout
isReactForm={false}
layout="flex"
id="isNullable"
label="Allow Nullable"
description="Allow the column to assume a NULL value if no value is provided"
>
<Switch
<TooltipTrigger asChild>
{/* Wrapped in a div as the Switch is a button itself and cannot be nested within the trigger button */}
<div>
<FormItemLayout
isReactForm={false}
layout="flex"
id="isNullable"
disabled={columnFields.isPrimaryKey}
checked={columnFields.isNullable}
onCheckedChange={() =>
onUpdateField({ isNullable: !columnFields.isNullable })
}
/>
</FormItemLayout>
label="Allow Nullable"
description="Allow the column to assume a NULL value if no value is provided"
>
<Switch
id="isNullable"
aria-label="Toggle is nullable"
disabled={columnFields.isPrimaryKey}
checked={columnFields.isNullable}
onCheckedChange={() =>
onUpdateField({ isNullable: !columnFields.isNullable })
}
/>
</FormItemLayout>
</div>
</TooltipTrigger>
{columnFields.isPrimaryKey && (
<TooltipContent side="left" align="start">
@@ -472,21 +477,25 @@ export const ColumnEditor = ({
</Tooltip>
<Tooltip>
<TooltipTrigger>
<FormItemLayout
isReactForm={false}
layout="flex"
id="isUnique"
label="Is Unique"
description="Enforce values in the column to be unique across rows"
>
<Switch
<TooltipTrigger asChild>
{/* Wrapped in a div as the Switch is a button itself and cannot be nested within the trigger button */}
<div>
<FormItemLayout
isReactForm={false}
layout="flex"
id="isUnique"
disabled={columnFields.isPrimaryKey}
checked={columnFields.isUnique}
onCheckedChange={() => onUpdateField({ isUnique: !columnFields.isUnique })}
/>
</FormItemLayout>
label="Is Unique"
description="Enforce values in the column to be unique across rows"
>
<Switch
id="isUnique"
aria-label="Toggle is unique"
disabled={columnFields.isPrimaryKey}
checked={columnFields.isUnique}
onCheckedChange={() => onUpdateField({ isUnique: !columnFields.isUnique })}
/>
</FormItemLayout>
</div>
</TooltipTrigger>
{columnFields.isPrimaryKey && (
<TooltipContent side="left" align="start">
@@ -518,8 +527,14 @@ export const ColumnEditor = ({
>
{isNewRecord && (
<div className="flex items-center gap-x-2">
<Switch checked={createMore} onCheckedChange={() => setCreateMore(!createMore)} />
<Switch
id="toggle-create-more"
aria-label="Toggle create more"
checked={createMore}
onCheckedChange={() => setCreateMore(!createMore)}
/>
<label
htmlFor="toggle-create-more"
className="text-foreground-light text-sm cursor-pointer select-none"
onClick={() => setCreateMore(!createMore)}
>
@@ -5,6 +5,7 @@ import ConfirmationModal from 'ui-patterns/Dialogs/ConfirmationModal'
import { useContentDeleteMutation } from '@/data/content/content-delete-mutation'
import { Snippet } from '@/data/content/sql-folders-query'
import { useDashboardHistory } from '@/hooks/misc/useDashboardHistory'
import { useSqlEditorV2StateSnapshot } from '@/state/sql-editor/sql-editor-state'
import { createTabId, useTabsStateSnapshot } from '@/state/tabs'
@@ -21,8 +22,13 @@ export const DeleteSnippetsModal = ({
const { ref: projectRef, id } = useParams()
const tabs = useTabsStateSnapshot()
const snapV2 = useSqlEditorV2StateSnapshot()
const { clearSnippetsFromHistory } = useDashboardHistory()
const postDeleteCleanup = (ids: string[]) => {
// Purge the deleted snippets from dashboard history first, so that navigating
// to the SQL editor doesn't redirect back to a deleted snippet
clearSnippetsFromHistory(ids)
if (!!id && ids.includes(id)) {
const openedSQLTabs = tabs.openTabs.filter((x) => x.startsWith('sql-') && !x.includes(id))
if (openedSQLTabs.length > 0) {
@@ -35,6 +35,7 @@ import { useContentDeleteMutation } from '@/data/content/content-delete-mutation
import { useSQLSnippetFoldersDeleteMutation } from '@/data/content/sql-folders-delete-mutation'
import { Snippet, SnippetFolder, useSQLSnippetFoldersQuery } from '@/data/content/sql-folders-query'
import { useSqlSnippetsQuery } from '@/data/content/sql-snippets-query'
import { useDashboardHistory } from '@/hooks/misc/useDashboardHistory'
import { useLocalStorage } from '@/hooks/misc/useLocalStorage'
import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject'
import { useProfile } from '@/lib/profile'
@@ -54,6 +55,7 @@ export const SQLEditorNav = ({ sort = 'inserted_at' }: SQLEditorNavProps) => {
const { data: project } = useSelectedProjectQuery()
const tabs = useTabsStateSnapshot()
const snapV2 = useSqlEditorV2StateSnapshot()
const { clearSnippetsFromHistory } = useDashboardHistory()
const [sectionVisibility, setSectionVisibility] = useLocalStorage<SectionState>(
LOCAL_STORAGE_KEYS.SQL_EDITOR_SECTION_STATE(projectRef ?? ''),
@@ -324,6 +326,10 @@ export const SQLEditorNav = ({ sort = 'inserted_at' }: SQLEditorNavProps) => {
// ===============
const postDeleteCleanup = (ids: string[]) => {
// Purge the deleted snippets from dashboard history first, so that navigating
// to the SQL editor doesn't redirect back to a deleted snippet
clearSnippetsFromHistory(ids)
// [Refactor] To investigate - deleting a snippet while it's open, will have it in the side nav
// for a bit, before it gets removed (assumingly invalidated)
setShowDeleteModal(false)
@@ -0,0 +1,63 @@
import { HttpResponse } from 'msw'
import { describe, expect, it } from 'vitest'
import { getMigrations } from './migrations-query'
import { addAPIMock } from '@/tests/lib/msw'
describe('getMigrations', () => {
it('returns the list of migrations', async () => {
addAPIMock({
method: 'post',
path: '/platform/pg-meta/:ref/query',
response: () =>
HttpResponse.json([
{
version: '20240202000000',
name: 'add_projects',
statements: ['create table public.projects (id int)'],
},
{ version: '20240101000000', name: 'create_users', statements: null },
]),
})
const result = await getMigrations({ projectRef: 'default' })
expect(result).toEqual([
{
version: '20240202000000',
name: 'add_projects',
statements: ['create table public.projects (id int)'],
},
{ version: '20240101000000', name: 'create_users', statements: null },
])
})
it('treats a missing migrations table as an empty list instead of an error', async () => {
// Safety net: the SQL itself is defensive (see @supabase/pg-meta getMigrationsSql),
// but if the relation-missing error still surfaces it must not become a failed query
// that 400s and retries on every project page load.
addAPIMock({
method: 'post',
path: '/platform/pg-meta/:ref/query',
response: () =>
HttpResponse.json(
{ message: 'relation "supabase_migrations.schema_migrations" does not exist' },
{ status: 400 }
),
})
const result = await getMigrations({ projectRef: 'default' })
expect(result).toEqual([])
})
it('rethrows other errors', async () => {
addAPIMock({
method: 'post',
path: '/platform/pg-meta/:ref/query',
response: () => HttpResponse.json({ message: 'permission denied' }, { status: 400 }),
})
await expect(getMigrations({ projectRef: 'default' })).rejects.toThrowError('permission denied')
})
})
@@ -0,0 +1,67 @@
import { act, waitFor } from '@testing-library/react'
import { beforeEach, describe, expect, it } from 'vitest'
import { useDashboardHistory } from './useDashboardHistory'
import { customRenderHook } from '@/tests/lib/custom-render'
// useParams from 'common' is globally mocked to { ref: 'default' } in vitestSetup
const renderDashboardHistory = async () => {
const utils = customRenderHook(() => useDashboardHistory())
await waitFor(() => expect(utils.result.current.isHistoryLoaded).toBe(true))
return utils
}
describe('useDashboardHistory', () => {
beforeEach(() => {
localStorage.clear()
})
it('stores the last visited snippet', async () => {
const { result } = await renderDashboardHistory()
act(() => result.current.setLastVisitedSnippet('snippet-a'))
await waitFor(() => expect(result.current.history.sql).toBe('snippet-a'))
})
describe('clearSnippetsFromHistory', () => {
it('purges the last visited snippet when it is one of the deleted ids', async () => {
const { result } = await renderDashboardHistory()
act(() => result.current.setLastVisitedSnippet('snippet-a'))
await waitFor(() => expect(result.current.history.sql).toBe('snippet-a'))
act(() => result.current.clearSnippetsFromHistory(['snippet-b', 'snippet-a']))
await waitFor(() => expect(result.current.history.sql).toBeUndefined())
})
it('keeps the last visited snippet when it is not among the deleted ids', async () => {
const { result } = await renderDashboardHistory()
act(() => result.current.setLastVisitedSnippet('snippet-a'))
await waitFor(() => expect(result.current.history.sql).toBe('snippet-a'))
act(() => result.current.clearSnippetsFromHistory(['snippet-b']))
await waitFor(() => expect(result.current.history.sql).toBe('snippet-a'))
})
it('does not touch the table editor history', async () => {
const { result } = await renderDashboardHistory()
act(() => result.current.setLastVisitedTable('table-1'))
// setLastVisitedSnippet spreads the render-time history, so wait for the
// table update to propagate before setting the snippet
await waitFor(() => expect(result.current.history.editor).toBe('table-1'))
act(() => result.current.setLastVisitedSnippet('snippet-a'))
await waitFor(() => expect(result.current.history.sql).toBe('snippet-a'))
act(() => result.current.clearSnippetsFromHistory(['snippet-a']))
await waitFor(() => expect(result.current.history.sql).toBeUndefined())
expect(result.current.history.editor).toBe('table-1')
})
})
})
@@ -22,10 +22,23 @@ export const useDashboardHistory = () => {
setHistory({ ...history, sql: id })
}
/**
* Purge the last-visited snippet when it's one of the deleted snippets, so that
* navigating back to the SQL editor doesn't resurrect a deleted snippet.
*/
const clearSnippetsFromHistory = (ids: string[]) => {
setHistory((current) =>
current.sql !== undefined && ids.includes(current.sql)
? { ...current, sql: undefined }
: current
)
}
return {
history,
setLastVisitedTable,
setLastVisitedSnippet,
clearSnippetsFromHistory,
isHistoryLoaded: isSuccess,
}
}
@@ -44,7 +44,11 @@ export const LogsSavedPage: NextPageWithLayout = () => {
</>
}
body={saved.map((item) => (
<SavedQueriesItem key={item.id} item={item} />
<Table.tr key={item.id}>
<Table.td colSpan={5} className="p-0!">
<SavedQueriesItem item={item} />
</Table.td>
</Table.tr>
))}
/>
</div>
+27 -2
View File
@@ -3,6 +3,7 @@ import { useParams } from 'common/hooks/useParams'
import Link from 'next/link'
import { useRouter } from 'next/router'
import { useEffect } from 'react'
import { toast } from 'sonner'
import { Button } from 'ui'
import { Admonition } from 'ui-patterns/admonition'
@@ -31,7 +32,7 @@ const SqlEditor: NextPageWithLayout = () => {
const editor = useEditorType()
const tabs = useTabsStateSnapshot()
const snapV2 = useSqlEditorV2StateSnapshot()
const { history, setLastVisitedSnippet } = useDashboardHistory()
const { history, setLastVisitedSnippet, clearSnippetsFromHistory } = useDashboardHistory()
const allSnippets = useSnippets(ref!)
const snippet = allSnippets.find((x) => x.id === id)
@@ -63,6 +64,8 @@ const SqlEditor: NextPageWithLayout = () => {
const snippetMissingImmediatelyAfterCreating =
!!snippet && snippetMissing && previousRoute === 'new' && wasNeverPersisted(snippet.status)
const isSnippetDeleted = snippetMissing && !snippetMissingImmediatelyAfterCreating
useEffect(() => {
if (ref && data && project) {
// [Joshen] Check if snippet belongs to the current project
@@ -109,7 +112,29 @@ const SqlEditor: NextPageWithLayout = () => {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [router.isReady, id])
if ((snippetMissing || invalidId) && !snippetMissingImmediatelyAfterCreating) {
// The snippet no longer exists (e.g. deleted from another tab or session): clean up
// any stale tab and dashboard history references so navigation doesn't resurrect it,
// then fall back to a new snippet instead of rendering a dead state
useEffect(() => {
if (!ref || !id || id === 'new') return
if (!isSnippetDeleted) return
const staleTabId = createTabId('sql', { id })
if (tabs.hasTab(staleTabId)) tabs.removeTab(staleTabId)
if (snippet !== undefined) snapV2.removeSnippet(id)
clearSnippetsFromHistory([id])
toast(`The SQL snippet you were trying to open no longer exists. Opened a new query instead.`)
router.replace(`/project/${ref}/sql/new`)
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isSnippetDeleted, id, ref])
// Render nothing while the effect above redirects away from the deleted snippet
if (isSnippetDeleted) {
return null
}
if (invalidId) {
return (
<div className="flex items-center justify-center h-full">
<div className="w-[400px]">
+67
View File
@@ -60,3 +60,70 @@ describe('tabs recent items', () => {
expect(store.recentItems[0].metadata?.name).toBe('routines')
})
})
describe('tabs removal', () => {
beforeEach(() => {
localStorage.clear()
})
const addSqlTab = (store: ReturnType<typeof createTabsState>, id: string, isPreview = false) => {
store.addTab({
id: `sql-${id}`,
type: 'sql',
label: `Snippet ${id}`,
metadata: { sqlId: id, name: `Snippet ${id}` },
isPreview,
})
}
it('removes deleted snippet tabs and reassigns the active tab', () => {
const store = createTabsState('default')
addSqlTab(store, 'a')
addSqlTab(store, 'b')
addSqlTab(store, 'c')
expect(store.activeTab).toBe('sql-c')
store.removeTabs(['sql-c', 'sql-b'])
expect(store.openTabs).toEqual(['sql-a'])
expect(store.tabsMap['sql-b']).toBeUndefined()
expect(store.tabsMap['sql-c']).toBeUndefined()
expect(store.activeTab).toBe('sql-a')
})
it('clears the active tab when the last tab is removed', () => {
const store = createTabsState('default')
addSqlTab(store, 'a')
store.removeTab('sql-a')
expect(store.openTabs).toEqual([])
expect(store.activeTab).toBeNull()
})
it('clears previewTabId when the preview tab is removed', () => {
const store = createTabsState('default')
addSqlTab(store, 'a')
addSqlTab(store, 'b', true)
expect(store.previewTabId).toBe('sql-b')
store.removeTab('sql-b')
expect(store.previewTabId).toBeUndefined()
expect(store.openTabs).toEqual(['sql-a'])
})
it('keeps previewTabId when a non-preview tab is removed', () => {
const store = createTabsState('default')
addSqlTab(store, 'a')
addSqlTab(store, 'b', true)
store.removeTab('sql-a')
expect(store.previewTabId).toBe('sql-b')
expect(store.openTabs).toEqual(['sql-b'])
})
})
+6
View File
@@ -248,6 +248,12 @@ export function createTabsState(projectRef: string) {
store.openTabs = store.openTabs.filter((tabId) => tabId !== id)
delete store.tabsMap[id]
// Clear the preview tab reference if the removed tab was the preview tab,
// so it doesn't linger in (persisted) state pointing at a closed tab
if (store.previewTabId === id) {
store.previewTabId = undefined
}
// Update active tab if the removed tab was active
if (id === store.activeTab) {
store.activeTab = store.openTabs[idx - 1] || store.openTabs[idx + 1] || null
@@ -1,11 +1,53 @@
import { literal, safeSql, type SafeSqlFragment } from '../../../pg-format'
export const getMigrationsSql = (): SafeSqlFragment => {
// The migrations table only exists once a migration has been applied (e.g. via
// the CLI or the dashboard). Guarding the select inside a PL/pgSQL block (behind
// a to_regclass check) defers planning of the table reference, so this returns
// zero rows instead of erroring with 42P01 (undefined_table) when the table is
// absent. query_to_xml is deliberately avoided: it is forbidden through
// Multigres's connection pooler.
//
// The do-block stashes the result in a transaction-local GUC (set_config with
// is_local = true) and the trailing select reads it back. pg-meta sends this
// whole string as a single simple-protocol query, which Postgres executes in one
// implicit transaction, so the GUC is visible to the select and reverts once the
// query finishes (nothing leaks into pooled connections).
//
// Rows are serialized with to_jsonb so the query also tolerates older tables
// that only have a `version` column (no `name`/`statements`).
const sql = safeSql`
do $$
declare
migrations text;
begin
if pg_catalog.to_regclass('supabase_migrations.schema_migrations') is not null then
select coalesce(
pg_catalog.jsonb_agg(pg_catalog.to_jsonb(sm) order by sm.version desc),
'[]'::pg_catalog.jsonb
)::text
into migrations
from supabase_migrations.schema_migrations sm;
else
migrations := '[]';
end if;
perform pg_catalog.set_config('supabase.studio_migrations', migrations, true);
end $$;
select
*
from supabase_migrations.schema_migrations sm
order by sm.version desc
m->>'version' as version,
m->>'name' as name,
case
when pg_catalog.jsonb_typeof(m->'statements') = 'array'
then array(select pg_catalog.jsonb_array_elements_text(m->'statements'))
else null
end as statements
from pg_catalog.jsonb_array_elements(
coalesce(
nullif(pg_catalog.current_setting('supabase.studio_migrations', true), '')::pg_catalog.jsonb,
'[]'::pg_catalog.jsonb
)
) as m
order by m->>'version' desc
`
return sql
+10 -3
View File
@@ -1,5 +1,5 @@
import { randomUUID } from 'crypto'
import pg, { Pool } from 'pg'
import pg, { Pool, type QueryResult } from 'pg'
import { parse as parseArray } from 'postgres-array'
// Those types override are in sync with `postgres-meta` since the queries
@@ -50,8 +50,15 @@ export async function createTestDatabase() {
client: 'pg' as const,
executeQuery: async <T = any>(query: string): Promise<T> => {
try {
const res = await pool.query(query)
return res.rows as T
const res: QueryResult | QueryResult[] = await pool.query(query)
// node-pg resolves multi-statement queries with one result per
// statement. Mirror `postgres-meta`, which returns the rows of the
// last statement that produced any:
// https://github.com/supabase/postgres-meta/blob/master/src/lib/db.ts
const rows = Array.isArray(res)
? (res.reverse().find((x) => x.rows.length !== 0)?.rows ?? [])
: res.rows
return rows as T
} catch (error) {
if (error instanceof Error) {
throw new Error(`Failed to execute query: ${error.message}`)
@@ -0,0 +1,155 @@
import { afterAll, expect, test } from 'vitest'
import {
getCreateMigrationsTableSQL,
getInsertMigrationSQL,
getMigrationsSql,
} from '../../../src/sql/studio/database/migrations'
import { cleanupRoot, createTestDatabase } from '../../db/utils'
afterAll(async () => {
await cleanupRoot()
})
const withTestDatabase = (
name: string,
fn: (db: Awaited<ReturnType<typeof createTestDatabase>>) => Promise<void>
) => {
test(name, async () => {
const db = await createTestDatabase()
try {
await fn(db)
} finally {
await db.cleanup()
}
})
}
withTestDatabase(
'returns zero rows (not an error) when the migrations table does not exist',
async ({ executeQuery }) => {
// Regression test: this used to throw 42P01 (undefined_table), which surfaced
// as a 400 on every dashboard load for projects without migrations.
const result = await executeQuery(getMigrationsSql())
expect(result).toEqual([])
}
)
withTestDatabase(
'returns migrations ordered by version desc when the table exists',
async ({ executeQuery }) => {
await executeQuery(getCreateMigrationsTableSQL())
await executeQuery(
getInsertMigrationSQL({
version: '20240101000000',
name: 'create_users',
statements: JSON.stringify(['create table public.users (id int primary key)']),
})
)
await executeQuery(
getInsertMigrationSQL({
version: '20240202000000',
name: `special <chars> & "quotes" 'apostrophes'`,
statements: JSON.stringify([`select '<a>&amp;</a>' as x`, 'select 2']),
})
)
const result = await executeQuery(getMigrationsSql())
expect(result).toEqual([
{
version: '20240202000000',
name: `special <chars> & "quotes" 'apostrophes'`,
statements: [`select '<a>&amp;</a>' as x`, 'select 2'],
},
{
version: '20240101000000',
name: 'create_users',
statements: ['create table public.users (id int primary key)'],
},
])
}
)
withTestDatabase('handles null name and statements', async ({ executeQuery }) => {
await executeQuery(getCreateMigrationsTableSQL())
await executeQuery(
`insert into supabase_migrations.schema_migrations (version) values ('20240303000000')`
)
const result = await executeQuery(getMigrationsSql())
expect(result).toEqual([{ version: '20240303000000', name: null, statements: null }])
})
withTestDatabase(
'works as the single multi-statement string pg-meta sends (statement_timeout prefix included)',
async ({ executeQuery }) => {
// postgres-meta concatenates a statement_timeout prefix and sends the whole
// thing as one simple-protocol query (a single implicit transaction), which
// is what lets the transaction-local GUC set in the do-block survive to the
// trailing select:
// https://github.com/supabase/postgres-meta/blob/master/src/lib/db.ts
await executeQuery(getCreateMigrationsTableSQL())
await executeQuery(
getInsertMigrationSQL({
version: '20240101000000',
name: 'create_users',
statements: JSON.stringify(['create table public.users (id int primary key)']),
})
)
const result = await executeQuery(
`SET statement_timeout='10s'; SET idle_session_timeout='10s';${getMigrationsSql()}`
)
expect(result).toEqual([
{
version: '20240101000000',
name: 'create_users',
statements: ['create table public.users (id int primary key)'],
},
])
}
)
withTestDatabase(
'does not leak the migrations GUC into the (pooled) session',
async ({ executeQuery }) => {
await executeQuery(getCreateMigrationsTableSQL())
await executeQuery(
getInsertMigrationSQL({
version: '20240101000000',
name: 'create_users',
statements: JSON.stringify(['create table public.users (id int primary key)']),
})
)
await executeQuery(getMigrationsSql())
// The pool is capped at one connection, so this runs on the same session.
// set_config(..., true) is transaction-local: once the query's implicit
// transaction ends, the migration payload must be gone.
const [{ value }] = await executeQuery(
`select current_setting('supabase.studio_migrations', true) as value`
)
expect([null, '']).toContain(value)
}
)
withTestDatabase(
'tolerates legacy migrations tables with only a version column',
async ({ executeQuery }) => {
// Older CLI versions created schema_migrations with a single `version` column
await executeQuery(`
create schema if not exists supabase_migrations;
create table supabase_migrations.schema_migrations (version text not null primary key);
insert into supabase_migrations.schema_migrations (version) values ('20230101000000');
`)
const result = await executeQuery(getMigrationsSql())
expect(result).toEqual([{ version: '20230101000000', name: null, statements: null }])
}
)
+18 -5
View File
@@ -61,6 +61,12 @@ const MetricCard = React.forwardRef<HTMLDivElement, MetricCardProps>(
MetricCard.displayName = 'MetricCard'
interface MetricCardHeaderProps extends React.HTMLAttributes<HTMLDivElement> {
/**
* Renders the chevron affordance as a link. Omit the href (while keeping
* linkTooltip) when the card is already wrapped in a link — nesting an
* anchor within an anchor is invalid HTML, and clicks on the chevron will
* fall through to the wrapping link instead.
*/
href?: string
children: React.ReactNode
linkTooltip?: string
@@ -78,7 +84,7 @@ const MetricCardHeader = React.forwardRef<HTMLDivElement, MetricCardHeaderProps>
{...props}
>
<div className="flex flex-row items-center gap-2">{children}</div>
{href ? (
{href || linkTooltip ? (
<Tooltip>
<TooltipTrigger asChild>
<Button
@@ -87,10 +93,17 @@ const MetricCardHeader = React.forwardRef<HTMLDivElement, MetricCardHeaderProps>
className="px-1 text-foreground-lighter group-hover:text-foreground absolute right-3 transition-colors"
asChild
>
<Link href={href}>
<ChevronRight aria-disabled={true} size={14} strokeWidth={1.5} />
<span className="sr-only">More information</span>
</Link>
{href ? (
<Link href={href}>
<ChevronRight aria-disabled={true} size={14} strokeWidth={1.5} />
<span className="sr-only">More information</span>
</Link>
) : (
<span>
<ChevronRight aria-disabled={true} size={14} strokeWidth={1.5} />
<span className="sr-only">More information</span>
</span>
)}
</Button>
</TooltipTrigger>
{linkTooltip ? <TooltipContent>{linkTooltip}</TooltipContent> : null}
@@ -318,12 +318,14 @@ export const FormLayout = React.forwardRef<
{description}
</FormDescription>
) : description ? (
<p
// Rendered as a div rather than a p as descriptions can be arbitrary JSX
// which may contain block-level elements (invalid HTML inside a p)
<div
className={cn(DescriptionVariants({ size, layout }), 'text-sm text-foreground-light')}
data-formlayout-id={'description'}
>
{description}
</p>
</div>
) : null
const LabelContents = () => (