Issue 2128: Fix Committee Contact Email in Excel Export#2129
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Issue #2128 where the “Committee Contact Email” column was empty in the Excel export by ensuring the value is materialized on Submission before EntityUtility.getValueFromPath(...) attempts to read it.
Changes:
- Added a transient backing field
committeeContactEmailtoSubmission. - Updated
getCommitteeContactEmail()to populate/return the backing field. - Invoked
getCommitteeContactEmail()inExcelPackagerprior to value-path lookup so reflection-based extraction can see the generated value.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/main/java/org/tdl/vireo/model/packager/ExcelPackager.java |
Forces committee email computation before reflection-based export value lookup. |
src/main/java/org/tdl/vireo/model/Submission.java |
Introduces a transient backing field and changes getter behavior to populate it. |
Comments suppressed due to low confidence (2)
src/main/java/org/tdl/vireo/model/packager/ExcelPackager.java:87
submission.getCommitteeContactEmail();is executed for every value-path based column, even when the column being exported is unrelated. This relies on a side-effect to populate a transient field for reflection and adds unnecessary work for large exports. Consider calling it only whenvaluePathtargetscommitteeContactEmail(e.g., when the first path element equals that name), or refactor to populate computed export-only fields in a more targeted way.
if(column.getValuePath().size() > 1){
valuePath = new String[] {valuePath[0]};
}
submission.getCommitteeContactEmail();
Object valueAsObject = EntityUtility.getValueFromPath(submission, valuePath);
src/main/java/org/tdl/vireo/model/packager/ExcelPackager.java:87
- There’s existing unit coverage for
Submission.getCommitteeContactEmail(), but this PR’s core regression fix is in Excel export behavior. Consider adding a focused test that builds aSubmissionwith an advisor contact email, runsExcelPackager.packageExport(...)with aSubmissionListColumnwhosevaluePathis["committeeContactEmail"], and asserts the exported row contains the email (and doesn’t log the warning).
submission.getCommitteeContactEmail();
Object valueAsObject = EntityUtility.getValueFromPath(submission, valuePath);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (optEmail.isPresent()) { | ||
| email = optEmail.get(); | ||
| committeeContactEmail = optEmail.get(); | ||
| } | ||
| } | ||
| return email; | ||
| return committeeContactEmail; |
There was a problem hiding this comment.
getCommitteeContactEmail() now caches into the committeeContactEmail field but never clears it when no advisor/contact is found. If the submission’s fieldValues change (or if a later call finds no email), this method can return a stale value from an earlier call. Consider recomputing into a local variable and assigning committeeContactEmail each call (including explicitly setting it back to null when not found) before returning.
Fix #2128
committeeContactEmailinSubmission.javagetCommitteeContactEmail()to return the valuegetCommitteeContactEmail()inExcelPackager.javabefore the value lookup so the value is getting generatedcommitteeContactEmailshould then be covered by theelse ... value = valueAsObject.toString();clause in the exportI've tested on our instance.