From 15b053de461b2e1e074a39fd2ebcbd0b26b1fe96 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Fri, 30 Jan 2026 17:40:36 +0100 Subject: [PATCH] [GroupBy] Fix order by date granularity (#17573) https://discord.com/channels/1130383047699738754/1466472357496623165/1466472357496623165 before image after image --- .../graphql-query-order-group-by.parser.ts | 6 +- .../get-group-by-order-expression.util.ts | 82 +++++++++ .../utils/build-group-by-field-object.util.ts | 6 +- .../utils/compare-dimension-values.util.ts | 9 +- ...order-by-with-group-by.integration-spec.ts | 162 ++++++++++++++++-- 5 files changed, 242 insertions(+), 23 deletions(-) create mode 100644 packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/get-group-by-order-expression.util.ts diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/graphql-query-order-group-by.parser.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/graphql-query-order-group-by.parser.ts index 4841573eef..06738d28ce 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/graphql-query-order-group-by.parser.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/graphql-query-order-group-by.parser.ts @@ -21,7 +21,7 @@ import { type GroupByField, type GroupByRegularField, } from 'src/engine/api/graphql/graphql-query-runner/group-by/resolvers/types/group-by-field.types'; -import { getGroupByExpression } from 'src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/get-group-by-expression.util'; +import { getGroupByOrderExpression } from 'src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/get-group-by-order-expression.util'; import { ProcessAggregateHelper } from 'src/engine/api/graphql/graphql-query-runner/helpers/process-aggregate.helper'; import { type AggregationField, @@ -493,7 +493,7 @@ export class GraphqlQueryOrderGroupByParser { )[0] }"`; - const expression = getGroupByExpression({ + const expression = getGroupByOrderExpression({ groupByField: associatedGroupByField, columnNameWithQuotes, }); @@ -620,7 +620,7 @@ export class GraphqlQueryOrderGroupByParser { const columnNameWithQuotes = `"${joinAlias}"."${nestedColumnName}"`; - const expression = getGroupByExpression({ + const expression = getGroupByOrderExpression({ groupByField: associatedGroupByField, columnNameWithQuotes, }); diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/get-group-by-order-expression.util.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/get-group-by-order-expression.util.ts new file mode 100644 index 0000000000..fc325e1faa --- /dev/null +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/get-group-by-order-expression.util.ts @@ -0,0 +1,82 @@ +import { + FirstDayOfTheWeek, + ObjectRecordGroupByDateGranularity, +} from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; + +import { type GroupByField } from 'src/engine/api/graphql/graphql-query-runner/group-by/resolvers/types/group-by-field.types'; +import { getGroupByExpression } from 'src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/get-group-by-expression.util'; +import { isGroupByDateField } from 'src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/is-group-by-date-field.util'; +import { isGroupByRelationField } from 'src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/is-group-by-relation-field.util'; + +const DAYS_OF_WEEK = [ + 'Monday', + 'Tuesday', + 'Wednesday', + 'Thursday', + 'Friday', + 'Saturday', + 'Sunday', +] as const; + +const getDayOfWeekOrderExpression = ( + groupByExpression: string, + weekStartDay?: FirstDayOfTheWeek, +): string => { + const startDay = weekStartDay ?? FirstDayOfTheWeek.MONDAY; + + const startIndex = + startDay === FirstDayOfTheWeek.SUNDAY + ? 6 + : startDay === FirstDayOfTheWeek.SATURDAY + ? 5 + : 0; + + const orderedDays = [ + ...DAYS_OF_WEEK.slice(startIndex), + ...DAYS_OF_WEEK.slice(0, startIndex), + ]; + + const caseConditions = orderedDays + .map((day, index) => `WHEN '${day}' THEN ${index + 1}`) + .join(' '); + + return `CASE ${groupByExpression} ${caseConditions} END`; +}; + +export const getGroupByOrderExpression = ({ + groupByField, + columnNameWithQuotes, +}: { + groupByField: GroupByField; + columnNameWithQuotes: string; +}): string => { + if ( + !(isGroupByDateField(groupByField) || isGroupByRelationField(groupByField)) + ) { + return getGroupByExpression({ groupByField, columnNameWithQuotes }); + } + + const dateGranularity = groupByField.dateGranularity; + + if (!isDefined(dateGranularity)) { + return getGroupByExpression({ groupByField, columnNameWithQuotes }); + } + + const groupByExpression = getGroupByExpression({ + groupByField, + columnNameWithQuotes, + }); + + switch (dateGranularity) { + case ObjectRecordGroupByDateGranularity.DAY_OF_THE_WEEK: + return getDayOfWeekOrderExpression( + groupByExpression, + groupByField.weekStartDay, + ); + case ObjectRecordGroupByDateGranularity.MONTH_OF_THE_YEAR: + return `CASE ${groupByExpression} WHEN 'January' THEN 1 WHEN 'February' THEN 2 WHEN 'March' THEN 3 WHEN 'April' THEN 4 WHEN 'May' THEN 5 WHEN 'June' THEN 6 WHEN 'July' THEN 7 WHEN 'August' THEN 8 WHEN 'September' THEN 9 WHEN 'October' THEN 10 WHEN 'November' THEN 11 WHEN 'December' THEN 12 END`; + default: + return groupByExpression; + } +}; diff --git a/packages/twenty-server/src/modules/dashboard/chart-data/utils/build-group-by-field-object.util.ts b/packages/twenty-server/src/modules/dashboard/chart-data/utils/build-group-by-field-object.util.ts index 8a6f5ddfa4..5d1d15a327 100644 --- a/packages/twenty-server/src/modules/dashboard/chart-data/utils/build-group-by-field-object.util.ts +++ b/packages/twenty-server/src/modules/dashboard/chart-data/utils/build-group-by-field-object.util.ts @@ -51,8 +51,12 @@ const buildDateGroupByObject = ({ result.timeZone = timeZone; } + const shouldIncludeWeekStartDay = + usedDateGranularity === ObjectRecordGroupByDateGranularity.WEEK || + usedDateGranularity === ObjectRecordGroupByDateGranularity.DAY_OF_THE_WEEK; + if ( - usedDateGranularity === ObjectRecordGroupByDateGranularity.WEEK && + shouldIncludeWeekStartDay && isDefined(firstDayOfTheWeek) && firstDayOfTheWeek !== CalendarStartDay.SYSTEM ) { diff --git a/packages/twenty-server/src/modules/dashboard/chart-data/utils/compare-dimension-values.util.ts b/packages/twenty-server/src/modules/dashboard/chart-data/utils/compare-dimension-values.util.ts index cee6530025..7245ba4dd1 100644 --- a/packages/twenty-server/src/modules/dashboard/chart-data/utils/compare-dimension-values.util.ts +++ b/packages/twenty-server/src/modules/dashboard/chart-data/utils/compare-dimension-values.util.ts @@ -49,10 +49,11 @@ export const compareDimensionValues = ({ direction === 'ASC' ? comparison : -comparison; if (isDefined(fieldType)) { - if ( - isFieldMetadataDateKind(fieldType) && - !isCyclicalDateGranularity(dateGranularity) - ) { + if (isFieldMetadataDateKind(fieldType)) { + if (isCyclicalDateGranularity(dateGranularity)) { + return 0; + } + const dateA = parseDate(rawValueA); const dateB = parseDate(rawValueB); diff --git a/packages/twenty-server/test/integration/graphql/suites/order-by-with-group-by.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/order-by-with-group-by.integration-spec.ts index 28dde66dd3..ea54549425 100644 --- a/packages/twenty-server/test/integration/graphql/suites/order-by-with-group-by.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/order-by-with-group-by.integration-spec.ts @@ -204,7 +204,7 @@ describe('group-by resolvers - order by', () => { totalCount: g.totalCount, })); - // Order by dayOfWeek then avgEmployees then city + // Order by dayOfWeek (chronological) then avgEmployees then city expect(groupInfos).toEqual([ { city: 'Dallas', @@ -234,13 +234,6 @@ describe('group-by resolvers - order by', () => { totalCount: 1, annualRecurringRevenue: '100', }, - { - city: 'Paris', - dayOfWeek: 'Thursday', - avgEmployees: 10, - totalCount: 1, - annualRecurringRevenue: '100', - }, { city: 'Barcelona', dayOfWeek: 'Wednesday', @@ -248,6 +241,13 @@ describe('group-by resolvers - order by', () => { totalCount: 2, annualRecurringRevenue: '100', }, + { + city: 'Paris', + dayOfWeek: 'Thursday', + avgEmployees: 10, + totalCount: 1, + annualRecurringRevenue: '100', + }, ]); }); it('should order results in the right order - createdAt, addressCity, avgEmployees', async () => { @@ -284,6 +284,7 @@ describe('group-by resolvers - order by', () => { totalCount: g.totalCount, })); + // Order by dayOfWeek (chronological) then addressCity then avgEmployees expect(groupInfos).toEqual([ { city: 'Anvers', @@ -313,13 +314,6 @@ describe('group-by resolvers - order by', () => { totalCount: 1, annualRecurringRevenue: '100', }, - { - city: 'Paris', - dayOfWeek: 'Thursday', - avgEmployees: 10, - totalCount: 1, - annualRecurringRevenue: '100', - }, { city: 'Barcelona', dayOfWeek: 'Wednesday', @@ -327,6 +321,13 @@ describe('group-by resolvers - order by', () => { totalCount: 2, annualRecurringRevenue: '100', }, + { + city: 'Paris', + dayOfWeek: 'Thursday', + avgEmployees: 10, + totalCount: 1, + annualRecurringRevenue: '100', + }, ]); }); it('should order results in the right order - addressCity, createdAt, avgEmployees', async () => { @@ -489,6 +490,137 @@ describe('group-by resolvers - order by', () => { }); }); + describe('chronological ordering for date granularities', () => { + it('should order DAY_OF_THE_WEEK chronologically (Monday=1 to Sunday=7), not alphabetically', async () => { + const response = await makeGraphqlAPIRequest( + groupByOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + groupBy: [{ createdAt: { granularity: 'DAY_OF_THE_WEEK' } }], + orderBy: [ + { + createdAt: { + granularity: 'DAY_OF_THE_WEEK', + orderBy: 'AscNullsFirst', + }, + }, + ], + filter: filter2025, + gqlFields: ` + totalCount + `, + }), + ); + + const groups = response.body.data.companiesGroupBy; + + expect(groups).toBeDefined(); + expect(Array.isArray(groups)).toBe(true); + + const dayOrder = groups.map((g: any) => g.groupByDimensionValues?.[0]); + + // Monday (1), Wednesday (3), Thursday (4) - chronological order + expect(dayOrder).toEqual(['Monday', 'Wednesday', 'Thursday']); + }); + + it('should order DAY_OF_THE_WEEK in descending chronological order', async () => { + const response = await makeGraphqlAPIRequest( + groupByOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + groupBy: [{ createdAt: { granularity: 'DAY_OF_THE_WEEK' } }], + orderBy: [ + { + createdAt: { + granularity: 'DAY_OF_THE_WEEK', + orderBy: 'DescNullsLast', + }, + }, + ], + filter: filter2025, + gqlFields: ` + totalCount + `, + }), + ); + + const groups = response.body.data.companiesGroupBy; + + expect(groups).toBeDefined(); + + const dayOrder = groups.map((g: any) => g.groupByDimensionValues?.[0]); + + // Thursday (4), Wednesday (3), Monday (1) - reverse chronological order + expect(dayOrder).toEqual(['Thursday', 'Wednesday', 'Monday']); + }); + + it('should order MONTH_OF_THE_YEAR chronologically (January=1 to December=12), not alphabetically', async () => { + // Test data has January (companies 4,5,6) and March (companies 1,2,3,7) + // Chronological order: January (1), March (3) + // Alphabetical would be: January, March (same in this case, but tests the mechanism) + const response = await makeGraphqlAPIRequest( + groupByOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + groupBy: [{ createdAt: { granularity: 'MONTH_OF_THE_YEAR' } }], + orderBy: [ + { + createdAt: { + granularity: 'MONTH_OF_THE_YEAR', + orderBy: 'AscNullsFirst', + }, + }, + ], + filter: filter2025, + gqlFields: ` + totalCount + `, + }), + ); + + const groups = response.body.data.companiesGroupBy; + + expect(groups).toBeDefined(); + expect(Array.isArray(groups)).toBe(true); + + const monthOrder = groups.map((g: any) => g.groupByDimensionValues?.[0]); + + // January (1), March (3) - chronological order + expect(monthOrder).toEqual(['January', 'March']); + }); + + it('should order MONTH_OF_THE_YEAR in descending chronological order', async () => { + const response = await makeGraphqlAPIRequest( + groupByOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + groupBy: [{ createdAt: { granularity: 'MONTH_OF_THE_YEAR' } }], + orderBy: [ + { + createdAt: { + granularity: 'MONTH_OF_THE_YEAR', + orderBy: 'DescNullsLast', + }, + }, + ], + filter: filter2025, + gqlFields: ` + totalCount + `, + }), + ); + + const groups = response.body.data.companiesGroupBy; + + expect(groups).toBeDefined(); + + const monthOrder = groups.map((g: any) => g.groupByDimensionValues?.[0]); + + // March (3), January (1) - reverse chronological order + expect(monthOrder).toEqual(['March', 'January']); + }); + }); + describe('invalid cases', () => { it('should fail if attempt to order by a field that is not part of the groupBy', async () => { const response = await makeGraphqlAPIRequest(