Fix page layout widget tab moves (#20915)
Fixes widget moves between page layout tabs by making pageLayoutTabId part of the flat-entity diff, so the save mutation no longer silently drops the new tab assignment. https://discord.com/channels/1130383047699738754/1508737039128985680
This commit is contained in:
+1
@@ -214,6 +214,7 @@ exports[`ALL_UNIVERSAL_FLAT_ENTITY_PROPERTIES_TO_COMPARE_AND_STRINGIFY should ma
|
||||
"position",
|
||||
"universalConfiguration",
|
||||
"deletedAt",
|
||||
"pageLayoutTabUniversalIdentifier",
|
||||
"conditionalDisplay",
|
||||
"conditionalAvailabilityExpression",
|
||||
"isActive",
|
||||
|
||||
+1
-1
@@ -930,7 +930,7 @@ export const ALL_ENTITY_PROPERTIES_CONFIGURATION_BY_METADATA_NAME = {
|
||||
universalProperty: undefined,
|
||||
},
|
||||
pageLayoutTabId: {
|
||||
toCompare: false,
|
||||
toCompare: true,
|
||||
toStringify: false,
|
||||
universalProperty: 'pageLayoutTabUniversalIdentifier',
|
||||
isOverridable: true,
|
||||
|
||||
+146
@@ -0,0 +1,146 @@
|
||||
import { Test, type TestingModule } from '@nestjs/testing';
|
||||
|
||||
import { PageLayoutTabLayoutMode } from 'twenty-shared/types';
|
||||
|
||||
import { createEmptyFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/constant/create-empty-flat-entity-maps.constant';
|
||||
import { FlatPageLayoutWidgetTypeValidatorService } from 'src/engine/metadata-modules/flat-page-layout-widget/services/flat-page-layout-widget-type-validator.service';
|
||||
import { type FlatPageLayoutWidget } from 'src/engine/metadata-modules/flat-page-layout-widget/types/flat-page-layout-widget.type';
|
||||
import { PageLayoutTabExceptionCode } from 'src/engine/metadata-modules/page-layout-tab/exceptions/page-layout-tab.exception';
|
||||
import { type UniversalFlatPageLayoutTab } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/types/universal-flat-page-layout-tab.type';
|
||||
import { FlatPageLayoutWidgetValidatorService } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/validators/services/flat-page-layout-widget-validator.service';
|
||||
|
||||
const EXISTING_TAB_UNIVERSAL_IDENTIFIER =
|
||||
'00000000-0000-0000-0000-000000000aa1';
|
||||
const MISSING_TAB_UNIVERSAL_IDENTIFIER = '00000000-0000-0000-0000-000000000aa2';
|
||||
const DESTINATION_TAB_UNIVERSAL_IDENTIFIER =
|
||||
'00000000-0000-0000-0000-000000000aa3';
|
||||
const WIDGET_UNIVERSAL_IDENTIFIER = '00000000-0000-0000-0000-000000000111';
|
||||
|
||||
const tab = (
|
||||
universalIdentifier = EXISTING_TAB_UNIVERSAL_IDENTIFIER,
|
||||
): UniversalFlatPageLayoutTab =>
|
||||
({
|
||||
universalIdentifier,
|
||||
layoutMode: PageLayoutTabLayoutMode.VERTICAL_LIST,
|
||||
}) as unknown as UniversalFlatPageLayoutTab;
|
||||
|
||||
const widget = (
|
||||
universalIdentifier = WIDGET_UNIVERSAL_IDENTIFIER,
|
||||
pageLayoutTabUniversalIdentifier = EXISTING_TAB_UNIVERSAL_IDENTIFIER,
|
||||
): FlatPageLayoutWidget =>
|
||||
({
|
||||
universalIdentifier,
|
||||
pageLayoutTabUniversalIdentifier,
|
||||
title: 'widget',
|
||||
type: 'FRONT_COMPONENT',
|
||||
gridPosition: { row: 0, column: 0, rowSpan: 12, columnSpan: 12 },
|
||||
position: { layoutMode: PageLayoutTabLayoutMode.VERTICAL_LIST, index: 0 },
|
||||
}) as unknown as FlatPageLayoutWidget;
|
||||
|
||||
const mapsFrom = (entities: { universalIdentifier: string }[]): any => {
|
||||
const maps = createEmptyFlatEntityMaps() as any;
|
||||
|
||||
for (const entity of entities) {
|
||||
maps.byUniversalIdentifier[entity.universalIdentifier] = entity;
|
||||
}
|
||||
|
||||
return maps;
|
||||
};
|
||||
|
||||
const buildUpdateArgs = ({
|
||||
update,
|
||||
tabs = [tab()],
|
||||
existingWidgets = [widget()],
|
||||
}: {
|
||||
update: Record<string, unknown>;
|
||||
tabs?: UniversalFlatPageLayoutTab[];
|
||||
existingWidgets?: FlatPageLayoutWidget[];
|
||||
}) =>
|
||||
({
|
||||
universalIdentifier: WIDGET_UNIVERSAL_IDENTIFIER,
|
||||
flatEntityUpdate: update,
|
||||
optimisticFlatEntityMapsAndRelatedFlatEntityMaps: {
|
||||
flatPageLayoutTabMaps: mapsFrom(tabs),
|
||||
flatPageLayoutWidgetMaps: mapsFrom(existingWidgets),
|
||||
},
|
||||
additionalCacheDataMaps: { featureFlagsMap: {} },
|
||||
workspaceId: 'workspace-id',
|
||||
buildOptions: {} as never,
|
||||
}) as unknown as Parameters<
|
||||
FlatPageLayoutWidgetValidatorService['validateFlatPageLayoutWidgetUpdate']
|
||||
>[0];
|
||||
|
||||
describe('FlatPageLayoutWidgetValidatorService', () => {
|
||||
let service: FlatPageLayoutWidgetValidatorService;
|
||||
|
||||
beforeEach(async () => {
|
||||
const moduleRef: TestingModule = await Test.createTestingModule({
|
||||
providers: [
|
||||
FlatPageLayoutWidgetValidatorService,
|
||||
{
|
||||
provide: FlatPageLayoutWidgetTypeValidatorService,
|
||||
useValue: {
|
||||
validateFlatPageLayoutWidgetTypeSpecificitiesForCreation: () => [],
|
||||
validateFlatPageLayoutWidgetTypeSpecificitiesForUpdate: () => [],
|
||||
},
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
|
||||
service = moduleRef.get(FlatPageLayoutWidgetValidatorService);
|
||||
});
|
||||
|
||||
describe('validateFlatPageLayoutWidgetUpdate', () => {
|
||||
it('rejects moving a widget to an unknown tab', async () => {
|
||||
const result = await service.validateFlatPageLayoutWidgetUpdate(
|
||||
buildUpdateArgs({
|
||||
update: {
|
||||
pageLayoutTabUniversalIdentifier: MISSING_TAB_UNIVERSAL_IDENTIFIER,
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result.errors.map((error) => error.code)).toContain(
|
||||
PageLayoutTabExceptionCode.PAGE_LAYOUT_TAB_NOT_FOUND,
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects moving an overridden widget to an unknown tab', async () => {
|
||||
const result = await service.validateFlatPageLayoutWidgetUpdate(
|
||||
buildUpdateArgs({
|
||||
update: {
|
||||
universalOverrides: {
|
||||
pageLayoutTabUniversalIdentifier:
|
||||
MISSING_TAB_UNIVERSAL_IDENTIFIER,
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result.errors.map((error) => error.code)).toContain(
|
||||
PageLayoutTabExceptionCode.PAGE_LAYOUT_TAB_NOT_FOUND,
|
||||
);
|
||||
});
|
||||
|
||||
it('accepts moving an overridden widget to a known tab and exposes the override target as the effective tab', async () => {
|
||||
const result = await service.validateFlatPageLayoutWidgetUpdate(
|
||||
buildUpdateArgs({
|
||||
update: {
|
||||
universalOverrides: {
|
||||
pageLayoutTabUniversalIdentifier:
|
||||
DESTINATION_TAB_UNIVERSAL_IDENTIFIER,
|
||||
},
|
||||
},
|
||||
tabs: [tab(), tab(DESTINATION_TAB_UNIVERSAL_IDENTIFIER)],
|
||||
}),
|
||||
);
|
||||
|
||||
expect(result.errors.map((error) => error.code)).not.toContain(
|
||||
PageLayoutTabExceptionCode.PAGE_LAYOUT_TAB_NOT_FOUND,
|
||||
);
|
||||
expect(
|
||||
result.flatEntityMinimalInformation.pageLayoutTabUniversalIdentifier,
|
||||
).toBe(DESTINATION_TAB_UNIVERSAL_IDENTIFIER);
|
||||
});
|
||||
});
|
||||
});
|
||||
+28
-3
@@ -17,6 +17,7 @@ import { validatePageLayoutWidgetGridPosition } from 'src/engine/metadata-module
|
||||
import { validatePageLayoutWidgetVerticalListPosition } from 'src/engine/metadata-modules/page-layout-widget/utils/validate-page-layout-widget-vertical-list-position.util';
|
||||
import { validateWidgetGridPosition } from 'src/engine/metadata-modules/page-layout-widget/utils/validate-widget-grid-position.util';
|
||||
import { type UniversalFlatPageLayoutTab } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/types/universal-flat-page-layout-tab.type';
|
||||
import { type UniversalFlatPageLayoutWidget } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/types/universal-flat-page-layout-widget.type';
|
||||
import {
|
||||
FailedFlatEntityValidation,
|
||||
FlatEntityValidationError,
|
||||
@@ -70,19 +71,31 @@ export class FlatPageLayoutWidgetValidatorService {
|
||||
...flatEntityUpdate,
|
||||
};
|
||||
|
||||
const effectivePageLayoutTabUniversalIdentifier =
|
||||
this.getEffectivePageLayoutTabUniversalIdentifier(
|
||||
updatedFlatPageLayoutWidget,
|
||||
);
|
||||
|
||||
validationResult.flatEntityMinimalInformation = {
|
||||
...validationResult.flatEntityMinimalInformation,
|
||||
pageLayoutTabUniversalIdentifier:
|
||||
updatedFlatPageLayoutWidget.pageLayoutTabUniversalIdentifier,
|
||||
effectivePageLayoutTabUniversalIdentifier,
|
||||
};
|
||||
|
||||
const referencedPageLayoutTab = findFlatEntityByUniversalIdentifier({
|
||||
universalIdentifier:
|
||||
updatedFlatPageLayoutWidget.pageLayoutTabUniversalIdentifier,
|
||||
universalIdentifier: effectivePageLayoutTabUniversalIdentifier,
|
||||
flatEntityMaps:
|
||||
optimisticFlatEntityMapsAndRelatedFlatEntityMaps.flatPageLayoutTabMaps,
|
||||
});
|
||||
|
||||
if (!isDefined(referencedPageLayoutTab)) {
|
||||
validationResult.errors.push({
|
||||
code: PageLayoutTabExceptionCode.PAGE_LAYOUT_TAB_NOT_FOUND,
|
||||
message: t`Page layout tab not found`,
|
||||
userFriendlyMessage: msg`Page layout tab not found`,
|
||||
});
|
||||
}
|
||||
|
||||
const gridPositionErrors = this.validateGridPosition({
|
||||
gridPosition: updatedFlatPageLayoutWidget.gridPosition,
|
||||
widgetTitle: updatedFlatPageLayoutWidget.title,
|
||||
@@ -235,6 +248,18 @@ export class FlatPageLayoutWidgetValidatorService {
|
||||
return validationResult;
|
||||
}
|
||||
|
||||
private getEffectivePageLayoutTabUniversalIdentifier(
|
||||
widget: Pick<
|
||||
UniversalFlatPageLayoutWidget,
|
||||
'pageLayoutTabUniversalIdentifier' | 'universalOverrides'
|
||||
>,
|
||||
): string {
|
||||
return (
|
||||
widget.universalOverrides?.pageLayoutTabUniversalIdentifier ??
|
||||
widget.pageLayoutTabUniversalIdentifier
|
||||
);
|
||||
}
|
||||
|
||||
private validateGridPosition({
|
||||
gridPosition,
|
||||
widgetTitle,
|
||||
|
||||
+89
-1
@@ -1,7 +1,10 @@
|
||||
import { createOnePageLayoutTab } from 'test/integration/metadata/suites/page-layout-tab/utils/create-one-page-layout-tab.util';
|
||||
import { destroyOnePageLayoutTab } from 'test/integration/metadata/suites/page-layout-tab/utils/destroy-one-page-layout-tab.util';
|
||||
import { findPageLayoutTabs } from 'test/integration/metadata/suites/page-layout-tab/utils/find-page-layout-tabs.util';
|
||||
import { findPageLayoutWidgets } from 'test/integration/metadata/suites/page-layout-widget/utils/find-page-layout-widgets.util';
|
||||
import { updateOnePageLayoutWidget } from 'test/integration/metadata/suites/page-layout-widget/utils/update-one-page-layout-widget.util';
|
||||
import { findPageLayouts } from 'test/integration/metadata/suites/page-layout/utils/find-page-layouts.util';
|
||||
import { PageLayoutTabLayoutMode } from 'twenty-shared/types';
|
||||
|
||||
import { PageLayoutType } from 'src/engine/metadata-modules/page-layout/enums/page-layout-type.enum';
|
||||
|
||||
@@ -17,7 +20,10 @@ const WIDGET_OVERRIDE_GQL_FIELDS = `
|
||||
`;
|
||||
|
||||
describe('Page layout widget override behavior', () => {
|
||||
let seededPageLayoutId: string;
|
||||
let seededWidgetId: string;
|
||||
let seededWidgetOriginalPageLayoutTabId: string;
|
||||
let seededWidgetOriginalTabLayoutMode: PageLayoutTabLayoutMode;
|
||||
let seededWidgetOriginalTitle: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
@@ -32,15 +38,27 @@ describe('Page layout widget override behavior', () => {
|
||||
|
||||
expect(recordPageLayout).toBeDefined();
|
||||
|
||||
seededPageLayoutId = recordPageLayout!.id;
|
||||
|
||||
const { data: tabsData } = await findPageLayoutTabs({
|
||||
expectToFail: false,
|
||||
input: { pageLayoutId: recordPageLayout!.id },
|
||||
input: { pageLayoutId: seededPageLayoutId },
|
||||
gqlFields: `
|
||||
id
|
||||
title
|
||||
position
|
||||
layoutMode
|
||||
pageLayoutId
|
||||
`,
|
||||
});
|
||||
|
||||
expect(tabsData.getPageLayoutTabs.length).toBeGreaterThanOrEqual(1);
|
||||
|
||||
const firstTabId = tabsData.getPageLayoutTabs[0].id;
|
||||
|
||||
seededWidgetOriginalTabLayoutMode = tabsData.getPageLayoutTabs[0]
|
||||
.layoutMode as PageLayoutTabLayoutMode;
|
||||
|
||||
const { data: widgetsData } = await findPageLayoutWidgets({
|
||||
expectToFail: false,
|
||||
input: { pageLayoutTabId: firstTabId },
|
||||
@@ -52,6 +70,7 @@ describe('Page layout widget override behavior', () => {
|
||||
const firstWidget = widgetsData.getPageLayoutWidgets[0];
|
||||
|
||||
seededWidgetId = firstWidget.id;
|
||||
seededWidgetOriginalPageLayoutTabId = firstWidget.pageLayoutTabId;
|
||||
seededWidgetOriginalTitle = firstWidget.title;
|
||||
});
|
||||
|
||||
@@ -60,6 +79,7 @@ describe('Page layout widget override behavior', () => {
|
||||
expectToFail: false,
|
||||
input: {
|
||||
id: seededWidgetId,
|
||||
pageLayoutTabId: seededWidgetOriginalPageLayoutTabId,
|
||||
title: seededWidgetOriginalTitle,
|
||||
},
|
||||
gqlFields: WIDGET_OVERRIDE_GQL_FIELDS,
|
||||
@@ -81,6 +101,74 @@ describe('Page layout widget override behavior', () => {
|
||||
expect(data.updatePageLayoutWidget.title).toBe(overriddenTitle);
|
||||
});
|
||||
|
||||
it('should move a seeded widget to another tab through an override', async () => {
|
||||
let destinationTabId: string | undefined;
|
||||
|
||||
try {
|
||||
const { data: tabData } = await createOnePageLayoutTab({
|
||||
expectToFail: false,
|
||||
input: {
|
||||
title: `Widget Move Override ${Date.now()}`,
|
||||
pageLayoutId: seededPageLayoutId,
|
||||
layoutMode: seededWidgetOriginalTabLayoutMode,
|
||||
},
|
||||
});
|
||||
|
||||
destinationTabId = tabData.createPageLayoutTab.id;
|
||||
|
||||
const { data } = await updateOnePageLayoutWidget({
|
||||
expectToFail: false,
|
||||
input: {
|
||||
id: seededWidgetId,
|
||||
pageLayoutTabId: destinationTabId,
|
||||
},
|
||||
gqlFields: WIDGET_OVERRIDE_GQL_FIELDS,
|
||||
});
|
||||
|
||||
expect(data.updatePageLayoutWidget.pageLayoutTabId).toBe(
|
||||
destinationTabId,
|
||||
);
|
||||
|
||||
const { data: originalTabWidgetsData } = await findPageLayoutWidgets({
|
||||
expectToFail: false,
|
||||
input: { pageLayoutTabId: seededWidgetOriginalPageLayoutTabId },
|
||||
gqlFields: WIDGET_OVERRIDE_GQL_FIELDS,
|
||||
});
|
||||
|
||||
expect(
|
||||
originalTabWidgetsData.getPageLayoutWidgets.map((widget) => widget.id),
|
||||
).not.toContain(seededWidgetId);
|
||||
|
||||
const { data: destinationTabWidgetsData } = await findPageLayoutWidgets({
|
||||
expectToFail: false,
|
||||
input: { pageLayoutTabId: destinationTabId },
|
||||
gqlFields: WIDGET_OVERRIDE_GQL_FIELDS,
|
||||
});
|
||||
|
||||
expect(
|
||||
destinationTabWidgetsData.getPageLayoutWidgets.map(
|
||||
(widget) => widget.id,
|
||||
),
|
||||
).toContain(seededWidgetId);
|
||||
} finally {
|
||||
await updateOnePageLayoutWidget({
|
||||
expectToFail: false,
|
||||
input: {
|
||||
id: seededWidgetId,
|
||||
pageLayoutTabId: seededWidgetOriginalPageLayoutTabId,
|
||||
},
|
||||
gqlFields: WIDGET_OVERRIDE_GQL_FIELDS,
|
||||
});
|
||||
|
||||
if (destinationTabId !== undefined) {
|
||||
await destroyOnePageLayoutTab({
|
||||
expectToFail: false,
|
||||
input: { id: destinationTabId },
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('should return the overridden title when querying the widget', async () => {
|
||||
const { data: tabsData } = await findPageLayoutTabs({
|
||||
expectToFail: false,
|
||||
|
||||
+4
@@ -42,12 +42,16 @@ export const updateOnePageLayoutWidgetQueryFactory = ({
|
||||
variables: {
|
||||
id: input.id,
|
||||
input: {
|
||||
pageLayoutTabId: input.pageLayoutTabId,
|
||||
title: input.title,
|
||||
type: input.type,
|
||||
objectMetadataId: input.objectMetadataId,
|
||||
gridPosition: input.gridPosition,
|
||||
position: input.position,
|
||||
configuration: input.configuration,
|
||||
conditionalDisplay: input.conditionalDisplay,
|
||||
conditionalAvailabilityExpression:
|
||||
input.conditionalAvailabilityExpression,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
+94
@@ -229,4 +229,98 @@ describe('Page layout with tabs update should succeed', () => {
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it('should move a widget to another tab when saving layout tabs and widgets', async () => {
|
||||
const widgetId = v4();
|
||||
|
||||
await updateOnePageLayoutWithTabsAndWidgets({
|
||||
expectToFail: false,
|
||||
input: {
|
||||
id: testPageLayoutId,
|
||||
name: 'Layout Before Widget Move',
|
||||
type: PageLayoutType.RECORD_PAGE,
|
||||
objectMetadataId: null,
|
||||
tabs: [
|
||||
{
|
||||
id: testTabId1,
|
||||
title: 'Source Tab',
|
||||
position: 0,
|
||||
widgets: [
|
||||
{
|
||||
id: widgetId,
|
||||
pageLayoutTabId: testTabId1,
|
||||
title: 'Iframe Widget',
|
||||
type: WidgetType.IFRAME,
|
||||
objectMetadataId: null,
|
||||
gridPosition: {
|
||||
row: 0,
|
||||
column: 0,
|
||||
rowSpan: 1,
|
||||
columnSpan: 1,
|
||||
},
|
||||
configuration: MOCK_IFRAME_CONFIGURATION,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
id: testTabId2,
|
||||
title: 'Destination Tab',
|
||||
position: 1,
|
||||
widgets: [],
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
|
||||
const { data } = await updateOnePageLayoutWithTabsAndWidgets({
|
||||
expectToFail: false,
|
||||
input: {
|
||||
id: testPageLayoutId,
|
||||
name: 'Layout After Widget Move',
|
||||
type: PageLayoutType.RECORD_PAGE,
|
||||
objectMetadataId: null,
|
||||
tabs: [
|
||||
{
|
||||
id: testTabId1,
|
||||
title: 'Source Tab',
|
||||
position: 0,
|
||||
widgets: [],
|
||||
},
|
||||
{
|
||||
id: testTabId2,
|
||||
title: 'Destination Tab',
|
||||
position: 1,
|
||||
widgets: [
|
||||
{
|
||||
id: widgetId,
|
||||
pageLayoutTabId: testTabId2,
|
||||
title: 'Iframe Widget',
|
||||
type: WidgetType.IFRAME,
|
||||
objectMetadataId: null,
|
||||
gridPosition: {
|
||||
row: 0,
|
||||
column: 0,
|
||||
rowSpan: 1,
|
||||
columnSpan: 1,
|
||||
},
|
||||
configuration: MOCK_IFRAME_CONFIGURATION,
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
|
||||
const updatedTabs = data.updatePageLayoutWithTabsAndWidgets.tabs ?? [];
|
||||
const sourceTab = updatedTabs.find((tab) => tab.id === testTabId1);
|
||||
const destinationTab = updatedTabs.find((tab) => tab.id === testTabId2);
|
||||
|
||||
expect(sourceTab?.widgets).toHaveLength(0);
|
||||
expect(destinationTab?.widgets).toHaveLength(1);
|
||||
expect(destinationTab?.widgets?.[0]).toMatchObject({
|
||||
id: widgetId,
|
||||
pageLayoutTabId: testTabId2,
|
||||
title: 'Iframe Widget',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user