Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a new term for subscriptions during product switch #2097

Merged
merged 32 commits into from
Nov 14, 2023

Conversation

rupertbates
Copy link
Member

@rupertbates rupertbates commented Oct 31, 2023

What does this change?

We have noticed that the current product switch from recurring contribution to supporter plus has an issue, which is that because the charge is aligned to the subscription end date, unless the switch happens on a term start date the billing amount and date will be unpredictable.

Screenshot 2023-11-01 at 10 25 33

This PR updates the switch mechanism fairly substantially to avoid this, changes are:

  • We check the term start date of the subscription to be switched before we begin - if it is not today we also reduce the length of the current term to today and then renew the sub for a new 12 month term. For subs where the term start date is today we skip this step.
  • I have removed the requirement for the client to pass in a checkChargeAmountBeforeUpdate parameter which then caused us to run an additional preview call to check whether to collect the amount payable or write it off if it was below $0.50. We now just check the balance of the invoice created by the switch for this amount.
  • I have moved the collection of payment to a separate api call so that we can separate out the logic involved with it (whether to collect or write off the amount owing) from the updates to the subscription that are being carried out.

Test Subscriptions

Subscription switched on the day it was taken out - no renewal required:
https://apisandbox.zuora.com/platform/subscriptions/8ad093fb8bc82179018bc86b11d25702

Subscription switched part way through the term - renewal required:
https://apisandbox.zuora.com/platform/subscriptions/8ad097048bc82163018bc86dba57092c

How to test

  • Find a recurring contribution
  • Switch it to a Supporter Plus subscription using
curl --location 'https://zvobga46la.execute-api.eu-west-1.amazonaws.com/CODE/product-move/[MY-SUBSCRIPTION-NUMBER]' \
  --header 'x-api-key: [MY-API-KEY]' \
--header 'Content-Type: application/json' \
--data '{
    "price": "120",
    "preview": false,
    "checkChargeAmountBeforeUpdate": true
}'
  • Check that the subscription has been switched to a Supporter Plus sub, that the term start date is today and the term end date is in 12 months time and that the outstanding balance on the account is zero.

@rupertbates rupertbates changed the title New term on switch Create a new term for subscriptions during product switch Oct 31, 2023
@johnduffell
Copy link
Member

as an aside, we should probably add a proper domain instead of "https://zvobga46la.execute-api.eu-west-1.amazonaws.com" as per the other API gateway lambdas

const membershipApisDomain = "membership.guardianapis.com"

build.sbt Show resolved Hide resolved
@rupertbates
Copy link
Member Author

as an aside, we should probably add a proper domain instead of "https://zvobga46la.execute-api.eu-west-1.amazonaws.com" as per the other API gateway lambdas

const membershipApisDomain = "membership.guardianapis.com"

Agreed, different PR though

Copy link
Member Author

Choose a reason for hiding this comment

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

I reordered a lot of functions in this class which is why the diff is so big

Copy link
Member

Choose a reason for hiding this comment

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

ok thanks for the warning, I'll do my best, I will try to comment on new code rather than reordered stuff!

ZLayer.succeed(new MockSQS(sqsStubs)),
ZLayer.succeed(new MockDynamo(dynamoStubs)),
ZLayer.succeed(new MockGetAccount(getAccountStubs, getPaymentMethodStubs)),
ZLayer.succeed(new MockInvoiceItemAdjustment(invoiceItemAdjustmentStubs)),
ZLayer.succeed(new MockGetInvoiceItems(getInvoiceItemsStubs)),
ZLayer.succeed(new MockGetInvoice(getInvoiceStubs)),
ZLayer.succeed(new MockCreatePayment(CreatePaymentResponse(Some(true)))),
Copy link
Member

Choose a reason for hiding this comment

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

I know these tests are ignored (temporarily?) but do we need to check the mutableStore as part of the test assertions, to make sure it really makes the calls it should?

Copy link
Member Author

@rupertbates rupertbates Nov 13, 2023

Choose a reason for hiding this comment

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

Those tests are not ignored, the mocked ones run, the ones with live classes don't

Copy link
Member

Choose a reason for hiding this comment

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

ok maybe I misread, in which case my question stands even more - do we need to check the mutableStore?

rupertbates and others added 3 commits November 13, 2023 14:57
…itchtype/RecurringContributionToSupporterPlus.scala

Co-authored-by: John Duffell <[email protected]>
…itchtype/RecurringContributionToSupporterPlus.scala

Co-authored-by: John Duffell <[email protected]>
…itchtype/RecurringContributionToSupporterPlus.scala

Co-authored-by: John Duffell <[email protected]>
Copy link
Member

@johnduffell johnduffell left a comment

Choose a reason for hiding this comment

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

nice work persisiting on that one, it's one of those quick jobs that turns into a mountain (of false summits..).

@rupertbates rupertbates merged commit 7f8cf39 into main Nov 14, 2023
36 checks passed
@rupertbates rupertbates deleted the new-term-on-switch branch November 14, 2023 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants