[REFACTOR] Split in two distinct forms Settings Object Model page

[prastoin]

Introduction

This PR contains around ~+300 tests + snapshot additions
Please check both object model creation and edition

Closes twentyhq/core-team-issues#355

Refactored into two agnostic forms the Object Model settings page for instance /settings/objects/notes#settings.

SettingsDataModelObjectAboutForm

Added a new abstraction SettingsUpdateDataModelObjectAboutForm to wrap SettingsDataModelObjectAboutForm in an update context

Schema:

const requiredFormFields = objectMetadataItemSchema.pick({
  description: true,
  icon: true,
  labelPlural: true,
  labelSingular: true,
});
const optionalFormFields = objectMetadataItemSchema
  .pick({
    nameSingular: true,
    namePlural: true,
    isLabelSyncedWithName: true,
  })
  .partial();
export const settingsDataModelObjectAboutFormSchema =
  requiredFormFields.merge(optionalFormFields);

SettingsDataModelObjectSettingsFormCard

Update on change

Schema:

export const settingsDataModelObjectIdentifiersFormSchema =
  objectMetadataItemSchema.pick({
    labelIdentifierFieldMetadataId: true,
    imageIdentifierFieldMetadataId: true,
  });

Error management and validation schema

Improved the frontend validation form in order to attest that:

• Names are in camelCase
• Names are differents
• Names are not empty string SHOULD BE DONE SERVER SIDE TOO ( will in a next PR, atm it literally breaks any workspace )
• Labels are differents
• Labels aren't empty strings

Hide the error messages as we need to decide what kind of styling we want for our errors with forms
( Example with error labels )

[prastoin]

Just discovered that we can corrup a workspace by sending an empty pluralNameIdentifier in advanced mode, investigating on this before continuing here

[prastoin]

@greptileai trigger

[greptile-apps]

PR Summary

Here's my review of the changes in this pull request:

This PR refactors the Object Model settings page by splitting it into two distinct forms (SettingsDataModelObjectAboutForm and SettingsDataModelObjectSettingsFormCard) with improved validation and error handling.

Key changes:

• Added settingsDataModelObjectAboutFormSchema with required fields (description, icon, labels) and optional fields (names, sync settings)
• Created new SettingsUpdateDataModelObjectAboutForm wrapper component to handle object metadata updates
• Introduced ReadonlyKeysArray and zodNonEmptyString types for improved type safety
• Implemented custom validation rules for label/name differences and camelCase formatting
• Removed debug console.log in SettingsUpdateDataModelObjectAboutForm.tsx that should be cleaned up

The refactoring improves code organization and maintainability, but there are a few items that need attention:

• Remove console.log statement in SettingsUpdateDataModelObjectAboutForm
• Consider adding error boundary around form components for better error handling
• Verify form submission timing with the new onNewDirtyField approach to prevent race conditions

_{15 file(s) reviewed, 13 comment(s)
Edit PR Review Bot Settings | Greptile}

[prastoin]

@greptileai trigger

[greptile-apps]

PR Summary

(updates since last review)

This update focuses on refining the validation schema for the Object Model settings page with improved error handling and field validation.

Key changes:

• Updated settingsDataModelObjectAboutFormSchema.ts to enforce case-insensitive comparison using trim().toLowerCase() for label/name validation
• Changed description field to use nullish() instead of optional() in form schema
• Added camelCase validation for nameSingular and namePlural fields using camelCaseStringSchema
• Implemented zodNonEmptyString validation to prevent empty string values

The changes improve data validation while maintaining clean code structure. Note that server-side empty string validation is still pending implementation.

_{1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile}

[guillim]

I cannot create a new object, even though the "Save" button sounds enabled. I cannot screenshot it because the mouse over disapears, but i cannot

[prastoin]

I cannot create a new object, even though the "Save" button sounds enabled. I cannot screenshot it because the mouse over disapears, but i cannot

Lets have a look together, regarding the saving button indeed this was already in place, it not changing its styling when disabled is not very intuitive

[guillim]

I am new to snapshotting in JEST. Correct me if I am wrong, but this is JEST that created the folder snapshot and its containg file.

Question: UI test tend to be flaky. even though I see in the doc the jest update function, it will break the CI. Are we ok and aware with implementing this as if a unit test ?

[prastoin]

I am new to snapshotting in JEST. Correct me if I am wrong, but this is JEST that created the folder snapshot and its containg file.

Exactly

Question: UI test tend to be flaky. even though I see in the doc the jest update function, it will break the CI. Are we ok and aware with implementing this ?

I think you're mistaking e2e tests and unit/integrations tests which by definition should be way less flaky ( querying a DOM always is etc etc), but regarding this PR tests suite we should never have flakiness as these are very low level javascript unit tests

[guillim]

True. But if we add a field to the form, do we want to break the test ? Answer could be yes, if we consider its a cruacial part. that's my point

[prastoin]

In my opinion yes, but before the test to fail, TypeScript would also raises regarding test definition itself saying: dude your mocks does not relate my inner types

Note that updating snapshot is pretty easy, but rely on the fact that users will review them
I always tend to snapshot errors because no need to be very strict
Regarding huge data set I'm not a fan, or at least binded with atomic assertion such as done within useDeleteOne.test.ts test suite with Apollo cache

[prastoin]

@guillim Thank you for the advanced mode issue, nice catch ! 🚀 Fixed the form to always have a default value for the isSyncNameLabel which was blocking the creation form submition

[guillim]

great job @prastoin !!!