killbill-memoizeit

entitlement, subscription: Modify changePlan api to have

4/21/2017 11:11:49 PM

Details

diff --git a/entitlement/src/main/java/org/killbill/billing/entitlement/api/DefaultEntitlement.java b/entitlement/src/main/java/org/killbill/billing/entitlement/api/DefaultEntitlement.java
index c7a0c88..b780626 100644
--- a/entitlement/src/main/java/org/killbill/billing/entitlement/api/DefaultEntitlement.java
+++ b/entitlement/src/main/java/org/killbill/billing/entitlement/api/DefaultEntitlement.java
@@ -641,8 +641,7 @@ public class DefaultEntitlement extends EntityBase implements Entitlement {
             @Override
             public Entitlement doCall(final EntitlementApi entitlementApi, final EntitlementContext updatedPluginContext) throws EntitlementApiException {
 
-                if ((effectiveDate == null && !eventsStream.isEntitlementActive()) ||
-                        (effectiveDate != null && effectiveDate.compareTo(eventsStream.getEntitlementEffectiveStartDate()) < 0)) {
+                if (effectiveDate != null && effectiveDate.compareTo(eventsStream.getEntitlementEffectiveStartDate()) < 0) {
                     throw new EntitlementApiException(ErrorCode.SUB_CHANGE_NON_ACTIVE, getId(), getState());
                 }
 
@@ -718,8 +717,7 @@ public class DefaultEntitlement extends EntityBase implements Entitlement {
             @Override
             public Entitlement doCall(final EntitlementApi entitlementApi, final EntitlementContext updatedPluginContext) throws EntitlementApiException {
 
-                if ((effectiveDate == null && !eventsStream.isEntitlementActive()) ||
-                        (effectiveDate != null && effectiveDate.compareTo(eventsStream.getEntitlementEffectiveStartDate()) < 0)) {
+                if (effectiveDate != null && effectiveDate.compareTo(eventsStream.getEntitlementEffectiveStartDate()) < 0) {
                     throw new EntitlementApiException(ErrorCode.SUB_CHANGE_NON_ACTIVE, getId(), getState());
                 }
 
diff --git a/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlement.java b/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlement.java
index 6023c86..6771c42 100644
--- a/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlement.java
+++ b/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlement.java
@@ -308,13 +308,6 @@ public class TestDefaultEntitlement extends EntitlementTestSuiteWithEmbeddedDB {
         }
 
         try {
-            entitlement.changePlanWithDate(spec2, ImmutableList.<PlanPhasePriceOverride>of(), null, ImmutableList.<PluginProperty>of(), callContext);
-            fail("Changing plan immediately prior the subscription is active is not allowed");
-        } catch (EntitlementApiException e) {
-            assertEquals(e.getCode(), ErrorCode.SUB_CHANGE_NON_ACTIVE.getCode());
-        }
-
-        try {
             entitlement.changePlanWithDate(spec2, ImmutableList.<PlanPhasePriceOverride>of(), startDate.minusDays(1), ImmutableList.<PluginProperty>of(), callContext);
             fail("Changing plan immediately prior the subscription is active is not allowed");
         } catch (EntitlementApiException e) {
diff --git a/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlementApi.java b/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlementApi.java
index 96cbcab..718b61e 100644
--- a/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlementApi.java
+++ b/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlementApi.java
@@ -604,14 +604,6 @@ public class TestDefaultEntitlementApi extends EntitlementTestSuiteWithEmbeddedD
         assertEquals(entitlement.getState(), EntitlementState.PENDING);
         assertEquals(entitlement.getEffectiveStartDate(), entitlementDate);
 
-        // effectiveDate is null and we are prior entitlementDate so call should fail with SUB_CHANGE_NON_ACTIVE
-        final PlanPhaseSpecifier spec2 = new PlanPhaseSpecifier("Pistol",  BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME, null);
-        try {
-            entitlement.changePlanWithDate(spec2, ImmutableList.<PlanPhasePriceOverride>of(), null, ImmutableList.<PluginProperty>of(), callContext);
-            fail("Change plan prior entitlementDate should fail");
-        } catch (EntitlementApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.SUB_CHANGE_NON_ACTIVE.getCode());
-        }
 
 
         // 2013-08-10 : entitlementDate
@@ -624,14 +616,6 @@ public class TestDefaultEntitlementApi extends EntitlementTestSuiteWithEmbeddedD
         entitlement = entitlementApi.getEntitlementForId(entitlement.getId(), callContext);
         assertEquals(entitlement.getState(), EntitlementState.ACTIVE);
 
-        // effectiveDate is null (same as first case above), but we did not reach the billing startDate so will fail with SUB_INVALID_REQUESTED_DATE
-        try {
-            entitlement.changePlanWithDate(spec2, ImmutableList.<PlanPhasePriceOverride>of(), null, ImmutableList.<PluginProperty>of(), callContext);
-            Assert.fail("Change plan prior billingStartDate should fail");
-        } catch (EntitlementApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.SUB_INVALID_REQUESTED_DATE.getCode());
-        }
-
         // 2013-08-12 : billingDate
         testListener.pushExpectedEvents(NextEvent.CREATE);
         clock.addDays(2);
@@ -639,6 +623,7 @@ public class TestDefaultEntitlementApi extends EntitlementTestSuiteWithEmbeddedD
 
 
         // effectiveDate = entitlementDate prior billingDate
+        final PlanPhaseSpecifier spec2 = new PlanPhaseSpecifier("Pistol",  BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME, null);
         try {
             entitlement.changePlanWithDate(spec2, ImmutableList.<PlanPhasePriceOverride>of(), entitlementDate, ImmutableList.<PluginProperty>of(), callContext);
             Assert.fail("Change plan prior billingStartDate should fail");
diff --git a/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java b/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
index 2f9eafe..fb97c35 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
@@ -615,9 +615,10 @@ public class DefaultSubscriptionInternalApi extends SubscriptionApiBase implemen
         // verify the number of subscriptions (of the same kind) allowed per bundle
         final Catalog catalog = catalogService.getFullCatalog(true, true, context);
         final DateTime now = clock.getUTCNow();
-        final DateTime effectiveDate = (requestedDateWithMs != null) ? DefaultClock.truncateMs(requestedDateWithMs) : now;
+        final DateTime effectiveDate = (requestedDateWithMs != null) ? DefaultClock.truncateMs(requestedDateWithMs) : null;
+        final DateTime effectiveCatalogDate = effectiveDate != null? effectiveDate : now;
         final PlanPhasePriceOverridesWithCallContext overridesWithContext = new DefaultPlanPhasePriceOverridesWithCallContext(overrides, callContext);
-        final Plan plan = catalog.createOrFindPlan(spec, overridesWithContext, effectiveDate);
+        final Plan plan = catalog.createOrFindPlan(spec, overridesWithContext, effectiveCatalogDate);
         if (ProductCategory.ADD_ON.toString().equalsIgnoreCase(plan.getProduct().getCategory().toString())) {
             if (plan.getPlansAllowedInBundle() != -1
                 && plan.getPlansAllowedInBundle() > 0
diff --git a/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBaseApiService.java b/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBaseApiService.java
index e257c39..e784b0b 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBaseApiService.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBaseApiService.java
@@ -76,10 +76,8 @@ import org.killbill.clock.Clock;
 import org.killbill.clock.DefaultClock;
 
 import com.google.common.base.Preconditions;
-import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
 import com.google.inject.Inject;
 
 public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiService {
@@ -341,7 +339,7 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
     public DateTime changePlan(final DefaultSubscriptionBase subscription, final PlanSpecifier spec, final List<PlanPhasePriceOverride> overrides, final CallContext context) throws SubscriptionBaseApiException {
         final DateTime now = clock.getUTCNow();
 
-        validateEntitlementState(subscription, null);
+        validateSubscriptionState(subscription, null);
 
         final PlanChangeResult planChangeResult = getPlanChangeResult(subscription, spec, now, context);
         final DateTime effectiveDate = dryRunChangePlan(subscription, spec, null, planChangeResult.getPolicy(), context);
@@ -361,7 +359,7 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
                                                 final DateTime requestedDateWithMs, final CallContext context) throws SubscriptionBaseApiException {
         final DateTime effectiveDate = dryRunChangePlan(subscription, spec, requestedDateWithMs, null, context);
         validateEffectiveDate(subscription, effectiveDate);
-        validateEntitlementState(subscription, requestedDateWithMs);
+        validateSubscriptionState(subscription, requestedDateWithMs);
 
         try {
             doChangePlan(subscription, spec, overrides, effectiveDate, context);
@@ -377,7 +375,7 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
 
         final DateTime effectiveDate = dryRunChangePlan(subscription, spec, null, policy, context);
 
-        validateEntitlementState(subscription, effectiveDate);
+        validateSubscriptionState(subscription, effectiveDate);
         try {
             doChangePlan(subscription, spec, overrides, effectiveDate, context);
         } catch (final CatalogApiException e) {
@@ -499,7 +497,7 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
 
         final TimedPhase currentTimedPhase = planAligner.getCurrentTimedPhaseOnChange(subscription, newPlan, effectiveDate, initialPhaseType, internalTenantContext);
 
-        validateEntitlementState(subscription, effectiveDate);
+        validateSubscriptionState(subscription, effectiveDate);
 
         final SubscriptionBaseEvent changeEvent = new ApiEventChange(new ApiEventBuilder()
                                                                              .setSubscriptionId(subscription.getId())
@@ -618,11 +616,8 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
         }
     }
 
-    private void validateEntitlementState(final DefaultSubscriptionBase subscription, @Nullable final DateTime effectiveDate) throws SubscriptionBaseApiException {
+    private void validateSubscriptionState(final DefaultSubscriptionBase subscription, @Nullable final DateTime effectiveDate) throws SubscriptionBaseApiException {
         final EntitlementState currentState = subscription.getState();
-        if (effectiveDate == null && currentState != EntitlementState.ACTIVE) {
-            throw new SubscriptionBaseApiException(ErrorCode.SUB_CHANGE_NON_ACTIVE, subscription.getId(), currentState);
-        }
         if (effectiveDate != null && effectiveDate.compareTo(subscription.getStartDate()) < 0) {
             throw new SubscriptionBaseApiException(ErrorCode.SUB_CHANGE_NON_ACTIVE, subscription.getId(), currentState);
         }
diff --git a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiChangePlan.java b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiChangePlan.java
index 33a1ef6..cd8aa14 100644
--- a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiChangePlan.java
+++ b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiChangePlan.java
@@ -490,12 +490,6 @@ public class TestUserApiChangePlan extends SubscriptionTestSuiteWithEmbeddedDB {
         // Check we are seeing the right PENDING transition (pistol-monthly), not the START but the CHANGE on the same date
         assertEquals(((DefaultSubscriptionBase) result1.get(0)).getCurrentOrPendingPlan().getName(), "pistol-monthly");
         assertEquals(((DefaultSubscriptionBase) result1.get(0)).getPendingTransition().getTransitionType(), SubscriptionBaseTransitionType.CREATE);
-        try {
-            subscription.changePlan(spec, null, callContext);
-            fail("Change plan should have failed : subscription PENDING");
-        } catch (final SubscriptionBaseApiException e) {
-            assertEquals(e.getCode(), ErrorCode.SUB_CHANGE_NON_ACTIVE.getCode());
-        }
 
         // Second try with date prior to startDate => Call should fail because subscription is PENDING
         try {