Skip to content

Commit

Permalink
Fix 'name' column wrongly added in standard objects (twentyhq#7428)
Browse files Browse the repository at this point in the history
## Context
Name shouldn't be added to all tables, especially standard objects
because they already have their own labelIdentifierFieldMetadata
specified in the workspace-entity schema. This PR removes this column
from the "base" list of columns to add when creating a new object/table
and moves it to the object-metadata service that is, as of today, only
used for custom objects. Also had to modify the migration-runner to
handle column creation in a table creation migration (this was available
in the migration definition already but was not doing anything)

This also fixes an issue in standard objects that already have a "name"
field defined with a different field type, this is even more important
when the said field is a composite field. For example people already has
a FULL_NAME name field which clashes with the default TEXT name field
meaning it was only creating 1 field metadata for 'name' but 3 columns
were created: `name, nameFirstName, nameLastName`. This inconsistency
with metadata (which is our source of truth everywhere) brought some
issues (lately, converting back typeorm response to gql (including
composition) was broken).
  • Loading branch information
Weiko authored and harshit078 committed Oct 14, 2024
1 parent 0d15069 commit 914b09d
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ export class GraphqlQueryUpdateManyResolverService
const data = formatData(args.data, objectMetadataMapItem);

const result = await withFilterQueryBuilder
.update()
.set(data)
.update(data)
.returning('*')
.execute();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,11 @@ export class GraphqlQueryUpdateOneResolverService
objectMetadataMapItem.nameSingular,
);

const withFilterQueryBuilder = queryBuilder.where({ id: args.id });

const data = formatData(args.data, objectMetadataMapItem);

const result = await withFilterQueryBuilder
.update()
.set(data)
const result = await queryBuilder
.update(data)
.where({ id: args.id })
.returning('*')
.execute();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ import { generateMigrationName } from 'src/engine/metadata-modules/workspace-mig
import {
WorkspaceMigrationColumnActionType,
WorkspaceMigrationColumnDrop,
WorkspaceMigrationTableAction,
WorkspaceMigrationTableActionType,
} from 'src/engine/metadata-modules/workspace-migration/workspace-migration.entity';
import { WorkspaceMigrationFactory } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.factory';
import { WorkspaceMigrationService } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.service';
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util';
Expand Down Expand Up @@ -103,6 +105,7 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
private readonly workspaceMigrationRunnerService: WorkspaceMigrationRunnerService,
private readonly workspaceMetadataVersionService: WorkspaceMetadataVersionService,
private readonly twentyORMGlobalManager: TwentyORMGlobalManager,
private readonly workspaceMigrationFactory: WorkspaceMigrationFactory,
) {
super(objectMetadataRepository);
}
Expand Down Expand Up @@ -563,9 +566,39 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
objectMetadataInput.primaryKeyFieldMetadataSettings,
);

