feat(next): Update success page, add latest invoice to cart data

[david1alvarez]

Because

• The success page was using fake cart data, rather than live data from the processed cart.
• The success page's styling needed an update

This pull request

• Updates the styling for the success page
• Adds in the concept of a cart's latest invoice (which includes details like the payment method used in the invoice, a requirement for the page's design)
• replaces the mock cart data in use on the success page with live cart data

Issue that this pull request solves

Closes: FXA-7575

Checklist

Put an x in the boxes that apply

• My commit is GPG signed.
• If applicable, I have modified or added tests which pass locally.
• I have added necessary documentation (if appropriate).
• I have verified that my changes render correctly in RTL (if appropriate).

Screenshots (Optional)

[xlisachan]

We can re-use the string below (just add the next- prefix).

payment-confirmation-thanks-heading-account-exists = Thanks, now check your email!

[david1alvarez]

I'd be happy to update it! Does re-using the string with an added next- prefix help improve translations?

[xlisachan]

Adding the next- prefix helps us keep track of which strings already have translations. :)

[StaberindeZA]

Just a bit of additional context to what Lisa mentioned. The intention is to reuse translations from SP2.0 in SP3.0, i.e. copy strings from payments.ftl into payments-next.ftl. The way the reusable strings are currently identified is by prefixing the id's with next-.

[xlisachan]

We can re-use this fluent ID with the next- prefix as well:

# $email (string) - The user's email.
payment-confirmation-thanks-subheading-account-exists = You’ll receive an email at { $email } with instructions for setting up your account, as well as your payment details.

Also, just so you're aware, we use smart characters (at least that's what I think they're called?) in ftl strings, eg:

• ’ instead of '
• … instead of ...

[david1alvarez]

Thanks for the note about the smart characters! I am not sure I understand about the fluent id re-use though, the two strings are similar, but not the same.

[xlisachan]

Aah, I didn't catch that the sentence changed at the end! Apologies, this would require a new fluent id.

[StaberindeZA]

"Services" should not call "Clients" directly. Instead add a method to the PaymentMethod manager

[StaberindeZA]

I left some pretty lengthy comments. Might be good to discuss more in Slack or on a call.

Also could we just get some confirmation from Stef that the mobile view is as expected?

[StaberindeZA]

What's the need for this change?

This method returns a preview of the invoice a customer might receive. At this point I wouldn't expect there to be a payment intent or payment method available.

[david1alvarez]

Ah, thank you! You are correct - an upcoming invoice has none of this information, this is leftover from early debugging, but because of the undefined case handling, it didn't impact the functionality of the method. Will remove.

[StaberindeZA]

This should be moved to the InvoiceManager

Also note, that previewLatestInvoice should only be responsible for fetching the latest invoice for a specific invoice Id.

Fetching the subscription, to get the latest invoice, is not the responsibility of the Invoice Manager.

Similarly returning the payment method data, should be part of the PaymentMethodManager.

Put another way, Managers are responsible for interacting with data in their domain, Services are responsible for orchestrating all the data to complete a specific function. I've added some pseudo code below of what I'd expect the CartService.getCart to change to

if (cart.stripeSubscriptionId) {
  const sub = await this.subManager.retrieve(cart.stripeSubscriptionId);
  const latestInvoice = await this.subManager.previewLatest(sub.latest_invoice)
  const paymentMethod = await this.paymentMethodManager.retrieve(paymentMethod.id)
}

[david1alvarez]

That's a nice way of doing this, will update

[david1alvarez]

due to the way everything may-or-may-not-be undefined, while trying to avoid a wildly nested if/else block I've invented something equally bad! I present to you: ternary hell

const subscription = cart.stripeSubscriptionId
  ? await this.subscriptionManager.retrieve(cart.stripeSubscriptionId)
  : undefined;

const latestInvoice = subscription?.latest_invoice
  ? await this.invoiceManager.retrieve(subscription.latest_invoice)
  : undefined;

const paymentIntent = latestInvoice?.payment_intent
  ? await this.stripeClient.paymentIntentRetrieve(
      latestInvoice.payment_intent
    )
  : undefined;

const paymentMethodId =
  typeof paymentIntent?.payment_method === 'string'
    ? paymentIntent.payment_method
    : paymentIntent?.payment_method?.id;

const paymentMethod = paymentMethodId
  ? await this.paymentMethodManager.retrieve(paymentMethodId)
  : undefined;

const latestInvoicePreview = latestInvoice?.id
  ? await this.invoiceManager.previewInvoice(latestInvoice.id)
  : undefined;
const last4 = paymentMethod?.card?.last4;

if (latestInvoicePreview) {
  latestInvoicePreview.last4 = last4;
}

Not sure this is better, lets chat through options tomorrow

[StaberindeZA]

Not sure this should be considered as part of the invoice preview

[david1alvarez]

It should exist somewhere between the invoice layer, and the cart layer. It seemed like invoicePreview was effectively an aggregate of fields related to invoices, rather than a method for surfacing the actual invoice itself.

In my brain, the specifics of payment details exist in relation to the payment event (here being on the completed invoice). Do you have a suggestion for where this should live?