Skip to content

Commit

Permalink
invoice: switch to Period-based BillingPeriod
Browse files Browse the repository at this point in the history 
Add a few tests for 30-days plans, to verify the behavior.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>
  • Loading branch information
@pierre
pierre committed Apr 14, 2016
1 parent 69d78b8 commit db4d2da
Show file tree
Hide file tree
Showing 10 changed files with 354 additions and 73 deletions.
73 changes: 73 additions & 0 deletions beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegration.java
View file
Expand Up @@ -733,4 +733,77 @@ public void testFixedTermPlan() throws Exception {
invoiceChecker.checkInvoice(account.getId(), 14, callContext, expectedInvoices); invoiceChecker.checkInvoice(account.getId(), 14, callContext, expectedInvoices);
expectedInvoices.clear(); expectedInvoices.clear();
} }

@Test(groups = "slow")
public void testThirtyDaysPlanWithFixedTermMonthlyAddOn() throws Exception {
// Set clock to the initial start date - we implicitly assume here that the account timezone is UTC
clock.setDay(new LocalDate(2015, 4, 1));

final AccountData accountData = getAccountData(1);
final Account account = createAccountWithNonOsgiPaymentMethod(accountData);
accountChecker.checkAccount(account.getId(), accountData, callContext);

final List<ExpectedInvoiceItemCheck> expectedInvoices = new ArrayList<ExpectedInvoiceItemCheck>();

// First invoice
final DefaultEntitlement bpSubscription = createBaseEntitlementAndCheckForCompletion(account.getId(), "bundleKey", "Pistol", ProductCategory.BASE, BillingPeriod.THIRTY_DAYS, NextEvent.CREATE, /* NextEvent.BLOCK, */ NextEvent.INVOICE);

expectedInvoices.add(new ExpectedInvoiceItemCheck(new LocalDate(2015, 4, 1), null, InvoiceItemType.FIXED, BigDecimal.ZERO));
invoiceChecker.checkInvoice(account.getId(), 1, callContext, expectedInvoices);
expectedInvoices.clear();

// Second invoice -> first recurring for Refurbish-Maintenance
addAOEntitlementAndCheckForCompletion(bpSubscription.getBundleId(), "Refurbish-Maintenance", ProductCategory.ADD_ON, BillingPeriod.MONTHLY, NextEvent.CREATE, /* NextEvent.BLOCK, */ NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);

expectedInvoices.add(new ExpectedInvoiceItemCheck(new LocalDate(2015, 4, 1), null, InvoiceItemType.FIXED, new BigDecimal("599.95")));
expectedInvoices.add(new ExpectedInvoiceItemCheck(new LocalDate(2015, 4, 1), new LocalDate(2015, 5, 1), InvoiceItemType.RECURRING, new BigDecimal("199.95")));
invoiceChecker.checkInvoice(account.getId(), 2, callContext, expectedInvoices);
expectedInvoices.clear();

// 2015-5-1
expectedInvoices.add(new ExpectedInvoiceItemCheck(new LocalDate(2015, 5, 1), new LocalDate(2015, 5, 31), InvoiceItemType.RECURRING, new BigDecimal("29.95")));
expectedInvoices.add(new ExpectedInvoiceItemCheck(new LocalDate(2015, 5, 1), new LocalDate(2015, 6, 1), InvoiceItemType.RECURRING, new BigDecimal("199.95")));
busHandler.pushExpectedEvents(NextEvent.PHASE, NextEvent.NULL_INVOICE, NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
clock.addDays(30); // Also = 1 month because or initial date 2015, 4, 1
assertListenerStatus();
invoiceChecker.checkInvoice(account.getId(), 3, callContext, expectedInvoices);
expectedInvoices.clear();

// Next 20 invoices, including last recurring for Refurbish-Maintenance
LocalDate startDateBase = new LocalDate(2015, 5, 31);
LocalDate startDateAddOn = new LocalDate(2015, 6, 1);
for (int i = 0; i < 10; i++) {
final LocalDate endDateBase = startDateBase.plusDays(30);
expectedInvoices.add(new ExpectedInvoiceItemCheck(startDateBase, endDateBase, InvoiceItemType.RECURRING, new BigDecimal("29.95")));
busHandler.pushExpectedEvents(NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
clock.setDay(startDateBase);
assertListenerStatus();
invoiceChecker.checkInvoice(account.getId(), 4 + 2 * i, callContext, expectedInvoices);
expectedInvoices.clear();

final LocalDate endDateAddOn = startDateAddOn.plusMonths(1);
expectedInvoices.add(new ExpectedInvoiceItemCheck(startDateAddOn, endDateAddOn, InvoiceItemType.RECURRING, new BigDecimal("199.95")));
busHandler.pushExpectedEvents(NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
clock.setDay(startDateAddOn);
assertListenerStatus();
invoiceChecker.checkInvoice(account.getId(), 5 + 2 * i, callContext, expectedInvoices);
expectedInvoices.clear();

startDateBase = endDateBase;
startDateAddOn = endDateAddOn;
}
// clock at 2016-03-01

expectedInvoices.add(new ExpectedInvoiceItemCheck(new LocalDate(2016, 3, 26), new LocalDate(2016, 4, 25), InvoiceItemType.RECURRING, new BigDecimal("29.95")));
busHandler.pushExpectedEvents(NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
clock.addDays(26);

This comment has been minimized.

Sign in to view

Copy link
@sbrossie

sbrossie Apr 14, 2016

Member

Two minor observations:

  1. The comment above says clock at 2016-03-01, so if the goal is to trigger an invoice at 2016, 3, 26, why do we add 26 days (and not 25)?
  2. The whole test uses clock.setDay instead of clock.addDays, might make sense to keep the same all the way (hard to understand why suddenly we need to add 26 or (25 days).

This comment has been minimized.

Sign in to view

Copy link
@pierre

pierre Apr 14, 2016

Author Member

The comment above says clock at 2016-03-01, so if the goal is to trigger an invoice at 2016, 3, 26, why do we add 26 days (and not 25)?

Ah sure, can fix.

The whole test uses clock.setDay instead of clock.addDays, might make sense to keep the same all the way (hard to understand why suddenly we need to add 26 or (25 days).

I'll update it. The reason I need to use setDay in the for loop is because we have two trains of invoice notifications in parallel, and they keep drifting apart (because of the 30 days plan). A bit hard to do with addDays like the rest of the tests.

See a427b44.
assertListenerStatus();
invoiceChecker.checkInvoice(account.getId(), 24, callContext, expectedInvoices);
expectedInvoices.clear();

// We check there is no more recurring for Refurbish-Maintenance
busHandler.pushExpectedEvents(NextEvent.NULL_INVOICE);
clock.addDays(15);
assertListenerStatus();
}
} }
2 changes: 1 addition & 1 deletion beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationBase.java
View file
Expand Up @@ -586,7 +586,7 @@ public Entitlement apply(@Nullable final Void dontcare) {
assertNotNull(entitlement); assertNotNull(entitlement);
return entitlement; return entitlement;
} catch (final EntitlementApiException e) { } catch (final EntitlementApiException e) {
fail(); fail("Unable to create entitlement", e);
return null; return null;
} }
} }
Expand Down
39 changes: 38 additions & 1 deletion catalog/src/test/resources/catalogTest.xml
View file
Expand Up @@ -275,7 +275,43 @@
</recurring> </recurring>
</finalPhase> </finalPhase>
</plan> </plan>

<plan name="pistol-thirty-days">
<product>Pistol</product>
<initialPhases>
<phase type="TRIAL">
<duration>
<unit>DAYS</unit>
<number>30</number>
</duration>
<fixed>
<fixedPrice>
</fixedPrice>
</fixed>
</phase>
</initialPhases>
<finalPhase type="EVERGREEN">
<duration>
<unit>UNLIMITED</unit>
</duration>
<recurring>
<billingPeriod>THIRTY_DAYS</billingPeriod>
<recurringPrice>
<price>
<currency>GBP</currency>
<value>29.95</value>
</price>
<price>
<currency>EUR</currency>
<value>29.95</value>
</price>
<price>
<currency>USD</currency>
<value>29.95</value>
</price>
</recurringPrice>
</recurring>
</finalPhase>
</plan>
<plan name="pistol-monthly"> <plan name="pistol-monthly">
<product>Pistol</product> <product>Pistol</product>
<initialPhases> <initialPhases>
Expand Down Expand Up @@ -1101,6 +1137,7 @@
<priceLists> <priceLists>
<defaultPriceList name="DEFAULT"> <defaultPriceList name="DEFAULT">
<plans> <plans>
<plan>pistol-thirty-days</plan>
<plan>blowdart-monthly</plan> <plan>blowdart-monthly</plan>
<plan>pistol-monthly</plan> <plan>pistol-monthly</plan>
<plan>shotgun-monthly</plan> <plan>shotgun-monthly</plan>
Expand Down
39 changes: 19 additions & 20 deletions invoice/src/main/java/org/killbill/billing/invoice/generator/BillingIntervalDetail.java
View file
@@ -1,7 +1,9 @@
/* /*
* Copyright 2010-2013 Ning, Inc. * Copyright 2010-2014 Ning, Inc.
* Copyright 2014-2016 Groupon, Inc
* Copyright 2014-2016 The Billing Project, LLC
* *
* Ning licenses this file to you under the Apache License, version 2.0 * The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the * (the "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at: * License. You may obtain a copy of the License at:
* *
Expand Down Expand Up @@ -45,7 +47,11 @@ public BillingIntervalDetail(final LocalDate startDate,
this.startDate = startDate; this.startDate = startDate;
this.endDate = endDate; this.endDate = endDate;
this.targetDate = targetDate; this.targetDate = targetDate;
this.billingCycleDay = billingCycleDay; if (billingPeriod.getPeriod().getMonths() != 0 || billingPeriod.getPeriod().getYears() != 0) {
this.billingCycleDay = billingCycleDay;
} else {
this.billingCycleDay = startDate.getDayOfMonth();

This comment has been minimized.

Sign in to view

Copy link
@sbrossie

sbrossie Apr 14, 2016

Member

confused?

This comment has been minimized.

Sign in to view

Copy link
@pierre

pierre Apr 14, 2016

Author Member

In case of billing with a period that is not a multiple of months, we cannot use the BCD. For instance, for weekly billing (every 7 days), we shouldn't try to align the billing date.

For these, it's essentially a subscription start date alignment.
}
this.billingPeriod = billingPeriod; this.billingPeriod = billingPeriod;
this.billingMode = billingMode; this.billingMode = billingMode;
computeAll(); computeAll();
Expand All @@ -59,9 +65,8 @@ public LocalDate getEffectiveEndDate() {
return effectiveEndDate; return effectiveEndDate;
} }


public LocalDate getFutureBillingDateFor(int nbPeriod) { public LocalDate getFutureBillingDateFor(final int nbPeriod) {
final int numberOfMonthsPerBillingPeriod = billingPeriod.getNumberOfMonths(); final LocalDate proposedDate = InvoiceDateUtils.advanceByNPeriods(firstBillingCycleDate, billingPeriod, nbPeriod);
LocalDate proposedDate = firstBillingCycleDate.plusMonths((nbPeriod) * numberOfMonthsPerBillingPeriod);
return alignProposedBillCycleDate(proposedDate, billingCycleDay); return alignProposedBillCycleDate(proposedDate, billingCycleDay);
} }


Expand All @@ -70,8 +75,7 @@ public LocalDate getLastBillingCycleDate() {
} }


public LocalDate getNextBillingCycleDate() { public LocalDate getNextBillingCycleDate() {
final int numberOfMonthsInPeriod = billingPeriod.getNumberOfMonths(); final LocalDate proposedDate = lastBillingCycleDate != null ? lastBillingCycleDate.plus(billingPeriod.getPeriod()) : firstBillingCycleDate;
final LocalDate proposedDate = lastBillingCycleDate != null ? lastBillingCycleDate.plusMonths(numberOfMonthsInPeriod) : firstBillingCycleDate;
final LocalDate nextBillingCycleDate = alignProposedBillCycleDate(proposedDate, billingCycleDay); final LocalDate nextBillingCycleDate = alignProposedBillCycleDate(proposedDate, billingCycleDay);
return nextBillingCycleDate; return nextBillingCycleDate;
} }
Expand All @@ -88,8 +92,7 @@ private void computeAll() {
calculateLastBillingCycleDate(); calculateLastBillingCycleDate();
} }


@VisibleForTesting private void calculateFirstBillingCycleDate() {
void calculateFirstBillingCycleDate() {


final int lastDayOfMonth = startDate.dayOfMonth().getMaximumValue(); final int lastDayOfMonth = startDate.dayOfMonth().getMaximumValue();
final LocalDate billingCycleDate; final LocalDate billingCycleDate;
Expand All @@ -99,10 +102,9 @@ void calculateFirstBillingCycleDate() {
billingCycleDate = new LocalDate(startDate.getYear(), startDate.getMonthOfYear(), billingCycleDay, startDate.getChronology()); billingCycleDate = new LocalDate(startDate.getYear(), startDate.getMonthOfYear(), billingCycleDay, startDate.getChronology());
} }


final int numberOfMonthsInPeriod = billingPeriod.getNumberOfMonths();
LocalDate proposedDate = billingCycleDate; LocalDate proposedDate = billingCycleDate;
while (proposedDate.isBefore(startDate)) { while (proposedDate.isBefore(startDate)) {
proposedDate = proposedDate.plusMonths(numberOfMonthsInPeriod); proposedDate = proposedDate.plus(billingPeriod.getPeriod());
} }
firstBillingCycleDate = alignProposedBillCycleDate(proposedDate, billingCycleDay); firstBillingCycleDate = alignProposedBillCycleDate(proposedDate, billingCycleDay);
} }
Expand All @@ -127,15 +129,13 @@ private void calculateInArrearEffectiveEndDate() {
return; return;
} }


final int numberOfMonthsInPeriod = billingPeriod.getNumberOfMonths();
int numberOfPeriods = 0; int numberOfPeriods = 0;
LocalDate proposedDate = firstBillingCycleDate; LocalDate proposedDate = firstBillingCycleDate;
LocalDate nextProposedDate = proposedDate.plusMonths(numberOfPeriods * numberOfMonthsInPeriod); LocalDate nextProposedDate = InvoiceDateUtils.advanceByNPeriods(firstBillingCycleDate, billingPeriod, numberOfPeriods);



while (!nextProposedDate.isAfter(targetDate)) { while (!nextProposedDate.isAfter(targetDate)) {
proposedDate = nextProposedDate; proposedDate = nextProposedDate;
nextProposedDate = firstBillingCycleDate.plusMonths(numberOfPeriods * numberOfMonthsInPeriod); nextProposedDate = InvoiceDateUtils.advanceByNPeriods(firstBillingCycleDate, billingPeriod, numberOfPeriods);
numberOfPeriods += 1; numberOfPeriods += 1;
} }
proposedDate = alignProposedBillCycleDate(proposedDate, billingCycleDay); proposedDate = alignProposedBillCycleDate(proposedDate, billingCycleDay);
Expand All @@ -161,12 +161,11 @@ private void calculateInAdvanceEffectiveEndDate() {
return; return;
} }


final int numberOfMonthsInPeriod = billingPeriod.getNumberOfMonths();
int numberOfPeriods = 0; int numberOfPeriods = 0;
LocalDate proposedDate = firstBillingCycleDate; LocalDate proposedDate = firstBillingCycleDate;


while (!proposedDate.isAfter(targetDate)) { while (!proposedDate.isAfter(targetDate)) {
proposedDate = firstBillingCycleDate.plusMonths(numberOfPeriods * numberOfMonthsInPeriod); proposedDate = InvoiceDateUtils.advanceByNPeriods(firstBillingCycleDate, billingPeriod, numberOfPeriods);
numberOfPeriods += 1; numberOfPeriods += 1;
} }
proposedDate = alignProposedBillCycleDate(proposedDate, billingCycleDay); proposedDate = alignProposedBillCycleDate(proposedDate, billingCycleDay);
Expand All @@ -191,12 +190,12 @@ private void calculateLastBillingCycleDate() {
LocalDate proposedDate = firstBillingCycleDate; LocalDate proposedDate = firstBillingCycleDate;
int numberOfPeriods = 0; int numberOfPeriods = 0;
while (!proposedDate.isAfter(effectiveEndDate)) { while (!proposedDate.isAfter(effectiveEndDate)) {
proposedDate = firstBillingCycleDate.plusMonths(numberOfPeriods * billingPeriod.getNumberOfMonths()); proposedDate = InvoiceDateUtils.advanceByNPeriods(firstBillingCycleDate, billingPeriod, numberOfPeriods);
numberOfPeriods += 1; numberOfPeriods += 1;
} }


// Our proposed date is billingCycleDate prior to the effectiveEndDate // Our proposed date is billingCycleDate prior to the effectiveEndDate
proposedDate = proposedDate.plusMonths(-billingPeriod.getNumberOfMonths()); proposedDate = proposedDate.minus(billingPeriod.getPeriod());
proposedDate = alignProposedBillCycleDate(proposedDate, billingCycleDay); proposedDate = alignProposedBillCycleDate(proposedDate, billingCycleDay);

This comment has been minimized.

Sign in to view

Copy link
@sbrossie

sbrossie Apr 14, 2016

Member

I am a bit confused about our existing code (i think your change is in the same spirit of what we had before, but still want to take that opportunity to comment on it): I thought we never did minus because of the fact that date arithmetic does not play nicely (i.e originalDate.plusMonth(1).minusMonth(1) != originalDate). I think this works because we have the alignProposedBillCycleDate below that will do its magic to realign things correctly, but still seems a bit dangerous.

This comment has been minimized.

Sign in to view

Copy link
@pierre

pierre Apr 14, 2016

Author Member

We should be safe here because we never go forward afterwards: lastBillingCycleDate is computed once.

The reason we go backward exactly one period is just an artifact of the code: we find the first date strictly after effectiveEndDate and go back exactly one period so that lastBillingCycleDate <= effectiveEndDate.


if (proposedDate.isBefore(firstBillingCycleDate)) { if (proposedDate.isBefore(firstBillingCycleDate)) {
Expand Down
40 changes: 34 additions & 6 deletions invoice/src/main/java/org/killbill/billing/invoice/generator/InvoiceDateUtils.java
View file
Expand Up @@ -23,18 +23,30 @@
import org.joda.time.Days; import org.joda.time.Days;
import org.joda.time.LocalDate; import org.joda.time.LocalDate;
import org.joda.time.Months; import org.joda.time.Months;

import org.joda.time.Weeks;
import org.joda.time.Years;
import org.killbill.billing.catalog.api.BillingPeriod; import org.killbill.billing.catalog.api.BillingPeriod;
import org.killbill.billing.util.currency.KillBillMoney; import org.killbill.billing.util.currency.KillBillMoney;


import com.google.common.annotations.VisibleForTesting;

public class InvoiceDateUtils { public class InvoiceDateUtils {


public static int calculateNumberOfWholeBillingPeriods(final LocalDate startDate, final LocalDate endDate, final BillingPeriod billingPeriod) { public static int calculateNumberOfWholeBillingPeriods(final LocalDate startDate, final LocalDate endDate, final BillingPeriod billingPeriod) {
final int numberOfMonths = Months.monthsBetween(startDate, endDate).getMonths(); final int numberBetween;
final int numberOfMonthsInPeriod = billingPeriod.getNumberOfMonths(); final int numberInPeriod;
return numberOfMonths / numberOfMonthsInPeriod; if (billingPeriod.getPeriod().getDays() != 0) {
numberBetween = Days.daysBetween(startDate, endDate).getDays();
numberInPeriod = billingPeriod.getPeriod().getDays();
} else if (billingPeriod.getPeriod().getWeeks() != 0) {
numberBetween = Weeks.weeksBetween(startDate, endDate).getWeeks();
numberInPeriod = billingPeriod.getPeriod().getWeeks();
} else if (billingPeriod.getPeriod().getMonths() != 0) {
numberBetween = Months.monthsBetween(startDate, endDate).getMonths();
numberInPeriod = billingPeriod.getPeriod().getMonths();
} else {
numberBetween = Years.yearsBetween(startDate, endDate).getYears();
numberInPeriod = billingPeriod.getPeriod().getYears();
}
return numberBetween / numberInPeriod;
} }

This comment has been minimized.

Sign in to view

Copy link
@sbrossie

sbrossie Apr 14, 2016

Member

nice.


public static BigDecimal calculateProRationBeforeFirstBillingPeriod(final LocalDate startDate, final LocalDate nextBillingCycleDate, public static BigDecimal calculateProRationBeforeFirstBillingPeriod(final LocalDate startDate, final LocalDate nextBillingCycleDate,
Expand Down Expand Up @@ -73,4 +85,20 @@ public static BigDecimal calculateProrationBetweenDates(final LocalDate startDat


return days.divide(daysInPeriod, KillBillMoney.MAX_SCALE, KillBillMoney.ROUNDING_METHOD); return days.divide(daysInPeriod, KillBillMoney.MAX_SCALE, KillBillMoney.ROUNDING_METHOD);
} }

public static LocalDate advanceByNPeriods(final LocalDate initialDate, final BillingPeriod billingPeriod, final int nbPeriods) {
LocalDate proposedDate = initialDate;
for (int i = 0; i < nbPeriods; i++) {
proposedDate = proposedDate.plus(billingPeriod.getPeriod());
}
return proposedDate;
}

public static LocalDate returnByNPeriods(final LocalDate initialDate, final BillingPeriod billingPeriod, final int nbPeriods) {
LocalDate proposedDate = initialDate;

This comment has been minimized.

Sign in to view

Copy link
@sbrossie

sbrossie Apr 14, 2016

Member

return is confusing. stepBackByNPeriod or recedeByNPeriod. Also we really need it?

This comment has been minimized.

Sign in to view

Copy link
@pierre

pierre Apr 14, 2016

Author Member

recede is nicer indeed (couldn't find a better name when I wrote the code).

Fixed in b4c50ae.
for (int i = 0; i < nbPeriods; i++) {
proposedDate = proposedDate.minus(billingPeriod.getPeriod());
}
return proposedDate;
}
} }
9 changes: 5 additions & 4 deletions invoice/src/main/java/org/killbill/billing/invoice/usage/RawUsageOptimizer.java
View file
@@ -1,6 +1,6 @@
/* /*
* Copyright 2014-2015 Groupon, Inc * Copyright 2014-2016 Groupon, Inc
* Copyright 2014-2015 The Billing Project, LLC * Copyright 2014-2016 The Billing Project, LLC
* *
* The Billing Project licenses this file to you under the Apache License, version 2.0 * The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the * (the "License"); you may not use this file except in compliance with the
Expand Down Expand Up @@ -30,6 +30,7 @@
import org.killbill.billing.catalog.api.BillingPeriod; import org.killbill.billing.catalog.api.BillingPeriod;
import org.killbill.billing.catalog.api.Usage; import org.killbill.billing.catalog.api.Usage;
import org.killbill.billing.invoice.api.InvoiceItem; import org.killbill.billing.invoice.api.InvoiceItem;
import org.killbill.billing.invoice.generator.InvoiceDateUtils;
import org.killbill.billing.invoice.model.UsageInvoiceItem; import org.killbill.billing.invoice.model.UsageInvoiceItem;
import org.killbill.billing.usage.InternalUserApi; import org.killbill.billing.usage.InternalUserApi;
import org.killbill.billing.usage.RawUsage; import org.killbill.billing.usage.RawUsage;
Expand Down Expand Up @@ -101,7 +102,7 @@ public BillingPeriod apply(final Usage input) {
int idx = 0; int idx = 0;
for (BillingPeriod bp : BillingPeriod.values()) { for (BillingPeriod bp : BillingPeriod.values()) {
if (bp != BillingPeriod.NO_BILLING_PERIOD) { if (bp != BillingPeriod.NO_BILLING_PERIOD) {
final LocalDate makerDateThanCannotBeChosenAsTheMinOfAllDates = targetDate.plusMonths(config.getMaxRawUsagePreviousPeriod() * bp.getNumberOfMonths()); final LocalDate makerDateThanCannotBeChosenAsTheMinOfAllDates = InvoiceDateUtils.advanceByNPeriods(targetDate, bp, config.getMaxRawUsagePreviousPeriod());
perBillingPeriodMostRecentConsumableInArrearItemEndDate[idx++] = (knownUsageBillingPeriod.contains(bp)) ? null : makerDateThanCannotBeChosenAsTheMinOfAllDates; perBillingPeriodMostRecentConsumableInArrearItemEndDate[idx++] = (knownUsageBillingPeriod.contains(bp)) ? null : makerDateThanCannotBeChosenAsTheMinOfAllDates;
} }
} }
Expand All @@ -127,7 +128,7 @@ public BillingPeriod apply(final Usage input) {
for (BillingPeriod bp : BillingPeriod.values()) { for (BillingPeriod bp : BillingPeriod.values()) {
if (bp != BillingPeriod.NO_BILLING_PERIOD) { if (bp != BillingPeriod.NO_BILLING_PERIOD) {
final LocalDate tmp = perBillingPeriodMostRecentConsumableInArrearItemEndDate[idx]; final LocalDate tmp = perBillingPeriodMostRecentConsumableInArrearItemEndDate[idx];
final LocalDate targetBillingPeriodDate = tmp != null ? tmp.minusMonths(config.getMaxRawUsagePreviousPeriod() * bp.getNumberOfMonths()) : null; final LocalDate targetBillingPeriodDate = tmp != null ? InvoiceDateUtils.returnByNPeriods(tmp, bp, config.getMaxRawUsagePreviousPeriod()) : null;
if (targetStartDate == null || (targetBillingPeriodDate != null && targetBillingPeriodDate.compareTo(targetStartDate) < 0)) { if (targetStartDate == null || (targetBillingPeriodDate != null && targetBillingPeriodDate.compareTo(targetStartDate) < 0)) {
targetStartDate = targetBillingPeriodDate; targetStartDate = targetBillingPeriodDate;
} }
Expand Down

2 comments on commit db4d2da

@sbrossie
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good.

Few comments we should discuss (probably more for my understanding). Also, is that true that nowhere in our code do we see plusMonths orminusMonths after that change?

@pierre
Copy link
Member Author

@pierre pierre commented on db4d2da Apr 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still have:

Please sign in to comment.