Fix navigation menu edition (#19115)
# Description Fix "Already in navbar" false positive in the sidebar object picker: `getObjectMetadataIdsInDraft` was collecting `objectMetadataId` from all navigation menu item types (OBJECT, VIEW, RECORD), so having a pinned view or record for an object type would incorrectly block re-adding that object. Now only OBJECT-type items contribute to the duplicate check. # Video QA ## Before https://github.com/user-attachments/assets/7b853a55-6d7a-444c-92ee-bf6a75d9927c ## After https://github.com/user-attachments/assets/65e651e2-9df8-45fd-872d-00782d6b6490 Co-authored-by: Charles Bochet <charlesBochet@users.noreply.github.com>
This commit is contained in:
+111
@@ -0,0 +1,111 @@
|
||||
import { getObjectMetadataIdsInDraft } from '@/navigation-menu-item/common/utils/getObjectMetadataIdsInDraft';
|
||||
import { NavigationMenuItemType } from 'twenty-shared/types';
|
||||
|
||||
describe('getObjectMetadataIdsInDraft', () => {
|
||||
it('should collect objectMetadataId from OBJECT-type items', () => {
|
||||
const draft = [
|
||||
{
|
||||
type: NavigationMenuItemType.OBJECT,
|
||||
targetObjectMetadataId: 'object-people',
|
||||
},
|
||||
];
|
||||
|
||||
const result = getObjectMetadataIdsInDraft(draft);
|
||||
|
||||
expect(result).toEqual(new Set(['object-people']));
|
||||
});
|
||||
|
||||
it('should NOT collect objectMetadataId from VIEW-type items', () => {
|
||||
const draft = [
|
||||
{
|
||||
type: NavigationMenuItemType.VIEW,
|
||||
targetObjectMetadataId: 'object-people',
|
||||
},
|
||||
];
|
||||
|
||||
const result = getObjectMetadataIdsInDraft(draft);
|
||||
|
||||
expect(result.size).toBe(0);
|
||||
});
|
||||
|
||||
it('should NOT collect objectMetadataId from RECORD-type items', () => {
|
||||
const draft = [
|
||||
{
|
||||
type: NavigationMenuItemType.RECORD,
|
||||
targetObjectMetadataId: 'object-people',
|
||||
},
|
||||
];
|
||||
|
||||
const result = getObjectMetadataIdsInDraft(draft);
|
||||
|
||||
expect(result.size).toBe(0);
|
||||
});
|
||||
|
||||
it('should skip RECORD and VIEW items but collect from OBJECT items', () => {
|
||||
const draft = [
|
||||
{
|
||||
type: NavigationMenuItemType.RECORD,
|
||||
targetObjectMetadataId: 'object-people',
|
||||
},
|
||||
{
|
||||
type: NavigationMenuItemType.VIEW,
|
||||
targetObjectMetadataId: 'object-people',
|
||||
},
|
||||
{
|
||||
type: NavigationMenuItemType.OBJECT,
|
||||
targetObjectMetadataId: 'object-companies',
|
||||
},
|
||||
];
|
||||
|
||||
const result = getObjectMetadataIdsInDraft(draft);
|
||||
|
||||
expect(result.has('object-people')).toBe(false);
|
||||
expect(result.has('object-companies')).toBe(true);
|
||||
});
|
||||
|
||||
it('should collect from multiple OBJECT items', () => {
|
||||
const draft = [
|
||||
{
|
||||
type: NavigationMenuItemType.OBJECT,
|
||||
targetObjectMetadataId: 'object-people',
|
||||
},
|
||||
{
|
||||
type: NavigationMenuItemType.OBJECT,
|
||||
targetObjectMetadataId: 'object-companies',
|
||||
},
|
||||
];
|
||||
|
||||
const result = getObjectMetadataIdsInDraft(draft);
|
||||
|
||||
expect(result).toEqual(new Set(['object-people', 'object-companies']));
|
||||
});
|
||||
|
||||
it('should skip FOLDER and LINK items', () => {
|
||||
const draft = [
|
||||
{ type: NavigationMenuItemType.FOLDER },
|
||||
{ type: NavigationMenuItemType.LINK },
|
||||
];
|
||||
|
||||
const result = getObjectMetadataIdsInDraft(draft);
|
||||
|
||||
expect(result.size).toBe(0);
|
||||
});
|
||||
|
||||
it('should handle an empty draft', () => {
|
||||
const result = getObjectMetadataIdsInDraft([]);
|
||||
|
||||
expect(result.size).toBe(0);
|
||||
});
|
||||
|
||||
it('should skip OBJECT items without targetObjectMetadataId', () => {
|
||||
const draft = [
|
||||
{
|
||||
type: NavigationMenuItemType.OBJECT,
|
||||
},
|
||||
];
|
||||
|
||||
const result = getObjectMetadataIdsInDraft(draft);
|
||||
|
||||
expect(result.size).toBe(0);
|
||||
});
|
||||
});
|
||||
+7
-17
@@ -1,28 +1,18 @@
|
||||
import { NavigationMenuItemType } from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
type NavigationMenuItemDraftForObjectIds = {
|
||||
viewId?: string | null;
|
||||
targetObjectMetadataId?: string | null;
|
||||
};
|
||||
|
||||
type ViewForObjectIds = {
|
||||
id: string;
|
||||
objectMetadataId: string;
|
||||
};
|
||||
import { type NavigationMenuItem } from '~/generated-metadata/graphql';
|
||||
|
||||
export const getObjectMetadataIdsInDraft = (
|
||||
draft: NavigationMenuItemDraftForObjectIds[],
|
||||
views: ViewForObjectIds[],
|
||||
draft: Pick<NavigationMenuItem, 'type' | 'targetObjectMetadataId'>[],
|
||||
): Set<string> =>
|
||||
draft.reduce<Set<string>>((ids, item) => {
|
||||
const view = isDefined(item.viewId)
|
||||
? views.find((view) => view.id === item.viewId)
|
||||
: undefined;
|
||||
if (isDefined(view)) {
|
||||
ids.add(view.objectMetadataId);
|
||||
if (item.type !== NavigationMenuItemType.OBJECT) {
|
||||
return ids;
|
||||
}
|
||||
|
||||
if (isDefined(item.targetObjectMetadataId)) {
|
||||
ids.add(item.targetObjectMetadataId);
|
||||
}
|
||||
|
||||
return ids;
|
||||
}, new Set<string>());
|
||||
|
||||
+4
-5
@@ -9,7 +9,6 @@ import { addToNavPayloadRegistryState } from '@/navigation-menu-item/common/stat
|
||||
import { navigationMenuItemsDraftState } from '@/navigation-menu-item/common/states/navigationMenuItemsDraftState';
|
||||
import { openNavigationMenuItemFolderIdsState } from '@/navigation-menu-item/common/states/openNavigationMenuItemFolderIdsState';
|
||||
import { canNavigationMenuItemBeDroppedIn } from '@/navigation-menu-item/common/utils/canNavigationMenuItemBeDroppedIn';
|
||||
import { getObjectColorWithFallback } from '@/object-metadata/utils/getObjectColorWithFallback';
|
||||
import { getObjectMetadataIdsInDraft } from '@/navigation-menu-item/common/utils/getObjectMetadataIdsInDraft';
|
||||
import { validateAndExtractWorkspaceFolderId } from '@/navigation-menu-item/common/utils/validateAndExtractWorkspaceFolderId';
|
||||
import { useAddFolderToNavigationMenuDraft } from '@/navigation-menu-item/edit/folder/hooks/useAddFolderToNavigationMenuDraft';
|
||||
@@ -20,6 +19,7 @@ import { useAddObjectToNavigationMenuDraft } from '@/navigation-menu-item/edit/o
|
||||
import { useAddRecordToNavigationMenuDraft } from '@/navigation-menu-item/edit/record/hooks/useAddRecordToNavigationMenuDraft';
|
||||
import { useAddViewToNavigationMenuDraft } from '@/navigation-menu-item/edit/view/hooks/useAddViewToNavigationMenuDraft';
|
||||
import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems';
|
||||
import { getObjectColorWithFallback } from '@/object-metadata/utils/getObjectColorWithFallback';
|
||||
import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue';
|
||||
import { useSetAtomState } from '@/ui/utilities/state/jotai/hooks/useSetAtomState';
|
||||
import { viewsSelector } from '@/views/states/selectors/viewsSelector';
|
||||
@@ -130,10 +130,9 @@ export const useHandleAddToNavigationDrop = () => {
|
||||
return;
|
||||
}
|
||||
case NavigationMenuItemType.OBJECT: {
|
||||
const objectMetadataIdsInWorkspace = getObjectMetadataIdsInDraft(
|
||||
currentDraft,
|
||||
views,
|
||||
);
|
||||
const objectMetadataIdsInWorkspace =
|
||||
getObjectMetadataIdsInDraft(currentDraft);
|
||||
|
||||
if (objectMetadataIdsInWorkspace.has(payload.objectMetadataId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
+4
-15
@@ -1,6 +1,4 @@
|
||||
import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue';
|
||||
import { viewsSelector } from '@/views/states/selectors/viewsSelector';
|
||||
|
||||
import { NavigationMenuItemType } from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { useNavigationMenuItemsData } from './useNavigationMenuItemsData';
|
||||
|
||||
@@ -9,22 +7,13 @@ export const useWorkspaceNavigationMenuItems = (): {
|
||||
} => {
|
||||
const { workspaceNavigationMenuItems: rawWorkspaceNavigationMenuItems } =
|
||||
useNavigationMenuItemsData();
|
||||
const views = useAtomStateValue(viewsSelector);
|
||||
|
||||
const workspaceNavViewIds = new Set(
|
||||
const objectMetadataIdsInWorkspaceNav = new Set(
|
||||
rawWorkspaceNavigationMenuItems
|
||||
.map((item) => item.viewId)
|
||||
.filter((viewId) => isDefined(viewId)),
|
||||
);
|
||||
|
||||
const objectMetadataIdsInWorkspaceNav = new Set([
|
||||
...views
|
||||
.filter((view) => workspaceNavViewIds.has(view.id))
|
||||
.map((view) => view.objectMetadataId),
|
||||
...rawWorkspaceNavigationMenuItems
|
||||
.filter((item) => item.type === NavigationMenuItemType.OBJECT)
|
||||
.map((item) => item.targetObjectMetadataId)
|
||||
.filter((objectMetadataId) => isDefined(objectMetadataId)),
|
||||
]);
|
||||
);
|
||||
|
||||
return {
|
||||
objectMetadataIdsInWorkspaceNav,
|
||||
|
||||
+7
-9
@@ -4,22 +4,20 @@ import { getObjectMetadataIdsInDraft } from '@/navigation-menu-item/common/utils
|
||||
import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue';
|
||||
import { viewsSelector } from '@/views/states/selectors/viewsSelector';
|
||||
import { ViewKey } from '@/views/types/ViewKey';
|
||||
import { type NavigationMenuItem } from '~/generated-metadata/graphql';
|
||||
|
||||
type NavigationMenuItemDraft = {
|
||||
id?: string;
|
||||
viewId?: string | null;
|
||||
targetObjectMetadataId?: string | null;
|
||||
};
|
||||
type NavigationMenuItemDraft = Pick<
|
||||
NavigationMenuItem,
|
||||
'id' | 'type' | 'viewId' | 'targetObjectMetadataId'
|
||||
>;
|
||||
|
||||
export const useNavigationMenuObjectMetadataFromDraft = (
|
||||
currentDraft: NavigationMenuItemDraft[],
|
||||
) => {
|
||||
const views = useAtomStateValue(viewsSelector);
|
||||
|
||||
const objectMetadataIdsInWorkspace = getObjectMetadataIdsInDraft(
|
||||
currentDraft,
|
||||
views,
|
||||
);
|
||||
const objectMetadataIdsInWorkspace =
|
||||
getObjectMetadataIdsInDraft(currentDraft);
|
||||
|
||||
const objectMetadataIdsWithIndexView = new Set(
|
||||
views
|
||||
|
||||
Reference in New Issue
Block a user