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

Introduce last amendment check #925

Merged
merged 13 commits into from
Sep 25, 2023

Conversation

shtukas
Copy link
Contributor

@shtukas shtukas commented Sep 11, 2023

(This PR description is the comment that I put at the top of the new case class.)

Class ZuoraSubscriptionAmendment is introduced to detect whether or not there has been an amendment on a subscription between the estimation step and the amendment step.

The check is introduced to prevent a problem by which, at least for the digital products we have scheduled for price rise in 2023, (where people would be notified by email, and where the subscriptions can be managed online), there was also possibility for user to perform user-triggered subscription mutations online (for instance product changes), which would not be known to the engine and which would then be lost and/or overridden by the engine at amendment step. We then decided that the engine would not proceed with any price rise if there were amendments on the subscription between the estimation step and the amendment step.

This logic solves the immediate problem, but such a simple check is too crude and too inefficient, as it will prevent some subscriptions from being migrated if they undergo mutations (amendments) that have nothing to do with user journeys. In an ideal world we would retrieve the entire collection of amendments, analyse the recent ones and make a determination on whether or not a price rise can happen.

Unfortunately Zuora, as far as we know, only offers us the retrieval of the last amendment. This is enough for the check since we can then compare the date of estimation with the effective date of the amendment (which seems to coincide with the date the amendment was created). But, this is clearly not enough for the type of thorough evaluation that would lead to a better check (without false positives).

  1. At the moment we only need the effectiveDate. This is the reason why this first version of the class only carries that one attribute.

  2. It will be great if in the future the following is done

  • 2.1 Find a way to retrieve more amendments, ideally the entire sequence of amendments of a subscription.
  • 2.2 Implement an analysis of that sequence to provide a better answer to the question: Should the engine perform the scheduled price rise on the corresponding subscription.

Together with ZuoraSubscriptionAmendment we introduce

  • case class IncompatibleAmendmentHistory, and
  • checkMigrationRelevanceBasedOnLastAmendment in the amendment handler

Note: This version is going to fail noisily when the amendment condition occurs. This is on purpose. Once that happens, we will edit the check to cause the intended IncompatibleAmendmentHistory failure mode.

@shtukas shtukas changed the title Introduce ZuoraSubscriptionAmendment and fetchLastSubscriptionAmendment fetchLastSubscriptionAmendment Sep 12, 2023
@shtukas shtukas changed the title fetchLastSubscriptionAmendment @shtukas Introduce last amendment check Sep 17, 2023
@shtukas shtukas force-pushed the ph-20230911-2344-ZuoraSubscriptionAmendment branch from 19f2346 to a570c4c Compare September 17, 2023 12:53
@shtukas shtukas marked this pull request as ready for review September 17, 2023 12:55
@shtukas shtukas changed the title @shtukas Introduce last amendment check Introduce last amendment check Sep 17, 2023
Copy link
Member

@kelvin-chappell kelvin-chappell left a comment

Choose a reason for hiding this comment

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

👍

// Note: the Zuora documentation
// https://www.zuora.com/developer/api-references/older-api/operation/GET_AmendmentsBySubscriptionID/
// specifies that a subscriptionId is to be provided, but it also works with a subscription number
// (aka subscription name for a cohort item).
Copy link
Member

Choose a reason for hiding this comment

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

I think Zuora also refers to subscription numbers as subscription names in some of their docs as well! (And also possibly the UI as well somewhere)

@shtukas
Copy link
Contributor Author

shtukas commented Sep 20, 2023

Tiny observation in the wild (PROD)

We processed this subscription this morning (from the supporter+ migration)

Screenshot 2023-09-20 at 09 39 31 copy

The log specified that the ZuoraSubscriptionAmendment object was

Screenshot 2023-09-20 at 09 39 31

In order to validate that we just needed to look up the subscription in Zuora

Screenshot 2023-09-20 at 09 39 49

Where we see that indeed the last amendment before the engine's mutation happened on 2023-03-23

We also need to check how that date relates to the estimation date. The record in the Dynamo table is

Screenshot 2023-09-20 at 09 41 00

The estimation happened after the last amendment.

@shtukas shtukas force-pushed the ph-20230911-2344-ZuoraSubscriptionAmendment branch from fc6f653 to 81651a1 Compare September 25, 2023 15:55
@shtukas shtukas force-pushed the ph-20230911-2344-ZuoraSubscriptionAmendment branch from 81651a1 to 144cf7a Compare September 25, 2023 16:47
@shtukas
Copy link
Contributor Author

shtukas commented Sep 25, 2023

Failure case:

Screenshot 2023-09-25 at 19 17 34

@shtukas shtukas merged commit c37434f into main Sep 25, 2023
1 check passed
@shtukas shtukas deleted the ph-20230911-2344-ZuoraSubscriptionAmendment branch September 25, 2023 18:56
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