From b6828e2bd347540c4e361b10c8236826e9089d49 Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 23 Jul 2026 12:36:42 -0500 Subject: [PATCH 1/6] ViewInfo: update isSystemView to only care if view name starts and ends with ~~ --- packages/components/src/internal/ViewInfo.test.ts | 1 + packages/components/src/internal/ViewInfo.ts | 6 +----- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/components/src/internal/ViewInfo.test.ts b/packages/components/src/internal/ViewInfo.test.ts index d0dd0e8478..2d216127f9 100644 --- a/packages/components/src/internal/ViewInfo.test.ts +++ b/packages/components/src/internal/ViewInfo.test.ts @@ -76,6 +76,7 @@ describe('ViewInfo', () => { expect(ViewInfo.fromJson({ name: ViewInfo.DEFAULT_NAME }).isSystemView).toBeTruthy(); expect(ViewInfo.fromJson({ name: ViewInfo.DETAIL_NAME }).isSystemView).toBeTruthy(); expect(ViewInfo.fromJson({ name: ViewInfo.UPDATE_NAME }).isSystemView).toBeTruthy(); + expect(ViewInfo.fromJson({ name: '~~SOME THING~~' }).isSystemView).toBeTruthy(); }); test('modifiers', () => { diff --git a/packages/components/src/internal/ViewInfo.ts b/packages/components/src/internal/ViewInfo.ts index f01c3f9aa2..dc79aca338 100644 --- a/packages/components/src/internal/ViewInfo.ts +++ b/packages/components/src/internal/ViewInfo.ts @@ -167,11 +167,7 @@ export class ViewInfo { get isSystemView(): boolean { const lcName = this.name?.toLowerCase(); - return ( - lcName === ViewInfo.DEFAULT_NAME.toLowerCase() || - lcName === ViewInfo.DETAIL_NAME.toLowerCase() || - lcName === ViewInfo.UPDATE_NAME.toLowerCase() - ); + return lcName.startsWith('~~') && lcName.endsWith('~~'); } get modifiers(): string[] { From 24b2998d2295109e2350c0e5c2a109e00f856554 Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 23 Jul 2026 13:10:52 -0500 Subject: [PATCH 2/6] Move addSystemViewColumns from ViewInfo to QueryModel/utils --- .../components/src/internal/ViewInfo.test.ts | 184 ----------------- packages/components/src/internal/ViewInfo.ts | 32 +-- .../src/public/QueryModel/GridPanel.tsx | 4 +- .../src/public/QueryModel/utils.test.ts | 191 ++++++++++++++++++ .../components/src/public/QueryModel/utils.ts | 31 +++ 5 files changed, 225 insertions(+), 217 deletions(-) create mode 100644 packages/components/src/public/QueryModel/utils.test.ts diff --git a/packages/components/src/internal/ViewInfo.test.ts b/packages/components/src/internal/ViewInfo.test.ts index 2d216127f9..1512385058 100644 --- a/packages/components/src/internal/ViewInfo.test.ts +++ b/packages/components/src/internal/ViewInfo.test.ts @@ -96,188 +96,4 @@ describe('ViewInfo', () => { view = ViewInfo.fromJson({ shared: true, inherit: true }); expect(view.modifiers).toStrictEqual(['inherited', 'shared']); }); - - test('addSystemViewColumns, default view', () => { - let view = ViewInfo.fromJson({ - default: true, - columns: [ - { - fieldKey: 'col1', - key: 'col1', - name: 'Column 1', - }, - ], - }); - const queryInfo = new QueryInfo({ - columns: new ExtendedMap({ - hideMe: new QueryColumn({ - name: 'Hide Me', - fieldKey: 'hideMe', - }), - systemCol1: new QueryColumn({ - name: 'System Col 1', - addToSystemView: true, - fieldKey: 'systemCol1', - }), - notSystem: new QueryColumn({ - name: 'Not System', - addToSystemView: false, - fieldKey: 'notSystem', - }), - otherSystemCol: new QueryColumn({ - name: 'other', - addToSystemView: true, - fieldKey: 'other', - caption: 'Other Column', - }), - }), - }); - view = view.addSystemViewColumns(queryInfo); - expect(view.columns).toStrictEqual([ - { - fieldKey: 'col1', - key: 'col1', - name: 'Column 1', - }, - { - name: 'System Col 1', - fieldKey: 'systemCol1', - key: 'systemCol1', - title: 'System Col 1', - }, - { - name: 'other', - fieldKey: 'other', - key: 'other', - title: 'Other Column', - }, - ]); - }); - - test('addSystemViewColumns, default session view', () => { - let view = ViewInfo.fromJson({ - default: true, - session: true, - columns: [ - { - fieldKey: 'col1', - key: 'col1', - name: 'Column 1', - }, - ], - }); - const queryInfo = new QueryInfo({ - columns: new ExtendedMap({ - hideMe: new QueryColumn({ - name: 'Hide Me', - hidden: true, - fieldKey: 'hideMe', - }), - systemCol1: new QueryColumn({ - name: 'System Col 1', - addToSystemView: true, - fieldKey: 'systemCol1', - }), - }), - }); - view = view.addSystemViewColumns(queryInfo); - // if it's a session view, no additional columns should be added - expect(view.columns).toStrictEqual([ - { - fieldKey: 'col1', - key: 'col1', - name: 'Column 1', - }, - ]); - }); - - test('addSystemViewColumns, not default view', () => { - let view = ViewInfo.fromJson({ - default: false, - name: 'Not Default', - session: true, - columns: [ - { - fieldKey: 'col1', - key: 'col1', - name: 'Column 1', - }, - ], - }); - const queryInfo = new QueryInfo({ - columns: new ExtendedMap({ - hideMe: new QueryColumn({ - name: 'Hide Me', - hidden: true, - fieldKey: 'hideMe', - }), - systemCol1: new QueryColumn({ - name: 'System Col 1', - addToSystemView: true, - fieldKey: 'systemCol1', - }), - }), - }); - view = view.addSystemViewColumns(queryInfo); - // if it's not the default view, no additional columns shoulb be added - expect(view.columns).toStrictEqual([ - { - fieldKey: 'col1', - key: 'col1', - name: 'Column 1', - }, - ]); - }); - - test('addSystemViewColumns, default view, with disabledSysFields', () => { - let view = ViewInfo.fromJson({ - default: true, - columns: [ - { - fieldKey: 'col1', - key: 'col1', - name: 'Column 1', - }, - ], - }); - const queryInfo = new QueryInfo({ - columns: new ExtendedMap({ - hideMe: new QueryColumn({ - name: 'Hide Me', - fieldKey: 'hideMe', - }), - systemCol1: new QueryColumn({ - name: 'System Col 1', - addToSystemView: true, - fieldKey: 'systemCol1', - }), - notSystem: new QueryColumn({ - name: 'Not System', - addToSystemView: false, - fieldKey: 'notSystem', - }), - otherSystemCol: new QueryColumn({ - name: 'other', - addToSystemView: true, - fieldKey: 'other', - caption: 'Other Column', - }), - }), - disabledSystemFields: new Set(['Other']), - }); - view = view.addSystemViewColumns(queryInfo); - expect(view.columns).toStrictEqual([ - { - fieldKey: 'col1', - key: 'col1', - name: 'Column 1', - }, - { - name: 'System Col 1', - fieldKey: 'systemCol1', - key: 'systemCol1', - title: 'System Col 1', - }, - ]); - }); }); diff --git a/packages/components/src/internal/ViewInfo.ts b/packages/components/src/internal/ViewInfo.ts index dc79aca338..6ac3bd554a 100644 --- a/packages/components/src/internal/ViewInfo.ts +++ b/packages/components/src/internal/ViewInfo.ts @@ -3,10 +3,9 @@ * in any form or by any electronic or mechanical means without written permission from LabKey Corporation. */ import { Filter } from '@labkey/api'; -import { IQueryColumn } from '../public/IQueryColumn'; +import { IQueryColumn } from '../public/IQueryColumn'; import { QuerySort, QuerySortJson } from '../public/QuerySort'; -import { QueryInfo } from '../public/QueryInfo'; function getFiltersFromView(filters: ViewInfoFilter[]): Filter.IFilter[] { if (filters) { @@ -181,35 +180,6 @@ export class ViewInfo { return modifiers; } - addSystemViewColumns(queryInfo: QueryInfo) { - if (this.isDefault && !this.session) { - const columns = [...this.columns]; - const columnFieldKeys = columns.map(col => col.fieldKey.toLowerCase()); - const disabledSysFields = Array.from(queryInfo.disabledSystemFields ?? []).map(field => - field.toLowerCase() - ); - - queryInfo.columns.forEach(queryCol => { - const fieldKey = queryCol.fieldKey?.toLowerCase(); - if ( - fieldKey && - queryCol.addToSystemView && - columnFieldKeys.indexOf(fieldKey) === -1 && - disabledSysFields.indexOf(fieldKey) === -1 - ) { - columns.push({ - fieldKey: queryCol.fieldKey, - key: queryCol.fieldKey, - name: queryCol.name, - title: queryCol.caption || queryCol.name, - }); - } - }); - return this.mutate({ columns }); - } - return this; - } - mutate(updates: Partial) { return new ViewInfo({ ...this, diff --git a/packages/components/src/public/QueryModel/GridPanel.tsx b/packages/components/src/public/QueryModel/GridPanel.tsx index 3340ec9cd1..6baad00d17 100644 --- a/packages/components/src/public/QueryModel/GridPanel.tsx +++ b/packages/components/src/public/QueryModel/GridPanel.tsx @@ -70,7 +70,7 @@ import { ExportMenu } from './ExportMenu'; import { SelectionStatus } from './SelectionStatus'; import { ChartMenu } from './ChartMenu'; import { SearchBox } from './SearchBox'; -import { actionValuesToString, filterArraysEqual, filtersEqual, sortsEqual } from './utils'; +import { actionValuesToString, addSystemViewColumns, filterArraysEqual, filtersEqual, sortsEqual } from './utils'; import { GridFilterModal } from './GridFilterModal'; import { FiltersButton } from './FiltersButton'; import { FilterStatus } from './FilterStatus'; @@ -852,7 +852,7 @@ export class GridPanel extends PureComponent, State> { return new Promise((resolve, reject) => { const view = queryInfo?.getView(viewName, true); - let updatedViewInfo = view.addSystemViewColumns(queryInfo); + let updatedViewInfo = addSystemViewColumns(view, queryInfo); updatedViewInfo = updatedViewInfo.mutate({ // update/set sorts and filters to combine view and user-defined items filters: model.filterArray.concat(view.filters), diff --git a/packages/components/src/public/QueryModel/utils.test.ts b/packages/components/src/public/QueryModel/utils.test.ts new file mode 100644 index 0000000000..f9caf72934 --- /dev/null +++ b/packages/components/src/public/QueryModel/utils.test.ts @@ -0,0 +1,191 @@ +import { ViewInfo } from '../../internal/ViewInfo'; +import { QueryInfo } from '../QueryInfo'; +import { ExtendedMap } from '../ExtendedMap'; +import { QueryColumn } from '../QueryColumn'; +import { addSystemViewColumns } from './utils'; + +describe('addSystemViewColumns', () => { + test('default view', () => { + let view = ViewInfo.fromJson({ + default: true, + columns: [ + { + fieldKey: 'col1', + key: 'col1', + name: 'Column 1', + }, + ], + }); + const queryInfo = new QueryInfo({ + columns: new ExtendedMap({ + hideMe: new QueryColumn({ + name: 'Hide Me', + fieldKey: 'hideMe', + }), + systemCol1: new QueryColumn({ + name: 'System Col 1', + addToSystemView: true, + fieldKey: 'systemCol1', + }), + notSystem: new QueryColumn({ + name: 'Not System', + addToSystemView: false, + fieldKey: 'notSystem', + }), + otherSystemCol: new QueryColumn({ + name: 'other', + addToSystemView: true, + fieldKey: 'other', + caption: 'Other Column', + }), + }), + }); + view = addSystemViewColumns(view, queryInfo); + expect(view.columns).toStrictEqual([ + { + fieldKey: 'col1', + key: 'col1', + name: 'Column 1', + }, + { + name: 'System Col 1', + fieldKey: 'systemCol1', + key: 'systemCol1', + title: 'System Col 1', + }, + { + name: 'other', + fieldKey: 'other', + key: 'other', + title: 'Other Column', + }, + ]); + }); + + test('default session view', () => { + let view = ViewInfo.fromJson({ + default: true, + session: true, + columns: [ + { + fieldKey: 'col1', + key: 'col1', + name: 'Column 1', + }, + ], + }); + const queryInfo = new QueryInfo({ + columns: new ExtendedMap({ + hideMe: new QueryColumn({ + name: 'Hide Me', + hidden: true, + fieldKey: 'hideMe', + }), + systemCol1: new QueryColumn({ + name: 'System Col 1', + addToSystemView: true, + fieldKey: 'systemCol1', + }), + }), + }); + view = addSystemViewColumns(view, queryInfo); + // if it's a session view, no additional columns should be added + expect(view.columns).toStrictEqual([ + { + fieldKey: 'col1', + key: 'col1', + name: 'Column 1', + }, + ]); + }); + + test('not default view', () => { + let view = ViewInfo.fromJson({ + default: false, + name: 'Not Default', + session: true, + columns: [ + { + fieldKey: 'col1', + key: 'col1', + name: 'Column 1', + }, + ], + }); + const queryInfo = new QueryInfo({ + columns: new ExtendedMap({ + hideMe: new QueryColumn({ + name: 'Hide Me', + hidden: true, + fieldKey: 'hideMe', + }), + systemCol1: new QueryColumn({ + name: 'System Col 1', + addToSystemView: true, + fieldKey: 'systemCol1', + }), + }), + }); + view = addSystemViewColumns(view, queryInfo); + // if it's not the default view, no additional columns shoulb be added + expect(view.columns).toStrictEqual([ + { + fieldKey: 'col1', + key: 'col1', + name: 'Column 1', + }, + ]); + }); + + test('default view, with disabledSysFields', () => { + let view = ViewInfo.fromJson({ + default: true, + columns: [ + { + fieldKey: 'col1', + key: 'col1', + name: 'Column 1', + }, + ], + }); + const queryInfo = new QueryInfo({ + columns: new ExtendedMap({ + hideMe: new QueryColumn({ + name: 'Hide Me', + fieldKey: 'hideMe', + }), + systemCol1: new QueryColumn({ + name: 'System Col 1', + addToSystemView: true, + fieldKey: 'systemCol1', + }), + notSystem: new QueryColumn({ + name: 'Not System', + addToSystemView: false, + fieldKey: 'notSystem', + }), + otherSystemCol: new QueryColumn({ + name: 'other', + addToSystemView: true, + fieldKey: 'other', + caption: 'Other Column', + }), + }), + disabledSystemFields: new Set(['Other']), + }); + view = addSystemViewColumns(view, queryInfo); + expect(view.columns).toStrictEqual([ + { + fieldKey: 'col1', + key: 'col1', + name: 'Column 1', + }, + { + name: 'System Col 1', + fieldKey: 'systemCol1', + key: 'systemCol1', + title: 'System Col 1', + }, + ]); + }); +}); diff --git a/packages/components/src/public/QueryModel/utils.ts b/packages/components/src/public/QueryModel/utils.ts index af48fd3bd7..ed5aa8349e 100644 --- a/packages/components/src/public/QueryModel/utils.ts +++ b/packages/components/src/public/QueryModel/utils.ts @@ -24,6 +24,8 @@ import { caseInsensitive } from '../../internal/util/utils'; import { ActionValue } from './grid/actions/Action'; import { QueryModel } from './QueryModel'; +import { QueryInfo } from '../QueryInfo'; +import { ViewInfo } from '../../internal/ViewInfo'; export function filterToString(filter: Filter.IFilter): string { return `${filter.getColumnName()}-${filter.getFilterType().getURLSuffix()}-${filter.getValue()}`; @@ -207,3 +209,32 @@ export async function createOrderedSnapshotSelectionKey(model: QueryModel): Prom const orderedRows = rows.map(row => caseInsensitive(row, pkFieldKey).value.toString()); return _createSnapshotSelectionKey(model, orderedRows); } + +export function addSystemViewColumns(view: ViewInfo, queryInfo: QueryInfo) { + if (view.isDefault && !view.session) { + const columns = [...view.columns]; + const columnFieldKeys = columns.map(col => col.fieldKey.toLowerCase()); + const disabledSysFields = Array.from(queryInfo.disabledSystemFields ?? []).map(field => + field.toLowerCase() + ); + + queryInfo.columns.forEach(queryCol => { + const fieldKey = queryCol.fieldKey?.toLowerCase(); + if ( + fieldKey && + queryCol.addToSystemView && + columnFieldKeys.indexOf(fieldKey) === -1 && + disabledSysFields.indexOf(fieldKey) === -1 + ) { + columns.push({ + fieldKey: queryCol.fieldKey, + key: queryCol.fieldKey, + name: queryCol.name, + title: queryCol.caption || queryCol.name, + }); + } + }); + return view.mutate({ columns }); + } + return view; +} From b5ea003b7190baa13ceff6ec0ed8a5d99d75a65e Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 23 Jul 2026 15:23:28 -0500 Subject: [PATCH 3/6] useLoadableState: add optional passthrough args for resolveErrorMessage --- .../components/src/internal/useLoadableState.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/components/src/internal/useLoadableState.ts b/packages/components/src/internal/useLoadableState.ts index 2822389b41..003efe4a1f 100644 --- a/packages/components/src/internal/useLoadableState.ts +++ b/packages/components/src/internal/useLoadableState.ts @@ -8,6 +8,12 @@ import { LoadingState } from '../public/LoadingState'; import { resolveErrorMessage } from './util/messaging'; +interface ResolveErrorArgs { + nounPlural: string; + nounSingular: string; + verb: string; +} + export interface LoadableState { error: string; load: () => Promise; @@ -20,7 +26,7 @@ export interface LoadableState { export type Loader = () => Promise; -export function useLoadableState(loader: Loader): LoadableState { +export function useLoadableState(loader: Loader, resolveErrorArgs?: ResolveErrorArgs): LoadableState { const [error, setError] = useState(undefined); const [loadingState, setLoadingState] = useState(LoadingState.INITIALIZED); const [value, setValue] = useState(undefined); @@ -35,14 +41,15 @@ export function useLoadableState(loader: Loader): LoadableState { // Note: it's important to log the error here, because if consumers don't use the error object returned here // then you may not know an error happened, so this way we at least have some trace of an issue. console.error(e); - setError(resolveErrorMessage(e)); + const { nounPlural, nounSingular, verb } = resolveErrorArgs ?? {}; + setError(resolveErrorMessage(e, nounSingular, nounPlural, verb)); // We set value to undefined here because it's possible we loaded something correctly once before, but the // loader changed and load got triggered again. setValue(undefined); } finally { setLoadingState(LoadingState.LOADED); } - }, [loader]); + }, [loader, resolveErrorArgs]); const state = useMemo( () => ({ error, From 1e74ee081a8a8ba085ea2011faa6f3cb707cb612 Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 23 Jul 2026 15:43:11 -0500 Subject: [PATCH 4/6] ViewInfo: make isSystemView more resilient --- packages/components/src/internal/ViewInfo.test.ts | 3 +++ packages/components/src/internal/ViewInfo.ts | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/components/src/internal/ViewInfo.test.ts b/packages/components/src/internal/ViewInfo.test.ts index 1512385058..af3a8074b4 100644 --- a/packages/components/src/internal/ViewInfo.test.ts +++ b/packages/components/src/internal/ViewInfo.test.ts @@ -71,12 +71,15 @@ describe('ViewInfo', () => { test('isSystemView', () => { expect(ViewInfo.fromJson({}).isSystemView).toBeTruthy(); expect(ViewInfo.fromJson({ name: '' }).isSystemView).toBeTruthy(); + // undefined is turned into the default view + expect(ViewInfo.fromJson({ name: undefined }).isSystemView).toBeTruthy(); expect(ViewInfo.fromJson({ name: 'testing' }).isSystemView).toBeFalsy(); expect(ViewInfo.fromJson({ name: ViewInfo.BIO_DETAIL_NAME }).isSystemView).toBeFalsy(); expect(ViewInfo.fromJson({ name: ViewInfo.DEFAULT_NAME }).isSystemView).toBeTruthy(); expect(ViewInfo.fromJson({ name: ViewInfo.DETAIL_NAME }).isSystemView).toBeTruthy(); expect(ViewInfo.fromJson({ name: ViewInfo.UPDATE_NAME }).isSystemView).toBeTruthy(); expect(ViewInfo.fromJson({ name: '~~SOME THING~~' }).isSystemView).toBeTruthy(); + expect(ViewInfo.fromJson({ name: null }).isSystemView).toBeFalsy(); }); test('modifiers', () => { diff --git a/packages/components/src/internal/ViewInfo.ts b/packages/components/src/internal/ViewInfo.ts index 6ac3bd554a..bd1d61e1ad 100644 --- a/packages/components/src/internal/ViewInfo.ts +++ b/packages/components/src/internal/ViewInfo.ts @@ -166,7 +166,7 @@ export class ViewInfo { get isSystemView(): boolean { const lcName = this.name?.toLowerCase(); - return lcName.startsWith('~~') && lcName.endsWith('~~'); + return lcName?.startsWith('~~') && lcName?.endsWith('~~'); } get modifiers(): string[] { From aa59ae9c48d7e17aaa5c42d80b20fb5320bc4e78 Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 23 Jul 2026 16:06:29 -0500 Subject: [PATCH 5/6] useLoadableState: reduce chances of accidental reloads with resolveErrorArgs --- packages/components/src/internal/useLoadableState.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/src/internal/useLoadableState.ts b/packages/components/src/internal/useLoadableState.ts index 003efe4a1f..1ea7f5c1ec 100644 --- a/packages/components/src/internal/useLoadableState.ts +++ b/packages/components/src/internal/useLoadableState.ts @@ -27,6 +27,7 @@ export interface LoadableState { export type Loader = () => Promise; export function useLoadableState(loader: Loader, resolveErrorArgs?: ResolveErrorArgs): LoadableState { + const { nounPlural, nounSingular, verb } = resolveErrorArgs ?? {}; const [error, setError] = useState(undefined); const [loadingState, setLoadingState] = useState(LoadingState.INITIALIZED); const [value, setValue] = useState(undefined); @@ -41,7 +42,6 @@ export function useLoadableState(loader: Loader, resolveErrorArgs?: Resolv // Note: it's important to log the error here, because if consumers don't use the error object returned here // then you may not know an error happened, so this way we at least have some trace of an issue. console.error(e); - const { nounPlural, nounSingular, verb } = resolveErrorArgs ?? {}; setError(resolveErrorMessage(e, nounSingular, nounPlural, verb)); // We set value to undefined here because it's possible we loaded something correctly once before, but the // loader changed and load got triggered again. @@ -49,7 +49,7 @@ export function useLoadableState(loader: Loader, resolveErrorArgs?: Resolv } finally { setLoadingState(LoadingState.LOADED); } - }, [loader, resolveErrorArgs]); + }, [loader, nounPlural, nounSingular, verb]); const state = useMemo( () => ({ error, From 6f3f285570983a807b272f0c361c68833cb94d87 Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 23 Jul 2026 16:08:24 -0500 Subject: [PATCH 6/6] ViewInfo.isSystemView: force boolean return --- packages/components/src/internal/ViewInfo.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/internal/ViewInfo.ts b/packages/components/src/internal/ViewInfo.ts index bd1d61e1ad..5d5c53b64a 100644 --- a/packages/components/src/internal/ViewInfo.ts +++ b/packages/components/src/internal/ViewInfo.ts @@ -166,7 +166,7 @@ export class ViewInfo { get isSystemView(): boolean { const lcName = this.name?.toLowerCase(); - return lcName?.startsWith('~~') && lcName?.endsWith('~~'); + return !!(lcName?.startsWith('~~') && lcName?.endsWith('~~')); } get modifiers(): string[] {