Skip to content

Commit

Permalink
Add connection failed status (#4939)
Browse files Browse the repository at this point in the history
1/ When the user inputs wrong connection informations, we do not inform
him. He will only see that no tables are available.
We will display a connection failed status if an error is raised testing
the connection

2/ If the connection fails, it should still be possible to delete the
server. Today, since we try first to delete the tables, the connection
failure throws an error that will prevent server deletion. Using the
foreign tables instead of calling the distant DB.

3/ Redirect to connection show page instead of connection list after
creation

4/ Today, foreign tables are fetched without the server name. This is a
mistake because we need to know which foreign table is linked with which
server. Updating the associated query.

<img width="632" alt="Capture d’écran 2024-04-12 à 10 52 49"
src="https://github.com/twentyhq/twenty/assets/22936103/9e8406b8-75d0-494c-ac1f-5e9fa7100f5c">

---------

Co-authored-by: Thomas Trompette <[email protected]>
  • Loading branch information
thomtrp and Thomas Trompette authored Apr 15, 2024
1 parent 3e65fbd commit 756de8a
Show file tree
Hide file tree
Showing 12 changed files with 145 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const useGetDatabaseConnectionTables = ({
}: UseGetDatabaseConnectionTablesParams) => {
const apolloMetadataClient = useApolloMetadataClient();

const { data } = useQuery<
const { data, error } = useQuery<
GetManyRemoteTablesQuery,
GetManyRemoteTablesQueryVariables
>(GET_MANY_REMOTE_TABLES, {
Expand All @@ -33,5 +33,6 @@ export const useGetDatabaseConnectionTables = ({

return {
tables: data?.findAvailableRemoteTablesByServerId || [],
error,
};
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { useContext } from 'react';
import { EntityChip } from 'twenty-ui';

import { useMapToObjectRecordIdentifier } from '@/object-metadata/hooks/useMapToObjectRecordIdentifier';
import { RecordTableRowContext } from '@/object-record/record-table/contexts/RecordTableRowContext';
import { ObjectRecord } from '@/object-record/types/ObjectRecord';
import { getImageAbsoluteURIOrBase64 } from '~/utils/image/getImageAbsoluteURIOrBase64';

Expand All @@ -21,8 +23,16 @@ export const RecordChip = ({
objectNameSingular,
});

// Will only exists if the chip is inside a record table.
// This is temporary until we have the show page for remote objects.
const { isReadOnly } = useContext(RecordTableRowContext);

const objectRecordIdentifier = mapToObjectRecordIdentifier(record);

const linkToEntity = isReadOnly
? undefined
: objectRecordIdentifier.linkToShowPage;

return (
<EntityChip
entityId={record.id}
Expand All @@ -31,7 +41,7 @@ export const RecordChip = ({
avatarUrl={
getImageAbsoluteURIOrBase64(objectRecordIdentifier.avatarUrl) || ''
}
linkToEntity={objectRecordIdentifier.linkToShowPage}
linkToEntity={linkToEntity}
maxWidth={maxWidth}
className={className}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useGetDatabaseConnectionTables } from '@/databases/hooks/useGetDatabaseConnectionTables';
import { Status } from '@/ui/display/status/components/Status';
import { RemoteTableStatus } from '~/generated-metadata/graphql';
import { isDefined } from '~/utils/isDefined';

type SettingsIntegrationDatabaseConnectionSyncStatusProps = {
connectionId: string;
Expand All @@ -11,11 +12,15 @@ export const SettingsIntegrationDatabaseConnectionSyncStatus = ({
connectionId,
skip,
}: SettingsIntegrationDatabaseConnectionSyncStatusProps) => {
const { tables } = useGetDatabaseConnectionTables({
const { tables, error } = useGetDatabaseConnectionTables({
connectionId,
skip,
});

if (isDefined(error)) {
return <Status color="red" text="Connection failed" />;
}

const syncedTables = tables.filter(
(table) => table.status === RemoteTableStatus.Synced,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@ export const RecordIndexPageHeader = ({
const canAddRecord =
recordIndexViewType === ViewType.Table && !objectMetadataItem?.isRemote;

const pageHeaderTitle =
objectMetadataItem?.labelPlural ?? capitalize(objectNamePlural);

return (
<PageHeader title={capitalize(objectNamePlural)} Icon={Icon}>
<PageHeader title={pageHeaderTitle} Icon={Icon}>
<PageHotkeysEffect onAddButtonClick={createRecord} />
{canAddRecord && <PageAddButton onClick={createRecord} />}
</PageHeader>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,18 @@ export const SettingsIntegrationNewDatabaseConnection = () => {
const formValues = formConfig.getValues();

try {
await createOneDatabaseConnection(
const createdConnection = await createOneDatabaseConnection(
createRemoteServerInputSchema.parse({
...formValues,
foreignDataWrapperType: getForeignDataWrapperType(databaseKey),
}),
);

navigate(`${settingsIntegrationsPagePath}/${databaseKey}`);
const connectionId = createdConnection.data?.createOneRemoteServer.id;

navigate(
`${settingsIntegrationsPagePath}/${databaseKey}/${connectionId}`,
);
} catch (error) {
enqueueSnackBar((error as Error).message, {
variant: 'error',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
[
createdObjectMetadata.id,
'table',
`All ${createdObjectMetadata.namePlural}`,
`All ${createdObjectMetadata.labelPlural}`,
'INDEX',
createdObjectMetadata.icon,
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
} from 'src/engine/metadata-modules/remote-server/utils/validate-remote-server-input';
import { ForeignDataWrapperQueryFactory } from 'src/engine/api/graphql/workspace-query-builder/factories/foreign-data-wrapper-query.factory';
import { RemoteTableService } from 'src/engine/metadata-modules/remote-server/remote-table/remote-table.service';
import { RemoteTableStatus } from 'src/engine/metadata-modules/remote-server/remote-table/dtos/remote-table.dto';

@Injectable()
export class RemoteServerService<T extends RemoteServerType> {
Expand Down Expand Up @@ -117,21 +116,18 @@ export class RemoteServerService<T extends RemoteServerType> {
throw new NotFoundException('Object does not exist');
}

const remoteTablesToRemove = (
await this.remoteTableService.findAvailableRemoteTablesByServerId(
id,
const foreignTablesToRemove =
await this.remoteTableService.fetchForeignTableNamesWithinWorkspace(
workspaceId,
)
).filter((remoteTable) => remoteTable.status === RemoteTableStatus.SYNCED);

if (remoteTablesToRemove.length) {
for (const remoteTable of remoteTablesToRemove) {
await this.remoteTableService.unsyncRemoteTable(
{
remoteServerId: id,
name: remoteTable.name,
},
remoteServer.foreignDataWrapperId,
);

if (foreignTablesToRemove.length) {
for (const foreignTableName of foreignTablesToRemove) {
await this.remoteTableService.removeForeignTableAndMetadata(
foreignTableName,
workspaceId,
remoteServer,
);
}
}
Expand Down
Loading

0 comments on commit 756de8a

Please sign in to comment.