From 5e9b9d4e4df008aab4987bc57da164766b1aaa24 Mon Sep 17 00:00:00 2001 From: pavish Date: Mon, 30 Sep 2024 13:01:12 +0530 Subject: [PATCH 01/15] Fix duplicate requests for databases.get --- .../pages/database/DatabasePageWrapper.svelte | 2 +- .../database/schemas/SchemasSection.svelte | 3 +-- .../settings/collaborators/Collaborators.svelte | 2 +- .../pages/database/settings/roles/Roles.svelte | 2 +- mathesar_ui/src/stores/AsyncRpcApiStore.ts | 17 ++++++++--------- mathesar_ui/src/stores/AsyncStore.ts | 14 ++++++++++---- 6 files changed, 22 insertions(+), 18 deletions(-) diff --git a/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte b/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte index d238a38a7e..495e1aed6b 100644 --- a/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte +++ b/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte @@ -34,7 +34,7 @@ const databaseRouteContext = DatabaseRouteContext.get(); $: ({ database, underlyingDatabase } = $databaseRouteContext); - $: void underlyingDatabase.run({ database_id: database.id }); + $: void underlyingDatabase.runOptimally({ database_id: database.id }); const commonData = preloadCommonData(); diff --git a/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte b/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte index a71420b6e4..b435544285 100644 --- a/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte +++ b/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte @@ -7,7 +7,6 @@ import { RichText } from '@mathesar/components/rich-text'; import { DatabaseRouteContext } from '@mathesar/contexts/DatabaseRouteContext'; import { iconAddNew } from '@mathesar/icons'; - import type { Database } from '@mathesar/models/Database'; import type { Schema } from '@mathesar/models/Schema'; import { confirmDelete } from '@mathesar/stores/confirmation'; import { modal } from '@mathesar/stores/modal'; @@ -31,7 +30,7 @@ const databaseRouteContext = DatabaseRouteContext.get(); $: ({ database, underlyingDatabase } = $databaseRouteContext); - $: void underlyingDatabase.run({ database_id: database.id }); + $: void underlyingDatabase.runOptimally({ database_id: database.id }); $: schemasMap = $schemasStore.data; $: schemasRequestStatus = $schemasStore.requestStatus; $: isLoading = diff --git a/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte b/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte index 78970e3981..f04ba69e37 100644 --- a/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte +++ b/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte @@ -36,7 +36,7 @@ collaborators.batchRunner({ database_id: database.id }), configuredRoles.batchRunner({ server_id: database.server.id }), ]); - $: void users.runIfNotInitialized(); + $: void users.runOptimally(); $: isLoading = $collaborators.isLoading || $configuredRoles.isLoading || $users.isLoading; $: isSuccess = $collaborators.isOk && $configuredRoles.isOk && $users.isOk; diff --git a/mathesar_ui/src/pages/database/settings/roles/Roles.svelte b/mathesar_ui/src/pages/database/settings/roles/Roles.svelte index 7f7777a5aa..5778618f17 100644 --- a/mathesar_ui/src/pages/database/settings/roles/Roles.svelte +++ b/mathesar_ui/src/pages/database/settings/roles/Roles.svelte @@ -22,7 +22,7 @@ $: ({ database, roles } = $routeContext.databaseRouteContext); - $: void roles.runIfNotInitialized({ database_id: database.id }); + $: void roles.runOptimally({ database_id: database.id }); $: roleList = [...($roles.resolvedValue?.values() ?? [])]; let targetRole: Role | undefined = undefined; diff --git a/mathesar_ui/src/stores/AsyncRpcApiStore.ts b/mathesar_ui/src/stores/AsyncRpcApiStore.ts index b6d482c0c3..b21a999a98 100644 --- a/mathesar_ui/src/stores/AsyncRpcApiStore.ts +++ b/mathesar_ui/src/stores/AsyncRpcApiStore.ts @@ -72,31 +72,30 @@ export default class AsyncRpcApiStore extends AsyncStore< static async runBatched( batchRunners: BatchRunner[], options?: Partial<{ - when?: 'always' | 'not-initialized' | ('always' | 'not-initialized')[]; - onlyRunIfNotInitialized: boolean; + mode?: 'force-run' | 'optimal' | ('force-run' | 'optimal')[]; }>, ) { const toRun = (() => { - if (options?.when && Array.isArray(options.when)) { - if (options.when.length !== batchRunners.length) { + if (options?.mode && Array.isArray(options.mode)) { + if (options.mode.length !== batchRunners.length) { throw new Error( 'Number of run options do not match number of batchRunners', ); } return batchRunners.filter((runner, index) => { - switch (options.when?.[index]) { - case 'always': + switch (options.mode?.[index]) { + case 'force-run': return true; - case 'not-initialized': + case 'optimal': default: return !runner.getValue().hasInitialized; } }); } - if (options?.when === 'always') { + if (options?.mode === 'force-run') { return batchRunners; } - // default is `not-initialized` + // default is `optimal` return batchRunners.filter((runner) => !runner.getValue().hasInitialized); })(); diff --git a/mathesar_ui/src/stores/AsyncStore.ts b/mathesar_ui/src/stores/AsyncStore.ts index 94c46640f8..b6a48a48b3 100644 --- a/mathesar_ui/src/stores/AsyncStore.ts +++ b/mathesar_ui/src/stores/AsyncStore.ts @@ -152,12 +152,18 @@ export default class AsyncStore } } - async runIfNotInitialized(props: Props): Promise> { + /** + * Only runs if the current value is one of + * - not initialized + * - rejected + * - not loading + */ + async runOptimally(props: Props): Promise> { const value = get(this.value); - if (!value.hasInitialized) { - return this.run(props); + if (value.isLoading || value.isOk) { + return value; } - return value; + return this.run(props); } /** From ceb899802f1cbd5b6f3b895763c610e22cf4c853 Mon Sep 17 00:00:00 2001 From: pavish Date: Mon, 30 Sep 2024 13:08:41 +0530 Subject: [PATCH 02/15] Remove caching in schemas store --- .../database/schemas/SchemasSection.svelte | 4 +- mathesar_ui/src/stores/schemas.ts | 179 +++++++----------- 2 files changed, 68 insertions(+), 115 deletions(-) diff --git a/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte b/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte index b435544285..1aa5237f54 100644 --- a/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte +++ b/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte @@ -11,7 +11,7 @@ import { confirmDelete } from '@mathesar/stores/confirmation'; import { modal } from '@mathesar/stores/modal'; import { - type DBSchemaStoreData, + type SchemaStoreData, deleteSchema as deleteSchemaAPI, schemas as schemasStore, } from '@mathesar/stores/schemas'; @@ -43,7 +43,7 @@ let targetSchema: Schema | undefined; function filterSchemas( - schemaData: DBSchemaStoreData['data'], + schemaData: SchemaStoreData['data'], filter: string, ): Schema[] { const filtered: Schema[] = []; diff --git a/mathesar_ui/src/stores/schemas.ts b/mathesar_ui/src/stores/schemas.ts index 25af9a80a4..658035084a 100644 --- a/mathesar_ui/src/stores/schemas.ts +++ b/mathesar_ui/src/stores/schemas.ts @@ -1,6 +1,5 @@ import { type Readable, - type Unsubscriber, type Writable, derived, get, @@ -12,8 +11,9 @@ import { api } from '@mathesar/api/rpc'; import type { RawSchema } from '@mathesar/api/rpc/schemas'; import type { Database } from '@mathesar/models/Database'; import { Schema } from '@mathesar/models/Schema'; +import { getErrorMessage } from '@mathesar/utils/errors'; import { preloadCommonData } from '@mathesar/utils/preloadData'; -import type { CancellablePromise } from '@mathesar-component-library'; +import { type CancellablePromise, collapse } from '@mathesar-component-library'; import { databasesStore } from './databases'; @@ -23,50 +23,41 @@ export const currentSchemaId: Writable = writable( commonData.current_schema ?? undefined, ); -export interface DBSchemaStoreData { +export interface SchemaStoreData { + databaseId?: Database['id']; requestStatus: RequestStatus; data: Map; } -const dbSchemaStoreMap: Map< - Database['id'], - Writable -> = new Map(); -const dbSchemasRequestMap: Map< - Database['id'], - CancellablePromise -> = new Map(); +function makeEmptySchemasData(): SchemaStoreData { + return { + requestStatus: { state: 'success' }, + data: new Map(), + }; +} -function setDBSchemaStore( - database: Database, - rawSchemas: RawSchema[], -): Writable { - const schemaMap: DBSchemaStoreData['data'] = new Map(); +const schemasStore: Writable = writable( + makeEmptySchemasData(), +); + +let request: CancellablePromise; + +function setSchemasInStore(database: Database, rawSchemas: RawSchema[]) { + const schemasMap = new Map(); rawSchemas.forEach((rawSchema) => { - schemaMap.set(rawSchema.oid, new Schema({ database, rawSchema })); + schemasMap.set(rawSchema.oid, new Schema({ database, rawSchema })); }); - const storeValue: DBSchemaStoreData = { + schemasStore.set({ + databaseId: database.id, requestStatus: { state: 'success' }, - data: schemaMap, - }; - - let store = dbSchemaStoreMap.get(database.id); - if (!store) { - store = writable(storeValue); - dbSchemaStoreMap.set(database.id, store); - } else { - store.set(storeValue); - } - return store; + data: schemasMap, + }); } -function updateSchemaInDBSchemaStore( - databaseId: Database['id'], - schema: Schema, -) { - const store = dbSchemaStoreMap.get(databaseId); - if (store) { - store.update((value) => { +function updateSchemaInStore(database: Database, schema: Schema) { + const $schemasStore = get(schemasStore); + if ($schemasStore.databaseId === database.id) { + schemasStore.update((value) => { value.data?.set(schema.oid, schema); return { ...value, @@ -76,10 +67,10 @@ function updateSchemaInDBSchemaStore( } } -function removeSchemaInDBSchemaStore(database: Database, schema: Schema) { - const store = dbSchemaStoreMap.get(database.id); - if (store) { - store.update((value) => { +function removeSchemaInStore(database: Database, schema: Schema) { + const $schemasStore = get(schemasStore); + if ($schemasStore.databaseId === database.id) { + schemasStore.update((value) => { value.data?.delete(schema.oid); return { ...value, @@ -89,64 +80,30 @@ function removeSchemaInDBSchemaStore(database: Database, schema: Schema) { } } -async function refetchSchemasForDB( - database: Database, -): Promise { - const store = dbSchemaStoreMap.get(database.id); - if (!store) { - console.error(`DB Schemas store for database: ${database.id} not found.`); - return undefined; +async function fetchSchemasForCurrentDatabase() { + request?.cancel(); + const $currentDatabase = get(databasesStore.currentDatabase); + if (!$currentDatabase) { + schemasStore.set(makeEmptySchemasData()); + return; } + schemasStore.set({ + databaseId: $currentDatabase.id, + requestStatus: { state: 'processing' }, + data: new Map(), + }); try { - store.update((currentData) => ({ - ...currentData, - requestStatus: { state: 'processing' }, - })); - - dbSchemasRequestMap.get(database.id)?.cancel(); - - const schemaRequest = api.schemas.list({ database_id: database.id }).run(); - dbSchemasRequestMap.set(database.id, schemaRequest); - const schemas = await schemaRequest; - - const dbSchemasStore = setDBSchemaStore(database, schemas); - - return get(dbSchemasStore); + request = api.schemas.list({ database_id: $currentDatabase.id }).run(); + const rawSchemas = await request; + setSchemasInStore($currentDatabase, rawSchemas); } catch (err) { - store.update((currentData) => ({ - ...currentData, - requestStatus: { - state: 'failure', - errors: [ - err instanceof Error ? err.message : 'Error in fetching schemas', - ], - }, - })); - return undefined; - } -} - -let preload = true; - -function getSchemasStoreForDB(database: Database): Writable { - let store = dbSchemaStoreMap.get(database.id); - if (!store) { - store = writable({ - requestStatus: { state: 'processing' }, + schemasStore.set({ + databaseId: $currentDatabase.id, + requestStatus: { state: 'failure', errors: [getErrorMessage(err)] }, data: new Map(), }); - dbSchemaStoreMap.set(database.id, store); - if (preload && commonData.current_database === database.id) { - store = setDBSchemaStore(database, commonData.schemas ?? []); - } else { - void refetchSchemasForDB(database); - } - preload = false; - } else if (get(store).requestStatus.state === 'failure') { - void refetchSchemasForDB(database); } - return store; } export async function createSchema( @@ -163,35 +120,31 @@ export async function createSchema( description: props.description, }) .run(); - updateSchemaInDBSchemaStore(database.id, new Schema({ database, rawSchema })); + updateSchemaInStore(database, new Schema({ database, rawSchema })); } export async function deleteSchema(schema: Schema): Promise { await schema.delete(); - removeSchemaInDBSchemaStore(schema.database, schema); + removeSchemaInStore(schema.database, schema); } -export const schemas: Readable = derived( - databasesStore.currentDatabase, - ($currentDatabase, set) => { - let unsubscribe: Unsubscriber; - - if (!$currentDatabase) { - set({ - requestStatus: { state: 'success' }, - data: new Map(), - }); - } else { - const store = getSchemasStoreForDB($currentDatabase); - unsubscribe = store.subscribe((dbSchemasData) => { - set(dbSchemasData); - }); - } +let preload = true; - return () => { - unsubscribe?.(); - }; - }, +export const schemas = collapse( + derived(databasesStore.currentDatabase, ($currentDatabase) => { + const $schemasStore = get(schemasStore); + if ($schemasStore.databaseId !== $currentDatabase?.id) { + if (preload && commonData.current_database === $currentDatabase?.id) { + setSchemasInStore($currentDatabase, commonData.schemas); + } else { + void fetchSchemasForCurrentDatabase(); + } + preload = false; + } else if ($schemasStore.requestStatus.state === 'failure') { + void fetchSchemasForCurrentDatabase(); + } + return schemasStore; + }), ); export const currentSchema: Readable = derived( From 23de40715c09707bf545e8878a7b6b8580100349 Mon Sep 17 00:00:00 2001 From: pavish Date: Mon, 30 Sep 2024 16:15:05 +0530 Subject: [PATCH 03/15] Remove table list caching for multiple schemas in tables store --- .../database/schemas/SchemasSection.svelte | 3 - .../src/pages/schema/SchemaOverview.svelte | 4 +- mathesar_ui/src/stores/tables.ts | 277 ++++++++---------- .../link-table/LinkTableForm.svelte | 4 +- 4 files changed, 130 insertions(+), 158 deletions(-) diff --git a/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte b/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte index 1aa5237f54..1f4907a5c2 100644 --- a/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte +++ b/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte @@ -15,7 +15,6 @@ deleteSchema as deleteSchemaAPI, schemas as schemasStore, } from '@mathesar/stores/schemas'; - import { removeTablesStore } from '@mathesar/stores/tables'; import AddEditSchemaModal from '@mathesar/systems/schemas/AddEditSchemaModal.svelte'; import { Button, @@ -74,8 +73,6 @@ body: [$_('schema_delete_warning'), $_('are_you_sure_to_proceed')], onProceed: async () => { await deleteSchemaAPI(schema); - // TODO: Create common util to handle data clearing & sync between stores - removeTablesStore(schema); }, }); } diff --git a/mathesar_ui/src/pages/schema/SchemaOverview.svelte b/mathesar_ui/src/pages/schema/SchemaOverview.svelte index c33ad6abe7..593aa8b424 100644 --- a/mathesar_ui/src/pages/schema/SchemaOverview.svelte +++ b/mathesar_ui/src/pages/schema/SchemaOverview.svelte @@ -11,7 +11,7 @@ import type { Table } from '@mathesar/models/Table'; import { getDataExplorerPageUrl } from '@mathesar/routes/urls'; import { refetchExplorationsForSchema } from '@mathesar/stores/queries'; - import { refetchTablesForSchema } from '@mathesar/stores/tables'; + import { fetchTablesForCurrentSchema } from '@mathesar/stores/tables'; import { AnchorButton, Button } from '@mathesar-component-library'; import CreateNewExplorationTutorial from './CreateNewExplorationTutorial.svelte'; @@ -59,7 +59,7 @@
{ - await refetchTablesForSchema(schema); + await fetchTablesForCurrentSchema(); }} label={$_('retry')} icon={iconRefresh} diff --git a/mathesar_ui/src/stores/tables.ts b/mathesar_ui/src/stores/tables.ts index 6d834bc03b..a19b56e513 100644 --- a/mathesar_ui/src/stores/tables.ts +++ b/mathesar_ui/src/stores/tables.ts @@ -10,14 +10,7 @@ */ import { execPipe, filter, map } from 'iter-tools'; -import { - type Readable, - type Writable, - derived, - get, - readable, - writable, -} from 'svelte/store'; +import { type Readable, derived, get, writable } from 'svelte/store'; import { _ } from 'svelte-i18n'; import type { DataFile } from '@mathesar/api/rest/types/dataFiles'; @@ -33,7 +26,7 @@ import { type RpcRequest, batchSend, } from '@mathesar/packages/json-rpc-client-builder'; -import { TupleMap } from '@mathesar/packages/tuple-map'; +import { getErrorMessage } from '@mathesar/utils/errors'; import { preloadCommonData } from '@mathesar/utils/preloadData'; import { tableRequiresImportConfirmation } from '@mathesar/utils/tables'; import { @@ -49,6 +42,8 @@ const commonData = preloadCommonData(); type TablesMap = Map; interface TablesData { + databaseId?: Database['id']; + schemaOid?: Schema['oid']; tablesMap: TablesMap; requestStatus: RequestStatus; } @@ -60,136 +55,100 @@ function makeEmptyTablesData(): TablesData { }; } -type TablesStore = Writable; - -/** Maps [databaseId, schemaOid] to TablesStore */ -const tablesStores = new TupleMap< - [Database['id'], Schema['oid']], - TablesStore ->(); - -const tablesRequests = new TupleMap< - [Database['id'], Schema['oid']], - CancellablePromise ->(); +const tablesStore = writable(makeEmptyTablesData()); function sortTables(tables: Iterable): Table[] { return [...tables].sort((a, b) => a.name.localeCompare(b.name)); } -function setTablesStore(schema: Schema, tables: Table[]): TablesStore { +function setTablesStore( + schema: Schema, + rawTablesWithMetadata: RawTableWithMetadata[], +) { + const tables = rawTablesWithMetadata.map( + (t) => + new Table({ + schema, + rawTableWithMetadata: t, + }), + ); + const tablesMap: TablesMap = new Map(); - if (tables) { - sortTables(tables).forEach((t) => tablesMap.set(t.oid, t)); - } + sortTables(tables).forEach((t) => tablesMap.set(t.oid, t)); - const storeValue: TablesData = { + tablesStore.set({ + databaseId: schema.database.id, + schemaOid: schema.oid, tablesMap, requestStatus: { state: 'success' }, - }; - - let store = tablesStores.get([schema.database.id, schema.oid]); - if (!store) { - store = writable(storeValue); - tablesStores.set([schema.database.id, schema.oid], store); - } else { - store.set(storeValue); - } - return store; + }); } -export function removeTablesStore(schema: Schema): void { - tablesStores.delete([schema.database.id, schema.oid]); -} +let request: CancellablePromise; -export async function refetchTablesForSchema( - schema: Schema, -): Promise { - const store = tablesStores.get([schema.database.id, schema.oid]); - if (!store) { - // TODO: why are we logging an error here? I would expect that we'd either - // throw or ignore. If there's a reason for this logging, please add a code - // comment explaining why. - console.error('Tables store not found.'); - return undefined; +export async function fetchTablesForCurrentSchema() { + request?.cancel(); + + const $currentSchema = get(currentSchema); + if (!$currentSchema) { + tablesStore.set(makeEmptyTablesData()); + return; } try { - store.update((currentData) => ({ - ...currentData, - requestStatus: { state: 'processing' }, - })); - - tablesRequests.get([schema.database.id, schema.oid])?.cancel(); + tablesStore.update(($tablesStore) => { + if ( + $tablesStore.databaseId === $currentSchema.database.id && + $tablesStore.schemaOid === $currentSchema.oid + ) { + return { + ...$tablesStore, + requestStatus: { state: 'processing' }, + }; + } + return { + databaseId: $currentSchema.database.id, + schemaOid: $currentSchema.oid, + tablesMap: new Map(), + requestStatus: { state: 'processing' }, + }; + }); - const tablesRequest = api.tables + request = api.tables .list_with_metadata({ - database_id: schema.database.id, - schema_oid: schema.oid, + database_id: $currentSchema.database.id, + schema_oid: $currentSchema.oid, }) .run(); - tablesRequests.set([schema.database.id, schema.oid], tablesRequest); - const tableEntries = await tablesRequest; - - const schemaTablesStore = setTablesStore( - schema, - tableEntries.map( - (t) => - new Table({ - schema, - rawTableWithMetadata: t, - }), - ), - ); - - return get(schemaTablesStore); - } catch (err) { - store.update((currentData) => ({ - ...currentData, - requestStatus: { - state: 'failure', - errors: [ - err instanceof Error ? err.message : 'Error in fetching schemas', - ], - }, - })); - return undefined; - } -} -let preload = true; + const tableEntries = await request; -function getTablesStore(schema: Schema): TablesStore { - let store = tablesStores.get([schema.database.id, schema.oid]); - if (!store) { - store = writable({ - requestStatus: { state: 'processing' }, - tablesMap: new Map(), + setTablesStore($currentSchema, tableEntries); + } catch (err) { + tablesStore.update(($tablesStore) => { + if ( + $tablesStore.databaseId === $currentSchema.database.id && + $tablesStore.schemaOid === $currentSchema.oid + ) { + return { + ...$tablesStore, + requestStatus: { + state: 'failure', + errors: [getErrorMessage(err)], + }, + }; + } + return { + databaseId: $currentSchema.database.id, + schemaOid: $currentSchema.oid, + tablesMap: new Map(), + requestStatus: { + state: 'failure', + errors: [getErrorMessage(err)], + }, + }; }); - tablesStores.set([schema.database.id, schema.oid], store); - if ( - preload && - commonData.current_schema === schema.oid && - commonData.current_database === schema.database.id - ) { - store = setTablesStore( - schema, - commonData.tables.map( - (t) => - new Table({ - schema, - rawTableWithMetadata: t, - }), - ), - ); - } else { - void refetchTablesForSchema(schema); - } - preload = false; - } else if (get(store).requestStatus.state === 'failure') { - void refetchTablesForSchema(schema); } - return store; } export function deleteTable( @@ -206,15 +165,19 @@ export function deleteTable( (resolve, reject) => { void promise.then(() => { schema.setTableCount(get(schema.tableCount) - 1); - tablesStores - .get([schema.database.id, schema.oid]) - ?.update((tableStoreData) => { + const $tablesStore = get(tablesStore); + if ( + $tablesStore.databaseId === schema.database.id && + $tablesStore.schemaOid === schema.oid + ) { + tablesStore.update((tableStoreData) => { tableStoreData.tablesMap.delete(tableOid); return { ...tableStoreData, tablesMap: new Map(tableStoreData.tablesMap), }; }); + } return resolve(); }, reject); }, @@ -224,7 +187,7 @@ export function deleteTable( ); } -function updateTableStoreIfPresent({ +function putTableInStore({ schema, rawTableWithMetadata, }: { @@ -235,8 +198,11 @@ function updateTableStoreIfPresent({ schema, rawTableWithMetadata, }); - const tablesStore = tablesStores.get([schema.database.id, schema.oid]); - if (tablesStore) { + const $tablesStore = get(tablesStore); + if ( + $tablesStore.databaseId === schema.database.id && + $tablesStore.schemaOid === schema.oid + ) { tablesStore.update(({ requestStatus, tablesMap }) => { tablesMap.set(fullTable.oid, fullTable); const newTablesMap = new Map( @@ -314,7 +280,7 @@ export async function updateTable({ table_oid: rawPartialTable.oid, }) .run(); - return updateTableStoreIfPresent({ schema, rawTableWithMetadata }); + return putTableInStore({ schema, rawTableWithMetadata }); } export async function createTable({ @@ -338,7 +304,7 @@ export async function createTable({ .run(); schema.setTableCount(get(schema.tableCount) + 1); - return updateTableStoreIfPresent({ schema, rawTableWithMetadata }); + return putTableInStore({ schema, rawTableWithMetadata }); } export async function createTableFromDataFile(props: { @@ -378,39 +344,34 @@ export function getTableFromStoreOrApi({ schema: Schema; tableOid: Table['oid']; }): CancellablePromise
{ - const tablesStore = tablesStores.get([schema.database.id, schema.oid]); - if (tablesStore) { - const table = get(tablesStore).tablesMap.get(tableOid); + const $tablesStore = get(tablesStore); + + if ( + $tablesStore.databaseId === schema.database.id && + $tablesStore.schemaOid === schema.oid + ) { + const table = $tablesStore.tablesMap.get(tableOid); if (table) { return new CancellablePromise((resolve) => { resolve(table); }); } } + const promise = api.tables .get_with_metadata({ database_id: schema.database.id, table_oid: tableOid, }) .run(); + return new CancellablePromise( (resolve, reject) => { void promise.then((rawTableWithMetadata) => { - const table = new Table({ schema, rawTableWithMetadata }); - const store = tablesStores.get([schema.database.id, schema.oid]); - if (store) { - store.update((existing) => { - const tableMap = new Map(); - const tables = [...existing.tablesMap.values(), table]; - sortTables(tables).forEach((t) => { - tableMap.set(t.oid, t); - }); - return { - ...existing, - tablesMap: tableMap, - }; - }); - } + const table = putTableInStore({ + schema, + rawTableWithMetadata, + }); return resolve(table); }, reject); }, @@ -420,10 +381,30 @@ export function getTableFromStoreOrApi({ ); } +let preload = true; + export const currentTablesData = collapse( - derived([currentSchema], ([schema]) => - !schema ? readable(makeEmptyTablesData()) : getTablesStore(schema), - ), + derived(currentSchema, ($currentSchema) => { + const $tablesStore = get(tablesStore); + if ( + $tablesStore.databaseId !== $currentSchema?.database.id || + $tablesStore.schemaOid !== $currentSchema?.oid + ) { + if ( + preload && + commonData.current_schema === $currentSchema?.oid && + commonData.current_database === $currentSchema?.database.id + ) { + setTablesStore($currentSchema, commonData.tables); + } else { + void fetchTablesForCurrentSchema(); + } + preload = false; + } else if ($tablesStore.requestStatus.state === 'failure') { + void fetchTablesForCurrentSchema(); + } + return tablesStore; + }), ); export const currentTablesMap = derived( @@ -466,12 +447,6 @@ export const currentTable = derived( export function factoryToGetTableNameValidationErrors( table: Table, ): Readable<(n: string) => string[]> { - const tablesStore = tablesStores.get([ - table.schema.database.id, - table.schema.oid, - ]); - if (!tablesStore) throw new Error('Tables store not found'); - const otherTableNames = derived( tablesStore, (d) => diff --git a/mathesar_ui/src/systems/table-view/link-table/LinkTableForm.svelte b/mathesar_ui/src/systems/table-view/link-table/LinkTableForm.svelte index 7f107efb1d..5a5d15d98c 100644 --- a/mathesar_ui/src/systems/table-view/link-table/LinkTableForm.svelte +++ b/mathesar_ui/src/systems/table-view/link-table/LinkTableForm.svelte @@ -23,8 +23,8 @@ } from '@mathesar/stores/table-data'; import { currentTables, + fetchTablesForCurrentSchema, importVerifiedTables as importVerifiedTablesStore, - refetchTablesForSchema, validateNewTableName, } from '@mathesar/stores/tables'; import { toast } from '@mathesar/stores/toast'; @@ -161,7 +161,7 @@ async function reFetchOtherThingsThatChanged() { if ($linkType === 'manyToMany') { - await refetchTablesForSchema(base.schema); + await fetchTablesForCurrentSchema(); return; } const tableWithNewColumn = $linkType === 'oneToMany' ? target : base; From bc7863bf5cf70cccf6abfd90a2c1a0d3b3896d62 Mon Sep 17 00:00:00 2001 From: pavish Date: Mon, 30 Sep 2024 17:26:03 +0530 Subject: [PATCH 04/15] Fix: Re-add accidentally removed translation from https://github.com/mathesar-foundation/mathesar/commit/a66873b866a2ec34125a204bf86e3090c1fbf191#r147380449 --- mathesar_ui/src/i18n/languages/en/dict.json | 1 + 1 file changed, 1 insertion(+) diff --git a/mathesar_ui/src/i18n/languages/en/dict.json b/mathesar_ui/src/i18n/languages/en/dict.json index 49faeebfb6..4fccfa95bb 100644 --- a/mathesar_ui/src/i18n/languages/en/dict.json +++ b/mathesar_ui/src/i18n/languages/en/dict.json @@ -531,6 +531,7 @@ "select_columns_cells_view_properties": "Select one or more columns or cells to view the associated column properties and actions.", "select_columns_extract": "Select the columns you want to extract", "select_columns_extract_into_table": "Select the columns you want to extract into [targetTableName]", + "select_columns_for_exploration_help": "Select the columns that will be used for the exploration. Columns are limited to those from the base table and its linked tables.", "select_columns_move": "Select the columns you want to move", "select_columns_move_into_table": "Select the columns you want to move into [targetTableName]", "select_columns_to_hide": "Select Columns to Hide", From dfb8336168706b35094450c3ac7a40276069b68e Mon Sep 17 00:00:00 2001 From: pavish Date: Mon, 30 Sep 2024 17:27:40 +0530 Subject: [PATCH 05/15] Fix: Handle None case while providing explorations list in common data, remove abstract_types from common data --- mathesar/views.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/mathesar/views.py b/mathesar/views.py index 7d5c140955..be4246eb0d 100644 --- a/mathesar/views.py +++ b/mathesar/views.py @@ -45,19 +45,15 @@ def get_table_list(request, database_id, schema_oid): return [] -def get_queries_list(request, database_id, schema_id): - return explorations_list(request=request, database_id=database_id, schema_oid=schema_id) - - -def get_ui_type_list(request, database_id): - if database_id is None: +def get_queries_list(request, database_id, schema_oid): + if database_id is not None and schema_oid is not None: + return explorations_list( + request=request, + database_id=database_id, + schema_oid=schema_oid + ) + else: return [] - type_serializer = TypeSerializer( - UIType, - many=True, - context={'request': request} - ) - return type_serializer.data def get_user_data(request): @@ -84,7 +80,6 @@ def _get_internal_db_meta(): def _get_base_data_all_routes(request, database_id=None, schema_id=None): return { - 'abstract_types': get_ui_type_list(request, database_id), 'current_database': int(database_id) if database_id else None, 'current_schema': int(schema_id) if schema_id else None, 'current_release_tag_name': __version__, From 63a3980e25b171e6132c9f9b08b8e840daca882c Mon Sep 17 00:00:00 2001 From: pavish Date: Mon, 30 Sep 2024 17:36:45 +0530 Subject: [PATCH 06/15] Only cache explorations of current schema --- .../src/pages/schema/SchemaOverview.svelte | 4 +- mathesar_ui/src/stores/queries.ts | 275 +++++++++--------- 2 files changed, 133 insertions(+), 146 deletions(-) diff --git a/mathesar_ui/src/pages/schema/SchemaOverview.svelte b/mathesar_ui/src/pages/schema/SchemaOverview.svelte index 593aa8b424..f38ef5e373 100644 --- a/mathesar_ui/src/pages/schema/SchemaOverview.svelte +++ b/mathesar_ui/src/pages/schema/SchemaOverview.svelte @@ -10,7 +10,7 @@ import type { Schema } from '@mathesar/models/Schema'; import type { Table } from '@mathesar/models/Table'; import { getDataExplorerPageUrl } from '@mathesar/routes/urls'; - import { refetchExplorationsForSchema } from '@mathesar/stores/queries'; + import { fetchExplorationsForCurrentSchema } from '@mathesar/stores/queries'; import { fetchTablesForCurrentSchema } from '@mathesar/stores/tables'; import { AnchorButton, Button } from '@mathesar-component-library'; @@ -88,7 +88,7 @@
{ - await refetchExplorationsForSchema(schema); + await fetchExplorationsForCurrentSchema(); }} label={$_('retry')} icon={iconRefresh} diff --git a/mathesar_ui/src/stores/queries.ts b/mathesar_ui/src/stores/queries.ts index 65ed5910a3..1b21e692cf 100644 --- a/mathesar_ui/src/stores/queries.ts +++ b/mathesar_ui/src/stores/queries.ts @@ -32,14 +32,7 @@ * to having us use the currentSchemaId store directly. */ -import { - type Readable, - type Unsubscriber, - type Writable, - derived, - get, - writable, -} from 'svelte/store'; +import { derived, get, writable } from 'svelte/store'; import type { RequestStatus } from '@mathesar/api/rest/utils/requestUtils'; import { api } from '@mathesar/api/rpc'; @@ -48,31 +41,31 @@ import type { ExplorationResult, SavedExploration, } from '@mathesar/api/rpc/explorations'; +import type { Database } from '@mathesar/models/Database'; import type { Schema } from '@mathesar/models/Schema'; -import CacheManager from '@mathesar/utils/CacheManager'; +import { getErrorMessage } from '@mathesar/utils/errors'; import { preloadCommonData } from '@mathesar/utils/preloadData'; -import type { CancellablePromise } from '@mathesar-component-library'; +import { type CancellablePromise, collapse } from '@mathesar-component-library'; import { currentSchema } from './schemas'; const commonData = preloadCommonData(); export interface QueriesStoreSubstance { - schemaId: Schema['oid']; + databaseId?: Database['id']; + schemaOid?: Schema['oid']; requestStatus: RequestStatus; data: Map; } -// Cache the query list of the last 3 opened schemas -const schemasCacheManager = new CacheManager< - Schema['oid'], - Writable ->(3); +function makeEmptyQueriesStoreSubstance(): QueriesStoreSubstance { + return { + data: new Map(), + requestStatus: { state: 'success' }, + }; +} -const requestMap: Map< - Schema['oid'], - CancellablePromise -> = new Map(); +const queriesStore = writable(makeEmptyQueriesStoreSubstance()); function sortedQueryEntries( queryEntries: SavedExploration[], @@ -80,139 +73,110 @@ function sortedQueryEntries( return [...queryEntries].sort((a, b) => a.name?.localeCompare(b.name)); } -function setSchemaQueriesStore( - schemaId: Schema['oid'], - queryEntries?: SavedExploration[], -): Writable { +function setExplorationsStore( + schema: Schema, + queryEntries: SavedExploration[], +) { const queries: QueriesStoreSubstance['data'] = new Map(); - if (queryEntries) { - sortedQueryEntries(queryEntries).forEach((entry) => { - queries.set(entry.id, entry); - }); - } + sortedQueryEntries(queryEntries).forEach((entry) => { + queries.set(entry.id, entry); + }); - const storeValue: QueriesStoreSubstance = { - schemaId, - requestStatus: { state: 'success' }, + queriesStore.set({ + databaseId: schema.database.id, + schemaOid: schema.oid, data: queries, - }; - - let store = schemasCacheManager.get(schemaId); - if (!store) { - store = writable(storeValue); - schemasCacheManager.set(schemaId, store); - } else { - store.set(storeValue); - } - return store; + requestStatus: { state: 'success' }, + }); } -function findSchemaStoreForQuery(id: SavedExploration['id']) { - return [...schemasCacheManager.cache.values()].find((entry) => - get(entry).data.has(id), - ); -} +let request: CancellablePromise; -export async function refetchExplorationsForSchema(schema: { - oid: number; - database: { id: number }; -}): Promise { - const store = schemasCacheManager.get(schema.oid); - if (!store) { - console.error(`Queries store for schema: ${schema.oid} not found.`); - return undefined; +export async function fetchExplorationsForCurrentSchema(): Promise { + request?.cancel(); + + const $currentSchema = get(currentSchema); + if (!$currentSchema) { + queriesStore.set(makeEmptyQueriesStoreSubstance()); + return; } try { - store.update((currentData) => ({ - ...currentData, - requestStatus: { state: 'processing' }, - })); - - requestMap.get(schema.oid)?.cancel(); + queriesStore.update(($queriesStore) => { + if ( + $queriesStore.databaseId === $currentSchema.database.id && + $queriesStore.schemaOid === $currentSchema.oid + ) { + return { + ...$queriesStore, + requestStatus: { state: 'processing' }, + }; + } + return { + databaseId: $currentSchema.database.id, + schemaOid: $currentSchema.oid, + data: new Map(), + requestStatus: { state: 'processing' }, + }; + }); - const queriesRequest = api.explorations + request = api.explorations .list({ - database_id: schema.database.id, - schema_oid: schema.oid, + database_id: $currentSchema.database.id, + schema_oid: $currentSchema.oid, }) .run(); - requestMap.set(schema.oid, queriesRequest); - - const queriesResult = await queriesRequest; - const schemaQueriesStore = setSchemaQueriesStore(schema.oid, queriesResult); - return get(schemaQueriesStore); + const queriesResult = await request; + setExplorationsStore($currentSchema, queriesResult); } catch (err) { - store.update((currentData) => ({ - ...currentData, - requestStatus: { - state: 'failure', - errors: [ - err instanceof Error ? err.message : 'Error in fetching schemas', - ], - }, - })); - return undefined; + queriesStore.update(($queriesStore) => { + if ( + $queriesStore.databaseId === $currentSchema.database.id && + $queriesStore.schemaOid === $currentSchema.oid + ) { + return { + ...$queriesStore, + requestStatus: { + state: 'failure', + errors: [getErrorMessage(err)], + }, + }; + } + return { + databaseId: $currentSchema.database.id, + schemaOid: $currentSchema.oid, + data: new Map(), + requestStatus: { + state: 'failure', + errors: [getErrorMessage(err)], + }, + }; + }); } } -let preload = true; +function putExplorationInStore(exploration: SavedExploration) { + const $currentSchema = get(currentSchema); + if (!$currentSchema) { + return; + } -export function getExplorationsStoreForSchema(schema: { - oid: number; - database: { id: number }; -}): Writable { - let store = schemasCacheManager.get(schema.oid); - if (!store) { - store = writable({ - schemaId: schema.oid, - requestStatus: { state: 'processing' }, - data: new Map(), - }); - schemasCacheManager.set(schema.oid, store); - if (preload && commonData.current_schema === schema.oid) { - store = setSchemaQueriesStore(schema.oid, commonData.queries ?? []); - } else { - void refetchExplorationsForSchema(schema); - } - preload = false; - } else if (get(store).requestStatus.state === 'failure') { - void refetchExplorationsForSchema(schema); + if ( + exploration.database_id === $currentSchema.database.id && + exploration.schema_oid === $currentSchema.oid + ) { + const queryData = get(queriesStore).data.set(exploration.id, exploration); + setExplorationsStore($currentSchema, [...queryData.values()]); } - return store; } -export const queries: Readable> = - derived(currentSchema, ($currentSchema, set) => { - let unsubscribe: Unsubscriber; - - if (!$currentSchema) { - set({ - requestStatus: { state: 'success' }, - data: new Map(), - }); - } else { - const store = getExplorationsStoreForSchema($currentSchema); - unsubscribe = store.subscribe((dbSchemasData) => { - set(dbSchemasData); - }); - } - - return () => { - unsubscribe?.(); - }; - }); - export function addExploration( exploration: AddableExploration, ): CancellablePromise { const promise = api.explorations.add({ exploration_def: exploration }).run(); void promise.then((savedExploration) => { - void refetchExplorationsForSchema({ - oid: savedExploration.schema_oid, - database: { id: exploration.database_id }, - }); + putExplorationInStore(savedExploration); return savedExploration; }); return promise; @@ -226,14 +190,8 @@ export function replaceExploration( .run(); void promise.then((newlySavedExploration) => { - const schemaId = newlySavedExploration.schema_oid; - const store = getExplorationsStoreForSchema({ - oid: schemaId, - database: { id: newlySavedExploration.database_id }, - }); - get(store).data.set(exploration.id, newlySavedExploration); - setSchemaQueriesStore(schemaId, [...get(store).data.values()]); - return undefined; + putExplorationInStore(newlySavedExploration); + return newlySavedExploration; }); return promise; @@ -264,15 +222,44 @@ export function runSavedExploration( export function deleteExploration(id: number): CancellablePromise { const promise = api.explorations.delete({ exploration_id: id }).run(); - void promise.then(() => { - const store = findSchemaStoreForQuery(id); - if (store) { - store.update((storeData) => { - storeData.data.delete(id); - return { ...storeData, data: new Map(storeData.data) }; - }); - } - return undefined; - }); + const $queriesStore = get(queriesStore); + const $currentSchema = get(currentSchema); + if ( + $queriesStore.databaseId === $currentSchema?.database.id && + $queriesStore.schemaOid === $currentSchema?.oid + ) { + $queriesStore.data.delete(id); + queriesStore.update((store) => ({ + ...store, + data: new Map($queriesStore.data), + })); + } + return promise; } + +let preload = true; + +export const queries = collapse( + derived(currentSchema, ($currentSchema) => { + const $queriesStore = get(queriesStore); + if ( + $queriesStore.databaseId !== $currentSchema?.database.id || + $queriesStore.schemaOid !== $currentSchema?.oid + ) { + if ( + preload && + commonData.current_schema === $currentSchema?.oid && + commonData.current_database === $currentSchema?.database.id + ) { + setExplorationsStore($currentSchema, commonData.queries); + } else { + void fetchExplorationsForCurrentSchema(); + } + preload = false; + } else if ($queriesStore.requestStatus.state === 'failure') { + void fetchExplorationsForCurrentSchema(); + } + return queriesStore; + }), +); From 800dae7a603f2bc23a811859c7c58be77768998f Mon Sep 17 00:00:00 2001 From: pavish Date: Mon, 30 Sep 2024 17:37:22 +0530 Subject: [PATCH 07/15] Hide count in schema page when tables & explorations are being loaded --- mathesar_ui/src/pages/schema/SchemaPage.svelte | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mathesar_ui/src/pages/schema/SchemaPage.svelte b/mathesar_ui/src/pages/schema/SchemaPage.svelte index 4106eec79b..3f5b07b4cd 100644 --- a/mathesar_ui/src/pages/schema/SchemaPage.svelte +++ b/mathesar_ui/src/pages/schema/SchemaPage.svelte @@ -56,12 +56,14 @@ { label: $_('tables'), id: 'tables', + showCount: tablesRequestStatus.state === 'success', count: tablesMap.size, href: getSchemaPageTablesSectionUrl(database.id, schema.oid), }, { label: $_('explorations'), id: 'explorations', + showCount: explorationsRequestStatus.state === 'success', count: explorationsMap.size, href: getSchemaPageExplorationsSectionUrl(database.id, schema.oid), }, @@ -125,7 +127,7 @@
{tab.label} - {#if tab.count !== undefined} + {#if tab.count !== undefined && tab.showCount} {tab.count} {/if}
From 901c4612ed58179ea89c07727222fd3ae3c41f2e Mon Sep 17 00:00:00 2001 From: pavish Date: Mon, 30 Sep 2024 17:39:46 +0530 Subject: [PATCH 08/15] Fix: Translate toast message when role is created --- .../src/pages/database/settings/roles/CreateRoleModal.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mathesar_ui/src/pages/database/settings/roles/CreateRoleModal.svelte b/mathesar_ui/src/pages/database/settings/roles/CreateRoleModal.svelte index a2637b057b..386a494c4d 100644 --- a/mathesar_ui/src/pages/database/settings/roles/CreateRoleModal.svelte +++ b/mathesar_ui/src/pages/database/settings/roles/CreateRoleModal.svelte @@ -60,7 +60,7 @@ login: false, }); } - toast.success('role_created_successfully'); + toast.success($_('role_created_successfully')); controller.close(); } From c621e9071a228134beb74c75b3925b828818672b Mon Sep 17 00:00:00 2001 From: pavish Date: Mon, 30 Sep 2024 18:38:06 +0530 Subject: [PATCH 09/15] Handle side effects when collaborator changes for currently logged in admin user --- mathesar_ui/src/i18n/languages/en/dict.json | 1 + .../collaborators/CollaboratorRow.svelte | 10 +++++- .../collaborators/Collaborators.svelte | 33 +++++++++++++++-- .../EditRoleForCollaboratorModal.svelte | 2 ++ mathesar_ui/src/stores/schemas.ts | 35 ++++++++++++++----- 5 files changed, 69 insertions(+), 12 deletions(-) diff --git a/mathesar_ui/src/i18n/languages/en/dict.json b/mathesar_ui/src/i18n/languages/en/dict.json index 4fccfa95bb..f1429c3cf2 100644 --- a/mathesar_ui/src/i18n/languages/en/dict.json +++ b/mathesar_ui/src/i18n/languages/en/dict.json @@ -66,6 +66,7 @@ "click_save_button_to_save_changes": "Click Save button to save changes.", "collaborator": "Collaborator", "collaborator_added_successfully": "Collaborator added successfully", + "collaborator_removed_successfully": "Collaborator removed successfully", "collaborator_role_help": "This role determines permissions for this collaborator. Mathesar will use this PostgreSQL role for all database interactions by this user.", "collaborator_role_updated_successfully": "Role updated successfully for collaborator.", "collaborators": "Collaborators", diff --git a/mathesar_ui/src/pages/database/settings/collaborators/CollaboratorRow.svelte b/mathesar_ui/src/pages/database/settings/collaborators/CollaboratorRow.svelte index f5048b68ee..810caa0d14 100644 --- a/mathesar_ui/src/pages/database/settings/collaborators/CollaboratorRow.svelte +++ b/mathesar_ui/src/pages/database/settings/collaborators/CollaboratorRow.svelte @@ -7,11 +7,13 @@ import { iconDeleteMajor, iconEdit } from '@mathesar/icons'; import type { Collaborator } from '@mathesar/models/Collaborator'; import { confirmDelete } from '@mathesar/stores/confirmation'; + import { toast } from '@mathesar/stores/toast'; import { getUserProfileStoreFromContext } from '@mathesar/stores/userProfile'; import { Button, SpinnerButton } from '@mathesar-component-library'; export let collaborator: Collaborator; export let editRoleForCollaborator: (collaborator: Collaborator) => void; + export let checkAndHandleSideEffects: (collaboror: Collaborator) => void; const routeContext = DatabaseSettingsRouteContext.get(); $: ({ configuredRoles, users } = $routeContext); @@ -23,6 +25,12 @@ $: configuredRoleId = collaborator.configuredRoleId; $: configuredRole = $configuredRoles.resolvedValue?.get($configuredRoleId); $: userName = user ? user.full_name || user.username : ''; + + async function deleteCollaborator() { + await $routeContext.deleteCollaborator(collaborator); + checkAndHandleSideEffects(collaborator); + toast.success($_('collaborator_removed_successfully')); + } @@ -62,7 +70,7 @@ identifierName: userName, identifierType: $_('collaborator'), })} - onClick={() => $routeContext.deleteCollaborator(collaborator)} + onClick={deleteCollaborator} icon={{ ...iconDeleteMajor, size: '0.8em' }} label="" appearance="secondary" diff --git a/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte b/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte index f04ba69e37..e0a92134a2 100644 --- a/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte +++ b/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte @@ -10,6 +10,7 @@ import type { Collaborator } from '@mathesar/models/Collaborator'; import AsyncRpcApiStore from '@mathesar/stores/AsyncRpcApiStore'; import { modal } from '@mathesar/stores/modal'; + import { fetchSchemasForCurrentDatabase } from '@mathesar/stores/schemas'; import { getUserProfileStoreFromContext } from '@mathesar/stores/userProfile'; import { Button, @@ -30,7 +31,13 @@ const userProfileStore = getUserProfileStoreFromContext(); $: ({ isMathesarAdmin } = $userProfileStore); - $: ({ database, configuredRoles, collaborators, users } = $routeContext); + $: ({ + database, + configuredRoles, + collaborators, + users, + databaseRouteContext, + } = $routeContext); $: void AsyncRpcApiStore.runBatched([ collaborators.batchRunner({ database_id: database.id }), @@ -52,6 +59,23 @@ targetCollaborator = collaborator; editCollaboratorRoleModal.open(); } + + function checkAndHandleSideEffects(collaborator: Collaborator) { + if (collaborator.userId === $userProfileStore.id) { + void AsyncRpcApiStore.runBatched( + [ + databaseRouteContext.underlyingDatabase.batchRunner({ + database_id: database.id, + }), + databaseRouteContext.roles.batchRunner({ database_id: database.id }), + ], + { + mode: 'force-run', + }, + ); + void fetchSchemasForCurrentDatabase(); + } + } @@ -79,7 +103,11 @@ {$_('role')} {$_('actions')} {#each [...($collaborators.resolvedValue?.values() ?? [])] as collaborator (collaborator.id)} - + {/each}
@@ -103,6 +131,7 @@ usersMap={$users.resolvedValue} controller={editCollaboratorRoleModal} configuredRolesMap={$configuredRoles.resolvedValue} + {checkAndHandleSideEffects} /> {/if} diff --git a/mathesar_ui/src/pages/database/settings/collaborators/EditRoleForCollaboratorModal.svelte b/mathesar_ui/src/pages/database/settings/collaborators/EditRoleForCollaboratorModal.svelte index 77f1eb1155..44ebac328a 100644 --- a/mathesar_ui/src/pages/database/settings/collaborators/EditRoleForCollaboratorModal.svelte +++ b/mathesar_ui/src/pages/database/settings/collaborators/EditRoleForCollaboratorModal.svelte @@ -23,6 +23,7 @@ export let collaborator: Collaborator; export let configuredRolesMap: ImmutableMap; export let usersMap: ImmutableMap; + export let checkAndHandleSideEffects: (collaboror: Collaborator) => void; $: savedConfiguredRoleId = collaborator.configuredRoleId; $: configuredRoleId = requiredField($savedConfiguredRoleId); @@ -33,6 +34,7 @@ async function updateRoleForCollaborator() { await collaborator.setConfiguredRole($configuredRoleId); + checkAndHandleSideEffects(collaborator); controller.close(); toast.success($_('collaborator_role_updated_successfully')); } diff --git a/mathesar_ui/src/stores/schemas.ts b/mathesar_ui/src/stores/schemas.ts index 658035084a..493ff7e5da 100644 --- a/mathesar_ui/src/stores/schemas.ts +++ b/mathesar_ui/src/stores/schemas.ts @@ -80,7 +80,7 @@ function removeSchemaInStore(database: Database, schema: Schema) { } } -async function fetchSchemasForCurrentDatabase() { +export async function fetchSchemasForCurrentDatabase() { request?.cancel(); const $currentDatabase = get(databasesStore.currentDatabase); if (!$currentDatabase) { @@ -88,20 +88,37 @@ async function fetchSchemasForCurrentDatabase() { return; } - schemasStore.set({ - databaseId: $currentDatabase.id, - requestStatus: { state: 'processing' }, - data: new Map(), + schemasStore.update(($schemasStore) => { + if ($schemasStore.databaseId === $currentDatabase.id) { + return { + ...$schemasStore, + requestStatus: { state: 'processing' }, + }; + } + return { + databaseId: $currentDatabase.id, + requestStatus: { state: 'processing' }, + data: new Map(), + }; }); + try { request = api.schemas.list({ database_id: $currentDatabase.id }).run(); const rawSchemas = await request; setSchemasInStore($currentDatabase, rawSchemas); } catch (err) { - schemasStore.set({ - databaseId: $currentDatabase.id, - requestStatus: { state: 'failure', errors: [getErrorMessage(err)] }, - data: new Map(), + schemasStore.update(($schemasStore) => { + if ($schemasStore.databaseId === $currentDatabase.id) { + return { + ...$schemasStore, + requestStatus: { state: 'failure', errors: [getErrorMessage(err)] }, + }; + } + return { + databaseId: $currentDatabase.id, + requestStatus: { state: 'failure', errors: [getErrorMessage(err)] }, + data: new Map(), + }; }); } } From 96c846b89d096545739248a3ba32ae908688cb7d Mon Sep 17 00:00:00 2001 From: pavish Date: Mon, 30 Sep 2024 19:43:01 +0530 Subject: [PATCH 10/15] Handle side effects for role member change or deletion --- .../contexts/DatabaseSettingsRouteContext.ts | 17 +++++----- mathesar_ui/src/models/Database.ts | 2 +- .../settings/roles/ModifyRoleMembers.svelte | 2 ++ .../database/settings/roles/RoleRow.svelte | 6 ++-- .../database/settings/roles/Roles.svelte | 33 ++++++++++++++++++- .../systems/permissions/permissionsUtils.ts | 2 +- 6 files changed, 49 insertions(+), 13 deletions(-) diff --git a/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts b/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts index fd2a9bda62..6dbac9318f 100644 --- a/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts +++ b/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts @@ -70,11 +70,19 @@ export class DatabaseSettingsRouteContext { (cr) => cr.name, ); if (isStable && loginRoles && configuredRoles) { - return [...loginRoles.values()].map((role) => ({ + const allRoles = [...loginRoles.values()].map((role) => ({ name: role.name, role, configuredRole: configuredRoles.get(role.name), })); + const allLoginRoleNames = new Set(allRoles.map((ar) => ar.name)); + const configuredRolesNotInAllRoles = [...configuredRoles.values()] + .filter((cr) => !allLoginRoleNames.has(cr.name)) + .map((configuredRole) => ({ + name: configuredRole.name, + configuredRole, + })); + return [...allRoles, ...configuredRolesNotInAllRoles]; } if ($configuredRoles.isStable && configuredRoles) { return [...configuredRoles.values()].map((configuredRole) => ({ @@ -136,13 +144,6 @@ export class DatabaseSettingsRouteContext { this.collaborators.updateResolvedValue((c) => c.without(collaborator.id)); } - async deleteRoleAndResetDependents(role: Role) { - await this.databaseRouteContext.deleteRole(role); - // When a role is deleted, both Collaborators & ConfiguredRoles need to be reset - this.configuredRoles.reset(); - this.collaborators.reset(); - } - static construct(databaseRouteContext: DatabaseRouteContext) { return setRouteContext( contextKey, diff --git a/mathesar_ui/src/models/Database.ts b/mathesar_ui/src/models/Database.ts index 35c90a72dc..166b17264a 100644 --- a/mathesar_ui/src/models/Database.ts +++ b/mathesar_ui/src/models/Database.ts @@ -90,7 +90,7 @@ export class Database { return new AsyncRpcApiStore(api.roles.get_current_role, { postProcess: (currentRole) => ({ currentRoleOid: currentRole.current_role.oid, - parentRoleOids: currentRole.parent_roles.map((pr) => pr.oid), + parentRoleOids: new Set(currentRole.parent_roles.map((pr) => pr.oid)), }), }); } diff --git a/mathesar_ui/src/pages/database/settings/roles/ModifyRoleMembers.svelte b/mathesar_ui/src/pages/database/settings/roles/ModifyRoleMembers.svelte index cdfd9db7df..121bc8b6d4 100644 --- a/mathesar_ui/src/pages/database/settings/roles/ModifyRoleMembers.svelte +++ b/mathesar_ui/src/pages/database/settings/roles/ModifyRoleMembers.svelte @@ -26,6 +26,7 @@ export let controller: ModalController; export let parentRole: Role; export let rolesMap: ImmutableMap; + export let handleRoleChangeSideEffects: (role: Role) => void; $: savedMembers = parentRole.members; $: savedMemberOids = new Set([...$savedMembers.keys()]); @@ -51,6 +52,7 @@ async function saveMembers() { await parentRole.setMembers(new Set($memberOids)); + handleRoleChangeSideEffects(parentRole); controller.close(); toast.success($_('child_roles_saved_successfully')); } diff --git a/mathesar_ui/src/pages/database/settings/roles/RoleRow.svelte b/mathesar_ui/src/pages/database/settings/roles/RoleRow.svelte index a0f8b33953..60c95adb1f 100644 --- a/mathesar_ui/src/pages/database/settings/roles/RoleRow.svelte +++ b/mathesar_ui/src/pages/database/settings/roles/RoleRow.svelte @@ -20,13 +20,15 @@ export let role: Role; export let rolesMap: ImmutableMap; - export let modifyMembersForRole: (role: Role) => unknown; + export let modifyMembersForRole: (role: Role) => void; + export let handleRoleChangeSideEffects: (role: Role) => void; $: members = role.members; async function dropRole() { try { - await $routeContext.deleteRoleAndResetDependents(role); + await $routeContext.databaseRouteContext.deleteRole(role); + handleRoleChangeSideEffects(role); toast.success($_('role_dropped_successfully')); } catch (err) { toast.error(getErrorMessage(err)); diff --git a/mathesar_ui/src/pages/database/settings/roles/Roles.svelte b/mathesar_ui/src/pages/database/settings/roles/Roles.svelte index 5778618f17..2611897101 100644 --- a/mathesar_ui/src/pages/database/settings/roles/Roles.svelte +++ b/mathesar_ui/src/pages/database/settings/roles/Roles.svelte @@ -1,4 +1,5 @@ @@ -65,6 +94,7 @@ {role} rolesMap={$roles.resolvedValue} {modifyMembersForRole} + {handleRoleChangeSideEffects} /> {/each} @@ -83,6 +113,7 @@ parentRole={targetRole} rolesMap={$roles.resolvedValue} controller={modifyRoleMembersModalController} + {handleRoleChangeSideEffects} /> {/if} diff --git a/mathesar_ui/src/systems/permissions/permissionsUtils.ts b/mathesar_ui/src/systems/permissions/permissionsUtils.ts index d715f7b4b2..bf3bcfd45f 100644 --- a/mathesar_ui/src/systems/permissions/permissionsUtils.ts +++ b/mathesar_ui/src/systems/permissions/permissionsUtils.ts @@ -21,7 +21,7 @@ export interface PermissionsStoreValues { permissionsMetaData: PermissionsMetaData; currentRole: { currentRoleOid: Role['oid']; - parentRoleOids: Role['oid'][]; + parentRoleOids: Set; }; } From 2a2bb6e5acf2fcc22b87701e605f387ee1311079 Mon Sep 17 00:00:00 2001 From: pavish Date: Mon, 30 Sep 2024 19:54:23 +0530 Subject: [PATCH 11/15] Fixes linting errors --- mathesar/views.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/mathesar/views.py b/mathesar/views.py index be4246eb0d..2a5e4b8f4c 100644 --- a/mathesar/views.py +++ b/mathesar/views.py @@ -12,12 +12,10 @@ from mathesar.rpc.schemas import list_ as schemas_list from mathesar.rpc.servers.configured import list_ as get_servers_list from mathesar.rpc.tables import list_with_metadata as tables_list -from mathesar.api.serializers.databases import TypeSerializer from mathesar.api.serializers.tables import TableSerializer from mathesar.api.serializers.queries import QuerySerializer from mathesar.api.ui.serializers.users import UserSerializer from mathesar.api.utils import is_valid_uuid_v4 -from mathesar.database.types import UIType from mathesar.models.shares import SharedTable, SharedQuery from mathesar.state import reset_reflection from mathesar import __version__ From 2c39ff9229700115cd76a79f52c21bec01bb4afc Mon Sep 17 00:00:00 2001 From: Sean Colsen Date: Mon, 30 Sep 2024 13:55:36 -0400 Subject: [PATCH 12/15] Adjust some naming --- .../src/pages/database/DatabasePageWrapper.svelte | 2 +- .../src/pages/database/schemas/SchemasSection.svelte | 2 +- .../settings/collaborators/CollaboratorRow.svelte | 8 ++++---- .../settings/collaborators/Collaborators.svelte | 8 ++++---- .../collaborators/EditRoleForCollaboratorModal.svelte | 4 ++-- .../database/settings/roles/ModifyRoleMembers.svelte | 4 ++-- .../src/pages/database/settings/roles/RoleRow.svelte | 11 ++++------- .../src/pages/database/settings/roles/Roles.svelte | 8 ++++---- mathesar_ui/src/stores/AsyncStore.ts | 2 +- 9 files changed, 23 insertions(+), 26 deletions(-) diff --git a/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte b/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte index 495e1aed6b..5c20d303c4 100644 --- a/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte +++ b/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte @@ -34,7 +34,7 @@ const databaseRouteContext = DatabaseRouteContext.get(); $: ({ database, underlyingDatabase } = $databaseRouteContext); - $: void underlyingDatabase.runOptimally({ database_id: database.id }); + $: void underlyingDatabase.runConservatively({ database_id: database.id }); const commonData = preloadCommonData(); diff --git a/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte b/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte index 1f4907a5c2..a911f24042 100644 --- a/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte +++ b/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte @@ -29,7 +29,7 @@ const databaseRouteContext = DatabaseRouteContext.get(); $: ({ database, underlyingDatabase } = $databaseRouteContext); - $: void underlyingDatabase.runOptimally({ database_id: database.id }); + $: void underlyingDatabase.runConservatively({ database_id: database.id }); $: schemasMap = $schemasStore.data; $: schemasRequestStatus = $schemasStore.requestStatus; $: isLoading = diff --git a/mathesar_ui/src/pages/database/settings/collaborators/CollaboratorRow.svelte b/mathesar_ui/src/pages/database/settings/collaborators/CollaboratorRow.svelte index 810caa0d14..076a27e7e3 100644 --- a/mathesar_ui/src/pages/database/settings/collaborators/CollaboratorRow.svelte +++ b/mathesar_ui/src/pages/database/settings/collaborators/CollaboratorRow.svelte @@ -12,8 +12,8 @@ import { Button, SpinnerButton } from '@mathesar-component-library'; export let collaborator: Collaborator; - export let editRoleForCollaborator: (collaborator: Collaborator) => void; - export let checkAndHandleSideEffects: (collaboror: Collaborator) => void; + export let onClickEditRole: (collaborator: Collaborator) => void; + export let onDelete: (collaborator: Collaborator) => void; const routeContext = DatabaseSettingsRouteContext.get(); $: ({ configuredRoles, users } = $routeContext); @@ -28,7 +28,7 @@ async function deleteCollaborator() { await $routeContext.deleteCollaborator(collaborator); - checkAndHandleSideEffects(collaborator); + onDelete(collaborator); toast.success($_('collaborator_removed_successfully')); } @@ -55,7 +55,7 @@
-
diff --git a/mathesar_ui/src/pages/database/settings/roles/Roles.svelte b/mathesar_ui/src/pages/database/settings/roles/Roles.svelte index 2611897101..6cfa45c2d5 100644 --- a/mathesar_ui/src/pages/database/settings/roles/Roles.svelte +++ b/mathesar_ui/src/pages/database/settings/roles/Roles.svelte @@ -27,7 +27,7 @@ $: ({ database, roles, underlyingDatabase, currentRole } = databaseRouteContext); - $: void roles.runOptimally({ database_id: database.id }); + $: void roles.runConservatively({ database_id: database.id }); $: roleList = [...($roles.resolvedValue?.values() ?? [])]; let targetRole: Role | undefined = undefined; @@ -93,8 +93,8 @@ {/each} @@ -113,7 +113,7 @@ parentRole={targetRole} rolesMap={$roles.resolvedValue} controller={modifyRoleMembersModalController} - {handleRoleChangeSideEffects} + onSave={({ parentRole }) => handleRoleChangeSideEffects(parentRole)} /> {/if} diff --git a/mathesar_ui/src/stores/AsyncStore.ts b/mathesar_ui/src/stores/AsyncStore.ts index b6a48a48b3..d7192c80d0 100644 --- a/mathesar_ui/src/stores/AsyncStore.ts +++ b/mathesar_ui/src/stores/AsyncStore.ts @@ -158,7 +158,7 @@ export default class AsyncStore * - rejected * - not loading */ - async runOptimally(props: Props): Promise> { + async runConservatively(props: Props): Promise> { const value = get(this.value); if (value.isLoading || value.isOk) { return value; From 7b0604ecab3adf3ec501040ed7e82e127421f29a Mon Sep 17 00:00:00 2001 From: Sean Colsen Date: Mon, 30 Sep 2024 14:13:04 -0400 Subject: [PATCH 13/15] Simplify AsyncRpcApiStore.runBatch --- .../DatabasePermissionsModal.svelte | 2 +- .../collaborators/Collaborators.svelte | 19 +++---- .../RoleConfiguration.svelte | 2 +- .../database/settings/roles/Roles.svelte | 13 ++--- .../schema/SchemaPermissionsModal.svelte | 2 +- mathesar_ui/src/stores/AsyncRpcApiStore.ts | 51 +++++-------------- .../table/TablePermissions.svelte | 2 +- 7 files changed, 28 insertions(+), 63 deletions(-) diff --git a/mathesar_ui/src/pages/database/permissions/DatabasePermissionsModal.svelte b/mathesar_ui/src/pages/database/permissions/DatabasePermissionsModal.svelte index 1368844383..215337f097 100644 --- a/mathesar_ui/src/pages/database/permissions/DatabasePermissionsModal.svelte +++ b/mathesar_ui/src/pages/database/permissions/DatabasePermissionsModal.svelte @@ -65,7 +65,7 @@ }; function getAsyncStoresForPermissions() { - void AsyncRpcApiStore.runBatched([ + void AsyncRpcApiStore.runBatchConservatively([ databasePrivileges.batchRunner({ database_id: database.id }), roles.batchRunner({ database_id: database.id }), underlyingDatabase.batchRunner({ database_id: database.id }), diff --git a/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte b/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte index 040dc771a0..dae579b3b0 100644 --- a/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte +++ b/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte @@ -39,7 +39,7 @@ databaseRouteContext, } = $routeContext); - $: void AsyncRpcApiStore.runBatched([ + $: void AsyncRpcApiStore.runBatchConservatively([ collaborators.batchRunner({ database_id: database.id }), configuredRoles.batchRunner({ server_id: database.server.id }), ]); @@ -62,17 +62,12 @@ function checkAndHandleSideEffects(collaborator: Collaborator) { if (collaborator.userId === $userProfileStore.id) { - void AsyncRpcApiStore.runBatched( - [ - databaseRouteContext.underlyingDatabase.batchRunner({ - database_id: database.id, - }), - databaseRouteContext.roles.batchRunner({ database_id: database.id }), - ], - { - mode: 'force-run', - }, - ); + void AsyncRpcApiStore.runBatch([ + databaseRouteContext.underlyingDatabase.batchRunner({ + database_id: database.id, + }), + databaseRouteContext.roles.batchRunner({ database_id: database.id }), + ]); void fetchSchemasForCurrentDatabase(); } } diff --git a/mathesar_ui/src/pages/database/settings/role-configuration/RoleConfiguration.svelte b/mathesar_ui/src/pages/database/settings/role-configuration/RoleConfiguration.svelte index 852b17fd87..8b038f39ae 100644 --- a/mathesar_ui/src/pages/database/settings/role-configuration/RoleConfiguration.svelte +++ b/mathesar_ui/src/pages/database/settings/role-configuration/RoleConfiguration.svelte @@ -39,7 +39,7 @@ $: ({ database, databaseRouteContext, configuredRoles, combinedLoginRoles } = $routeContext); $: ({ roles } = databaseRouteContext); - $: void AsyncRpcApiStore.runBatched([ + $: void AsyncRpcApiStore.runBatchConservatively([ configuredRoles.batchRunner({ server_id: database.server.id }), roles.batchRunner({ database_id: database.id }), ]); diff --git a/mathesar_ui/src/pages/database/settings/roles/Roles.svelte b/mathesar_ui/src/pages/database/settings/roles/Roles.svelte index 6cfa45c2d5..0a9cea19b1 100644 --- a/mathesar_ui/src/pages/database/settings/roles/Roles.svelte +++ b/mathesar_ui/src/pages/database/settings/roles/Roles.svelte @@ -48,15 +48,10 @@ ); if (currentRoleInheritsOrIsEditedRole) { - void AsyncRpcApiStore.runBatched( - [ - currentRole.batchRunner({ database_id: database.id }), - underlyingDatabase.batchRunner({ database_id: database.id }), - ], - { - mode: 'force-run', - }, - ); + void AsyncRpcApiStore.runBatch([ + currentRole.batchRunner({ database_id: database.id }), + underlyingDatabase.batchRunner({ database_id: database.id }), + ]); void fetchSchemasForCurrentDatabase(); } } diff --git a/mathesar_ui/src/pages/schema/SchemaPermissionsModal.svelte b/mathesar_ui/src/pages/schema/SchemaPermissionsModal.svelte index cc02e589a0..452f9970d6 100644 --- a/mathesar_ui/src/pages/schema/SchemaPermissionsModal.svelte +++ b/mathesar_ui/src/pages/schema/SchemaPermissionsModal.svelte @@ -66,7 +66,7 @@ }; function getAsyncStoresForPermissions() { - void AsyncRpcApiStore.runBatched([ + void AsyncRpcApiStore.runBatchConservatively([ schemaPrivileges.batchRunner({ database_id: schema.database.id, schema_oid: schema.oid, diff --git a/mathesar_ui/src/stores/AsyncRpcApiStore.ts b/mathesar_ui/src/stores/AsyncRpcApiStore.ts index b21a999a98..32c4a3b74f 100644 --- a/mathesar_ui/src/stores/AsyncRpcApiStore.ts +++ b/mathesar_ui/src/stores/AsyncRpcApiStore.ts @@ -69,44 +69,19 @@ export default class AsyncRpcApiStore extends AsyncStore< }; } - static async runBatched( - batchRunners: BatchRunner[], - options?: Partial<{ - mode?: 'force-run' | 'optimal' | ('force-run' | 'optimal')[]; - }>, - ) { - const toRun = (() => { - if (options?.mode && Array.isArray(options.mode)) { - if (options.mode.length !== batchRunners.length) { - throw new Error( - 'Number of run options do not match number of batchRunners', - ); - } - return batchRunners.filter((runner, index) => { - switch (options.mode?.[index]) { - case 'force-run': - return true; - case 'optimal': - default: - return !runner.getValue().hasInitialized; - } - }); - } - if (options?.mode === 'force-run') { - return batchRunners; - } - // default is `optimal` - return batchRunners.filter((runner) => !runner.getValue().hasInitialized); - })(); + static async runBatch(runners: BatchRunner[]) { + if (runners.length === 0) return; + runners.forEach((runner) => runner.beforeRequest()); + const results = await batchSend(runners.map((runner) => runner.send)); + runners.forEach((runner, index) => runner.onResponse(results[index])); + } - if (toRun.length > 0) { - toRun.forEach((runner) => { - runner.beforeRequest(); - }); - const results = await batchSend(toRun.map((runner) => runner.send)); - toRun.forEach((runner, index) => { - runner.onResponse(results[index]); - }); - } + /** + * Runs the BatchRunners that have not yet been initialized. Skips the ones + * that have already been initialized. + */ + static async runBatchConservatively(runners: BatchRunner[]) { + const toRun = runners.filter((runner) => !runner.getValue().hasInitialized); + await AsyncRpcApiStore.runBatch(toRun); } } diff --git a/mathesar_ui/src/systems/table-view/table-inspector/table/TablePermissions.svelte b/mathesar_ui/src/systems/table-view/table-inspector/table/TablePermissions.svelte index 8ebeb20635..d6a27bf2d1 100644 --- a/mathesar_ui/src/systems/table-view/table-inspector/table/TablePermissions.svelte +++ b/mathesar_ui/src/systems/table-view/table-inspector/table/TablePermissions.svelte @@ -86,7 +86,7 @@ }; function getAsyncStoresForPermissions() { - void AsyncRpcApiStore.runBatched([ + void AsyncRpcApiStore.runBatchConservatively([ tablePrivileges.batchRunner({ database_id: table.schema.database.id, table_oid: table.oid, From 4291b3b96985f40a575f0270ea057239abdf4068 Mon Sep 17 00:00:00 2001 From: pavish Date: Tue, 1 Oct 2024 13:58:30 +0530 Subject: [PATCH 14/15] Make conservative run and conservative batchRun consistent with eachother --- .../import/preview/ImportPreviewContent.svelte | 2 +- mathesar_ui/src/stores/AsyncRpcApiStore.ts | 4 +++- mathesar_ui/src/stores/AsyncStore.ts | 18 ++++++++---------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/mathesar_ui/src/pages/import/preview/ImportPreviewContent.svelte b/mathesar_ui/src/pages/import/preview/ImportPreviewContent.svelte index fb21769193..9cb4763cc5 100644 --- a/mathesar_ui/src/pages/import/preview/ImportPreviewContent.svelte +++ b/mathesar_ui/src/pages/import/preview/ImportPreviewContent.svelte @@ -227,7 +227,7 @@ on:retry={init} on:delete={cancel} /> - {:else if !$previewRequest.hasInitialized} + {:else if !$previewRequest.hasSettled}
{:else if $previewRequest.error} extends AsyncStore< * that have already been initialized. */ static async runBatchConservatively(runners: BatchRunner[]) { - const toRun = runners.filter((runner) => !runner.getValue().hasInitialized); + const toRun = runners.filter( + (runner) => runner.getValue().isIdleAndUnsettled, + ); await AsyncRpcApiStore.runBatch(toRun); } } diff --git a/mathesar_ui/src/stores/AsyncStore.ts b/mathesar_ui/src/stores/AsyncStore.ts index d7192c80d0..15e93a026d 100644 --- a/mathesar_ui/src/stores/AsyncStore.ts +++ b/mathesar_ui/src/stores/AsyncStore.ts @@ -94,9 +94,13 @@ export class AsyncStoreValue { return !this.isLoading && this.settlement?.state === 'rejected'; } - get hasInitialized(): boolean { + get hasSettled(): boolean { return this.settlement !== undefined; } + + get isIdleAndUnsettled(): boolean { + return !this.hasSettled && !this.isLoading; + } } export default class AsyncStore @@ -152,18 +156,12 @@ export default class AsyncStore } } - /** - * Only runs if the current value is one of - * - not initialized - * - rejected - * - not loading - */ async runConservatively(props: Props): Promise> { const value = get(this.value); - if (value.isLoading || value.isOk) { - return value; + if (value.isIdleAndUnsettled) { + return this.run(props); } - return this.run(props); + return value; } /** From 98e48eab1534c24b4c6734040270f5e298b75b4b Mon Sep 17 00:00:00 2001 From: pavish Date: Tue, 1 Oct 2024 14:01:50 +0530 Subject: [PATCH 15/15] Handle side effects when logged in user is added as a collaborator --- mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts | 1 + .../settings/collaborators/AddCollaboratorModal.svelte | 7 ++++++- .../database/settings/collaborators/Collaborators.svelte | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts b/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts index 6dbac9318f..810e4ed0d0 100644 --- a/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts +++ b/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts @@ -137,6 +137,7 @@ export class DatabaseSettingsRouteContext { this.collaborators.updateResolvedValue((collaborators) => collaborators.with(newCollaborator.id, newCollaborator), ); + return newCollaborator; } async deleteCollaborator(collaborator: Collaborator) { diff --git a/mathesar_ui/src/pages/database/settings/collaborators/AddCollaboratorModal.svelte b/mathesar_ui/src/pages/database/settings/collaborators/AddCollaboratorModal.svelte index db762e0a97..0be74c4447 100644 --- a/mathesar_ui/src/pages/database/settings/collaborators/AddCollaboratorModal.svelte +++ b/mathesar_ui/src/pages/database/settings/collaborators/AddCollaboratorModal.svelte @@ -28,6 +28,7 @@ export let configuredRolesMap: ImmutableMap; export let usersMap: ImmutableMap; export let collaboratorsMap: ImmutableMap; + export let onAdd: (collaborator: Collaborator) => void; const userId = requiredField(undefined); const configuredRoleId = requiredField(undefined); @@ -41,7 +42,11 @@ async function addCollaborator() { if ($userId && $configuredRoleId) { - await $routeContext.addCollaborator($userId, $configuredRoleId); + const collaborator = await $routeContext.addCollaborator( + $userId, + $configuredRoleId, + ); + onAdd(collaborator); controller.close(); toast.success($_('collaborator_added_successfully')); form.reset(); diff --git a/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte b/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte index dae579b3b0..093f736d53 100644 --- a/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte +++ b/mathesar_ui/src/pages/database/settings/collaborators/Collaborators.svelte @@ -117,6 +117,7 @@ usersMap={$users.resolvedValue} configuredRolesMap={$configuredRoles.resolvedValue} collaboratorsMap={$collaborators.resolvedValue} + onAdd={checkAndHandleSideEffects} /> {/if}