Add label identifier to object decorator #6227

Weiko · 2024-07-11T16:48:01Z

Context

LabelIdentifier and ImageIdentifier are metadata info attached to objectMetadata that are used to display a record in a more readable way. Those columns point to existing fields that are part of the object.
For example, for a relation picker of a person, we will show a record using the "name" labelIdentifier and the "avatarUrl" imageIdentifier.

Currently, the FE has a specific logic for company and people objects and we have a way to update this value via the API for custom objects, but the code is not flexible enough to change other standard objects.

This PR updates the WorkspaceEntity API so we can now provide the labelIdentifier and imageIdentifier in the WorkspaceEntity decorator.

Example:

@WorkspaceEntity({
  standardId: STANDARD_OBJECT_IDS.activity,
  namePlural: 'activities',
  labelSingular: 'Activity',
  labelPlural: 'Activities',
  description: 'An activity',
  icon: 'IconCheckbox',
  labelIdentifierStandardId: ACTIVITY_STANDARD_FIELD_IDS.title,
})
@WorkspaceIsSystem()
export class ActivityWorkspaceEntity extends BaseWorkspaceEntity {
  @WorkspaceField({
    standardId: ACTIVITY_STANDARD_FIELD_IDS.title,
    type: FieldMetadataType.TEXT,
    label: 'Title',
    description: 'Activity title',
    icon: 'IconNotes',
  })
  title: string;
...

greptile-apps

PR Summary

Added labelIdentifierFieldMetadataId and imageIdentifierFieldMetadataId to ObjectMetadataInterface in /packages/twenty-server/src/engine/metadata-modules/field-metadata/interfaces/object-metadata.interface.ts
Updated ObjectMetadataDTO with new identifier fields in /packages/twenty-server/src/engine/metadata-modules/object-metadata/dtos/object-metadata.dto.ts
Enhanced ObjectMetadataEntity to support new identifier properties in /packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.entity.ts
Introduced WorkspaceSyncObjectMetadataIdentifiersService for synchronizing metadata identifiers in /packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/services/workspace-sync-object-metadata-identifiers.service.ts
Modified WorkspaceEntity decorator to include label and image identifiers in /packages/twenty-server/src/engine/twenty-orm/decorators/workspace-entity.decorator.ts

_{11 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

Refactored synchronization logic to handle label and image identifiers in packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/services/workspace-sync-object-metadata-identifiers.service.ts
Added methods to fetch original object metadata and create a standard object metadata map
Processed object metadata collection to include identifier fields
Validated field metadata types for label and image identifiers

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

Weiko · 2024-07-11T18:05:16Z

packages/twenty-server/src/engine/metadata-modules/object-metadata/dtos/object-metadata.dto.ts


-  @Field({ nullable: true })
-  imageIdentifierFieldMetadataId?: string;
+  @Field(() => String, { nullable: true })


This is needed because nestjs/graphql usually infers the type from the property bellow but since now it can be either a string or null it does not know what to do so you have to specify the return type in the Field decorator

greptile-apps

PR Summary

(updates since last review)

No major changes found since last review.

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

Added labelIdentifier and imageIdentifier to WorkspaceEntity decorator for better record display (packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/constants/standard-field-ids.ts, packages/twenty-server/src/modules/opportunity/standard-objects/opportunity.workspace-entity.ts).
Removed probability field from various GraphQL queries and mock data (packages/twenty-front/src/modules/object-record/hooks/__mocks__/useFindManyRecords.ts, packages/twenty-front/src/modules/search/hooks/__mocks__/useFilteredSearchEntityQuery.ts).
Introduced new command to add new address field to views with deprecated address field (packages/twenty-server/src/database/commands/0-22-add-new-address-field-to-views-with-deprecated-address.command.ts).
Updated imports and dependencies to support new metadata functionality (packages/twenty-server/src/engine/core-modules/workspace/workspace.module.ts, packages/twenty-server/src/engine/workspace-manager/workspace-manager.module.ts).
Enhanced sync-workspace-metadata.command.ts to support multi-workspace metadata sync and error handling.

_{25 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

FelixMalfait

Great!

FelixMalfait · 2024-07-12T05:51:04Z

...nager/workspace-sync-metadata/services/workspace-sync-object-metadata-identifiers.service.ts

+        imageIdentifierFieldMetadata,
+      );
+
+      // TODO: Add image identifier field metadata


Would you mind tackling this in this PR? I feel it's going to get lost otherwise

I'm not sure what to do with them. I feel like except for people we don't really have any use case of imageIdentifier for standard objects. Also we don't have a file/image field type yet so I wanted to wait to implement properly. Wdyt?

...nager/workspace-sync-metadata/services/workspace-sync-object-metadata-identifiers.service.ts

...ne/workspace-manager/workspace-sync-metadata/interfaces/partial-object-metadata.interface.ts

packages/twenty-server/src/modules/activity/standard-objects/activity.workspace-entity.ts

Weiko · 2024-07-12T08:31:22Z

Thanks @FelixMalfait, answered your comments!

charlesBochet

LGTM,
IMO: we should not implement imageIdentifier as long as we don't have a robust field type. It's completely fine for chips to fallback on taking the first letter of the labelIdentifier

We can't put the labelIdentifierFieldMetadataId to non nullable in database as the current process of object creation is: create object, then create fields, then set identifier (for standard, we could wrap in a transaction but for custom these steps are done by the FE in different calls).
But we could make it required in the decorator and use Ids for standard object that do not have a legit TEXT field that could be used as labelIdentifier

FelixMalfait · 2024-07-12T09:22:29Z

Thanks @charlesBochet - agree, except I think we should do a fast-follow after this PR and implement a file field type. It might not be perfect but it takes 30 min to create a very basic one and then we can avoid creating more debt (like here, or like when Thomas built the token protection, or in the frontend code as we try to make "attachments" fit in the global model, etc.)

Weiko · 2024-07-12T09:57:56Z

I've updated the PR @charlesBochet @FelixMalfait, this should now include a fallback logic on PK and set a label for the rest when possible

greptile-apps

PR Summary

(updates since last review)

Added labelIdentifierStandardId to WorkspaceEntity decorator in multiple files for better record readability (e.g., packages/twenty-server/src/modules/activity/standard-objects/activity.workspace-entity.ts, packages/twenty-server/src/modules/api-key/standard-objects/api-key.workspace-entity.ts).
Updated workspace-entity.decorator.ts to provide a default value for labelIdentifierStandardId.
Enhanced workspace-sync-object-metadata-identifiers.service.ts to support additional field types for label identifiers.
Reordered and added imports in workspace-sync-metadata/standard-objects/index.ts for better organization.
Reorganized imports in several files for improved clarity and consistency.

_{21 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

Added migration to objectMetadata table for UUID columns and constraints (packages/twenty-server/src/database/typeorm/metadata/migrations/1720792873225-createIdentifiersFK.ts)
Introduced OneToOne relations for labelIdentifier and imageIdentifier fields (packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.entity.ts)
Added new columns and relations for labelIdentifierFieldMetadata and imageIdentifierFieldMetadata (packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.entity.ts)

_{3 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

charlesBochet · 2024-07-12T16:28:24Z

@FelixMalfait fast follow implying data migration is not cheap! Can't we stay with the computed coloured avatar based on labelIdentifer for now? Not sure the users will value being able to add a picture manually anyway

FelixMalfait · 2024-07-12T16:30:54Z

@charlesBochet I'm doing similar migrations on the PR to standardize of Activity, should I introduce the FILE field type myself in that PR to manage all migrations at once? I have a feeling this would be a similar migration that the activity.body TEXT -> RICH_TEXT migration I added, so if it helps I'm happy to do it at the same time?

FelixMalfait · 2024-07-14T13:26:25Z

(Discussed in DM - I thought it was a fast follow but it's not)

This reverts commit fab4b56.

greptile-apps

PR Summary

(updates since last review)

Deleted migration file packages/twenty-server/src/database/typeorm/metadata/migrations/1720792873225-createIdentifiersFK.ts
Removed relations to labelIdentifierFieldMetadata and imageIdentifierFieldMetadata in packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.entity.ts
Updated column types for labelIdentifierFieldMetadataId and imageIdentifierFieldMetadataId from 'uuid' to 'text' in packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.entity.ts
Removed OneToOne relations for labelIdentifierFieldMetadataId and imageIdentifierFieldMetadataId in packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.entity.ts

_{3 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

Added migration file packages/twenty-server/src/database/typeorm/metadata/migrations/1721057142509-fixIdentifierTypes.ts to change column types to uuid
Updated packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.entity.ts to change column types to uuid for labelIdentifierFieldMetadataId and imageIdentifierFieldMetadataId

_{2 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

…rator

greptile-apps

PR Summary

(updates since last review)

Added labelIdentifier and imageIdentifier to WorkspaceEntity decorator in multiple files for better metadata display
Introduced new hooks and utilities for fetching and managing record data in packages/twenty-front
Refactored import paths and module structures in packages/twenty-server for better organization
Added new constants for batch sizes and query limits in packages/twenty-front
Updated test files to cover new functionalities and ensure robustness

_{148 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

…rator

greptile-apps

PR Summary

(updates since last review)

Added labelIdentifier and imageIdentifier to WorkspaceEntity decorator for enhanced metadata display (packages/twenty-front/package.json, packages/twenty-server/package.json)
Introduced useScrollToPosition hook for custom scroll behavior (packages/twenty-front/src/hooks/useScrollToPosition.ts)
Added CalendarEventDetailsEffect component for upserting calendar event records (packages/twenty-front/src/modules/activities/calendar/components/CalendarEventDetailsEffect.tsx)
Replaced EntityChip with RecordChip and AvatarChip components for better record display (packages/twenty-front/src/modules/object-record/record-field/components/FieldDisplay.tsx, packages/twenty-front/src/modules/object-record/record-field/meta-types/display/components/ChipFieldDisplay.tsx)
Updated pagination logic and added cursor-based pagination support (packages/twenty-front/src/modules/object-record/hooks/useFindManyRecords.ts, packages/twenty-front/src/modules/object-record/hooks/useFetchAllRecordIds.ts)

_{136 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

…rator

greptile-apps

PR Summary

(updates since last review)

Ensured URLs in LinkDisplay.tsx are absolute by prepending 'https://'
Added 'upgrade-guide' entry to 'Self-Hosting' section in packages/twenty-website/src/content/developers/constants/DocsIndex.ts
Created new upgrade guide in packages/twenty-website/src/content/developers/self-hosting/upgrade-guide.mdx

_{3 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

Weiko added 3 commits July 11, 2024 18:27

Add label identifier to object decorator

1969c0e

fix

87fd98d

add validation

721fe99

Weiko added the -PR: awaiting review label Jul 11, 2024

greptile-apps bot reviewed Jul 11, 2024

View reviewed changes

reorganize code

3a316d7

greptile-apps bot reviewed Jul 11, 2024

View reviewed changes

add TODO

7bb0a27

Weiko commented Jul 11, 2024

View reviewed changes

greptile-apps bot reviewed Jul 11, 2024

View reviewed changes

merge main

9ca022e

greptile-apps bot reviewed Jul 11, 2024

View reviewed changes

FelixMalfait added -PR: awaiting author and removed -PR: awaiting review labels Jul 12, 2024

FelixMalfait reviewed Jul 12, 2024

View reviewed changes

charlesBochet approved these changes Jul 12, 2024

View reviewed changes

add label for all standard objects or fallback to PK

5ea0ef0

greptile-apps bot reviewed Jul 12, 2024

View reviewed changes

add missing FK for identifiers

fab4b56

greptile-apps bot reviewed Jul 12, 2024

View reviewed changes

Weiko added -PR: awaiting review and removed -PR: awaiting author labels Jul 12, 2024

charlesBochet added -PR: awaiting author and removed -PR: awaiting review labels Jul 12, 2024

charlesBochet assigned Weiko Jul 12, 2024

FelixMalfait mentioned this pull request Jul 15, 2024

Data explorer #6159

Closed

Revert "add missing FK for identifiers"

9bc7f02

This reverts commit fab4b56.

greptile-apps bot reviewed Jul 15, 2024

View reviewed changes

fix identifiers type from text to uuid

f302e1c

greptile-apps bot reviewed Jul 15, 2024

View reviewed changes

Merge branch 'main' into add-label-identifier-to-standard-object-deco…

c3ecff6

…rator

Weiko added -PR: awaiting review and removed -PR: awaiting author labels Jul 15, 2024

greptile-apps bot reviewed Jul 15, 2024

View reviewed changes

Merge branch 'main' into add-label-identifier-to-standard-object-deco…

7d10239

…rator

greptile-apps bot reviewed Jul 16, 2024

View reviewed changes

Merge branch 'main' into add-label-identifier-to-standard-object-deco…

e54515f

…rator

greptile-apps bot reviewed Jul 17, 2024

View reviewed changes

charlesBochet merged commit 67e2d5c into main Jul 19, 2024
6 checks passed

charlesBochet deleted the add-label-identifier-to-standard-object-decorator branch July 19, 2024 12:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add label identifier to object decorator #6227

Add label identifier to object decorator #6227

Weiko commented Jul 11, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot left a comment

Weiko Jul 11, 2024

greptile-apps bot left a comment

greptile-apps bot left a comment

FelixMalfait left a comment

FelixMalfait Jul 12, 2024

Weiko Jul 12, 2024

Weiko commented Jul 12, 2024

charlesBochet left a comment

FelixMalfait commented Jul 12, 2024

Weiko commented Jul 12, 2024

greptile-apps bot left a comment

greptile-apps bot left a comment

charlesBochet commented Jul 12, 2024

FelixMalfait commented Jul 12, 2024

FelixMalfait commented Jul 14, 2024

greptile-apps bot left a comment

greptile-apps bot left a comment

greptile-apps bot left a comment

greptile-apps bot left a comment

greptile-apps bot left a comment

Add label identifier to object decorator #6227

Add label identifier to object decorator #6227

Conversation

Weiko commented Jul 11, 2024 • edited Loading

Context

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Weiko Jul 11, 2024

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

FelixMalfait left a comment

Choose a reason for hiding this comment

FelixMalfait Jul 12, 2024

Choose a reason for hiding this comment

Weiko Jul 12, 2024

Choose a reason for hiding this comment

Weiko commented Jul 12, 2024

charlesBochet left a comment

Choose a reason for hiding this comment

FelixMalfait commented Jul 12, 2024

Weiko commented Jul 12, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

charlesBochet commented Jul 12, 2024

FelixMalfait commented Jul 12, 2024

FelixMalfait commented Jul 14, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Weiko commented Jul 11, 2024 •

edited

Loading