diff --git a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index b5105945f..5f311ce52 100644 --- a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -35,6 +35,7 @@ import google.registry.config.RegistryConfig.Config; import google.registry.export.sheet.SyncRegistrarsSheetAction; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; +import google.registry.model.registrar.RegistrarContact.Type; import google.registry.request.Action; import google.registry.request.HttpException.BadRequestException; import google.registry.request.JsonActionRunner; @@ -268,12 +269,25 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA throw new ContactRequirementException(t); } } - // Ensure at least one tech contact has a phone number if one was present before. - if (any(oldContactsByType.get(RegistrarContact.Type.TECH), HAS_PHONE) - && !any(newContactsByType.get(RegistrarContact.Type.TECH), HAS_PHONE)) { - throw new ContactRequirementException(String.format( - "At least one %s contact must have a phone number", - RegistrarContact.Type.TECH.getDisplayName())); + ensurePhoneNumberNotRemovedForContactTypes( + oldContactsByType, newContactsByType, Type.ABUSE, Type.TECH); + } + + /** + * Ensure that for each given registrar type, a phone number is present after update, if there + * was one before. + */ + private static void ensurePhoneNumberNotRemovedForContactTypes( + Multimap oldContactsByType, + Multimap newContactsByType, + RegistrarContact.Type... types) { + for (RegistrarContact.Type type : types) { + if (any(oldContactsByType.get(type), HAS_PHONE) + && !any(newContactsByType.get(type), HAS_PHONE)) { + throw new ContactRequirementException( + String.format( + "At least one %s contact must have a phone number", type.getDisplayName())); + } } } diff --git a/javatests/google/registry/ui/server/registrar/ContactSettingsTest.java b/javatests/google/registry/ui/server/registrar/ContactSettingsTest.java index e4d81fc7b..839892243 100644 --- a/javatests/google/registry/ui/server/registrar/ContactSettingsTest.java +++ b/javatests/google/registry/ui/server/registrar/ContactSettingsTest.java @@ -128,4 +128,28 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase { assertThat(response).containsEntry("message", "At least one " + RegistrarContact.Type.TECH.getDisplayName() + " contact must have a phone number"); } + + @Test + public void testPost_updateContacts_requireAbusePhone_error() throws Exception { + // First make the contact a abuse contact as well. + Registrar registrar = Registrar.loadByClientId(CLIENT_ID); + RegistrarContact rc = AppEngineRule.makeRegistrarContact2() + .asBuilder() + .setTypes(ImmutableSet.of(RegistrarContact.Type.ADMIN, RegistrarContact.Type.ABUSE)) + .build(); + // Lest we anger the timestamp inversion bug. + persistResource(registrar); + persistSimpleResource(rc); + + // Now try to remove the phone number. + rc = rc.asBuilder().setPhoneNumber(null).build(); + Map reqJson = registrar.toJsonMap(); + reqJson.put("contacts", ImmutableList.of(rc.toJsonMap())); + Map response = action.handleJsonRequest(ImmutableMap.of( + "op", "update", + "args", reqJson)); + assertThat(response).containsEntry("status", "ERROR"); + assertThat(response).containsEntry("message", "At least one " + + RegistrarContact.Type.ABUSE.getDisplayName() + " contact must have a phone number"); + } }