diff --git a/java/google/registry/ui/externs/json.js b/java/google/registry/ui/externs/json.js index 33355f0fe..13d72b53f 100644 --- a/java/google/registry/ui/externs/json.js +++ b/java/google/registry/ui/externs/json.js @@ -77,7 +77,7 @@ registry.json.Response.prototype.results; * ianaIdentifier: (number?|undefined), * icannReferralEmail: string, * ipAddressWhitelist: !Array, - * emailAddress: string, + * emailAddress: (string?|undefined), * phonePasscode: (string?|undefined), * phoneNumber: (string?|undefined), * faxNumber: (string?|undefined), diff --git a/java/google/registry/ui/server/RegistrarFormFields.java b/java/google/registry/ui/server/RegistrarFormFields.java index 745b50988..3d288c426 100644 --- a/java/google/registry/ui/server/RegistrarFormFields.java +++ b/java/google/registry/ui/server/RegistrarFormFields.java @@ -71,12 +71,17 @@ public final class RegistrarFormFields { .required() .build(); - public static final FormField EMAIL_ADDRESS_FIELD = + public static final FormField EMAIL_ADDRESS_FIELD_REQUIRED = FormFields.EMAIL.asBuilderNamed("emailAddress") .matches(ASCII_PATTERN, ASCII_ERROR) .required() .build(); + public static final FormField EMAIL_ADDRESS_FIELD_OPTIONAL = + FormFields.EMAIL.asBuilderNamed("emailAddress") + .matches(ASCII_PATTERN, ASCII_ERROR) + .build(); + public static final FormField ICANN_REFERRAL_EMAIL_FIELD = FormFields.EMAIL.asBuilderNamed("icannReferralEmail") .matches(ASCII_PATTERN, ASCII_ERROR) diff --git a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index 24cefa3f1..f85c94853 100644 --- a/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -20,6 +20,7 @@ import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.security.JsonResponseHelper.Status.ERROR; import static google.registry.security.JsonResponseHelper.Status.SUCCESS; +import com.google.common.base.Strings; import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -125,14 +126,14 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA initialRegistrar.getClientId(), args); return JsonResponseHelper.createFormFieldError(e.getMessage(), e.getFieldName()); - } catch (FormException ee) { + } catch (FormException e) { logger.warningfmt( - ee, + e, "Failed to perform operation '%s' on registrar '%s' for args %s", op, initialRegistrar.getClientId(), args); - return JsonResponseHelper.create(ERROR, ee.getMessage()); + return JsonResponseHelper.create(ERROR, e.getMessage()); } } @@ -196,7 +197,12 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA RegistrarFormFields.WHOIS_SERVER_FIELD.extractUntyped(args).orElse(null)); builder.setReferralUrl( RegistrarFormFields.REFERRAL_URL_FIELD.extractUntyped(args).orElse(null)); - RegistrarFormFields.EMAIL_ADDRESS_FIELD + + // If the email is already null / empty - we can keep it so. But if it's set - it's required to + // remain set. + (Strings.isNullOrEmpty(existingRegistrarObj.getEmailAddress()) + ? RegistrarFormFields.EMAIL_ADDRESS_FIELD_OPTIONAL + : RegistrarFormFields.EMAIL_ADDRESS_FIELD_REQUIRED) .extractUntyped(args) .ifPresent(builder::setEmailAddress); builder.setPhoneNumber( diff --git a/java/google/registry/ui/soy/registrar/WhoisSettings.soy b/java/google/registry/ui/soy/registrar/WhoisSettings.soy index 620a264ec..3b9d77667 100644 --- a/java/google/registry/ui/soy/registrar/WhoisSettings.soy +++ b/java/google/registry/ui/soy/registrar/WhoisSettings.soy @@ -25,7 +25,7 @@ {@param? whoisServer: string} {@param? referralUrl: string} // Passed to .contactInfo_ - {@param emailAddress: string} + {@param? emailAddress: string} {@param? localizedAddress: ?} {@param? phoneNumber: string} {@param? faxNumber: string} @@ -79,7 +79,7 @@ * Contact info. */ {template .contactInfo_ visibility="private"} - {@param emailAddress: string} + {@param? emailAddress: string} {@param readonly: bool} {@param? localizedAddress: ?} {@param? phoneNumber: string} @@ -107,11 +107,13 @@ value="{$faxNumber}">
{$faxNumber} (Fax)
{/if} - -
{$emailAddress}
+ {if isNonnull($emailAddress)} + +
{$emailAddress}
+ {/if} {else} {call registry.soy.forms.inputFieldRowWithValue} diff --git a/javatests/google/registry/server/RegistryTestServer.java b/javatests/google/registry/server/RegistryTestServer.java index 911eb7a52..4cf4c95fb 100644 --- a/javatests/google/registry/server/RegistryTestServer.java +++ b/javatests/google/registry/server/RegistryTestServer.java @@ -88,6 +88,8 @@ public final class RegistryTestServer { route("/registrar", google.registry.module.frontend.FrontendServlet.class), route("/registrar-settings", google.registry.module.frontend.FrontendServlet.class), + route("/registrar-premium-price-ack", + google.registry.module.frontend.FrontendServlet.class), route("/registrar-payment", google.registry.module.frontend.FrontendServlet.class), route("/registrar-payment-setup", diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java index b1a639883..650158cdc 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java @@ -16,6 +16,7 @@ package google.registry.ui.server.registrar; import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.DatastoreHelper.loadRegistrar; +import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.JUnitBackports.assertThrows; import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued; import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; @@ -98,6 +99,16 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase assertNoTasksEnqueued("sheet"); } + @Test + public void testUpdate_emptyJsonObject_emailFieldNotRequiredWhenEmpty() throws Exception { + persistResource(loadRegistrar(CLIENT_ID).asBuilder().setEmailAddress(null).build()); + + Map response = action.handleJsonRequest(ImmutableMap.of( + "op", "update", + "args", ImmutableMap.of())); + assertThat(response).containsEntry("status", "SUCCESS"); + } + @Test public void testUpdate_badEmail_errorEmailField() throws Exception { Map response = action.handleJsonRequest(ImmutableMap.of( diff --git a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java index 433fc5fd5..b10c325d0 100644 --- a/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java +++ b/javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTestCase.java @@ -106,8 +106,12 @@ public class RegistrarSettingsActionTestCase { when(rsp.getWriter()).thenReturn(new PrintWriter(writer)); when(req.getContentType()).thenReturn("application/json"); when(req.getReader()).thenReturn(createJsonPayload(ImmutableMap.of("op", "read"))); + // We mock getRegistrarForAuthResult to reload the registrar each time it's called. Otherwise - + // the result is out of date after mutations. + // (for example, if someone wants to change the registrar to prepare for a test, the function + // would still return the old value) when(sessionUtils.getRegistrarForAuthResult(req, action.authResult)) - .thenReturn(loadRegistrar(CLIENT_ID)); + .thenAnswer(x -> loadRegistrar(CLIENT_ID)); when(modulesService.getVersionHostname("backend", null)).thenReturn("backend.hostname"); }