return this.workspaceMigrationService.createCustomMigration(
await this.workspaceMigrationService.createCustomMigration(
generateMigrationName(`create-${createdObjectMetadata.nameSingular}`),
createdObjectMetadata.workspaceId,
[
{
name: computeObjectTargetTable(createdObjectMetadata),
action: WorkspaceMigrationTableActionType.CREATE,
} satisfies WorkspaceMigrationTableAction,
],
);

for (const fieldMetadata of createdObjectMetadata.fields) {
await this.workspaceMigrationService.createCustomMigration(
generateMigrationName(`create-${fieldMetadata.name}`),
createdObjectMetadata.workspaceId,
[
{
name: computeObjectTargetTable(createdObjectMetadata),
action: WorkspaceMigrationTableActionType.ALTER,
columns: this.workspaceMigrationFactory.createColumnActions(
WorkspaceMigrationColumnActionType.CREATE,
fieldMetadata,
),
},
] satisfies WorkspaceMigrationTableAction[],
);
}

await this.workspaceMigrationService.createCustomMigration(
generateMigrationName(
`create-${createdObjectMetadata.nameSingular}-relations`,
),
createdObjectMetadata.workspaceId,
buildMigrationsForCustomObjectRelations(
createdObjectMetadata,
activityTargetObjectMetadata,
Expand Down Expand Up @@ -611,7 +644,9 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
}

this.workspaceMigrationService.createCustomMigration(
generateMigrationName(`create-${createdObjectMetadata.nameSingular}`),
generateMigrationName(
`update-${createdObjectMetadata.nameSingular}-add-searchVector`,
),
createdObjectMetadata.workspaceId,
[
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ export const buildMigrationsForCustomObjectRelations = (
noteTargetObjectMetadata: ObjectMetadataEntity,
taskTargetObjectMetadata: ObjectMetadataEntity,
): WorkspaceMigrationTableAction[] => [
{
name: computeObjectTargetTable(createdObjectMetadata),
action: WorkspaceMigrationTableActionType.CREATE,
} satisfies WorkspaceMigrationTableAction,
// Add activity target relation
{
name: computeObjectTargetTable(activityTargetObjectMetadata),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,5 @@ export abstract class BaseWorkspaceEntity {
},
})
@WorkspaceIsNullable()
deletedAt?: string | null;
deletedAt: string | null;
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { TableColumnOptions } from 'typeorm';

export const tableDefaultColumns = (): TableColumnOptions[] => [
{
name: 'id',
type: 'uuid',
isPrimary: true,
default: 'public.uuid_generate_v4()',
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ import { WorkspaceMigrationService } from 'src/engine/metadata-modules/workspace
import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/workspace-datasource.service';
import { WorkspaceMigrationEnumService } from 'src/engine/workspace-manager/workspace-migration-runner/services/workspace-migration-enum.service';
import { convertOnDeleteActionToOnDelete } from 'src/engine/workspace-manager/workspace-migration-runner/utils/convert-on-delete-action-to-on-delete.util';
import { tableDefaultColumns } from 'src/engine/workspace-manager/workspace-migration-runner/utils/table-default-column.util';
import { isDefined } from 'src/utils/is-defined';

import { WorkspaceMigrationTypeService } from './services/workspace-migration-type.service';
import { customTableDefaultColumns } from './utils/custom-table-default-column.util';

@Injectable()
export class WorkspaceMigrationRunnerService {
Expand Down Expand Up @@ -121,7 +121,12 @@ export class WorkspaceMigrationRunnerService {
) {
switch (tableMigration.action) {
case WorkspaceMigrationTableActionType.CREATE:
await this.createTable(queryRunner, schemaName, tableMigration.name);
await this.createTable(
queryRunner,
schemaName,
tableMigration.name,
tableMigration.columns,
);
break;
case WorkspaceMigrationTableActionType.ALTER: {
if (tableMigration.newName) {
Expand Down Expand Up @@ -244,16 +249,26 @@ export class WorkspaceMigrationRunnerService {
queryRunner: QueryRunner,
schemaName: string,
tableName: string,
columns?: WorkspaceMigrationColumnAction[],
) {
await queryRunner.createTable(
new Table({
name: tableName,
schema: schemaName,
columns: customTableDefaultColumns(tableName),
columns: tableDefaultColumns(),
}),
true,
);

if (columns && columns.length > 0) {
await this.handleColumnChanges(
queryRunner,
schemaName,
tableName,
columns,
);
}

// Enable totalCount for the table
await queryRunner.query(`
COMMENT ON TABLE "${schemaName}"."${tableName}" IS '@graphql({"totalCount": {"enabled": true}})';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export type CalendarEvent = Omit<
CalendarEventWorkspaceEntity,
| 'createdAt'
| 'updatedAt'
| 'deletedAt'
| 'calendarChannelEventAssociations'
| 'calendarEventParticipants'
| 'conferenceLink'
Expand All @@ -19,6 +20,7 @@ export type CalendarEventParticipant = Omit<
| 'id'
| 'createdAt'
| 'updatedAt'
| 'deletedAt'
| 'personId'
| 'workspaceMemberId'
| 'person'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export type Message = Omit<
MessageWorkspaceEntity,
| 'createdAt'
| 'updatedAt'
| 'deletedAt'
| 'messageChannelMessageAssociations'
| 'messageParticipants'
| 'messageThread'
Expand All @@ -25,6 +26,7 @@ export type MessageParticipant = Omit<
| 'id'
| 'createdAt'
| 'updatedAt'
| 'deletedAt'
| 'personId'
| 'workspaceMemberId'
| 'person'
Expand Down

0 comments on commit 914b09d

Please sign in to comment.