From 3aa5e73a7093d4053055007954952dbc8a72a3df Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Thu, 27 Aug 2026 22:36:32 -0700 Subject: [PATCH 01/10] QueryModelLoader: use selectRows() --- packages/components/src/internal/query/api.ts | 37 ++++++++++++------- .../src/internal/query/selectRows.ts | 3 ++ .../src/public/QueryModel/QueryModelLoader.ts | 33 +++++++++++++---- 3 files changed, 52 insertions(+), 21 deletions(-) diff --git a/packages/components/src/internal/query/api.ts b/packages/components/src/internal/query/api.ts index df5f5a3a5f..dd8af89288 100644 --- a/packages/components/src/internal/query/api.ts +++ b/packages/components/src/internal/query/api.ts @@ -512,20 +512,16 @@ export async function selectRowsDeprecated(options_: SelectRowsDeprecatedOptions }; } -export function handleSelectRowsResponse(response: Query.Response, queryInfo: QueryInfo): any { - const resolved = new URLResolver().resolveSelectRows(response, queryInfo); - - let count = 0, - hasRows = false, - models = {}, - orderedModels = {}, - qsKey = 'queries', - rowCount = response.rowCount || 0; - - let metadataAltKey: string, metadataKey: string; - if (resolved.metaData) { +export function resolveRowKey( + metaData: Query.ResponseMetadata, + queryInfo: QueryInfo +): { metadataAltKey: string; metadataKey: string } { + let metadataAltKey: string; + let metadataKey: string; + + if (metaData) { // If metaData is present, then use its "id" value regardless of presence of a queryInfo - metadataKey = resolved.metaData.id; + metadataKey = metaData.id; } else if (queryInfo) { // Match ApiQueryResponse logic for determining "metaData.id" if (queryInfo.pkCols.length === 1) { @@ -536,6 +532,21 @@ export function handleSelectRowsResponse(response: Query.Response, queryInfo: Qu } } } + + return { metadataAltKey, metadataKey }; +} + +export function handleSelectRowsResponse(response: Query.Response, queryInfo: QueryInfo): any { + const resolved = new URLResolver().resolveSelectRows(response, queryInfo); + + let count = 0, + hasRows = false, + models = {}, + orderedModels = {}, + qsKey = 'queries', + rowCount = response.rowCount || 0; + + const { metadataAltKey, metadataKey } = resolveRowKey(resolved.metaData, queryInfo); const modelKey = resolveKeyFromJson(resolved); // ensure id -- unfortunately, with normalizr 3.x there doesn't seem to be a way to generate the id diff --git a/packages/components/src/internal/query/selectRows.ts b/packages/components/src/internal/query/selectRows.ts index 1ca85e0894..fab2714b6e 100644 --- a/packages/components/src/internal/query/selectRows.ts +++ b/packages/components/src/internal/query/selectRows.ts @@ -30,6 +30,8 @@ export type Row = Record; export interface SelectRowsResponse { messages: Record[]; + /** Only available when "includeMetadata" is set to true. */ + metaData: Query.ResponseMetadata | undefined; queryInfo: QueryInfo; rowCount: number; rows: Row[]; @@ -86,6 +88,7 @@ export async function selectRows(options: SelectRowsOptions): Promise): ExtendedMap { if (columns) { @@ -124,20 +125,36 @@ export const DefaultQueryModelLoader: QueryModelLoader = { return queryInfo.mutate({ columns: bindColumnRenderers(queryInfo.columns) }); }, async loadRows(model, requestHandler) { - const result = await selectRowsDeprecated({ + const result = await selectRows({ ...model.loadRowsConfig, - schemaName: model.schemaName, - queryName: model.queryName, + includeMetadata: true, includeTotalCount: false, // if requesting to includeTotalCount, it will be loaded separately via loadTotalCount includeStyle: true, // Issue 49100 requestHandler, }); - const { key, models, orderedModels, rowCount, messages } = result; + + const { messages, metaData, queryInfo, rowCount } = result; + const { metadataAltKey, metadataKey } = resolveRowKey(metaData, queryInfo); + const orderedRows: string[] = []; + const rows: Record = {}; + + result.rows.forEach(row => { + if (metadataKey || metadataAltKey) { + const val = row[metadataKey] ?? row[metadataAltKey]; + if (val !== undefined) { + const value = val.value.toString(); + orderedRows.push(value); + rows[value] = row; + } else { + console.error('Missing entry', metadataKey, row, result.schemaQuery.toString(true)); + } + } + }); return { - messages: messages.toJS(), - rows: models[key], - orderedRows: orderedModels[key].toArray(), + messages: messages as unknown as GridMessage[], + orderedRows, + rows, rowCount, }; }, From d62d7e01b77a1576c63462ee36a7cdfcc08b563c Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Fri, 28 Aug 2026 11:46:31 -0700 Subject: [PATCH 02/10] Account for no rowKey --- .../components/src/public/QueryModel/QueryModelLoader.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/components/src/public/QueryModel/QueryModelLoader.ts b/packages/components/src/public/QueryModel/QueryModelLoader.ts index 201ffeabed..52e80e93b5 100644 --- a/packages/components/src/public/QueryModel/QueryModelLoader.ts +++ b/packages/components/src/public/QueryModel/QueryModelLoader.ts @@ -135,11 +135,12 @@ export const DefaultQueryModelLoader: QueryModelLoader = { const { messages, metaData, queryInfo, rowCount } = result; const { metadataAltKey, metadataKey } = resolveRowKey(metaData, queryInfo); + const hasKey = metadataKey || metadataAltKey; const orderedRows: string[] = []; const rows: Record = {}; - result.rows.forEach(row => { - if (metadataKey || metadataAltKey) { + result.rows.forEach((row, index) => { + if (hasKey) { const val = row[metadataKey] ?? row[metadataAltKey]; if (val !== undefined) { const value = val.value.toString(); @@ -148,6 +149,9 @@ export const DefaultQueryModelLoader: QueryModelLoader = { } else { console.error('Missing entry', metadataKey, row, result.schemaQuery.toString(true)); } + } else { + orderedRows.push(index.toString()); + rows[index] = row; } }); From f81e1dab4445082db31040e70e7b6cc2509bcfc5 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Fri, 28 Aug 2026 11:49:53 -0700 Subject: [PATCH 03/10] Revise --- .../components/src/public/QueryModel/QueryModelLoader.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/components/src/public/QueryModel/QueryModelLoader.ts b/packages/components/src/public/QueryModel/QueryModelLoader.ts index 52e80e93b5..6a10bd4ecd 100644 --- a/packages/components/src/public/QueryModel/QueryModelLoader.ts +++ b/packages/components/src/public/QueryModel/QueryModelLoader.ts @@ -135,19 +135,18 @@ export const DefaultQueryModelLoader: QueryModelLoader = { const { messages, metaData, queryInfo, rowCount } = result; const { metadataAltKey, metadataKey } = resolveRowKey(metaData, queryInfo); - const hasKey = metadataKey || metadataAltKey; const orderedRows: string[] = []; const rows: Record = {}; result.rows.forEach((row, index) => { - if (hasKey) { + if (metadataKey || metadataAltKey) { const val = row[metadataKey] ?? row[metadataAltKey]; if (val !== undefined) { const value = val.value.toString(); orderedRows.push(value); rows[value] = row; } else { - console.error('Missing entry', metadataKey, row, result.schemaQuery.toString(true)); + console.error('Missing entry', result.schemaQuery.toString(true), metadataKey, metadataAltKey, row); } } else { orderedRows.push(index.toString()); From b82f7b84395a778ce135fddae0f57af00adc8d83 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Fri, 28 Aug 2026 12:59:35 -0700 Subject: [PATCH 04/10] SelectRowsMessage --- packages/components/src/index.ts | 8 +++++++- .../components/src/internal/query/selectRows.ts | 8 +++++++- .../components/src/public/QueryModel/QueryModel.ts | 12 +++--------- .../src/public/QueryModel/QueryModelLoader.ts | 13 ++++--------- .../src/public/QueryModel/withQueryModels.tsx | 9 ++++----- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/packages/components/src/index.ts b/packages/components/src/index.ts index 7e65104f96..dc0c6c4bb8 100644 --- a/packages/components/src/index.ts +++ b/packages/components/src/index.ts @@ -1946,7 +1946,13 @@ export type { ExecuteSqlResponseWithoutSession, ExecuteSqlResponseWithSession, } from './internal/query/executeSql'; -export type { Row, RowValue, SelectRowsOptions, SelectRowsResponse } from './internal/query/selectRows'; +export type { + Row, + RowValue, + SelectRowsMessage, + SelectRowsOptions, + SelectRowsResponse, +} from './internal/query/selectRows'; export type { IAttachment } from './internal/renderers/AttachmentCard'; export type { RequestHandler, RequestOptions } from './internal/request'; export type { AppContextTestProviderProps } from './internal/test/testHelpers'; diff --git a/packages/components/src/internal/query/selectRows.ts b/packages/components/src/internal/query/selectRows.ts index fab2714b6e..1872df5a73 100644 --- a/packages/components/src/internal/query/selectRows.ts +++ b/packages/components/src/internal/query/selectRows.ts @@ -11,6 +11,12 @@ import { URLResolver } from '../url/URLResolver'; import { getContainerFilter, getQueryDetails, isSelectRowMetadataRequired } from './api'; import { RequestHandler } from '../request'; +export interface SelectRowsMessage { + area?: string; + content: string; + type?: string; +} + export interface SelectRowsOptions extends Omit< Query.SelectRowsOptions, @@ -29,7 +35,7 @@ export interface RowValue { export type Row = Record; export interface SelectRowsResponse { - messages: Record[]; + messages: SelectRowsMessage[]; /** Only available when "includeMetadata" is set to true. */ metaData: Query.ResponseMetadata | undefined; queryInfo: QueryInfo; diff --git a/packages/components/src/public/QueryModel/QueryModel.ts b/packages/components/src/public/QueryModel/QueryModel.ts index f331ed474e..a942435e5f 100644 --- a/packages/components/src/public/QueryModel/QueryModel.ts +++ b/packages/components/src/public/QueryModel/QueryModel.ts @@ -19,7 +19,7 @@ import { QueryColumn } from '../QueryColumn'; import { caseInsensitive } from '../../internal/util/utils'; import { naturalSortByProperty } from '../sort'; import { PaginationData } from '../../internal/components/pagination/Pagination'; -import { SelectRowsOptions } from '../../internal/query/selectRows'; +import { SelectRowsMessage, SelectRowsOptions } from '../../internal/query/selectRows'; export function flattenValuesFromRow( row: any, @@ -108,12 +108,6 @@ export function createQueryModelId(schemaQuery: SchemaQuery): string { const sortStringMapper = (s: QuerySort): string => s.toRequestString(); -export interface GridMessage { - area?: string; - content: string; - type?: string; -} - export enum SavedSettings { all = 'all', // Restores filters, maxRows, sorts, and view noFilters = 'noFilters', // Restores maxRows and sorts only @@ -387,9 +381,9 @@ export class QueryModel { readonly filterArray: Filter.IFilter[]; // QueryModel only fields /** - * Array of [[GridMessage]]. When used with a [[GridPanel]], these message will be shown above the table of data rows. + * Array of [[SelectRowsMessage]]. When used with a [[GridPanel]], these messages will be shown above the table of data rows. */ - readonly messages?: GridMessage[]; + readonly messages?: SelectRowsMessage[]; /** * Array of row key values in sort order from the loaded data rows object. */ diff --git a/packages/components/src/public/QueryModel/QueryModelLoader.ts b/packages/components/src/public/QueryModel/QueryModelLoader.ts index 6a10bd4ecd..3a28172b39 100644 --- a/packages/components/src/public/QueryModel/QueryModelLoader.ts +++ b/packages/components/src/public/QueryModel/QueryModelLoader.ts @@ -27,8 +27,8 @@ import { QueryColumn } from '../QueryColumn'; import { QueryInfo } from '../QueryInfo'; import { naturalSortByProperty } from '../sort'; -import { GridMessage, QueryModel } from './QueryModel'; -import { Row, selectRows } from '../../internal/query/selectRows'; +import { QueryModel } from './QueryModel'; +import { Row, selectRows, SelectRowsMessage } from '../../internal/query/selectRows'; export function bindColumnRenderers(columns: ExtendedMap): ExtendedMap { if (columns) { @@ -53,7 +53,7 @@ export function bindColumnRenderers(columns: ExtendedMap): } export interface RowsResponse { - messages: GridMessage[]; + messages: SelectRowsMessage[]; orderedRows: string[]; rowCount: number; // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -154,12 +154,7 @@ export const DefaultQueryModelLoader: QueryModelLoader = { } }); - return { - messages: messages as unknown as GridMessage[], - orderedRows, - rows, - rowCount, - }; + return { messages, orderedRows, rows, rowCount }; }, // The selection related methods may seem like overly simple passthroughs, but by putting them on QueryModelLoader, // instead of in withQueryModels, it allows us to easily mock them or provide alternate implementations. diff --git a/packages/components/src/public/QueryModel/withQueryModels.tsx b/packages/components/src/public/QueryModel/withQueryModels.tsx index d0cda64ceb..9fa60c3f9b 100644 --- a/packages/components/src/public/QueryModel/withQueryModels.tsx +++ b/packages/components/src/public/QueryModel/withQueryModels.tsx @@ -17,7 +17,7 @@ import { isLoading, LoadingState } from '../LoadingState'; import { naturalSort } from '../sort'; import { resolveErrorMessage } from '../../internal/util/messaging'; -import { selectRows } from '../../internal/query/selectRows'; +import { selectRows, SelectRowsMessage } from '../../internal/query/selectRows'; import { incrementClientSideMetricCount } from '../../internal/actions'; @@ -26,7 +26,6 @@ import { DefaultQueryModelLoader, QueryModelLoader } from './QueryModelLoader'; import { RequestHandler } from '../../internal/request'; import { getSettingsFromLocalStorage, - GridMessage, locationHasQueryParamSettings, QueryConfig, QueryModel, @@ -113,7 +112,7 @@ export interface UpdateChange extends BaseModelChange { export type ModelChange = AddChange | DeleteChange | UpdateChange; export interface Actions { - addMessage: (id: string, message: GridMessage, duration?: number) => void; + addMessage: (id: string, message: SelectRowsMessage, duration?: number) => void; addModel: (queryConfig: QueryConfig, load?: boolean, loadSelections?: boolean) => void; clearSelectedReports: (id: string) => void; clearSelections: (id: string) => void; @@ -1355,7 +1354,7 @@ export function withQueryModels( ); }; - addMessage = (id: string, message: GridMessage, duration?: number): void => { + addMessage = (id: string, message: SelectRowsMessage, duration?: number): void => { this.setState( produce((draft: WritableDraft) => { const model = draft.queryModels[id]; @@ -1373,7 +1372,7 @@ export function withQueryModels( ); }; - removeMessage = (id: string, message: GridMessage): void => { + removeMessage = (id: string, message: SelectRowsMessage): void => { this.setState( produce((draft: WritableDraft) => { const model = draft.queryModels[id]; From 92b9f197d01f51fc8aca460ba9c9a8c6fd314b5d Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Sat, 29 Aug 2026 13:24:01 -0700 Subject: [PATCH 05/10] Match normalizr behavior --- packages/components/src/internal/query/api.ts | 2 +- .../src/public/QueryModel/QueryModelLoader.ts | 28 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/components/src/internal/query/api.ts b/packages/components/src/internal/query/api.ts index dd8af89288..f2593c652c 100644 --- a/packages/components/src/internal/query/api.ts +++ b/packages/components/src/internal/query/api.ts @@ -513,7 +513,7 @@ export async function selectRowsDeprecated(options_: SelectRowsDeprecatedOptions } export function resolveRowKey( - metaData: Query.ResponseMetadata, + metaData: Query.ResponseMetadata | undefined, queryInfo: QueryInfo ): { metadataAltKey: string; metadataKey: string } { let metadataAltKey: string; diff --git a/packages/components/src/public/QueryModel/QueryModelLoader.ts b/packages/components/src/public/QueryModel/QueryModelLoader.ts index 3a28172b39..c1fe59bec3 100644 --- a/packages/components/src/public/QueryModel/QueryModelLoader.ts +++ b/packages/components/src/public/QueryModel/QueryModelLoader.ts @@ -127,7 +127,6 @@ export const DefaultQueryModelLoader: QueryModelLoader = { async loadRows(model, requestHandler) { const result = await selectRows({ ...model.loadRowsConfig, - includeMetadata: true, includeTotalCount: false, // if requesting to includeTotalCount, it will be loaded separately via loadTotalCount includeStyle: true, // Issue 49100 requestHandler, @@ -135,23 +134,24 @@ export const DefaultQueryModelLoader: QueryModelLoader = { const { messages, metaData, queryInfo, rowCount } = result; const { metadataAltKey, metadataKey } = resolveRowKey(metaData, queryInfo); + const hasKeyColumn = !!metadataKey || !!metadataAltKey; const orderedRows: string[] = []; const rows: Record = {}; + let fallbackKey = 0; - result.rows.forEach((row, index) => { - if (metadataKey || metadataAltKey) { - const val = row[metadataKey] ?? row[metadataAltKey]; - if (val !== undefined) { - const value = val.value.toString(); - orderedRows.push(value); - rows[value] = row; - } else { - console.error('Missing entry', result.schemaQuery.toString(true), metadataKey, metadataAltKey, row); - } - } else { - orderedRows.push(index.toString()); - rows[index] = row; + result.rows.forEach(row => { + const val = hasKeyColumn ? (row[metadataKey] ?? row[metadataAltKey]) : undefined; + + if (hasKeyColumn && val === undefined) { + console.error('Missing entry', result.schemaQuery.toString(true), metadataKey, metadataAltKey, row); } + + // Repeated null keys collapse into a single entry here, as they did under normalizr + const key = String(val === undefined ? fallbackKey++ : val.value); + rows[key] = row; + + // Null-keyed rows would all collide on one key, so leave them out of the sort order as normalizr did + if (val?.value !== null) orderedRows.push(key); }); return { messages, orderedRows, rows, rowCount }; From 419a7c80c7ed99b270d6c01ac17e42662be7b9a4 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Sat, 29 Aug 2026 13:24:07 -0700 Subject: [PATCH 06/10] QueryModelLoader.test.ts --- .../QueryModel/QueryModelLoader.test.ts | 202 ++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 packages/components/src/public/QueryModel/QueryModelLoader.test.ts diff --git a/packages/components/src/public/QueryModel/QueryModelLoader.test.ts b/packages/components/src/public/QueryModel/QueryModelLoader.test.ts new file mode 100644 index 0000000000..8996de368a --- /dev/null +++ b/packages/components/src/public/QueryModel/QueryModelLoader.test.ts @@ -0,0 +1,202 @@ +/* + * Copyright (c) 2026 LabKey Corporation. All rights reserved. No portion of this work may be reproduced + * in any form or by any electronic or mechanical means without written permission from LabKey Corporation. + */ +import { Query } from '@labkey/api'; + +import { makeQueryInfo } from '../../internal/test/testHelpers'; +import mixturesQueryInfo from '../../test/data/mixtures-getQueryDetails.json'; +import { Row, selectRows, SelectRowsResponse } from '../../internal/query/selectRows'; + +import { ExtendedMap } from '../ExtendedMap'; +import { QueryColumn } from '../QueryColumn'; +import { QueryInfo } from '../QueryInfo'; +import { SchemaQuery } from '../SchemaQuery'; + +import { QueryModel } from './QueryModel'; +import { DefaultQueryModelLoader } from './QueryModelLoader'; +import { makeTestQueryModel } from './testUtils'; + +jest.mock('../../internal/query/selectRows', () => ({ + ...jest.requireActual('../../internal/query/selectRows'), + selectRows: jest.fn(), +})); + +const mockSelectRows = selectRows as jest.MockedFunction; + +const SCHEMA_QUERY = new SchemaQuery('exp.data', 'mixtures'); + +// Nothing for resolveRowKey() to key on: no metaData.id and no single-column primary key +const NO_PK_QUERY_INFO = new QueryInfo({}); + +// pkCol.name and pkCol.fieldKey differ, the only case where resolveRowKey()'s alt key does any work +const LOOKUP_PK_QUERY_INFO = new QueryInfo({ + pkCols: ['Parent/RowId'], + columns: new ExtendedMap({ + 'parent/rowid': new QueryColumn({ fieldKey: 'Parent/RowId', name: 'RowId' }), + }), +}); + +let MIXTURES_QUERY_INFO: QueryInfo; + +const makeRow = (values: Record): Row => + Object.entries(values).reduce((row, [fieldKey, value]) => ({ ...row, [fieldKey]: { value } }), {}); + +const mockResponse = (overrides: Partial): void => { + mockSelectRows.mockResolvedValue({ + messages: [], + metaData: undefined, + queryInfo: NO_PK_QUERY_INFO, + rowCount: 0, + rows: [], + schemaQuery: SCHEMA_QUERY, + ...overrides, + }); +}; + +beforeAll(() => { + MIXTURES_QUERY_INFO = makeQueryInfo(mixturesQueryInfo); +}); + +describe('DefaultQueryModelLoader', () => { + describe('loadRows', () => { + let consoleError: jest.SpyInstance; + const model = (): QueryModel => makeTestQueryModel(SCHEMA_QUERY, MIXTURES_QUERY_INFO); + + beforeEach(() => { + mockSelectRows.mockReset(); + consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + }); + + afterEach(() => { + consoleError.mockRestore(); + }); + + test('request options', async () => { + mockResponse({}); + const requestHandler = jest.fn(); + + await DefaultQueryModelLoader.loadRows(model(), requestHandler); + + const options = mockSelectRows.mock.calls[0][0]; + expect(options.schemaQuery).toEqual(SCHEMA_QUERY); + // left unset so selectRows() decides via isSelectRowMetadataRequired() + expect(options.includeMetadata).toBeUndefined(); + expect(options.includeTotalCount).toBe(false); + expect(options.includeStyle).toBe(true); + expect(options.requestHandler).toBe(requestHandler); + }); + + test('keys rows by metaData.id', async () => { + mockResponse({ + metaData: { id: 'RowId' } as Query.ResponseMetadata, + rowCount: 2, + rows: [makeRow({ RowId: 11, Name: 'a' }), makeRow({ RowId: 22, Name: 'b' })], + }); + + const result = await DefaultQueryModelLoader.loadRows(model()); + + expect(result.orderedRows).toEqual(['11', '22']); + expect(Object.keys(result.rows)).toEqual(['11', '22']); + expect(result.rows['11'].Name.value).toEqual('a'); + expect(consoleError).not.toHaveBeenCalled(); + }); + + // The server names a single-column PK in metaData.id regardless of the requested columns, but strips that + // column from the rows when it was not requested (minimalColumns). Such rows must still render. + test('keys rows by position when the key column is missing from the response', async () => { + mockResponse({ + metaData: { id: 'RowId' } as Query.ResponseMetadata, + rowCount: 2, + rows: [makeRow({ Name: 'a' }), makeRow({ Name: 'b' })], + }); + + const result = await DefaultQueryModelLoader.loadRows(model()); + + expect(result.orderedRows).toEqual(['0', '1']); + expect(result.rows['0'].Name.value).toEqual('a'); + expect(result.rows['1'].Name.value).toEqual('b'); + expect(consoleError).toHaveBeenCalledTimes(2); + }); + + test('retains rows missing the key column alongside keyed rows', async () => { + mockResponse({ + metaData: { id: 'RowId' } as Query.ResponseMetadata, + rowCount: 3, + rows: [makeRow({ RowId: 11, Name: 'a' }), makeRow({ Name: 'b' }), makeRow({ RowId: 33, Name: 'c' })], + }); + + const result = await DefaultQueryModelLoader.loadRows(model()); + + expect(result.orderedRows).toEqual(['11', '0', '33']); + expect(result.orderedRows.map(key => result.rows[key].Name.value)).toEqual(['a', 'b', 'c']); + expect(consoleError).toHaveBeenCalledTimes(1); + }); + + test('keys rows by position when the query has no single-column primary key', async () => { + mockResponse({ + metaData: {} as Query.ResponseMetadata, + rowCount: 2, + rows: [makeRow({ Name: 'a' }), makeRow({ Name: 'b' })], + }); + + const result = await DefaultQueryModelLoader.loadRows(model()); + + expect(result.orderedRows).toEqual(['0', '1']); + expect(consoleError).not.toHaveBeenCalled(); + }); + + test('falls back to the QueryInfo primary key when metaData is not included', async () => { + mockResponse({ + metaData: undefined, + queryInfo: MIXTURES_QUERY_INFO, + rowCount: 1, + rows: [makeRow({ RowId: 11, Name: 'a' })], + }); + + const result = await DefaultQueryModelLoader.loadRows(model()); + + expect(result.orderedRows).toEqual(['11']); + expect(consoleError).not.toHaveBeenCalled(); + }); + + test('falls back to the primary key fieldKey when the row is not keyed by column name', async () => { + mockResponse({ + metaData: undefined, + queryInfo: LOOKUP_PK_QUERY_INFO, + rowCount: 1, + rows: [makeRow({ 'Parent/RowId': 11 })], + }); + + const result = await DefaultQueryModelLoader.loadRows(model()); + + expect(result.orderedRows).toEqual(['11']); + expect(consoleError).not.toHaveBeenCalled(); + }); + + test('tolerates a null key value', async () => { + mockResponse({ + metaData: { id: 'RowId' } as Query.ResponseMetadata, + rowCount: 1, + rows: [makeRow({ RowId: null, Name: 'a' })], + }); + + const result = await DefaultQueryModelLoader.loadRows(model()); + + expect(result.orderedRows).toEqual([]); + expect(result.rows.null.Name.value).toEqual('a'); + }); + + test('passes through messages and rowCount', async () => { + const messages = [{ area: 'view', content: 'Showing 5 of 10 rows', type: 'INFO' }]; + mockResponse({ messages, rowCount: 10 }); + + const result = await DefaultQueryModelLoader.loadRows(model()); + + expect(result.messages).toStrictEqual(messages); + expect(result.rowCount).toEqual(10); + expect(result.orderedRows).toEqual([]); + expect(result.rows).toEqual({}); + }); + }); +}); From 66d6c61dbafa10d5a6dd0651969d19a39120789c Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Tue, 1 Sep 2026 11:14:40 -0700 Subject: [PATCH 07/10] Improve comments, variable naming --- .../src/public/QueryModel/QueryModelLoader.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/components/src/public/QueryModel/QueryModelLoader.ts b/packages/components/src/public/QueryModel/QueryModelLoader.ts index c1fe59bec3..d0d3b3ae69 100644 --- a/packages/components/src/public/QueryModel/QueryModelLoader.ts +++ b/packages/components/src/public/QueryModel/QueryModelLoader.ts @@ -140,23 +140,23 @@ export const DefaultQueryModelLoader: QueryModelLoader = { let fallbackKey = 0; result.rows.forEach(row => { - const val = hasKeyColumn ? (row[metadataKey] ?? row[metadataAltKey]) : undefined; + const keyCell = hasKeyColumn ? (row[metadataKey] ?? row[metadataAltKey]) : undefined; - if (hasKeyColumn && val === undefined) { + if (hasKeyColumn && keyCell === undefined) { console.error('Missing entry', result.schemaQuery.toString(true), metadataKey, metadataAltKey, row); } - // Repeated null keys collapse into a single entry here, as they did under normalizr - const key = String(val === undefined ? fallbackKey++ : val.value); + // A missing key column gets a positional key, so the row still renders + const key = String(keyCell === undefined ? fallbackKey++ : keyCell.value); rows[key] = row; - // Null-keyed rows would all collide on one key, so leave them out of the sort order as normalizr did - if (val?.value !== null) orderedRows.push(key); + // Every null key value stringifies to the same 'null', so drop those rows from the order as normalizr did + if (keyCell?.value !== null) orderedRows.push(key); }); return { messages, orderedRows, rows, rowCount }; }, - // The selection related methods may seem like overly simple passthroughs, but by putting them on QueryModelLoader, + // The selection-related methods may seem like overly simple passthroughs, but by putting them on QueryModelLoader, // instead of in withQueryModels, it allows us to easily mock them or provide alternate implementations. clearSelections(model) { const { containerFilter, selectionKey, schemaQuery, filters, queryParameters, selectionContainerPath } = model; From de8ddf178422cb7ebe0debff8e1f8b5e81ee16f5 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Tue, 1 Sep 2026 11:16:27 -0700 Subject: [PATCH 08/10] Improve types --- packages/components/src/internal/query/api.ts | 52 +++++++------------ 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/packages/components/src/internal/query/api.ts b/packages/components/src/internal/query/api.ts index f2593c652c..3563cf1fce 100644 --- a/packages/components/src/internal/query/api.ts +++ b/packages/components/src/internal/query/api.ts @@ -26,6 +26,7 @@ import { URLResolver } from '../url/URLResolver'; import { ModuleContext } from '../components/base/ServerContext'; import { handleRequestFailure, RequestHandler } from '../request'; import { EDIT_METHOD } from '../constants'; +import { Row } from './selectRows'; let queryDetailsCache: Record> = {}; @@ -442,9 +443,9 @@ export function isSelectRowMetadataRequired(includeMetadata?: boolean, columns?: export interface ISelectRowsResult { key: SchemaQueryKey; messages?: List>; - models: any; - orderedModels: List; - queries: Record; + models: Record>; + orderedModels: Record>; + queries: Record; rowCount: number; } @@ -536,22 +537,20 @@ export function resolveRowKey( return { metadataAltKey, metadataKey }; } -export function handleSelectRowsResponse(response: Query.Response, queryInfo: QueryInfo): any { +export function handleSelectRowsResponse(response: Query.Response, queryInfo: QueryInfo): Partial { const resolved = new URLResolver().resolveSelectRows(response, queryInfo); - - let count = 0, - hasRows = false, - models = {}, - orderedModels = {}, - qsKey = 'queries', - rowCount = response.rowCount || 0; - const { metadataAltKey, metadataKey } = resolveRowKey(resolved.metaData, queryInfo); const modelKey = resolveKeyFromJson(resolved); + const models: Record> = {}; + const orderedModels: Record> = {}; + const qsKey = 'queries'; + + let count = 0; + const idAttribute = '_id_'; // ensure id -- unfortunately, with normalizr 3.x there doesn't seem to be a way to generate the id // without attaching directly to the object - resolved.rows.forEach((row: any) => { + resolved.rows.forEach(row => { if (metadataKey || metadataAltKey) { const val = row[metadataKey] ?? row[metadataAltKey]; if (val !== undefined) { @@ -561,30 +560,17 @@ export function handleSelectRowsResponse(response: Query.Response, queryInfo: Qu console.error('Missing entry', metadataKey, row, resolved.schemaKey, resolved.queryName); } } - row._id_ = count++; + row[idAttribute] = count++; }); - const modelSchema = new schema.Entity( - modelKey, - {}, - { - idAttribute: '_id_', - } - ); + const modelSchema = new schema.Entity(modelKey, {}, { idAttribute }); + const querySchema = new schema.Entity(qsKey, {}, { idAttribute: queryJson => resolveKeyFromJson(queryJson) }); - const querySchema = new schema.Entity( - qsKey, - {}, - { - idAttribute: queryJson => resolveKeyFromJson(queryJson), - } - ); - - querySchema.define({ - rows: new schema.Array(modelSchema), - }); + querySchema.define({ rows: new schema.Array(modelSchema) }); const instance = normalize(resolved, querySchema); + let hasRows = false; + let rowCount = response.rowCount || 0; Object.keys(instance.entities).forEach(key => { if (key !== qsKey) { @@ -592,7 +578,7 @@ export function handleSelectRowsResponse(response: Query.Response, queryInfo: Qu const rows = instance.entities[key]; // cleanup generated ids Object.keys(rows).forEach(rowKey => { - delete rows[rowKey]['_id_']; + delete rows[rowKey][idAttribute]; }); models[key] = rows; orderedModels[key] = fromJS(instance.entities[qsKey][key].rows) From 56f60f6fe54479cb2e665f8b686f3631f8454ce1 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Tue, 1 Sep 2026 11:17:02 -0700 Subject: [PATCH 09/10] 7.61.1-fb-query-model-select-rows.0 --- packages/components/package-lock.json | 4 ++-- packages/components/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index bec98e7919..7ccfea3a02 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "7.61.0", + "version": "7.61.1-fb-query-model-select-rows.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.61.0", + "version": "7.61.1-fb-query-model-select-rows.0", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/packages/components/package.json b/packages/components/package.json index 778a5ae592..c91ffb3ead 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.61.0", + "version": "7.61.1-fb-query-model-select-rows.0", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [ From a5870eb939bb05f038696678df42234128f358d5 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Tue, 1 Sep 2026 16:22:10 -0700 Subject: [PATCH 10/10] 7.62.0 --- packages/components/package-lock.json | 4 ++-- packages/components/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index 7ccfea3a02..223032bc9d 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "7.61.1-fb-query-model-select-rows.0", + "version": "7.62.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.61.1-fb-query-model-select-rows.0", + "version": "7.62.0", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/packages/components/package.json b/packages/components/package.json index c91ffb3ead..79697f08c9 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.61.1-fb-query-model-select-rows.0", + "version": "7.62.0", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [