Change billing for multi-year domain creation

[weiminyu]

From the second year on, charge the renewal price.

See b/322833077

---

This change is 

[CydeWeys]

Reviewable status: 0 of 16 files reviewed, 3 unresolved discussions (waiting on @weiminyu)

---

core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java line 290 at r1 (raw file):

  }

  private Money getDomainCostWithDiscount(

It might be worth adding a Javadoc here clarifying that for renewals, firstYearCost is the per-year renewal cost and subsequentYearCost is empty, and then how they work for creates, with subsequentYearCost always being present even if in most cases it will be the same as firstYearCost.

---

core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java line 297 at r1 (raw file):

      Optional<Money> subsequentYearCost)
      throws AllocationTokenInvalidForPremiumNameException {
    checkArgument(years > 0, "Domain creation/renew for zero years.");

A better error message here would be something like "Registration years to get cost for must be positive"

The error message as written is saying the error state, rather than the valid state that wasn't met, also it's not technically accurate (if you passed in -1 it would still say "zero years" which is just confusing).

---

core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java line 306 at r1 (raw file):

    if (allocationToken.isPresent()
        && allocationToken.get().getTokenBehavior().equals(TokenBehavior.DEFAULT)
        && (discountedYears = Math.min(years, allocationToken.get().getDiscountYears())) > 0) {

We shouldn't be inlining a variable assignment inside a conditional clause; it's confusing to understand at first pass. Nested ifs (if that's logically the best) would be more understandable.

[weiminyu]

Reviewable status: 0 of 16 files reviewed, 3 unresolved discussions (waiting on @CydeWeys)

---

core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java line 290 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

It might be worth adding a Javadoc here clarifying that for renewals, firstYearCost is the per-year renewal cost and subsequentYearCost is empty, and then how they work for creates, with subsequentYearCost always being present even if in most cases it will be the same as firstYearCost.

Done.

---

core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java line 297 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

A better error message here would be something like "Registration years to get cost for must be positive"

The error message as written is saying the error state, rather than the valid state that wasn't met, also it's not technically accurate (if you passed in -1 it would still say "zero years" which is just confusing).

Done.

---

core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java line 306 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

We shouldn't be inlining a variable assignment inside a conditional clause; it's confusing to understand at first pass. Nested ifs (if that's logically the best) would be more understandable.

Done.

[CydeWeys]

Reviewable status: 0 of 16 files reviewed, 1 unresolved discussion (waiting on @weiminyu)

---

core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java line 320 at r2 (raw file):

                .plus(subsequentYearCost.orElse(firstYearCost).multipliedBy(discountedYears - 1))
                .multipliedBy(allocationToken.get().getDiscountFraction(), RoundingMode.HALF_EVEN);
        totalDomainFlowCost = totalDomainFlowCost.minus(discount);

Rather than reassigning a value to totalDomainFlowCost, this could just be return totalDomainFlowCost.minus(discount);

[CydeWeys]

Reviewable status: 0 of 16 files reviewed, 1 unresolved discussion (waiting on @weiminyu)