Skip to content

DT-1591: Consolidate send mail methods#2911

Open
rushtong wants to merge 7 commits into
developfrom
gr-DT-1591-consolidate-send-mail
Open

DT-1591: Consolidate send mail methods#2911
rushtong wants to merge 7 commits into
developfrom
gr-DT-1591-consolidate-send-mail

Conversation

@rushtong
Copy link
Copy Markdown
Contributor

@rushtong rushtong commented May 15, 2026

Addresses

Partially addresses https://broadworkbench.atlassian.net/browse/DT-1591
Specifically, bullet point 2:

The EmailService methods saveEmailAndResponse and sendMessage could be further refactored now that saveEmailAndResponse is only used in one place.

Summary

This PR simplifies EmailService by combining methods. We now delegate the creation date to the database instead of passing it in via application code. Additionally, tests are updated for coverage and comprehensiveness.


Have you read CONTRIBUTING.md lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

@sonarqubecloud
Copy link
Copy Markdown

@rushtong rushtong marked this pull request as ready for review May 16, 2026 11:41
@rushtong rushtong requested a review from a team as a code owner May 16, 2026 11:41
@rushtong rushtong requested review from Copilot, fboulnois and otchet-broad and removed request for a team May 16, 2026 11:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR consolidates email persistence into EmailService.sendMessage and introduces an insert-specific mail DTO so the database owns create_date assignment.

Changes:

  • Adds MailMessageInsert for new email records without emailId or createDate.
  • Updates MailMessageDAO.insert to populate create_date with NOW().
  • Refreshes affected DAO/service tests for the new insert model and date behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/main/java/org/broadinstitute/consent/http/service/EmailService.java Inlines send response persistence into sendMessage using MailMessageInsert.
src/main/java/org/broadinstitute/consent/http/models/mail/MailMessageInsert.java Adds insert-only record for persisted email fields.
src/main/java/org/broadinstitute/consent/http/db/MailMessageDAO.java Changes insert binding type and delegates create_date to the database.
src/test/java/org/broadinstitute/consent/http/service/EmailServiceTest.java Updates and expands service tests for SendGrid responses, categories, and persistence.
src/test/java/org/broadinstitute/consent/http/db/MailMessageDAOTest.java Updates DAO tests for MailMessageInsert and database-provided create dates.
src/test/java/org/broadinstitute/consent/http/db/UserDAOTest.java Updates email insert test setup to use MailMessageInsert.
src/test/java/org/broadinstitute/consent/http/db/ElectionDAOTest.java Updates reminder email insert setup to use MailMessageInsert.
src/test/java/org/broadinstitute/consent/http/db/DataAccessRequestDAOTest.java Updates DAR email insert setup to use MailMessageInsert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants