Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a configurable secondary delimiter (app.secondaryDelimiter, default |) used by the Excel/CSV exporter to join multiple field values (e.g. keywords) within a single CSV cell, addressing issue #2122 where comma-joined multi-values were ambiguous in CSV output.
Changes:
- New
app.secondaryDelimiterproperty inapplication.ymland injected intoSubmissionController, passed throughPackagerUtilityto packagers during batch export. - New
packageExport(Submission, List<SubmissionListColumn>, String delimiter)method added to thePackagerinterface, with a defaultUnsupportedFormatterExceptionimplementation inAbstractPackagerand a concrete implementation inExcelPackager. SubmissionController#processBatchExportnow invokes the delimiter-aware overload.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/application.yml | Adds `app.secondaryDelimiter: ' |
| src/main/java/org/tdl/vireo/controller/SubmissionController.java | Injects secondaryDelimiter via @Value and passes it to packagerUtility.packageExport. |
| src/main/java/org/tdl/vireo/utility/PackagerUtility.java | Adds delegating overload that forwards the delimiter to the packager. |
| src/main/java/org/tdl/vireo/model/packager/Packager.java | Declares new delimiter-aware packageExport method on the interface. |
| src/main/java/org/tdl/vireo/model/packager/AbstractPackager.java | Default unsupported implementation throwing UnsupportedFormatterException. |
| src/main/java/org/tdl/vireo/model/packager/ExcelPackager.java | New ~75-line packageExport overload duplicating existing logic; intended to use the configurable delimiter when joining multi-valued predicate fields. |
Comments suppressed due to low confidence (1)
src/main/java/org/tdl/vireo/model/packager/ExcelPackager.java:157
- The predicate-matching block is logically inverted and will not produce the intended delimited output. When a
FieldValuematches the predicate, the row is written with a hardcoded", "join (line 153), and only on non-matching iterations is the configureddelimiterused (line 155). Since the configured delimiter is applied only when a field does not match, columns that contain only matching values (the common case for keywords) will be joined with", "and the secondary delimiter will never appear in the export.
Additionally, row.put(...) is called inside the inner for loop on every iteration, so the final value depends on whether the last field value in the submission matches or not, rather than on the accumulated list of matching values. The row.put should be performed once after the loop, joining only the matched fieldValues with delimiter + " " (mirroring the original method's structure but with the configurable delimiter).
for (FieldValue fieldValue : submission.getFieldValues()) {
if (fieldValue.getFieldPredicate().getValue().equals(predicate.get().trim())) {
fieldValues.add(fieldValue.getValue());
row.put(column.getTitle(), String.join(", ", fieldValues));
} else {
row.put(column.getTitle(), String.join(delimiter+" ", fieldValues));
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+143
to
+220
| @Override | ||
| public ExcelExportPackage packageExport(Submission submission, List<SubmissionListColumn> columns, String delimiter) { | ||
| Map<String, String> row = new HashMap<String, String>(); | ||
| columns.forEach(column -> { | ||
| Optional<String> predicate = Optional.ofNullable(column.getPredicate()); | ||
| if (predicate.isPresent()) { | ||
| List<String> fieldValues = new ArrayList<String>(); | ||
| for (FieldValue fieldValue : submission.getFieldValues()) { | ||
| if (fieldValue.getFieldPredicate().getValue().equals(predicate.get().trim())) { | ||
| fieldValues.add(fieldValue.getValue()); | ||
| row.put(column.getTitle(), String.join(", ", fieldValues)); | ||
| } else { | ||
| row.put(column.getTitle(), String.join(delimiter+" ", fieldValues)); | ||
| } | ||
| } | ||
| } else { | ||
| if (column.getValuePath().size() > 0) { | ||
| String[] valuePath = column.getValuePath().toArray(new String[column.getValuePath().size()]); | ||
| try { | ||
| if(column.getValuePath().size() > 1){ | ||
| valuePath = new String[] {valuePath[0]}; | ||
| } | ||
| submission.getCommitteeContactEmail(); | ||
| Object valueAsObject = EntityUtility.getValueFromPath(submission, valuePath); | ||
|
|
||
| String value = ""; | ||
|
|
||
| if (valueAsObject instanceof Calendar) { | ||
| Calendar calendar = (Calendar) valueAsObject; | ||
| value = simpleDateFormat.format(calendar.getTime()); | ||
| } else if (valueAsObject instanceof Date) { | ||
| Date date = (Date) valueAsObject; | ||
| value = simpleDateFormat.format(date); | ||
| } else if (valueAsObject instanceof Set && ((Set<?>) valueAsObject).stream().allMatch(o -> o instanceof CustomActionValue)) { | ||
| StringBuilder sb = new StringBuilder(); | ||
| ((Set<?>) valueAsObject).forEach(o -> { | ||
| CustomActionValue customActionValue = (CustomActionValue) o; | ||
| if (customActionValue.getValue()) { | ||
| sb.append("☑ "); | ||
| } else { | ||
| sb.append("☐ "); | ||
| } | ||
| sb.append(customActionValue.getDefinition().getLabel()+"\n"); | ||
| }); | ||
| value = sb.toString(); | ||
| } else if (valueAsObject instanceof SubmissionStatus){ | ||
| SubmissionStatus submissionStatus = (SubmissionStatus) valueAsObject; | ||
| value = submissionStatus.getName().toString(); | ||
| } else if (valueAsObject instanceof User){ | ||
| User user = (User) valueAsObject; | ||
| value = user.getName().toString(); | ||
| } else if (valueAsObject instanceof ActionLog){ | ||
| ActionLog actionLog = (ActionLog) valueAsObject; | ||
| switch (column.getTitle()){ | ||
| case "Event Time": | ||
| Calendar actionDate = (Calendar) actionLog.getActionDate(); | ||
| value = simpleDateFormat.format(actionDate.getTime()); | ||
| break; | ||
| case "Last Event": | ||
| value = actionLog.getEntry().toString(); | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
| } else { | ||
| value = valueAsObject.toString(); | ||
| } | ||
| row.put(column.getTitle(), value.toString()); | ||
| } catch (Exception exception) { | ||
| logger.warn("Unable to get value from " + String.join(",", valuePath)); | ||
| } | ||
| } else { | ||
| logger.warn("Column " + column.getTitle() + " has no predicate or value path!"); | ||
| } | ||
| } | ||
| }); | ||
| return new ExcelExportPackage(submission, "Excel", row); | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…l_fp replace delimeter for all repeatable field profiles
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes issue #2122 with an application.yml field app.secondaryDelimiter to delimit lists within a CSV element (e.g. keywords) for the Excel export. This will keep the UI as is for the export and provide site consistency for all Excel exports for that site.
This does not correct the data problem from the past where students entered all their keywords separated by commas on one line instead of on separate lines (and hence field_value entries). For those who did enter separate field_value keywords this will produce the keyword list within its column such as ',keyword1| keyword2| keyword3,' using the default '|' delimiter.