Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: linebreaks are not imported into rich text fields #65

Merged
merged 2 commits into from
Nov 29, 2024
Merged

fix: linebreaks are not imported into rich text fields #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bbd1cc5
fix: linebreaks are not imported into rich text fields
Jumas Nov 28, 2024
2c713c2
Merge branch 'main' into 63-linebreaks-are-not-imported
grigoriev Nov 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pom.xml
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading