diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index 88d5fa2172..247bed0d4f 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,16 +1,16 @@ { "name": "@labkey/components", - "version": "7.13.1", + "version": "7.14.0-fb-mvtc-convert.9", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.13.1", + "version": "7.14.0-fb-mvtc-convert.9", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", - "@labkey/api": "1.45.0", + "@labkey/api": "1.45.1-fb-mvtc-convert.1", "@testing-library/dom": "~10.4.1", "@testing-library/jest-dom": "~6.9.1", "@testing-library/react": "~16.3.0", @@ -3535,9 +3535,9 @@ } }, "node_modules/@labkey/api": { - "version": "1.45.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.45.0.tgz", - "integrity": "sha512-7KN2SvmcY46OtRBtlsUxlmGaE5LN/cg6OfPyc837pSGl+cIndPxOJMqFCvxO26h7c7Fd7cAK1/oOuAzAbvKHUw==", + "version": "1.45.1-fb-mvtc-convert.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.45.1-fb-mvtc-convert.1.tgz", + "integrity": "sha512-IlQwnZzi9whzKTBdAur3La0wJmIpFhMHpoQDIdKuo2NByygI+920EBGBiqjrSpTZYQbM0TnJjAZvIk0+5TtsWg==", "license": "Apache-2.0" }, "node_modules/@labkey/build": { diff --git a/packages/components/package.json b/packages/components/package.json index ac1be00930..6dd61c4cff 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.13.1", + "version": "7.14.0-fb-mvtc-convert.9", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [ @@ -50,7 +50,7 @@ "homepage": "https://github.com/LabKey/labkey-ui-components#readme", "dependencies": { "@hello-pangea/dnd": "18.0.1", - "@labkey/api": "1.45.0", + "@labkey/api": "1.45.1-fb-mvtc-convert.1", "@testing-library/dom": "~10.4.1", "@testing-library/jest-dom": "~6.9.1", "@testing-library/react": "~16.3.0", diff --git a/packages/components/releaseNotes/components.md b/packages/components/releaseNotes/components.md index f91fa3da4e..3a1ba5a4be 100644 --- a/packages/components/releaseNotes/components.md +++ b/packages/components/releaseNotes/components.md @@ -1,6 +1,13 @@ # @labkey/components Components, models, actions, and utility functions for LabKey applications and pages +### version 7.X +*Released*: X January 2026 +- Multi value text choices: field type conversion + - Updates the Text Choice options UI to add an “Allow multiple selections” toggle, multi-choice-specific edit restrictions, and improved confirmation messaging/tests for data-type changes involving text/multi-choice. + - Make multi-choice behaves as an internal variant of Text Choice rather than a separate visible type in data type dropdown + - Modified updateDataType and text choice usage counting to correctly handle conversions between string, Text Choice, and Multi Choice fields, clearing validators/flags and tracking multi-value usage where appropriate. + ### version 7.13.1 *Released*: 26 January 2026 - Merge from release26.1-SNAPSHOT to develop diff --git a/packages/components/src/internal/components/domainproperties/ConfirmDataTypeChangeModal.test.tsx b/packages/components/src/internal/components/domainproperties/ConfirmDataTypeChangeModal.test.tsx index f649a982e5..75f0e7520d 100644 --- a/packages/components/src/internal/components/domainproperties/ConfirmDataTypeChangeModal.test.tsx +++ b/packages/components/src/internal/components/domainproperties/ConfirmDataTypeChangeModal.test.tsx @@ -2,19 +2,75 @@ import React from 'react'; import { render } from '@testing-library/react'; import { ConfirmDataTypeChangeModal, getDataTypeConfirmDisplayText } from './ConfirmDataTypeChangeModal'; -import { DATE_TYPE, DATETIME_TYPE, PROP_DESC_TYPES, TIME_TYPE } from './PropDescType'; +import { + BOOLEAN_TYPE, + DATE_TYPE, + DATETIME_TYPE, + FILE_TYPE, + INTEGER_TYPE, + MULTI_CHOICE_TYPE, + MULTILINE_TYPE, + PROP_DESC_TYPES, + TEXT_CHOICE_TYPE, + TEXT_TYPE, + TIME_TYPE, +} from './PropDescType'; import { BOOLEAN_RANGE_URI, DATETIME_RANGE_URI, FILELINK_RANGE_URI, - INT_RANGE_URI, + MULTI_CHOICE_RANGE_URI, MULTILINE_RANGE_URI, + TEXT_CHOICE_CONCEPT_URI, TIME_RANGE_URI, } from './constants'; describe('ConfirmDataTypeChangeModal', () => { + const intType = { + rangeURI: 'http://www.w3.org/2001/XMLSchema#int', + dataType: INTEGER_TYPE, + }; + + const multiLineType = { + rangeURI: MULTILINE_RANGE_URI, + dataType: MULTILINE_TYPE, + }; + + const fileLinkType = { + rangeURI: FILELINK_RANGE_URI, + dataType: FILE_TYPE, + }; + + const booleanType = { + rangeURI: BOOLEAN_RANGE_URI, + dataType: BOOLEAN_TYPE, + }; + + const dateTimeType = { + rangeURI: DATETIME_RANGE_URI, + dataType: DATETIME_TYPE, + }; + + const timeType = { + rangeURI: TIME_RANGE_URI, + dataType: TIME_TYPE, + }; + + const multiChoiceType = { + rangeURI: MULTI_CHOICE_RANGE_URI, + dataType: MULTI_CHOICE_TYPE, + }; + + const textChoiceType = { + conceptURI: TEXT_CHOICE_CONCEPT_URI, + dataType: TEXT_CHOICE_TYPE, + }; + const DEFAULT_PROPS = { - originalRangeURI: 'http://www.w3.org/2001/XMLSchema#boolean', + original: { + rangeURI: 'http://www.w3.org/2001/XMLSchema#boolean', + dataType: BOOLEAN_TYPE, + }, newDataType: PROP_DESC_TYPES.get(0), onConfirm: jest.fn, onCancel: jest.fn, @@ -29,49 +85,59 @@ describe('ConfirmDataTypeChangeModal', () => { }); test('getDataTypeConfirmDisplayText', () => { - expect(getDataTypeConfirmDisplayText(INT_RANGE_URI)).toBe('integer'); - expect(getDataTypeConfirmDisplayText(MULTILINE_RANGE_URI)).toBe('string'); - expect(getDataTypeConfirmDisplayText(FILELINK_RANGE_URI)).toBe('file'); - expect(getDataTypeConfirmDisplayText(BOOLEAN_RANGE_URI)).toBe('boolean'); - expect(getDataTypeConfirmDisplayText(DATETIME_RANGE_URI)).toBe('dateTime'); + expect(getDataTypeConfirmDisplayText(intType.dataType)).toBe('integer'); + expect(getDataTypeConfirmDisplayText(multiLineType.dataType)).toBe('string'); + expect(getDataTypeConfirmDisplayText(fileLinkType.dataType)).toBe('file'); + expect(getDataTypeConfirmDisplayText(booleanType.dataType)).toBe('boolean'); + expect(getDataTypeConfirmDisplayText(dateTimeType.dataType)).toBe('dateTime'); + expect(getDataTypeConfirmDisplayText(multiChoiceType.dataType)).toBe('Text Choice (multiple select)'); + expect(getDataTypeConfirmDisplayText(textChoiceType.dataType)).toBe('Text Choice (single select)'); }); test('from datetime to time', () => { - render( - - ); + render(); expect(document.body).toHaveTextContent( 'This change will convert the values in the field from dateTime to time. This will cause the Date portion of the value to be removed. Once you save your changes, you will not be able to change it back to dateTime.' ); }); test('from datetime to date', () => { + render(); + expect(document.body).toHaveTextContent( + 'This change will convert the values in the field from dateTime to date. This will cause the Time portion of the value to be removed.' + ); + }); + + test('from date to datetime', () => { + render(); + expect(document.body).toHaveTextContent( + 'This change will convert the values in the field from time to dateTime. Once you save your changes, you will not be able to change it back to time.' + ); + }); + + test('from text choice to mvtc', () => { render( ); expect(document.body).toHaveTextContent( - 'This change will convert the values in the field from dateTime to date. This will cause the Time portion of the value to be removed.' + 'Confirm Data Type ChangeThis change will convert the values in the field from Text Choice (single select) to Text Choice (multiple select). Filters in saved views might not function as expected and any conditional formatting configured for this field will be removed.' ); }); - test('from date to datetime', () => { + test('from mvtc to tc', () => { render( ); expect(document.body).toHaveTextContent( - 'This change will convert the values in the field from time to dateTime. Once you save your changes, you will not be able to change it back to time.' + 'This change will convert the values in the field from Text Choice (multiple select) to Text Choice (single select). Filters in saved views might not function as expected' ); }); }); diff --git a/packages/components/src/internal/components/domainproperties/ConfirmDataTypeChangeModal.tsx b/packages/components/src/internal/components/domainproperties/ConfirmDataTypeChangeModal.tsx index 9357af84c1..dd5386258d 100644 --- a/packages/components/src/internal/components/domainproperties/ConfirmDataTypeChangeModal.tsx +++ b/packages/components/src/internal/components/domainproperties/ConfirmDataTypeChangeModal.tsx @@ -11,25 +11,40 @@ import { MULTILINE_RANGE_URI, TIME_RANGE_URI, } from './constants'; +import { IDomainField } from './models'; interface Props { - originalRangeURI: string; newDataType: PropDescType; - onConfirm: () => void; onCancel: () => void; + onConfirm: () => void; + original: Partial; } export const ConfirmDataTypeChangeModal: FC = memo(props => { - const { originalRangeURI, newDataType, onConfirm, onCancel } = props; - const origTypeLabel = getDataTypeConfirmDisplayText(originalRangeURI); - const newTypeLabel = getDataTypeConfirmDisplayText(newDataType.rangeURI); + const { original, newDataType, onConfirm, onCancel } = props; + const originalRangeURI = original.rangeURI || ''; + const origTypeLabel = getDataTypeConfirmDisplayText(original.dataType); + const newTypeLabel = getDataTypeConfirmDisplayText(newDataType); + const newMultiChoice = PropDescType.isMultiChoice(newDataType.rangeURI); + const oldMultiChoice = PropDescType.isMultiChoice(original.dataType.rangeURI); + const newTextChoice = PropDescType.isTextChoice(newDataType.conceptURI); const reversible = (PropDescType.isDate(originalRangeURI) && PropDescType.isDateTime(newDataType.rangeURI)) || - (PropDescType.isDateTime(originalRangeURI) && PropDescType.isDate(newDataType.rangeURI)); + (PropDescType.isDateTime(originalRangeURI) && PropDescType.isDate(newDataType.rangeURI)) || + newMultiChoice; let dataLossWarning = null; - if ( + if (newMultiChoice) { + dataLossWarning = ( + <> + Filters in saved views might not function as expected and any conditional formatting configured for this + field will be removed.{' '} + + ); + } else if (oldMultiChoice && newTextChoice) { + dataLossWarning = <>Filters in saved views might not function as expected. ; + } else if ( originalRangeURI === DATETIME_RANGE_URI && (newDataType.rangeURI === DATE_RANGE_URI || newDataType.rangeURI === TIME_RANGE_URI) ) { @@ -43,11 +58,11 @@ export const ConfirmDataTypeChangeModal: FC = memo(props => { return (
This change will convert the values in the field from{' '} @@ -67,7 +82,12 @@ export const ConfirmDataTypeChangeModal: FC = memo(props => { ConfirmDataTypeChangeModal.displayName = 'ConfirmDataTypeChangeModal'; // exported for jest testing -export const getDataTypeConfirmDisplayText = (rangeURI: string): string => { + +export const getDataTypeConfirmDisplayText = (dataType: PropDescType): string => { + if (dataType?.longDisplay) { + return dataType.longDisplay; + } + const rangeURI = dataType?.rangeURI || ''; if (rangeURI === INT_RANGE_URI) return 'integer'; if (rangeURI === MULTILINE_RANGE_URI) return 'string'; if (rangeURI === FILELINK_RANGE_URI) return 'file'; diff --git a/packages/components/src/internal/components/domainproperties/DomainForm.tsx b/packages/components/src/internal/components/domainproperties/DomainForm.tsx index 182c5756c7..7ee58ffd56 100644 --- a/packages/components/src/internal/components/domainproperties/DomainForm.tsx +++ b/packages/components/src/internal/components/domainproperties/DomainForm.tsx @@ -1263,6 +1263,7 @@ export class DomainFormImpl extends React.PureComponent return ( { await act(async () => { renderWithAppContext( wrapDraggable( - + ) ); }); @@ -149,7 +154,7 @@ describe('DomainRow', () => { await act(async () => { renderWithAppContext( wrapDraggable( - + ) ); }); @@ -181,7 +186,7 @@ describe('DomainRow', () => { await act(async () => { renderWithAppContext( wrapDraggable( - + ) ); }); @@ -213,7 +218,7 @@ describe('DomainRow', () => { await act(async () => { renderWithAppContext( wrapDraggable( - + ) ); }); @@ -263,10 +268,10 @@ describe('DomainRow', () => { wrapDraggable( ) ); @@ -317,7 +322,7 @@ describe('DomainRow', () => { await act(async () => { renderWithAppContext( wrapDraggable( - + ) ); }); @@ -405,3 +410,38 @@ describe('DomainRow', () => { expect(rowDetails[0].textContent).toContain(expected); }); }); + +describe('shouldShowConfirmDataTypeChange', () => { + test('should return false for same type', () => { + expect(shouldShowConfirmDataTypeChange(STRING_RANGE_URI, STRING_RANGE_URI)).toBe(false); + }); + + test('should return true for converting to number', () => { + expect(shouldShowConfirmDataTypeChange(STRING_RANGE_URI, DOUBLE_RANGE_URI)).toBe(true); + expect(shouldShowConfirmDataTypeChange(INT_RANGE_URI, DOUBLE_RANGE_URI)).toBe(true); + }); + + test('should return true for converting to date', () => { + expect(shouldShowConfirmDataTypeChange(STRING_RANGE_URI, DATETIME_RANGE_URI)).toBe(true); + }); + + test('should return true for converting to multi-choice', () => { + expect(shouldShowConfirmDataTypeChange(STRING_RANGE_URI, MULTI_CHOICE_RANGE_URI)).toBe(true); + }); + + test('should return true for converting non-string/non-multichoice to string', () => { + expect(shouldShowConfirmDataTypeChange(INT_RANGE_URI, STRING_RANGE_URI)).toBe(true); + }); + + test('should return false for converting multichoice to string', () => { + expect(shouldShowConfirmDataTypeChange(MULTI_CHOICE_RANGE_URI, STRING_RANGE_URI)).toBe(false); + }); + + test('should return true for converting multichoice to textChoice', () => { + expect(shouldShowConfirmDataTypeChange(MULTI_CHOICE_RANGE_URI, TEXT_CHOICE_CONCEPT_URI)).toBe(true); + }); + + test('should return true for converting textChoice to multiChoice', () => { + expect(shouldShowConfirmDataTypeChange(TEXT_CHOICE_CONCEPT_URI, MULTI_CHOICE_RANGE_URI)).toBe(true); + }); +}); diff --git a/packages/components/src/internal/components/domainproperties/DomainRow.tsx b/packages/components/src/internal/components/domainproperties/DomainRow.tsx index 662c3e1202..7cad7ef059 100644 --- a/packages/components/src/internal/components/domainproperties/DomainRow.tsx +++ b/packages/components/src/internal/components/domainproperties/DomainRow.tsx @@ -74,6 +74,7 @@ import { ConfirmDataTypeChangeModal } from './ConfirmDataTypeChangeModal'; import { Collapsible } from './Collapsible'; export interface DomainRowProps { + allowMultiChoiceField: boolean; allowUniqueConstraintProperties: boolean; appPropertiesOnly?: boolean; availableTypes: List; @@ -263,26 +264,34 @@ export class DomainRow extends React.PureComponent { - const { field } = this.props; - const { value } = evt.target; + handleDataTypeChange = (targetId: string, value: any): void => { + const { field, index } = this.props; // warn for a saved field changing from any non-string -> string OR int/long -> double/float/decimal if (field.isSaved()) { const typeConvertingTo = PropDescType.fromName(value); - if (shouldShowConfirmDataTypeChange(field.original.rangeURI, typeConvertingTo.rangeURI)) { + if ( + shouldShowConfirmDataTypeChange( + field.original.conceptURI?? field.original.rangeURI, + typeConvertingTo.conceptURI ?? typeConvertingTo.rangeURI + ) + ) { this.onShowConfirmTypeChange(value); return; } } - this.onFieldChange( - evt, + const expand = PropDescType.isLookup(value) || - PropDescType.isTextChoice(value) || - PropDescType.isUser(value) || - PropDescType.isCalculation(value) - ); + PropDescType.isTextChoice(value) || + PropDescType.isUser(value) || + PropDescType.isCalculation(value); + + this.onSingleFieldChange(targetId, value, index, expand); + }; + + onDataTypeChange = (evt: any): void => { + this.handleDataTypeChange(evt.target.id, evt.target.value); }; onShowConfirmTypeChange = (dataTypeChangeToConfirm: string): void => { @@ -376,6 +385,7 @@ export class DomainRow extends React.PureComponent {isPrimaryKeyFieldLocked(field.lockType) ? ( - ) : ( @@ -491,7 +501,7 @@ export class DomainRow extends React.PureComponent ( - )) @@ -555,12 +565,14 @@ export class DomainRow extends React.PureComponent
)}
@@ -585,13 +597,22 @@ export class DomainRow extends React.PureComponent { +export const shouldShowConfirmDataTypeChange = (originalRangeURI: string, newRangeURI: string): boolean => { if (newRangeURI && originalRangeURI !== newRangeURI) { const wasString = STRING_CONVERT_URIS.indexOf(originalRangeURI) > -1; const toString = STRING_CONVERT_URIS.indexOf(newRangeURI) > -1; const toNumber = NUMBER_CONVERT_URIS.indexOf(newRangeURI) > -1; const toDate = DATETIME_CONVERT_URIS.indexOf(newRangeURI) > -1; - return toNumber || (toString && !wasString) || toDate; + const wasMultiChoice = PropDescType.isMultiChoice(originalRangeURI); + const newTextChoice = PropDescType.isTextChoice(newRangeURI); + const toMultiChoice = PropDescType.isMultiChoice(newRangeURI); + return ( + toNumber || + (toString && !wasString && !wasMultiChoice) || + toDate || + toMultiChoice || + (wasMultiChoice && newTextChoice) + ); } return false; }; diff --git a/packages/components/src/internal/components/domainproperties/DomainRowExpandedOptions.tsx b/packages/components/src/internal/components/domainproperties/DomainRowExpandedOptions.tsx index 8c91100903..7641f1f4c6 100644 --- a/packages/components/src/internal/components/domainproperties/DomainRowExpandedOptions.tsx +++ b/packages/components/src/internal/components/domainproperties/DomainRowExpandedOptions.tsx @@ -49,6 +49,8 @@ interface Props { queryName?: string; schemaName?: string; showingModal: (boolean) => void; + handleDataTypeChange: (targetId: string, value: any) => void; + allowMultiChoiceField: boolean; } export class DomainRowExpandedOptions extends React.Component { @@ -65,6 +67,8 @@ export class DomainRowExpandedOptions extends React.Component { domainContainerPath, schemaName, queryName, + handleDataTypeChange, + allowMultiChoiceField } = this.props; // In most cases we will use the selected data type to determine which field options to show, @@ -239,6 +243,8 @@ export class DomainRowExpandedOptions extends React.Component { schemaName={schemaName} lockedForDomain={domainFormDisplayOptions.textChoiceLockedForDomain} lockedSqlFragment={domainFormDisplayOptions.textChoiceLockedSqlFragment} + handleDataTypeChange={handleDataTypeChange} + allowMultiChoice={allowMultiChoiceField} /> ); case 'fileLink': diff --git a/packages/components/src/internal/components/domainproperties/PropDescType.ts b/packages/components/src/internal/components/domainproperties/PropDescType.ts index f0aba0c5a8..b68c36f347 100644 --- a/packages/components/src/internal/components/domainproperties/PropDescType.ts +++ b/packages/components/src/internal/components/domainproperties/PropDescType.ts @@ -34,8 +34,10 @@ import { export type JsonType = 'array' | 'boolean' | 'date' | 'float' | 'int' | 'string' | 'time'; interface IPropDescType { + altName?: string; conceptURI: string; display: string; + longDisplay?: string; lookupQuery?: string; lookupSchema?: string; name: string; @@ -47,18 +49,22 @@ export class PropDescType extends Record({ conceptURI: undefined, display: undefined, + longDisplay: undefined, name: undefined, rangeURI: undefined, alternateRangeURI: undefined, shortDisplay: undefined, lookupSchema: undefined, lookupQuery: undefined, + altName: undefined, }) implements IPropDescType { declare conceptURI: string; declare display: string; + declare longDisplay?: string; declare name: string; + declare altName?: string; declare rangeURI: string; declare alternateRangeURI: string; declare shortDisplay: string; @@ -236,6 +242,10 @@ export class PropDescType isDateTime(): boolean { return PropDescType.isDateTime(this.rangeURI); } + + get selectName(): string { + return this.altName ?? this.name; + } } export const TEXT_TYPE = new PropDescType({ @@ -360,13 +370,16 @@ export const UNIQUE_ID_TYPE = new PropDescType({ export const TEXT_CHOICE_TYPE = new PropDescType({ name: 'textChoice', display: 'Text Choice', + longDisplay: 'Text Choice (single select)', rangeURI: STRING_RANGE_URI, conceptURI: TEXT_CHOICE_CONCEPT_URI, }); export const MULTI_CHOICE_TYPE = new PropDescType({ name: 'multiChoice', - display: 'Multi Choice', + altName: 'textChoice', + display: 'Text Choice', + longDisplay: 'Text Choice (multiple select)', rangeURI: MULTI_CHOICE_RANGE_URI, }); diff --git a/packages/components/src/internal/components/domainproperties/TextChoiceOptions.spec.tsx b/packages/components/src/internal/components/domainproperties/TextChoiceOptions.spec.tsx deleted file mode 100644 index ab9065d776..0000000000 --- a/packages/components/src/internal/components/domainproperties/TextChoiceOptions.spec.tsx +++ /dev/null @@ -1,283 +0,0 @@ -import React from 'react'; -import { mount, ReactWrapper } from 'enzyme'; - -import { ChoicesListItem } from '../base/ChoicesListItem'; - -import { waitForLifecycle } from '../../test/enzymeTestHelpers'; - -import { LoadingSpinner } from '../base/LoadingSpinner'; - -import { AddEntityButton } from '../buttons/AddEntityButton'; - -import { TextChoiceOptionsImpl } from './TextChoiceOptions'; -import { DomainField } from './models'; -import { SectionHeading } from './SectionHeading'; -import { DomainFieldLabel } from './DomainFieldLabel'; - -describe('TextChoiceOptions', () => { - const DEFAULT_PROPS = { - label: 'Test Label', - field: DomainField.create({}), - fieldValues: {}, - loading: false, - replaceValues: jest.fn, - validValues: [], - index: 0, - domainIndex: 0, - onChange: jest.fn, - lockType: undefined, - }; - - function validate( - wrapper: ReactWrapper, - isLoading = false, - validValues = 0, - inUse = 0, - hasSelection = false, - hasValueUpdate = false, - hasValueError = false - ): void { - expect(wrapper.find(SectionHeading)).toHaveLength(1); - expect(wrapper.find(SectionHeading).prop('title')).toBe('Test Label'); - expect(wrapper.find(DomainFieldLabel)).toHaveLength(hasSelection ? 2 : 1); - expect(wrapper.find(LoadingSpinner)).toHaveLength(isLoading ? 1 : 0); - - expect(wrapper.find('.domain-text-choices-list')).toHaveLength(!isLoading ? 1 : 0); - - if (!isLoading) { - expect(wrapper.find('.domain-text-choices-left-panel')).toHaveLength(validValues > 0 ? 1 : 0); - expect(wrapper.find(ChoicesListItem)).toHaveLength(validValues); - expect(wrapper.find('.choices-list__locked')).toHaveLength(inUse); - expect(wrapper.find(AddEntityButton)).toHaveLength(1); - expect(wrapper.find('.choices-detail__empty-message')).toHaveLength( - validValues > 0 && !hasSelection ? 1 : 0 - ); - expect(wrapper.find('input.full-width')).toHaveLength(hasSelection ? 1 : 0); - expect(wrapper.find('button')).toHaveLength(validValues + (hasSelection ? 2 : 0)); - expect(wrapper.find('.domain-text-choices-info').hostNodes()).toHaveLength(hasValueUpdate ? 1 : 0); - expect(wrapper.find('.alert-danger')).toHaveLength(hasValueError ? 1 : 0); - expect(wrapper.find('input.domain-text-choices-search')).toHaveLength(validValues > 2 ? 1 : 0); - } - } - - test('default props', () => { - const wrapper = mount(); - validate(wrapper); - expect(wrapper.find(AddEntityButton).prop('disabled')).toBeFalsy(); - wrapper.unmount(); - }); - - test('loading', () => { - const wrapper = mount(); - validate(wrapper, true); - wrapper.unmount(); - }); - - test('with validValues, no selection', () => { - const wrapper = mount(); - validate(wrapper, false, 2); - expect(wrapper.find(ChoicesListItem).first().prop('active')).toBeFalsy(); - wrapper.unmount(); - }); - - test('with validValues, with selection', async () => { - const wrapper = mount(); - wrapper.find(ChoicesListItem).first().simulate('click'); - await waitForLifecycle(wrapper); - validate(wrapper, false, 2, 0, true); - expect(wrapper.find(ChoicesListItem).first().prop('active')).toBeTruthy(); - wrapper.unmount(); - }); - - test('apply button disabled', async () => { - const wrapper = mount(); - wrapper.find(ChoicesListItem).first().simulate('click'); - await waitForLifecycle(wrapper); - - expect(wrapper.find('input.full-width').prop('value')).toBe('a'); - expect(wrapper.find('input.full-width').prop('disabled')).toBeFalsy(); - expect(wrapper.find('.btn-success').prop('disabled')).toBeTruthy(); - - wrapper.find('input.full-width').simulate('change', { target: { name: 'value', value: 'aa' } }); - await waitForLifecycle(wrapper); - expect(wrapper.find('input.full-width').prop('value')).toBe('aa'); - expect(wrapper.find('.btn-success').prop('disabled')).toBeFalsy(); - - wrapper.unmount(); - }); - - test('choice item empty', async () => { - const wrapper = mount(); - expect(wrapper.find(ChoicesListItem).first().prop('label')).toBe('a'); - expect(wrapper.find(ChoicesListItem).first().prop('subLabel')).toBe(undefined); - expect(wrapper.find(ChoicesListItem).last().prop('label')).toBe('b'); - expect(wrapper.find(ChoicesListItem).last().prop('subLabel')).toBe(undefined); - - wrapper.setProps({ - validValues: ['', 'b'], - }); - await waitForLifecycle(wrapper); - - expect(wrapper.find(ChoicesListItem).first().prop('label')).toBe(''); - expect(wrapper.find(ChoicesListItem).first().prop('subLabel')).toBe('Empty Value'); - expect(wrapper.find(ChoicesListItem).last().prop('label')).toBe('b'); - expect(wrapper.find(ChoicesListItem).last().prop('subLabel')).toBe(undefined); - - wrapper.unmount(); - }); - - test('with inUse values', async () => { - const wrapper = mount( - - ); - validate(wrapper, false, 2, 1); - - // select the in-use value and check right hand items - wrapper.find(ChoicesListItem).last().simulate('click'); - await waitForLifecycle(wrapper); - validate(wrapper, false, 2, 1, true); - expect(wrapper.find('input.full-width').prop('disabled')).toBeFalsy(); - - wrapper.unmount(); - }); - - test('with inUse value update info', async () => { - const wrapper = mount( - - ); - validate(wrapper, false, 2, 1); - - // select the in-use value, change it, and apply - wrapper.find(ChoicesListItem).last().simulate('click'); - await waitForLifecycle(wrapper); - wrapper.find('input.full-width').simulate('change', { target: { name: 'value', value: 'bb' } }); - await waitForLifecycle(wrapper); - wrapper.find('.btn-success').simulate('click'); - await waitForLifecycle(wrapper); - wrapper.setProps({ validValues: ['a', 'bb'] }); - await waitForLifecycle(wrapper); - - validate(wrapper, false, 2, 1, true, true); - expect(wrapper.find('.domain-text-choices-info').hostNodes().text()).toBe( - '1 row with value b will be updated to bb on save.' - ); - - wrapper.unmount(); - }); - - test('with locked values', async () => { - const wrapper = mount( - - ); - validate(wrapper, false, 2, 1); - - // select the locked value and check right hand items - wrapper.find(ChoicesListItem).last().simulate('click'); - await waitForLifecycle(wrapper); - validate(wrapper, false, 2, 1, true); - expect(wrapper.find('input.full-width').prop('disabled')).toBeTruthy(); - - wrapper.unmount(); - }); - - test('value update error checks', async () => { - const wrapper = mount(); - - wrapper.find(ChoicesListItem).last().simulate('click'); - await waitForLifecycle(wrapper); - validate(wrapper, false, 2, 0, true); - - // don't allow empty string - wrapper.find('input.full-width').simulate('change', { target: { name: 'value', value: 'bb' } }); - await waitForLifecycle(wrapper); - expect(wrapper.find('.btn-success').prop('disabled')).toBeFalsy(); - wrapper.find('input.full-width').simulate('change', { target: { name: 'value', value: ' ' } }); - await waitForLifecycle(wrapper); - expect(wrapper.find('.btn-success').prop('disabled')).toBeTruthy(); - - // don't allow duplicates - wrapper.find('input.full-width').simulate('change', { target: { name: 'value', value: ' a ' } }); - await waitForLifecycle(wrapper); - expect(wrapper.find('.btn-success').prop('disabled')).toBeTruthy(); - validate(wrapper, false, 2, 0, true, false, true); - expect(wrapper.find('.alert-danger').text()).toBe('"a" already exists in the list of values.'); - - wrapper.unmount(); - }); - - test('delete button disabled', async () => { - const wrapper = mount( - - ); - validate(wrapper, false, 2, 1); - - // first value, not in use - wrapper.find(ChoicesListItem).first().simulate('click'); - await waitForLifecycle(wrapper); - expect(wrapper.find('.btn-default').last().prop('disabled')).toBeFalsy(); - - // second value, in use - wrapper.find(ChoicesListItem).last().simulate('click'); - await waitForLifecycle(wrapper); - expect(wrapper.find('.btn-default').last().prop('disabled')).toBeTruthy(); - - wrapper.unmount(); - }); - - test('AddEntityButton disabled if max reached', () => { - const wrapper = mount(); - validate(wrapper, false, 2); - expect(wrapper.find(AddEntityButton).prop('disabled')).toBeTruthy(); - wrapper.unmount(); - }); - - test('search', async () => { - const wrapper = mount(); - validate(wrapper, false, 4); - wrapper - .find('input.domain-text-choices-search') - .simulate('change', { target: { name: 'value', value: ' a ' } }); - await waitForLifecycle(wrapper); - let values = wrapper.find(ChoicesListItem); - expect(values).toHaveLength(3); - expect(values.at(0).text()).toBe('a'); - expect(values.at(1).text()).toBe('aa'); - expect(values.at(2).text()).toBe('aaa'); - wrapper.find('input.domain-text-choices-search').simulate('change', { target: { name: 'value', value: 'b' } }); - await waitForLifecycle(wrapper); - values = wrapper.find(ChoicesListItem); - expect(values).toHaveLength(1); - expect(values.at(0).text()).toBe('b'); - wrapper.find('input.domain-text-choices-search').simulate('change', { target: { name: 'value', value: 'AA' } }); - await waitForLifecycle(wrapper); - values = wrapper.find(ChoicesListItem); - expect(values).toHaveLength(2); - expect(values.at(0).text()).toBe('aa'); - expect(values.at(1).text()).toBe('aaa'); - wrapper.find('input.domain-text-choices-search').simulate('change', { target: { name: 'value', value: '' } }); - await waitForLifecycle(wrapper); - values = wrapper.find(ChoicesListItem); - expect(values).toHaveLength(4); - expect(values.at(0).text()).toBe('a'); - expect(values.at(1).text()).toBe('aa'); - expect(values.at(2).text()).toBe('aaa'); - expect(values.at(3).text()).toBe('b'); - wrapper.unmount(); - }); -}); diff --git a/packages/components/src/internal/components/domainproperties/TextChoiceOptions.test.tsx b/packages/components/src/internal/components/domainproperties/TextChoiceOptions.test.tsx new file mode 100644 index 0000000000..b9a3f3451c --- /dev/null +++ b/packages/components/src/internal/components/domainproperties/TextChoiceOptions.test.tsx @@ -0,0 +1,342 @@ +/* + * Copyright (c) 2019 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React from 'react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; + +import { TextChoiceOptionsImpl } from './TextChoiceOptions'; +import { DomainField } from './models'; +import { MULTI_CHOICE_RANGE_URI } from './constants'; + +describe('TextChoiceOptions', () => { + const DEFAULT_PROPS = { + label: 'Test Label', + field: DomainField.create({}), + fieldValues: {}, + loading: false, + replaceValues: jest.fn(), + validValues: [], + index: 0, + domainIndex: 0, + onChange: jest.fn(), + handleDataTypeChange: jest.fn(), + lockType: undefined, + allowMultiChoice: true, + }; + + function validate( + isLoading = false, + validValuesCount = 0, + inUse = 0, + hasSelection = false, + hasValueUpdate = false, + hasValueError = false + ): void { + expect(screen.getByText('Test Label')).toBeInTheDocument(); + + if (isLoading) { + expect(screen.getByText('Loading...')).toBeInTheDocument(); + expect(document.querySelector('.domain-text-choices-list')).not.toBeInTheDocument(); + } else { + expect(screen.queryByText('Loading...')).not.toBeInTheDocument(); + + const list = document.querySelector('.domain-text-choices-list'); + if (validValuesCount > 0 || hasSelection) { + expect(list).toBeInTheDocument(); + } + + const items = list?.querySelectorAll('.list-group-item'); + if (items) { + expect(items.length).toBe(validValuesCount); + } else { + expect(validValuesCount).toBe(0); + } + + expect(document.querySelectorAll('.choices-list__locked').length).toBe(inUse); + const addBtn = document.querySelector('span.container--action-button'); + expect(addBtn.textContent).toBe(' Add Values'); + + if (validValuesCount > 0 && !hasSelection) { + expect( + screen.getByText('Select a value from the list on the left to view details.') + ).toBeInTheDocument(); + } + + const inputs = screen.queryAllByPlaceholderText('Enter a text choice value'); + expect(inputs).toHaveLength(hasSelection ? 1 : 0); + + const updateInfos = document.querySelectorAll('.domain-text-choices-info'); + expect(updateInfos).toHaveLength(hasValueUpdate ? 1 : 0); + + const errors = document.querySelectorAll('.alert-danger'); + expect(errors).toHaveLength(hasValueError ? 1 : 0); + + const searchInputs = screen.queryAllByPlaceholderText('Find a value'); + expect(searchInputs).toHaveLength(validValuesCount > 2 ? 1 : 0); + } + } + + test('default props', () => { + render(); + validate(); + const addBtn = document.querySelector('span.container--action-button'); + expect(addBtn.textContent).toBe(' Add Values'); + expect(addBtn.getAttribute('class').indexOf('disabled')).toBe(-1); + + // verify multi-choice checkbox exists, is unchecked, and enabled by default + const multiCheckbox = document.querySelector('input.domain-text-choice-multi') as HTMLInputElement; + expect(multiCheckbox).toBeInTheDocument(); + expect(multiCheckbox.checked).toBe(false); + expect(multiCheckbox).toBeEnabled(); + expect(screen.getByText('Allow multiple selections')).toBeInTheDocument(); + }); + + test('loading', () => { + render(); + validate(true); + }); + + test('multi-choice checkbox checked when field is multi-choice', () => { + render( + + ); + const multiCheckbox = document.querySelector('input.domain-text-choice-multi') as HTMLInputElement; + expect(multiCheckbox).toBeInTheDocument(); + expect(multiCheckbox).toBeEnabled(); + expect(multiCheckbox.checked).toBe(true); + }); + + test('multi-choice checkbox disabled when multi values are in use', () => { + render(); + const multiCheckbox = document.querySelector('input.domain-text-choice-multi') as HTMLInputElement; + expect(multiCheckbox).toBeInTheDocument(); + expect(multiCheckbox).toBeDisabled(); + const labelSpan = screen.getByText('Allow multiple selections'); + expect(labelSpan.getAttribute('title')).toBe('Multiple values are currently used by at least one data row.'); + }); + + test('multi-choice checkbox not present', () => { + render(); + expect(document.querySelectorAll('input.domain-text-choice-multi')).toHaveLength(0); + }); + + test('with validValues, no selection', () => { + render(); + validate(false, 2); + const items = document.querySelectorAll('.list-group-item'); + expect(items[0]).not.toHaveClass('active'); + }); + + test('with validValues, with selection', async () => { + render(); + fireEvent.click(screen.getByRole('button', { name: 'a' })); + await waitFor(() => { + validate(false, 2, 0, true); + }); + const items = document.querySelectorAll('.list-group-item'); + expect(items[0]).toHaveClass('active'); + }); + + test('apply button disabled', async () => { + render(); + fireEvent.click(screen.getByRole('button', { name: 'a' })); + + const input = screen.getByPlaceholderText('Enter a text choice value'); + expect(input).toHaveValue('a'); + expect(input).toBeEnabled(); + expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled(); + + fireEvent.change(input, { target: { value: 'aa' } }); + await waitFor(() => { + expect(input).toHaveValue('aa'); + }); + expect(screen.getByRole('button', { name: 'Apply' })).toBeEnabled(); + }); + + test('choice item empty', async () => { + const { rerender } = render(); + + let items = screen.getAllByRole('button').filter(b => b.classList.contains('list-group-item')); + expect(items[0]).toHaveTextContent('a'); + expect(items[1]).toHaveTextContent('b'); + + rerender(); + + items = screen.getAllByRole('button').filter(b => b.classList.contains('list-group-item')); + expect(items[0]).toHaveTextContent('Empty Value'); + expect(items[1]).toHaveTextContent('b'); + }); + + test('with inUse values', async () => { + render( + + ); + validate(false, 2, 1); + + // select the in-use value and check right hand items + // 'b' is the label, but it also has the lock icon. The button text content includes 'b'. + // Because of the icon, the accessible name might be tricky. + // We can query by text content. + const bButton = screen.getAllByRole('button').find(b => b.textContent?.includes('b')); + fireEvent.click(bButton); + + await waitFor(() => { + validate(false, 2, 1, true); + }); + expect(screen.getByPlaceholderText('Enter a text choice value')).toBeEnabled(); + }); + + test('with inUse value update info', async () => { + const replaceValues = jest.fn(); + const { rerender } = render( + + ); + validate(false, 2, 1); + + // select the in-use value, change it, and apply + const bButton = screen.getAllByRole('button').find(b => b.textContent?.includes('b')); + fireEvent.click(bButton); + + const input = screen.getByPlaceholderText('Enter a text choice value'); + fireEvent.change(input, { target: { value: 'bb' } }); + + const applyBtn = screen.getByRole('button', { name: 'Apply' }); + await waitFor(() => expect(applyBtn).toBeEnabled()); + fireEvent.click(applyBtn); + + expect(replaceValues).toHaveBeenCalled(); + }); + + test('with locked values', async () => { + render( + + ); + validate(false, 2, 1); + + // select the locked value and check right hand items + const bButton = screen.getAllByRole('button').find(b => b.textContent?.includes('b')); + fireEvent.click(bButton); + + await waitFor(() => { + validate(false, 2, 1, true); + }); + expect(screen.getByPlaceholderText('Enter a text choice value')).toBeDisabled(); + }); + + test('value update error checks', async () => { + render(); + + const bButton = screen.getByRole('button', { name: 'b' }); + fireEvent.click(bButton); + validate(false, 2, 0, true); + + // don't allow empty string + const input = screen.getByPlaceholderText('Enter a text choice value'); + fireEvent.change(input, { target: { value: 'bb' } }); + const applyBtn = screen.getByRole('button', { name: 'Apply' }); + expect(applyBtn).toBeEnabled(); + + fireEvent.change(input, { target: { value: ' ' } }); + expect(applyBtn).toBeDisabled(); + + // don't allow duplicates + fireEvent.change(input, { target: { value: ' a ' } }); + expect(applyBtn).toBeDisabled(); + + validate(false, 2, 0, true, false, true); + const alert = document.querySelector('.alert-danger'); + expect(alert).toHaveTextContent('"a" already exists in the list of values.'); + }); + + test('delete button disabled', async () => { + render( + + ); + validate(false, 2, 1); + + // first value, not in use + const aButton = screen.getByRole('button', { name: 'a' }); + fireEvent.click(aButton); + + // Delete button is the one with trash icon. "Delete" text is in span after icon. + // DisableableButton renders children. + const deleteBtn = screen.getByRole('button', { name: /Delete/ }); + expect(deleteBtn).toBeEnabled(); + + // second value, in use + const bButton = screen.getAllByRole('button').find(b => b.textContent?.includes('b')); + fireEvent.click(bButton); + + const deleteBtn2 = screen.getByRole('button', { name: /Delete/ }); + expect(deleteBtn2).toBeDisabled(); + }); + + test('AddEntityButton disabled if max reached', () => { + render(); + validate(false, 2); + const addBtn = document.querySelector('span.container--action-button'); + expect(addBtn.textContent).toBe(' Add Values'); + expect(addBtn.getAttribute('class')).toContain(' disabled'); + }); + + test('search', async () => { + render(); + validate(false, 4); + + const searchInput = screen.getByPlaceholderText('Find a value'); + + fireEvent.change(searchInput, { target: { value: ' a ' } }); + let items = document.querySelectorAll('.list-group-item'); + expect(items).toHaveLength(3); + expect(items[0]).toHaveTextContent('a'); + expect(items[1]).toHaveTextContent('aa'); + expect(items[2]).toHaveTextContent('aaa'); + + fireEvent.change(searchInput, { target: { value: 'b' } }); + items = document.querySelectorAll('.list-group-item'); + expect(items).toHaveLength(1); + expect(items[0]).toHaveTextContent('b'); + + fireEvent.change(searchInput, { target: { value: 'AA' } }); + items = document.querySelectorAll('.list-group-item'); + expect(items).toHaveLength(2); + expect(items[0]).toHaveTextContent('aa'); + expect(items[1]).toHaveTextContent('aaa'); + + fireEvent.change(searchInput, { target: { value: '' } }); + items = document.querySelectorAll('.list-group-item'); + expect(items).toHaveLength(4); + }); +}); diff --git a/packages/components/src/internal/components/domainproperties/TextChoiceOptions.tsx b/packages/components/src/internal/components/domainproperties/TextChoiceOptions.tsx index a831e63a9b..9bbb249a38 100644 --- a/packages/components/src/internal/components/domainproperties/TextChoiceOptions.tsx +++ b/packages/components/src/internal/components/domainproperties/TextChoiceOptions.tsx @@ -12,14 +12,21 @@ import { DisableableButton } from '../buttons/DisableableButton'; import { DisableableInput } from '../forms/DisableableInput'; -import { DOMAIN_VALIDATOR_TEXTCHOICE, MAX_VALID_TEXT_CHOICES } from './constants'; +import { + DOMAIN_FIELD_TEXTCHOICE_MULTI, + DOMAIN_FIELD_TYPE, + DOMAIN_VALIDATOR_TEXTCHOICE, + MAX_VALID_TEXT_CHOICES, +} from './constants'; import { DEFAULT_TEXT_CHOICE_VALIDATOR, DomainField, ITypeDependentProps, PropertyValidator } from './models'; import { SectionHeading } from './SectionHeading'; import { DomainFieldLabel } from './DomainFieldLabel'; import { TextChoiceAddValuesModal } from './TextChoiceAddValuesModal'; -import { getTextChoiceInUseValues } from './actions'; +import { getTextChoiceInUseValues, TextChoiceInUseValues } from './actions'; import { createFormInputId } from './utils'; +import { isFieldFullyLocked } from './propertiesUtil'; +import { MULTI_CHOICE_TYPE, TEXT_CHOICE_TYPE } from './PropDescType'; const MIN_VALUES_FOR_SEARCH_COUNT = 2; const HELP_TIP_BODY =

The set of values to be used as drop-down options to restrict data entry into this field.

; @@ -28,9 +35,9 @@ const IN_USE_TITLE = 'Text Choice In Use'; const IN_USE_TIP = 'This text choice value cannot be deleted because it is in use.'; const VALUE_IN_USE = ( @@ -41,16 +48,18 @@ const LOCKED_TIP = 'This text choice value cannot be deleted because it is in use and cannot be edited because one or more usages are for read-only items.'; const VALUE_LOCKED = ( ); interface Props extends ITypeDependentProps { + allowMultiChoice: boolean; field: DomainField; + handleDataTypeChange: (targetId: string, value: any) => void; lockedForDomain?: boolean; lockedSqlFragment?: string; queryName?: string; @@ -61,6 +70,7 @@ interface ImplProps extends Props { // mapping existing field values (existence in this object signals "in use") to locked status (only applicable // to some domain types) and row count for the given value fieldValues: Record>; + hasMultiValueInUse?: boolean; loading: boolean; maxValueCount?: number; replaceValues: (newValues: string[], valueUpdates?: Record) => void; @@ -78,12 +88,19 @@ export const TextChoiceOptionsImpl: FC = memo(props => { replaceValues, maxValueCount = MAX_VALID_TEXT_CHOICES, lockedForDomain, + domainIndex, + index, + handleDataTypeChange, + hasMultiValueInUse, + allowMultiChoice, } = props; const [selectedIndex, setSelectedIndex] = useState(); const [currentValue, setCurrentValue] = useState(); const [currentError, setCurrentError] = useState(); const [showAddValuesModal, setShowAddValuesModal] = useState(); const [search, setSearch] = useState(''); + const fieldTypeId = createFormInputId(DOMAIN_FIELD_TYPE, domainIndex, index); + const isMultiChoiceField = field.dataType.name === MULTI_CHOICE_TYPE.name; // keep a map from the updated values for the in-use field values to their original values const [fieldValueUpdates, setFieldValueUpdates] = useState>({}); @@ -188,6 +205,14 @@ export const TextChoiceOptionsImpl: FC = memo(props => { setSearch(event.target.value); }, []); + const onAllowMultiChange = useCallback( + (event: any): void => { + const { checked } = event.target; + handleDataTypeChange?.(fieldTypeId, checked ? MULTI_CHOICE_TYPE.name : TEXT_CHOICE_TYPE.name); + }, + [handleDataTypeChange, fieldTypeId] + ); + return (
@@ -198,7 +223,7 @@ export const TextChoiceOptionsImpl: FC = memo(props => {
- +
@@ -238,12 +263,12 @@ export const TextChoiceOptionsImpl: FC = memo(props => { return ( ); })} @@ -254,6 +279,27 @@ export const TextChoiceOptionsImpl: FC = memo(props => { onClick={toggleAddValues} title={`Add Values (max ${maxValueCount})`} /> + {allowMultiChoice && ( + <> + + + Allow multiple selections + + + )}
{validValues.length > 0 && selectedIndex === undefined && ( @@ -261,7 +307,12 @@ export const TextChoiceOptionsImpl: FC = memo(props => { Select a value from the list on the left to view details.

)} - {selectedIndex !== undefined && ( + {selectedIndex !== undefined && currentInUse && isMultiChoiceField && ( +

+ Value is currently in use and cannot be updated. +

+ )} + {selectedIndex !== undefined && (!currentInUse || !isMultiChoiceField) && ( <>
@@ -320,9 +371,9 @@ export const TextChoiceOptionsImpl: FC = memo(props => { {showAddValuesModal && ( )}
@@ -333,9 +384,9 @@ TextChoiceOptionsImpl.displayName = 'TextChoiceOptionsImpl'; export const TextChoiceOptions: FC = memo(props => { const { field, onChange, domainIndex, index, schemaName, queryName, lockedSqlFragment = 'FALSE' } = props; const [loading, setLoading] = useState(true); - const [fieldValues, setFieldValues] = useState>>({}); + const [fieldValues, setFieldValues] = useState(null); const [validValues, setValidValues] = useState(field.textChoiceValidator?.properties.validValues ?? []); - const fieldId = createFormInputId(DOMAIN_VALIDATOR_TEXTCHOICE, domainIndex, index); + const fieldValidatorId = createFormInputId(DOMAIN_VALIDATOR_TEXTCHOICE, domainIndex, index); const replaceValues = useCallback( (newValues: string[], newValueUpdates?: Record) => { @@ -349,7 +400,7 @@ export const TextChoiceOptions: FC = memo(props => { }, {}); onChange( - fieldId, + fieldValidatorId, new PropertyValidator({ // keep the existing validator Id/props, if present, and override the expression / properties ...field.textChoiceValidator, @@ -361,7 +412,7 @@ export const TextChoiceOptions: FC = memo(props => { }) ); }, - [field.textChoiceValidator, fieldId, onChange] + [field.textChoiceValidator, fieldValidatorId, onChange] ); useEffect( @@ -369,7 +420,8 @@ export const TextChoiceOptions: FC = memo(props => { // for an existing field, we query for the distinct set of values in the Text column to be used for // the initial set of values and/or setting fields as locked (i.e. in use) if (!field.isNew() && schemaName && queryName) { - getTextChoiceInUseValues(field, schemaName, queryName, lockedSqlFragment) + const isMulti = field.dataType.name === MULTI_CHOICE_TYPE.name; + getTextChoiceInUseValues(field, schemaName, queryName, lockedSqlFragment, isMulti) .then(values => { setFieldValues(values); @@ -377,7 +429,7 @@ export const TextChoiceOptions: FC = memo(props => { // that is being changed to data type = Text Choice (that "is new field" check is above), // then we will use the existing distinct values for that field as the initial options if (!field.textChoiceValidator?.rowId) { - replaceValues(Object.keys(values).sort()); + replaceValues(Object.keys(values.useCount).sort()); } setLoading(false); @@ -397,7 +449,8 @@ export const TextChoiceOptions: FC = memo(props => { return ( { expect(field.textChoiceValidator).toBe(DEFAULT_TEXT_CHOICE_VALIDATOR); }); + test('updateDataType clear properties when changing to MultiChoice field - from TextChoice', () => { + let field = DomainField.create({ + propertyId: 1, + propertyValidators: [ + { type: 'Range', name: 'Range Validator', expression: '' }, + { type: 'RegEx', name: 'RegEx Validator', expression: '' }, + { type: 'Lookup', name: 'Lookup Validator', expression: '' }, + ], + measure: true, + dimension: true, + mvEnabled: true, + recommendedVariable: true, + uniqueConstraint: true, + nonUniqueConstraint: true, + }); + + // Change to Text Choice to ensure textChoiceValidator exists + field = updateDataType(field, 'textChoice'); + expect(field.dataType).toBe(TEXT_CHOICE_TYPE); + expect(field.textChoiceValidator).toBe(DEFAULT_TEXT_CHOICE_VALIDATOR); + expect(field.measure).toBe(true); + expect(field.dimension).toBe(true); + expect(field.mvEnabled).toBe(true); + expect(field.recommendedVariable).toBe(true); + expect(field.uniqueConstraint).toBe(true); + expect(field.nonUniqueConstraint).toBe(true); + + // Change to MultiChoice + field = updateDataType(field, 'multiChoice'); + expect(field.dataType).toBe(MULTI_CHOICE_TYPE); + // validators and expressions cleared for multiChoice + expect(field.lookupValidator).toBeUndefined(); + expect(field.rangeValidators?.size).toBe(0); + expect(field.regexValidators?.size).toBe(0); + expect(field.valueExpression).toBeUndefined(); + // flags cleared for multiChoice + expect(field.measure).toBe(false); + expect(field.dimension).toBe(false); + expect(field.mvEnabled).toBe(false); + expect(field.recommendedVariable).toBe(false); + expect(field.uniqueConstraint).toBe(false); + expect(field.nonUniqueConstraint).toBe(false); + // textChoiceValidator should still be present for text/multi choice types + expect(field.textChoiceValidator).toBeDefined(); + }); + + function convertToMultiChoiceFromDataType(isNewField: boolean) { + let field = DomainField.create({ + propertyId: isNewField ? 0 : 1, + propertyValidators: [ + { type: 'Range', name: 'Range Validator', expression: '' }, + { type: 'RegEx', name: 'RegEx Validator', expression: '' }, + { type: 'Lookup', name: 'Lookup Validator', expression: '' }, + ], + scale: 10, + measure: true, + dimension: true, + mvEnabled: true, + recommendedVariable: true, + uniqueConstraint: true, + nonUniqueConstraint: true, + rangeURI: STRING_RANGE_URI, + }); + expect(field.dataType).toBe(TEXT_TYPE); + expect(field.scale).toBe(10); + expect(field.lookupValidator).toBeDefined(); + expect(field.rangeValidators.size).toBe(1); + expect(field.regexValidators.size).toBe(1); + + field = updateDataType(field, 'multiChoice'); + expect(field.dataType).toBe(MULTI_CHOICE_TYPE); + // default textChoiceValidator added when transitioning from non-textChoice + expect(field.textChoiceValidator).toBe(DEFAULT_TEXT_CHOICE_VALIDATOR); + // validators cleared + expect(field.lookupValidator).toBeUndefined(); + expect(field.rangeValidators.size).toBe(0); + expect(field.regexValidators.size).toBe(0); + // scale set to MAX for choice types + expect(field.scale).toBe(MAX_TEXT_LENGTH); + // flags cleared for multiChoice + expect(field.measure).toBe(false); + expect(field.dimension).toBe(false); + expect(field.mvEnabled).toBe(false); + expect(field.recommendedVariable).toBe(false); + expect(field.uniqueConstraint).toBe(false); + expect(field.nonUniqueConstraint).toBe(false); + expect(field.valueExpression).toBeUndefined(); + } + test('updateDataType clear properties when changing to MultiChoice field - from String', () => { + convertToMultiChoiceFromDataType(true); + convertToMultiChoiceFromDataType(false); + }); + test('updateDataType isLookup', () => { let field = DomainField.create({}); expect(field.dataType).toBe(TEXT_TYPE); diff --git a/packages/components/src/internal/components/domainproperties/actions.ts b/packages/components/src/internal/components/domainproperties/actions.ts index a1fd4210ab..afccab199a 100644 --- a/packages/components/src/internal/components/domainproperties/actions.ts +++ b/packages/components/src/internal/components/domainproperties/actions.ts @@ -86,6 +86,7 @@ import { VISIT_LABEL_TYPE, } from './PropDescType'; import { + ConditionalFormat, decodeLookup, DEFAULT_TEXT_CHOICE_VALIDATOR, DomainDesign, @@ -891,17 +892,35 @@ export function updateDataType(field: DomainField, value: any): DomainField { field = field.merge(DomainField.resolveLookupConfig(field, dataType)) as DomainField; } - if ((field.isTextChoiceField() || field.isMultiChoiceField()) && !wasTextChoice) { + if (field.isTextChoiceField() || field.isMultiChoiceField()) { // when changing a field to a Text Choice, add the default textChoiceValidator and // remove/reset all other propertyValidators and other text option settings - field = field.merge({ - textChoiceValidator: DEFAULT_TEXT_CHOICE_VALIDATOR, - lookupValidator: undefined, - rangeValidators: [], - regexValidators: [], - scale: MAX_TEXT_LENGTH, - valueExpression: undefined, - }) as DomainField; + if (!wasTextChoice) { + field = field.merge({ + textChoiceValidator: DEFAULT_TEXT_CHOICE_VALIDATOR, + lookupValidator: undefined, + rangeValidators: [], + regexValidators: [], + scale: MAX_TEXT_LENGTH, + valueExpression: undefined, + }) as DomainField; + } + + if (field.isMultiChoiceField()) { + field = field.merge({ + lookupValidator: undefined, + rangeValidators: [], + regexValidators: [], + valueExpression: undefined, + dimension: false, + measure: false, + mvEnabled: false, + recommendedVariable: false, + uniqueConstraint: false, + nonUniqueConstraint: false, + conditionalFormats: List(), + }) as DomainField; + } } else if (field.isCalculatedField()) { field = field.merge({ importAliases: undefined, @@ -1365,17 +1384,45 @@ export function getDomainNamePreviews( }); } -type TextChoiceInUseValues = Record; +export type TextChoiceInUseValues = { + hasMultiValue: boolean; + useCount: Record; +}; + +function processTextChoiceRow( value: any, isMultiField: boolean, isLocked: boolean, rowCount: number, useCount: Record, hasMultiValue: boolean ): boolean { + if (!isMultiField && !isValidTextChoiceValue(value)) return hasMultiValue; + + const values: string[] = []; + if (isMultiField && Array.isArray(value)) { + values.push(...value); + hasMultiValue = hasMultiValue || value.length > 1; + } else { + values.push(value); + } + + values.forEach(val => { + if (!useCount[val]) { + useCount[val] = { count: 0, locked: false }; + } + useCount[val].count += rowCount; + useCount[val].locked = useCount[val].locked || isLocked; + }); + + return hasMultiValue; +} export async function getTextChoiceInUseValues( field: DomainField, schemaName: string, queryName: string, - lockedSqlFragment: string + lockedSqlFragment: string, + isMultiField: boolean ): Promise { const containerFilter = Query.ContainerFilter.allInProjectPlusShared; // to account for a shared domain at project or /Shared const fieldName = field.original?.name ?? field.name; + const useCount: Record = {}; + let hasMultiValue = false; // If the field is set as PHI, we need the query to include the RowId for logging, so we have to do the aggregate client side if (field.isPHI()) { const result = await selectRows({ @@ -1386,37 +1433,30 @@ export async function getTextChoiceInUseValues( schemaQuery: new SchemaQuery(schemaName, queryName), }); - const values: TextChoiceInUseValues = {}; result.rows.forEach(row => { - const value = row[fieldName]?.value; - if (isValidTextChoiceValue(value)) { - if (!values[value]) { - values[value] = { count: 0, locked: false }; - } - values[value].count++; - values[value].locked = - values[value].locked || caseInsensitive(row, 'SampleState/StatusType').value === 'Locked'; - } + const value = caseInsensitive(row, fieldName)?.value; + const rowLocked = caseInsensitive(row, 'SampleState/StatusType').value === 'Locked'; + hasMultiValue = processTextChoiceRow(value, isMultiField, rowLocked, 1, useCount, hasMultiValue); + }); + } else { + const response = await executeSql({ + containerFilter, + schemaName, + sql: `SELECT "${fieldName}", ${lockedSqlFragment} AS IsLocked, COUNT(*) AS RowCount FROM "${queryName}" WHERE "${fieldName}" IS NOT NULL GROUP BY "${fieldName}"`, }); - return values; - } - const response = await executeSql({ - containerFilter, - schemaName, - sql: `SELECT "${fieldName}", ${lockedSqlFragment} AS IsLocked, COUNT(*) AS RowCount FROM "${queryName}" WHERE "${fieldName}" IS NOT NULL GROUP BY "${fieldName}"`, - }); + response.rows.forEach(row => { + const value = caseInsensitive(row, fieldName)?.value; + const rowLocked = row.IsLocked.value === 1; + const rowCount = row.RowCount.value; + hasMultiValue = processTextChoiceRow(value, isMultiField, rowLocked, rowCount, useCount, hasMultiValue); + }); + } - return response.rows - .filter(row => isValidTextChoiceValue(row[fieldName].value)) - .reduce((prev, row) => { - const value = row[fieldName].value; - prev[value] = { - count: row.RowCount.value, - locked: row.IsLocked.value === 1, - }; - return prev; - }, {}); + return { + useCount, + hasMultiValue, + }; } export function getGenId(rowId: number, kindName: 'DataClass' | 'SampleSet', containerPath?: string): Promise { diff --git a/packages/components/src/internal/components/domainproperties/constants.ts b/packages/components/src/internal/components/domainproperties/constants.ts index c39a9237a4..51d3c6f790 100644 --- a/packages/components/src/internal/components/domainproperties/constants.ts +++ b/packages/components/src/internal/components/domainproperties/constants.ts @@ -82,6 +82,7 @@ export const DOMAIN_VALIDATOR_NAME = 'name'; export const DOMAIN_VALIDATOR_REMOVE = 'removeValidator'; export const DOMAIN_VALIDATOR_LOOKUP = 'lookupValidator'; export const DOMAIN_VALIDATOR_TEXTCHOICE = 'textChoiceValidator'; +export const DOMAIN_FIELD_TEXTCHOICE_MULTI = 'textChoiceAllowMulti'; export const DOMAIN_VALIDATOR_BOLD = 'bold'; export const DOMAIN_VALIDATOR_ITALIC = 'italic'; diff --git a/packages/components/src/internal/components/domainproperties/models.test.ts b/packages/components/src/internal/components/domainproperties/models.test.ts index e2b16df8e7..7004e96303 100644 --- a/packages/components/src/internal/components/domainproperties/models.test.ts +++ b/packages/components/src/internal/components/domainproperties/models.test.ts @@ -504,6 +504,8 @@ describe('PropDescType', () => { expect(isPropertyTypeAllowed(true, VISIT_DATE_TYPE, true, true)).toBeTruthy(); expect(isPropertyTypeAllowed(true, VISIT_ID_TYPE, true, false)).toBeFalsy(); expect(isPropertyTypeAllowed(true, VISIT_ID_TYPE, true, true)).toBeTruthy(); + expect(isPropertyTypeAllowed(true, MULTI_CHOICE_TYPE, true, true)).toBeFalsy(); + expect(isPropertyTypeAllowed(false, MULTI_CHOICE_TYPE, true, true)).toBeFalsy(); expect(isPropertyTypeAllowed(false, VISIT_ID_TYPE, true, true)).toBeTruthy(); expect(isPropertyTypeAllowed(false, VISIT_ID_TYPE, true, false)).toBeFalsy(); expect(isPropertyTypeAllowed(false, FILE_TYPE, false, false)).toBeFalsy(); diff --git a/packages/components/src/internal/components/domainproperties/models.tsx b/packages/components/src/internal/components/domainproperties/models.tsx index 91a6070049..4f2fcc6d20 100644 --- a/packages/components/src/internal/components/domainproperties/models.tsx +++ b/packages/components/src/internal/components/domainproperties/models.tsx @@ -1740,6 +1740,8 @@ export function isPropertyTypeAllowed( showFilePropertyType: boolean, showStudyPropertyTypes: boolean ): boolean { + if (type === MULTI_CHOICE_TYPE) return false; + if (type === FILE_TYPE) return showFilePropertyType; if (STUDY_PROPERTY_TYPES.includes(type)) return showStudyPropertyTypes; diff --git a/packages/components/src/internal/components/search/utils.test.ts b/packages/components/src/internal/components/search/utils.test.ts index 52be535f1f..a34124894d 100644 --- a/packages/components/src/internal/components/search/utils.test.ts +++ b/packages/components/src/internal/components/search/utils.test.ts @@ -575,6 +575,7 @@ describe('isEmptyValue', () => { const distinctValues = ['[All]', '[blank]', 'ed', 'ned', 'ted', 'red', 'bed']; const distinctValuesNoBlank = ['[All]', 'ed', 'ned', 'ted', 'red', 'bed']; const distinctValuesExcludeAll = ['[blank]', 'ed', 'ned', 'ted', 'red', 'bed']; +const distinctValuesExcludeBlankAll = ['ed', 'ned', 'ted', 'red', 'bed']; const fieldKey = 'thing'; const checkedOne = Filter.create(fieldKey, 'ed'); @@ -901,6 +902,47 @@ describe('getUpdatedChooseValuesFilter', () => { 'isnonblank' ); }); + + test('check all, array', () => { + validate( + getUpdatedChooseValuesFilter(distinctValuesNoBlank, fieldKey, ALL_VALUE_DISPLAY, true, arrayContainsAll), + 'arraycontainsall', + distinctValuesExcludeBlankAll + ); + }); + + test('check some value, array', () => { + validate( + getUpdatedChooseValuesFilter(distinctValuesNoBlank, fieldKey, 'ed', true, arrayContainsAll), + 'arraycontainsall', + ['red', 'ted', 'ned', 'ed'] + ); + }); + + test('check some value, array', () => { + validate( + getUpdatedChooseValuesFilter(distinctValuesNoBlank, fieldKey, 'ted', false, arrayContainsAll), + 'arraycontainsall', + ['red', 'ned'] + ); + }); + + test('check all values one by one, array', () => { + const updated = getUpdatedChooseValuesFilter(distinctValuesNoBlank, fieldKey, 'ed', true, arrayContainsAll); + const allChecked = getUpdatedChooseValuesFilter(distinctValuesNoBlank, fieldKey, 'bed', true, updated); + validate( + allChecked, + 'arraycontainsall', + distinctValuesExcludeBlankAll + ); + const allCheckedThenCheckAll = getUpdatedChooseValuesFilter(distinctValuesNoBlank, fieldKey, ALL_VALUE_DISPLAY, true, allChecked); + validate( + allCheckedThenCheckAll, + 'arraycontainsall', + distinctValuesExcludeBlankAll + ); + }); + }); describe('isValidFilterField', () => { diff --git a/packages/components/src/internal/components/search/utils.ts b/packages/components/src/internal/components/search/utils.ts index 9ae44bd1b8..19a0fad48c 100644 --- a/packages/components/src/internal/components/search/utils.ts +++ b/packages/components/src/internal/components/search/utils.ts @@ -412,6 +412,9 @@ export function getUpdatedChooseValuesFilter( if (isArrayFilter) { if ((newValue === ALL_VALUE_DISPLAY && !check) || newCheckedValues.length === 0) return Filter.create(fieldKey, [], oldFilter.getFilterType()); + if (allValues && newCheckedValues.length >= allValues.length) { + return Filter.create(fieldKey, allValues.filter(v => v !== ALL_VALUE_DISPLAY), oldFilter.getFilterType()); + } return Filter.create(fieldKey, newCheckedValues, oldFilter.getFilterType()); }