Handle Purchase Order numbers explicitly in billing pipeline

Note that these aren't used for most registrars.

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

java/google/registry/beam/invoicing/BillingEvent.java

@@ -56,7 +56,6 @@ public abstract class BillingEvent implements Serializable {
   private static final DateTimeFormatter DATE_TIME_FORMATTER =
       DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss zzz");
 
-
   /** The amount we multiply the price for sunrise creates. This is currently a 15% discount. */
   private static final double SUNRISE_DISCOUNT_PRICE_MODIFIER = 0.85;
 
@@ -67,6 +66,7 @@ public abstract class BillingEvent implements Serializable {
           "eventTime",
           "registrarId",
           "billingId",
+          "poNumber",
           "tld",
           "action",
           "domain",
@@ -78,28 +78,43 @@ public abstract class BillingEvent implements Serializable {
 
   /** Returns the unique Objectify ID for the {@code OneTime} associated with this event. */
   abstract long id();
+
   /** Returns the UTC DateTime this event becomes billable. */
   abstract ZonedDateTime billingTime();
+
   /** Returns the UTC DateTime this event was generated. */
   abstract ZonedDateTime eventTime();
+
   /** Returns the billed registrar's name. */
   abstract String registrarId();
+
   /** Returns the billed registrar's billing account key. */
   abstract String billingId();
+
+  /** Returns the Purchase Order number. */
+  abstract String poNumber();
+
   /** Returns the tld this event was generated for. */
   abstract String tld();
+
   /** Returns the billable action this event was generated for (i.e. RENEW, CREATE, TRANSFER...) */
   abstract String action();
+
   /** Returns the fully qualified domain name this event was generated for. */
   abstract String domain();
+
   /** Returns the unique RepoID associated with the billed domain. */
   abstract String repositoryId();
+
   /** Returns the number of years this billing event is made out for. */
   abstract int years();
+
   /** Returns the 3-letter currency code for the billing event (i.e. USD or JPY.) */
   abstract String currency();
+
   /** Returns the cost associated with this billing event. */
   abstract double amount();
+
   /** Returns a list of space-delimited flags associated with the event. */
   abstract String flags();
 
@@ -126,6 +141,7 @@ public abstract class BillingEvent implements Serializable {
             .atZone(ZoneId.of("UTC")),
         extractField(record, "registrarId"),
         extractField(record, "billingId"),
+        extractField(record, "poNumber"),
         extractField(record, "tld"),
         extractField(record, "action"),
         extractField(record, "domain"),
@@ -171,6 +187,7 @@ public abstract class BillingEvent implements Serializable {
       ZonedDateTime eventTime,
       String registrarId,
       String billingId,
+      String poNumber,
       String tld,
       String action,
       String domain,
@@ -185,6 +202,7 @@ public abstract class BillingEvent implements Serializable {
         eventTime,
         registrarId,
         billingId,
+        poNumber,
         tld,
         action,
         domain,
@@ -241,7 +259,7 @@ public abstract class BillingEvent implements Serializable {
         String.format("%s | TLD: %s | TERM: %d-year", action(), tld(), years()),
         amount(),
         currency(),
-        "");
+        poNumber());
   }
 
   /** Key for each {@code BillingEvent}, when aggregating for the overall invoice. */
@@ -267,18 +285,25 @@ public abstract class BillingEvent implements Serializable {
 
     /** Returns the first day this invoice is valid, in yyyy-MM-dd format. */
     abstract String startDate();
+
     /** Returns the last day this invoice is valid, in yyyy-MM-dd format. */
     abstract String endDate();
+
     /** Returns the billing account id, which is the {@code BillingEvent.billingId}. */
     abstract String productAccountKey();
+
     /** Returns the invoice grouping key, which is in the format "registrarId - tld". */
     abstract String usageGroupingKey();
+
     /** Returns a description of the item, formatted as "action | TLD: tld | TERM: n-year." */
     abstract String description();
+
     /** Returns the cost per invoice item. */
     abstract Double unitPrice();
+
     /** Returns the 3-digit currency code the unit price uses. */
     abstract String unitPriceCurrency();
+
     /** Returns the purchase order number for the item, blank for most registrars. */
     abstract String poNumber();
 

java/google/registry/beam/invoicing/sql/billing_events.sql

@@ -22,6 +22,7 @@ SELECT
   eventTime,
   BillingEvent.clientId AS registrarId,
   RegistrarData.accountId AS billingId,
+  RegistrarData.poNumber AS poNumber,
   tld,
   reason as action,
   targetId as domain,
@@ -63,6 +64,7 @@ JOIN (
   SELECT
     __key__.name AS clientId,
     billingIdentifier,
+    IFNULL(poNumber, '') AS poNumber,
     r.billingAccountMap.currency[SAFE_OFFSET(index)] AS currency,
     r.billingAccountMap.accountId[SAFE_OFFSET(index)] AS accountId
   FROM

java/google/registry/model/registrar/Registrar.java

@@ -310,6 +310,10 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable
   @Nullable
   Long billingIdentifier;
 
+  /** Purchase Order number used for invoices in external billing system, if applicable. */
+  @Nullable
+  String poNumber;
+
   /**
    * Map of currency-to-billing account for the registrar.
    *
@@ -422,6 +426,10 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable
     return billingIdentifier;
   }
 
+  public Optional<String> getPoNumber() {
+    return Optional.ofNullable(poNumber);
+  }
+
   public ImmutableMap<CurrencyUnit, String> getBillingAccountMap() {
     if (billingAccountMap == null) {
       return ImmutableMap.of();
@@ -644,20 +652,25 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable
       return this;
     }
 
-    public Builder setIanaIdentifier(Long ianaIdentifier) {
+    public Builder setIanaIdentifier(@Nullable Long ianaIdentifier) {
       checkArgument(ianaIdentifier == null || ianaIdentifier > 0,
           "IANA ID must be a positive number");
       getInstance().ianaIdentifier = ianaIdentifier;
       return this;
     }
 
-    public Builder setBillingIdentifier(Long billingIdentifier) {
+    public Builder setBillingIdentifier(@Nullable Long billingIdentifier) {
       checkArgument(billingIdentifier == null || billingIdentifier > 0,
           "Billing ID must be a positive number");
       getInstance().billingIdentifier = billingIdentifier;
       return this;
     }
 
+    public Builder setPoNumber(Optional<String> poNumber) {
+      getInstance().poNumber = poNumber.orElse(null);
+      return this;
+    }
+
     public Builder setBillingAccountMap(@Nullable Map<CurrencyUnit, String> billingAccountMap) {
       if (billingAccountMap == null) {
         getInstance().billingAccountMap = null;

java/google/registry/tools/CreateOrUpdateRegistrarCommand.java

@@ -173,6 +173,14 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
       validateWith = OptionalLongParameter.class)
   private Optional<Long> billingId;
 
+  @Nullable
+  @Parameter(
+      names = "--po_number",
+      description = "Purchase Order number used for billing invoices",
+      converter = OptionalStringParameter.class,
+      validateWith = OptionalStringParameter.class)
+  private Optional<String> poNumber;
+
   @Nullable
   @Parameter(
     names = "--billing_account_map",
@@ -352,6 +360,7 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
       if (billingId != null) {
         builder.setBillingIdentifier(billingId.orElse(null));
       }
+      Optional.ofNullable(poNumber).ifPresent(builder::setPoNumber);
       if (billingAccountMap != null) {
         LinkedHashMap<CurrencyUnit, String> newBillingAccountMap = new LinkedHashMap<>();
         if (oldRegistrar != null && oldRegistrar.getBillingAccountMap() != null) {

javatests/google/registry/beam/invoicing/BillingEventTest.java

@@ -47,6 +47,7 @@ public class BillingEventTest {
           + "{\"name\": \"eventTime\", \"type\": \"string\"},"
           + "{\"name\": \"registrarId\", \"type\": \"string\"},"
           + "{\"name\": \"billingId\", \"type\": \"long\"},"
+          + "{\"name\": \"poNumber\", \"type\": \"string\"},"
           + "{\"name\": \"tld\", \"type\": \"string\"},"
           + "{\"name\": \"action\", \"type\": \"string\"},"
           + "{\"name\": \"domain\", \"type\": \"string\"},"
@@ -62,12 +63,17 @@ public class BillingEventTest {
   @Before
   public void initializeRecord() {
     // Create a record with a given JSON schema.
+    schemaAndRecord = new SchemaAndRecord(createRecord(), null);
+  }
+
+  private GenericRecord createRecord() {
     GenericRecord record = new GenericData.Record(new Schema.Parser().parse(BILLING_EVENT_SCHEMA));
     record.put("id", "1");
     record.put("billingTime", 1508835963000000L);
     record.put("eventTime", 1484870383000000L);
     record.put("registrarId", "myRegistrar");
     record.put("billingId", "12345-CRRHELLO");
+    record.put("poNumber", "");
     record.put("tld", "test");
     record.put("action", "RENEW");
     record.put("domain", "example.test");
@@ -76,7 +82,7 @@ public class BillingEventTest {
     record.put("currency", "USD");
     record.put("amount", 20.5);
     record.put("flags", "AUTO_RENEW SYNTHETIC");
-    schemaAndRecord = new SchemaAndRecord(record, null);
+    return record;
   }
 
   @Test
@@ -89,6 +95,7 @@ public class BillingEventTest {
         .isEqualTo(ZonedDateTime.of(2017, 1, 19, 23, 59, 43, 0, ZoneId.of("UTC")));
     assertThat(event.registrarId()).isEqualTo("myRegistrar");
     assertThat(event.billingId()).isEqualTo("12345-CRRHELLO");
+    assertThat(event.poNumber()).isEmpty();
     assertThat(event.tld()).isEqualTo("test");
     assertThat(event.action()).isEqualTo("RENEW");
     assertThat(event.domain()).isEqualTo("example.test");
@@ -149,6 +156,16 @@ public class BillingEventTest {
     assertThat(invoiceKey.poNumber()).isEmpty();
   }
 
+  @Test
+  public void test_nonNullPoNumber() {
+    GenericRecord record = createRecord();
+    record.put("poNumber", "905610");
+    BillingEvent event = BillingEvent.parseFromRecord(new SchemaAndRecord(record, null));
+    assertThat(event.poNumber()).isEqualTo("905610");
+    InvoiceGroupingKey invoiceKey = event.getInvoiceGroupingKey();
+    assertThat(invoiceKey.poNumber()).isEqualTo("905610");
+  }
+
   @Test
   public void testConvertInvoiceGroupingKey_toCsv() {
     BillingEvent event = BillingEvent.parseFromRecord(schemaAndRecord);
@@ -174,7 +191,7 @@ public class BillingEventTest {
   public void testGetDetailReportHeader() {
     assertThat(BillingEvent.getHeader())
         .isEqualTo(
-            "id,billingTime,eventTime,registrarId,billingId,tld,action,"
+            "id,billingTime,eventTime,registrarId,billingId,poNumber,tld,action,"
                 + "domain,repositoryId,years,currency,amount,flags");
   }
 

javatests/google/registry/beam/invoicing/InvoicingPipelineTest.java

@@ -76,6 +76,7 @@ public class InvoicingPipelineTest {
             ZonedDateTime.of(2017, 10, 4, 0, 0, 0, 0, ZoneId.of("UTC")),
             "theRegistrar",
             "234",
+            "",
             "test",
             "RENEW",
             "mydomain.test",
@@ -90,6 +91,7 @@ public class InvoicingPipelineTest {
             ZonedDateTime.of(2017, 10, 4, 0, 0, 0, 0, ZoneId.of("UTC")),
             "theRegistrar",
             "234",
+            "",
             "test",
             "RENEW",
             "mydomain2.test",
@@ -104,6 +106,7 @@ public class InvoicingPipelineTest {
             ZonedDateTime.of(2017, 9, 29, 0, 0, 0, 0, ZoneId.of("UTC")),
             "theRegistrar",
             "234",
+            "",
             "hello",
             "CREATE",
             "mydomain3.hello",
@@ -116,8 +119,9 @@ public class InvoicingPipelineTest {
             1,
             ZonedDateTime.of(2017, 10, 4, 0, 0, 0, 0, ZoneId.of("UTC")),
             ZonedDateTime.of(2017, 10, 4, 0, 0, 0, 0, ZoneId.of("UTC")),
-            "googledomains",
+            "bestdomains",
             "456",
+            "116688",
             "test",
             "RENEW",
             "mydomain4.test",
@@ -132,6 +136,7 @@ public class InvoicingPipelineTest {
             ZonedDateTime.of(2017, 10, 4, 0, 0, 0, 0, ZoneId.of("UTC")),
             "anotherRegistrar",
             "789",
+            "",
             "test",
             "CREATE",
             "mydomain5.test",
@@ -155,9 +160,9 @@ public class InvoicingPipelineTest {
         ImmutableList.of(
             "1,2017-10-02 00:00:00 UTC,2017-09-29 00:00:00 UTC,theRegistrar,234,"
                 + "hello,CREATE,mydomain3.hello,REPO-ID,5,JPY,70.75,"),
-        "invoice_details_2017-10_googledomains_test.csv",
+        "invoice_details_2017-10_bestdomains_test.csv",
         ImmutableList.of(
-            "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,googledomains,456,"
+            "1,2017-10-04 00:00:00 UTC,2017-10-04 00:00:00 UTC,bestdomains,456,"
                 + "test,RENEW,mydomain4.test,REPO-ID,1,USD,20.50,"),
         "invoice_details_2017-10_anotherRegistrar_test.csv",
         ImmutableList.of(
@@ -171,8 +176,8 @@ public class InvoicingPipelineTest {
             + "RENEW | TLD: test | TERM: 3-year,20.50,USD,",
         "2017-10-01,2022-09-30,234,70.75,JPY,10125,1,PURCHASE,theRegistrar - hello,1,"
             + "CREATE | TLD: hello | TERM: 5-year,70.75,JPY,",
-        "2017-10-01,2018-09-30,456,20.50,USD,10125,1,PURCHASE,googledomains - test,1,"
+        "2017-10-01,2018-09-30,456,20.50,USD,10125,1,PURCHASE,bestdomains - test,1,"
-            + "RENEW | TLD: test | TERM: 1-year,20.50,USD,",
+            + "RENEW | TLD: test | TERM: 1-year,20.50,USD,116688",
         "2017-10-01,2018-09-30,789,0.00,USD,10125,1,PURCHASE,anotherRegistrar - test,1,"
             + "CREATE | TLD: test | TERM: 1-year,0.00,USD,");
   }
@@ -187,7 +192,7 @@ public class InvoicingPipelineTest {
     for (Entry<String, ImmutableList<String>> entry : getExpectedDetailReportMap().entrySet()) {
       ImmutableList<String> detailReport = resultFileContents(entry.getKey());
       assertThat(detailReport.get(0))
-          .isEqualTo("id,billingTime,eventTime,registrarId,billingId,tld,action,"
+          .isEqualTo("id,billingTime,eventTime,registrarId,billingId,poNumber,tld,action,"
               + "domain,repositoryId,years,currency,amount,flags");
       assertThat(detailReport.subList(1, detailReport.size()))
           .containsExactlyElementsIn(entry.getValue());

javatests/google/registry/beam/invoicing/testdata/billing_events_test.sql

@@ -22,6 +22,7 @@ SELECT
   eventTime,
   BillingEvent.clientId AS registrarId,
   RegistrarData.accountId AS billingId,
+  RegistrarData.poNumber AS poNumber,
   tld,
   reason as action,
   targetId as domain,
@@ -63,6 +64,7 @@ JOIN (
   SELECT
     __key__.name AS clientId,
     billingIdentifier,
+    IFNULL(poNumber, '') AS poNumber,
     r.billingAccountMap.currency[SAFE_OFFSET(index)] AS currency,
     r.billingAccountMap.accountId[SAFE_OFFSET(index)] AS accountId
   FROM

javatests/google/registry/model/testdata/schema.txt

@@ -571,6 +571,7 @@ class google.registry.model.registrar.Registrar {
   java.lang.String passwordHash;
   java.lang.String phoneNumber;
   java.lang.String phonePasscode;
+  java.lang.String poNumber;
   java.lang.String registrarName;
   java.lang.String salt;
   java.lang.String url;

javatests/google/registry/tools/CreateRegistrarCommandTest.java

@@ -91,6 +91,7 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
     assertThat(registrar.getLastUpdateTime()).isEqualTo(registrar.getCreationTime());
     assertThat(registrar.getBlockPremiumNames()).isFalse();
     assertThat(registrar.getPremiumPriceAckRequired()).isFalse();
+    assertThat(registrar.getPoNumber()).isEmpty();
 
     verify(connection).send(
         eq("/_dr/admin/createGroups"),
@@ -416,6 +417,28 @@ public class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarC
     assertThat(registrar.get().getBillingIdentifier()).isEqualTo(12345);
   }
 
+  @Test
+  public void testSuccess_poNumber() throws Exception {
+    runCommandForced(
+        "--name=blobio",
+        "--password=some_password",
+        "--registrar_type=REAL",
+        "--iana_id=8",
+        "--po_number=AA55G",
+        "--passcode=01234",
+        "--icann_referral_email=foo@bar.test",
+        "--street=\"123 Fake St\"",
+        "--city Fakington",
+        "--state MA",
+        "--zip 00351",
+        "--cc US",
+        "clientz");
+
+    Optional<Registrar> registrar = Registrar.loadByClientId("clientz");
+    assertThat(registrar).isPresent();
+    assertThat(registrar.get().getPoNumber()).hasValue("AA55G");
+  }
+
   @Test
   public void testSuccess_billingAccountMap() throws Exception {
     runCommandForced(

javatests/google/registry/tools/UpdateRegistrarCommandTest.java

@@ -16,6 +16,7 @@ package google.registry.tools;
 
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT_HASH;
 import static google.registry.testing.DatastoreHelper.createTlds;
@@ -32,6 +33,7 @@ import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.Registrar.State;
 import google.registry.model.registrar.Registrar.Type;
 import google.registry.util.CidrAddressBlock;
+import java.util.Optional;
 import org.joda.money.CurrencyUnit;
 import org.joda.time.DateTime;
 import org.junit.Test;
@@ -205,6 +207,13 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
     assertThat(loadRegistrar("NewRegistrar").getBillingIdentifier()).isEqualTo(12345);
   }
 
+  @Test
+  public void testSuccess_poNumber() throws Exception {
+    assertThat(loadRegistrar("NewRegistrar").getPoNumber()).isEmpty();
+    runCommand("--po_number=52345", "--force", "NewRegistrar");
+    assertThat(loadRegistrar("NewRegistrar").getPoNumber()).hasValue("52345");
+  }
+
   @Test
   public void testSuccess_billingAccountMap() throws Exception {
     assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty();
@@ -698,4 +707,24 @@ public class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarC
         IllegalArgumentException.class,
         () -> runCommand("--name tHeRe GiStRaR", "--force", "NewRegistrar"));
   }
+
+  @Test
+  public void testSuccess_poNumberNotSpecified_doesntWipeOutExisting() throws Exception {
+    Registrar registrar =
+        persistResource(
+            loadRegistrar("NewRegistrar").asBuilder().setPoNumber(Optional.of("1664")).build());
+    assertThat(registrar.testPassword("some_password")).isFalse();
+    runCommand("--password=some_password", "--force", "NewRegistrar");
+    Registrar reloadedRegistrar = loadRegistrar("NewRegistrar");
+    assertThat(reloadedRegistrar.testPassword("some_password")).isTrue();
+    assertThat(reloadedRegistrar.getPoNumber()).hasValue("1664");
+  }
+
+  @Test
+  public void testSuccess_poNumber_canBeBlanked() throws Exception {
+    persistResource(
+        loadRegistrar("NewRegistrar").asBuilder().setPoNumber(Optional.of("1664")).build());
+    runCommand("--po_number=null", "--force", "NewRegistrar");
+    assertThat(loadRegistrar("NewRegistrar").getPoNumber()).isEmpty();
+  }
 }