diff --git a/apps/nestjs-backend/src/features/field/open-api/field-open-api.service.ts b/apps/nestjs-backend/src/features/field/open-api/field-open-api.service.ts index 7ecb9c1108..417419ca7d 100644 --- a/apps/nestjs-backend/src/features/field/open-api/field-open-api.service.ts +++ b/apps/nestjs-backend/src/features/field/open-api/field-open-api.service.ts @@ -24,6 +24,7 @@ import type { ILinkFieldOptions, IConditionalRollupFieldOptions, IConditionalLookupOptions, + IRollupFieldOptions, IGetFieldsQuery, IFilter, IFilterItem, @@ -145,11 +146,22 @@ export class FieldOpenApiService { return true; } - private async validateConditionalRollupAggregation(hostTableId: string, field: IFieldInstance) { - const options = field.options as IConditionalRollupFieldOptions | undefined; - const expression = options?.expression; - const lookupFieldId = options?.lookupFieldId; - const foreignTableId = options?.foreignTableId; + private normalizeCellValueType(rawCellType: unknown): CellValueType { + if ( + typeof rawCellType === 'string' && + Object.values(CellValueType).includes(rawCellType as CellValueType) + ) { + return rawCellType as CellValueType; + } + return CellValueType.String; + } + + private async isRollupAggregationSupported(params: { + expression?: IRollupFieldOptions['expression']; + lookupFieldId?: string; + foreignTableId?: string; + }): Promise { + const { expression, lookupFieldId, foreignTableId } = params; if (!expression || !lookupFieldId || !foreignTableId) { return false; @@ -164,20 +176,42 @@ export class FieldOpenApiService { return false; } - const rawCellType = foreignField.cellValueType as string; - const availableTypes = new Set(Object.values(CellValueType)); - const cellValueType = availableTypes.has(rawCellType) - ? (rawCellType as CellValueType) - : CellValueType.String; + const cellValueType = this.normalizeCellValueType(foreignField.cellValueType); + return isRollupFunctionSupportedForCellValueType(expression, cellValueType); + } + + private async validateRollupAggregation(field: IFieldInstance): Promise { + if (!field.lookupOptions || !isLinkLookupOptions(field.lookupOptions)) { + return false; + } - const aggregationSupported = isRollupFunctionSupportedForCellValueType( + const options = field.options as IRollupFieldOptions | undefined; + return this.isRollupAggregationSupported({ + expression: options?.expression, + lookupFieldId: field.lookupOptions.lookupFieldId, + foreignTableId: field.lookupOptions.foreignTableId, + }); + } + + private async validateConditionalRollupAggregation(hostTableId: string, field: IFieldInstance) { + const options = field.options as IConditionalRollupFieldOptions | undefined; + const expression = options?.expression; + const lookupFieldId = options?.lookupFieldId; + const foreignTableId = options?.foreignTableId; + + const aggregationSupported = await this.isRollupAggregationSupported({ expression, - cellValueType - ); + lookupFieldId, + foreignTableId, + }); if (!aggregationSupported) { return false; } + if (!foreignTableId) { + return false; + } + return await this.validateFilterFieldReferences(hostTableId, foreignTableId, options?.filter); } @@ -206,6 +240,38 @@ export class FieldOpenApiService { return this.validateFilterFieldReferences(tableId, foreignTableId, meta?.filter); } + private async isFieldConfigurationValid( + tableId: string, + field: IFieldInstance + ): Promise { + if ( + field.lookupOptions && + field.type !== FieldType.ConditionalRollup && + !field.isConditionalLookup + ) { + const lookupValid = await this.validateLookupField(field); + if (!lookupValid) { + return false; + } + + if (field.type === FieldType.Rollup) { + return await this.validateRollupAggregation(field); + } + + return true; + } + + if (field.type === FieldType.ConditionalRollup) { + return await this.validateConditionalRollupAggregation(tableId, field); + } + + if (field.isConditionalLookup) { + return await this.validateConditionalLookup(tableId, field); + } + + return true; + } + private async findConditionalFilterDependentFields(startFieldIds: readonly string[]): Promise< Array<{ id: string; @@ -536,24 +602,8 @@ export class FieldOpenApiService { }); } - let hasError = false; - - if ( - field.lookupOptions && - field.type !== FieldType.ConditionalRollup && - !field.isConditionalLookup - ) { - const isValid = await this.validateLookupField(field); - hasError = !isValid; - } else if (field.type === FieldType.ConditionalRollup) { - const isValid = await this.validateConditionalRollupAggregation(tableId, field); - hasError = !isValid; - } else if (field.isConditionalLookup) { - const isValid = await this.validateConditionalLookup(tableId, field); - hasError = !isValid; - } - - await this.markError(tableId, field, hasError); + const isValid = await this.isFieldConfigurationValid(tableId, field); + await this.markError(tableId, field, !isValid); } async restoreReference(references: string[]) { @@ -1060,14 +1110,7 @@ export class FieldOpenApiService { }); for (const raw of dependentFieldRaws) { const instance = createFieldInstanceByRaw(raw); - let isValid = true; - - if (instance.type === FieldType.ConditionalRollup) { - isValid = await this.validateConditionalRollupAggregation(raw.tableId, instance); - } else if (instance.isConditionalLookup) { - isValid = await this.validateConditionalLookup(raw.tableId, instance); - } - + const isValid = await this.isFieldConfigurationValid(raw.tableId, instance); await this.markError(raw.tableId, instance, !isValid); } diff --git a/apps/nestjs-backend/src/features/record/query-builder/field-cte-visitor.ts b/apps/nestjs-backend/src/features/record/query-builder/field-cte-visitor.ts index 6938663cb5..d53a46f0fa 100644 --- a/apps/nestjs-backend/src/features/record/query-builder/field-cte-visitor.ts +++ b/apps/nestjs-backend/src/features/record/query-builder/field-cte-visitor.ts @@ -39,6 +39,7 @@ import { DbFieldType, SortFunc, isLinkLookupOptions, + normalizeConditionalLimit, } from '@teable/core'; import type { Knex } from 'knex'; import { match } from 'ts-pattern'; @@ -1283,8 +1284,9 @@ export class FieldCteVisitor implements IFieldVisitor { aggregateSourceQuery.orderByRaw(orderByClause); } - if (supportsOrdering && typeof limit === 'number' && Number.isFinite(limit) && limit > 0) { - aggregateSourceQuery.limit(limit); + if (supportsOrdering) { + const resolvedLimit = normalizeConditionalLimit(limit); + aggregateSourceQuery.limit(resolvedLimit); } const aggregateQuery = this.qb.client @@ -1446,9 +1448,8 @@ export class FieldCteVisitor implements IFieldVisitor { aggregateSourceQuery.orderByRaw(orderByClause); } - if (typeof limit === 'number' && limit > 0) { - aggregateSourceQuery.limit(limit); - } + const resolvedLimit = normalizeConditionalLimit(limit); + aggregateSourceQuery.limit(resolvedLimit); const aggregateQuery = this.qb.client .queryBuilder() diff --git a/apps/nestjs-backend/test/conditional-lookup.e2e-spec.ts b/apps/nestjs-backend/test/conditional-lookup.e2e-spec.ts index dc47a16ded..2317c50fc6 100644 --- a/apps/nestjs-backend/test/conditional-lookup.e2e-spec.ts +++ b/apps/nestjs-backend/test/conditional-lookup.e2e-spec.ts @@ -3393,4 +3393,109 @@ describe('OpenAPI Conditional Lookup field (e2e)', () => { expect(bobSummary.fields[scoresWithoutExcludedField.id]).toEqual([7]); }); }); + + describe('limit enforcement', () => { + const limitCap = Number(process.env.CONDITIONAL_QUERY_MAX_LIMIT ?? '5000'); + const totalActive = limitCap + 2; + let foreign: ITableFullVo; + let host: ITableFullVo; + let titleId: string; + let statusId: string; + let statusFilterId: string; + let lookupFieldId: string; + const activeTitles = Array.from({ length: totalActive }, (_, idx) => `Active ${idx + 1}`); + + beforeAll(async () => { + foreign = await createTable(baseId, { + name: 'ConditionalLookup_Limit_Foreign', + fields: [ + { name: 'Title', type: FieldType.SingleLineText, options: {} } as IFieldRo, + { name: 'Status', type: FieldType.SingleLineText, options: {} } as IFieldRo, + ], + records: [ + ...activeTitles.map((title) => ({ + fields: { Title: title, Status: 'Active' }, + })), + { fields: { Title: 'Closed Item', Status: 'Closed' } }, + ], + }); + titleId = foreign.fields.find((field) => field.name === 'Title')!.id; + statusId = foreign.fields.find((field) => field.name === 'Status')!.id; + + host = await createTable(baseId, { + name: 'ConditionalLookup_Limit_Host', + fields: [{ name: 'StatusFilter', type: FieldType.SingleLineText, options: {} } as IFieldRo], + records: [{ fields: { StatusFilter: 'Active' } }], + }); + statusFilterId = host.fields.find((field) => field.name === 'StatusFilter')!.id; + }); + + afterAll(async () => { + await permanentDeleteTable(baseId, host.id); + await permanentDeleteTable(baseId, foreign.id); + }); + + it('rejects creating a conditional lookup with limit beyond configured maximum', async () => { + const statusMatchFilter: IFilter = { + conjunction: 'and', + filterSet: [ + { + fieldId: statusId, + operator: 'is', + value: { type: 'field', fieldId: statusFilterId }, + }, + ], + }; + + await createField( + host.id, + { + name: 'TooManyRecords', + type: FieldType.SingleLineText, + isLookup: true, + isConditionalLookup: true, + lookupOptions: { + foreignTableId: foreign.id, + lookupFieldId: titleId, + filter: statusMatchFilter, + limit: limitCap + 1, + } as ILookupOptionsRo, + } as IFieldRo, + 400 + ); + }); + + it('caps resolved lookup results to the maximum limit when limit is omitted', async () => { + const statusMatchFilter: IFilter = { + conjunction: 'and', + filterSet: [ + { + fieldId: statusId, + operator: 'is', + value: { type: 'field', fieldId: statusFilterId }, + }, + ], + }; + + const lookupField = await createField(host.id, { + name: 'Limited Titles', + type: FieldType.SingleLineText, + isLookup: true, + isConditionalLookup: true, + lookupOptions: { + foreignTableId: foreign.id, + lookupFieldId: titleId, + filter: statusMatchFilter, + } as ILookupOptionsRo, + } as IFieldRo); + lookupFieldId = lookupField.id; + + const record = await getRecord(host.id, host.records[0].id); + const values = record.fields[lookupFieldId] as string[]; + expect(Array.isArray(values)).toBe(true); + expect(values.length).toBe(limitCap); + expect(values).toEqual(activeTitles.slice(0, limitCap)); + expect(values).not.toContain(activeTitles[limitCap]); + }); + }); }); diff --git a/apps/nestjs-backend/test/conditional-rollup.e2e-spec.ts b/apps/nestjs-backend/test/conditional-rollup.e2e-spec.ts index ed79e1848d..7d1f3ffab8 100644 --- a/apps/nestjs-backend/test/conditional-rollup.e2e-spec.ts +++ b/apps/nestjs-backend/test/conditional-rollup.e2e-spec.ts @@ -141,6 +141,112 @@ describe('OpenAPI Conditional Rollup field (e2e)', () => { }); }); + describe('limit enforcement', () => { + const limitCap = Number(process.env.CONDITIONAL_QUERY_MAX_LIMIT ?? '5000'); + const totalActive = limitCap + 3; + const activeTitles = Array.from({ length: totalActive }, (_, idx) => `Score ${idx + 1}`); + let foreign: ITableFullVo; + let host: ITableFullVo; + let titleId: string; + let statusId: string; + let scoreId: string; + let statusFilterId: string; + + beforeAll(async () => { + foreign = await createTable(baseId, { + name: 'ConditionalRollup_Limit_Foreign', + fields: [ + { name: 'Title', type: FieldType.SingleLineText } as IFieldRo, + { name: 'Status', type: FieldType.SingleLineText } as IFieldRo, + { name: 'Score', type: FieldType.Number } as IFieldRo, + ], + records: [ + ...activeTitles.map((title, idx) => ({ + fields: { Title: title, Status: 'Active', Score: idx + 1 }, + })), + { fields: { Title: 'Closed Item', Status: 'Closed', Score: 999 } }, + ], + }); + titleId = foreign.fields.find((field) => field.name === 'Title')!.id; + statusId = foreign.fields.find((field) => field.name === 'Status')!.id; + scoreId = foreign.fields.find((field) => field.name === 'Score')!.id; + + host = await createTable(baseId, { + name: 'ConditionalRollup_Limit_Host', + fields: [{ name: 'StatusFilter', type: FieldType.SingleLineText } as IFieldRo], + records: [{ fields: { StatusFilter: 'Active' } }], + }); + statusFilterId = host.fields.find((field) => field.name === 'StatusFilter')!.id; + }); + + afterAll(async () => { + await permanentDeleteTable(baseId, host.id); + await permanentDeleteTable(baseId, foreign.id); + }); + + it('rejects creating conditional rollups with limit above the configured cap', async () => { + const statusMatchFilter: IFilter = { + conjunction: 'and', + filterSet: [ + { + fieldId: statusId, + operator: 'is', + value: { type: 'field', fieldId: statusFilterId }, + }, + ], + }; + + await createField( + host.id, + { + name: 'TooManyRollupValues', + type: FieldType.ConditionalRollup, + options: { + foreignTableId: foreign.id, + lookupFieldId: titleId, + expression: 'array_compact({values})', + filter: statusMatchFilter, + sort: { fieldId: scoreId, order: SortFunc.Asc }, + limit: limitCap + 1, + } as IConditionalRollupFieldOptions, + } as IFieldRo, + 400 + ); + }); + + it('caps array aggregation results to the configured maximum when limit is omitted', async () => { + const statusMatchFilter: IFilter = { + conjunction: 'and', + filterSet: [ + { + fieldId: statusId, + operator: 'is', + value: { type: 'field', fieldId: statusFilterId }, + }, + ], + }; + + const rollupField = await createField(host.id, { + name: 'Limited Titles Rollup', + type: FieldType.ConditionalRollup, + options: { + foreignTableId: foreign.id, + lookupFieldId: titleId, + expression: 'array_compact({values})', + filter: statusMatchFilter, + sort: { fieldId: scoreId, order: SortFunc.Asc }, + } as IConditionalRollupFieldOptions, + } as IFieldRo); + + const record = await getRecord(host.id, host.records[0].id); + const values = record.fields[rollupField.id] as string[]; + expect(Array.isArray(values)).toBe(true); + expect(values.length).toBe(limitCap); + expect(values).toEqual(activeTitles.slice(0, limitCap)); + expect(values).not.toContain(activeTitles[limitCap]); + }); + }); + describe('filter option synchronization', () => { let foreign: ITableFullVo; let host: ITableFullVo; diff --git a/apps/nestjs-backend/test/rollup.e2e-spec.ts b/apps/nestjs-backend/test/rollup.e2e-spec.ts index 95cc0cec62..abbfef3b2e 100644 --- a/apps/nestjs-backend/test/rollup.e2e-spec.ts +++ b/apps/nestjs-backend/test/rollup.e2e-spec.ts @@ -22,8 +22,10 @@ import { import type { ITableFullVo } from '@teable/openapi'; import { createField, + convertField, createTable, permanentDeleteTable, + getField, getFields, initApp, updateRecord, @@ -746,6 +748,101 @@ describe('OpenAPI Rollup field (e2e)', () => { }); }); + describe('Rollup aggregation validation', () => { + it('keeps numeric aggregation valid for numeric sources', async () => { + const foreign = await createTable(baseId, { + name: 'RollupValidationForeign', + fields: [{ name: 'Amount', type: FieldType.Number } as IFieldRo], + }); + const host = await createTable(baseId, { + name: 'RollupValidationHost', + fields: [{ name: 'Label', type: FieldType.SingleLineText } as IFieldRo], + }); + const amountFieldId = foreign.fields.find((field) => field.name === 'Amount')!.id; + + try { + const linkField = await createField(host.id, { + name: 'Link to Foreign', + type: FieldType.Link, + options: { + relationship: Relationship.OneMany, + foreignTableId: foreign.id, + }, + } as IFieldRo); + + const rollupField = await createField(host.id, { + name: 'Sum Amount', + type: FieldType.Rollup, + options: { + expression: 'sum({values})', + }, + lookupOptions: { + foreignTableId: foreign.id, + linkFieldId: linkField.id, + lookupFieldId: amountFieldId, + } as ILookupOptionsRo, + } as IFieldRo); + + const fetched = await getField(host.id, rollupField.id); + expect(fetched.hasError).toBeFalsy(); + } finally { + await permanentDeleteTable(baseId, host.id); + await permanentDeleteTable(baseId, foreign.id); + } + }); + + it('marks rollup as errored when numeric source becomes text', async () => { + const foreign = await createTable(baseId, { + name: 'RollupValidationForeignConversion', + fields: [{ name: 'Amount', type: FieldType.Number } as IFieldRo], + }); + const host = await createTable(baseId, { + name: 'RollupValidationHostConversion', + fields: [{ name: 'Label', type: FieldType.SingleLineText } as IFieldRo], + }); + const amountFieldId = foreign.fields.find((field) => field.name === 'Amount')!.id; + + try { + const linkField = await createField(host.id, { + name: 'Link to Foreign', + type: FieldType.Link, + options: { + relationship: Relationship.OneMany, + foreignTableId: foreign.id, + }, + } as IFieldRo); + + const rollupField = await createField(host.id, { + name: 'Sum Amount', + type: FieldType.Rollup, + options: { + expression: 'sum({values})', + }, + lookupOptions: { + foreignTableId: foreign.id, + linkFieldId: linkField.id, + lookupFieldId: amountFieldId, + } as ILookupOptionsRo, + } as IFieldRo); + + const initial = await getField(host.id, rollupField.id); + expect(initial.hasError).toBeFalsy(); + + await convertField(foreign.id, amountFieldId, { + name: 'Amount', + type: FieldType.SingleLineText, + options: {}, + } as IFieldRo); + + const afterConvert = await getField(host.id, rollupField.id); + expect(afterConvert.hasError).toBe(true); + } finally { + await permanentDeleteTable(baseId, host.id); + await permanentDeleteTable(baseId, foreign.id); + } + }); + }); + describe('Roll up corner case', () => { let table1: ITableFullVo; let table2: ITableFullVo; diff --git a/apps/nestjs-backend/vitest-e2e.setup.ts b/apps/nestjs-backend/vitest-e2e.setup.ts index 2e8e294eed..fe2ec1ac13 100644 --- a/apps/nestjs-backend/vitest-e2e.setup.ts +++ b/apps/nestjs-backend/vitest-e2e.setup.ts @@ -78,6 +78,13 @@ function compileWorkerFile() { async function setup() { dotenv.config({ path: '../nextjs-app' }); + if (!process.env.CONDITIONAL_QUERY_MAX_LIMIT) { + process.env.CONDITIONAL_QUERY_MAX_LIMIT = '7'; + } + if (!process.env.CONDITIONAL_QUERY_DEFAULT_LIMIT) { + process.env.CONDITIONAL_QUERY_DEFAULT_LIMIT = process.env.CONDITIONAL_QUERY_MAX_LIMIT; + } + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const databaseUrl = process.env.PRISMA_DATABASE_URL!; diff --git a/apps/nextjs-app/config/tests/AppTestProviders.tsx b/apps/nextjs-app/config/tests/AppTestProviders.tsx index 07a3b67918..d23016a08a 100644 --- a/apps/nextjs-app/config/tests/AppTestProviders.tsx +++ b/apps/nextjs-app/config/tests/AppTestProviders.tsx @@ -1,8 +1,10 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import type { IAppContext } from '@teable/sdk/context'; import { AppContext, FieldContext, TableContext, ViewContext } from '@teable/sdk/context'; import { defaultLocale } from '@teable/sdk/context/app/i18n'; import type { IFieldInstance, IViewInstance } from '@teable/sdk/model'; import type { FC, PropsWithChildren } from 'react'; +import { useRef } from 'react'; import { I18nextTestStubProvider } from './I18nextTestStubProvider'; export const createAppContext = (context: Partial = {}) => { @@ -18,9 +20,25 @@ export const createAppContext = (context: Partial = {}) => { const MockProvider = createAppContext(); export const AppTestProviders: FC = ({ children }) => { + const queryClientRef = useRef(); + + if (!queryClientRef.current) { + queryClientRef.current = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + refetchOnWindowFocus: false, + }, + }, + }); + } + const queryClient = queryClientRef.current; + return ( - {children} + + {children} + ); }; diff --git a/apps/nextjs-app/src/features/app/components/field-setting/FieldOptions.tsx b/apps/nextjs-app/src/features/app/components/field-setting/FieldOptions.tsx index da549d57bf..6fd5858c18 100644 --- a/apps/nextjs-app/src/features/app/components/field-setting/FieldOptions.tsx +++ b/apps/nextjs-app/src/features/app/components/field-setting/FieldOptions.tsx @@ -1,3 +1,4 @@ +import { useQuery } from '@tanstack/react-query'; import type { IFieldVo, IDateFieldOptions, @@ -16,7 +17,15 @@ import type { IButtonFieldOptions, IConditionalRollupFieldOptions, } from '@teable/core'; -import { FieldType } from '@teable/core'; +import { + CellValueType, + FieldType, + getRollupFunctionsByCellValueType, + isLinkLookupOptions, +} from '@teable/core'; +import { getField } from '@teable/openapi'; +import { useFields } from '@teable/sdk/hooks'; +import { useMemo } from 'react'; import { ButtonOptions } from './options/ButtonOptions'; import { CheckboxOptions } from './options/CheckboxOptions'; import { ConditionalRollupOptions } from './options/ConditionalRollupOptions'; @@ -41,6 +50,22 @@ export interface IFieldOptionsProps { export const FieldOptions: React.FC = ({ field, onChange, onSave }) => { const { id, type, isLookup, cellValueType, isMultipleCellValue, options } = field; + const lookupField = useRollupLookupField(field.lookupOptions); + const lookupCellValueType = useMemo( + () => normalizeCellValueType(lookupField?.cellValueType), + [lookupField?.cellValueType] + ); + const normalizedFieldCellValueType = useMemo( + () => normalizeCellValueType(cellValueType), + [cellValueType] + ); + const effectiveRollupCellValueType = lookupCellValueType ?? normalizedFieldCellValueType; + const rollupAvailableExpressions = useMemo(() => { + return effectiveRollupCellValueType + ? getRollupFunctionsByCellValueType(effectiveRollupCellValueType) + : undefined; + }, [effectiveRollupCellValueType]); + const effectiveIsMultiple = isMultipleCellValue ?? lookupField?.isMultipleCellValue ?? undefined; switch (type) { case FieldType.SingleLineText: return ( @@ -144,8 +169,9 @@ export const FieldOptions: React.FC = ({ field, onChange, on ); @@ -170,3 +196,40 @@ export const FieldOptions: React.FC = ({ field, onChange, on return <>; } }; + +const normalizeCellValueType = (value: unknown): CellValueType | undefined => { + if ( + typeof value === 'string' && + (Object.values(CellValueType) as string[]).includes(value as CellValueType) + ) { + return value as CellValueType; + } + return undefined; +}; + +const useRollupLookupField = ( + lookupOptions: IFieldEditorRo['lookupOptions'] +): Pick | undefined => { + const linkOptions = isLinkLookupOptions(lookupOptions) ? lookupOptions : undefined; + const lookupFieldId = linkOptions?.lookupFieldId; + const foreignTableId = linkOptions?.foreignTableId; + const fields = useFields({ withHidden: true, withDenied: true }); + + const localLookupField = useMemo(() => { + if (!lookupFieldId) return undefined; + return fields.find((field) => field.id === lookupFieldId); + }, [fields, lookupFieldId]); + + const shouldFetchLookupField = Boolean(foreignTableId && lookupFieldId) && !localLookupField; + + const { data: remoteLookupField } = useQuery({ + queryKey: ['rollup-lookup-field', foreignTableId, lookupFieldId], + queryFn: async () => { + const res = await getField(foreignTableId!, lookupFieldId!); + return res.data; + }, + enabled: shouldFetchLookupField, + }); + + return localLookupField ?? remoteLookupField; +}; diff --git a/apps/nextjs-app/src/features/app/components/field-setting/options/ConditionalLookupOptions.tsx b/apps/nextjs-app/src/features/app/components/field-setting/options/ConditionalLookupOptions.tsx index 14871fed9f..f387eaff6d 100644 --- a/apps/nextjs-app/src/features/app/components/field-setting/options/ConditionalLookupOptions.tsx +++ b/apps/nextjs-app/src/features/app/components/field-setting/options/ConditionalLookupOptions.tsx @@ -1,4 +1,4 @@ -import type { IConditionalLookupOptions } from '@teable/core'; +import { CONDITIONAL_QUERY_DEFAULT_LIMIT, type IConditionalLookupOptions } from '@teable/core'; import { StandaloneViewProvider } from '@teable/sdk/context'; import { useBaseId, useTable, useTableId } from '@teable/sdk/hooks'; import type { IFieldInstance } from '@teable/sdk/model'; @@ -143,7 +143,7 @@ const ConditionalLookupForeignSection = ({ limit={limit} onSortChange={onSortChange} onLimitChange={onLimitChange} - defaultLimit={1} + defaultLimit={CONDITIONAL_QUERY_DEFAULT_LIMIT} toggleTestId="conditional-lookup-sort-limit-toggle" /> diff --git a/apps/nextjs-app/src/features/app/components/field-setting/options/ConditionalRollupOptions.tsx b/apps/nextjs-app/src/features/app/components/field-setting/options/ConditionalRollupOptions.tsx index f0db61240c..cb4eaad643 100644 --- a/apps/nextjs-app/src/features/app/components/field-setting/options/ConditionalRollupOptions.tsx +++ b/apps/nextjs-app/src/features/app/components/field-setting/options/ConditionalRollupOptions.tsx @@ -4,7 +4,12 @@ import type { RollupFunction, IRollupFieldOptions, } from '@teable/core'; -import { CellValueType, getRollupFunctionsByCellValueType, ROLLUP_FUNCTIONS } from '@teable/core'; +import { + CellValueType, + getRollupFunctionsByCellValueType, + ROLLUP_FUNCTIONS, + CONDITIONAL_QUERY_DEFAULT_LIMIT, +} from '@teable/core'; import { StandaloneViewProvider } from '@teable/sdk/context'; import { useBaseId, useFields, useTable, useTableId } from '@teable/sdk/hooks'; import type { IFieldInstance } from '@teable/sdk/model'; @@ -192,7 +197,7 @@ const ConditionalRollupForeignSection = (props: IConditionalRollupForeignSection limit={options.limit} onSortChange={(sortValue) => onOptionsChange({ sort: sortValue })} onLimitChange={(limitValue) => onOptionsChange({ limit: limitValue })} - defaultLimit={1} + defaultLimit={CONDITIONAL_QUERY_DEFAULT_LIMIT} toggleTestId="conditional-rollup-sort-limit-toggle" /> ) : null} diff --git a/apps/nextjs-app/src/features/app/components/field-setting/options/LinkedRecordSortLimitConfig.tsx b/apps/nextjs-app/src/features/app/components/field-setting/options/LinkedRecordSortLimitConfig.tsx index fa3057b20f..d280064849 100644 --- a/apps/nextjs-app/src/features/app/components/field-setting/options/LinkedRecordSortLimitConfig.tsx +++ b/apps/nextjs-app/src/features/app/components/field-setting/options/LinkedRecordSortLimitConfig.tsx @@ -1,4 +1,11 @@ -import { FieldType, SortFunc } from '@teable/core'; +import { + FieldType, + SortFunc, + clampConditionalLimit, + normalizeConditionalLimit, + CONDITIONAL_QUERY_DEFAULT_LIMIT, + CONDITIONAL_QUERY_MAX_LIMIT, +} from '@teable/core'; import { FieldCommand, FieldSelector, OrderSelect } from '@teable/sdk'; import { useFields } from '@teable/sdk/hooks'; import { Input, Switch } from '@teable/ui-lib/shadcn'; @@ -22,7 +29,9 @@ interface ILinkedRecordSortLimitConfigProps { toggleTestId?: string; } -const DEFAULT_LIMIT = 1; +const DEFAULT_LIMIT = CONDITIONAL_QUERY_DEFAULT_LIMIT; +const MAX_LIMIT = CONDITIONAL_QUERY_MAX_LIMIT; +const TOGGLE_FALLBACK_LIMIT = 1; export const LinkedRecordSortLimitConfig = ({ sort, @@ -38,6 +47,7 @@ export const LinkedRecordSortLimitConfig = ({ const sortLabel = t('table:field.editor.conditionalLookup.sortLabel'); const limitLabel = t('table:field.editor.conditionalLookup.limitLabel'); const limitPlaceholder = t('table:field.editor.conditionalLookup.limitPlaceholder'); + const limitHint = t('table:field.editor.conditionalLookup.limitHint', { limit: MAX_LIMIT }); const sortMissingTitle = t('table:field.editor.conditionalLookup.sortMissingWarningTitle'); const sortMissingDescription = t( 'table:field.editor.conditionalLookup.sortMissingWarningDescription' @@ -57,13 +67,25 @@ export const LinkedRecordSortLimitConfig = ({ useEffect(() => { if (limit != null) { - setLimitDraft(String(limit)); + const sanitized = clampConditionalLimit(limit); + if (sanitized != null) { + setLimitDraft(String(sanitized)); + if (sanitized !== limit) { + onLimitChange(sanitized); + } + return; + } + const fallback = normalizeConditionalLimit(TOGGLE_FALLBACK_LIMIT); + setLimitDraft(String(fallback)); + if (limit !== fallback) { + onLimitChange(fallback); + } return; } - if ((localOverride ?? derivedEnabled) === false) { + if (!sortLimitEnabled) { setLimitDraft(''); } - }, [derivedEnabled, limit, localOverride]); + }, [limit, onLimitChange, sortLimitEnabled]); useEffect(() => { if (localOverride !== null && derivedEnabled === localOverride) { @@ -77,8 +99,12 @@ export const LinkedRecordSortLimitConfig = ({ if (checked) { if (limit == null) { - const normalizedDefault = + const baseDefault = Number.isInteger(defaultLimit) && defaultLimit > 0 ? defaultLimit : DEFAULT_LIMIT; + const toggleDefault = baseDefault >= MAX_LIMIT ? TOGGLE_FALLBACK_LIMIT : baseDefault; + const normalizedDefault = + clampConditionalLimit(toggleDefault) ?? + normalizeConditionalLimit(TOGGLE_FALLBACK_LIMIT); setLimitDraft(String(normalizedDefault)); onLimitChange(normalizedDefault); } @@ -120,17 +146,24 @@ export const LinkedRecordSortLimitConfig = ({ return; } - setLimitDraft(value); if (value === '') { + setLimitDraft(''); onLimitChange(undefined); return; } const parsed = Number(value); - if (Number.isInteger(parsed) && parsed > 0) { - onLimitChange(parsed); + if (!Number.isInteger(parsed)) { return; } - onLimitChange(undefined); + const sanitized = clampConditionalLimit(parsed); + if (sanitized == null) { + setLimitDraft(value); + onLimitChange(undefined); + return; + } + + setLimitDraft(String(sanitized)); + onLimitChange(sanitized); }, [onLimitChange] ); @@ -145,6 +178,7 @@ export const LinkedRecordSortLimitConfig = ({ data-testid={toggleTestId} /> + {limitHint} {!sortLimitEnabled ? null : (
diff --git a/apps/nextjs-app/src/features/app/components/field-setting/options/RollupOptions.tsx b/apps/nextjs-app/src/features/app/components/field-setting/options/RollupOptions.tsx index bcb71fa6ed..9783099bca 100644 --- a/apps/nextjs-app/src/features/app/components/field-setting/options/RollupOptions.tsx +++ b/apps/nextjs-app/src/features/app/components/field-setting/options/RollupOptions.tsx @@ -17,9 +17,8 @@ import { BaseSingleSelect } from '@teable/sdk/components/filter/view-filter/comp import { RollupField } from '@teable/sdk/model'; import { isEmpty, isEqual } from 'lodash'; import { useTranslation } from 'next-i18next'; -import { useCallback, useMemo } from 'react'; +import { useCallback, useEffect, useMemo } from 'react'; import { RequireCom } from '@/features/app/blocks/setting/components/RequireCom'; -import { TimeZoneFormatting } from '../formatting/TimeZoneFormatting'; import { UnionFormatting } from '../formatting/UnionFormatting'; import { UnionShowAs } from '../show-as/UnionShowAs'; @@ -95,6 +94,20 @@ export const RollupOptions = (props: { [cellValueType, formatting, isMultipleCellValue, isLookup, options.timeZone, onChange] ); + useEffect(() => { + if (!availableExpressions || availableExpressions.length === 0) { + return; + } + if (expression && availableExpressions.includes(expression)) { + return; + } + const fallbackExpression = availableExpressions[0]; + if (!fallbackExpression) { + return; + } + onExpressionChange(fallbackExpression); + }, [availableExpressions, expression, onExpressionChange]); + const onFormattingChange = useCallback( (newFormatting?: IUnionFormatting) => { const { cellValueType } = typedValue; diff --git a/packages/common-i18n/src/locales/de/table.json b/packages/common-i18n/src/locales/de/table.json index cfc1336eb8..ab7831edb5 100644 --- a/packages/common-i18n/src/locales/de/table.json +++ b/packages/common-i18n/src/locales/de/table.json @@ -272,6 +272,7 @@ "clearSort": "Clear sort", "limitLabel": "Maximum records to include", "limitPlaceholder": "Leave blank to include all matches", + "limitHint": "We only keep up to {{limit}} matching records.", "sortMissingWarningTitle": "Sorting field unavailable", "sortMissingWarningDescription": "The field that powered this sort was deleted. Results ignore the sort and only enforce the limit." } diff --git a/packages/common-i18n/src/locales/en/table.json b/packages/common-i18n/src/locales/en/table.json index f15fde89fe..6b298624fa 100644 --- a/packages/common-i18n/src/locales/en/table.json +++ b/packages/common-i18n/src/locales/en/table.json @@ -265,6 +265,7 @@ "clearSort": "Clear sort", "limitLabel": "Maximum records to include", "limitPlaceholder": "Leave blank to include all matches", + "limitHint": "We only keep up to {{limit}} matching records.", "sortMissingWarningTitle": "Sorting field unavailable", "sortMissingWarningDescription": "The field that powered this sort was deleted. Results ignore the sort and only enforce the limit." }, diff --git a/packages/common-i18n/src/locales/es/table.json b/packages/common-i18n/src/locales/es/table.json index de9d1691e1..9b6ed38e05 100644 --- a/packages/common-i18n/src/locales/es/table.json +++ b/packages/common-i18n/src/locales/es/table.json @@ -269,6 +269,7 @@ "clearSort": "Clear sort", "limitLabel": "Maximum records to include", "limitPlaceholder": "Leave blank to include all matches", + "limitHint": "We only keep up to {{limit}} matching records.", "sortMissingWarningTitle": "Sorting field unavailable", "sortMissingWarningDescription": "The field that powered this sort was deleted. Results ignore the sort and only enforce the limit." } diff --git a/packages/common-i18n/src/locales/fr/table.json b/packages/common-i18n/src/locales/fr/table.json index af30e9a20a..2737822a18 100644 --- a/packages/common-i18n/src/locales/fr/table.json +++ b/packages/common-i18n/src/locales/fr/table.json @@ -263,6 +263,7 @@ "clearSort": "Clear sort", "limitLabel": "Maximum records to include", "limitPlaceholder": "Leave blank to include all matches", + "limitHint": "We only keep up to {{limit}} matching records.", "sortMissingWarningTitle": "Sorting field unavailable", "sortMissingWarningDescription": "The field that powered this sort was deleted. Results ignore the sort and only enforce the limit." } diff --git a/packages/common-i18n/src/locales/it/table.json b/packages/common-i18n/src/locales/it/table.json index 44f4f4201e..1ce5b598ba 100644 --- a/packages/common-i18n/src/locales/it/table.json +++ b/packages/common-i18n/src/locales/it/table.json @@ -272,6 +272,7 @@ "clearSort": "Clear sort", "limitLabel": "Maximum records to include", "limitPlaceholder": "Leave blank to include all matches", + "limitHint": "We only keep up to {{limit}} matching records.", "sortMissingWarningTitle": "Sorting field unavailable", "sortMissingWarningDescription": "The field that powered this sort was deleted. Results ignore the sort and only enforce the limit." } diff --git a/packages/common-i18n/src/locales/ja/table.json b/packages/common-i18n/src/locales/ja/table.json index 06f77362e8..12aa506225 100644 --- a/packages/common-i18n/src/locales/ja/table.json +++ b/packages/common-i18n/src/locales/ja/table.json @@ -263,6 +263,7 @@ "clearSort": "Clear sort", "limitLabel": "Maximum records to include", "limitPlaceholder": "Leave blank to include all matches", + "limitHint": "We only keep up to {{limit}} matching records.", "sortMissingWarningTitle": "Sorting field unavailable", "sortMissingWarningDescription": "The field that powered this sort was deleted. Results ignore the sort and only enforce the limit." } diff --git a/packages/common-i18n/src/locales/ru/table.json b/packages/common-i18n/src/locales/ru/table.json index 6deb82a5ed..c97d543db4 100644 --- a/packages/common-i18n/src/locales/ru/table.json +++ b/packages/common-i18n/src/locales/ru/table.json @@ -263,6 +263,7 @@ "clearSort": "Clear sort", "limitLabel": "Maximum records to include", "limitPlaceholder": "Leave blank to include all matches", + "limitHint": "We only keep up to {{limit}} matching records.", "sortMissingWarningTitle": "Sorting field unavailable", "sortMissingWarningDescription": "The field that powered this sort was deleted. Results ignore the sort and only enforce the limit." } diff --git a/packages/common-i18n/src/locales/tr/table.json b/packages/common-i18n/src/locales/tr/table.json index 64ba9627f1..5b4be53590 100644 --- a/packages/common-i18n/src/locales/tr/table.json +++ b/packages/common-i18n/src/locales/tr/table.json @@ -260,6 +260,7 @@ "clearSort": "Clear sort", "limitLabel": "Maximum records to include", "limitPlaceholder": "Leave blank to include all matches", + "limitHint": "We only keep up to {{limit}} matching records.", "sortMissingWarningTitle": "Sorting field unavailable", "sortMissingWarningDescription": "The field that powered this sort was deleted. Results ignore the sort and only enforce the limit." } diff --git a/packages/common-i18n/src/locales/uk/table.json b/packages/common-i18n/src/locales/uk/table.json index 4b3c387f51..6cd67032ed 100644 --- a/packages/common-i18n/src/locales/uk/table.json +++ b/packages/common-i18n/src/locales/uk/table.json @@ -272,6 +272,7 @@ "clearSort": "Clear sort", "limitLabel": "Maximum records to include", "limitPlaceholder": "Leave blank to include all matches", + "limitHint": "We only keep up to {{limit}} matching records.", "sortMissingWarningTitle": "Sorting field unavailable", "sortMissingWarningDescription": "The field that powered this sort was deleted. Results ignore the sort and only enforce the limit." } diff --git a/packages/common-i18n/src/locales/zh/table.json b/packages/common-i18n/src/locales/zh/table.json index f320c6c939..a503800c85 100644 --- a/packages/common-i18n/src/locales/zh/table.json +++ b/packages/common-i18n/src/locales/zh/table.json @@ -266,6 +266,7 @@ "clearSort": "清除排序", "limitLabel": "显示的最大记录数", "limitPlaceholder": "留空表示显示全部匹配项", + "limitHint": "最多仅会保存 {{limit}} 条匹配记录。", "sortMissingWarningTitle": "排序字段已被删除", "sortMissingWarningDescription": "用于排序的字段已删除，结果会忽略排序，仅保留数量限制。" }, diff --git a/packages/core/src/models/field/conditional.constants.ts b/packages/core/src/models/field/conditional.constants.ts new file mode 100644 index 0000000000..a829845d37 --- /dev/null +++ b/packages/core/src/models/field/conditional.constants.ts @@ -0,0 +1,29 @@ +const parsePositiveInt = (raw: string | undefined, fallback: number): number => { + if (!raw) return fallback; + const parsed = Number(raw); + if (!Number.isFinite(parsed) || parsed <= 0) { + return fallback; + } + return Math.floor(parsed); +}; + +const resolvedMax = parsePositiveInt(process.env.CONDITIONAL_QUERY_MAX_LIMIT, 5000); +const resolvedDefault = parsePositiveInt(process.env.CONDITIONAL_QUERY_DEFAULT_LIMIT, resolvedMax); + +export const CONDITIONAL_QUERY_MAX_LIMIT = resolvedMax; +export const CONDITIONAL_QUERY_DEFAULT_LIMIT = Math.min(resolvedDefault, resolvedMax); + +export const clampConditionalLimit = (limit?: number | null): number | undefined => { + if (typeof limit !== 'number' || !Number.isFinite(limit)) { + return undefined; + } + const truncated = Math.trunc(limit); + if (truncated <= 0) { + return undefined; + } + return Math.min(truncated, CONDITIONAL_QUERY_MAX_LIMIT); +}; + +export const normalizeConditionalLimit = (limit?: number | null): number => { + return clampConditionalLimit(limit) ?? CONDITIONAL_QUERY_DEFAULT_LIMIT; +}; diff --git a/packages/core/src/models/field/derivate/conditional-rollup-option.schema.ts b/packages/core/src/models/field/derivate/conditional-rollup-option.schema.ts index 79cf173bf6..bbb96c1da4 100644 --- a/packages/core/src/models/field/derivate/conditional-rollup-option.schema.ts +++ b/packages/core/src/models/field/derivate/conditional-rollup-option.schema.ts @@ -1,6 +1,7 @@ import { z } from '../../../zod'; import { filterSchema } from '../../view/filter'; import { SortFunc } from '../../view/sort'; +import { CONDITIONAL_QUERY_MAX_LIMIT } from '../conditional.constants'; import { rollupFieldOptionsSchema } from './rollup-option.schema'; export const conditionalRollupFieldOptionsSchema = rollupFieldOptionsSchema.extend({ @@ -14,7 +15,7 @@ export const conditionalRollupFieldOptionsSchema = rollupFieldOptionsSchema.exte order: z.nativeEnum(SortFunc), }) .optional(), - limit: z.number().int().positive().optional(), + limit: z.number().int().positive().max(CONDITIONAL_QUERY_MAX_LIMIT).optional(), }); export type IConditionalRollupFieldOptions = z.infer; diff --git a/packages/core/src/models/field/index.ts b/packages/core/src/models/field/index.ts index 0f892afdd0..5c223bd4bf 100644 --- a/packages/core/src/models/field/index.ts +++ b/packages/core/src/models/field/index.ts @@ -1,5 +1,6 @@ export * from './derivate'; export * from './constant'; +export * from './conditional.constants'; export * from './field'; export * from './field.type'; export * from './field-visitor.interface'; diff --git a/packages/core/src/models/field/lookup-options-base.schema.ts b/packages/core/src/models/field/lookup-options-base.schema.ts index fee18827de..b2f4489da4 100644 --- a/packages/core/src/models/field/lookup-options-base.schema.ts +++ b/packages/core/src/models/field/lookup-options-base.schema.ts @@ -1,6 +1,7 @@ import { z } from '../../zod'; import { filterSchema } from '../view/filter'; import { SortFunc } from '../view/sort'; +import { CONDITIONAL_QUERY_MAX_LIMIT } from './conditional.constants'; import { Relationship } from './constant'; const lookupLinkOptionsVoSchema = z.object({ @@ -67,7 +68,7 @@ const lookupConditionalOptionsVoSchema = z.object({ .openapi({ description: 'Optional sort configuration applied before aggregating lookup values.', }), - limit: z.number().int().positive().optional().openapi({ + limit: z.number().int().positive().max(CONDITIONAL_QUERY_MAX_LIMIT).optional().openapi({ description: 'Maximum number of matching records to include in the lookup result.', }), });