Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add field isLabelSyncedWithName #8829

Merged
merged 12 commits into from
Dec 3, 2024
Merged

Conversation

Weiko
Copy link
Member

@Weiko Weiko commented Dec 2, 2024

Context

The recent addition of object renaming introduced issues with enum names. Enum names should follow the pattern ${schemaName}.${tableName}_${columnName}_enum. To address this, and to allow users to customize the API name (which is included in the enum name, columnName), this PR implements behavior similar to object renaming by introducing a isLabelSyncedWithName boolean.

Screenshot 2024-12-02 at 11 58 49 Screenshot 2024-12-02 at 11 58 39

@Weiko Weiko marked this pull request as ready for review December 2, 2024 10:51
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR introduces a new boolean field isLabelSyncedWithName to control synchronization between field labels and API names, particularly for enum naming patterns.

  • Added isLabelSyncedWithName column to metadata.fieldMetadata table with default value true in /packages/twenty-server/src/database/typeorm/metadata/migrations/1733072126592-addIsLabelSyncedWithNameToFieldMetadata.ts
  • Added advanced settings section with API Name input and sync toggle in /packages/twenty-front/src/modules/settings/data-model/fields/forms/components/SettingsDataModelFieldIconLabelForm.tsx
  • Modified workspace-migration-enum.service.ts to handle enum type name retrieval before column rename operations
  • Removed automatic name computation from label in formatFieldMetadataItemInput.ts to support independent API naming
  • Updated GraphQL schema, queries, and types across multiple files to support the new field synchronization functionality

15 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -187,6 +187,7 @@ export type CreateFieldInput = {
icon?: InputMaybe<Scalars['String']['input']>;
isActive?: InputMaybe<Scalars['Boolean']['input']>;
isCustom?: InputMaybe<Scalars['Boolean']['input']>;
isLabelSyncedWithName: Scalars['Boolean']['input'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: isLabelSyncedWithName is required in CreateFieldInput but optional in UpdateFieldInput - this inconsistency could cause issues

type SettingsDataModelFieldIconLabelFormProps = {
disabled?: boolean;
fieldMetadataItem?: FieldMetadataItem;
maxLength?: number;
};

const infoCircleElementId = 'info-circle-id';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: infoCircleElementId should include a more unique prefix to avoid potential ID conflicts

Comment on lines +150 to +151
@Field()
isLabelSyncedWithName?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Field should be marked as nullable since it's optional

Copy link
Collaborator

@ijreilly ijreilly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capture d’écran 2024-12-02 à 16 03 17

I think there is too much spacing between Icon and Name section and Values. i remember i had the same issue with object form


public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "metadata"."fieldMetadata" ADD "isLabelSyncedWithName" boolean NOT NULL DEFAULT true`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this defaults to false for objects, let's do the same?

@Weiko Weiko merged commit 3c7805c into main Dec 3, 2024
19 checks passed
@Weiko Weiko deleted the c--add-field-metadata-sync-label-and-name branch December 3, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants