Skip to content

Commit a0f9425

Browse files
authored
GitHub Issue 796: App assay Samples > Find Derivatives in Sample Finder not working for field with special characters (#1809)
### version 6.49.1 *Released*: 16 June 2025 - GH Issue 796: App assay Samples > Find Derivatives in Sample Finder not working for field with special characters - update getLegalIdentifier to include separator param - FindDerivativesMenuItem update to allow titleCol prop to be passed in - GH Issue 740: Disable "Find Derivatives in Sample Finder" if grid has no rows
1 parent 3292f8d commit a0f9425

7 files changed

Lines changed: 91 additions & 50 deletions

File tree

packages/components/package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/components/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@labkey/components",
3-
"version": "6.49.0",
3+
"version": "6.49.1",
44
"description": "Components, models, actions, and utility functions for LabKey applications and pages",
55
"sideEffects": false,
66
"files": [

packages/components/releaseNotes/components.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
# @labkey/components
22
Components, models, actions, and utility functions for LabKey applications and pages
33

4+
### version 6.49.1
5+
*Released*: 16 June 2025
6+
- GH Issue 796: App assay Samples > Find Derivatives in Sample Finder not working for field with special characters
7+
- update getLegalIdentifier to include separator param
8+
- FindDerivativesMenuItem update to allow titleCol prop to be passed in
9+
- GH Issue 740: Disable "Find Derivatives in Sample Finder" if grid has no rows
10+
411
### version 6.49.0
512
*Released*: 11 June 2025
613
- Remove SOURCE_TYPE_KEY constant

packages/components/src/internal/components/entities/FindDerivativesButton.spec.tsx renamed to packages/components/src/internal/components/entities/FindDerivativesButton.test.tsx

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,14 @@ import { QueryColumn } from '../../../public/QueryColumn';
88
import { makeTestQueryModel } from '../../../public/QueryModel/testUtils';
99
import { SchemaQuery } from '../../../public/SchemaQuery';
1010

11-
import { mountWithAppServerContext } from '../../test/enzymeTestHelpers';
12-
import { DisableableMenuItem } from '../samples/DisableableMenuItem';
13-
1411
import { TestTypeDataType, TestTypeDataTypeWithEntityFilter } from '../../../test/data/constants';
1512

1613
import { FieldFilter } from '../search/models';
1714

1815
import { SCHEMAS } from '../../schemas';
1916

17+
import { renderWithAppContext } from '../../test/reactTestLibraryHelpers';
18+
2019
import {
2120
FindDerivativesMenuItem,
2221
getFieldFilter,
@@ -87,6 +86,7 @@ const QUERY_INFO = QueryInfo.fromJsonForTests({
8786
const MODEL = makeTestQueryModel(new SchemaQuery('samples', 'query', VIEW_NAME), QUERY_INFO).mutate({
8887
baseFilters: [Filter.create('a', null, Filter.Types.ISBLANK)],
8988
filterArray: [Filter.create('b', null, Filter.Types.ISBLANK)],
89+
rowCount: 1,
9090
});
9191

9292
describe('FindDerivativesButton', () => {
@@ -96,11 +96,18 @@ describe('FindDerivativesButton', () => {
9696
};
9797

9898
test('default props', () => {
99-
const wrapper = mountWithAppServerContext(<FindDerivativesMenuItem {...DEFAULT_PROPS} asSubMenu />);
100-
expect(wrapper.find(DisableableMenuItem)).toHaveLength(1);
101-
expect(wrapper.find(DisableableMenuItem).prop('disabled')).toBe(false);
102-
expect(wrapper.find(DisableableMenuItem).prop('disabledMessage')).toContain(' ()');
103-
wrapper.unmount();
99+
renderWithAppContext(<FindDerivativesMenuItem {...DEFAULT_PROPS} asSubMenu />);
100+
expect(document.querySelectorAll('.lk-menu-item')).toHaveLength(1);
101+
expect(document.querySelectorAll('.lk-menu-item.disabled')).toHaveLength(0);
102+
});
103+
104+
test('disable menu item for grid with no rows', () => {
105+
const model2 = MODEL.mutate({
106+
rowCount: 0,
107+
});
108+
renderWithAppContext(<FindDerivativesMenuItem {...DEFAULT_PROPS} model={model2} asSubMenu />);
109+
expect(document.querySelectorAll('.lk-menu-item')).toHaveLength(1);
110+
expect(document.querySelectorAll('.lk-menu-item.disabled')).toHaveLength(1);
104111
});
105112

106113
test('invalidFilterNames from search, disabled button', () => {
@@ -110,22 +117,18 @@ describe('FindDerivativesButton', () => {
110117
Filter.create('*', 'test', Filter.Types.CONTIANS),
111118
],
112119
});
113-
const wrapper = mountWithAppServerContext(<FindDerivativesMenuItem {...DEFAULT_PROPS} model={model2} />);
114-
expect(wrapper.find(DisableableMenuItem)).toHaveLength(1);
115-
expect(wrapper.find(DisableableMenuItem).prop('disabled')).toBe(true);
116-
expect(wrapper.find(DisableableMenuItem).prop('disabledMessage')).toContain(' (Search Filter)');
117-
wrapper.unmount();
120+
renderWithAppContext(<FindDerivativesMenuItem {...DEFAULT_PROPS} model={model2} />);
121+
expect(document.querySelectorAll('.lk-menu-item')).toHaveLength(1);
122+
expect(document.querySelectorAll('.lk-menu-item.disabled')).toHaveLength(1);
118123
});
119124

120125
test('invalidFilterNames from MVFK, disabled button', () => {
121126
const model2 = MODEL.mutate({
122127
filterArray: [Filter.create('e', null, Filter.Types.ISBLANK)],
123128
});
124-
const wrapper = mountWithAppServerContext(<FindDerivativesMenuItem {...DEFAULT_PROPS} model={model2} />);
125-
expect(wrapper.find(DisableableMenuItem)).toHaveLength(1);
126-
expect(wrapper.find(DisableableMenuItem).prop('disabled')).toBe(true);
127-
expect(wrapper.find(DisableableMenuItem).prop('disabledMessage')).toContain(' (FieldE)');
128-
wrapper.unmount();
129+
renderWithAppContext(<FindDerivativesMenuItem {...DEFAULT_PROPS} model={model2} />);
130+
expect(document.querySelectorAll('.lk-menu-item')).toHaveLength(1);
131+
expect(document.querySelectorAll('.lk-menu-item.disabled')).toHaveLength(1);
129132
});
130133
});
131134

@@ -160,43 +163,43 @@ describe('getFieldFilter', () => {
160163
});
161164

162165
describe('getSessionSearchFilterProps', () => {
163-
test('no filters', () => {
164-
const props = getSessionSearchFilterProps(SampleTypeDataType, MODEL, []);
166+
test('no filters', async () => {
167+
const props = await getSessionSearchFilterProps(SampleTypeDataType, MODEL, []);
165168
expect(props).toHaveLength(1);
166169
expect(props[0].filterArray).toHaveLength(0);
167170
expect(props[0].dataTypeDisplayName).toBe('query');
168171
expect(props[0].entityDataType).toBe(SampleTypeDataType);
169172
});
170173

171-
test('with model title', () => {
174+
test('with model title', async () => {
172175
const model2 = MODEL.mutate({ title: 'TestingTitle' });
173-
const props = getSessionSearchFilterProps(SampleTypeDataType, model2, []);
176+
const props = await getSessionSearchFilterProps(SampleTypeDataType, model2, []);
174177
expect(props).toHaveLength(1);
175178
expect(props[0].dataTypeDisplayName).toBe('TestingTitle');
176179
});
177180

178-
test('with filters', () => {
179-
const props = getSessionSearchFilterProps(SampleTypeDataType, MODEL, [
181+
test('with filters', async () => {
182+
const props = await getSessionSearchFilterProps(SampleTypeDataType, MODEL, [
180183
Filter.create('a', 'val1'),
181184
Filter.create('b', 'val2'),
182185
]);
183186
expect(props).toHaveLength(1);
184187
expect(props[0].filterArray).toHaveLength(2);
185188
});
186189

187-
test('baseFilter, without baseModel', () => {
188-
const props = getSessionSearchFilterProps(SampleTypeDataType, MODEL, [], undefined, undefined, [
190+
test('baseFilter, without baseModel', async () => {
191+
const props = await getSessionSearchFilterProps(SampleTypeDataType, MODEL, [], undefined, undefined, [
189192
Filter.create('a', 'Something'),
190193
]);
191194
expect(props).toHaveLength(1);
192195
expect(props[0].filterArray).toHaveLength(1);
193196
});
194197

195-
test('baseFilter, with baseModel for sample type', () => {
198+
test('baseFilter, with baseModel for sample type', async () => {
196199
const baseModel = MODEL.mutate({
197200
schemaQuery: new SchemaQuery('samples', 'query2'),
198201
});
199-
const props = getSessionSearchFilterProps(SampleTypeDataType, MODEL, [], SampleTypeDataType, baseModel, [
202+
const props = await getSessionSearchFilterProps(SampleTypeDataType, MODEL, [], SampleTypeDataType, baseModel, [
200203
Filter.create('a', 'Something'),
201204
]);
202205
expect(props).toHaveLength(2);
@@ -206,11 +209,11 @@ describe('getSessionSearchFilterProps', () => {
206209
expect(props[1].filterArray).toHaveLength(0);
207210
});
208211

209-
test('baseFilter, with baseModel for data class', () => {
212+
test('baseFilter, with baseModel for data class', async () => {
210213
const baseModel = MODEL.mutate({
211214
schemaQuery: new SchemaQuery('exp.data', 'query3'),
212215
});
213-
const props = getSessionSearchFilterProps(SampleTypeDataType, MODEL, [], DataClassDataType, baseModel, [
216+
const props = await getSessionSearchFilterProps(SampleTypeDataType, MODEL, [], DataClassDataType, baseModel, [
214217
Filter.create('a', 'Something'),
215218
]);
216219
expect(props).toHaveLength(2);

packages/components/src/internal/components/entities/FindDerivativesButton.tsx

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ import { ResponsiveMenuButton } from '../buttons/ResponsiveMenuButton';
1818
import { getSelectedDataDeprecated } from '../../actions';
1919
import { caseInsensitive } from '../../util/utils';
2020

21-
import { EntityDataType, FilterProps } from './models';
2221
import { getPrimaryAppProductId } from '../../app/products';
2322

23+
import { EntityDataType, FilterProps } from './models';
24+
2425
export const SAMPLE_FINDER_SESSION_PREFIX = 'Searched ';
2526

2627
const MAX_SELECTION = 200;
@@ -62,7 +63,8 @@ export const getSessionSearchFilterProps = async (
6263
filters: Filter.IFilter[],
6364
baseEntityDataType?: EntityDataType,
6465
baseModel?: QueryModel,
65-
baseFilter?: Filter.IFilter[]
66+
baseFilter?: Filter.IFilter[],
67+
titleCol?: QueryColumn
6668
): Promise<FilterProps[]> => {
6769
let fieldFilters = [];
6870
// optionally include baseFilter when passed without a baseModel (i.e. apply to the same schemaQuery as the other filters)
@@ -75,28 +77,28 @@ export const getSessionSearchFilterProps = async (
7577
// Issue 47087: if model has selections, include those as IN clause
7678
if (model.hasSelections && model.queryInfo.pkCols?.length === 1 && model.queryInfo.titleColumn) {
7779
const pkCol = model.queryInfo.getPkCols()[0];
78-
const titleCol = model.queryInfo.getColumn(model.queryInfo.titleColumn);
80+
const titleCol_ = titleCol ?? model.queryInfo.getColumnFromName(model.queryInfo.titleColumn);
7981

8082
const selectedData = await getSelectedDataDeprecated(
8183
model.schemaName,
8284
model.queryName,
8385
Array.from(model.selections),
84-
[pkCol.fieldKey, titleCol.fieldKey],
86+
[pkCol.fieldKey, titleCol_.fieldKey],
8587
undefined,
8688
model.queryParameters,
8789
model.viewName,
8890
pkCol.fieldKey
8991
);
9092
const selectedValues = [];
9193
selectedData.data.forEach(row => {
92-
selectedValues.push(caseInsensitive(row.toJS(), titleCol.fieldKey).value);
94+
selectedValues.push(caseInsensitive(row.toJS(), titleCol_.name).value);
9395
});
9496

9597
fieldFilters.push({
96-
fieldKey: titleCol.fieldKey,
98+
fieldKey: titleCol_.fieldKey,
9799
fieldCaption: 'Selection',
98-
filter: Filter.create(titleCol.fieldKey, selectedValues, Filter.Types.IN),
99-
jsonType: titleCol.jsonType,
100+
filter: Filter.create(titleCol_.fieldKey, selectedValues, Filter.Types.IN),
101+
jsonType: titleCol_.jsonType,
100102
});
101103
}
102104

@@ -171,10 +173,11 @@ interface Props {
171173
entityDataType: EntityDataType;
172174
metricFeatureArea?: string;
173175
model: QueryModel;
176+
titleCol?: QueryColumn;
174177
}
175178

176179
export const FindDerivativesMenuItem: FC<Props> = memo(props => {
177-
const { baseEntityDataType, baseModel, baseFilter, model, entityDataType, metricFeatureArea } = props;
180+
const { baseEntityDataType, baseModel, baseFilter, model, entityDataType, metricFeatureArea, titleCol } = props;
178181
const { api } = useAppContext();
179182

180183
const viewAndUserFilters = useMemo(
@@ -207,7 +210,8 @@ export const FindDerivativesMenuItem: FC<Props> = memo(props => {
207210
viewAndUserFilters,
208211
baseEntityDataType,
209212
baseModel,
210-
baseFilter
213+
baseFilter,
214+
titleCol
211215
);
212216

213217
sessionStorage.setItem(getSampleFinderLocalStorageKey(), searchFiltersToJson(filterProps, 0, currentTimestamp));
@@ -221,10 +225,10 @@ export const FindDerivativesMenuItem: FC<Props> = memo(props => {
221225
if (!model.queryInfo) return null;
222226

223227
const validSelection = !model.selections?.size || model.selections.size <= MAX_SELECTION;
224-
const disabled = !validSelection || invalidFilterNames !== '';
225-
const disabledMessage = !validSelection
226-
? 'At most ' + MAX_SELECTION + ' can be selected'
227-
: DISABLED_FIND_DERIVATIVES_MSG + ' (' + invalidFilterNames + ').';
228+
const disabled = model.rowCount === 0 || !validSelection || invalidFilterNames !== '';
229+
let disabledMessage = DISABLED_FIND_DERIVATIVES_MSG + ' (' + invalidFilterNames + ').';
230+
if (!validSelection) disabledMessage = 'At most ' + MAX_SELECTION + ' can be selected.';
231+
if (model.rowCount === 0) disabledMessage = 'No samples available in the current grid view.';
228232

229233
return (
230234
<DisableableMenuItem disabled={disabled} disabledMessage={disabledMessage} onClick={onClick} placement="right">

packages/components/src/internal/query/filter.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Filter } from '@labkey/api';
22

33
import { formatDate, getNDaysStrFromToday } from '../util/Date';
44

5-
import { COLUMN_NOT_IN_FILTER_TYPE, getFilterLabKeySql } from './filter';
5+
import { COLUMN_NOT_IN_FILTER_TYPE, getFilterLabKeySql, getLegalIdentifier } from './filter';
66

77
const datePOSIX = 1596750283812; // Aug 6, 2020 14:44 America/Los_Angeles
88
const testDate = new Date(datePOSIX);
@@ -535,3 +535,26 @@ describe('getFilterLabKeySql', () => {
535535
expect(getFilterLabKeySql(Filter.create('StringField', 'abc', Filter.Types.MEMBER_OF), 'string')).toBeNull();
536536
});
537537
});
538+
539+
describe('getLegalIdentifier', () => {
540+
test('columnName', () => {
541+
expect(getLegalIdentifier('testColumn')).toBe('"testColumn"');
542+
expect(getLegalIdentifier('test"Column')).toBe('"test""Column"');
543+
expect(getLegalIdentifier('test""Column')).toBe('"test""""Column"');
544+
expect(getLegalIdentifier('test$S$P$CColumn')).toBe('"test/.,Column"');
545+
expect(getLegalIdentifier('test/Column')).toBe('"test"."Column"');
546+
});
547+
548+
test('tableAlias', () => {
549+
expect(getLegalIdentifier('test', undefined)).toBe('"test"');
550+
expect(getLegalIdentifier('test', null)).toBe('"test"');
551+
expect(getLegalIdentifier('test', 'alias')).toBe('alias."test"');
552+
expect(getLegalIdentifier('test/column', 'alias')).toBe('alias."test"."column"');
553+
});
554+
555+
test('separator', () => {
556+
expect(getLegalIdentifier('test', undefined, '.')).toBe('"test"');
557+
expect(getLegalIdentifier('test/a/b/c', undefined, '.')).toBe('"test/a/b/c"');
558+
expect(getLegalIdentifier('test.a.b.c', undefined, '.')).toBe('"test"."a"."b"."c"');
559+
});
560+
});

packages/components/src/internal/query/filter.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,12 @@ export function isEqual(first: List<Filter.IFilter>, second: List<Filter.IFilter
6868
return isEqual;
6969
}
7070

71-
export function getLegalIdentifier(columnName: string, tableAlias?: string): string {
72-
const columnNameParts = columnName.split('/');
71+
/**
72+
* Make a legal LK SQL identifier from the column name by decoding parts, escaping quotes, and adding table alias if provided.
73+
* See usage in ui-premium entities/utils.tsx getLabKeySql()
74+
*/
75+
export function getLegalIdentifier(columnName: string, tableAlias?: string, separator = '/'): string {
76+
const columnNameParts = columnName.split(separator);
7377
const formattedParts = [];
7478
columnNameParts.forEach(part => {
7579
if (part) {

0 commit comments

Comments
 (0)