Support custom object renaming #7504

gitstart-app · 2024-10-08T14:36:21Z

This PR was created by GitStart to address the requirements from this ticket: TWNTY-5491.
This ticket was imported from: TWNTY-5491

Description

How To Test:\

Reset db using npx nx database:reset twenty-server on this PR
Run both backend and frontend
Navigate to settings/data-model/objects/ page
Select a Custom object from the list or create a new Custom object
Navigate to custom object details page and click on edit button
Finally edit the object details.

Issues and bugs
The Typecheck is failing but we could not see this error locally
There is a bug after updating the label of a custom object. View title is not updated till refreshing the page. We could not find a consistent way to update this, should we reload the page after editing an object?

### Demo

https://www.loom.com/share/64ecb57efad7498d99085cb11480b5dd?sid=28d0868c-e54f-454d-8432-3f789be9e2b7

Refs

#5491

greptile-apps

PR Summary

This PR implements custom object renaming functionality in the Twenty application. Key changes include:

Added 'areLabelAndNameSync' field to ObjectMetadataEntity for controlling label and API name synchronization
Updated SettingsDataModelObjectAboutForm with new UI elements for advanced settings and API name fields
Modified ObjectMetadataService to handle object renaming and related metadata updates
Introduced SyncObjectLabelAndNameToggle component for toggling synchronization
Updated validation schemas and GraphQL queries to support the new renaming feature

_{20 file(s) reviewed, 10 comment(s)}
_{Edit PR Review Bot Settings}

packages/twenty-front/src/modules/settings/data-model/objects/SyncObjectLabelAndNameToggle.tsx

greptile-apps · 2024-10-08T14:42:40Z

...rc/modules/settings/data-model/objects/forms/components/SettingsDataModelObjectAboutForm.tsx

+  const areLabelAndNameSync = watch('areLabelAndNameSync');
+  const labelSingular = watch('labelSingular');
+  const labelPlural = watch('labelPlural');
+  const apiNameTooltipText = areLabelAndNameSync
+    ? 'Deactivate "Synchronize Objects Labels and API Names" to set a custom API name'
+    : 'Input must be in camel case and cannot start with a number';


style: Consider memoizing these watch calls to optimize performance

greptile-apps · 2024-10-08T14:42:41Z

...rc/modules/settings/data-model/objects/forms/components/SettingsDataModelObjectAboutForm.tsx

+  useEffect(() => {
+    if (areLabelAndNameSync) {
+      labelSingular &&
+        setValue(
+          'nameSingular',
+          computeMetadataNameFromLabelOrThrow(labelSingular),
+          { shouldDirty: true },
+        );
+    }
+  }, [areLabelAndNameSync, labelSingular, setValue]);


style: Consider combining this effect with the previous one to reduce code duplication

greptile-apps · 2024-10-08T14:42:42Z

...rc/modules/settings/data-model/objects/forms/components/SettingsDataModelObjectAboutForm.tsx

+          )}
+        />
+      </StyledSectionWrapper>
+      <AnimatePresence>


style: Large conditional render block may impact performance. Consider extracting to a separate component

greptile-apps · 2024-10-08T14:50:36Z

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

