E-mail changes initiated from console to registrar contacts

Also, fix misspelling of "recipient."

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=237857289

java/google/registry/ui/server/SendEmailUtils.java

@@ -24,6 +24,7 @@ import google.registry.config.RegistryConfig.Config;
 import google.registry.util.SendEmailService;
 import java.util.List;
 import java.util.Objects;
+import java.util.stream.Stream;
 import javax.inject.Inject;
 import javax.mail.Message;
 import javax.mail.internet.AddressException;
@@ -55,22 +56,24 @@ public class SendEmailUtils {
   }
 
   /**
-   * Sends an email from Nomulus to the registrarChangesNotificationEmailAddresses. Returns true iff
+   * Sends an email from Nomulus to the registrarChangesNotificationEmailAddresses and the specified
-   * sending to at least 1 address was successful.
+   * additionalAddresses. Returns true iff sending to at least 1 address was successful.
    *
-   * <p>This means that if there are no recepients ({@link #hasRecepients} returns false), this will
+   * <p>This means that if there are no recipients ({@link #hasRecipients} returns false), this will
    * return false even thought no error happened.
    *
-   * <p>This also means that if there are multiple recepients, it will return true even if some (but
+   * <p>This also means that if there are multiple recipients, it will return true even if some (but
-   * not all) of the recepients had an error.
+   * not all) of the recipients had an error.
    */
-  public boolean sendEmail(final String subject, String body) {
+  public boolean sendEmail(
+      final String subject, String body, ImmutableList<String> additionalAddresses) {
     try {
       Message msg = emailService.createMessage();
       msg.setFrom(
           new InternetAddress(gSuiteOutgoingEmailAddress, gSuiteOutgoingEmailDisplayName));
       List<InternetAddress> emails =
-          registrarChangesNotificationEmailAddresses.stream()
+          Stream.concat(
+                  registrarChangesNotificationEmailAddresses.stream(), additionalAddresses.stream())
               .map(
                   emailAddress -> {
                     try {
@@ -101,11 +104,19 @@ public class SendEmailUtils {
     return true;
   }
 
-  /**
+  /** Sends an email from Nomulus to the registrarChangesNotificationEmailAddresses.
-   * Returns whether there are any recepients set up. {@link #sendEmail} will always return false if
+   *
-   * there are no recepients.
+   * <p>See {@link #sendEmail(String, String, ImmutableList<String>)}.
    */
-  public boolean hasRecepients() {
+  public boolean sendEmail(final String subject, String body) {
+    return sendEmail(subject, body, ImmutableList.of());
+  }
+
+  /**
+   * Returns whether there are any recipients set up. {@link #sendEmail} will always return false if
+   * there are no recipients.
+   */
+  public boolean hasRecipients() {
     return !registrarChangesNotificationEmailAddresses.isEmpty();
   }
 }

java/google/registry/ui/server/registrar/ConsoleOteSetupAction.java

@@ -221,7 +221,7 @@ public final class ConsoleOteSetupAction implements Runnable {
 
 
   private void sendExternalUpdates(ImmutableMap<String, String> clientIdToTld) {
-    if (!sendEmailUtils.hasRecepients()) {
+    if (!sendEmailUtils.hasRecipients()) {
       return;
     }
     String environment = Ascii.toLowerCase(String.valueOf(registryEnvironment));

java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorAction.java

@@ -347,7 +347,7 @@ public final class ConsoleRegistrarCreatorAction implements Runnable {
     return String.format("    %s: %s\n", name, value.orElse(null));
   }
   private void sendExternalUpdates() {
-    if (!sendEmailUtils.hasRecepients()) {
+    if (!sendEmailUtils.hasRecipients()) {
       return;
     }
     String environment = Ascii.toLowerCase(String.valueOf(registryEnvironment));

java/google/registry/ui/server/registrar/RegistrarSettingsAction.java

@@ -15,6 +15,7 @@
 package google.registry.ui.server.registrar;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.collect.Sets.difference;
 import static google.registry.export.sheet.SyncRegistrarsSheetAction.enqueueRegistrarSheetSync;
@@ -463,15 +464,15 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
 
   /**
    * Determines if any changes were made to the registrar besides the lastUpdateTime, and if so,
-   * sends an email with a diff of the changes to the configured notification email address and
+   * sends an email with a diff of the changes to the configured notification email address and all
-   * enqueues a task to re-sync the registrar sheet.
+   * contact addresses and enqueues a task to re-sync the registrar sheet.
    */
   private void sendExternalUpdatesIfNecessary(
       Registrar existingRegistrar,
       ImmutableSet<RegistrarContact> existingContacts,
       Registrar updatedRegistrar,
       ImmutableSet<RegistrarContact> updatedContacts) {
-    if (!sendEmailUtils.hasRecepients()) {
+    if (!sendEmailUtils.hasRecipients() && existingContacts.isEmpty()) {
       return;
     }
 
@@ -498,7 +499,13 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
             environment,
             existingRegistrar.getClientId(),
             authResult.userIdForLogging(),
-            DiffUtils.prettyPrintDiffedMap(diffs, null)));
+            DiffUtils.prettyPrintDiffedMap(diffs, null)),
+        existingContacts.stream()
+            .map(
+                contact -> {
+                  return contact.getEmailAddress();
+                })
+            .collect(toImmutableList()));
   }
 
   /** Thrown when a set of contacts doesn't meet certain constraints. */

javatests/google/registry/ui/server/SendEmailUtilsTest.java

@@ -51,19 +51,19 @@ public class SendEmailUtilsTest {
     when(emailService.createMessage()).thenReturn(message);
   }
 
-  private void setRecepients(ImmutableList<String> recepients) {
+  private void setRecipients(ImmutableList<String> recipients) {
     sendEmailUtils =
         new SendEmailUtils(
             "outgoing@registry.example",
             "outgoing display name",
-            recepients,
+            recipients,
             emailService);
   }
 
   @Test
   public void testSuccess_sendToOneAddress() throws Exception {
-    setRecepients(ImmutableList.of("johnny@fakesite.tld"));
+    setRecipients(ImmutableList.of("johnny@fakesite.tld"));
-    assertThat(sendEmailUtils.hasRecepients()).isTrue();
+    assertThat(sendEmailUtils.hasRecipients()).isTrue();
     assertThat(
             sendEmailUtils.sendEmail(
                 "Welcome to the Internet",
@@ -78,8 +78,8 @@ public class SendEmailUtilsTest {
 
   @Test
   public void testSuccess_sendToMultipleAddresses() throws Exception {
-    setRecepients(ImmutableList.of("foo@example.com", "bar@example.com"));
+    setRecipients(ImmutableList.of("foo@example.com", "bar@example.com"));
-    assertThat(sendEmailUtils.hasRecepients()).isTrue();
+    assertThat(sendEmailUtils.hasRecipients()).isTrue();
     assertThat(
             sendEmailUtils.sendEmail(
                 "Welcome to the Internet",
@@ -93,8 +93,8 @@ public class SendEmailUtilsTest {
 
   @Test
   public void testSuccess_ignoresMalformedEmailAddress() throws Exception {
-    setRecepients(ImmutableList.of("foo@example.com", "1iñvalidemail"));
+    setRecipients(ImmutableList.of("foo@example.com", "1iñvalidemail"));
-    assertThat(sendEmailUtils.hasRecepients()).isTrue();
+    assertThat(sendEmailUtils.hasRecipients()).isTrue();
     assertThat(
             sendEmailUtils.sendEmail(
                 "Welcome to the Internet",
@@ -107,8 +107,8 @@ public class SendEmailUtilsTest {
 
   @Test
   public void testFailure_noAddresses() throws Exception {
-    setRecepients(ImmutableList.of());
+    setRecipients(ImmutableList.of());
-    assertThat(sendEmailUtils.hasRecepients()).isFalse();
+    assertThat(sendEmailUtils.hasRecipients()).isFalse();
     assertThat(
             sendEmailUtils.sendEmail(
                 "Welcome to the Internet",
@@ -119,8 +119,8 @@ public class SendEmailUtilsTest {
 
   @Test
   public void testFailure_onlyGivenMalformedAddress() throws Exception {
-    setRecepients(ImmutableList.of("1iñvalidemail"));
+    setRecipients(ImmutableList.of("1iñvalidemail"));
-    assertThat(sendEmailUtils.hasRecepients()).isTrue();
+    assertThat(sendEmailUtils.hasRecipients()).isTrue();
     assertThat(
             sendEmailUtils.sendEmail(
                 "Welcome to the Internet",
@@ -131,8 +131,8 @@ public class SendEmailUtilsTest {
 
   @Test
   public void testFailure_exceptionThrownDuringSend() throws Exception {
-    setRecepients(ImmutableList.of("foo@example.com"));
+    setRecipients(ImmutableList.of("foo@example.com"));
-    assertThat(sendEmailUtils.hasRecepients()).isTrue();
+    assertThat(sendEmailUtils.hasRecipients()).isTrue();
     doThrow(new MessagingException()).when(emailService).sendMessage(any(Message.class));
     assertThat(
             sendEmailUtils.sendEmail(
@@ -145,6 +145,42 @@ public class SendEmailUtilsTest {
         .containsExactly(new InternetAddress("foo@example.com"));
   }
 
+  @Test
+  public void testAdditionalRecipients() throws Exception {
+    setRecipients(ImmutableList.of("foo@example.com"));
+    assertThat(sendEmailUtils.hasRecipients()).isTrue();
+    assertThat(
+            sendEmailUtils.sendEmail(
+                "Welcome to the Internet",
+                "It is a dark and scary place.",
+                ImmutableList.of("bar@example.com", "baz@example.com")))
+        .isTrue();
+    verifyMessageSent();
+    assertThat(message.getAllRecipients())
+        .asList()
+        .containsExactly(
+            new InternetAddress("foo@example.com"),
+            new InternetAddress("bar@example.com"),
+            new InternetAddress("baz@example.com"));
+  }
+
+  @Test
+  public void testOnlyAdditionalRecipients() throws Exception {
+    setRecipients(ImmutableList.of());
+    assertThat(sendEmailUtils.hasRecipients()).isFalse();
+    assertThat(
+            sendEmailUtils.sendEmail(
+                "Welcome to the Internet",
+                "It is a dark and scary place.",
+                ImmutableList.of("bar@example.com", "baz@example.com")))
+        .isTrue();
+    verifyMessageSent();
+    assertThat(message.getAllRecipients())
+        .asList()
+        .containsExactly(
+            new InternetAddress("bar@example.com"), new InternetAddress("baz@example.com"));
+  }
+
   private void verifyMessageSent() throws Exception {
     verify(emailService).sendMessage(message);
     assertThat(message.getSubject()).isEqualTo("Welcome to the Internet");

javatests/google/registry/ui/server/registrar/ContactSettingsTest.java

@@ -65,7 +65,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
   }
 
   @Test
-  public void testPost_updateContacts_success() {
+  public void testPost_updateContacts_success() throws Exception {
     // Remove all the contacts but the first by updating with list of
     // just it.
     Map<String, /* @Nullable */ Object> adminContact1 = new HashMap<>();
@@ -91,6 +91,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
         .build();
     assertThat(loadRegistrar(CLIENT_ID).getContacts()).containsExactly(newContact);
     assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
+    verifyContactsNotified();
   }
 
   @Test

javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java

@@ -40,7 +40,6 @@ import google.registry.util.CidrAddressBlock;
 import java.util.Map;
 import java.util.function.BiFunction;
 import java.util.function.Function;
-import javax.mail.internet.InternetAddress;
 import org.json.simple.JSONValue;
 import org.json.simple.parser.ParseException;
 import org.junit.Test;
@@ -56,11 +55,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
     String expectedEmailBody = loadFile(getClass(), "update_registrar_email.txt");
     action.handleJsonRequest(readJsonFromFile("update_registrar.json", getLastUpdateTime()));
     verify(rsp, never()).setStatus(anyInt());
-    verify(emailService).createMessage();
+    verifyContactsNotified();
-    verify(emailService).sendMessage(message);
-    assertThat(message.getAllRecipients()).asList().containsExactly(
-        new InternetAddress("notification@test.example"),
-        new InternetAddress("notification2@test.example"));
     assertThat(message.getContent()).isEqualTo(expectedEmailBody);
     assertTasksEnqueued("sheet", new TaskMatcher()
         .url(SyncRegistrarsSheetAction.PATH)

javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java

@@ -22,12 +22,14 @@ import static google.registry.request.auth.AuthenticatedRegistrarAccessor.Role.O
 import static google.registry.security.JsonHttpTestUtils.createJsonPayload;
 import static google.registry.testing.DatastoreHelper.createTlds;
 import static google.registry.testing.DatastoreHelper.disallowRegistrarAccess;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import com.google.appengine.api.users.User;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSetMultimap;
+import com.google.common.truth.Truth;
 import google.registry.config.RegistryEnvironment;
 import google.registry.model.ofy.Ofy;
 import google.registry.request.JsonActionRunner;
@@ -48,6 +50,7 @@ import java.io.StringWriter;
 import java.util.Properties;
 import javax.mail.Message;
 import javax.mail.Session;
+import javax.mail.internet.InternetAddress;
 import javax.mail.internet.MimeMessage;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -156,4 +159,16 @@ public class RegistrarSettingsActionTestCase {
         AuthenticatedRegistrarAccessor.createForTesting(
             ImmutableSetMultimap.of(CLIENT_ID, ADMIN));
   }
+
+  /** Verifies that the original contact of TheRegistrar is among those notified of a change. */
+  protected void verifyContactsNotified() throws Exception {
+    verify(emailService).createMessage();
+    verify(emailService).sendMessage(message);
+    Truth.assertThat(message.getAllRecipients())
+        .asList()
+        .containsExactly(
+            new InternetAddress("notification@test.example"),
+            new InternetAddress("notification2@test.example"),
+            new InternetAddress("johndoe@theregistrar.com"));
+  }
 }

javatests/google/registry/ui/server/registrar/SecuritySettingsTest.java

@@ -41,7 +41,7 @@ import org.junit.runners.JUnit4;
 public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
 
   @Test
-  public void testPost_updateCert_success() {
+  public void testPost_updateCert_success() throws Exception {
     Registrar modified =
         loadRegistrar(CLIENT_ID)
             .asBuilder()
@@ -55,6 +55,7 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
     assertThat(response).containsEntry("results", ImmutableList.of(modified.toJsonMap()));
     assertThat(loadRegistrar(CLIENT_ID)).isEqualTo(modified);
     assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
+    verifyContactsNotified();
   }
 
   @Test
@@ -71,7 +72,7 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
   }
 
   @Test
-  public void testChangeCertificates() {
+  public void testChangeCertificates() throws Exception {
     Map<String, Object> jsonMap = loadRegistrar(CLIENT_ID).toJsonMap();
     jsonMap.put("clientCertificate", SAMPLE_CERT);
     jsonMap.put("failoverClientCertificate", null);
@@ -84,10 +85,11 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
     assertThat(registrar.getFailoverClientCertificate()).isNull();
     assertThat(registrar.getFailoverClientCertificateHash()).isNull();
     assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
+    verifyContactsNotified();
   }
 
   @Test
-  public void testChangeFailoverCertificate() {
+  public void testChangeFailoverCertificate() throws Exception {
     Map<String, Object> jsonMap = loadRegistrar(CLIENT_ID).toJsonMap();
     jsonMap.put("failoverClientCertificate", SAMPLE_CERT2);
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
@@ -97,10 +99,11 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
     assertThat(registrar.getFailoverClientCertificate()).isEqualTo(SAMPLE_CERT2);
     assertThat(registrar.getFailoverClientCertificateHash()).isEqualTo(SAMPLE_CERT2_HASH);
     assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
+    verifyContactsNotified();
   }
 
   @Test
-  public void testEmptyOrNullCertificate_doesNotClearOutCurrentOne() {
+  public void testEmptyOrNullCertificate_doesNotClearOutCurrentOne() throws Exception {
     Registrar initialRegistrar =
         persistResource(
             loadRegistrar(CLIENT_ID)

javatests/google/registry/ui/server/registrar/WhoisSettingsTest.java

@@ -36,7 +36,7 @@ import org.junit.runners.JUnit4;
 public class WhoisSettingsTest extends RegistrarSettingsActionTestCase {
 
   @Test
-  public void testPost_update_success() {
+  public void testPost_update_success() throws Exception {
     Registrar modified =
         loadRegistrar(CLIENT_ID)
             .asBuilder()
@@ -61,6 +61,7 @@ public class WhoisSettingsTest extends RegistrarSettingsActionTestCase {
     assertThat(response.get("results")).isEqualTo(ImmutableList.of(modified.toJsonMap()));
     assertThat(loadRegistrar(CLIENT_ID)).isEqualTo(modified);
     assertMetric(CLIENT_ID, "update", "[OWNER]", "SUCCESS");
+    verifyContactsNotified();
   }
 
   @Test