-
Notifications
You must be signed in to change notification settings - Fork 2
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shtukas
changed the title
Introduce ZuoraSubscriptionAmendment and fetchLastSubscriptionAmendment
fetchLastSubscriptionAmendment
Sep 12, 2023
shtukas
changed the title
fetchLastSubscriptionAmendment
@shtukas Introduce last amendment check
Sep 17, 2023
shtukas
force-pushed
the
ph-20230911-2344-ZuoraSubscriptionAmendment
branch
from
September 17, 2023 12:53
19f2346
to
a570c4c
Compare
shtukas
changed the title
@shtukas Introduce last amendment check
Introduce last amendment check
Sep 17, 2023
kelvin-chappell
approved these changes
Sep 18, 2023
There was a problem hiding this 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). |
There was a problem hiding this comment.
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
force-pushed
the
ph-20230911-2344-ZuoraSubscriptionAmendment
branch
from
September 25, 2023 15:55
fc6f653
to
81651a1
Compare
shtukas
force-pushed
the
ph-20230911-2344-ZuoraSubscriptionAmendment
branch
from
September 25, 2023 16:47
81651a1
to
144cf7a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(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).
At the moment we only need the effectiveDate. This is the reason why this first version of the class only carries that one attribute.
It will be great if in the future the following is done
Together with ZuoraSubscriptionAmendment we introduce
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.