Fix orphan navigation menu items for deleted views (#18791)
## Summary Fixes #18757 This fixes a set of Favorites / navigation-menu-item integrity problems related to deleted views, stale hidden items, and upgraded workspaces with orphaned navigation items. ## What changed - delete `navigationMenuItem` entries when their favorited view is deleted - keep the client metadata store in sync immediately when a view is deleted - determine whether a view is already favorited from visible valid navigation items instead of raw stale items - add a `1.20.0` upgrade repair command that deletes orphan navigation menu items and normalizes positions - add regression coverage for deletion of both record-based and view-based navigation menu items ## Details Server: - extend `NavigationMenuItemDeletionService` so cleanup applies to deleted views as well as deleted records - add regression tests covering record-based deletion, view-based deletion, and no-op behavior - add `DeleteOrphanNavigationMenuItemsCommand` to remove orphaned items pointing to: - deleted views - deleted records - missing folders - normalize positions per scope (`userWorkspaceId + folderId`) after repair - wire the new repair command into the `1.20.0` upgrade flow Frontend: - add `useRemoveNavigationMenuItemByViewId` - remove the related navigation item from client metadata immediately when deleting a view - use sorted / visible navigation items for favorite detection so stale hidden rows do not block re-adding a favorite ## Why Issue `#18757` reports mismatches between Favorites shown in the UI and rows users can still find in the database. We found that current Favorites behavior is driven by `navigationMenuItem`, not the legacy `favorite` table, and that stale / orphaned `navigationMenuItem` rows could: - remain after deleting a favorited view - stay hidden from the UI if they point to invalid targets - still cause the UI to think a view was already favorited - persist in workspaces with migration damage from skipped sequential upgrades This patch addresses those cases directly and adds an upgrade-time repair path for older corrupted workspaces. ## Validation Passed: - `./node_modules/.bin/jest --config packages/twenty-server/jest.config.mjs --runInBand packages/twenty-server/src/engine/metadata-modules/navigation-menu-item/services/__tests__/navigation-menu-item-deletion.service.spec.ts` - `./node_modules/.bin/tsc -p packages/twenty-front/tsconfig.json --noEmit --pretty false` Known unrelated existing failure: - `./node_modules/.bin/tsc -p packages/twenty-server/tsconfig.json --noEmit --pretty false` The server typecheck failure is pre-existing and unrelated to this branch. Current errors are around `@file-type/pdf` module resolution and `is-psl-parsed-domain.type.ts`. --------- Co-authored-by: Charles Bochet <charles@twenty.com> Co-authored-by: Charles Bochet <charlesBochet@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
89300564ba
commit
7c8f060b08
+35
@@ -0,0 +1,35 @@
|
||||
import { useCallback } from 'react';
|
||||
|
||||
import { useMetadataStore } from '@/metadata-store/hooks/useMetadataStore';
|
||||
import { metadataStoreState } from '@/metadata-store/states/metadataStoreState';
|
||||
import { type NavigationMenuItem } from '~/generated-metadata/graphql';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { useStore } from 'jotai';
|
||||
|
||||
export const useOptimisticRemoveNavigationMenuItemsByViewId = () => {
|
||||
const store = useStore();
|
||||
const { replaceDraft, applyChanges } = useMetadataStore();
|
||||
|
||||
const removeNavigationMenuItemsByViewIds = useCallback(
|
||||
(viewIds: string[]) => {
|
||||
const viewIdsSet = new Set(viewIds);
|
||||
const entry = store.get(
|
||||
metadataStoreState.atomFamily('navigationMenuItems'),
|
||||
);
|
||||
const currentNavigationMenuItems =
|
||||
entry.current as unknown as NavigationMenuItem[];
|
||||
|
||||
const updatedNavigationMenuItems = currentNavigationMenuItems.filter(
|
||||
(item) => !isDefined(item.viewId) || !viewIdsSet.has(item.viewId),
|
||||
);
|
||||
|
||||
replaceDraft('navigationMenuItems', updatedNavigationMenuItems);
|
||||
applyChanges();
|
||||
},
|
||||
[store, replaceDraft, applyChanges],
|
||||
);
|
||||
|
||||
return {
|
||||
removeNavigationMenuItemsByViewIds,
|
||||
};
|
||||
};
|
||||
+4
-3
@@ -1,5 +1,6 @@
|
||||
import { useCreateNavigationMenuItem } from '@/navigation-menu-item/common/hooks/useCreateNavigationMenuItem';
|
||||
import { useNavigationMenuItemsData } from '@/navigation-menu-item/display/hooks/useNavigationMenuItemsData';
|
||||
import { useSortedNavigationMenuItems } from '@/navigation-menu-item/display/hooks/useSortedNavigationMenuItems';
|
||||
import { type ObjectRecord } from '@/object-record/types/ObjectRecord';
|
||||
import { useHasPermissionFlag } from '@/settings/roles/hooks/useHasPermissionFlag';
|
||||
import { DropdownContent } from '@/ui/layout/dropdown/components/DropdownContent';
|
||||
@@ -54,15 +55,15 @@ export const ViewPickerOptionDropdown = ({
|
||||
const hasViewsPermission = useHasPermissionFlag(PermissionFlagType.VIEWS);
|
||||
|
||||
const { createNavigationMenuItem } = useCreateNavigationMenuItem();
|
||||
const { navigationMenuItems, currentWorkspaceMemberId } =
|
||||
useNavigationMenuItemsData();
|
||||
const { currentWorkspaceMemberId } = useNavigationMenuItemsData();
|
||||
const { navigationMenuItemsSorted } = useSortedNavigationMenuItems();
|
||||
|
||||
// Users with VIEWS permission can edit all views
|
||||
// Users without VIEWS permission can only edit unlisted views (which are always their own, filtered by backend)
|
||||
const canEditView =
|
||||
hasViewsPermission || view.visibility === ViewVisibility.UNLISTED;
|
||||
|
||||
const isFavorite = navigationMenuItems.some(
|
||||
const isFavorite = navigationMenuItemsSorted.some(
|
||||
(item) =>
|
||||
item.viewId === view.id &&
|
||||
item.userWorkspaceId === currentWorkspaceMemberId,
|
||||
|
||||
+5
@@ -2,6 +2,7 @@ import { useCallback } from 'react';
|
||||
import { useStore } from 'jotai';
|
||||
|
||||
import { useContextStoreObjectMetadataItemOrThrow } from '@/context-store/hooks/useContextStoreObjectMetadataItemOrThrow';
|
||||
import { useOptimisticRemoveNavigationMenuItemsByViewId } from '@/navigation-menu-item/edit/hooks/useOptimisticRemoveNavigationMenuItemsByViewId';
|
||||
import { useAtomComponentStateCallbackState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateCallbackState';
|
||||
import { useAtomFamilySelectorValue } from '@/ui/utilities/state/jotai/hooks/useAtomFamilySelectorValue';
|
||||
import { usePerformViewAPIPersist } from '@/views/hooks/internal/usePerformViewAPIPersist';
|
||||
@@ -45,6 +46,8 @@ export const useDestroyViewFromCurrentState = (viewBarInstanceId?: string) => {
|
||||
const { changeView } = useChangeView();
|
||||
|
||||
const { performViewAPIDestroy } = usePerformViewAPIPersist();
|
||||
const { removeNavigationMenuItemsByViewIds } =
|
||||
useOptimisticRemoveNavigationMenuItemsByViewId();
|
||||
|
||||
const store = useStore();
|
||||
|
||||
@@ -72,11 +75,13 @@ export const useDestroyViewFromCurrentState = (viewBarInstanceId?: string) => {
|
||||
}
|
||||
|
||||
await performViewAPIDestroy({ id: viewPickerReferenceViewId });
|
||||
removeNavigationMenuItemsByViewIds([viewPickerReferenceViewId]);
|
||||
}, [
|
||||
currentView,
|
||||
closeAndResetViewPicker,
|
||||
changeView,
|
||||
performViewAPIDestroy,
|
||||
removeNavigationMenuItemsByViewIds,
|
||||
store,
|
||||
viewPickerIsDirtyCallbackState,
|
||||
viewPickerIsPersistingCallbackState,
|
||||
|
||||
+91
@@ -0,0 +1,91 @@
|
||||
import { Logger } from '@nestjs/common';
|
||||
import { InjectRepository } from '@nestjs/typeorm';
|
||||
|
||||
import { Command } from 'nest-commander';
|
||||
import { In, Repository } from 'typeorm';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { NavigationMenuItemType } from 'twenty-shared/types';
|
||||
|
||||
import { ActiveOrSuspendedWorkspacesMigrationCommandRunner } from 'src/database/commands/command-runners/active-or-suspended-workspaces-migration.command-runner';
|
||||
import { RunOnWorkspaceArgs } from 'src/database/commands/command-runners/workspaces-migration.command-runner';
|
||||
import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity';
|
||||
import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service';
|
||||
import { NavigationMenuItemEntity } from 'src/engine/metadata-modules/navigation-menu-item/entities/navigation-menu-item.entity';
|
||||
import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager';
|
||||
import { WorkspaceCacheService } from 'src/engine/workspace-cache/services/workspace-cache.service';
|
||||
|
||||
@Command({
|
||||
name: 'upgrade:1-20:delete-orphan-navigation-menu-items',
|
||||
description: 'Delete navigation menu items pointing to deleted views',
|
||||
})
|
||||
export class DeleteOrphanNavigationMenuItemsCommand extends ActiveOrSuspendedWorkspacesMigrationCommandRunner {
|
||||
protected readonly logger = new Logger(
|
||||
DeleteOrphanNavigationMenuItemsCommand.name,
|
||||
);
|
||||
|
||||
constructor(
|
||||
@InjectRepository(WorkspaceEntity)
|
||||
protected readonly workspaceRepository: Repository<WorkspaceEntity>,
|
||||
protected readonly twentyORMGlobalManager: GlobalWorkspaceOrmManager,
|
||||
protected readonly dataSourceService: DataSourceService,
|
||||
@InjectRepository(NavigationMenuItemEntity)
|
||||
private readonly navigationMenuItemRepository: Repository<NavigationMenuItemEntity>,
|
||||
private readonly workspaceCacheService: WorkspaceCacheService,
|
||||
) {
|
||||
super(workspaceRepository, twentyORMGlobalManager, dataSourceService);
|
||||
}
|
||||
|
||||
override async runOnWorkspace({
|
||||
workspaceId,
|
||||
options,
|
||||
}: RunOnWorkspaceArgs): Promise<void> {
|
||||
const { flatViewMaps, flatNavigationMenuItemMaps } =
|
||||
await this.workspaceCacheService.getOrRecompute(workspaceId, [
|
||||
'flatViewMaps',
|
||||
'flatNavigationMenuItemMaps',
|
||||
]);
|
||||
|
||||
const activeViewIds = new Set(
|
||||
Object.values(flatViewMaps.byUniversalIdentifier)
|
||||
.filter((view): view is NonNullable<typeof view> => isDefined(view))
|
||||
.filter((view) => view.deletedAt === null)
|
||||
.map((view) => view.id),
|
||||
);
|
||||
|
||||
const orphanViewNavigationMenuItemIds = Object.values(
|
||||
flatNavigationMenuItemMaps.byUniversalIdentifier,
|
||||
)
|
||||
.filter(
|
||||
(item): item is NonNullable<typeof item> =>
|
||||
isDefined(item) &&
|
||||
item.type === NavigationMenuItemType.VIEW &&
|
||||
isDefined(item.viewId) &&
|
||||
!activeViewIds.has(item.viewId),
|
||||
)
|
||||
.map((item) => item.id);
|
||||
|
||||
if (orphanViewNavigationMenuItemIds.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (options.dryRun) {
|
||||
this.logger.log(
|
||||
`[DRY RUN] Would delete ${orphanViewNavigationMenuItemIds.length} orphan navigation menu item(s) for workspace ${workspaceId}`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
await this.navigationMenuItemRepository.delete({
|
||||
workspaceId,
|
||||
id: In(orphanViewNavigationMenuItemIds),
|
||||
});
|
||||
|
||||
this.logger.log(
|
||||
`Deleted ${orphanViewNavigationMenuItemIds.length} orphan navigation menu item(s) for workspace ${workspaceId}`,
|
||||
);
|
||||
|
||||
await this.workspaceCacheService.flush(workspaceId, [
|
||||
'flatNavigationMenuItemMaps',
|
||||
]);
|
||||
}
|
||||
}
|
||||
+5
@@ -5,6 +5,7 @@ import { BackfillCommandMenuItemsCommand } from 'src/database/commands/upgrade-v
|
||||
import { BackfillNavigationMenuItemTypeCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-backfill-navigation-menu-item-type.command';
|
||||
import { BackfillPageLayoutsCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-backfill-page-layouts.command';
|
||||
import { BackfillSelectFieldOptionIdsCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-backfill-select-field-option-ids.command';
|
||||
import { DeleteOrphanNavigationMenuItemsCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-delete-orphan-navigation-menu-items.command';
|
||||
import { IdentifyObjectPermissionMetadataCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-identify-object-permission-metadata.command';
|
||||
import { IdentifyPermissionFlagMetadataCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-identify-permission-flag-metadata.command';
|
||||
import { MakeObjectPermissionUniversalIdentifierAndApplicationIdNotNullableMigrationCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-make-object-permission-universal-identifier-and-application-id-not-nullable-migration.command';
|
||||
@@ -22,6 +23,7 @@ import { ConnectedAccountEntity } from 'src/engine/metadata-modules/connected-ac
|
||||
import { DataSourceModule } from 'src/engine/metadata-modules/data-source/data-source.module';
|
||||
import { MessageChannelEntity } from 'src/engine/metadata-modules/message-channel/entities/message-channel.entity';
|
||||
import { MessageFolderEntity } from 'src/engine/metadata-modules/message-folder/entities/message-folder.entity';
|
||||
import { NavigationMenuItemEntity } from 'src/engine/metadata-modules/navigation-menu-item/entities/navigation-menu-item.entity';
|
||||
import { WorkspaceMetadataVersionModule } from 'src/engine/metadata-modules/workspace-metadata-version/workspace-metadata-version.module';
|
||||
import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module';
|
||||
import { WorkspaceCacheModule } from 'src/engine/workspace-cache/workspace-cache.module';
|
||||
@@ -37,6 +39,7 @@ import { WorkspaceMigrationModule } from 'src/engine/workspace-manager/workspace
|
||||
CalendarChannelEntity,
|
||||
MessageFolderEntity,
|
||||
UserWorkspaceEntity,
|
||||
NavigationMenuItemEntity,
|
||||
]),
|
||||
DataSourceModule,
|
||||
WorkspaceCacheModule,
|
||||
@@ -57,6 +60,7 @@ import { WorkspaceMigrationModule } from 'src/engine/workspace-manager/workspace
|
||||
BackfillNavigationMenuItemTypeCommand,
|
||||
BackfillPageLayoutsCommand,
|
||||
BackfillSelectFieldOptionIdsCommand,
|
||||
DeleteOrphanNavigationMenuItemsCommand,
|
||||
SeedCliApplicationRegistrationCommand,
|
||||
MigrateRichTextToTextCommand,
|
||||
MigrateMessagingInfrastructureToMetadataCommand,
|
||||
@@ -70,6 +74,7 @@ import { WorkspaceMigrationModule } from 'src/engine/workspace-manager/workspace
|
||||
BackfillNavigationMenuItemTypeCommand,
|
||||
BackfillPageLayoutsCommand,
|
||||
BackfillSelectFieldOptionIdsCommand,
|
||||
DeleteOrphanNavigationMenuItemsCommand,
|
||||
SeedCliApplicationRegistrationCommand,
|
||||
MigrateRichTextToTextCommand,
|
||||
MigrateMessagingInfrastructureToMetadataCommand,
|
||||
|
||||
+3
@@ -35,6 +35,7 @@ import { FixInvalidStandardUniversalIdentifiersCommand } from 'src/database/comm
|
||||
import { SeedServerIdCommand } from 'src/database/commands/upgrade-version-command/1-19/1-19-seed-server-id.command';
|
||||
import { BackfillCommandMenuItemsCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-backfill-command-menu-items.command';
|
||||
import { BackfillNavigationMenuItemTypeCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-backfill-navigation-menu-item-type.command';
|
||||
import { DeleteOrphanNavigationMenuItemsCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-delete-orphan-navigation-menu-items.command';
|
||||
import { BackfillPageLayoutsCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-backfill-page-layouts.command';
|
||||
import { BackfillSelectFieldOptionIdsCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-backfill-select-field-option-ids.command';
|
||||
import { IdentifyObjectPermissionMetadataCommand } from 'src/database/commands/upgrade-version-command/1-20/1-20-identify-object-permission-metadata.command';
|
||||
@@ -100,6 +101,7 @@ export class UpgradeCommand extends UpgradeCommandRunner {
|
||||
protected readonly makeObjectPermissionUniversalIdentifierAndApplicationIdNotNullableMigrationCommand: MakeObjectPermissionUniversalIdentifierAndApplicationIdNotNullableMigrationCommand,
|
||||
protected readonly backfillNavigationMenuItemTypeCommand: BackfillNavigationMenuItemTypeCommand,
|
||||
protected readonly backfillCommandMenuItemsCommand: BackfillCommandMenuItemsCommand,
|
||||
protected readonly deleteOrphanNavigationMenuItemsCommand: DeleteOrphanNavigationMenuItemsCommand,
|
||||
protected readonly backfillPageLayoutsCommand: BackfillPageLayoutsCommand,
|
||||
protected readonly seedCliApplicationRegistrationCommand: SeedCliApplicationRegistrationCommand,
|
||||
protected readonly migrateRichTextToTextCommand: MigrateRichTextToTextCommand,
|
||||
@@ -160,6 +162,7 @@ export class UpgradeCommand extends UpgradeCommandRunner {
|
||||
.makeObjectPermissionUniversalIdentifierAndApplicationIdNotNullableMigrationCommand,
|
||||
this.backfillNavigationMenuItemTypeCommand,
|
||||
this.migrateRichTextToTextCommand,
|
||||
this.deleteOrphanNavigationMenuItemsCommand,
|
||||
this.backfillCommandMenuItemsCommand,
|
||||
this.backfillPageLayoutsCommand,
|
||||
this.seedCliApplicationRegistrationCommand,
|
||||
|
||||
+16
-3
@@ -4,10 +4,24 @@ import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { ApplicationService } from 'src/engine/core-modules/application/application.service';
|
||||
import { WorkspaceManyOrAllFlatEntityMapsCacheService } from 'src/engine/metadata-modules/flat-entity/services/workspace-many-or-all-flat-entity-maps-cache.service';
|
||||
import { type FlatNavigationMenuItem } from 'src/engine/metadata-modules/flat-navigation-menu-item/types/flat-navigation-menu-item.type';
|
||||
import { fromDeleteNavigationMenuItemInputToFlatNavigationMenuItemOrThrow } from 'src/engine/metadata-modules/flat-navigation-menu-item/utils/from-delete-navigation-menu-item-input-to-flat-navigation-menu-item-or-throw.util';
|
||||
import { WorkspaceMigrationBuilderException } from 'src/engine/workspace-manager/workspace-migration/exceptions/workspace-migration-builder-exception';
|
||||
import { WorkspaceMigrationValidateBuildAndRunService } from 'src/engine/workspace-manager/workspace-migration/services/workspace-migration-validate-build-and-run-service';
|
||||
|
||||
const isNavigationMenuItemForDeletedRecord = (
|
||||
item: FlatNavigationMenuItem,
|
||||
deletedIdsSet: Set<string>,
|
||||
): boolean =>
|
||||
isDefined(item.targetRecordId) &&
|
||||
!isDefined(item.viewId) &&
|
||||
deletedIdsSet.has(item.targetRecordId);
|
||||
|
||||
const isNavigationMenuItemForDeletedView = (
|
||||
item: FlatNavigationMenuItem,
|
||||
deletedIdsSet: Set<string>,
|
||||
): boolean => isDefined(item.viewId) && deletedIdsSet.has(item.viewId);
|
||||
|
||||
@Injectable()
|
||||
export class NavigationMenuItemDeletionService {
|
||||
constructor(
|
||||
@@ -40,9 +54,8 @@ export class NavigationMenuItemDeletionService {
|
||||
).filter(
|
||||
(item): item is NonNullable<typeof item> =>
|
||||
isDefined(item) &&
|
||||
isDefined(item.targetRecordId) &&
|
||||
!isDefined(item.viewId) &&
|
||||
deletedRecordIdsSet.has(item.targetRecordId),
|
||||
(isNavigationMenuItemForDeletedRecord(item, deletedRecordIdsSet) ||
|
||||
isNavigationMenuItemForDeletedView(item, deletedRecordIdsSet)),
|
||||
);
|
||||
|
||||
if (navigationMenuItemsToDelete.length === 0) {
|
||||
|
||||
+1
-1
@@ -175,7 +175,7 @@ export class DevSeederMetadataService {
|
||||
},
|
||||
};
|
||||
|
||||
private getLightConfig(config: WorkspaceSeedConfig): WorkspaceSeedConfig {
|
||||
private getLightConfig(_config: WorkspaceSeedConfig): WorkspaceSeedConfig {
|
||||
return {
|
||||
objects: [],
|
||||
fields: [],
|
||||
|
||||
-1
@@ -18,7 +18,6 @@ import {
|
||||
MessageChannelPendingGroupEmailsAction,
|
||||
MessageChannelSyncStage,
|
||||
MessageChannelSyncStatus,
|
||||
type MessageChannelWorkspaceEntity,
|
||||
} from 'src/modules/messaging/common/standard-objects/message-channel.workspace-entity';
|
||||
import { MessageFolderPendingSyncAction } from 'src/modules/messaging/common/standard-objects/message-folder.workspace-entity';
|
||||
import { type WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity';
|
||||
|
||||
Reference in New Issue
Block a user