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

Sync metadata generate migrations #2864

Merged
merged 11 commits into from
Dec 7, 2023

Conversation

Weiko
Copy link
Member

@Weiko Weiko commented Dec 7, 2023

Context

We are introducing a new way to define standard object-metadata, this is a follow-up from #2807
This PR introduces relation setup using the same class and run the necessary migrations after the sync of metadata.

We will improve the documentation to explain how to add or edit standard objects.

  • Moving standard-objects definition from workspace-manager to a new workspace-sync-metadata module
  • Use this new service in our dev-seed command and our workspace-manager.init method for new workspaces, we don't need to differentiate this code anymore.
  • Update our sync-metadata command yarn command workspace:sync-metadata -w "20202020-1c25-4d02-bf25-6aeccf7ea419" should now call the new service and only update necessary objects/fields and run the migrations accordingly. A few things are not fully implemented yet such as migration updates/delete, I left a few TODO in the code.
  • A first version of a generateDefaultField method that should generate default defaultField for our types.
  • Updated workspaceMigrationFactory to handle composite fields such as FULL_NAME, LINK, etc...
  • Removing the old definition of standard objects / relations in plain object since those are not needed anymore
  • As stated above, all standard-objects should have now been migrated to the new ObjectMetadata classes.
  • Added RelationMetadata decorator for relations and updated the parser accordingly, its logic is executed after the object/fields to ensure we have all the ids available in DB.

Important note: For objects and fields, I'm using their names as the key to run the diff, I'm using a map to simplify the diff and ensure we don't have any issue with ordering. This might not a good idea in the long run because if we want to change the name of an object or a field, the diff will interpret it as a different object/field and will run a CREATE/REMOVE instead of CHANGE action.
Note2: relationMetadata does not contain an isCustom field, we should add it before exposing relations to the API because the sync-metadata service should only affect non-custom entities and we have no easy way to filter them out as of today
Note3: I use stringify sometimes for the diff, this is because we sometimes store jsonb, for example targetColumnMap, defaultValues, options, etc... then I have to parse again before saving in DB, this could be improved in the future for readability

Here is an example of how you can use the new class. Note that extending BaseObjectMetadata will generate default fields such as ['id', 'createdAt', 'updatedAt']

@ObjectMetadata({
  namePlural: 'myObjects',
  labelSingular: 'My Object',
  labelPlural: 'My Objects',
  description: 'An object',
  icon: 'IconAbc'
})
@IsSystem() // If an object has this decorator, all its fields will also be considered as system fields.
export class MyObjectObjectMetadata extends BaseObjectMetadata { // Will create a "myObject" objectMetadata
  @FieldMetadata({
    type: FieldMetadataType.TEXT,
    label: 'Title',
    description: 'Activity title',
    icon: 'IconNotes',
  })
  @IsNullable()
  title: string;

  @FieldMetadata({
    type: FieldMetadataType.TEXT,
    label: 'Type',
    description: 'Object type',
    icon: 'IconCheckbox',
    defaultValue: { value: 'Something' },
  })
  type: string;

  // A ONE_TO_MANY relation
  @FieldMetadata({
    type: FieldMetadataType.RELATION,
    label: 'Fields,
    description: "List of fields",
    icon: 'IconCheckbox',
  })
  @RelationMetadata({
    type: RelationMetadataType.ONE_TO_MANY,
    objectName: 'field',
  })
  fields: object[];

  // A MANY_TO_ONE relation, we don't use @RelationMetadata here because we only store 1 direction
  @FieldMetadata({
    type: FieldMetadataType.RELATION,
    label: 'Owner',
    description: 'Object owner',
    icon: 'IconUserCircle',
    joinColumn: 'authorId', // However we want the joinColumn here for the other direction
  })
  owner: object;
}

Copy link

github-actions bot commented Dec 7, 2023

TODOs/FIXMEs:

  • // TODO: This solution could be improved, using a diff for example, we should not have to delete all metadata and recreate them.: server/src/workspace/workspace-sync-metadata/commands/sync-workspace-metadata.command.ts
  • // TODO: We could create a relation decorator but let's keep it simple for now.: server/src/workspace/workspace-sync-metadata/standard-objects/view-sort.object-metadata.ts
  • // TODO: Sync relationMetadata: server/src/workspace/workspace-manager/workspace-manager.service.ts
  • // TODO: Use transactions: server/src/workspace/workspace-manager/workspace-manager.service.ts
  • // TODO: Create migrations based on diff from above.: server/src/workspace/workspace-manager/workspace-manager.service.ts

Generated by 🚫 dangerJS against 7bec903

@Weiko Weiko force-pushed the c--sync-metadata-generate-migrations branch from 590a3b3 to 8bc5096 Compare December 7, 2023 15:16
@Weiko Weiko changed the title C sync metadata generate migrations Sync metadata generate migrations Dec 7, 2023
@Weiko Weiko marked this pull request as ready for review December 7, 2023 16:26
@@ -41,3 +44,22 @@ export const mapObjectMetadataByUniqueIdentifier = (
return acc;
}, {});
};

export const convertStringifiedFieldsToJSON = (fieldMetadata) => {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe introduce a type: FieldMetadataFormattedForDiff

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @charlesBochet on this

Copy link
Member

@charlesBochet charlesBochet 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! We will keep cleaning the code :)

@charlesBochet charlesBochet merged commit 5efc2f0 into main Dec 7, 2023
9 of 13 checks passed
@charlesBochet charlesBochet deleted the c--sync-metadata-generate-migrations branch December 7, 2023 18:22
Copy link
Member

@magrinj magrinj 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 ! :)

@@ -41,3 +44,22 @@ export const mapObjectMetadataByUniqueIdentifier = (
return acc;
}, {});
};

export const convertStringifiedFieldsToJSON = (fieldMetadata) => {
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @charlesBochet on this

@charlesBochet charlesBochet mentioned this pull request Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants