From bbd1cc53d37cf5fd84328eaf9d6f66da9d8b7e6e Mon Sep 17 00:00:00 2001 From: e551763 Date: Thu, 28 Nov 2024 18:40:21 +0100 Subject: [PATCH] fix: linebreaks are not imported into rich text fields Refs: #63 --- pom.xml | 6 ++ .../excel_importer/service/ImportService.java | 56 ++++++++++++------- .../service/ImportServiceTest.java | 52 +++++++++++------ 3 files changed, 76 insertions(+), 38 deletions(-) diff --git a/pom.xml b/pom.xml index ad325b0..e74330b 100644 --- a/pom.xml +++ b/pom.xml @@ -83,6 +83,12 @@ test-jar test + + com.polarion.thirdparty + net.htmlparser.jericho_3.1.0 + ${polarion.version} + test + diff --git a/src/main/java/ch/sbb/polarion/extension/excel_importer/service/ImportService.java b/src/main/java/ch/sbb/polarion/extension/excel_importer/service/ImportService.java index ce722e3..b5202f3 100644 --- a/src/main/java/ch/sbb/polarion/extension/excel_importer/service/ImportService.java +++ b/src/main/java/ch/sbb/polarion/extension/excel_importer/service/ImportService.java @@ -10,6 +10,7 @@ import com.polarion.alm.tracker.model.ITypeOpt; import com.polarion.alm.tracker.model.IWorkItem; import com.polarion.core.util.StringUtils; +import com.polarion.core.util.xml.HTMLHelper; import com.polarion.subterra.base.data.model.IType; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.VisibleForTesting; @@ -112,30 +113,44 @@ private String getIdentifierValue(Map recordMap, String columnLe return stringIdValue; } - private void fillWorkItemFields(IWorkItem workItem, Map mappingRecord, ExcelSheetMappingSettingsModel model, String linkColumnId) { + private void fillWorkItemFields(@NotNull IWorkItem workItem, Map mappingRecord, ExcelSheetMappingSettingsModel model, @NotNull String linkColumnId) { + Set fieldMetadataSet = polarionServiceExt.getWorkItemsFields(workItem.getProjectId(), workItem.getType() == null ? "" : workItem.getType().getId()); mappingRecord.forEach((columnId, value) -> { String fieldId = model.getColumnsMapping().get(columnId); - // we need to know possible mapped value asap because some types (at least boolean) need it to check value for modification - String mappedOption = OptionsMappingUtils.getMappedOptionKey(fieldId, value, model.getEnumsMapping()); - value = mappedOption != null ? mappedOption : value; - Set fieldMetadataSet = polarionServiceExt.getWorkItemsFields(workItem.getProjectId(), workItem.getType() == null ? "" : workItem.getType().getId()); - // The linkColumn field's value can't change, therefore it doesn't need to be overwritten. - // However, it must be saved to the newly created work item otherwise sequential imports will produce several objects. - if (fieldId != null && !fieldId.equals(IUniqueObject.KEY_ID) && (!fieldId.equals(linkColumnId) || !workItem.isPersisted()) && - (model.isOverwriteWithEmpty() || !isEmpty(value)) && - ensureValidValue(fieldId, value, fieldMetadataSet) && - existingValueDiffers(workItem, fieldId, value, fieldMetadataSet)) { - polarionServiceExt.setFieldValue(workItem, fieldId, value, model.getEnumsMapping()); - } else if (fieldId != null && fieldId.equals(IUniqueObject.KEY_ID) && !fieldId.equals(linkColumnId)) { - // If the work item id is imported, it must be the Link Column. Its value also can't be set by imported data unlike other possible Link Column fields. - throw new IllegalArgumentException("WorkItem id can only be imported if it is used as Link Column."); + if (fieldId != null) { + // we need to know possible mapped value asap because some types (at least boolean) need it to check value for modification + String mappedOption = OptionsMappingUtils.getMappedOptionKey(fieldId, value, model.getEnumsMapping()); + value = mappedOption != null ? mappedOption : value; + setFieldValue(value, getFieldMetadataForField(fieldMetadataSet, fieldId), workItem, model, linkColumnId); } }); } + private void setFieldValue(Object value, @NotNull FieldMetadata fieldMetadata, IWorkItem workItem, ExcelSheetMappingSettingsModel model, @NotNull String linkColumnId) { + // The linkColumn field's value can't change, therefore it doesn't need to be overwritten. + // However, it must be saved to the newly created work item otherwise sequential imports will produce several objects. + String fieldId = fieldMetadata.getId(); + if (!IUniqueObject.KEY_ID.equals(fieldId) && (!linkColumnId.equals(fieldId) || !workItem.isPersisted()) && + (model.isOverwriteWithEmpty() || !isEmpty(value)) && + ensureValidValue(value, fieldMetadata) && + existingValueDiffers(workItem, fieldId, value, fieldMetadata)) { + polarionServiceExt.setFieldValue(workItem, fieldId, preProcessValue(value, fieldMetadata), model.getEnumsMapping()); + } else if (IUniqueObject.KEY_ID.equals(fieldId) && !linkColumnId.equals(fieldId)) { + // If the work item id is imported, it must be the Link Column. Its value also can't be set by imported data unlike other possible Link Column fields. + throw new IllegalArgumentException("WorkItem id can only be imported if it is used as Link Column."); + } + } + @VisibleForTesting - boolean ensureValidValue(String fieldId, Object value, Set fieldMetadataSet) { - FieldMetadata fieldMetadata = getFieldMetadataForField(fieldMetadataSet, fieldId); + Object preProcessValue(Object value, @NotNull FieldMetadata fieldMetadata) { + if (FieldType.RICH.getType().equals(fieldMetadata.getType()) && value instanceof String richTextString) { + return HTMLHelper.convertPlainToHTML(richTextString); + } + return value; + } + + @VisibleForTesting + boolean ensureValidValue(Object value, FieldMetadata fieldMetadata) { if (FieldType.BOOLEAN.getType().equals(fieldMetadata.getType()) && (!(value instanceof String stringValue) || !("true".equalsIgnoreCase(stringValue) || "false".equalsIgnoreCase(stringValue)))) { throw new IllegalArgumentException(String.format("'%s' isn't a valid boolean value", value == null ? "" : value)); @@ -150,8 +165,7 @@ boolean ensureValidValue(String fieldId, Object value, Set fieldM */ @SuppressWarnings("java:S1125") //will be improved later @VisibleForTesting - boolean existingValueDiffers(IWorkItem workItem, String fieldId, Object newValue, Set fieldMetadataSet) { - FieldMetadata fieldMetadata = getFieldMetadataForField(fieldMetadataSet, fieldId); + boolean existingValueDiffers(IWorkItem workItem, String fieldId, Object newValue, FieldMetadata fieldMetadata) { Object existingValue = polarionServiceExt.getFieldValue(workItem, fieldId); if (existingValue == null && newValue == null) { return false; @@ -169,7 +183,9 @@ boolean existingValueDiffers(IWorkItem workItem, String fieldId, Object newValue } } - private FieldMetadata getFieldMetadataForField(Set fieldMetadataSet, String fieldId) { + @NotNull + @VisibleForTesting + FieldMetadata getFieldMetadataForField(Set fieldMetadataSet, String fieldId) { return fieldMetadataSet.stream() .filter(m -> m.getId().equals(fieldId)) .findFirst() diff --git a/src/test/java/ch/sbb/polarion/extension/excel_importer/service/ImportServiceTest.java b/src/test/java/ch/sbb/polarion/extension/excel_importer/service/ImportServiceTest.java index ce8659d..3f6bc15 100644 --- a/src/test/java/ch/sbb/polarion/extension/excel_importer/service/ImportServiceTest.java +++ b/src/test/java/ch/sbb/polarion/extension/excel_importer/service/ImportServiceTest.java @@ -260,42 +260,58 @@ void testImportViaId() { } } + @Test + void testPreProcessValue() { + ImportService service = new ImportService(mock(PolarionServiceExt.class)); + + FieldMetadata stringMetadata = FieldMetadata.builder().id("fieldId").type(FieldType.STRING.getType()).build(); + assertEquals("aaa\nbbb", service.preProcessValue("aaa\nbbb", stringMetadata)); + + // in case of rich text some characters must be converted to their HTML equivalents + FieldMetadata richMetadata = FieldMetadata.builder().id("fieldId").type(FieldType.RICH.getType()).build(); + assertEquals("aaa
bbb", service.preProcessValue("aaa\nbbb", richMetadata)); + assertEquals("aaa    bbb", service.preProcessValue("aaa\tbbb", richMetadata)); + assertEquals("<tag>&", service.preProcessValue("&", richMetadata)); + } + @Test void testEnsureValidValue() { ImportService service = new ImportService(mock(PolarionServiceExt.class)); - IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> service.ensureValidValue("someField", "true", Set.of())); - assertEquals("Cannot find field metadata for ID 'someField'", exception.getMessage()); + FieldMetadata metadata = mock(FieldMetadata.class); + when(metadata.getType()).thenReturn(FieldType.BOOLEAN.getType()); - Set metadataSet = Set.of(FieldMetadata.builder() - .id("someField") - .type(FieldType.BOOLEAN.getType()) - .build()); - assertTrue(service.ensureValidValue("someField", "true", metadataSet)); - assertTrue(service.ensureValidValue("someField", "True", metadataSet)); - assertTrue(service.ensureValidValue("someField", "FALSE", metadataSet)); + assertTrue(service.ensureValidValue("true", metadata)); + assertTrue(service.ensureValidValue("True", metadata)); + assertTrue(service.ensureValidValue("FALSE", metadata)); - exception = assertThrows(IllegalArgumentException.class, () -> service.ensureValidValue("someField", "FALSE ", metadataSet)); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> service.ensureValidValue("FALSE ", metadata)); assertEquals("'FALSE ' isn't a valid boolean value", exception.getMessage()); - exception = assertThrows(IllegalArgumentException.class, () -> service.ensureValidValue("someField", 42, metadataSet)); + exception = assertThrows(IllegalArgumentException.class, () -> service.ensureValidValue(42, metadata)); assertEquals("'42' isn't a valid boolean value", exception.getMessage()); - exception = assertThrows(IllegalArgumentException.class, () -> service.ensureValidValue("someField", null, metadataSet)); + exception = assertThrows(IllegalArgumentException.class, () -> service.ensureValidValue(null, metadata)); assertEquals("'' isn't a valid boolean value", exception.getMessage()); } @Test - void testExistingValueDiffers() { + void testGetFieldMetadataForField() { PolarionServiceExt polarionService = mock(PolarionServiceExt.class); ImportService service = new ImportService(polarionService); - IWorkItem workItem = mock(IWorkItem.class); IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, - () -> service.existingValueDiffers(workItem, "unknownFieldId", "someValue", Set.of())); + () -> service.getFieldMetadataForField(Set.of(), "unknownFieldId")); assertEquals("Cannot find field metadata for ID 'unknownFieldId'", exception.getMessage()); + } + + @Test + void testExistingValueDiffers() { + PolarionServiceExt polarionService = mock(PolarionServiceExt.class); + ImportService service = new ImportService(polarionService); + IWorkItem workItem = mock(IWorkItem.class); - Set stringMetadata = Set.of(FieldMetadata.builder().id("fieldId").type(FieldType.STRING.getType()).build()); + FieldMetadata stringMetadata = FieldMetadata.builder().id("fieldId").type(FieldType.STRING.getType()).build(); when(polarionService.getFieldValue(workItem, "fieldId")).thenReturn(null); assertFalse(service.existingValueDiffers(workItem, "fieldId", null, stringMetadata)); @@ -311,7 +327,7 @@ void testExistingValueDiffers() { when(polarionService.getFieldValue(workItem, "fieldId")).thenReturn("someValue"); assertTrue(service.existingValueDiffers(workItem, "fieldId", "someValue ", stringMetadata)); - Set booleanMetadata = Set.of(FieldMetadata.builder().id("fieldId").type(FieldType.BOOLEAN.getType()).build()); + FieldMetadata booleanMetadata = FieldMetadata.builder().id("fieldId").type(FieldType.BOOLEAN.getType()).build(); when(polarionService.getFieldValue(workItem, "fieldId")).thenReturn(null); assertFalse(service.existingValueDiffers(workItem, "fieldId", null, booleanMetadata)); assertFalse(service.existingValueDiffers(workItem, "fieldId", "someValue", booleanMetadata)); // unrealistic scenario @@ -327,7 +343,7 @@ void testExistingValueDiffers() { assertTrue(service.existingValueDiffers(workItem, "fieldId", "true", booleanMetadata)); assertFalse(service.existingValueDiffers(workItem, "fieldId", "false", booleanMetadata)); - Set floatMetadata = Set.of(FieldMetadata.builder().id("fieldId").type(FieldType.FLOAT.getType()).build()); + FieldMetadata floatMetadata = FieldMetadata.builder().id("fieldId").type(FieldType.FLOAT.getType()).build(); when(polarionService.getFieldValue(workItem, "fieldId")).thenReturn(null); assertFalse(service.existingValueDiffers(workItem, "fieldId", null, floatMetadata)); assertTrue(service.existingValueDiffers(workItem, "fieldId", 0f, floatMetadata));