jwplayer / ott-web-app

Reference implementation for JWP-powered apps
Apache License 2.0
70 stars 52 forks source link

Integration offerId inconsistency #518

Open ChristiaanScheermeijer opened 5 months ago

ChristiaanScheermeijer commented 5 months ago

We found some issues with the offerIds being used differently between the JWP and Cleeng integrations.

The difference between JWP assets and Cleeng offers is that JWP uses a single asset with two pricing options (monthly & annual). But for Cleeng, two offers need to be created. In the codebase, JWP imitates having two offers and this has some problems with backtracking the correct assetId for some places.

We should consider making this more generic and not integration-specific in the business logic.

See #492 for more information about this.

ChristiaanScheermeijer commented 4 months ago

@dbudzins @AntonLantukh

Unfortunately, we also ran into this problem while integrating IAP and need to solve this for a good flow.

Problem

A bit more context to this problem.

The JWP integration is configured using one asset. Based on this asset, the access fees configured in the dashboard will be transformed into two Offer objects.

The offerId and id are generated using the AccessFee#id, and the asset ID is not added.

https://github.com/jwplayer/ott-web-app/blob/71469721cacb6224e080910e5491da7935f5eea9/packages/common/src/services/integrations/jwp/JWPCheckoutService.ts#L49-L55

When the PayPal payment succeeds, the user is sent to the following URL, which adds the offerId of the selected offer to the query params. For a JWP offer, this will be S123456 (S + access fee)

https://github.com/jwplayer/ott-web-app/blob/71469721cacb6224e080910e5491da7935f5eea9/packages/ui-react/src/containers/AccountModal/forms/Checkout.tsx#L105

The WaitingForPayment component will poll until it has access to the given offerId:

https://github.com/jwplayer/ott-web-app/blob/71469721cacb6224e080910e5491da7935f5eea9/packages/ui-react/src/components/WaitingForPayment/WaitingForPayment.tsx#L15

In the useCheckAccess hook, which does the polling, we use the given offerId and call the checkEntitlements function.

Also note the fallback here: when no offerId is given, the offerId is obtained from the checkoutController.getSubscriptionOfferIds function. This uses the offerId from the app config, which is the asset ID and not the access fee ID.

https://github.com/jwplayer/ott-web-app/blob/71469721cacb6224e080910e5491da7935f5eea9/packages/hooks-react/src/useCheckAccess.ts#L22-L31

Finally, we call it checkAccessForAsset using the offerId.

https://github.com/jwplayer/ott-web-app/blob/71469721cacb6224e080910e5491da7935f5eea9/packages/common/src/services/integrations/jwp/JWPCheckoutService.ts#L218-L224


Possible Solution

Instead of refactoring everything to generic types (which we eventually will do), I realised while writing this we could fix this in the JWPCheckoutService. The offerId inconsistency still remains, but that will be fixed in the future.

When retrieving the offers, we could hold a small dictionary with an offerId -> assetId mapping in the JWPCheckoutService. When the getEntitlements is called, is uses the mapped assetId if it exists and otherwise uses the given offerId

To make the asset ID more stable, we can also test the offerId for a C or S prefix before resolving it.

  offersMap = new Map<string, number>()

  getOffers: GetOffers = async (payload) => {
    const offers = await Promise.all(
      payload.offerIds.map(async (assetId) => {
        try {
          const { data } = await InPlayer.Asset.getAssetAccessFees(parseInt(`${assetId}`));

          return data?.map((accessFee) => {
            const offer = this.formatOffer(accessFee);
            this.offersMap.set(offer.offerId, assetId);
            return offer;
          });
        } catch {
          throw new Error('Failed to get offers');
        }
      }),
    );

    return offers.flat();
  };

We can use it as such:

  resolveAssetId(offerId: string) {
    // when the offer id starts with a C or S, it's an access fee id
    if (offerId.startsWith('C') || offerId.startsWith('S')) {
      const assetId = this.offersMap.get(offerId);
      if (typeof assetId !== 'undefined') return assetId;
      throw new Error(`Couldn't resolve assetId from the given offerId: ${offerId}`);
    }

    // the offerId is an asset id
    return parseInt(offerId);
  }

  getEntitlements: GetEntitlements = async ({ offerId }) => {
    try {
      const response = await InPlayer.Asset.checkAccessForAsset(this.resolveAssetId(offerId));
      return this.formatEntitlements(response.data.expires_at, true);
    } catch {
      return this.formatEntitlements();
    }
  };

I can only think of 1 scenario where this doesn't work. When buying access to a PPV item, the asset and offers are fetched. However, when the user is redirected back to the site from PayPal or 3D Secure, this asset might not be fetched.

royschut commented 4 months ago

However, when the user is redirected back to the site from PayPal or 3D Secure, this asset might not be fetched.

Maybe we can append the resolved assetId to the paypal return url?

ChristiaanScheermeijer commented 4 months ago

Maybe we can append the resolved assetId to the paypal return URL?

Yes, but it's preferred that only the JWP integration is aware of the assetId. A possible solution would be to let the integration build the return URLs...