diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index 84dcc15273..2c8313cea4 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,16 +1,16 @@ { "name": "@labkey/components", - "version": "7.15.0", + "version": "7.15.1-fb-mvtc-convert.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.15.0", + "version": "7.15.1-fb-mvtc-convert.1", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", - "@labkey/api": "1.46.0", + "@labkey/api": "1.46.1-fb-mvtc-convert.1", "@testing-library/dom": "~10.4.1", "@testing-library/jest-dom": "~6.9.1", "@testing-library/react": "~16.3.2", @@ -3845,9 +3845,9 @@ } }, "node_modules/@labkey/api": { - "version": "1.46.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.46.0.tgz", - "integrity": "sha512-QwSm82KBc9Hhd5GUDe99J35xvXIWBtbvFbXxJh81x6le62EZFQdptDoxJyKGpDQiv+cITlNEfvYXjY3CO0y1Kw==", + "version": "1.46.1-fb-mvtc-convert.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.46.1-fb-mvtc-convert.1.tgz", + "integrity": "sha512-i5uiP2ueHs+3PzRdD0wdGBKCGAOD0+CCKVgoIBE6bz0KI+ncbeBxIJXc4n+dAGVvnxxXnLoyUo7Xd6oQ/+2mhQ==", "license": "Apache-2.0" }, "node_modules/@labkey/build": { diff --git a/packages/components/package.json b/packages/components/package.json index 9d8d2a2b6e..166645248b 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.15.0", + "version": "7.15.1-fb-mvtc-convert.1", "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.46.0", + "@labkey/api": "1.46.1-fb-mvtc-convert.1", "@testing-library/dom": "~10.4.1", "@testing-library/jest-dom": "~6.9.1", "@testing-library/react": "~16.3.2", diff --git a/packages/components/releaseNotes/components.md b/packages/components/releaseNotes/components.md index aceaf1578e..0354052eef 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.15.0 *Released*: 4 February 2026 - Package updates diff --git a/packages/components/src/internal/components/domainproperties/AdvancedSettings.tsx b/packages/components/src/internal/components/domainproperties/AdvancedSettings.tsx index d1b1485553..d0b6596a48 100644 --- a/packages/components/src/internal/components/domainproperties/AdvancedSettings.tsx +++ b/packages/components/src/internal/components/domainproperties/AdvancedSettings.tsx @@ -280,6 +280,8 @@ export class AdvancedSettings extends React.PureComponent { + 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..698542bdfc 100644 --- a/packages/components/src/internal/components/domainproperties/ConfirmDataTypeChangeModal.tsx +++ b/packages/components/src/internal/components/domainproperties/ConfirmDataTypeChangeModal.tsx @@ -11,25 +11,41 @@ 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 || + oldMultiChoice; 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 +59,11 @@ export const ConfirmDataTypeChangeModal: FC = memo(props => { return (
This change will convert the values in the field from{' '} @@ -67,7 +83,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..b38ea5e992 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..3a1caa264e 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, @@ -230,6 +234,7 @@ export class DomainRowExpandedOptions extends React.Component { return ( { schemaName={schemaName} lockedForDomain={domainFormDisplayOptions.textChoiceLockedForDomain} lockedSqlFragment={domainFormDisplayOptions.textChoiceLockedSqlFragment} + handleDataTypeChange={handleDataTypeChange} + allowMultiChoice={allowMultiChoiceField} /> ); case 'fileLink': diff --git a/packages/components/src/internal/components/domainproperties/NameAndLinkingOptions.test.tsx b/packages/components/src/internal/components/domainproperties/NameAndLinkingOptions.test.tsx index e17502eac4..7ee106ef94 100644 --- a/packages/components/src/internal/components/domainproperties/NameAndLinkingOptions.test.tsx +++ b/packages/components/src/internal/components/domainproperties/NameAndLinkingOptions.test.tsx @@ -11,6 +11,7 @@ import { DOMAIN_FIELD_ONTOLOGY_PRINCIPAL_CONCEPT, DOMAIN_FIELD_URL, DOMAIN_FIELD_URL_TARGET, + MULTI_CHOICE_RANGE_URI, STORAGE_UNIQUE_ID_CONCEPT_URI, STRING_RANGE_URI, } from './constants'; @@ -71,6 +72,14 @@ const DEFAULT_PROPS = { serverModuleNames: undefined, }; +const MULTI_CHOICE_FIELD = DomainField.create({ + name: 'mvtcField', + rangeURI: MULTI_CHOICE_RANGE_URI, + propertyId: 5, + description: 'array', + label: 'multi value', +}); + describe('NameAndLinkingOptions', () => { test('Name and Linking options', () => { const container = render().container; @@ -180,4 +189,13 @@ describe('NameAndLinkingOptions', () => { document.querySelector('#' + createFormInputId(DOMAIN_FIELD_URL_TARGET, 1, 1)).hasAttribute('disabled') ).toEqual(true); }); + + test('multi value text choice field', () => { + render(); + expect(document.querySelectorAll('#' + createFormInputId(DOMAIN_FIELD_LABEL, 1, 1))).toHaveLength(1); + expect(document.querySelectorAll('#' + createFormInputId(DOMAIN_FIELD_DESCRIPTION, 1, 1))).toHaveLength(1); + expect(document.querySelectorAll('#' + createFormInputId(DOMAIN_FIELD_IMPORTALIASES, 1, 1))).toHaveLength(1); + expect(document.querySelectorAll('#' + createFormInputId(DOMAIN_FIELD_URL, 1, 1))).toHaveLength(0); + expect(document.querySelectorAll('#' + createFormInputId(DOMAIN_FIELD_URL_TARGET, 1, 1))).toHaveLength(0); + }); }); diff --git a/packages/components/src/internal/components/domainproperties/NameAndLinkingOptions.tsx b/packages/components/src/internal/components/domainproperties/NameAndLinkingOptions.tsx index 4f2e195824..ec229081c1 100644 --- a/packages/components/src/internal/components/domainproperties/NameAndLinkingOptions.tsx +++ b/packages/components/src/internal/components/domainproperties/NameAndLinkingOptions.tsx @@ -92,6 +92,8 @@ export class NameAndLinkingOptions extends PureComponent { render(): ReactNode { const { index, field, domainIndex, appPropertiesOnly, domainFormDisplayOptions } = this.props; + const allowUrl = DomainField.allowConditionalFormats(field); + return (
@@ -145,43 +147,49 @@ export class NameAndLinkingOptions extends PureComponent { )}
-
- {!appPropertiesOnly && - hasModule(ONTOLOGY_MODULE_NAME) && - !field.isUniqueIdField() && - !field.isCalculatedField() && ( - - )} -
- -
- - {/*GitHub Issue 503: Field editor URL option to set target window (i.e. _blank)*/} -
+ {allowUrl && ( +
+ {!appPropertiesOnly && + hasModule(ONTOLOGY_MODULE_NAME) && + !field.isUniqueIdField() && + !field.isCalculatedField() && ( + + )} +
+ +
- Open links in a new tab + {/*GitHub Issue 503: Field editor URL option to set target window (i.e. _blank)*/} +
+ + Open links in a new tab +
-
+ )}
); 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..dbc05aee17 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,11 +400,12 @@ 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, ...DEFAULT_TEXT_CHOICE_VALIDATOR.toJS(), + rowId: field.textChoiceValidator?.rowId, shouldShowWarning: true, expression: PropertyValidator.joinValidValues(newValues), properties: { validValues: newValues }, @@ -361,7 +413,7 @@ export const TextChoiceOptions: FC = memo(props => { }) ); }, - [field.textChoiceValidator, fieldId, onChange] + [field.textChoiceValidator, fieldValidatorId, onChange] ); useEffect( @@ -369,7 +421,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 +430,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 +450,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..7bdf505a79 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,36 @@ 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(), + URL: undefined, + }) as DomainField; + } } else if (field.isCalculatedField()) { field = field.merge({ importAliases: undefined, @@ -1365,17 +1385,52 @@ 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 +1441,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..cab31a419e 100644 --- a/packages/components/src/internal/components/domainproperties/models.tsx +++ b/packages/components/src/internal/components/domainproperties/models.tsx @@ -1422,6 +1422,10 @@ export class DomainField return !field.isMultiChoiceField(); } + static allowUrl(field: DomainField): boolean { + return !field.isMultiChoiceField(); + } + static hasRegExValidation(field: DomainField): boolean { return ( field.dataType.isString() && @@ -1740,6 +1744,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/editable/actions.ts b/packages/components/src/internal/components/editable/actions.ts index 6ff75cfdb2..f4adf3da62 100644 --- a/packages/components/src/internal/components/editable/actions.ts +++ b/packages/components/src/internal/components/editable/actions.ts @@ -35,7 +35,7 @@ import { import { decimalDifference, genCellKey, getLookupFilters, getValidatedEditableGridValue, parseCellKey } from './utils'; import { SchemaQuery } from '../../../public/SchemaQuery'; -import { naturalSort } from '../../../public/sort'; +import { caseSensitiveNaturalSort } from '../../../public/sort'; /** * Do not use this method directly, use initEditorModel instead. @@ -1524,7 +1524,7 @@ async function insertPastedData( cv = valueDescriptors; msg = message; } else if (col?.isMultiChoice && Utils.isString(val)) { - const parsedValues = parseCsvString(val, ',', true).sort(naturalSort); + const parsedValues = parseCsvString(val, ',', true).sort(caseSensitiveNaturalSort); const unmatched: string[] = []; const values = []; diff --git a/packages/components/src/internal/components/forms/input/SelectInput.tsx b/packages/components/src/internal/components/forms/input/SelectInput.tsx index 3d3d1bbe96..86648fc65f 100644 --- a/packages/components/src/internal/components/forms/input/SelectInput.tsx +++ b/packages/components/src/internal/components/forms/input/SelectInput.tsx @@ -482,7 +482,7 @@ export class SelectInputImpl extends Component { if (autoValue) { if (sortValues && Array.isArray(selectedOptions)) - selectedOptions.sort(naturalSortByProperty(valueKey ?? 'value')); + selectedOptions.sort(naturalSortByProperty(valueKey ?? 'value', true)); this.setState({ selectedOptions }); } diff --git a/packages/components/src/internal/components/search/FilterFacetedSelector.tsx b/packages/components/src/internal/components/search/FilterFacetedSelector.tsx index 8f93c27852..7497375235 100644 --- a/packages/components/src/internal/components/search/FilterFacetedSelector.tsx +++ b/packages/components/src/internal/components/search/FilterFacetedSelector.tsx @@ -254,7 +254,7 @@ export const FilterFacetedSelector: FC = memo(props => { const newActiveFilterType = filterOptions.find(option => option.value === filterUrlSuffix); if (newActiveFilterType) { const filterType = resolveFilterType(newActiveFilterType?.value, field); - let updatedFilterValues = fieldFilters[0]?.getValue(); + let updatedFilterValues = fieldFilters?.[0]?.getValue(); const shouldClearUpdatedFilterValues = updatedFilterValues && multiChoices && !filterType.isMultiValued(); if (shouldClearUpdatedFilterValues) updatedFilterValues = null; @@ -267,6 +267,10 @@ export const FilterFacetedSelector: FC = memo(props => { [onFieldFilterUpdate, filterOptions, field, fieldFilters, fieldKey, multiChoices] ); + const hideOptions = useMemo(() => { + return !!multiChoices && fieldFilters?.[0] && !fieldFilters?.[0]?.getFilterType().isDataValueRequired(); + }, [fieldFilters, multiChoices]); + if (!fieldDistinctValues || allShown === undefined) return ; return ( @@ -313,7 +317,7 @@ export const FilterFacetedSelector: FC = memo(props => {
0 ? 6 : 12}`}> {loading && } - {!loading && ( + {(!loading && !hideOptions) && (
    {filteredFieldDistinctValues?.map((value, index) => { let displayValue = value; diff --git a/packages/components/src/internal/components/search/utils.test.ts b/packages/components/src/internal/components/search/utils.test.ts index 52be535f1f..939f812a13 100644 --- a/packages/components/src/internal/components/search/utils.test.ts +++ b/packages/components/src/internal/components/search/utils.test.ts @@ -174,6 +174,12 @@ describe('getFilterValuesAsArray', () => { ]); }); + test('array value, array filter type', () => { + expect( + getFilterValuesAsArray(Filter.create('textField', ['a', 'b'], Filter.Types.ARRAY_CONTAINS_ALL)) + ).toStrictEqual(['a', 'b']); + }); + test('string value', () => { expect(getFilterValuesAsArray(Filter.create('textField', 'a;b;c', Filter.Types.IN))).toStrictEqual([ 'a', @@ -182,6 +188,12 @@ describe('getFilterValuesAsArray', () => { ]); }); + test('string value, array filter type', () => { + expect( + getFilterValuesAsArray(Filter.create('textField', 'a;b;c', Filter.Types.ARRAY_CONTAINS_ALL)) + ).toStrictEqual(['a', 'b', 'c']); + }); + test('with blank value', () => { expect(getFilterValuesAsArray(Filter.create('textField', 'a;b;;c', Filter.Types.IN))).toStrictEqual([ 'a', @@ -575,6 +587,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 +914,44 @@ 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..02919977bf 100644 --- a/packages/components/src/internal/components/search/utils.ts +++ b/packages/components/src/internal/components/search/utils.ts @@ -412,6 +412,13 @@ 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()); } diff --git a/packages/components/src/internal/query/filter.test.ts b/packages/components/src/internal/query/filter.test.ts index bf368eaf26..ac79c218e4 100644 --- a/packages/components/src/internal/query/filter.test.ts +++ b/packages/components/src/internal/query/filter.test.ts @@ -534,6 +534,45 @@ describe('getFilterLabKeySql', () => { test('filter types not supported', () => { expect(getFilterLabKeySql(Filter.create('StringField', 'abc', Filter.Types.MEMBER_OF), 'string')).toBeNull(); }); + + test('array filters', () => { + expect( + getFilterLabKeySql(Filter.create('ArrayField', 'value1;value2', Filter.Types.ARRAY_CONTAINS_ALL), 'array') + ).toEqual("array_contains_all(\"ArrayField\", ARRAY['value1', 'value2'])"); + expect( + getFilterLabKeySql( + Filter.create('ArrayField', ['value1', 'value2'], Filter.Types.ARRAY_CONTAINS_ALL), + 'array' + ) + ).toEqual("array_contains_all(\"ArrayField\", ARRAY['value1', 'value2'])"); + expect( + getFilterLabKeySql( + Filter.create('ArrayField', ["val'ue1", 'value2'], Filter.Types.ARRAY_CONTAINS_ANY), + 'array' + ) + ).toEqual("array_contains_any(\"ArrayField\", ARRAY['val''ue1', 'value2'])"); + expect( + getFilterLabKeySql(Filter.create('ArrayField', 'value1', Filter.Types.ARRAY_CONTAINS_EXACT), 'array') + ).toEqual('array_is_same("ArrayField", ARRAY[\'value1\'])'); + expect( + getFilterLabKeySql( + Filter.create('ArrayField', ["val'ue1", 'value2'], Filter.Types.ARRAY_CONTAINS_NOT_EXACT), + 'array' + ) + ).toEqual("NOT array_is_same(\"ArrayField\", ARRAY['val''ue1', 'value2'])"); + expect( + getFilterLabKeySql( + Filter.create('ArrayField', ["val'ue1", 'value2'], Filter.Types.ARRAY_CONTAINS_NONE), + 'array' + ) + ).toEqual("array_contains_none(\"ArrayField\", ARRAY['val''ue1', 'value2'])"); + expect(getFilterLabKeySql(Filter.create('ArrayField', null, Filter.Types.ARRAY_ISEMPTY), 'array')).toEqual( + 'array_is_empty("ArrayField")' + ); + expect(getFilterLabKeySql(Filter.create('ArrayField', '', Filter.Types.ARRAY_ISNOTEMPTY), 'array')).toEqual( + 'NOT array_is_empty("ArrayField")' + ); + }); }); describe('getLegalIdentifier', () => { diff --git a/packages/components/src/internal/query/filter.ts b/packages/components/src/internal/query/filter.ts index 285a4bb940..69b6a9f896 100644 --- a/packages/components/src/internal/query/filter.ts +++ b/packages/components/src/internal/query/filter.ts @@ -375,6 +375,37 @@ export function isNegativeFilterType(filterType: Filter.IFilterType) { return NEGATIVE_FILTERS.indexOf(filterType.getURLSuffix()) > -1; } +function getArrayFilterLabKeySql(filter: Filter.IFilter, tableAlias?: string): string { + const filterType = filter.getFilterType().getURLSuffix(); + const columnNameSelect = getLegalIdentifier(filter.getColumnName(), tableAlias); + + if (filterType === Filter.Types.ARRAY_ISEMPTY.getURLSuffix()) return 'array_is_empty(' + columnNameSelect + ')'; + if (filterType === Filter.Types.ARRAY_ISNOTEMPTY.getURLSuffix()) + return 'NOT array_is_empty(' + columnNameSelect + ')'; + + const values = filter.getFilterType().parseValue(filter.getValue()); + + const sqlValues = []; + values.forEach(val => { + sqlValues.push(getLabKeySqlValue(val, 'string')); + }); + const sqlValueStr = 'ARRAY[' + sqlValues.join(', ') + ']'; + + if (filterType === Filter.Types.ARRAY_CONTAINS_ANY.getURLSuffix()) { + return 'array_contains_any(' + columnNameSelect + ', ' + sqlValueStr + ')'; + } else if (filterType === Filter.Types.ARRAY_CONTAINS_NONE.getURLSuffix()) { + return 'array_contains_none(' + columnNameSelect + ', ' + sqlValueStr + ')'; + } else if (filterType === Filter.Types.ARRAY_CONTAINS_ALL.getURLSuffix()) { + return 'array_contains_all(' + columnNameSelect + ', ' + sqlValueStr + ')'; + } else if (filterType === Filter.Types.ARRAY_CONTAINS_EXACT.getURLSuffix()) { + return 'array_is_same(' + columnNameSelect + ', ' + sqlValueStr + ')'; + } else if (filterType === Filter.Types.ARRAY_CONTAINS_NOT_EXACT.getURLSuffix()) { + return 'NOT array_is_same(' + columnNameSelect + ', ' + sqlValueStr + ')'; + } + + return null; +} + /** * Note: this is an experimental API that may change unexpectedly in future releases. * From a filter and its column jsonType, return the LabKey sql operator clause @@ -405,6 +436,10 @@ export function getFilterLabKeySql( ) return null; + if (jsonType === 'array') { + return getArrayFilterLabKeySql(filter, tableAlias); + } + if (jsonType === 'date' && filterType.isDataValueRequired()) { let dateValue: string; if (filterType.isMultiValued()) { diff --git a/packages/components/src/internal/renderers/MultiValueRenderer.tsx b/packages/components/src/internal/renderers/MultiValueRenderer.tsx index 28e04aed43..fff7ed9bc0 100644 --- a/packages/components/src/internal/renderers/MultiValueRenderer.tsx +++ b/packages/components/src/internal/renderers/MultiValueRenderer.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ import React, { FC, memo, ReactNode } from 'react'; -import { List, Map } from 'immutable'; +import { Iterable, List, Map } from 'immutable'; import { QueryColumn } from '../../public/QueryColumn'; import { FileColumnRenderer } from './FileColumnRenderer'; @@ -50,7 +50,8 @@ export const MultiValueRenderer: FC = memo(({ data, col text = item.get('formattedValue'); } else { const o = item.has('displayValue') ? item.get('displayValue') : item.get('value'); - text = o !== null && o !== undefined ? o.toString() : null; + if (Iterable.isIterable(o)) text = o.join(', '); + else text = o !== null && o !== undefined ? o.toString() : null; } url = item.get('url'); diff --git a/packages/components/src/public/sort.test.ts b/packages/components/src/public/sort.test.ts index c21f9b60d5..b954cb5bb1 100644 --- a/packages/components/src/public/sort.test.ts +++ b/packages/components/src/public/sort.test.ts @@ -1,4 +1,4 @@ -import { naturalSort } from './sort'; +import { caseSensitiveNaturalSort, naturalSort } from './sort'; describe('naturalSort', () => { test('alphabetic', () => { @@ -6,6 +6,7 @@ describe('naturalSort', () => { expect(naturalSort('anything', '')).toBe(-1); expect(naturalSort(undefined, 'anything')).toBe(1); expect(naturalSort('a', 'a')).toBe(0); + expect(naturalSort('a', 'A')).toBe(0); expect(naturalSort('alpha', 'aLPha')).toBe(0); expect(naturalSort(' ', 'anything')).toBe(-1); expect(naturalSort('a', 'b')).toBe(-1); @@ -30,3 +31,78 @@ describe('naturalSort', () => { expect(nonStringValues.sort(naturalSort).length).toEqual(nonStringValues.length); }); }); + +describe('caseSensitiveNaturalSort', () => { + test('case sensitive equality and ordering', () => { + // exact same string and case => equal + expect(caseSensitiveNaturalSort('alpha', 'alpha')).toBe(0); + + // same letters but different case => not equal (case-sensitive) + // 'l' (lowercase) has a higher char code than 'L' (uppercase), so 'alpha' should come after 'aLPha' + expect(caseSensitiveNaturalSort('alpha', 'aLPha')).toBeGreaterThan(0); + + // 'A' is less than 'a' in ASCII ordering + expect(caseSensitiveNaturalSort('A', 'a')).toBeLessThan(0); + expect(caseSensitiveNaturalSort('a', 'A')).toBeGreaterThan(0); + + expect(caseSensitiveNaturalSort('a', 'B')).toBeLessThan(0); + + + expect(['Abnormal', 'cDNA', 'Plasma', 'agent'].sort(caseSensitiveNaturalSort)).toEqual([ + 'Abnormal', + 'agent', + 'cDNA', + 'Plasma', + ]); + + expect(caseSensitiveNaturalSort('a', 'Abnormal')).toBeGreaterThan(0); + + expect(caseSensitiveNaturalSort('a', 'Plasma')).toBeLessThan(0); + + expect(['Abnormal', 'a'].sort(caseSensitiveNaturalSort)).toEqual([ + 'Abnormal', + 'a', + ]); + + expect(['Abnormal', 'cDNA', 'Plasma', 'a'].sort(caseSensitiveNaturalSort)).toEqual([ + 'Abnormal', + 'a', + 'cDNA', + 'Plasma', + ]); + }); + + test('numeric/alphanumeric behavior preserved', () => { + // numeric/alphanumeric comparison should still behave as natural sort + expect(caseSensitiveNaturalSort('a1.2', 'a1.3')).toBeLessThan(0); + expect(caseSensitiveNaturalSort('10', '1.0')).toBeGreaterThan(0); + }); + + + test('alphabetic', () => { + expect(caseSensitiveNaturalSort('', 'anything')).toBe(1); + expect(caseSensitiveNaturalSort('anything', '')).toBe(-1); + expect(caseSensitiveNaturalSort(undefined, 'anything')).toBe(1); + expect(caseSensitiveNaturalSort('a', 'a')).toBe(0); + + expect(caseSensitiveNaturalSort('alpha', 'aLPha')).toBeGreaterThan(0); + + expect(caseSensitiveNaturalSort(' ', 'anything')).toBeLessThan(0); + expect(caseSensitiveNaturalSort('a', 'b')).toBeLessThan(0); + expect(caseSensitiveNaturalSort('a', 'B')).toBeLessThan(0); + expect(caseSensitiveNaturalSort('A', 'b')).toBeLessThan(0); + expect(caseSensitiveNaturalSort('A', 'Z')).toBeLessThan(0); + expect(caseSensitiveNaturalSort('alpha', 'zeta')).toBeLessThan(0); + expect(caseSensitiveNaturalSort('zeta', 'atez')).toBeGreaterThan(0); + expect(caseSensitiveNaturalSort('Zeta', 'Atez')).toBeGreaterThan(0); + }); + + test('alphanumeric', () => { + expect(caseSensitiveNaturalSort('a1.2', 'a1.3')).toBeLessThan(0); + expect(caseSensitiveNaturalSort('1.431', '14.31')).toBeLessThan(0); + expect(caseSensitiveNaturalSort('10', '1.0')).toBeGreaterThan(0); + expect(caseSensitiveNaturalSort('1.2ABC', '1.2XY')).toBeLessThan(0); + }); + + +}); diff --git a/packages/components/src/public/sort.ts b/packages/components/src/public/sort.ts index f15b8d7573..3bb808c419 100644 --- a/packages/components/src/public/sort.ts +++ b/packages/components/src/public/sort.ts @@ -5,9 +5,10 @@ * For in-depth documentation and examples see components/docs/sort.md. * @param aso * @param bso + * @param caseSensitive */ // eslint-disable-next-line @typescript-eslint/no-explicit-any -export function naturalSort(aso: any, bso: any): number { +function _naturalSort(aso: any, bso: any, caseSensitive?: boolean): number { // http://stackoverflow.com/questions/19247495/alphanumeric-sorting-an-array-in-javascript if (aso === bso) return 0; if (aso === undefined || aso === null || aso === '') return 1; @@ -20,8 +21,10 @@ export function naturalSort(aso: any, bso: any): number { // If no matches are found in the group, then match() will return null const rx = /(\.\d+)|(\d+(\.\d+)?)|([^\d.]+)|(\.\D+)|(\.$)/g; - const a = aso.toString().toLowerCase().match(rx) ?? []; - const b = bso.toString().toLowerCase().match(rx) ?? []; + const asoStr = caseSensitive ? aso.toString() : aso.toString().toLowerCase(); + const bsoStr = caseSensitive ? bso.toString() : bso.toString().toLowerCase(); + const a = asoStr.match(rx) ?? []; + const b = bsoStr.match(rx) ?? []; const L = a.length; while (i < L) { @@ -31,12 +34,43 @@ export function naturalSort(aso: any, bso: any): number { if (a1 !== b1) { n = a1 - b1; if (!isNaN(n)) return n; + if (caseSensitive) { + // Compare character by character + const minLength = Math.min(a1.length, b1.length); + for (let charIndex = 0; charIndex < minLength; charIndex++) { + const charCodeA = a1.charCodeAt(charIndex); + const charCodeB = b1.charCodeAt(charIndex); + if (charCodeA !== charCodeB) { + const lowerCharA = String.fromCharCode(charCodeA).toLowerCase(); + const lowerCharB = String.fromCharCode(charCodeB).toLowerCase(); + // First compare lowercase versions to handle case-insensitive comparison + if (lowerCharA !== lowerCharB) { + return lowerCharA.charCodeAt(0) - lowerCharB.charCodeAt(0); + } + // If lowercase versions are equal, compare original char codes for case-sensitive ordering + if (charCodeA !== charCodeB) { + return charCodeA - charCodeB; + } + } + } + // If all characters so far are equal, the shorter string is "less than" the longer one + if (a1.length !== b1.length) + return a1.length - b1.length; + } return a1 > b1 ? 1 : -1; } } return b[i] ? -1 : 0; } +export function naturalSort(aso: any, bso: any): number { + return _naturalSort(aso, bso); +} + +export function caseSensitiveNaturalSort(aso: any, bso: any): number { + return _naturalSort(aso, bso, true); +} + type SortFn = (a: T, b: T) => number; /** @@ -45,8 +79,9 @@ type SortFn = (a: T, b: T) => number; * Ex:` * const myArray = [{ displayName: 'Nick' }, { displayName: 'Alan' }, { displayName: 'Susan' }]; * myArray.sort(naturalSortByProperty('displayName')); - * @param property: string, the property you want to sort on. + * @param property + * @param caseSensitive */ -export function naturalSortByProperty(property: keyof T): SortFn { - return (a, b) => naturalSort(a[property], b[property]); +export function naturalSortByProperty(property: keyof T, caseSensitive?: boolean): SortFn { + return (a, b) => (caseSensitive ? caseSensitiveNaturalSort : naturalSort)(a[property], b[property]); } diff --git a/packages/components/src/theme/domainproperties.scss b/packages/components/src/theme/domainproperties.scss index 98f1a5914f..f5c9d69cec 100644 --- a/packages/components/src/theme/domainproperties.scss +++ b/packages/components/src/theme/domainproperties.scss @@ -755,6 +755,7 @@ .list-group-item { overflow-wrap: anywhere; + white-space: pre-wrap; } } .domain-text-choices-error { diff --git a/packages/components/src/theme/grid.scss b/packages/components/src/theme/grid.scss index 4f3636fa7d..e7c93e00e6 100644 --- a/packages/components/src/theme/grid.scss +++ b/packages/components/src/theme/grid.scss @@ -395,7 +395,6 @@ $table-cell-padding: 4px 2px; } .cell-menu-value { - min-width: 200px; margin-right: 24px; }