diff --git a/apps/design-system/content/docs/ui-patterns/modality.mdx b/apps/design-system/content/docs/ui-patterns/modality.mdx index eca2d40bdce..9c4358cce0c 100644 --- a/apps/design-system/content/docs/ui-patterns/modality.mdx +++ b/apps/design-system/content/docs/ui-patterns/modality.mdx @@ -96,6 +96,7 @@ Implementation checklist: - Render a separate discard confirmation dialog when dirty. - Keep `Cancel` non-destructive; use `Discard`/`Discard changes` for one-click destructive exits. - Guard controlled close attempts only; do not try to block route changes or arbitrary unmounts. +- If dismissal is route-driven or tied to page unload, use a navigation guard that composes the same discard-confirmation UI instead of extending the dialog/sheet close guard. Studio implementation (preferred in Studio code): diff --git a/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.tsx b/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.tsx index 11c008d3839..2ae983efc8e 100644 --- a/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.tsx +++ b/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.tsx @@ -1,32 +1,39 @@ import type { PostgresTable } from '@supabase/postgres-meta' +import { DiscardChangesConfirmationDialog } from 'components/ui-patterns/Dialogs/DiscardChangesConfirmationDialog' +import { DocsButton } from 'components/ui/DocsButton' +import InformationBox from 'components/ui/InformationBox' +import { FOREIGN_KEY_CASCADE_ACTION } from 'data/database/database-query-constants' +import { useSchemasQuery } from 'data/database/schemas-query' +import { useTableQuery } from 'data/tables/table-retrieve-query' +import { useTablesQuery } from 'data/tables/tables-query' +import { useQuerySchemaState } from 'hooks/misc/useSchemaQueryState' +import { useSelectedProjectQuery } from 'hooks/misc/useSelectedProject' +import { useConfirmOnClose } from 'hooks/ui/useConfirmOnClose' +import { DOCS_URL } from 'lib/constants' +import { uuidv4 } from 'lib/helpers' import { sortBy } from 'lodash' import { ArrowRight, Database, HelpCircle, Loader2, Table, X } from 'lucide-react' import { Fragment, useEffect, useState } from 'react' import { + Alert_Shadcn_, AlertDescription_Shadcn_, AlertTitle_Shadcn_, - Alert_Shadcn_, Button, Listbox, SidePanel, } from 'ui' -import { DocsButton } from 'components/ui/DocsButton' -import InformationBox from 'components/ui/InformationBox' -import { FOREIGN_KEY_CASCADE_ACTION } from 'data/database/database-query-constants' -import { useSchemasQuery } from 'data/database/schemas-query' -import { useTableQuery } from 'data/tables/table-retrieve-query' -import { useTablesQuery } from 'data/tables/tables-query' -import { useQuerySchemaState } from 'hooks/misc/useSchemaQueryState' -import { useSelectedProjectQuery } from 'hooks/misc/useSelectedProject' -import { DOCS_URL } from 'lib/constants' -import { uuidv4 } from 'lib/helpers' import { ActionBar } from '../ActionBar' import { NUMERICAL_TYPES, TEXT_TYPES } from '../SidePanelEditor.constants' import type { ColumnField } from '../SidePanelEditor.types' import { FOREIGN_KEY_CASCADE_OPTIONS } from './ForeignKeySelector.constants' import type { ForeignKey, SelectorErrors, SelectorTypeError } from './ForeignKeySelector.types' -import { generateCascadeActionDescription } from './ForeignKeySelector.utils' +import { + generateCascadeActionDescription, + hasForeignKeySelectorChanges, + normalizeForeignKeyForDirtyCheck, + type ForeignKeyDirtyState, +} from './ForeignKeySelector.utils' const EMPTY_STATE: ForeignKey = { id: undefined, @@ -63,6 +70,7 @@ export const ForeignKeySelector = ({ const [fk, setFk] = useState(EMPTY_STATE) const [errors, setErrors] = useState({}) + const [initialSnapshot, setInitialSnapshot] = useState() const hasTypeErrors = (errors.types ?? []).length > 0 const hasTypeNotices = (errors.typeNotice ?? []).length > 0 @@ -93,6 +101,11 @@ export const ForeignKeySelector = ({ const disableApply = isLoadingSelectedTable || selectedTable === undefined || hasTypeErrors + const { confirmOnClose, modalProps } = useConfirmOnClose({ + checkIsDirty: () => hasForeignKeySelectorChanges(initialSnapshot, fk), + onClose, + }) + const updateSelectedSchema = (schema: string) => { const updatedFk = { ...EMPTY_STATE, id: fk.id, schema } setFk(updatedFk) @@ -203,8 +216,11 @@ export const ForeignKeySelector = ({ useEffect(() => { if (visible) { - if (foreignKey !== undefined) setFk(foreignKey) - else setFk({ ...EMPTY_STATE, id: uuidv4() }) + const initialFk = foreignKey !== undefined ? foreignKey : { ...EMPTY_STATE, id: uuidv4() } + + setErrors({}) + setFk(initialFk) + setInitialSnapshot(normalizeForeignKeyForDirtyCheck(initialFk)) } // eslint-disable-next-line react-hooks/exhaustive-deps }, [visible]) @@ -215,137 +231,176 @@ export const ForeignKeySelector = ({ }, [fk, visible]) return ( - 0 ? table.name : 'new table'}`} - customFooter={ - validateSelection(resolve)} - /> - } - > - -
- } - title="What are foreign keys?" - description={`Foreign keys help maintain referential integrity of your data by ensuring that no + <> + 0 ? table.name : 'new table'}`} + customFooter={ + validateSelection(resolve)} + /> + } + > + +
+ } + title="What are foreign keys?" + description={`Foreign keys help maintain referential integrity of your data by ensuring that no one can insert rows into the table that do not have a matching entry to another table.`} - url="https://www.postgresql.org/docs/current/tutorial-fk.html" - urlLabel="Postgres Foreign Key Documentation" - /> - - updateSelectedSchema(value)} - > - {sortedSchemas.map((schema) => { - return ( - } - > -
- {/* For aria searching to target the schema name instead of schema */} - {schema.name} - {schema.name} -
- - ) - })} - - - updateSelectedTable(Number(value))} - disabled={isLoadingSelectedTable} - > - - --- - - {sortBy(tables, ['schema']).map((table) => { - return ( - } - > -
- {/* For aria searching to target the table name instead of schema */} - {table.name} - {table.schema} - {table.name} -
- - ) - })} - - - {fk.schema && fk.table && ( - <> - {isLoadingSelectedTable ? ( -
- -

Loading table columns

-
- ) : ( -
- -
-
- {selectedSchema}.{table.name.length > 0 ? table.name : '[unnamed table]'} + url="https://www.postgresql.org/docs/current/tutorial-fk.html" + urlLabel="Postgres Foreign Key Documentation" + /> + + updateSelectedSchema(value)} + > + {sortedSchemas.map((schema) => { + return ( + } + > +
+ {/* For aria searching to target the schema name instead of schema */} + {schema.name} + {schema.name}
-
- {fk.schema}.{fk.table} + + ) + })} + + + updateSelectedTable(Number(value))} + disabled={isLoadingSelectedTable} + > + + --- + + {sortBy(tables, ['schema']).map((table) => { + return ( +
} + > +
+ {/* For aria searching to target the table name instead of schema */} + {table.name} + {table.schema} + {table.name}
- {fk.columns.length === 0 && ( - - - There are no foreign key relations between the tables - - - )} - {fk.columns.map((_, idx) => ( - -
- updateSelectedColumn(idx, 'source', value)} - > - + ) + })} + + + {fk.schema && fk.table && ( + <> + {isLoadingSelectedTable ? ( +
+ +

Loading table columns

+
+ ) : ( +
+ +
+
+ {selectedSchema}.{table.name.length > 0 ? table.name : '[unnamed table]'} +
+
+ {fk.schema}.{fk.table} +
+ {fk.columns.length === 0 && ( + + + There are no foreign key relations between the tables + + + )} + {fk.columns.map((_, idx) => ( + +
+ + updateSelectedColumn(idx, 'source', value) + } > - --- - - {(table?.columns ?? []) - .filter((x) => x.name.length !== 0) - .map((column) => ( + + --- + + {(table?.columns ?? []) + .filter((x) => x.name.length !== 0) + .map((column) => ( + +
+ {column.name} + + {column.format === '' ? '-' : column.format} + +
+ + ))} + +
+
+ +
+
+ + updateSelectedColumn(idx, 'target', value) + } + > + + --- + + {(selectedTable?.columns ?? []).map((column) => (
{column.name} - - {column.format === '' ? '-' : column.format} - + {column.format}
))} - -
-
- -
-
- updateSelectedColumn(idx, 'target', value)} - > - - --- - - {(selectedTable?.columns ?? []).map((column) => ( - -
- {column.name} - {column.format} -
- - ))} - -
-
-
- - ))} -
-
- - {errors.columns &&

{errors.columns}

} - {hasTypeErrors && ( - - Column types do not match - - The following columns cannot be referenced as they are not of the same - type: - -
    - {(errors?.types ?? []).map((x, idx: number) => { - if (x === undefined) return null - return ( -
  • - {x.source} ({x.sourceType} - ) and {x.target}( - {x.targetType}) -
    • - ) - })} -
- - )} - {hasTypeNotices && ( - - Column types will be updated - - The following columns will have their types updated to match their - referenced column - -
    - {(errors?.typeNotice ?? []).map((x, idx: number) => { - if (x === undefined) return null - return ( -
  • -
    - {x.source}{' '} - {x.targetType} -
    -
    • - ) - })} -
- - )} + +
+
+
+ + ))} +
+
+ + {errors.columns &&

{errors.columns}

} + {hasTypeErrors && ( + + Column types do not match + + The following columns cannot be referenced as they are not of the same + type: + +
    + {(errors?.types ?? []).map((x, idx: number) => { + if (x === undefined) return null + return ( +
  • + {x.source} ( + {x.sourceType}) and{' '} + {x.target}( + {x.targetType}) +
    • + ) + })} +
+ + )} + {hasTypeNotices && ( + + Column types will be updated + + The following columns will have their types updated to match their + referenced column + +
    + {(errors?.typeNotice ?? []).map((x, idx: number) => { + if (x === undefined) return null + return ( +
  • +
    + {x.source}{' '} + {x.targetType} +
    +
    • + ) + })} +
+ + )} +
- - )} - - {!isLoadingSelectedTable && ( - <> - - - } - title="Which action is most appropriate?" - description={ - <> -

- The choice of the action depends on what kinds of objects the related - tables represent: -

-
    -
  • - Cascade: if the referencing - table represents something that is a component of what is represented by - the referenced table and cannot exist independently -
    • -
  • - Restrict or{' '} - No action: if the two tables - represent independent objects -
    • -
  • - Set NULL or{' '} - Set default: if a foreign-key - relationship represents optional information -
    • -
-

- Typically, restricting and cascading deletes are the most common options, - but the default behavior is no action -

- - } - url="https://www.postgresql.org/docs/current/ddl-constraints.html#DDL-CONSTRAINTS-FK" - urlLabel="More information" - /> - - - {generateCascadeActionDescription( - 'update', - fk.updateAction, - `${fk.schema}.${fk.table}` - )} -

- } - onChange={(value: string) => updateCascadeAction('updateAction', value)} - > - {FOREIGN_KEY_CASCADE_OPTIONS.filter((option) => - ['no-action', 'cascade', 'restrict'].includes(option.key) - ).map((option) => ( - -

{option.label}

- - ))} - - - - } - descriptionText={ - <> + )} + + {!isLoadingSelectedTable && ( + <> + + + } + title="Which action is most appropriate?" + description={ + <> +

+ The choice of the action depends on what kinds of objects the related + tables represent: +

+
    +
  • + Cascade: if the referencing + table represents something that is a component of what is represented + by the referenced table and cannot exist independently +
    • +
  • + Restrict or{' '} + No action: if the two tables + represent independent objects +
    • +
  • + Set NULL or{' '} + Set default: if a + foreign-key relationship represents optional information +
    • +
+

+ Typically, restricting and cascading deletes are the most common + options, but the default behavior is no action +

+ + } + url="https://www.postgresql.org/docs/current/ddl-constraints.html#DDL-CONSTRAINTS-FK" + urlLabel="More information" + /> + + {generateCascadeActionDescription( - 'delete', - fk.deletionAction, + 'update', + fk.updateAction, `${fk.schema}.${fk.table}` )}

- - } - onChange={(value: string) => updateCascadeAction('deletionAction', value)} - > - {FOREIGN_KEY_CASCADE_OPTIONS.map((option) => ( - -

{option.label}

- - ))} - - - )} - - )} - - - + } + onChange={(value: string) => updateCascadeAction('updateAction', value)} + > + {FOREIGN_KEY_CASCADE_OPTIONS.filter((option) => + ['no-action', 'cascade', 'restrict'].includes(option.key) + ).map((option) => ( + +

{option.label}

+ + ))} + + + + } + descriptionText={ + <> +

+ {generateCascadeActionDescription( + 'delete', + fk.deletionAction, + `${fk.schema}.${fk.table}` + )} +

+ + } + onChange={(value: string) => updateCascadeAction('deletionAction', value)} + > + {FOREIGN_KEY_CASCADE_OPTIONS.map((option) => ( + +

{option.label}

+ + ))} + + + )} + + )} + + + + + ) } diff --git a/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.utils.test.ts b/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.utils.test.ts new file mode 100644 index 00000000000..6bad5f68239 --- /dev/null +++ b/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.utils.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from 'vitest' + +import type { ForeignKey } from './ForeignKeySelector.types' +import { + hasForeignKeySelectorChanges, + normalizeForeignKeyForDirtyCheck, +} from './ForeignKeySelector.utils' + +const BASE_FOREIGN_KEY: ForeignKey = { + id: 'fk-1', + name: 'messages_author_id_fkey', + tableId: 42, + schema: 'public', + table: 'profiles', + columns: [{ source: 'author_id', target: 'id' }], + deletionAction: 'NO ACTION', + updateAction: 'NO ACTION', +} + +describe('ForeignKeySelector dirty state', () => { + it('is clean on first open', () => { + const initialState = normalizeForeignKeyForDirtyCheck(BASE_FOREIGN_KEY) + + expect(hasForeignKeySelectorChanges(initialState, BASE_FOREIGN_KEY)).toBe(false) + }) + + it('is dirty after semantic foreign key changes', () => { + const initialState = normalizeForeignKeyForDirtyCheck(BASE_FOREIGN_KEY) + + expect( + hasForeignKeySelectorChanges(initialState, { + ...BASE_FOREIGN_KEY, + schema: 'storage', + }) + ).toBe(true) + + expect( + hasForeignKeySelectorChanges(initialState, { + ...BASE_FOREIGN_KEY, + table: 'users', + tableId: 99, + }) + ).toBe(true) + + expect( + hasForeignKeySelectorChanges(initialState, { + ...BASE_FOREIGN_KEY, + columns: [{ source: 'owner_id', target: 'id' }], + }) + ).toBe(true) + + expect( + hasForeignKeySelectorChanges(initialState, { + ...BASE_FOREIGN_KEY, + deletionAction: 'CASCADE', + }) + ).toBe(true) + + expect( + hasForeignKeySelectorChanges(initialState, { + ...BASE_FOREIGN_KEY, + updateAction: 'CASCADE', + }) + ).toBe(true) + }) + + it('ignores derived column type metadata when checking dirty state', () => { + const initialState = normalizeForeignKeyForDirtyCheck(BASE_FOREIGN_KEY) + + expect( + hasForeignKeySelectorChanges(initialState, { + ...BASE_FOREIGN_KEY, + columns: [ + { + source: 'author_id', + sourceType: 'uuid', + target: 'id', + targetType: 'uuid', + }, + ], + }) + ).toBe(false) + }) +}) diff --git a/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.utils.tsx b/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.utils.tsx index fcb10df4f84..c909ee00a7a 100644 --- a/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.utils.tsx +++ b/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.utils.tsx @@ -1,10 +1,23 @@ import { FOREIGN_KEY_CASCADE_ACTION } from 'data/database/database-query-constants' import type { ForeignKeyConstraint } from 'data/database/foreign-key-constraints-query' +import { isEqual } from 'lodash' import { HelpCircle } from 'lucide-react' import { Tooltip, TooltipContent, TooltipTrigger } from 'ui' + import { getForeignKeyCascadeAction } from '../ColumnEditor/ColumnEditor.utils' import type { ForeignKey } from './ForeignKeySelector.types' +export interface ForeignKeyDirtyState { + id?: number | string + name?: string + tableId?: number + schema: string + table: string + columns: { source: string; target: string }[] + deletionAction: string + updateAction: string +} + export const formatForeignKeys = (fks: ForeignKeyConstraint[]): ForeignKey[] => { return fks.map((x) => { return { @@ -20,6 +33,28 @@ export const formatForeignKeys = (fks: ForeignKeyConstraint[]): ForeignKey[] => }) } +export const normalizeForeignKeyForDirtyCheck = (foreignKey: ForeignKey): ForeignKeyDirtyState => { + return { + id: foreignKey.id, + name: foreignKey.name, + tableId: foreignKey.tableId, + schema: foreignKey.schema, + table: foreignKey.table, + columns: foreignKey.columns.map(({ source, target }) => ({ source, target })), + deletionAction: foreignKey.deletionAction, + updateAction: foreignKey.updateAction, + } +} + +export const hasForeignKeySelectorChanges = ( + initialState: ForeignKeyDirtyState | undefined, + foreignKey: ForeignKey +) => { + if (initialState === undefined) return false + + return !isEqual(initialState, normalizeForeignKeyForDirtyCheck(foreignKey)) +} + export const generateCascadeActionDescription = ( action: 'update' | 'delete', cascadeAction: string,