diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index bec98e7919..223032bc9d 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.62.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.61.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 778a5ae592..79697f08c9 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.61.0", + "version": "7.62.0", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [ 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/api.ts b/packages/components/src/internal/query/api.ts index df5f5a3a5f..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; } @@ -512,20 +513,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; +export function resolveRowKey( + metaData: Query.ResponseMetadata | undefined, + queryInfo: QueryInfo +): { metadataAltKey: string; metadataKey: string } { + let metadataAltKey: string; + let metadataKey: string; - let metadataAltKey: string, metadataKey: string; - if (resolved.metaData) { + 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,11 +533,24 @@ export function handleSelectRowsResponse(response: Query.Response, queryInfo: Qu } } } + + return { metadataAltKey, metadataKey }; +} + +export function handleSelectRowsResponse(response: Query.Response, queryInfo: QueryInfo): Partial { + const resolved = new URLResolver().resolveSelectRows(response, queryInfo); + 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) { @@ -550,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 querySchema = new schema.Entity( - qsKey, - {}, - { - idAttribute: queryJson => resolveKeyFromJson(queryJson), - } - ); + const modelSchema = new schema.Entity(modelKey, {}, { idAttribute }); + 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) { @@ -581,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) diff --git a/packages/components/src/internal/query/selectRows.ts b/packages/components/src/internal/query/selectRows.ts index 1ca85e0894..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,9 @@ 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; rowCount: number; rows: Row[]; @@ -86,6 +94,7 @@ export async function selectRows(options: SelectRowsOptions): Promise 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.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({}); + }); + }); +}); diff --git a/packages/components/src/public/QueryModel/QueryModelLoader.ts b/packages/components/src/public/QueryModel/QueryModelLoader.ts index e8ca825f46..d0d3b3ae69 100644 --- a/packages/components/src/public/QueryModel/QueryModelLoader.ts +++ b/packages/components/src/public/QueryModel/QueryModelLoader.ts @@ -19,7 +19,7 @@ import { DataViewInfoTypes, VISUALIZATION_REPORTS } from '../../internal/constan import { DataViewInfo, IDataViewInfo } from '../../internal/DataViewInfo'; import { RequestHandler } from '../../internal/request'; import { getQueryColumnRenderers } from '../../internal/global'; -import { getQueryDetails, selectRowsDeprecated } from '../../internal/query/api'; +import { getQueryDetails, resolveRowKey } from '../../internal/query/api'; import { DefaultRenderer } from '../../internal/renderers/DefaultRenderer'; import { ExtendedMap } from '../ExtendedMap'; import { QueryColumn } from '../QueryColumn'; @@ -27,7 +27,8 @@ import { QueryColumn } from '../QueryColumn'; import { QueryInfo } from '../QueryInfo'; import { naturalSortByProperty } from '../sort'; -import { GridMessage, QueryModel } from './QueryModel'; +import { QueryModel } from './QueryModel'; +import { Row, selectRows, SelectRowsMessage } from '../../internal/query/selectRows'; export function bindColumnRenderers(columns: ExtendedMap): ExtendedMap { if (columns) { @@ -52,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 @@ -124,24 +125,38 @@ 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, 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; - - return { - messages: messages.toJS(), - rows: models[key], - orderedRows: orderedModels[key].toArray(), - rowCount, - }; + + 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 => { + const keyCell = hasKeyColumn ? (row[metadataKey] ?? row[metadataAltKey]) : undefined; + + if (hasKeyColumn && keyCell === undefined) { + console.error('Missing entry', result.schemaQuery.toString(true), metadataKey, metadataAltKey, row); + } + + // A missing key column gets a positional key, so the row still renders + const key = String(keyCell === undefined ? fallbackKey++ : keyCell.value); + rows[key] = row; + + // 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; 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];