-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update the /mma response for Supporter Plus + Guardian Weekly (T3) subs #1080
Conversation
"subscription" -> Json.obj( | ||
"subscriptionId" -> subscription.name.get, | ||
"cancellationEffectiveDate" -> subscription.termEndDate, | ||
"start" -> subscription.acceptanceDate, | ||
"end" -> Seq(subscription.termEndDate, subscription.acceptanceDate).max, | ||
"readerType" -> subscription.readerType.value, | ||
"accountId" -> subscription.accountId.get, |
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.
removed unneeded brackets
@@ -99,6 +99,7 @@ object AccountDetails { | |||
def externalisePlanName(plan: RatePlan): Option[String] = plan.product match { | |||
case _: Product.Weekly => if (plan.name.contains("Six for Six")) Some("currently on '6 for 6'") else None | |||
case _: Product.Paper => Some(plan.name.replace("+", " plus Digital Subscription")) | |||
case _: Product.SupporterPlus => if (plan.name.contains("Guardian Weekly")) Some("Supporter Plus with Guardian Weekly") else None |
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 don't think MMA needs to know if it is a Domestic or ROW Guardian Weekly?
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.
looks like we look for "plus Digi"
https://github.com/guardian/manage-frontend/blob/cb8b4a85ddac0697d461d00ea4463ba98b71686b/client/components/mma/cancel/voucher/VoucherCancellationFlowStart.tsx#L28
and "6 for 6"
https://github.com/guardian/manage-frontend/blob/98923b8d2735de2bd2b4343b5b7701852118c1c6/shared/productResponse.ts#L131
as well as returning it as the last part of something like "Newspaper Delivery - Everyday"
https://github.com/guardian/manage-frontend/blob/c164820979e345e8c079086b884d7fc8e5c9dcd5/shared/productTypes.ts#L471
So as far as I can see it is there's no code that looks at GW at all in this particular field, let alone whether it's domestic or otherwise.
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.
it looks like the system relies much more heavily on the "tier" field to work out what funcionality to offer in manage UI .
https://github.com/guardian/manage-frontend/blob/c164820979e345e8c079086b884d7fc8e5c9dcd5/shared/productTypes.ts#L721-L720
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 I would say from an MDAPI point of view it's ok, but I'm not sure how manage uses other fields and where it would make more sense from manage's point of view. I think we might have to rely on Richard to confirm this.
As an aside...
There doesn't seem to be clear distinction between fields used as ids/enums in manage, and fields that are just free text, which is not ideal but it's where we are. It does just make a little harder when we want to change the description of things without breaking existing code.
I.e. I'd prefer to see something like product = "supporter_plus_tier_3" and name = "Supporter Plus including Guardian Weekly" and then when they decide it should be called "Super Pack" we can rename it more safely knowing that the product is what things key off.
Hey 👋 I’m a little concerned about the impact this will have on manage-frontend. MMA uses the data about each of the products from the MDAPI response (which in typescript we type/call a productDetail object) in conjunction with some static config in MMA called productTypes and groupedProductTypes … The productType and groupedProductType objects contain generic information about products that it wouldn’t make any sense for the api to return in a bloated http request. There is a unique productType object in the codebase for each of the products that MMA manages, currently:
These objects contain properties associated with the particular journeys that the user might be able to take with them, for example the ability to self serve cancellation, whether or not a user can book a holiday stop or which soft opt in newsletter they may be signed up for. Throughout the codebase there is a link between the productDetail object (the product specific info that comes from MDAPI) and its corresponding productType object (and also groupedProductType). This connection up until now has been via the productDetail.tier property for the corresponding productType object and the productDetail.mmaCategory property for the corresponding groupedProductType object. These changes break that connection and will require a non insignificant refactor to MMA wherever we were previously doing this lookup where productDetail.tier and productDetail.mmaCategory are now not unique enough to establish which specific product or group of products they are associated with. |
Ok, good to know. We can have a rethink about how to do this then. So basically what you need is a value in mmaCategory and tier which will allow you to map to the correct productType? |
e73ac2c
to
70a7e3c
Compare
I'm going to close this PR and open another one with the new approach of adding a new product in Zuora |
Why do we need this?
When we move the tier 3 Supporter Plus & Guardian Weekly product to being one subscription rather than two, we will need a way to identify these subscriptions in MMA so that we can offer holiday stops and delivery address updates for them.
The changes
This PR uses the
currentPlan.name
field in the response to indicate when a subscription is a T3 sub.This is the same field used by newspaper subs to indicate the type of plan (Weekend, Everyday etc. and it is also used by GW to indicate when a subs is six for six), for all other types of subscription it is blank.
This is the format of the response of a T3 sub:
This is the relevant field: