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

Refactor standard relations update at custom object renaming #8638

Merged
merged 4 commits into from
Nov 21, 2024

Conversation

ijreilly
Copy link
Collaborator

Refactoring update of standard relations when a custom object is renamed, after observing occasional issues.

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

Refactored standard relations update logic during custom object renaming to improve reliability and clarity of metadata handling.

  • Extracted foreign key metadata generation into new utility buildNameLabelAndDescriptionForForeignKeyFieldMetadata.util.ts for consistent naming patterns
  • Added settings: { isForeignKey: true } property in relation-metadata.service.ts to properly identify foreign key fields
  • Improved precision of relation updates in object-metadata-migration.service.ts by adding workspaceId and objectMetadataId checks
  • Renamed createRelationsUpdatesMigrations to createStandardRelationsUpdatesMigrations for better clarity of purpose
  • Added proper field metadata updates for name, label and description during object renaming operations

5 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 166 to 177
await this.fieldMetadataRepository.update(
{
name: existingObjectMetadata.nameSingular,
label: existingObjectMetadata.labelSingular,
objectMetadataId: relatedObject.id,
workspaceId: workspaceId,
},
{
name: updatedObjectMetadata.nameSingular,
label: updatedObjectMetadata.labelSingular,
},
);
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: This update could affect multiple records unintentionally since it's not scoped to a specific field type or relation

}) => {
const name = `${targetObjectNameSingular}Id`;
const label = `${targetObjectLabelSingular} ID (foreign key)`;
const description = `${relatedObjectLabelSingular} ${targetObjectLabelSingular} id foreign key`;
Copy link
Contributor

Choose a reason for hiding this comment

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

style: description format is inconsistent with label format - 'id' should be capitalized as 'ID' to match label style

Copy link
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

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

Thank you @ijreilly

@Weiko Weiko merged commit 670c8a0 into main Nov 21, 2024
19 checks passed
@Weiko Weiko deleted the fix-settings-relations-issues branch November 21, 2024 14:54
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.

2 participants