diff --git a/internal/lookout/ui/src/hooks/useJobsTableData.ts b/internal/lookout/ui/src/hooks/useJobsTableData.ts index 9bcca0737c7..2f304266df9 100644 --- a/internal/lookout/ui/src/hooks/useJobsTableData.ts +++ b/internal/lookout/ui/src/hooks/useJobsTableData.ts @@ -80,10 +80,16 @@ const columnToGroupSortFieldMap = new Map([ [StandardColumnId.TimeSubmittedAgo, "submitted"], [StandardColumnId.LastTransitionTimeUtc, "lastTransitionTime"], [StandardColumnId.TimeInState, "lastTransitionTime"], + [StandardColumnId.JobSet, "jobSet"], ]) // Return ordering to request to API based on column -function getOrder(lookoutOrder: LookoutColumnOrder, isJobFetch: boolean): JobOrder { +function getOrder( + lookoutOrder: LookoutColumnOrder, + lookoutFilters: JobFilter[], + groupedColumns: ColumnId[], + isJobFetch: boolean, +): JobOrder { const defaultJobOrder: JobOrder = { field: "jobId", direction: "DESC", @@ -101,9 +107,13 @@ function getOrder(lookoutOrder: LookoutColumnOrder, isJobFetch: boolean): JobOrd } field = columnToJobSortFieldMap.get(lookoutOrder.id as ColumnId) as string } else { - if (!columnToGroupSortFieldMap.has(lookoutOrder.id as ColumnId)) { + if ( + !canOrderByField(lookoutFilters, groupedColumns, lookoutOrder.id) || + !columnToGroupSortFieldMap.has(lookoutOrder.id as ColumnId) + ) { return defaultGroupOrder } + field = columnToGroupSortFieldMap.get(lookoutOrder.id as ColumnId) as string } @@ -113,6 +123,30 @@ function getOrder(lookoutOrder: LookoutColumnOrder, isJobFetch: boolean): JobOrd } } +function canOrderByField(lookoutFilters: JobFilter[], groupedColumns: string[], id: string): boolean { + // A function that determines whether we can see the field in the UI, therefore allowing us to sort by it + // Extract 'field' values from lookoutFilters, excluding 'jobSet' + const filterFields = lookoutFilters.map((filter) => filter.field).filter((field) => field !== id) + + // Exclude 'jobSet' from groupedColumns for direct comparison + const filteredGroupedColumns = groupedColumns.filter((col) => col !== id) + + // Check if the lengths are different + if (filterFields.length !== filteredGroupedColumns.length) { + return false + } + + // Check each element for an exact match + for (let i = 0; i < filterFields.length; i++) { + if (filterFields[i] !== filteredGroupedColumns[i]) { + return false + } + } + + // If all checks pass, return true + return true +} + export const useFetchJobsTableData = ({ groupedColumns, visibleColumns, @@ -149,9 +183,10 @@ export const useFetchJobsTableData = ({ const expandedLevel = parentRowInfo ? parentRowInfo.rowIdPathFromRoot.length : 0 const isJobFetch = expandedLevel === groupingLevel - const order = getOrder(lookoutOrder, isJobFetch) + const filterValues = getFiltersForRows(lookoutFilters, columnMatches, parentRowInfo?.rowIdPartsPath ?? []) + const order = getOrder(lookoutOrder, filterValues, groupedColumns, isJobFetch) const rowRequest: FetchRowRequest = { - filters: getFiltersForRows(lookoutFilters, columnMatches, parentRowInfo?.rowIdPartsPath ?? []), + filters: filterValues, activeJobSets: activeJobSets, skip: nextRequest.skip ?? paginationState.pageIndex * paginationState.pageSize, take: nextRequest.take ?? paginationState.pageSize, diff --git a/internal/lookoutv2/repository/querybuilder.go b/internal/lookoutv2/repository/querybuilder.go index 3d7f04c220e..c66ac410144 100644 --- a/internal/lookoutv2/repository/querybuilder.go +++ b/internal/lookoutv2/repository/querybuilder.go @@ -222,7 +222,6 @@ func (qb *QueryBuilder) GroupBy( } } - groupBySql := fmt.Sprintf("GROUP BY %s.%s", groupCol.abbrev, groupCol.name) whereSql, err := qb.queryFiltersToSql(queryFilters, true) if err != nil { return nil, err @@ -239,6 +238,10 @@ func (qb *QueryBuilder) GroupBy( if err != nil { return nil, err } + groupBySql, err := qb.createGroupBySQL(order, groupCol, aggregates) + if err != nil { + return nil, err + } template := fmt.Sprintf(` SELECT %[1]s.%[2]s, %[3]s %[4]s @@ -254,6 +257,31 @@ func (qb *QueryBuilder) GroupBy( }, nil } +func (qb *QueryBuilder) createGroupBySQL(order *model.Order, groupCol *queryColumn, aggregates []string) (string, error) { + expr := fmt.Sprintf("GROUP BY %s.%s", groupCol.abbrev, groupCol.name) + isInAggregators := len(aggregates) > 0 && func(sl []string, t string) bool { + for _, s := range sl { + if s == t { + return true + } + } + return false + }(aggregates, order.Field) + if orderIsNull(order) || order.Field == countCol || isInAggregators { + return expr, nil + } + col, err := qb.lookoutTables.ColumnFromField(order.Field) + if err != nil { + return expr, err + } + + if groupCol.name == col { + return expr, nil + } + // If order is not already grouped by or aggregated by, include it in GROUP BY statement + return expr + fmt.Sprintf(", %s.%s", groupCol.abbrev, col), nil +} + func (qb *QueryBuilder) fieldsToCols(fields []string) ([]string, error) { var cols []string for _, field := range fields { @@ -940,8 +968,10 @@ func (qb *QueryBuilder) validateGroupOrder(order *model.Order) error { if err != nil { return errors.Errorf("unsupported field for order: %s", order.Field) } + _, err = qb.lookoutTables.GroupAggregateForCol(col) - if err != nil { + // If it is not an aggregate and not groupable, it can't be ordered by + if err != nil && !qb.lookoutTables.IsGroupable(col) { return errors.Errorf("unsupported field for order: %s, cannot sort by column %s", order.Field, col) } return nil