From 79cd5c509cb87e8c56be64926fffe4d7e83fbe71 Mon Sep 17 00:00:00 2001 From: anthonychengit Date: Thu, 13 Aug 2026 16:11:55 -0700 Subject: [PATCH] fix(amber): include requester name in account email --- .../texera/web/resource/EmailTemplate.scala | 3 ++ .../texera/web/resource/GmailResource.scala | 33 +++++++++++++---- .../web/resource/EmailTemplateSpec.scala | 11 +++++- .../web/resource/GmailResourceSpec.scala | 37 ++++++++++++++++++- 4 files changed, 74 insertions(+), 10 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/EmailTemplate.scala b/amber/src/main/scala/org/apache/texera/web/resource/EmailTemplate.scala index 17084b6c101..a79c0eabfa5 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/EmailTemplate.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/EmailTemplate.scala @@ -41,12 +41,14 @@ object EmailTemplate { * * @param receiverEmail the email address of the receiver (admin or user) * @param userEmail optional; the email address of the user requesting an account (only needed if toAdmin is true) + * @param userName optional; the stored name of the user requesting an account (only needed if toAdmin is true) * @param toAdmin flag indicating whether the notification is for the admin (true) or the user (false) * @return an EmailMessage ready to be sent */ def userRegistrationNotification( receiverEmail: String, userEmail: Option[String], + userName: Option[String], affiliation: Option[String], reason: Option[String], toAdmin: Boolean @@ -62,6 +64,7 @@ object EmailTemplate { |A new user has attempted to log in or register, but their account is not yet approved. |Please review the account request for the following user: | + |Name: ${userName.filter(_.trim.nonEmpty).getOrElse("Not provided")} |Email: ${userEmail.getOrElse("Unknown")} |Affiliation: ${affiliation.filter(_.trim.nonEmpty).getOrElse("Not provided")} |Reason: ${reason.filter(_.trim.nonEmpty).getOrElse("Not provided")} diff --git a/amber/src/main/scala/org/apache/texera/web/resource/GmailResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/GmailResource.scala index 348dad6ff7e..5f9197b5162 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/GmailResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/GmailResource.scala @@ -26,7 +26,13 @@ import org.apache.texera.dao.SqlServer import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum import org.apache.texera.dao.jooq.generated.tables.daos.UserDao import org.apache.texera.web.resource.EmailTemplate.userRegistrationNotification -import org.apache.texera.web.resource.GmailResource.{isValidEmail, sendEmail, senderGmail, userDao} +import org.apache.texera.web.resource.GmailResource.{ + adminRegistrationNotification, + isValidEmail, + sendEmail, + senderGmail, + userDao +} import org.slf4j.LoggerFactory import javax.annotation.security.RolesAllowed @@ -51,6 +57,22 @@ object GmailResource { .createDSLContext() private def userDao = new UserDao(context.configuration) + private[resource] def adminRegistrationNotification( + adminEmail: String, + emailMessage: EmailMessage + ): EmailMessage = { + val requesterName = + Option(userDao.fetchOneByEmail(emailMessage.receiver)).flatMap(user => Option(user.getName)) + userRegistrationNotification( + receiverEmail = adminEmail, + userEmail = Some(emailMessage.receiver), + userName = requesterName, + affiliation = emailMessage.affiliation, + reason = emailMessage.reason, + toAdmin = true + ) + } + private lazy val senderGmail: String = UserSystemConfig.gmail private val smtpProperties = Map( "mail.smtp.host" -> "smtp.gmail.com", @@ -179,13 +201,7 @@ class GmailResource { try { sendEmail( - userRegistrationNotification( - receiverEmail = adminEmail, - userEmail = Some(emailMessage.receiver), - affiliation = emailMessage.affiliation, - reason = emailMessage.reason, - toAdmin = true - ), + adminRegistrationNotification(adminEmail, emailMessage), adminEmail ) } catch { @@ -199,6 +215,7 @@ class GmailResource { userRegistrationNotification( receiverEmail = emailMessage.receiver, userEmail = None, + userName = None, affiliation = None, reason = None, toAdmin = false diff --git a/amber/src/test/scala/org/apache/texera/web/resource/EmailTemplateSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/EmailTemplateSpec.scala index 64e6636a367..1651e3a23d0 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/EmailTemplateSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/EmailTemplateSpec.scala @@ -38,12 +38,14 @@ class EmailTemplateSpec extends AnyFlatSpec with Matchers { val msg = EmailTemplate.userRegistrationNotification( receiverEmail = "admin@example.com", userEmail = Some("alice@example.com"), + userName = Some("Alice Example"), affiliation = Some("UC Irvine"), reason = Some("research"), toAdmin = true ) msg.receiver shouldBe "admin@example.com" msg.subject should startWith("New Account Request Pending Approval") + msg.content should include("Name: Alice Example") msg.content should include("Email: alice@example.com") msg.content should include("Affiliation: UC Irvine") msg.content should include("Reason: research") @@ -54,6 +56,7 @@ class EmailTemplateSpec extends AnyFlatSpec with Matchers { val msg = EmailTemplate.userRegistrationNotification( receiverEmail = "admin@example.com", userEmail = None, + userName = None, affiliation = Some("UC Irvine"), reason = Some("research"), toAdmin = true @@ -61,20 +64,23 @@ class EmailTemplateSpec extends AnyFlatSpec with Matchers { msg.content should include("Email: Unknown") } - it should "render 'Not provided' for affiliation/reason when None or whitespace-only" in { + it should "render 'Not provided' for name, affiliation, and reason when blank" in { val withNone = EmailTemplate.userRegistrationNotification( receiverEmail = "admin@example.com", userEmail = Some("alice@example.com"), + userName = None, affiliation = None, reason = None, toAdmin = true ) withNone.content should include("Affiliation: Not provided") withNone.content should include("Reason: Not provided") + withNone.content should include("Name: Not provided") val withBlank = EmailTemplate.userRegistrationNotification( receiverEmail = "admin@example.com", userEmail = Some("alice@example.com"), + userName = Some(" "), affiliation = Some(" "), reason = Some(""), toAdmin = true @@ -83,6 +89,7 @@ class EmailTemplateSpec extends AnyFlatSpec with Matchers { // strings the same as None. withBlank.content should include("Affiliation: Not provided") withBlank.content should include("Reason: Not provided") + withBlank.content should include("Name: Not provided") } // -- userRegistrationNotification (user branch) ----------------------------- @@ -92,6 +99,7 @@ class EmailTemplateSpec extends AnyFlatSpec with Matchers { val msg = EmailTemplate.userRegistrationNotification( receiverEmail = "alice@example.com", userEmail = Some("ignored@example.com"), + userName = Some("Ignored Name"), affiliation = Some("ignored"), reason = Some("ignored"), toAdmin = false @@ -103,6 +111,7 @@ class EmailTemplateSpec extends AnyFlatSpec with Matchers { // fields back to the requester — if a refactor accidentally surfaces // them, this assertion will catch the leak. msg.content should not include "Email: ignored" + msg.content should not include "Name: Ignored Name" msg.content should not include "Affiliation: ignored" msg.content should not include "Reason: ignored" } diff --git a/amber/src/test/scala/org/apache/texera/web/resource/GmailResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/GmailResourceSpec.scala index ec694f694f2..5e7b4437108 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/GmailResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/GmailResourceSpec.scala @@ -21,13 +21,29 @@ package org.apache.texera.web.resource import org.apache.texera.auth.SessionUser import org.apache.texera.common.config.UserSystemConfig +import org.apache.texera.dao.MockTexeraDB import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum +import org.apache.texera.dao.jooq.generated.tables.daos.UserDao import org.apache.texera.dao.jooq.generated.tables.pojos.User +import org.scalatest.BeforeAndAfterAll import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers import javax.ws.rs.{BadRequestException, ForbiddenException, WebApplicationException} -class GmailResourceSpec extends AnyFlatSpec { +class GmailResourceSpec extends AnyFlatSpec with Matchers with BeforeAndAfterAll with MockTexeraDB { + + private val requesterEmail = "gmail-requester@example.com" + + override protected def beforeAll(): Unit = { + initializeDBAndReplaceDSLContext() + val requester = new User + requester.setName("Alice Requester") + requester.setEmail(requesterEmail) + new UserDao(getDSLContext.configuration()).insert(requester) + } + + override protected def afterAll(): Unit = shutdownDB() private def newSessionUser(): SessionUser = { val user = new User @@ -132,4 +148,23 @@ class GmailResourceSpec extends AnyFlatSpec { } assert(ex.getResponse.getStatus == 403) } + + "adminRegistrationNotification" should "resolve the requester's stored name by email" in { + val message = GmailResource.adminRegistrationNotification( + "admin@example.com", + EmailMessage(receiver = requesterEmail, subject = "", content = "") + ) + + message.content should include("Name: Alice Requester") + message.content should include(s"Email: $requesterEmail") + } + + it should "render the fallback when no user matches the requester email" in { + val message = GmailResource.adminRegistrationNotification( + "admin@example.com", + EmailMessage(receiver = "missing-requester@example.com", subject = "", content = "") + ) + + message.content should include("Name: Not provided") + } }