Skip to content

Commit

Permalink
fix: linebreaks are not imported into rich text fields
Browse files Browse the repository at this point in the history 
Refs: #63
  • Loading branch information
@Jumas
Jumas committed Nov 28, 2024
1 parent 96db01d commit bbd1cc5
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 38 deletions.
6 changes: 6 additions & 0 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.polarion.thirdparty</groupId>
<artifactId>net.htmlparser.jericho_3.1.0</artifactId>
<version>${polarion.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
56 changes: 36 additions & 20 deletions src/main/java/ch/sbb/polarion/extension/excel_importer/service/ImportService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -112,30 +113,44 @@ private String getIdentifierValue(Map<String, Object> recordMap, String columnLe
return stringIdValue;
}

private void fillWorkItemFields(IWorkItem workItem, Map<String, Object> mappingRecord, ExcelSheetMappingSettingsModel model, String linkColumnId) {
private void fillWorkItemFields(@NotNull IWorkItem workItem, Map<String, Object> mappingRecord, ExcelSheetMappingSettingsModel model, @NotNull String linkColumnId) {
Set<FieldMetadata> 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<FieldMetadata> 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<FieldMetadata> 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));
Expand All @@ -150,8 +165,7 @@ boolean ensureValidValue(String fieldId, Object value, Set<FieldMetadata> fieldM
*/
@SuppressWarnings("java:S1125") //will be improved later
@VisibleForTesting
boolean existingValueDiffers(IWorkItem workItem, String fieldId, Object newValue, Set<FieldMetadata> 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;
Expand All @@ -169,7 +183,9 @@ boolean existingValueDiffers(IWorkItem workItem, String fieldId, Object newValue
}
}

private FieldMetadata getFieldMetadataForField(Set<FieldMetadata> fieldMetadataSet, String fieldId) {
@NotNull
@VisibleForTesting
FieldMetadata getFieldMetadataForField(Set<FieldMetadata> fieldMetadataSet, String fieldId) {
return fieldMetadataSet.stream()
.filter(m -> m.getId().equals(fieldId))
.findFirst()
Expand Down
52 changes: 34 additions & 18 deletions src/test/java/ch/sbb/polarion/extension/excel_importer/service/ImportServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<br/>bbb", service.preProcessValue("aaa\nbbb", richMetadata));
assertEquals("aaa&nbsp;&nbsp;&nbsp;&nbsp;bbb", service.preProcessValue("aaa\tbbb", richMetadata));
assertEquals("&lt;tag&gt;&amp;", service.preProcessValue("<tag>&", 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<FieldMetadata> 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<FieldMetadata> 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));
Expand All @@ -311,7 +327,7 @@ void testExistingValueDiffers() {
when(polarionService.getFieldValue(workItem, "fieldId")).thenReturn("someValue");
assertTrue(service.existingValueDiffers(workItem, "fieldId", "someValue ", stringMetadata));

Set<FieldMetadata> 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
Expand All @@ -327,7 +343,7 @@ void testExistingValueDiffers() {
assertTrue(service.existingValueDiffers(workItem, "fieldId", "true", booleanMetadata));
assertFalse(service.existingValueDiffers(workItem, "fieldId", "false", booleanMetadata));

Set<FieldMetadata> 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));
Expand Down

0 comments on commit bbd1cc5

Please sign in to comment.