diff --git a/api/src/org/labkey/api/exp/property/DomainUtil.java b/api/src/org/labkey/api/exp/property/DomainUtil.java index 32c2ec8e4cb..f2fcfae527d 100644 --- a/api/src/org/labkey/api/exp/property/DomainUtil.java +++ b/api/src/org/labkey/api/exp/property/DomainUtil.java @@ -37,6 +37,7 @@ import org.labkey.api.data.ContainerManager; import org.labkey.api.data.ContainerService; import org.labkey.api.data.CoreSchema; +import org.labkey.api.data.DatabaseIdentifier; import org.labkey.api.data.NameGenerator; import org.labkey.api.data.PHI; import org.labkey.api.data.PropertyStorageSpec; @@ -983,8 +984,9 @@ else if (PropertyType.MULTI_CHOICE.getTypeUri().equals(old.getRangeURI())) if (column != null) { var dialect = domainTable.getSchema().getSqlDialect(); + DatabaseIdentifier columnId = p.getPropertyDescriptor().getLegalSelectName(dialect); SQLFragment deletedArray = new SQLFragment("CAST(? AS TEXT[])").add(deletedValues.toArray(new String[0])); - SQLFragment columnFrag = new SQLFragment().appendIdentifier(column.getAlias()); + SQLFragment columnFrag = new SQLFragment().appendIdentifier(columnId); SQLFragment sql = new SQLFragment("SELECT 1 FROM ") .append(domainTable) @@ -1536,6 +1538,11 @@ private static boolean _copyValidator(IPropertyValidator pv, GWTPropertyValidato { return "Text choice value for field '" + field.getName() + "' must not use the reserved format '{json:[...]}': '" + StringUtils.abbreviate(option, 50) + "'"; } + // GitHub Issue 951: Multi-line values converted to text choices lose multi-line editability + if (StringUtils.containsAny(option, "\n\r")) + { + return "Text choice value for field '" + field.getName() + "' must not contain newline characters: '" + StringUtils.abbreviate(option, 50) + "'"; + } } } } diff --git a/core/package-lock.json b/core/package-lock.json index b73c7089e77..33e8607512a 100644 --- a/core/package-lock.json +++ b/core/package-lock.json @@ -8,7 +8,7 @@ "name": "labkey-core", "version": "0.0.0", "dependencies": { - "@labkey/components": "7.26.4", + "@labkey/components": "7.26.5-fb-mvtcBatch3.1", "@labkey/themes": "1.8.0" }, "devDependencies": { @@ -3716,9 +3716,9 @@ } }, "node_modules/@labkey/api": { - "version": "1.51.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.51.0.tgz", - "integrity": "sha512-pyYXCNbFF3HZQ8KVapcwBl/7cHlVDz0ysPlCRBUQ9czX6s2yLwiho0OWkBd9MpbGVkqGFihbTUsUAU+Y5rz5Pw==", + "version": "1.51.1-mvtcBatch3.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.51.1-mvtcBatch3.1.tgz", + "integrity": "sha512-HB/2tcDrlYiIxKewtqoWsHkFOBdXdndbabq8GkIJ8yzv0z08PNfzT50r34QTCHPNQM//0Sd4WIymYYSCEfOf4g==", "license": "Apache-2.0" }, "node_modules/@labkey/build": { @@ -3759,13 +3759,13 @@ } }, "node_modules/@labkey/components": { - "version": "7.26.4", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.26.4.tgz", - "integrity": "sha512-pJ/P/qEiOdV2QYtLC+aDHcxUlroo5ENkYToYqb83+N4ksgmRYmv4oaTi0sIvXH0xSpRn3b31qys7gcHbL0yIbA==", + "version": "7.26.5-fb-mvtcBatch3.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.26.5-fb-mvtcBatch3.1.tgz", + "integrity": "sha512-L5TPDzeL5heHUYZ9rr3rW8ZasjZ8tRFvLZMhmCqrRbQRIVQZesioQ1s2BW5I5Z5huXKxKKAVzNyuH3ceQjK7+w==", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", - "@labkey/api": "1.51.0", + "@labkey/api": "1.51.1-mvtcBatch3.1", "@testing-library/dom": "~10.4.1", "@testing-library/jest-dom": "~6.9.1", "@testing-library/react": "~16.3.2", diff --git a/core/package.json b/core/package.json index f36db022851..3e0150f45e5 100644 --- a/core/package.json +++ b/core/package.json @@ -53,7 +53,7 @@ } }, "dependencies": { - "@labkey/components": "7.26.4", + "@labkey/components": "7.26.5-fb-mvtcBatch3.1", "@labkey/themes": "1.8.0" }, "devDependencies": { diff --git a/experiment/package-lock.json b/experiment/package-lock.json index 06fb01a56d0..95a177e78b3 100644 --- a/experiment/package-lock.json +++ b/experiment/package-lock.json @@ -8,7 +8,7 @@ "name": "experiment", "version": "0.0.0", "dependencies": { - "@labkey/components": "7.26.4" + "@labkey/components": "7.26.5-fb-mvtcBatch3.1" }, "devDependencies": { "@labkey/build": "9.1.0", @@ -3566,9 +3566,9 @@ } }, "node_modules/@labkey/api": { - "version": "1.51.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.51.0.tgz", - "integrity": "sha512-pyYXCNbFF3HZQ8KVapcwBl/7cHlVDz0ysPlCRBUQ9czX6s2yLwiho0OWkBd9MpbGVkqGFihbTUsUAU+Y5rz5Pw==", + "version": "1.51.1-mvtcBatch3.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.51.1-mvtcBatch3.1.tgz", + "integrity": "sha512-HB/2tcDrlYiIxKewtqoWsHkFOBdXdndbabq8GkIJ8yzv0z08PNfzT50r34QTCHPNQM//0Sd4WIymYYSCEfOf4g==", "license": "Apache-2.0" }, "node_modules/@labkey/build": { @@ -3609,13 +3609,13 @@ } }, "node_modules/@labkey/components": { - "version": "7.26.4", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.26.4.tgz", - "integrity": "sha512-pJ/P/qEiOdV2QYtLC+aDHcxUlroo5ENkYToYqb83+N4ksgmRYmv4oaTi0sIvXH0xSpRn3b31qys7gcHbL0yIbA==", + "version": "7.26.5-fb-mvtcBatch3.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-7.26.5-fb-mvtcBatch3.1.tgz", + "integrity": "sha512-L5TPDzeL5heHUYZ9rr3rW8ZasjZ8tRFvLZMhmCqrRbQRIVQZesioQ1s2BW5I5Z5huXKxKKAVzNyuH3ceQjK7+w==", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", - "@labkey/api": "1.51.0", + "@labkey/api": "1.51.1-mvtcBatch3.1", "@testing-library/dom": "~10.4.1", "@testing-library/jest-dom": "~6.9.1", "@testing-library/react": "~16.3.2", diff --git a/experiment/package.json b/experiment/package.json index d6898a03485..18c1327df1e 100644 --- a/experiment/package.json +++ b/experiment/package.json @@ -13,7 +13,7 @@ "test-integration": "cross-env NODE_ENV=test jest --ci --runInBand -c test/js/jest.config.integration.js" }, "dependencies": { - "@labkey/components": "7.26.4" + "@labkey/components": "7.26.5-fb-mvtcBatch3.1" }, "devDependencies": { "@labkey/build": "9.1.0", diff --git a/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts b/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts index 8c13782acc4..59b6923397c 100644 --- a/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts +++ b/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts @@ -5,6 +5,7 @@ import { AssayDesignFieldOptions, createAssayDesign, createDomainField, + generateFieldNameForImport, getAuditLogsForTransaction, getRunQueryRow, ImportRunOptions, @@ -34,7 +35,7 @@ const RUN_TEXT_CHOICE_FIELD_NAME = 'runTextChoiceField'; const RESULT_FIELD_NAME = 'resultStringField'; const RESULT_FILE_FIELD_NAME = 'resultFileField'; const RESULT_TC_FIELD_NAME = 'resultTextChoiceField'; -const RESULT_MVTC_FIELD_NAME = 'resultMultiChoiceField'; +const RESULT_MVTC_FIELD_NAME = 'resultMC ' + generateFieldNameForImport(); let context; let ASSAY_A_ID: number; @@ -46,7 +47,6 @@ let supportMultiChoice = false; beforeAll(async () => { context = await initProject(server, PROJECT_NAME, ASSAY_DESIGNER_ROLE, ['assay', 'experiment']); - let supportMultiChoice = false; const createTestPayload = { kind: 'DataClass', domainDesign: { name: 'Test_mvtc_support_check', fields: [{ name: 'Prop' }] }, @@ -67,8 +67,10 @@ beforeAll(async () => { createDomainField({name: RESULT_TC_FIELD_NAME, ...TC_FIELD_PROP} as Partial) ]; - if (supportMultiChoice) + if (supportMultiChoice) { + console.log("Assay result MVTC field name: " + RESULT_MVTC_FIELD_NAME); resultFields.push(createDomainField({name: RESULT_MVTC_FIELD_NAME, ...MVTC_FIELD_PROP} as Partial)); + } let assayFields: AssayDesignFieldOptions = { batchFields: [ diff --git a/experiment/src/client/test/integration/DataClassCrud.ispec.ts b/experiment/src/client/test/integration/DataClassCrud.ispec.ts index dae73a5380c..bc9e3607b69 100644 --- a/experiment/src/client/test/integration/DataClassCrud.ispec.ts +++ b/experiment/src/client/test/integration/DataClassCrud.ispec.ts @@ -801,6 +801,83 @@ describe('Multi Value Text Choice', () => { result = await getDataClassDataByName(dataNameImported[0], dataType, '*', topFolderOptions, editorUserOptions); expect(caseInsensitive(result, fieldName)).toEqual('Abnormal, Plasma'); // convert from ['Abnormal', 'Plasma'] to 'Abnormal, Plasma' + + const textChoiceMultiLineOption = { + propertyValidators: [ + { + "type": "TextChoice", + "name": "Text Choice Validator", + "new": true, + "expression": "Abnormal|multi\nline|cDNA|Plasma" + } + ], + rangeURI: 'http://www.w3.org/2001/XMLSchema#string', + conceptURI: 'http://www.labkey.org/types#textChoice', + } + + const textMultiChoiceMultiLineOption = { + propertyValidators: [ + { + "type": "TextChoice", + "name": "Text Choice Validator", + "new": true, + "expression": "Abnormal|multi\nline|cDNA|Plasma" + } + ], + rangeURI: "http://cpas.fhcrc.org/exp/xml#multiChoice", + } + + // GitHub Issue 951: Multi-line values converted to text choices lose multi-line editability + // verify cannot convert MultiLine field to MultiValue Text Choice + updatePayload = { + domainId, + domainDesign: { + name: dataType, + fields: [ + { + ...textChoiceMultiLineOption, + name: fieldName, + propertyId, + propertyURI, + } + ], + domainId, + domainURI + }, + options: { + rowId: dataClassRowId, + name: dataType, + nameExpression: 'S-${' + fieldNameInExpression + '}' + } + }; + failedUpdate = await server.post('property', 'saveDomain', updatePayload, {...topFolderOptions, ...adminOptions}); + expect(failedUpdate?.['body']?.['exception']).toContain('must not contain newline characters'); + + // verify cannot convert MultiLine field to Text Choice field + updatePayload = { + domainId, + domainDesign: { + name: dataType, + fields: [ + { + ...textMultiChoiceMultiLineOption, + name: fieldName, + propertyId, + propertyURI, + } + ], + domainId, + domainURI + }, + options: { + rowId: dataClassRowId, + name: dataType, + nameExpression: 'S-${genId}' + } + }; + failedUpdate = await server.post('property', 'saveDomain', updatePayload, {...topFolderOptions, ...adminOptions}); + expect(failedUpdate?.['body']?.['exception']).toContain('must not contain newline characters'); + }); }); diff --git a/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java b/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java index aa530cf932f..af7734719c9 100644 --- a/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java @@ -68,6 +68,8 @@ import java.util.Objects; import java.util.Set; +import static org.labkey.api.data.ColumnRenderPropertiesImpl.TEXT_CHOICE_CONCEPT_URI; + public class DomainPropertyImpl implements DomainProperty { private final DomainImpl _domain; @@ -859,6 +861,13 @@ else if (newType == PropertyType.MULTI_CHOICE || oldType == PropertyType.MULTI_C if (PropertyType.FILE_LINK.getInputType().equalsIgnoreCase(oldType.getInputType()) && oldType != newType) throw new ChangePropertyDescriptorException("Cannot convert an instance of " + oldType.name() + " to " + newType.name() + "."); + // GitHub Issue 951: Multi-line values converted to text choices lose multi-line editability + if (oldType == PropertyType.MULTI_LINE && + (PropertyType.MULTI_CHOICE == newType ||TEXT_CHOICE_CONCEPT_URI.equals(_pd.getConceptURI()))) + { + throw new ChangePropertyDescriptorException("Cannot convert a multiline text field to a text choice field."); + } + OntologyManager.validatePropertyDescriptor(_pd); Table.update(user, OntologyManager.getTinfoPropertyDescriptor(), _pd, _pdOld.getPropertyId()); OntologyManager.ensurePropertyDomain(_pd, dd, sortOrder);