mirror of
https://github.com/twentyhq/twenty.git
synced 2026-08-02 18:54:43 -04:00
Follow-up to the discussion on #23485 and closes https://github.com/twentyhq/twenty/issues/23484. `FlatNavigationMenuItemValidatorService` receives `UniversalFlatEntityValidationArgs<'navigationMenuItem'>`, so the entity it validates is a `UniversalFlatNavigationMenuItem`: `viewId`, `pageLayoutId` and `targetObjectMetadataId` do not exist at that scope. The validator read the right universal keys but passed them through a bag of booleans named after the ids (`hasViewId`, `hasPageLayoutId`, ...) and then reported the id names in its errors. Nothing tied a message to the property it checked, so fixing one message string leaves the other five wrong. ## Changes - Replace the private `validateNavigationMenuItemType` boolean bag with `validateNavigationMenuItemTypeRequiredProperties({ flatNavigationMenuItem })` under `flat-navigation-menu-item/validators/utils/`, in line with `validateAgentRequiredProperties` and `validateNavigationMenuItemPageLayoutReferenceCrossEntity`. It takes the universal entity, so a message can only name a property that exists at that scope. - The util is an explicit `switch` on `NavigationMenuItemType` closed by `assertUnreachable`, so adding a type fails to compile until its contract is declared. - Each case validates its own properties instead of checking presence generically: - `FOLDER`: non blank `name` - `OBJECT`, `VIEW`, `PAGE_LAYOUT`: `targetObjectMetadataUniversalIdentifier` / `viewUniversalIdentifier` / `pageLayoutUniversalIdentifier` must be valid uuids - `RECORD`: `targetRecordId` and `targetObjectMetadataUniversalIdentifier`, both uuids, reported separately - `LINK`: `link` must pass `isValidUrl` - Both call sites spread the result; the update path passes the merged `{ ...from, ...update }` entity, which removes the redundant `name` re-merge. `targetRecordId` stays an id: it points at workspace record data rather than metadata, so it has no universal counterpart. ## Behaviour - Errors name the universal property (`viewUniversalIdentifier`) instead of the id (`viewId`). - Blank strings are now uniformly treated as missing; creation previously accepted `link: " "`. - `RECORD` reports each missing property separately instead of one merged error. - Values that are present but malformed are now rejected: non uuid identifiers and links that are not urls. Standard application identifiers are all v4 uuids and the create/update inputs already carry `@IsUUID`, so this only tightens the app manifest path. ## Verification - Unit tests for the util cover each type valid and invalid, blank names, non url links and non uuid identifiers (23 tests pass alongside the sibling suite) - `nx typecheck twenty-server` clean - oxlint (type-aware) and oxfmt clean on the changed files <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/23566?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->