Verify billing recurrence expansion is caught up before invoicing

[gbrodman]

This change is 

[CydeWeys]

@CydeWeys made 1 comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on gbrodman).

---

core/src/main/java/google/registry/reporting/billing/GenerateInvoicesAction.java line 122 at r1 (raw file):

                  () ->
                      tm().loadByKeyIfPresent(Cursor.createGlobalVKey(RECURRING_BILLING))
                          .orElse(Cursor.createGlobal(RECURRING_BILLING, START_INSTANT))

This is a round-about, semi-confusing way to handle the simple case of "the cursor doesn't exist".

This is clearer to me:

Optional<Cursor> lastBillingExpansion = .... tm().loadByKeyIfPresent(...);
checkState(lastBillingExpansion.isPresent() && lastBillingExpansion.get().getCursorTime().isBefore(...), ...);

[CydeWeys]

@CydeWeys made 2 comments.
Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on gbrodman).

---

core/src/main/java/google/registry/reporting/billing/GenerateInvoicesAction.java line 122 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

This is a round-about, semi-confusing way to handle the simple case of "the cursor doesn't exist".

This is clearer to me:

Optional<Cursor> lastBillingExpansion = .... tm().loadByKeyIfPresent(...);
checkState(lastBillingExpansion.isPresent() && lastBillingExpansion.get().getCursorTime().isBefore(...), ...);

Or maybe broken up into two separate checkState() calls for clearer error messages.

---

core/src/main/java/google/registry/reporting/billing/GenerateInvoicesAction.java line 126 at r1 (raw file):

      checkState(
          !cursorTime.isBefore(startOfNextMonth),
          "BillingRecurrence expansion cursor (%s) is before the start of the next month (%s). "

As currently written, this error message can be a little confusing, as it will say the cursor value is START_OF_TIME when the actual problem is that it doesn't exist.

[CydeWeys]

@CydeWeys made 1 comment.
Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on gbrodman).

---

core/src/main/java/google/registry/reporting/billing/GenerateInvoicesAction.java line 126 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

As currently written, this error message can be a little confusing, as it will say the cursor value is START_OF_TIME when the actual problem is that it doesn't exist.

(Well, it would say that the actual cursor value is 1970-01-01-...., which you'd have to know means START_OF_TIME, so even worse.)

[gbrodman]

@gbrodman made 2 comments.
Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on CydeWeys).

---

core/src/main/java/google/registry/reporting/billing/GenerateInvoicesAction.java line 122 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

Or maybe broken up into two separate checkState() calls for clearer error messages.

sure, splitting things up is always good

---

core/src/main/java/google/registry/reporting/billing/GenerateInvoicesAction.java line 126 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

(Well, it would say that the actual cursor value is 1970-01-01-...., which you'd have to know means START_OF_TIME, so even worse.)

sure

[CydeWeys]

@CydeWeys reviewed 2 files and all commit messages, and resolved 2 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on gbrodman).