+  private async updateObjectNamesandLabels(
+    oldObject: ObjectMetadataEntity,
+    updatedObject: ObjectMetadataEntity,
+    input: UpdateOneObjectInput,
+  ) {


syntax: Method name has a typo: 'updateObjectNamesandLabels' should be 'updateObjectNamesAndLabels'

greptile-apps · 2024-10-08T14:50:37Z

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

+      );
+    }
+
+    if (updatedObject.areLabelAndNameSync) {


logic: This block doesn't handle the case where both label and name are updated simultaneously. Consider adding a check for this scenario

greptile-apps · 2024-10-08T14:50:38Z

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

+        updatedObjectWithSyncedName,
+      );
+    } else {
+      if (input.update.nameSingular || input.update.namePlural) {


logic: Similar to the previous comment, this block doesn't handle simultaneous updates to both singular and plural names

greptile-apps · 2024-10-08T14:51:25Z

...ver/src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util.ts

@@ -40,6 +43,8 @@ const reservedKeywords = [
  'addresses',
 ];

+const METADATA_NAME_VALID_PATTERN = /^[a-zA-Z][a-zA-Z0-9]*$/;


style: Consider using a more descriptive name for this regex constant, e.g., VALID_METADATA_NAME_PATTERN

greptile-apps · 2024-10-08T14:51:26Z

...ver/src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util.ts

+  );
+
+  if (!formattedString.match(METADATA_NAME_VALID_PATTERN)) {
+    throw new Error(`"${string}" is not a valid name`);


style: This error message could be more specific about why the name is invalid

greptile-apps · 2024-10-08T14:51:27Z

...ver/src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util.ts

+  if (formattedString.match(METADATA_NAME_VALID_PATTERN) !== null) {
+    return toCamelCase(formattedString);
+  }


logic: This condition might allow some invalid names through. Consider using a stricter check

greptile-apps

PR Summary

(updates since last review)

This PR continues the implementation of custom object renaming functionality in the Twenty application. The main changes include:

Added 'areLabelAndNameSync' property to various test files and schemas
Updated GraphQL queries and mutations to support the new renaming feature
Modified ObjectMetadataService to handle object renaming and related metadata updates

Key points to highlight:

The 'areLabelAndNameSync' property has been added to multiple test files, reflecting the new feature for syncing labels and API names in custom objects
The settingsCreateObjectInputSchema and settingsUpdateObjectInputSchema have been updated to include new fields for custom object renaming
The ObjectMetadataService has been modified to improve handling of object name and label updates

The changes align with the requirements outlined in the related issue, particularly in supporting independent API names and labels for custom objects.

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

greptile-apps · 2024-10-08T18:15:04Z

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

+      );
+    }
+
+    if (updatedObject.areLabelAndNameSync) {


logic: This block doesn't handle the case where both label and name are updated simultaneously. Consider adding a check for this scenario

greptile-apps · 2024-10-08T18:15:05Z

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

+        updatedObjectWithSyncedName,
+      );
+    } else {
+      if (input.update.nameSingular || input.update.namePlural) {


logic: Similar to the previous comment, this block doesn't handle simultaneous updates to both singular and plural names

ijreilly · 2024-10-09T20:24:07Z

indicated on discord as actually still in internal review, @gitstart-twenty can you confirm?

gitstart-twenty · 2024-10-10T07:13:25Z

indicated on discord as actually still in internal review, @gitstart-twenty can you confirm?

Hi @ijreilly, this PR is ready for your review. We have clarified it here

ijreilly

Hi, I have noticed some important errors/unexpected behaviours while testing

When advanced settings are on, if label plural is updated, it updates the label singular, I am not sure this is expected? (see video)
When advanced settings are on, if I am only updating the label singular, it says object already exists, probably because it tries to save the object with the same name that is already taken (i suppose) (also in the video)
Label updates does not seem to work, I updated it and still see the old "Rockets" label everywhere

Can you please make sure to fully test the feature:

update of any of the labelSingular, labelPlural, nameSingular, namePlural independently works: check on the app + in the database (fieldMetadata, name of columns etc)
no weird behaviour on the form like unexpected dependencies between form fields (try updating a field then another, hiding and showing advanced mode, etc.)

Let us know if we can help!

errors1and2.mov

ijreilly · 2024-10-10T09:03:55Z

packages/twenty-front/src/generated-metadata/graphql.ts

@@ -189,6 +189,7 @@ export type CreateFieldInput = {
 };

 export type CreateObjectInput = {
+  areLabelAndNameSync?: InputMaybe<Scalars['Boolean']['input']>;


could we rename to shouldSyncLabelAndName?

packages/twenty-front/src/modules/settings/data-model/objects/SyncObjectLabelAndNameToggle.tsx

ijreilly · 2024-10-10T09:09:17Z

...rc/modules/settings/data-model/objects/forms/components/SettingsDataModelObjectAboutForm.tsx

+  const { contentRef, motionAnimationVariants } = useExpandedHeightAnimation(
+    isAdvancedModeEnabled,
+  );
+  const infoCircleElementId = `info-circle-id-${Math.random().toString(36).slice(2)}`;


I see this is used elsewhere but why is this id computed like this?

we were using this to avoid duplicated ids, but the duplication is not happening, so we will update this

ijreilly · 2024-10-10T09:30:49Z

...ty-server/src/engine/metadata-modules/object-metadata/hooks/before-update-one-object.hook.ts

@@ -111,7 +111,8 @@ export class BeforeUpdateOneObject<T extends UpdateObjectPayload>
  ) {
    if (
      update.nameSingular &&
-      update.nameSingular !== objectMetadata.nameSingular
+      update.nameSingular !== objectMetadata.nameSingular &&
+      !objectMetadata.isCustom


I am not sure about this condition here. Shouldn't it be
if (update.nameSingular && areLabelAndNameSync && (update.nameSingular !== objectMetadata.nameSingular)) ? this check only being valid if label and name are sync

ijreilly · 2024-10-10T09:31:54Z

...ty-server/src/engine/metadata-modules/object-metadata/hooks/before-update-one-object.hook.ts

@@ -120,22 +121,28 @@ export class BeforeUpdateOneObject<T extends UpdateObjectPayload>

    if (
      update.labelSingular &&
-      update.labelSingular !== objectMetadata.labelSingular
+      update.labelSingular !== objectMetadata.labelSingular &&
+      !objectMetadata.isCustom
    ) {
      throw new BadRequestException(


Actually with this feature we want to enable udpate of label and names !

with the changes in this file we are enabling the update but only for custom objects

ijreilly · 2024-10-10T09:48:45Z

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

@@ -441,7 +445,29 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
  ): Promise<ObjectMetadataEntity> {
    validateObjectMetadataInputOrThrow(input.update);

-    const updatedObject = await super.updateOne(input.id, input.update);
+    const oldObject = await this.objectMetadataRepository.findOne({


Suggested change

const oldObject = await this.objectMetadataRepository.findOne({

const existingObjectMetadata = await this.objectMetadataRepository.findOne({

ijreilly · 2024-10-10T09:49:32Z

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

+    oldObject: ObjectMetadataEntity,
+    updatedObject: ObjectMetadataEntity,
+  ) {
+    const oldTableName = computeObjectTargetTable(oldObject);


Suggested change

const oldTableName = computeObjectTargetTable(oldObject);

const existingTableName = computeObjectTargetTable(oldObject);

ijreilly · 2024-10-10T10:15:10Z

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

+        name: `${oldObject.nameSingular}Id`,
+      };
+
+      const fieldWithNonCustomRelation =


Suggested change

const fieldWithNonCustomRelation =

const fieldWihStandardRelation =

gitstart-twenty · 2024-10-10T17:26:24Z

Hello @ijreilly
regarding your comment about the label singular being synced with the label plural, this is mentioned on the issue. Also, if the advanced settings are off, the user can only see the label plural and we need to update the label singular as well

ijreilly · 2024-10-14T17:11:50Z

Hi @gitstart-twenty, thanks for your work I will take it from here !

charlesBochet · 2024-10-21T19:28:51Z

I'm a bit overwhelmed, @Weiko will take the review on this one

Weiko

LGTM

Support custom object renaming

0dd3ed1

greptile-apps bot reviewed Oct 8, 2024

View reviewed changes

gitstart-twenty marked this pull request as draft October 8, 2024 16:13

charlesBochet added -PR: wip -PR: awaiting author labels Oct 8, 2024

charlesBochet assigned ijreilly Oct 8, 2024

gitstart-twenty added 2 commits October 8, 2024 20:36

fix ci checks

86fffc3

fix object-metadata.service.ts lint

283431e

gitstart-twenty added -PR: awaiting review and removed -PR: wip -PR: awaiting author labels Oct 8, 2024

gitstart-twenty marked this pull request as ready for review October 8, 2024 18:03

greptile-apps bot reviewed Oct 8, 2024

View reviewed changes

ijreilly added -PR: awaiting author and removed -PR: awaiting review labels Oct 9, 2024

gitstart-twenty added -PR: awaiting review and removed -PR: awaiting author labels Oct 10, 2024

Merge branch 'main' of github.com:twentyhq/twenty into TWNTY-5491

101ca97

ijreilly reviewed Oct 10, 2024

View reviewed changes

ijreilly added -PR: awaiting author and removed -PR: awaiting review labels Oct 10, 2024

gitstart-twenty and others added 3 commits October 11, 2024 20:27

Merge branch 'main' into TWNTY-5491

18b0f11

update UI behavior, rename variables and new column name

304f7df

fix lint issues

0fcc21b

gitstart-twenty requested a review from ijreilly October 11, 2024 23:45

gitstart-twenty added -PR: awaiting review and removed -PR: awaiting author labels Oct 11, 2024

Merge branch 'TWNTY-5491' of github.com:twentyhq/twenty into TWNTY-5491

eb10349

ijreilly added 10 commits October 15, 2024 14:59

WIP

3586bf7

wip

acc86d6

Update form schema and transform values if needed

6039816

refactor object metadata service

a90f56e

Refactor form

83a5882

Merge branch 'main' of github.com:twentyhq/twenty into TWNTY-5491

3b30190

set default shouldSync value to false for existing objects

680ed90

Finish merge

2aca357

generate metadata

a5010a6

Minor fixes and code quality improvements

85da3af

ijreilly assigned charlesBochet Oct 16, 2024

charlesBochet added 2 commits October 17, 2024 19:40

Merge branch 'main' into TWNTY-5491

319542d

Fix tests

54bb45a

charlesBochet force-pushed the main branch from 1c484d5 to 2f72e22 Compare October 21, 2024 11:19

charlesBochet assigned Weiko and unassigned charlesBochet Oct 21, 2024

Weiko and others added 6 commits October 23, 2024 15:53

Merge branch 'main' into TWNTY-5491

4a68d29

fix typo

e24827f

fix after rebase

326cd92

Forbid update of object to same singular and plural names

8b9a5b8

fix timeline activities and view update

f9c33f9

Merge branch 'main' into TWNTY-5491

aefbe6c

Weiko approved these changes Oct 24, 2024

View reviewed changes

Weiko enabled auto-merge (squash) October 24, 2024 11:49

Weiko merged commit 414f2ac into main Oct 24, 2024
18 checks passed

Weiko deleted the TWNTY-5491 branch October 24, 2024 11:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support custom object renaming #7504

Support custom object renaming #7504

gitstart-app bot commented Oct 8, 2024 •

edited by gitstart-twenty

Loading

greptile-apps bot left a comment

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

greptile-apps bot left a comment

greptile-apps bot Oct 8, 2024

greptile-apps bot Oct 8, 2024

ijreilly commented Oct 9, 2024

gitstart-twenty commented Oct 10, 2024

ijreilly left a comment •

edited

Loading

ijreilly Oct 10, 2024

ijreilly Oct 10, 2024

gitstart-twenty Oct 10, 2024

ijreilly Oct 10, 2024

ijreilly Oct 10, 2024

gitstart-twenty Oct 10, 2024

ijreilly Oct 10, 2024

ijreilly Oct 10, 2024

ijreilly Oct 10, 2024

gitstart-twenty commented Oct 10, 2024

ijreilly commented Oct 14, 2024 •

edited

Loading

charlesBochet commented Oct 21, 2024

Weiko left a comment

	const oldObject = await this.objectMetadataRepository.findOne({
	const existingObjectMetadata = await this.objectMetadataRepository.findOne({

	const oldTableName = computeObjectTargetTable(oldObject);
	const existingTableName = computeObjectTargetTable(oldObject);

	const fieldWithNonCustomRelation =
	const fieldWihStandardRelation =

Support custom object renaming #7504

Support custom object renaming #7504

Conversation

gitstart-app bot commented Oct 8, 2024 • edited by gitstart-twenty Loading

Description

Refs

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 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 Oct 8, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 8, 2024

Choose a reason for hiding this comment

ijreilly commented Oct 9, 2024

gitstart-twenty commented Oct 10, 2024

ijreilly left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitstart-twenty commented Oct 10, 2024

ijreilly commented Oct 14, 2024 • edited Loading

charlesBochet commented Oct 21, 2024

Weiko left a comment

Choose a reason for hiding this comment

gitstart-app bot commented Oct 8, 2024 •

edited by gitstart-twenty

Loading

ijreilly left a comment •

edited

Loading

ijreilly commented Oct 14, 2024 •

edited

Loading