killbill-memoizeit

entitlement: ensure plans cannot be changed if blocked This

1/14/2016 8:49:35 PM

Details

diff --git a/api/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseInternalApi.java b/api/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseInternalApi.java
index f6a6c8f..df97745 100644
--- a/api/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseInternalApi.java
+++ b/api/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseInternalApi.java
@@ -1,7 +1,9 @@
 /*
  * Copyright 2010-2011 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
  * License.  You may obtain a copy of the License at:
  *
@@ -26,6 +28,7 @@ import org.joda.time.DateTime;
 import org.killbill.billing.callcontext.InternalCallContext;
 import org.killbill.billing.callcontext.InternalTenantContext;
 import org.killbill.billing.catalog.api.BillingActionPolicy;
+import org.killbill.billing.catalog.api.BillingPeriod;
 import org.killbill.billing.catalog.api.PlanPhasePriceOverride;
 import org.killbill.billing.catalog.api.PlanPhaseSpecifier;
 import org.killbill.billing.entitlement.api.EntitlementAOStatusDryRun;
@@ -82,6 +85,9 @@ public interface SubscriptionBaseInternalApi {
 
     public List<EffectiveSubscriptionInternalEvent> getBillingTransitions(SubscriptionBase subscription, InternalTenantContext context);
 
+    public DateTime getDryRunChangePlanEffectiveDate(SubscriptionBase subscription, String productName, BillingPeriod term,
+                                                     String priceList, DateTime requestedDate, BillingActionPolicy policy, InternalTenantContext context) throws SubscriptionBaseApiException;
+
     public List<EntitlementAOStatusDryRun> getDryRunChangePlanStatus(UUID subscriptionId, @Nullable String baseProductName,
                                                                      DateTime requestedDate, InternalTenantContext context) throws SubscriptionBaseApiException;
 
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 7e8a3dd..a4d2148 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
@@ -465,21 +465,27 @@ public class DefaultEntitlement extends EntityBase implements Entitlement {
                     throw new EntitlementApiException(ErrorCode.SUB_CHANGE_NON_ACTIVE, getId(), getState());
                 }
 
+                final InternalCallContext context = internalCallContextFactory.createInternalCallContext(getAccountId(), callContext);
 
                 final DateTime effectiveChangeDate;
                 try {
-                    effectiveChangeDate = getSubscriptionBase().changePlan(productName, billingPeriod, priceList, overrides, callContext);
+                    effectiveChangeDate = subscriptionInternalApi.getDryRunChangePlanEffectiveDate(getSubscriptionBase(), productName, billingPeriod, priceList, null, null, context);
                 } catch (final SubscriptionBaseApiException e) {
-                    throw new EntitlementApiException(e);
+                    throw new EntitlementApiException(e, e.getCode(), e.getMessage());
                 }
 
-                final InternalCallContext context = internalCallContextFactory.createInternalCallContext(getAccountId(), callContext);
                 try {
                     checker.checkBlockedChange(getSubscriptionBase(), effectiveChangeDate, context);
                 } catch (final BlockingApiException e) {
                     throw new EntitlementApiException(e, e.getCode(), e.getMessage());
                 }
 
+                try {
+                    getSubscriptionBase().changePlan(productName, billingPeriod, priceList, overrides, callContext);
+                } catch (final SubscriptionBaseApiException e) {
+                    throw new EntitlementApiException(e);
+                }
+
                 final Collection<NotificationEvent> notificationEvents = new ArrayList<NotificationEvent>();
                 final Iterable<BlockingState> addOnsBlockingStates = computeAddOnBlockingStates(effectiveChangeDate, notificationEvents, callContext, context);
 
@@ -520,11 +526,13 @@ public class DefaultEntitlement extends EntityBase implements Entitlement {
                 }
 
                 final InternalCallContext context = internalCallContextFactory.createInternalCallContext(getAccountId(), callContext);
-                final DateTime effectiveChangeDate = dateHelper.fromLocalDateAndReferenceTime(updatedPluginContext.getEffectiveDate(), getSubscriptionBase().getStartDate(), context);
+                final DateTime effectiveChangeDateComputed = dateHelper.fromLocalDateAndReferenceTime(updatedPluginContext.getEffectiveDate(), getSubscriptionBase().getStartDate(), context);
+
+                final DateTime effectiveChangeDate;
                 try {
-                    getSubscriptionBase().changePlanWithDate(productName, billingPeriod, priceList, overrides, effectiveChangeDate, callContext);
+                    effectiveChangeDate = subscriptionInternalApi.getDryRunChangePlanEffectiveDate(getSubscriptionBase(), productName, billingPeriod, priceList, effectiveChangeDateComputed, null, context);
                 } catch (final SubscriptionBaseApiException e) {
-                    throw new EntitlementApiException(e);
+                    throw new EntitlementApiException(e, e.getCode(), e.getMessage());
                 }
 
                 try {
@@ -533,6 +541,12 @@ public class DefaultEntitlement extends EntityBase implements Entitlement {
                     throw new EntitlementApiException(e, e.getCode(), e.getMessage());
                 }
 
+                try {
+                    getSubscriptionBase().changePlanWithDate(productName, billingPeriod, priceList, overrides, effectiveChangeDate, callContext);
+                } catch (final SubscriptionBaseApiException e) {
+                    throw new EntitlementApiException(e);
+                }
+
                 final Collection<NotificationEvent> notificationEvents = new ArrayList<NotificationEvent>();
                 final Iterable<BlockingState> addOnsBlockingStates = computeAddOnBlockingStates(effectiveChangeDate, notificationEvents, callContext, context);
 
@@ -577,9 +591,9 @@ public class DefaultEntitlement extends EntityBase implements Entitlement {
 
                 final DateTime effectiveChangeDate;
                 try {
-                    effectiveChangeDate = getSubscriptionBase().changePlanWithPolicy(productName, billingPeriod, priceList, overrides, actionPolicy, callContext);
+                    effectiveChangeDate = subscriptionInternalApi.getDryRunChangePlanEffectiveDate(getSubscriptionBase(), productName, billingPeriod, priceList, null, actionPolicy, context);
                 } catch (final SubscriptionBaseApiException e) {
-                    throw new EntitlementApiException(e);
+                    throw new EntitlementApiException(e, e.getCode(), e.getMessage());
                 }
 
                 try {
@@ -588,6 +602,12 @@ public class DefaultEntitlement extends EntityBase implements Entitlement {
                     throw new EntitlementApiException(e, e.getCode(), e.getMessage());
                 }
 
+                try {
+                    getSubscriptionBase().changePlanWithPolicy(productName, billingPeriod, priceList, overrides, actionPolicy, callContext);
+                } catch (final SubscriptionBaseApiException e) {
+                    throw new EntitlementApiException(e);
+                }
+
                 final Collection<NotificationEvent> notificationEvents = new ArrayList<NotificationEvent>();
                 final Iterable<BlockingState> addOnsBlockingStates = computeAddOnBlockingStates(effectiveChangeDate, notificationEvents, callContext, context);
 
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 f7e4372..e2a2bf1 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
@@ -1,7 +1,9 @@
 /*
  * Copyright 2010-2013 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
  * License.  You may obtain a copy of the License at:
  *
@@ -18,13 +20,11 @@ package org.killbill.billing.entitlement.api;
 
 import org.joda.time.DateTime;
 import org.joda.time.LocalDate;
-import org.killbill.billing.payment.api.PluginProperty;
-import org.killbill.billing.subscription.api.user.SubscriptionBaseApiException;
-import org.testng.annotations.Test;
-
+import org.killbill.billing.ErrorCode;
 import org.killbill.billing.account.api.Account;
 import org.killbill.billing.account.api.AccountApiException;
 import org.killbill.billing.api.TestApiListener.NextEvent;
+import org.killbill.billing.catalog.api.BillingActionPolicy;
 import org.killbill.billing.catalog.api.BillingPeriod;
 import org.killbill.billing.catalog.api.PlanPhaseSpecifier;
 import org.killbill.billing.catalog.api.PriceListSet;
@@ -32,10 +32,14 @@ import org.killbill.billing.catalog.api.ProductCategory;
 import org.killbill.billing.entitlement.EntitlementTestSuiteWithEmbeddedDB;
 import org.killbill.billing.entitlement.api.Entitlement.EntitlementActionPolicy;
 import org.killbill.billing.entitlement.api.Entitlement.EntitlementState;
+import org.killbill.billing.payment.api.PluginProperty;
+import org.killbill.billing.subscription.api.user.SubscriptionBaseApiException;
+import org.testng.annotations.Test;
 
 import com.google.common.collect.ImmutableList;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.fail;
 
 public class TestDefaultEntitlement extends EntitlementTestSuiteWithEmbeddedDB {
 
@@ -274,4 +278,53 @@ public class TestDefaultEntitlement extends EntitlementTestSuiteWithEmbeddedDB {
         final Subscription subscription = subscriptionApi.getSubscriptionForEntitlementId(entitlement.getBaseEntitlementId(), callContext);
         assertEquals(subscription.getBillingEndDate(), new LocalDate(2013, 8, 7));
     }
+
+    @Test(groups = "slow", description = "https://github.com/killbill/killbill/issues/452")
+    public void testBlockedEntitlementChange() throws AccountApiException, EntitlementApiException {
+        final LocalDate initialDate = new LocalDate(2013, 8, 7);
+        clock.setDay(initialDate);
+
+        final Account account = accountApi.createAccount(getAccountData(7), callContext);
+
+        final PlanPhaseSpecifier spec = new PlanPhaseSpecifier("Shotgun", ProductCategory.BASE, BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME, null);
+
+        // Create entitlement and check each field
+        testListener.pushExpectedEvent(NextEvent.CREATE);
+        final Entitlement entitlement = entitlementApi.createBaseEntitlement(account.getId(), spec, account.getExternalKey(), null, initialDate, ImmutableList.<PluginProperty>of(), callContext);
+        assertListenerStatus();
+
+        clock.addDays(1);
+        assertListenerStatus();
+
+        testListener.pushExpectedEvent(NextEvent.BLOCK);
+        entitlementApi.setBlockingState(entitlement.getBundleId(), "MY_BLOCK", "test", clock.getUTCToday(), false, false, true, ImmutableList.<PluginProperty>of(), callContext);
+        assertListenerStatus();
+
+        try {
+            entitlement.changePlan("Assault-Rifle", BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME, null, ImmutableList.<PluginProperty>of(), callContext);
+            fail();
+        } catch (final EntitlementApiException e) {
+            assertEquals(e.getCode(), ErrorCode.BLOCK_BLOCKED_ACTION.getCode());
+            final Entitlement latestEntitlement = entitlementApi.getEntitlementForId(entitlement.getId(), callContext);
+            assertEquals(latestEntitlement.getLastActivePlan().getProduct().getName(), "Shotgun");
+        }
+
+        try {
+            entitlement.changePlanWithDate("Assault-Rifle", BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME, null, clock.getUTCToday(), ImmutableList.<PluginProperty>of(), callContext);
+            fail();
+        } catch (final EntitlementApiException e) {
+            assertEquals(e.getCode(), ErrorCode.BLOCK_BLOCKED_ACTION.getCode());
+            final Entitlement latestEntitlement = entitlementApi.getEntitlementForId(entitlement.getId(), callContext);
+            assertEquals(latestEntitlement.getLastActivePlan().getProduct().getName(), "Shotgun");
+        }
+
+        try {
+            entitlement.changePlanOverrideBillingPolicy("Assault-Rifle", BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME, null, clock.getUTCToday(), BillingActionPolicy.IMMEDIATE, ImmutableList.<PluginProperty>of(), callContext);
+            fail();
+        } catch (final EntitlementApiException e) {
+            assertEquals(e.getCode(), ErrorCode.BLOCK_BLOCKED_ACTION.getCode());
+            final Entitlement latestEntitlement = entitlementApi.getEntitlementForId(entitlement.getId(), callContext);
+            assertEquals(latestEntitlement.getLastActivePlan().getProduct().getName(), "Shotgun");
+        }
+    }
 }
diff --git a/subscription/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseApiService.java b/subscription/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseApiService.java
index 9b3d03d..b803399 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseApiService.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseApiService.java
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2015 Groupon, Inc
- * Copyright 2014-2015 The Billing Project, LLC
+ * Copyright 2014-2016 Groupon, Inc
+ * Copyright 2014-2016 The Billing Project, LLC
  *
  * 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
@@ -70,6 +70,10 @@ public interface SubscriptionBaseApiService {
             throws SubscriptionBaseApiException;
 
     // Return the effective date of the change
+    public DateTime dryRunChangePlan(DefaultSubscriptionBase subscription, String productName, BillingPeriod term,
+                                     String priceList, DateTime requestedDate, BillingActionPolicy policy, TenantContext context) throws SubscriptionBaseApiException;
+
+    // Return the effective date of the change
     public DateTime changePlan(DefaultSubscriptionBase subscription, String productName, BillingPeriod term,
                                String priceList, List<PlanPhasePriceOverride> overrides, CallContext context)
             throws SubscriptionBaseApiException;
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 5d97842..4b17ee0 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
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2015 Groupon, Inc
- * Copyright 2014-2015 The Billing Project, LLC
+ * Copyright 2014-2016 Groupon, Inc
+ * Copyright 2014-2016 The Billing Project, LLC
  *
  * 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
@@ -32,6 +32,7 @@ import org.killbill.billing.ErrorCode;
 import org.killbill.billing.callcontext.InternalCallContext;
 import org.killbill.billing.callcontext.InternalTenantContext;
 import org.killbill.billing.catalog.api.BillingActionPolicy;
+import org.killbill.billing.catalog.api.BillingPeriod;
 import org.killbill.billing.catalog.api.Catalog;
 import org.killbill.billing.catalog.api.CatalogApiException;
 import org.killbill.billing.catalog.api.CatalogService;
@@ -422,6 +423,18 @@ public class DefaultSubscriptionInternalApi extends SubscriptionApiBase implemen
     }
 
     @Override
+    public DateTime getDryRunChangePlanEffectiveDate(final SubscriptionBase subscription,
+                                                     final String productName,
+                                                     final BillingPeriod term,
+                                                     final String priceList,
+                                                     final DateTime requestedDateWithMs,
+                                                     final BillingActionPolicy requestedPolicy,
+                                                     final InternalTenantContext context) throws SubscriptionBaseApiException {
+        final TenantContext tenantContext = internalCallContextFactory.createTenantContext(context);
+        return apiService.dryRunChangePlan((DefaultSubscriptionBase) subscription, productName, term, priceList, requestedDateWithMs, requestedPolicy, tenantContext);
+    }
+
+    @Override
     public List<EntitlementAOStatusDryRun> getDryRunChangePlanStatus(final UUID subscriptionId, @Nullable final String baseProductName, final DateTime requestedDate, final InternalTenantContext context) throws SubscriptionBaseApiException {
         try {
             final SubscriptionBase subscription = dao.getSubscriptionFromId(subscriptionId, context);
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 3a079aa..b12935f 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
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2015 Groupon, Inc
- * Copyright 2014-2015 The Billing Project, LLC
+ * Copyright 2014-2016 Groupon, Inc
+ * Copyright 2014-2016 The Billing Project, LLC
  *
  * 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
@@ -26,6 +26,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 
+import javax.annotation.Nullable;
+
 import org.joda.time.DateTime;
 import org.killbill.billing.ErrorCode;
 import org.killbill.billing.ObjectType;
@@ -337,6 +339,31 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
     }
 
     @Override
+    public DateTime dryRunChangePlan(final DefaultSubscriptionBase subscription,
+                                     final String productName,
+                                     final BillingPeriod term,
+                                     final String priceList,
+                                     @Nullable final DateTime requestedDateWithMs,
+                                     @Nullable final BillingActionPolicy requestedPolicy,
+                                     final TenantContext context) throws SubscriptionBaseApiException {
+        final DateTime now = clock.getUTCNow();
+
+        BillingActionPolicy policyMaybeNull = requestedPolicy;
+        if (requestedDateWithMs == null && requestedPolicy == null) {
+            final PlanChangeResult planChangeResult = getPlanChangeResult(subscription, productName, term, priceList, now, context);
+            policyMaybeNull = planChangeResult.getPolicy();
+        }
+
+        if (policyMaybeNull != null) {
+            return subscription.getPlanChangeEffectiveDate(policyMaybeNull);
+        } else if (requestedDateWithMs != null) {
+            return DefaultClock.truncateMs(requestedDateWithMs);
+        } else {
+            return now;
+        }
+    }
+
+    @Override
     public DateTime changePlan(final DefaultSubscriptionBase subscription, final String productName, final BillingPeriod term,
                                final String priceList, final List<PlanPhasePriceOverride> overrides, final CallContext context) throws SubscriptionBaseApiException {
         final DateTime now = clock.getUTCNow();
@@ -344,47 +371,48 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
         validateEntitlementState(subscription);
 
         final PlanChangeResult planChangeResult = getPlanChangeResult(subscription, productName, term, priceList, now, context);
-        final DateTime effectiveDate = subscription.getPlanChangeEffectiveDate(planChangeResult.getPolicy());
+        final DateTime effectiveDate = dryRunChangePlan(subscription, productName, term, priceList, null, planChangeResult.getPolicy(), context);
         validateEffectiveDate(subscription, effectiveDate);
 
         try {
-            return doChangePlan(subscription, productName, term, planChangeResult.getNewPriceList().getName(), overrides, now, effectiveDate, context);
+            doChangePlan(subscription, productName, term, planChangeResult.getNewPriceList().getName(), overrides, effectiveDate, context);
         } catch (final CatalogApiException e) {
             throw new SubscriptionBaseApiException(e);
         }
+
+        return effectiveDate;
     }
 
     @Override
     public DateTime changePlanWithRequestedDate(final DefaultSubscriptionBase subscription, final String productName, final BillingPeriod term,
                                                 final String priceList, final List<PlanPhasePriceOverride> overrides,
                                                 final DateTime requestedDateWithMs, final CallContext context) throws SubscriptionBaseApiException {
-        final DateTime now = clock.getUTCNow();
-        final DateTime effectiveDate = (requestedDateWithMs != null) ? DefaultClock.truncateMs(requestedDateWithMs) : now;
-
+        final DateTime effectiveDate = dryRunChangePlan(subscription, productName, term, priceList, requestedDateWithMs, null, context);
         validateEffectiveDate(subscription, effectiveDate);
         validateEntitlementState(subscription);
 
         try {
-            return doChangePlan(subscription, productName, term, priceList, overrides, now, effectiveDate, context);
+            doChangePlan(subscription, productName, term, priceList, overrides, effectiveDate, context);
         } catch (final CatalogApiException e) {
             throw new SubscriptionBaseApiException(e);
         }
+
+        return effectiveDate;
     }
 
     @Override
     public DateTime changePlanWithPolicy(final DefaultSubscriptionBase subscription, final String productName, final BillingPeriod term,
-                                         final String priceList, final List<PlanPhasePriceOverride> overrides, final BillingActionPolicy policy, final CallContext context)
-            throws SubscriptionBaseApiException {
-        final DateTime now = clock.getUTCNow();
-
+                                         final String priceList, final List<PlanPhasePriceOverride> overrides, final BillingActionPolicy policy, final CallContext context) throws SubscriptionBaseApiException {
         validateEntitlementState(subscription);
 
-        final DateTime effectiveDate = subscription.getPlanChangeEffectiveDate(policy);
+        final DateTime effectiveDate = dryRunChangePlan(subscription, productName, term, priceList, null, policy, context);
         try {
-            return doChangePlan(subscription, productName, term, priceList, overrides, now, effectiveDate, context);
+            doChangePlan(subscription, productName, term, priceList, overrides, effectiveDate, context);
         } catch (final CatalogApiException e) {
             throw new SubscriptionBaseApiException(e);
         }
+
+        return effectiveDate;
     }
 
     @Override
@@ -414,14 +442,13 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
         return planChangeResult;
     }
 
-    private DateTime doChangePlan(final DefaultSubscriptionBase subscription,
-                                  final String newProductName,
-                                  final BillingPeriod newBillingPeriod,
-                                  final String newPriceList,
-                                  final List<PlanPhasePriceOverride> overrides,
-                                  final DateTime now,
-                                  final DateTime effectiveDate,
-                                  final CallContext context) throws SubscriptionBaseApiException, CatalogApiException {
+    private void doChangePlan(final DefaultSubscriptionBase subscription,
+                              final String newProductName,
+                              final BillingPeriod newBillingPeriod,
+                              final String newPriceList,
+                              final List<PlanPhasePriceOverride> overrides,
+                              final DateTime effectiveDate,
+                              final CallContext context) throws SubscriptionBaseApiException, CatalogApiException {
         final InternalCallContext internalCallContext = createCallContextFromBundleId(subscription.getBundleId(), context);
         final PlanPhasePriceOverridesWithCallContext overridesWithContext = new DefaultPlanPhasePriceOverridesWithCallContext(overrides, context);
         final Plan newPlan = catalogService.getFullCatalog(internalCallContext).createOrFindPlan(newProductName, newBillingPeriod, newPriceList, overridesWithContext, effectiveDate, subscription.getStartDate());
@@ -438,8 +465,6 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
 
         final Catalog fullCatalog = catalogService.getFullCatalog(internalCallContext);
         subscription.rebuildTransitions(dao.getEventsForSubscription(subscription.getId(), internalCallContext), fullCatalog);
-
-        return effectiveDate;
     }
 
     @Override