From a959ae8fbf51b5cc806d6098f45a733c23dcd767 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Tue, 29 Apr 2025 16:24:44 -0700 Subject: [PATCH 1/6] Issue 52773: display warning for unresolved form lookup values --- .../internal/components/forms/QuerySelect.tsx | 5 +++++ .../components/forms/input/SelectInput.tsx | 20 +++++++++++-------- .../src/internal/components/forms/model.ts | 11 ++++++++++ packages/components/src/theme/form.scss | 12 +++++++++++ packages/components/src/theme/samples.scss | 5 ----- 5 files changed, 40 insertions(+), 13 deletions(-) diff --git a/packages/components/src/internal/components/forms/QuerySelect.tsx b/packages/components/src/internal/components/forms/QuerySelect.tsx index 9097279336..1a5721a42c 100644 --- a/packages/components/src/internal/components/forms/QuerySelect.tsx +++ b/packages/components/src/internal/components/forms/QuerySelect.tsx @@ -443,8 +443,13 @@ export const QuerySelect: FC = memo(props => { onFocus={onFocus} optionRenderer={optionRenderer} options={undefined} // prevent override + // Issue 52773: Allow for submission of required fields whose value is not found + required={model.initialValueNotFound ? false : required} selectedOptions={model.isInit ? model.selectedOptions : undefined} value={getValue(model, multiple)} // needed to initialize the Formsy "value" properly + warning={ + model.initialValueNotFound ? `Could not find ${value}. Data may have been moved or deleted.` : undefined + } /> ); }); diff --git a/packages/components/src/internal/components/forms/input/SelectInput.tsx b/packages/components/src/internal/components/forms/input/SelectInput.tsx index ab951e94d3..f8f3a74bbd 100644 --- a/packages/components/src/internal/components/forms/input/SelectInput.tsx +++ b/packages/components/src/internal/components/forms/input/SelectInput.tsx @@ -18,6 +18,7 @@ import ReactSelect, { components } from 'react-select'; import AsyncSelect from 'react-select/async'; import AsyncCreatableSelect from 'react-select/async-creatable'; import CreatableSelect from 'react-select/creatable'; +import classNames from 'classnames'; import { Utils } from '@labkey/api'; import { FieldLabel } from '../FieldLabel'; @@ -237,6 +238,7 @@ export interface SelectInputProps { value?: any; valueKey?: string; valueRenderer?: any; + warning?: ReactNode; } type SelectInputImplProps = SelectInputProps & FormsyInjectedProps; @@ -713,20 +715,22 @@ export class SelectInputImpl extends Component { }; render() { - const { containerClass, errorMessage, formsy, help, inputClass } = this.props; + const { containerClass, errorMessage, formsy, help, inputClass, warning } = this.props; const hasError = formsy && !!errorMessage; + const hasWarning = !hasError && !!warning; + + const className = classNames('select-input-container', containerClass, { + 'has-error': hasError, + 'has-warning': hasWarning, + }); return ( -
+
{this.renderLabel()}
{this.renderSelect()} - {hasError && ( -
- {errorMessage} -
- )} - {!hasError && !!help && {help}} + {hasError && {errorMessage}} + {!hasError && (hasWarning || !!help) && {warning ?? help}}
); diff --git a/packages/components/src/internal/components/forms/model.ts b/packages/components/src/internal/components/forms/model.ts index ee84a6fea4..88c1009770 100644 --- a/packages/components/src/internal/components/forms/model.ts +++ b/packages/components/src/internal/components/forms/model.ts @@ -169,6 +169,8 @@ export function setSelection(model: QuerySelectModel, rawSelectedValue: any): Qu const selectedItems = getSelectedOptions(model, rawSelectedValue); return model.merge({ + // Issue 52773: Unset initialValueNotFound once a value has been selected + initialValueNotFound: false, rawSelectedValue, selectedItems, selectedQuery: parseSelectedQuery(model, selectedItems), @@ -330,6 +332,7 @@ export async function initSelect(props: QuerySelectOwnProps): Promise(); + let initialValueNotFound: boolean = false; if (props.value !== undefined && props.value !== null) { let filter: Filter.IFilter; @@ -370,11 +373,16 @@ export async function initSelect(props: QuerySelectOwnProps): Promise Date: Fri, 2 May 2025 16:26:37 -0700 Subject: [PATCH 2/6] Support multiple missing values --- .../components/editable/actions.test.ts | 25 --- .../internal/components/editable/actions.ts | 26 +-- .../internal/components/forms/QuerySelect.tsx | 25 ++- .../internal/components/forms/model.test.ts | 115 ++++++++++++- .../src/internal/components/forms/model.ts | 153 ++++++++++++------ .../src/internal/util/messaging.test.ts | 31 +++- .../src/internal/util/messaging.tsx | 18 +++ 7 files changed, 286 insertions(+), 107 deletions(-) diff --git a/packages/components/src/internal/components/editable/actions.test.ts b/packages/components/src/internal/components/editable/actions.test.ts index 3db3fd1718..85d16446e5 100644 --- a/packages/components/src/internal/components/editable/actions.test.ts +++ b/packages/components/src/internal/components/editable/actions.test.ts @@ -16,7 +16,6 @@ import { changeColumn, detectPadLength, loadEditorModelData, - lookupValidationError, parseIntIfNumber, parsePastedLookup, removeColumn, @@ -1319,27 +1318,3 @@ describe('loadEditorModelData', () => { expect(api.query.selectRows).toHaveBeenCalledTimes(2); }); }); - -describe('lookupValidationError', () => { - test('value only', () => { - expect(lookupValidationError('s').message).toEqual('Could not find s. Data may have been moved or deleted.'); - expect(lookupValidationError(1.4).message).toEqual('Could not find 1.4. Data may have been moved or deleted.'); - expect(lookupValidationError(false).message).toEqual( - 'Could not find false. Data may have been moved or deleted.' - ); - }); - - test('fromPaste', () => { - expect(lookupValidationError(false, true).message).toEqual('Could not find false'); - expect(lookupValidationError('beep', true).message).toEqual('Could not find beep'); - expect(lookupValidationError('"sara", "pete"', true).message).toEqual( - 'Could not find "sara", "pete". Please make sure values that contain commas are properly quoted.' - ); - }); - - test('with displayValue', () => { - expect(lookupValidationError('beep,', false, 'vw').message).toEqual( - 'vw is no longer a valid value. Data may have been moved or deleted.' - ); - }); -}); diff --git a/packages/components/src/internal/components/editable/actions.ts b/packages/components/src/internal/components/editable/actions.ts index b556b1958b..69780aa7a0 100644 --- a/packages/components/src/internal/components/editable/actions.ts +++ b/packages/components/src/internal/components/editable/actions.ts @@ -21,7 +21,7 @@ import { getContainerFilterForLookups } from '../../query/api'; import { ComponentsAPIWrapper, getDefaultAPIWrapper } from '../../APIWrapper'; -import { resolveErrorMessage } from '../../util/messaging'; +import { lookupValidationErrorMessage, resolveErrorMessage } from '../../util/messaging'; import { CellMessage, @@ -396,7 +396,7 @@ async function getLookupValueDescriptors( messageAndValues.push({ message: { isWarning: true, - message: errorMap[value] ?? lookupValidationError(value, false, displayValue).message, + message: errorMap[value] ?? lookupValidationErrorMessage(value, false, displayValue), }, valueDescriptor: { display: displayValue ?? `<${value}>`, raw: value }, }); @@ -410,24 +410,6 @@ async function getLookupValueDescriptors( return lookupValues; } -export function lookupValidationError( - value: string | number | boolean, - fromPaste?: boolean, - displayValue?: any -): CellMessage { - let message = displayValue !== undefined ? `${displayValue} is no longer a valid value` : `Could not find ${value}`; - - if (fromPaste) { - if (typeof value === 'string' && value.toString().indexOf(',') > -1) { - message += '. Please make sure values that contain commas are properly quoted.'; - } - } else { - message += '. Data may have been moved or deleted.'; - } - - return { message }; -} - async function getLookupDisplayValue(column: QueryColumn, value: any, containerPath: string): Promise { if (value === undefined || value === null) { return { @@ -442,7 +424,7 @@ async function getLookupDisplayValue(column: QueryColumn, value: any, containerP const descriptors = await findLookupValues({ column, containerPath, forUpdate: false, lookupKeyValues: [value] }); if (!descriptors.length) { - message = lookupValidationError(value); + message = { message: lookupValidationErrorMessage(value) }; } return { @@ -1100,7 +1082,7 @@ export function parsePastedLookup( .slice(0, 4) .map(u => '"' + u + '"') .join(', '); - message = lookupValidationError(valueStr, true); + message = { message: lookupValidationErrorMessage(valueStr, true) }; } return { diff --git a/packages/components/src/internal/components/forms/QuerySelect.tsx b/packages/components/src/internal/components/forms/QuerySelect.tsx index 1a5721a42c..d5e901ae1d 100644 --- a/packages/components/src/internal/components/forms/QuerySelect.tsx +++ b/packages/components/src/internal/components/forms/QuerySelect.tsx @@ -13,13 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import React, { ComponentType, FC, memo, useCallback, useEffect, useState } from 'react'; +import React, { ComponentType, FC, memo, useCallback, useEffect, useMemo, useState } from 'react'; import { List, Map } from 'immutable'; import { Filter, Query, Utils } from '@labkey/api'; import { SchemaQuery } from '../../../public/SchemaQuery'; -import { resolveErrorMessage } from '../../util/messaging'; +import { lookupValidationErrorMessage, resolveErrorMessage } from '../../util/messaging'; import { Row } from '../../query/selectRows'; @@ -263,6 +263,7 @@ export const QuerySelect: FC = memo(props => { const [searches, setSearches] = useState([]); const debounceTO = useTimeout(); const shouldLoadOnFocus = loadOnFocus && !loadOnFocusLock; + const hasNotFoundValues = model.notFoundValues?.size > 0; useEffect(() => { if (!autoInit) return; @@ -399,6 +400,20 @@ export const QuerySelect: FC = memo(props => { [OptionComponent, model] ); + // Issue 52773: If a value is specified, but we are unable to resolve the value then display a warning to the user. + const warning = useMemo(() => { + if (!hasNotFoundValues) return undefined; + + let warningValue: string; + if (model.notFoundValues.size < 5) { + warningValue = model.notFoundValues.join(', '); + } else { + warningValue = `${model.notFoundValues.size} values`; + } + + return lookupValidationErrorMessage(warningValue); + }, [hasNotFoundValues, model.notFoundValues]); + if (error) { return ( = memo(props => { optionRenderer={optionRenderer} options={undefined} // prevent override // Issue 52773: Allow for submission of required fields whose value is not found - required={model.initialValueNotFound ? false : required} + required={hasNotFoundValues ? false : required} selectedOptions={model.isInit ? model.selectedOptions : undefined} value={getValue(model, multiple)} // needed to initialize the Formsy "value" properly - warning={ - model.initialValueNotFound ? `Could not find ${value}. Data may have been moved or deleted.` : undefined - } + warning={warning} /> ); }); diff --git a/packages/components/src/internal/components/forms/model.test.ts b/packages/components/src/internal/components/forms/model.test.ts index a4f153b9cd..0ee6ea7291 100644 --- a/packages/components/src/internal/components/forms/model.test.ts +++ b/packages/components/src/internal/components/forms/model.test.ts @@ -1,10 +1,11 @@ import { fromJS } from 'immutable'; +import { Filter } from '@labkey/api'; import { QueryInfo } from '../../../public/QueryInfo'; import { ExtendedMap } from '../../../public/ExtendedMap'; import { QueryColumn } from '../../../public/QueryColumn'; -import { parseSelectedQuery, QuerySelectModel, queryColumnNames } from './model'; +import { buildValueFilter, findNotFoundValues, parseSelectedQuery, QuerySelectModel, queryColumnNames } from './model'; describe('form actions', () => { const setSelectionModel = new QuerySelectModel({ @@ -92,4 +93,116 @@ describe('form actions', () => { ).sort() ).toEqual(['colA', displayColumn, groupByColumn, 'pkCol1', 'pkCol2', valueColumn, someRequiredColumn]); }); + + describe('buildValueFilter', () => { + const col = 'id'; + const delimiter = ','; + + test('handles single non-array, non-string value with multiple = false', () => { + const result = buildValueFilter('abc', col, false, delimiter); + expect(result.expectedValueCount).toBe(1); + expect(result.filter.getValue()).toBe('abc'); + expect(result.filter.getColumnName()).toBe(col); + }); + + test('handles array input when multiple = true', () => { + const result = buildValueFilter(['a', 'b', 'b'], col, true, delimiter); + expect(result.expectedValueCount).toBe(2); // unique values + expect(result.filter.getValue()).toEqual(['a', 'b', 'b']); + expect(result.filter.getColumnName()).toBe(col); + }); + + test('handles delimited string input when multiple = true', () => { + const result = buildValueFilter('x,y,y,z', col, true, ','); + expect(result.expectedValueCount).toBe(3); + expect(result.filter.getValue()).toEqual(['x', 'y', 'y', 'z']); + }); + + test('handles string value when multiple = false', () => { + const result = buildValueFilter('foo', col, false, delimiter); + expect(result.expectedValueCount).toBe(1); + expect(result.filter.getValue()).toBe('foo'); + }); + + test('handles non-string scalar values correctly', () => { + const result = buildValueFilter(123, col, false, delimiter); + expect(result.expectedValueCount).toBe(1); + expect(result.filter.getValue()).toBe(123); + }); + + test('defaults to non-IN filter if multiple = true but value is not string or array', () => { + const result = buildValueFilter(true, col, true, delimiter); + expect(result.expectedValueCount).toBe(1); + expect(result.filter.getValue()).toBe(true); + }); + }); + + describe('findNotFoundValues', () => { + const EMPTY_ITEMS = fromJS({}); + + function filter(value: any): Filter.IFilter { + return Filter.create('col', value, Filter.Types.IN); + } + + test('returns empty array when filter value is undefined or null', () => { + expect(findNotFoundValues(EMPTY_ITEMS, filter(undefined), 'id')).toHaveLength(0); + expect(findNotFoundValues(EMPTY_ITEMS, filter(null), 'id')).toHaveLength(0); + expect(findNotFoundValues(EMPTY_ITEMS, filter([undefined, null]), 'id')).toHaveLength(0); + }); + + test('returns single value as array when filter has a single value', () => { + expect(findNotFoundValues(EMPTY_ITEMS, filter('abc'), 'id')).toEqual(['abc']); + expect(findNotFoundValues(EMPTY_ITEMS, filter(['abc']), 'id')).toEqual(['abc']); + }); + + test('returns all values when no items are selected', () => { + const filterValues = ['x', 'y', 'z']; + expect(findNotFoundValues(EMPTY_ITEMS, filter(filterValues), 'id')).toEqual(filterValues); + }); + + test('returns missing values when some items are matched', () => { + const selectedItems = fromJS({ one: { id: { value: 'x' } }, two: { id: { value: 'z' } } }); + expect(findNotFoundValues(selectedItems, filter(['x', 'y', 'z']), 'id')).toEqual(['y']); + }); + + test('returns empty array when all values are found in selected items', () => { + const selectedAll = fromJS({ + one: { id: { value: 'x' } }, + two: { id: { value: 'y' } }, + three: { id: { value: 'z' } }, + }); + expect(findNotFoundValues(selectedAll, filter(['x', 'y', 'z']), 'id')).toEqual([]); + }); + + test('handles mixed types and converts to string for comparison', () => { + const mixedItems = fromJS({ one: { id: { value: 1 } }, two: { id: { value: 3 } } }); + expect(findNotFoundValues(mixedItems, filter([1, 2, 3]), 'id')).toEqual(['2']); + }); + + test('ignores items missing the valueColumn', () => { + const incompleteItems = fromJS({ + one: { id: { value: 'x' } }, + two: { other: { value: 'y' } }, // missing 'id' + }); + expect(findNotFoundValues(incompleteItems, filter(['x', 'y']), 'id')).toEqual(['y']); + }); + + test('ignores null or undefined item values', () => { + const nullItemValues = fromJS({ + one: { id: { value: 'x' } }, + two: { id: { value: null } }, + three: { id: { value: undefined } }, + }); + expect(findNotFoundValues(nullItemValues, filter(['x', 'y']), 'id')).toEqual(['y']); + }); + + test('handles duplicate values in filter input', () => { + expect(findNotFoundValues(EMPTY_ITEMS, filter(['a', 'a', 'b']), 'id')).toEqual(['a', 'b']); + }); + + test('handles mixed string/number types across filters and item values', () => { + const mixedTypes = fromJS({ one: { id: { value: '1' } }, two: { id: { value: 2 } } }); + expect(findNotFoundValues(mixedTypes, filter([1, 2, 3]), 'id')).toEqual(['3']); + }); + }); }); diff --git a/packages/components/src/internal/components/forms/model.ts b/packages/components/src/internal/components/forms/model.ts index 88c1009770..9a366664a7 100644 --- a/packages/components/src/internal/components/forms/model.ts +++ b/packages/components/src/internal/components/forms/model.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import { fromJS, List, Map, OrderedMap, Record as ImmutableRecord } from 'immutable'; -import { Filter, Query, Utils } from '@labkey/api'; +import { Filter, Query } from '@labkey/api'; import { QueryInfo } from '../../../public/QueryInfo'; @@ -30,6 +30,8 @@ import { import { similaritySortFactory } from '../../util/similaritySortFactory'; import { parseCsvString } from '../../util/utils'; +import { naturalSort } from '../../../public/sort'; + import { SelectInputOption } from './input/SelectInput'; import { DELIMITER } from './constants'; import { QuerySelectOwnProps } from './QuerySelect'; @@ -169,8 +171,8 @@ export function setSelection(model: QuerySelectModel, rawSelectedValue: any): Qu const selectedItems = getSelectedOptions(model, rawSelectedValue); return model.merge({ - // Issue 52773: Unset initialValueNotFound once a value has been selected - initialValueNotFound: false, + // Issue 52773: Unset notFoundValues once a value has been selected + notFoundValues: undefined, rawSelectedValue, selectedItems, selectedQuery: parseSelectedQuery(model, selectedItems), @@ -324,66 +326,113 @@ export function queryColumnNames( return Array.from(new Set(queryColumns)); } -export async function initSelect(props: QuerySelectOwnProps): Promise> { - const { containerFilter, containerPath, schemaQuery, queryFilters } = props; - +async function initQueryInfoWithColumns(props: QuerySelectOwnProps): Promise> { + const { containerPath, schemaQuery } = props; const queryInfo = await getQueryDetails({ containerPath, schemaQuery }); + const valueColumn = initValueColumn(queryInfo, props.valueColumn); const displayColumn = initDisplayColumn(queryInfo, valueColumn, props.displayColumn); const groupByColumn = initGroupByColumn(queryInfo, props.groupByColumn); - let selectedItems = Map(); - let initialValueNotFound: boolean = false; - - if (props.value !== undefined && props.value !== null) { - let filter: Filter.IFilter; - - if (props.multiple) { - if (Array.isArray(props.value)) { - filter = Filter.create(valueColumn, props.value, Filter.Types.IN); - } else if (typeof props.value === 'string') { - // Allow for setting multiValue value. - // This requires updating the filter and the string - filter = Filter.create( - valueColumn, - parseCsvString(props.value, props.delimiter, true), - Filter.Types.IN - ); - } - } - if (!filter) { - filter = Filter.create(valueColumn, props.value); + return { queryInfo, valueColumn, displayColumn, groupByColumn }; +} + +export function buildValueFilter( + value: any, + valueColumn: string, + multiple: boolean, + delimiter: string +): { expectedValueCount: number; filter: Filter.IFilter } { + let filter: Filter.IFilter; + let expectedValueCount = 1; + + if (multiple) { + if (Array.isArray(value)) { + filter = Filter.create(valueColumn, value, Filter.Types.IN); + expectedValueCount = new Set(value).size; + } else if (typeof value === 'string') { + const parsed = parseCsvString(value, delimiter, true); + filter = Filter.create(valueColumn, parsed, Filter.Types.IN); + expectedValueCount = new Set(parsed).size; } + } - const filters = queryFilters ? queryFilters.toArray() : []; - filters.push(filter); - - const { queryName, schemaName, viewName } = schemaQuery; - const data = await selectRowsDeprecated({ - columns: queryColumnNames(queryInfo, displayColumn, valueColumn, props.requiredColumns, groupByColumn), - containerFilter, - containerPath, - filterArray: filters, - parameters: props.queryParams, - queryName, - schemaName, - viewName, - }); - - selectedItems = fromJS( - quoteValueColumnWithDelimiters(data, props.valueColumn, props.delimiter).models[data.key] - ); + if (!filter) { + filter = Filter.create(valueColumn, value); + } + + return { expectedValueCount, filter }; +} + +export function findNotFoundValues( + selectedItems: Map, + filter: Filter.IFilter, + valueColumn: string +): string[] { + const filterValue = filter.getValue(); + if (filterValue === undefined || filterValue === null) return []; + + const rawValues = Array.isArray(filterValue) ? filterValue : [filterValue]; + const uniqueValues = new Set(rawValues.filter(v => v !== undefined && v !== null).map(v => v.toString())); - // Issue 52773: If a value is specified, but we are unable to resolve the value then display - // a warning to the user. - initialValueNotFound = selectedItems.size === 0; + selectedItems + .map(item => item.getIn([valueColumn, 'value'])) + .filter(v => v !== undefined && v !== null) + .forEach(itemValue => uniqueValues.delete(itemValue.toString())); + + return Array.from(uniqueValues).sort(naturalSort); +} + +async function initSelectedItems( + props: QuerySelectOwnProps, + queryInfo: QueryInfo, + valueColumn: string, + displayColumn: string, + groupByColumn: string, + filter: Filter.IFilter +): Promise> { + const filters = props.queryFilters ? props.queryFilters.toArray() : []; + filters.push(filter); + + const { queryName, schemaName, viewName } = props.schemaQuery; + const data = await selectRowsDeprecated({ + columns: queryColumnNames(queryInfo, displayColumn, valueColumn, props.requiredColumns, groupByColumn), + containerFilter: props.containerFilter, + containerPath: props.containerPath, + filterArray: filters, + parameters: props.queryParams, + queryName, + schemaName, + viewName, + }); + + return fromJS(quoteValueColumnWithDelimiters(data, props.valueColumn, props.delimiter).models[data.key]); +} + +export async function initSelect(props: QuerySelectOwnProps): Promise> { + const { delimiter, multiple, value } = props; + const { queryInfo, valueColumn, displayColumn, groupByColumn } = await initQueryInfoWithColumns(props); + + let selectedItems = Map(); + let notFoundValues: List; + + if (value !== undefined && value !== null) { + const { expectedValueCount, filter } = buildValueFilter(value, valueColumn, multiple, delimiter); + selectedItems = await initSelectedItems(props, queryInfo, valueColumn, displayColumn, groupByColumn, filter); + + if (selectedItems.size !== expectedValueCount) { + const notFoundValuesArray = findNotFoundValues(selectedItems, filter, valueColumn); + if (notFoundValuesArray) { + notFoundValues = List(notFoundValuesArray); + } + } } return { displayColumn, groupByColumn, - initialValueNotFound, isInit: true, + notFoundValues, queryInfo, selectedItems, valueColumn, @@ -398,10 +447,10 @@ export interface QuerySelectModelProps { delimiter: string; displayColumn: string; groupByColumn: string; - initialValueNotFound: boolean; isInit: boolean; maxRows: number; multiple: boolean; + notFoundValues: List; queryFilters: List; queryInfo: QueryInfo; queryParams: Record; @@ -423,10 +472,10 @@ export class QuerySelectModel displayColumn: undefined, delimiter: DELIMITER, groupByColumn: undefined, - initialValueNotFound: false, isInit: false, maxRows: 20, multiple: false, + notFoundValues: undefined, queryFilters: undefined, queryInfo: undefined, queryParams: undefined, @@ -447,10 +496,10 @@ export class QuerySelectModel declare displayColumn: string; declare delimiter: string; declare groupByColumn: string; - declare initialValueNotFound: boolean; declare isInit: boolean; declare maxRows: number; declare multiple: boolean; + declare notFoundValues: List; declare queryFilters: List; declare queryInfo: QueryInfo; declare queryParams: Record; diff --git a/packages/components/src/internal/util/messaging.test.ts b/packages/components/src/internal/util/messaging.test.ts index f3e6825526..051b4546c4 100644 --- a/packages/components/src/internal/util/messaging.test.ts +++ b/packages/components/src/internal/util/messaging.test.ts @@ -1,4 +1,9 @@ -import { getPermissionRestrictionMessage, makePresentParticiple, resolveErrorMessage } from './messaging'; +import { + getPermissionRestrictionMessage, + lookupValidationErrorMessage, + makePresentParticiple, + resolveErrorMessage, +} from './messaging'; describe('makePresentParticiple', () => { test('no verb', () => { @@ -370,3 +375,27 @@ describe('getPermissionRestrictionMessage', () => { ); }); }); + +describe('lookupValidationErrorMessage', () => { + test('value only', () => { + expect(lookupValidationErrorMessage('s')).toEqual('Could not find s. Data may have been moved or deleted.'); + expect(lookupValidationErrorMessage(1.4)).toEqual('Could not find 1.4. Data may have been moved or deleted.'); + expect(lookupValidationErrorMessage(false)).toEqual( + 'Could not find false. Data may have been moved or deleted.' + ); + }); + + test('fromPaste', () => { + expect(lookupValidationErrorMessage(false, true)).toEqual('Could not find false'); + expect(lookupValidationErrorMessage('beep', true)).toEqual('Could not find beep'); + expect(lookupValidationErrorMessage('"sara", "pete"', true)).toEqual( + 'Could not find "sara", "pete". Please make sure values that contain commas are properly quoted.' + ); + }); + + test('with displayValue', () => { + expect(lookupValidationErrorMessage('beep,', false, 'vw')).toEqual( + 'vw is no longer a valid value. Data may have been moved or deleted.' + ); + }); +}); diff --git a/packages/components/src/internal/util/messaging.tsx b/packages/components/src/internal/util/messaging.tsx index 2d5e3184f7..5bf5d250a9 100644 --- a/packages/components/src/internal/util/messaging.tsx +++ b/packages/components/src/internal/util/messaging.tsx @@ -232,3 +232,21 @@ export function getPermissionRestrictionMessage( const notPermittedNoun = Utils.pluralize(noPermissionCount, nounSingular, nounPlural); return `Selection includes ${notPermittedNoun} that you do not have permission to ${verb}${verbSuffix ?? ''}. Only the ${nounPlural} that you have permission for will be updated.`; } + +export function lookupValidationErrorMessage( + value: string | number | boolean, + fromPaste?: boolean, + displayValue?: any +): string { + let message = displayValue !== undefined ? `${displayValue} is no longer a valid value` : `Could not find ${value}`; + + if (fromPaste) { + if (typeof value === 'string' && value.toString().indexOf(',') > -1) { + message += '. Please make sure values that contain commas are properly quoted.'; + } + } else { + message += '. Data may have been moved or deleted.'; + } + + return message; +} From 446676a606b862d3fdc711529c7414b1305195ab Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Mon, 5 May 2025 08:53:25 -0700 Subject: [PATCH 3/6] initErrorMsg --- .../src/internal/components/forms/model.ts | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/packages/components/src/internal/components/forms/model.ts b/packages/components/src/internal/components/forms/model.ts index 9a366664a7..358b126dd0 100644 --- a/packages/components/src/internal/components/forms/model.ts +++ b/packages/components/src/internal/components/forms/model.ts @@ -182,7 +182,7 @@ export function setSelection(model: QuerySelectModel, rawSelectedValue: any): Qu export function fetchSearchResults(model: QuerySelectModel, input: any): Promise { const { addExactFilter, displayColumn, maxRows, queryFilters, schemaQuery, selectedItems, valueColumn } = model; - let allFilters = []; + let allFilters: Filter.IFilter[] = []; const filterVal = input.trim(); // fetch additional options and exclude previously selected so user can see more @@ -224,37 +224,39 @@ export function fetchSearchResults(model: QuerySelectModel, input: any): Promise ); } +function initErrorMsg(queryInfo: QueryInfo, suffix?: string): string { + const msg = `Unable to initialize QuerySelect for (${queryInfo.schemaName}.${queryInfo.name}).`; + if (suffix) return msg + ' ' + suffix; + return msg; +} + function initValueColumn(queryInfo: QueryInfo, column?: string): string { - // determine 'valueColumn' - let valueColumn: string; if (column) { - valueColumn = column; - const valueCol = queryInfo.getColumn(valueColumn) ?? queryInfo.getColumnFromName(valueColumn); + const valueCol = queryInfo.getColumn(column) ?? queryInfo.getColumnFromName(column); if (!valueCol) { - throw new Error( - `Unable to initialize QuerySelect for (${queryInfo.schemaName}.${queryInfo.name}). The "valueColumn" "${valueColumn}" does not exist.` - ); - } - valueColumn = valueCol.fieldKey; - } else { - const pkCols = queryInfo.getPkCols(); - - if (pkCols.length === 1) { - valueColumn = pkCols[0].fieldKey; - } else if (pkCols.length > 0) { - throw new Error( - `Unable to initialize QuerySelect for (${queryInfo.schemaName}.${queryInfo.name}). Set "valueColumn" explicitly to any of ` + - pkCols.map(col => col.fieldKey).join(', ') - ); - } else { - throw new Error( - `Unable to initialize QuerySelect for (${queryInfo.schemaName}.${queryInfo.name}). Set "valueColumn" explicitly as this query does not have any primary keys.` - ); + throw new Error(initErrorMsg(queryInfo, `The specified "valueColumn" "${column}" does not exist.`)); } + + return valueCol.fieldKey; + } + + const pkCols = queryInfo.getPkCols(); + + if (pkCols.length === 0) { + throw new Error( + initErrorMsg(queryInfo, 'Set "valueColumn" explicitly as this query does not have any primary keys.') + ); + } + + if (pkCols.length > 1) { + const availablePkKeys = pkCols.map(col => `"${col.fieldKey}"`).join(', '); + throw new Error( + initErrorMsg(queryInfo, `Set "valueColumn" explicitly to any of the primary keys: ${availablePkKeys}`) + ); } - return valueColumn; + return pkCols[0].fieldKey; } function initDisplayColumn(queryInfo: QueryInfo, valueColumn: string, column?: string): string { @@ -263,9 +265,7 @@ function initDisplayColumn(queryInfo: QueryInfo, valueColumn: string, column?: s if (column) { const col = queryInfo.getColumn(column) ?? queryInfo.getColumnFromName(column); if (!col) { - console.warn( - `Unable to initialize QuerySelect for (${queryInfo.schemaName}.${queryInfo.name}). The display column "${column}" does not exist.` - ); + console.warn(initErrorMsg(queryInfo, `The display column "${column}" does not exist.`)); } else { displayColumn = col.fieldKey; } @@ -293,9 +293,7 @@ function initGroupByColumn(queryInfo: QueryInfo, column?: string): string { if (column) { if (!queryInfo.getColumn(column)) { - console.warn( - `Unable to initialize QuerySelect for (${queryInfo.schemaName}.${queryInfo.name}). The group by column "${column}" does not exist.` - ); + console.warn(initErrorMsg(queryInfo, `The group by column "${column}" does not exist.`)); } else { groupByColumn = column; } @@ -373,14 +371,14 @@ export function findNotFoundValues( if (filterValue === undefined || filterValue === null) return []; const rawValues = Array.isArray(filterValue) ? filterValue : [filterValue]; - const uniqueValues = new Set(rawValues.filter(v => v !== undefined && v !== null).map(v => v.toString())); + const expectedValues = new Set(rawValues.filter(v => v !== undefined && v !== null).map(v => v.toString())); selectedItems .map(item => item.getIn([valueColumn, 'value'])) - .filter(v => v !== undefined && v !== null) - .forEach(itemValue => uniqueValues.delete(itemValue.toString())); + .filter(value => value !== undefined && value !== null) + .forEach(value => expectedValues.delete(value.toString())); - return Array.from(uniqueValues).sort(naturalSort); + return Array.from(expectedValues).sort(naturalSort); } async function initSelectedItems( From 79431ade8c77c30ffa6e4218470890f705c6aa52 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Mon, 5 May 2025 15:53:54 -0700 Subject: [PATCH 4/6] Revise to avoid quoted comparison --- .../internal/components/forms/model.test.ts | 20 ++++++------- .../src/internal/components/forms/model.ts | 30 +++++++++++-------- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/packages/components/src/internal/components/forms/model.test.ts b/packages/components/src/internal/components/forms/model.test.ts index 0ee6ea7291..0df2baacc6 100644 --- a/packages/components/src/internal/components/forms/model.test.ts +++ b/packages/components/src/internal/components/forms/model.test.ts @@ -138,7 +138,7 @@ describe('form actions', () => { }); describe('findNotFoundValues', () => { - const EMPTY_ITEMS = fromJS({}); + const EMPTY_ITEMS = {}; function filter(value: any): Filter.IFilter { return Filter.create('col', value, Filter.Types.IN); @@ -161,38 +161,38 @@ describe('form actions', () => { }); test('returns missing values when some items are matched', () => { - const selectedItems = fromJS({ one: { id: { value: 'x' } }, two: { id: { value: 'z' } } }); + const selectedItems = { one: { id: { value: 'x' } }, two: { id: { value: 'z' } } }; expect(findNotFoundValues(selectedItems, filter(['x', 'y', 'z']), 'id')).toEqual(['y']); }); test('returns empty array when all values are found in selected items', () => { - const selectedAll = fromJS({ + const selectedAll = { one: { id: { value: 'x' } }, two: { id: { value: 'y' } }, three: { id: { value: 'z' } }, - }); + }; expect(findNotFoundValues(selectedAll, filter(['x', 'y', 'z']), 'id')).toEqual([]); }); test('handles mixed types and converts to string for comparison', () => { - const mixedItems = fromJS({ one: { id: { value: 1 } }, two: { id: { value: 3 } } }); + const mixedItems = { one: { id: { value: 1 } }, two: { id: { value: 3 } } }; expect(findNotFoundValues(mixedItems, filter([1, 2, 3]), 'id')).toEqual(['2']); }); test('ignores items missing the valueColumn', () => { - const incompleteItems = fromJS({ + const incompleteItems = { one: { id: { value: 'x' } }, two: { other: { value: 'y' } }, // missing 'id' - }); + }; expect(findNotFoundValues(incompleteItems, filter(['x', 'y']), 'id')).toEqual(['y']); }); test('ignores null or undefined item values', () => { - const nullItemValues = fromJS({ + const nullItemValues = { one: { id: { value: 'x' } }, two: { id: { value: null } }, three: { id: { value: undefined } }, - }); + }; expect(findNotFoundValues(nullItemValues, filter(['x', 'y']), 'id')).toEqual(['y']); }); @@ -201,7 +201,7 @@ describe('form actions', () => { }); test('handles mixed string/number types across filters and item values', () => { - const mixedTypes = fromJS({ one: { id: { value: '1' } }, two: { id: { value: 2 } } }); + const mixedTypes = { one: { id: { value: '1' } }, two: { id: { value: 2 } } }; expect(findNotFoundValues(mixedTypes, filter([1, 2, 3]), 'id')).toEqual(['3']); }); }); diff --git a/packages/components/src/internal/components/forms/model.ts b/packages/components/src/internal/components/forms/model.ts index 358b126dd0..47b79ea933 100644 --- a/packages/components/src/internal/components/forms/model.ts +++ b/packages/components/src/internal/components/forms/model.ts @@ -28,10 +28,12 @@ import { selectRowsDeprecated, } from '../../query/api'; import { similaritySortFactory } from '../../util/similaritySortFactory'; -import { parseCsvString } from '../../util/utils'; +import { caseInsensitive, parseCsvString } from '../../util/utils'; import { naturalSort } from '../../../public/sort'; +import { Row } from '../../query/selectRows'; + import { SelectInputOption } from './input/SelectInput'; import { DELIMITER } from './constants'; import { QuerySelectOwnProps } from './QuerySelect'; @@ -363,7 +365,7 @@ export function buildValueFilter( } export function findNotFoundValues( - selectedItems: Map, + selectedRows: Record, filter: Filter.IFilter, valueColumn: string ): string[] { @@ -373,27 +375,28 @@ export function findNotFoundValues( const rawValues = Array.isArray(filterValue) ? filterValue : [filterValue]; const expectedValues = new Set(rawValues.filter(v => v !== undefined && v !== null).map(v => v.toString())); - selectedItems - .map(item => item.getIn([valueColumn, 'value'])) + Object.values(selectedRows) + .map(item => caseInsensitive(item, valueColumn)?.value) .filter(value => value !== undefined && value !== null) .forEach(value => expectedValues.delete(value.toString())); return Array.from(expectedValues).sort(naturalSort); } -async function initSelectedItems( +function initSelectedItems( props: QuerySelectOwnProps, queryInfo: QueryInfo, valueColumn: string, displayColumn: string, groupByColumn: string, filter: Filter.IFilter -): Promise> { +): Promise { const filters = props.queryFilters ? props.queryFilters.toArray() : []; filters.push(filter); const { queryName, schemaName, viewName } = props.schemaQuery; - const data = await selectRowsDeprecated({ + + return selectRowsDeprecated({ columns: queryColumnNames(queryInfo, displayColumn, valueColumn, props.requiredColumns, groupByColumn), containerFilter: props.containerFilter, containerPath: props.containerPath, @@ -403,23 +406,22 @@ async function initSelectedItems( schemaName, viewName, }); - - return fromJS(quoteValueColumnWithDelimiters(data, props.valueColumn, props.delimiter).models[data.key]); } export async function initSelect(props: QuerySelectOwnProps): Promise> { const { delimiter, multiple, value } = props; const { queryInfo, valueColumn, displayColumn, groupByColumn } = await initQueryInfoWithColumns(props); - let selectedItems = Map(); + let selectedItems: ISelectRowsResult; let notFoundValues: List; if (value !== undefined && value !== null) { const { expectedValueCount, filter } = buildValueFilter(value, valueColumn, multiple, delimiter); selectedItems = await initSelectedItems(props, queryInfo, valueColumn, displayColumn, groupByColumn, filter); + const selectedRows = selectedItems.models[selectedItems.key]; - if (selectedItems.size !== expectedValueCount) { - const notFoundValuesArray = findNotFoundValues(selectedItems, filter, valueColumn); + if (Object.keys(selectedRows).length !== expectedValueCount) { + const notFoundValuesArray = findNotFoundValues(selectedRows, filter, valueColumn); if (notFoundValuesArray) { notFoundValues = List(notFoundValuesArray); } @@ -432,7 +434,9 @@ export async function initSelect(props: QuerySelectOwnProps): Promise(), valueColumn, }; } From 0f66a6867bf0e684934e143f64b5dc8bea9fd008 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Tue, 6 May 2025 11:57:29 -0700 Subject: [PATCH 5/6] Prepare release notes --- packages/components/releaseNotes/components.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/components/releaseNotes/components.md b/packages/components/releaseNotes/components.md index 430da20f5b..ec9d554d75 100644 --- a/packages/components/releaseNotes/components.md +++ b/packages/components/releaseNotes/components.md @@ -1,6 +1,10 @@ # @labkey/components Components, models, actions, and utility functions for LabKey applications and pages +### version 6.40.2 +*Released*: 6 May 2025 +- Issue 52773: display warning for unresolved form lookup values + ### version 6.40.1 *Released*: 6 May 2025 - Issue 52556: Add data-fieldkey attribute to grid header elements and input elements From 3ea8c3fc52d7f438932013538d04f7e2803b922f Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Tue, 6 May 2025 12:00:41 -0700 Subject: [PATCH 6/6] 6.40.2 --- 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 36148cfc6b..475110c69a 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "6.40.1", + "version": "6.40.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "6.40.1", + "version": "6.40.2", "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 e9217f2b24..24f0dceb7a 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "6.40.1", + "version": "6.40.2", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [