diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/MorphRelationOneToManyFieldDisplay.tsx b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/MorphRelationOneToManyFieldDisplay.tsx index 1a21f98847..46b2c8ce79 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/MorphRelationOneToManyFieldDisplay.tsx +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/MorphRelationOneToManyFieldDisplay.tsx @@ -1,6 +1,7 @@ import { RecordChip } from '@/object-record/components/RecordChip'; import { FieldContext } from '@/object-record/record-field/ui/contexts/FieldContext'; import { useFieldFocus } from '@/object-record/record-field/ui/hooks/useFieldFocus'; +import { MAX_RELATION_CHIPS_DISPLAYED_INLINE } from '@/object-record/record-field/ui/meta-types/display/constants/MaxRelationChipsDisplayedInline'; import { useMorphRelationFromManyFieldDisplay } from '@/object-record/record-field/ui/meta-types/hooks/useMorphRelationFromManyFieldDisplay'; import { ExpandableList } from '@/ui/layout/expandable-list/components/ExpandableList'; @@ -39,7 +40,10 @@ export const MorphRelationOneToManyFieldDisplay = () => { ); return ( - + {flattenMorphValuesWithObjectNameSingular .filter(isDefined) .map(({ objectNameSingular, record }) => { diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/RelationFromManyFieldDisplay.tsx b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/RelationFromManyFieldDisplay.tsx index cdd8f1661e..98043a7291 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/RelationFromManyFieldDisplay.tsx +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/RelationFromManyFieldDisplay.tsx @@ -9,6 +9,7 @@ import { RecordChip } from '@/object-record/components/RecordChip'; import { isActivityTargetField } from '@/object-record/record-field-list/utils/categorizeRelationFields'; import { FieldContext } from '@/object-record/record-field/ui/contexts/FieldContext'; import { useFieldFocus } from '@/object-record/record-field/ui/hooks/useFieldFocus'; +import { MAX_RELATION_CHIPS_DISPLAYED_INLINE } from '@/object-record/record-field/ui/meta-types/display/constants/MaxRelationChipsDisplayedInline'; import { useRelationFromManyFieldDisplay } from '@/object-record/record-field/ui/meta-types/hooks/useRelationFromManyFieldDisplay'; import { extractTargetRecordsFromJunction } from '@/object-record/record-field/ui/utils/junction/extractTargetRecordsFromJunction'; import { getJunctionConfig } from '@/object-record/record-field/ui/utils/junction/getJunctionConfig'; @@ -110,13 +111,20 @@ export const RelationFromManyFieldDisplay = () => { if (isFocused) { return ( - + {chips} ); } - return {chips}; + return ( + + {chips.slice(0, MAX_RELATION_CHIPS_DISPLAYED_INLINE)} + + ); } if (isJunctionRelation && isDefined(junctionConfig)) { @@ -150,7 +158,10 @@ export const RelationFromManyFieldDisplay = () => { } return ( - + {targetRecordsWithMetadata.map(({ record, objectMetadata }) => ( { if (isRelationFromActivityTargets) { return ( - + {activityTargetObjectRecords.filter(isDefined).map((record) => ( { } return ( - + {fieldValue.filter(isDefined).map((record) => { const recordChipData = generateRecordChipData(record); return ( diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/constants/MaxRelationChipsDisplayedInline.ts b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/constants/MaxRelationChipsDisplayedInline.ts new file mode 100644 index 0000000000..b64c1ee5a0 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/constants/MaxRelationChipsDisplayedInline.ts @@ -0,0 +1 @@ +export const MAX_RELATION_CHIPS_DISPLAYED_INLINE = 10; diff --git a/packages/twenty-front/src/modules/ui/layout/expandable-list/components/ExpandableList.tsx b/packages/twenty-front/src/modules/ui/layout/expandable-list/components/ExpandableList.tsx index 925078e712..1a00601795 100644 --- a/packages/twenty-front/src/modules/ui/layout/expandable-list/components/ExpandableList.tsx +++ b/packages/twenty-front/src/modules/ui/layout/expandable-list/components/ExpandableList.tsx @@ -53,6 +53,8 @@ const StyledUnShrinkableContainer = styled.div` export type ExpandableListProps = { isChipCountDisplayed?: boolean; + // Caps children mounted inline; the dropdown still renders all. + maxInlineCount?: number; }; export type ChildrenProperty = { @@ -63,9 +65,13 @@ export type ChildrenProperty = { export const ExpandableList = ({ children, isChipCountDisplayed: isChipCountDisplayedFromProps, + maxInlineCount, }: { children: ReactElement[]; } & ExpandableListProps) => { + const cappedChildren = isDefined(maxInlineCount) + ? children.slice(0, maxInlineCount) + : children; // isChipCountDisplayedInternal => uncontrolled display of the chip count. // isChipCountDisplayedFromProps => controlled display of the chip count. // If isChipCountDisplayedFromProps is provided, isChipCountDisplayedInternal is not taken into account. @@ -89,15 +95,15 @@ export const ExpandableList = ({ const containerRef = useRef(null); const [firstHiddenChildIndex, setFirstHiddenChildIndex] = useState( - children.length, + cappedChildren.length, ); const hiddenChildrenCount = children.length - firstHiddenChildIndex; const canDisplayChipCount = isChipCountDisplayed && hiddenChildrenCount > 0; const visibleChildren = isChipCountDisplayed - ? children.slice(0, firstHiddenChildIndex) - : children; + ? cappedChildren.slice(0, firstHiddenChildIndex) + : cappedChildren; const handleChipCountClick = useCallback((event: React.MouseEvent) => { event.stopPropagation(); @@ -105,12 +111,12 @@ export const ExpandableList = ({ }, []); const resetFirstHiddenChildIndex = useCallback(() => { - setFirstHiddenChildIndex(children.length); - }, [children.length]); + setFirstHiddenChildIndex(cappedChildren.length); + }, [cappedChildren.length]); useEffect(() => { resetFirstHiddenChildIndex(); - }, [isChipCountDisplayed, children.length, resetFirstHiddenChildIndex]); + }, [isChipCountDisplayed, cappedChildren.length, resetFirstHiddenChildIndex]); const handleClickOutside = () => { setIsListExpanded(false); diff --git a/packages/twenty-front/src/modules/ui/layout/expandable-list/components/__tests__/ExpandableList.test.tsx b/packages/twenty-front/src/modules/ui/layout/expandable-list/components/__tests__/ExpandableList.test.tsx new file mode 100644 index 0000000000..43157d6137 --- /dev/null +++ b/packages/twenty-front/src/modules/ui/layout/expandable-list/components/__tests__/ExpandableList.test.tsx @@ -0,0 +1,24 @@ +import { render, screen } from '@testing-library/react'; + +import { ExpandableList } from '@/ui/layout/expandable-list/components/ExpandableList'; + +const buildChips = (count: number) => + Array.from({ length: count }, (_, index) => ( + + chip-{index} + + )); + +describe('ExpandableList', () => { + it('mounts every child inline when no cap is provided', () => { + render({buildChips(5)}); + + expect(screen.getAllByTestId('chip')).toHaveLength(5); + }); + + it('mounts only maxInlineCount children inline when capped', () => { + render({buildChips(5)}); + + expect(screen.getAllByTestId('chip')).toHaveLength(2); + }); +}); diff --git a/packages/twenty-server/src/engine/api/common/common-nested-relations-processor/process-nested-relations-v2.helper.ts b/packages/twenty-server/src/engine/api/common/common-nested-relations-processor/process-nested-relations-v2.helper.ts index c4e5db15f7..b0d939f59c 100644 --- a/packages/twenty-server/src/engine/api/common/common-nested-relations-processor/process-nested-relations-v2.helper.ts +++ b/packages/twenty-server/src/engine/api/common/common-nested-relations-processor/process-nested-relations-v2.helper.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { FieldMetadataType, type ObjectRecord } from 'twenty-shared/types'; -import { isDefined } from 'twenty-shared/utils'; +import { isDefined, isValidUuid } from 'twenty-shared/utils'; import { type FindOptionsRelations, type ObjectLiteral } from 'typeorm'; import { computeMorphOrRelationFieldJoinColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-morph-or-relation-field-join-column-name.util'; @@ -26,10 +26,14 @@ import { } from 'src/engine/metadata-modules/flat-field-metadata/utils/build-field-maps-from-flat-object-metadata.util'; import { FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; import { GlobalWorkspaceDataSource } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-datasource'; +import { type WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository'; import { type WorkspaceSelectQueryBuilder } from 'src/engine/twenty-orm/repository/workspace-select-query-builder'; import { type RolePermissionConfig } from 'src/engine/twenty-orm/types/role-permission-config'; import { isFieldMetadataEntityOfType } from 'src/engine/utils/is-field-metadata-of-type.util'; +const EMPTY_RELATION_SENTINEL_RECORD_ID = + '00000000-0000-0000-0000-000000000000'; + @Injectable() export class ProcessNestedRelationsV2Helper { constructor() {} @@ -228,12 +232,15 @@ export class ProcessNestedRelationsV2Helper { const { relationResults, relationAggregatedFieldsResult } = await this.findRelations({ referenceQueryBuilder: targetObjectQueryBuilder, + targetObjectRepository, column: relationType === RelationType.ONE_TO_MANY ? `"${fieldMetadataTargetRelationColumnName}"` : 'id', ids: relationIds, - limit: limit * parentObjectRecords.length, + relationType, + perParentLimit: limit, + parentRecordsCount: parentObjectRecords.length, aggregate, sourceFieldName, targetObjectNameSingular, @@ -340,19 +347,25 @@ export class ProcessNestedRelationsV2Helper { private async findRelations({ referenceQueryBuilder, + targetObjectRepository, column, ids, - limit, + relationType, + perParentLimit, + parentRecordsCount, aggregate, sourceFieldName, targetObjectNameSingular, }: { // oxlint-disable-next-line typescript/no-explicit-any referenceQueryBuilder: WorkspaceSelectQueryBuilder; + targetObjectRepository: WorkspaceRepository; column: string; // oxlint-disable-next-line typescript/no-explicit-any ids: any[]; - limit: number; + relationType: RelationType; + perParentLimit: number; + parentRecordsCount: number; // oxlint-disable-next-line typescript/no-explicit-any aggregate: Record; sourceFieldName: string; @@ -401,20 +414,95 @@ export class ProcessNestedRelationsV2Helper { const queryBuilderOptions = referenceQueryBuilder.getFindOptions(); const columnWithoutQuotes = column.replace(/["']/g, ''); - const result = await referenceQueryBuilder - .setFindOptions({ - ...queryBuilderOptions, - select: { ...queryBuilderOptions.select, [columnWithoutQuotes]: true }, - }) - .where(`${column} IN (:...ids)`, { + const findOptionsWithJoinColumn = { + ...queryBuilderOptions, + select: { ...queryBuilderOptions.select, [columnWithoutQuotes]: true }, + }; + + if (relationType !== RelationType.ONE_TO_MANY) { + const result = await referenceQueryBuilder + .setFindOptions(findOptionsWithJoinColumn) + .where(`${column} IN (:...ids)`, { ids }) + .take(perParentLimit * parentRecordsCount) + .getMany(); + + return { relationResults: result, relationAggregatedFieldsResult }; + } + + const allowedRelationRecordIds = + await this.findRelationRecordIdsLimitedPerParent({ + targetObjectRepository, + targetObjectNameSingular, + column, ids, + perParentLimit, + }); + + const recordIdsToHydrate = + allowedRelationRecordIds.length > 0 + ? allowedRelationRecordIds + : [EMPTY_RELATION_SENTINEL_RECORD_ID]; + + const result = await referenceQueryBuilder + .setFindOptions(findOptionsWithJoinColumn) + .where(`id IN (:...recordIdsToHydrate)`, { + recordIdsToHydrate, }) - .take(limit) .getMany(); return { relationResults: result, relationAggregatedFieldsResult }; } + private async findRelationRecordIdsLimitedPerParent({ + targetObjectRepository, + targetObjectNameSingular, + column, + ids, + perParentLimit, + }: { + targetObjectRepository: WorkspaceRepository; + targetObjectNameSingular: string; + column: string; + ids: string[]; + perParentLimit: number; + }): Promise { + const sanitizedIds = ids.filter(isValidUuid); + + if (sanitizedIds.length === 0) { + return []; + } + + const perParentRecordIdsSql = targetObjectRepository + .createQueryBuilder(targetObjectNameSingular) + .select('id', 'id') + .where(`${column} = "lateralParents"."parentId"`) + .limit(perParentLimit) + .getQuery(); + + const parentValues = sanitizedIds.map((id) => `('${id}'::uuid)`).join(', '); + + const lateralFromSubquery = + `(SELECT "lateralRecords"."id" AS "id" ` + + `FROM (VALUES ${parentValues}) AS "lateralParents"("parentId") ` + + `CROSS JOIN LATERAL (${perParentRecordIdsSql}) AS "lateralRecords")`; + + const limitedRecordsQueryBuilder = targetObjectRepository + .createQueryBuilder() + .from(lateralFromSubquery, 'limited_relation_records') + .select('limited_relation_records.id', 'id'); + + limitedRecordsQueryBuilder.expressionMap.aliases = + limitedRecordsQueryBuilder.expressionMap.aliases.filter((alias) => + isDefined(alias.subQuery), + ); + + const limitedRecords = await limitedRecordsQueryBuilder.getRawMany<{ + id: string; + }>(); + + return limitedRecords.map((limitedRecord) => limitedRecord.id); + } + private assignRelationResults({ parentRecords, parentObjectRecordsAggregatedValues, diff --git a/packages/twenty-server/test/integration/graphql/suites/nested-relation-per-parent-limit.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/nested-relation-per-parent-limit.integration-spec.ts new file mode 100644 index 0000000000..1c4f8b7331 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/suites/nested-relation-per-parent-limit.integration-spec.ts @@ -0,0 +1,243 @@ +import { createManyOperationFactory } from 'test/integration/graphql/utils/create-many-operation-factory.util'; +import { deleteManyOperationFactory } from 'test/integration/graphql/utils/delete-many-operation-factory.util'; +import { destroyManyOperationFactory } from 'test/integration/graphql/utils/destroy-many-operation-factory.util'; +import { findManyOperationFactory } from 'test/integration/graphql/utils/find-many-operation-factory.util'; +import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util'; +import { QUERY_MAX_RECORDS_FROM_RELATION } from 'twenty-shared/constants'; + +const HOT_COMPANY_ID = '20202020-ffff-4000-8000-000000000001'; +const SMALL_COMPANY_ID = '20202020-ffff-4000-8000-000000000002'; +const SOFT_DELETED_COMPANY_ID = '20202020-ffff-4000-8000-000000000003'; +const EMPTY_COMPANY_ID = '20202020-ffff-4000-8000-000000000004'; + +// A parent with more children than the per-parent budget, plus a sibling with +// only a few. The nested relation must cap the hot parent and still return all +// of the small parent's children (no starvation from a flat global limit). +const HOT_PEOPLE_COUNT = QUERY_MAX_RECORDS_FROM_RELATION + 5; +const SMALL_PEOPLE_COUNT = 3; +const SOFT_DELETED_TOTAL_PEOPLE_COUNT = 12; +const SOFT_DELETED_REMOVED_PEOPLE_COUNT = 5; +const SOFT_DELETED_REMAINING_PEOPLE_COUNT = + SOFT_DELETED_TOTAL_PEOPLE_COUNT - SOFT_DELETED_REMOVED_PEOPLE_COUNT; + +const buildPersonId = (index: number) => + `20202020-eeee-4000-8000-${index.toString().padStart(12, '0')}`; + +let nextPersonIndex = 1; +const takePersonIds = (count: number) => + Array.from({ length: count }, () => buildPersonId(nextPersonIndex++)); + +const HOT_PERSON_IDS = takePersonIds(HOT_PEOPLE_COUNT); +const SMALL_PERSON_IDS = takePersonIds(SMALL_PEOPLE_COUNT); +const SOFT_DELETED_PERSON_IDS = takePersonIds(SOFT_DELETED_TOTAL_PEOPLE_COUNT); +const SOFT_DELETED_REMOVED_PERSON_IDS = SOFT_DELETED_PERSON_IDS.slice( + 0, + SOFT_DELETED_REMOVED_PEOPLE_COUNT, +); + +const ALL_PERSON_IDS = [ + ...HOT_PERSON_IDS, + ...SMALL_PERSON_IDS, + ...SOFT_DELETED_PERSON_IDS, +]; +const ALL_COMPANY_IDS = [ + HOT_COMPANY_ID, + SMALL_COMPANY_ID, + SOFT_DELETED_COMPANY_ID, + EMPTY_COMPANY_ID, +]; + +describe('Nested relation per-parent limit (e2e)', () => { + beforeAll(async () => { + const createCompanies = createManyOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + gqlFields: 'id', + data: [ + { id: HOT_COMPANY_ID, name: 'Hot relation company' }, + { id: SMALL_COMPANY_ID, name: 'Small relation company' }, + { id: SOFT_DELETED_COMPANY_ID, name: 'Soft-deleted relation company' }, + { id: EMPTY_COMPANY_ID, name: 'Empty relation company' }, + ], + upsert: true, + }); + + await makeGraphqlAPIRequest(createCompanies); + + const createPeople = createManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: 'id', + data: [ + ...HOT_PERSON_IDS.map((id) => ({ id, companyId: HOT_COMPANY_ID })), + ...SMALL_PERSON_IDS.map((id) => ({ id, companyId: SMALL_COMPANY_ID })), + ...SOFT_DELETED_PERSON_IDS.map((id) => ({ + id, + companyId: SOFT_DELETED_COMPANY_ID, + })), + ], + upsert: true, + }); + + await makeGraphqlAPIRequest(createPeople); + + // Soft-delete a subset so the per-parent selection must exclude them. + const softDeletePeople = deleteManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: 'id', + filter: { id: { in: SOFT_DELETED_REMOVED_PERSON_IDS } }, + }); + + await makeGraphqlAPIRequest(softDeletePeople); + }); + + afterAll(async () => { + const destroyPeople = destroyManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: 'id', + filter: { id: { in: ALL_PERSON_IDS } }, + }); + + await makeGraphqlAPIRequest(destroyPeople); + + const destroyCompanies = destroyManyOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + gqlFields: 'id', + filter: { id: { in: ALL_COMPANY_IDS } }, + }); + + await makeGraphqlAPIRequest(destroyCompanies); + }); + + it('caps a hot parent at the per-parent limit without starving siblings', async () => { + const queryData = findManyOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + gqlFields: ` + id + people { + edges { + node { + id + } + } + } + `, + filter: { id: { in: [HOT_COMPANY_ID, SMALL_COMPANY_ID] } }, + }); + + const response = await makeGraphqlAPIRequest(queryData); + + expect(response.body.data).toBeDefined(); + expect(response.body.errors).toBeUndefined(); + + const edges = response.body.data.companies.edges; + + const hotCompany = edges.find( + (edge: { node: { id: string } }) => edge.node.id === HOT_COMPANY_ID, + ); + const smallCompany = edges.find( + (edge: { node: { id: string } }) => edge.node.id === SMALL_COMPANY_ID, + ); + + // Hot parent is capped to the per-parent budget instead of dumping all of + // its children into a single connection. + expect(hotCompany.node.people.edges).toHaveLength( + QUERY_MAX_RECORDS_FROM_RELATION, + ); + + // Small parent still receives every one of its children — the hot parent no + // longer consumes the whole shared budget. + expect(smallCompany.node.people.edges).toHaveLength(SMALL_PEOPLE_COUNT); + }); + + it('excludes soft-deleted records from the per-parent selection', async () => { + const queryData = findManyOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + gqlFields: ` + id + people { + edges { + node { + id + } + } + } + `, + filter: { id: { in: [SOFT_DELETED_COMPANY_ID] } }, + }); + + const response = await makeGraphqlAPIRequest(queryData); + + expect(response.body.errors).toBeUndefined(); + + const company = response.body.data.companies.edges[0]; + + expect(company.node.people.edges).toHaveLength( + SOFT_DELETED_REMAINING_PEOPLE_COUNT, + ); + + const returnedIds = company.node.people.edges.map( + (edge: { node: { id: string } }) => edge.node.id, + ); + + for (const removedId of SOFT_DELETED_REMOVED_PERSON_IDS) { + expect(returnedIds).not.toContain(removedId); + } + }); + + it('returns an empty connection for a parent with no related records', async () => { + const queryData = findManyOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + gqlFields: ` + id + people { + edges { + node { + id + } + } + } + `, + filter: { id: { in: [EMPTY_COMPANY_ID] } }, + }); + + const response = await makeGraphqlAPIRequest(queryData); + + expect(response.body.errors).toBeUndefined(); + expect( + response.body.data.companies.edges[0].node.people.edges, + ).toHaveLength(0); + }); + + it('still resolves many-to-one relations from the other side', async () => { + const queryData = findManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: ` + id + company { + id + } + `, + filter: { id: { in: SMALL_PERSON_IDS } }, + }); + + const response = await makeGraphqlAPIRequest(queryData); + + expect(response.body.errors).toBeUndefined(); + + const edges = response.body.data.people.edges; + + expect(edges).toHaveLength(SMALL_PEOPLE_COUNT); + + for (const edge of edges) { + expect(edge.node.company.id).toBe(SMALL_COMPANY_ID); + } + }); +});