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

Fix 'name' column wrongly added in standard objects #7428

Merged
merged 3 commits into from
Oct 4, 2024

Conversation

Weiko
Copy link
Member

@Weiko Weiko commented Oct 4, 2024

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).

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 pull request addresses an issue with the 'name' column being incorrectly added to standard objects in the Twenty application.

  • Removes 'name' column from default columns in custom-table-default-column.util.ts
  • Adds 'name' column specifically for custom objects in build-migrations-for-custom-object-relations.util.ts
  • Modifies WorkspaceMigrationRunnerService to allow column creation during table creation
  • Ensures standard objects use their own specified labelIdentifierFieldMetadata
  • Improves flexibility in the migration process for creating complex table structures

3 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings

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.

great work !!

@Weiko Weiko merged commit 2f223f3 into main Oct 4, 2024
5 of 8 checks passed
@Weiko Weiko deleted the c--fix-name-column-wrongly-added-in-standard-objects branch October 4, 2024 16:31
Weiko added a commit that referenced this pull request Oct 7, 2024
Following this #7428 we now need
to fix existing workspaces thanks to this migration command.

This command will fetch all standard objects and their fields then
filter out tables that don't have that column OR objects that have an
existing fieldMetadata "name" of type TEXT and delete the rest.
harshit078 pushed a commit to harshit078/twenty that referenced this pull request Oct 14, 2024
## 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).
harshit078 pushed a commit to harshit078/twenty that referenced this pull request Oct 14, 2024
Following this twentyhq#7428 we now need
to fix existing workspaces thanks to this migration command.

This command will fetch all standard objects and their fields then
filter out tables that don't have that column OR objects that have an
existing fieldMetadata "name" of type TEXT and delete the rest.
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.

2 participants