WIP get billing preview when holiday stop api is called #2243
Conversation
| @@ -171,12 +179,12 @@ object Handler extends Logging { | |||
| } | |||
| case "hsr" :: Nil => | |||
| httpMethod match { | |||
| case "POST" => stepsToCreate(getAccessToken, getSubscription, getAccount) _ | |||
| case "POST" => stepsToCreate(getAccessToken, getSubscription, getAccount, getBillingPreview, today) _ | |||
There was a problem hiding this comment.
coincidentally @tjmw has got a bit of refactoring on this to get rid of the underscores and neaten up the types. So we might end up with a small conflict in this area.
| billingPreview <- getBillingPreview | ||
| .getBillingPreview(accessToken, subscription.accountNumber, today.plusMonths(13)) | ||
| .toApiGatewayOp(s"get account ${subscription.accountNumber}") | ||
| _ = logger.info("billingPreview: " + billingPreview) |
There was a problem hiding this comment.
at this stage it just logs the response rather than passing it in to the subscriptiondata. we could try to paralellise the getAccount and getBillingPreview if we want to improve performance.
| ) | ||
| implicit val decode: Decoder[InvoiceItem] = (c: HCursor) => | ||
| for { | ||
| chargeDate <- c.downField("chargeDate").as[String].map(_.takeWhile(_ != ' ')).map(LocalDate.parse) |
There was a problem hiding this comment.
zuora returns a simplified date-time kind of this format yyyy-mm-dd hh:mm:ss that doesn't parse as a LocalDate, I did this as we're only interested in the date part anyway.
| config: Config, | ||
| backend: SttpBackend[Identity, Any], | ||
| ): GetBillingPreview = | ||
| GetBillingPreviewLive.billingPreviewGetResponse(config.zuoraConfig, backend) |
There was a problem hiding this comment.
we could inline this, I'm not sure why the convention is to have one function that calls another
There was a problem hiding this comment.
Should we remove Mario's invoicing-api experiment (testInProdPreviewPublications) as part of this? This is currently erroring due to a problem with invoicing-api credentials, but if it were working it would also be requesting a billing-preview.
handlers/holiday-stop-api/src/main/scala/com/gu/holiday_stops/Handler.scala
Outdated
Show resolved
Hide resolved
|
|
||
| val preview = op.getBillingPreview(accessToken, ACCOUNT_TO_USE, LocalDate.now.plusMonths(13)) | ||
| info("preview is: " + preview) | ||
| preview.toOption.get.length shouldBe 26 |
There was a problem hiding this comment.
what is this line checking, out of interest?
we might want to be aware that this is a real account in the api sandbox, and its state may change (it currently has two active subscriptions, but one or both could be cancelled any time, or more could be acquired).
There was a problem hiding this comment.
yeah it's good to flag this - this is more like a manual test, as I couldn't think of a quick way to make it reliable. It should be left as "ignore" unless it's being used.
If we can make it reliable then that's even better and we should do that (although even then, it's an Effects test and we don't have them automated yet)
To answer your question it's just checking that 26 items were returned for the 13 months (rather than an error). so not really a completely useful assertion.
handlers/holiday-stop-api/src/main/scala/com/gu/holiday_stops/Handler.scala
Outdated
Show resolved
Hide resolved
# Conflicts: # handlers/holiday-stop-api/src/main/scala/com/gu/holiday_stops/Handler.scala
…g-preview # Conflicts: # handlers/holiday-stop-api/src/main/scala/com/gu/holiday_stops/Handler.scala
johnduffell
changed the title
The holiday stop system creates (small) negative charges every time a subscription is suspended, one for each day that is not to be delivered.
These charges are added just before the fulfilment file is generated, and the dates are set up so that the credit due to the customer will be invoiced on the bill run when their next payment is due.
The date must be accurate - if it's correct then the bill run will collate all the charges and reduce the invoice value appropriately. If it's wrong then a negative invoice will be generated on the incorrect date, and the full invoice will be generated on the correct date. The negative invoice will just sit there for ever and the user will be overcharged, unless they notice and speak with a CSR who can manually refund the amount due.
The issue in this case is that the date calculation is not foolproof - at present the code does not work for older, migrated paper subscriptions which have a pending price rise amendment. At the moment, with several print price rises in progress, this is occurring quite often.
This is because the code assumes that either the charge effective date indicates the billing day, or the charge trigger day will indicate which date should be pulled from the subscription e.g. customer acceptance date as the relevant date. It chooses between the two based on the processedThroughDate.
The issue in this case is that the processedThroughDate is empty when the subscription is in pending price rise (as the latest charge has not been processed at all). The code defaults to the charge trigger day logic, which then incorrectly looks at the contract dates on the subscription, rather than on the amendment. These older subscriptions tend to have irrelevant dates to the current charges and amendments, and it ends up just plain wrong.
Possible fixes:
This PR is the first part of implementing 3 (billing preview)
It fetches the billing preview for that account and reads in the dates.
Next steps: