twenty icon indicating copy to clipboard operation
twenty copied to clipboard

Support custom object renaming

Open ijreilly opened this issue 1 year ago • 29 comments

Scope & Context

Currently, renaming a custom object is not permitted.

Desired behavior

Api name and table name are dependent: if I update the api name, the table name is updated. Label and api names can be independent or not:

  • dependent (sync): updating the label will entice api name update. Direct api name update is not allowed.
  • independent (not sync): updating the label does not update anything else. By default, they are dependent (sync).

Steps:

  1. Add an Advanced Settings option on the Developer Settings page that allows to sync or unsync all labels and API names:

CleanShot 2024-07-15 at 10 53 25

  1. Behavior on an object detail page when this sync setting is activated:
  • C) and D) are deactivated (Read-only)
  • Changing A) always changes B) and D)
  • C) reflects the value of B), regardless of whether B) is overridden or updated by A)
  • C) and D) field Info button (i) should inform the user that he should deactivate the Sync settings in Advanced Developer settings to set a custom API name

CleanShot 2024-07-15 at 10 28 13

  1. Behavior on an object detail page when this sync setting is deactivated:
  • Changing A) updates B)
  • C) & D) are not impacted when A) or B) are updated
  • C) & D) are activated and their value can be updated
  • C) & D) field info button (i) should explain the constraints of the API names
  1. Behavior on the Developer advanced Setting activation and deactivation:
  • Deactivating the toggle should stop the synchronization, the API names should stay what they are.
  • When reactivating the toggle, a for-each loop should iterate over every object to set the API names to match the object labels. C) should be updated with B) and D) should be updated with A)
  1. Object creation

When creating a new object, even if the Sync labels & API names setting is deactivated, the API names should match the labels.

Technical inputs

  1. In discussion: should sync/unsync choice be defined/stored at object or workspace level?
  2. In discussion: should sync/unsync choice allow/prevent api name update in the product only or in the api too?
  3. Table name migration is already handled in our workspace migration runner. Check out renameTable method.

ijreilly avatar May 20 '24 20:05 ijreilly

Disclaimer: This comment was automatically generated with AI. It can give some useful hints as to where to look for, but sometimes it can also give very inaccurate answers so takes this with a pinch of salt :).

To support custom object renaming, follow these steps:

  1. Locate Update Object Endpoint: Modify the update object endpoint in src/controllers/objectController.js.

  2. Implement Table Name Migration: Use the renameTable method in src/services/databaseService.js to rename the table associated with the custom object.

  3. Update Foreign Key Columns: Use the renameColumn method in src/services/databaseService.js to rename foreign key columns referencing the custom object.

  4. Example Code:

// src/controllers/objectController.js
const { renameTable, renameColumn } = require('../services/databaseService');

async function updateObject(req, res) {
  const { oldName, newName } = req.body;
  await renameTable(oldName, newName);
  await renameColumn('foreign_table', `${oldName}_id`, `${newName}_id`);
  // other update logic
}

Ensure to handle errors and edge cases appropriately.

References

greptile-apps[bot] avatar May 20 '24 20:05 greptile-apps[bot]

Issue needs to be refined / we said label and api names weren't necessarily correlated. All objects labels could be renamed. I think we introduce an advanced mode to help with that distinction - not sure how we "decorrelate the two" (@Bonapara knows / can decide whether to close and point to other issues)

FelixMalfait avatar Jul 10 '24 09:07 FelixMalfait

Related to https://github.com/twentyhq/twenty/issues/6145

Bonapara avatar Jul 10 '24 09:07 Bonapara

@Bonapara Do you want me to work on this as part of 6145? I would love to.

Faisal-imtiyaz123 avatar Jul 10 '24 17:07 Faisal-imtiyaz123

To support custom object renaming, follow these steps:

  1. Update updateOneObject method in object-metadata.service.ts:

    • Add logic to handle table name and foreign key column name migrations.
    public async updateOneObject(
      input: UpdateOneObjectInput,
      workspaceId: string,
    ): Promise<ObjectMetadataEntity> {
      validateObjectMetadataInputOrThrow(input.update);
    
      const updatedObject = await super.updateOne(input.id, input.update);
    
      if (input.update.nameSingular || input.update.namePlural) {
        await this.handleObjectRenaming(input.id, input.update, workspaceId);
      }
    
      await this.workspaceCacheVersionService.incrementVersion(workspaceId);
    
      return updatedObject;
    }
    
  2. Implement handleObjectRenaming method in object-metadata.service.ts:

    • Create migrations for renaming table and foreign key columns.
    private async handleObjectRenaming(
      objectId: string,
      update: UpdateObjectPayload,
      workspaceId: string,
    ) {
      const objectMetadata = await this.findOneOrFailWithinWorkspace(workspaceId, { where: { id: objectId } });
    
      const migrations = buildMigrationsForCustomObjectRenaming(objectMetadata, update);
    
      await this.workspaceMigrationService.createCustomMigration(
        generateMigrationName(`rename-${objectMetadata.nameSingular}`),
        workspaceId,
        migrations,
      );
    
      await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrations(workspaceId);
    }
    
  3. Create buildMigrationsForCustomObjectRenaming utility function:

    • Define the necessary migrations for renaming tables and foreign key columns.
    export const buildMigrationsForCustomObjectRenaming = (
      objectMetadata: ObjectMetadataEntity,
      update: UpdateObjectPayload,
    ): WorkspaceMigrationTableAction[] => {
      const migrations: WorkspaceMigrationTableAction[] = [];
    
      if (update.nameSingular) {
        migrations.push({
          name: computeObjectTargetTable(objectMetadata),
          action: WorkspaceMigrationTableActionType.RENAME,
          newName: computeObjectTargetTable({ ...objectMetadata, nameSingular: update.nameSingular }),
        });
      }
    
      // Add logic for renaming foreign key columns
    
      return migrations;
    };
    
  4. Update checkIfFieldIsEditable method in before-update-one-object.hook.ts:

    • Allow renaming of custom objects.
    private checkIfFieldIsEditable(
      update: UpdateObjectPayload,
      objectMetadata: ObjectMetadataEntity,
    ) {
      if (!objectMetadata.isCustom) {
        // Existing checks for non-custom objects
      }
    
      // Allow renaming for custom objects
    }
    

References

/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts /packages/twenty-server/src/engine/metadata-modules/object-metadata/hooks/before-update-one-object.hook.ts /packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/build-workspace-migrations-for-custom-object.util.ts /packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-migrations-for-custom-object-relations.util.ts /packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-remote-object-relations.util.ts /packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts /packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/build-migrations-for-custom-object-relations.util.ts /packages/twenty-server/src/engine/metadata-modules/remote-server/remote-table/remote-table.service.ts /packages/twenty-server/src/engine/metadata-modules/remote-server/remote-table/foreign-table/foreign-table.service.ts /packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/workspace-migration-relation.factory.ts

About Greptile

This response provides a starting point for your research, not a precise solution.

Help us improve! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

Ask Greptile · Edit Issue Bot Settings

greptile-apps[bot] avatar Jul 11 '24 09:07 greptile-apps[bot]

@Bonapara I think we need to clarify if we want to migrate the table names or not upon renaming the objects. (For now that is required for the application to work correctly, beyond the desire of consistency.)

@Faisal-imtiyaz123 thanks for your interest, lets wait until this is clarified !

ijreilly avatar Jul 11 '24 09:07 ijreilly

image

Everything should be deducted from the plural label at creation. The singular label should automatically update when the plural label is updated, even if it was overwritten in advanced settings.

The API names should not be updated when the labels are updated.

@FelixMalfait Does this look good to you too?

Bonapara avatar Jul 11 '24 10:07 Bonapara

@Bonapara the "api name" needs to follow a series of constraints: being camelCased, only contain latin characters etc .(from the product we do the transliteration from the label). I think some hints may be useful here, maybe a front-end validation with hints as we do here: https://github.com/twentyhq/twenty/issues/6094

Also just my opinion but I am not sure whether this section should show directly for all users, or be semi-hidden behind an expandable section. Thinking of cloud non-tech users here

ijreilly avatar Jul 11 '24 10:07 ijreilly

23af7638932672bcd6b8b976daaecdcc46a8af7c00a75b64611a389b82b07c1d

@Bonapara the "api name" needs to follow a series of constraints: being camelCased, only contain latin characters etc .(from the product we do the transliteration from the label). I think some hints may be useful here, maybe a front-end validation with hints as we do here: #6094

@ijreilly just added an info icon at the end of the input. We could also use a helper under the field, but it was redundant.

Also just my opinion but I am not sure whether this section should show directly for all users, or be semi-hidden behind an expandable section. Thinking of cloud non-tech users here

I think it should be hidden. It's under the Advanced Mode settings.

Bonapara avatar Jul 11 '24 10:07 Bonapara

I am not 100% convinced by this feature :

  1. if the label change is what is displayed in regular mode but does not change the apiName (nameSingular and namePlural in database), for most of the users the label will diverge from the database model which is an issue for readability using tools such as metabase. I feel that by default both should go together and we would need to introduce a way to in advance mode to: override the labels OR to use the apiNames.

  2. For labels we have singular and plural, for names we should have the same

charlesBochet avatar Jul 13 '24 09:07 charlesBochet

For 2. @charlesBochet did you see the screenshot in the thread?

(The idea would be to deduct the singular name for both the API and Labels from the plural label input)

Bonapara avatar Jul 13 '24 09:07 Bonapara

@charlesBochet I also didn't feel 100% good about the current state. Maybe one option to avoid introducing the overwrite boolean is to build a function that detects automatically if it's been 'unsychronized' for each field? From a UX perspective we could also display the predicted value as "suggested: xxx" if the field has been unsynchronized @Bonapara, that way if a user wants to "re-sync" they can set the suggested value. Even if the fields are not displayed in non-advanced mode, they would still be sent.

It puts the weight on the frontend which isn't great, but I don't think it's that bad, if someone uses the metadata api to update fields, they are tech-saavy enough to be on their own.

Edit : Just an example to be clear:

  • If I create a field, when I update the name it updates the 3 other fields on the frontend as I type (even if they are not visible if I'm not in advanced mode)
  • If I update one API name then it becomes "unsynchronized" and is not updated automatically on the frontend automatically on the frontend. If I want it to enter synchronization mode again, I should set it's value to the suggested value

FelixMalfait avatar Jul 13 '24 09:07 FelixMalfait

@FelixMalfait, I still worry about this unsynchronization not being shown in non-advanced mode. I feel the source should actually be the apiName (aka nameSingular + namePlural) and the overwrite should be the label (aka labelSingular + labelPlural).

Regarding your suggestion, I like the the suggested placeholder if unsynced and if they put the same value the FE considers it synced again. Let's see what @Bonapara thinks

charlesBochet avatar Jul 13 '24 10:07 charlesBochet

@charlesBochet In non-advanced mode we could show a small warning that indicates it's not going to be synced or something like that (@Bonapara what do you think?)

I disagree with apiName being the source of truth. Label works well for me. You can go from Label -> API Name but can't go from API Name to label (e.g. CamelCase "HelloC" and "Hello C" give the same output) . Also from a user-perspective, in non-advanced mode, they should care about label only.

FelixMalfait avatar Jul 14 '24 13:07 FelixMalfait

We could introduce the following system:

  1. An Advanced Settings option on the Developer Settings page that allows to sync or unsync all labels and API names:

CleanShot 2024-07-15 at 10 53 25

  1. Behavior on an object detail page when this sync setting is activated:
  • C) and D) are deactivated (Read-only)
  • Changing A) always changes B) and D)
  • C) reflects the value of B), regardless of whether B) is overridden or updated by A)
  • C) and D) field Info button (i) should inform the user that he should deactivate the Sync settings in Advanced Developer settings to set a custom API name

CleanShot 2024-07-15 at 10 28 13

  1. Behavior on an object detail page when this sync setting is deactivated:
  • Changing A) updates B)
  • C) & D) are not impacted when A) or B) are updated
  • C) & D) are activated and their value can be updated
  • C) & D) field info button (i) should explain the constraints of the API names
  1. Behavior on the Developer advanced Setting activation and deactivation:
  • Deactivating the toggle should stop the synchronization, the API names should stay what they are.
  • When reactivating the toggle, a for-each loop should iterate over every object to set the API names to match the object labels. C) should be updated with B) and D) should be updated with A)
  1. Object creation

When creating a new object, even if the Sync labels & API names setting is deactivated, the API names should match the labels.

Bonapara avatar Jul 15 '24 08:07 Bonapara

Good for me @Bonapara! 👌

FelixMalfait avatar Jul 15 '24 08:07 FelixMalfait

Here is the GitStart Ticket for this issue: https://clients.gitstart.com/twenty/5449/tickets/TWNTY-5491

gitstart-app[bot] avatar Jul 31 '24 08:07 gitstart-app[bot]

Blocked by https://github.com/twentyhq/twenty/issues/6147 @gitstart-twenty FYI !

ijreilly avatar Aug 29 '24 13:08 ijreilly

Comment from @charlesBochet:

_@gitstart-twenty @ijreilly @Bonapara I'm not a big fan of:

storing this at workspace level (we don't have a storage for it right now beside workspace table and keyValuePairs table but it feels off to put it in both table) having the for loop checking and updating all objects, this is a slow and risky operation and it can fail, it can be challenging to rollback in case of issues and display it properly to the user if we choose to go async Could we instead keep this on the Object Edit / Field Edit page and keep it field or object level?_

ijreilly avatar Sep 16 '24 11:09 ijreilly

We have two decisions to make

  1. should sync/unsync choice be defined/stored at object or workspace level? --> I agree with @charlesBochet above
  2. should sync/unsync choice allow/prevent api name update in the product only or in the api too? --> I think product and api should be iso

@Bonapara @charlesBochet @FelixMalfait

ijreilly avatar Sep 16 '24 12:09 ijreilly

should sync/unsync choice allow/prevent api name update in the product only or in the api too? --> I think product and api should be iso

Agreed on Iso!

dependent (sync): updating the label will entice api name update. Direct api name update is not allowed.

Bonapara avatar Sep 16 '24 12:09 Bonapara

Hello @ijreilly , we would like to confirm a few points:

  • the sync/unsync will only happens on the app and not in the API, right?
  • regarding the sync/unsync choice, where should we save this information at the workspace level?

storing this at workspace level (we don't have a storage for it right now beside workspace table and keyValuePairs table but it feels off to put it in both table)

  • what triggers the sync/unsync choice is the advanced settings button, but what happens if the user turn on the toggle to see another feature, in another page for example? should we update sync/unsync choice regardless of where the user is? It seems more appropriate just update if the user is editing a custom object, do you agree?

gitstart-twenty avatar Sep 27 '24 14:09 gitstart-twenty

Hi @gitstart-twenty

  1. Not sure to understand your question, but the sync/unsync does regard a back-end behaviour. Today each object has a labelSingular and a labelPlural (ex: "Companies" or "House listings" as label plural) and a namePlural that is the camel-cased version of the label in latin characters (ex: "companies" or "houseListings"). The object nameSingular gives its name to the table in database (prefixed with a _ for custom object: ex table for "House Listings" is _houseListing) where the data is stored on the individual workspace schema. (You can observe all this creating a custom object and inspecting your database's metadata.objectMetadata table and the custom table created in your workspace schema). So far in the product the name is directly inferred from the label in the back-end, there is no "name" input. It is not possible to update the label. We want to add the possibility for the user to update both the label and the name, and to be able to do so independently: if the sync is disabled, it is possible to update solely the label (from "House Listings" to "Listings" while leaving houseListing as name) or solely the name (from houseListings to listings while leaving "House Listings" as label). if the sync is enabled, it is only possible to update the label in the product, and the name will be inferred in the back-end from it (from "House Listings" to "Listings" as label plural will update the name plural to "_listings").

  2. From the conversation above, the sync/unsync status should be stored at object level, so you should add a column areLabelAndNameSync on objectMetadata table I think (@charlesBochet right?). So @Bonapara 's design should be adapted to move the case below to objects' individual settings pages: Capture d’écran 2024-09-30 à 09 26 44

  3. What triggers the sync/unsync choice is not the advanced settings button. The advanced settings button only fulfills a hide/display role on the product. The sync/unsync choice is stored at object level and can be updated with this toggle, that will trigger an objectMetadata update (calling object-metadata.service) Capture d’écran 2024-09-30 à 09 26 44

I hope this helps !

ijreilly avatar Sep 30 '24 07:09 ijreilly

Thanks for the clarification @ijreilly 🤝

@Bonapara , could you provide the Figma link for this page?

gitstart-twenty avatar Sep 30 '24 13:09 gitstart-twenty

https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=34855-565832&node-type=frame&t=YZppWDu8cDmzxDmJ-11

Bonapara avatar Sep 30 '24 13:09 Bonapara

Hello @Bonapara image

are these settings and fields tabs part of this ticket? Also, we could not see the toggle below in the design, is it on another Figma page?

image

gitstart-twenty avatar Sep 30 '24 17:09 gitstart-twenty

Hi @gitstart-twenty yes it is.

You're right, here is the updated design:

image

https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=34855-565832&node-type=frame&t=tQMdDCnS2jhEWHND-11

Bonapara avatar Oct 01 '24 11:10 Bonapara

Hello @Bonapara about the UI changes, the action button is different when the design is on the field tab, but there is no save button for the settings tab image

image

we are assuming that the save button would appear as the action button if the user is on the settings tabs, is this correct? but if this is correct how about the cancel button?

gitstart-twenty avatar Oct 15 '24 15:10 gitstart-twenty

Hi @gitstart-twenty, we should do some auto-save for the Object Settings Tab edits and not display a save button at the top right.

Bonapara avatar Oct 15 '24 16:10 Bonapara