Skip to content

ViewInfo: simplify isSystemView, remove circular import #2040

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
labkey-alan wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

ViewInfo: simplify isSystemView, remove circular import #2040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b6828e2
ViewInfo: update isSystemView to only care if view name starts and en…
labkey-alan Jul 23, 2026
24b2998
Move addSystemViewColumns from ViewInfo to QueryModel/utils
labkey-alan Jul 23, 2026
b5ea003
useLoadableState: add optional passthrough args for resolveErrorMessage
labkey-alan Jul 23, 2026
1e74ee0
ViewInfo: make isSystemView more resilient
labkey-alan Jul 23, 2026
aa59ae9
useLoadableState: reduce chances of accidental reloads with resolveEr…
labkey-alan Jul 23, 2026
6f3f285
ViewInfo.isSystemView: force boolean return
labkey-alan Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 4 additions & 184 deletions packages/components/src/internal/ViewInfo.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +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', () => {
Expand All @@ -95,188 +99,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',
},
]);
});
});
38 changes: 2 additions & 36 deletions packages/components/src/internal/ViewInfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -167,11 +166,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[] {
Expand All @@ -185,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<ViewInfo>) {
return new ViewInfo({
...this,
Expand Down
13 changes: 10 additions & 3 deletions packages/components/src/internal/useLoadableState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
error: string;
load: () => Promise<void>;
Expand All @@ -20,7 +26,8 @@ export interface LoadableState<T> {

export type Loader<T> = () => Promise<T>;

export function useLoadableState<T>(loader: Loader<T>): LoadableState<T> {
export function useLoadableState<T>(loader: Loader<T>, resolveErrorArgs?: ResolveErrorArgs): LoadableState<T> {
const { nounPlural, nounSingular, verb } = resolveErrorArgs ?? {};
const [error, setError] = useState<string>(undefined);
const [loadingState, setLoadingState] = useState<LoadingState>(LoadingState.INITIALIZED);
const [value, setValue] = useState<T>(undefined);
Expand All @@ -35,14 +42,14 @@ export function useLoadableState<T>(loader: Loader<T>): LoadableState<T> {
// 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));
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, nounPlural, nounSingular, verb]);
const state = useMemo(
() => ({
error,
Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/public/QueryModel/GridPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -852,7 +852,7 @@ export class GridPanel<T = {}> extends PureComponent<Props<T>, 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),
Expand Down
Loading