Skip to content
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

Integration offerId inconsistency #518

Open
ChristiaanScheermeijer opened this issue Apr 24, 2024 · 3 comments
Open

Integration offerId inconsistency #518

ChristiaanScheermeijer opened this issue Apr 24, 2024 · 3 comments
Labels
technical-debt Things that could use a little refactoring...

Comments

@ChristiaanScheermeijer
Copy link
Collaborator

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 ChristiaanScheermeijer added the technical-debt Things that could use a little refactoring... label Apr 24, 2024
@ChristiaanScheermeijer
Copy link
Collaborator Author

@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.

private formatOffer = (offer: AccessFee): Offer => {
const ppvOffers = ['ppv', 'ppv_custom'];
const offerId = ppvOffers.includes(offer.access_type.name) ? `C${offer.id}` : `S${offer.id}`;
return {
id: offer.id,
offerId,

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)

const waitingUrl = modalURLFromWindowLocation('waiting-for-payment', { offerId: selectedOffer?.offerId });

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

const offerId = useQueryParam('offerId') || undefined;

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.

const offers = checkoutController.getSubscriptionOfferIds();
const intervalCheckAccess = useCallback(
({ interval = 3000, iterations = 5, offerId, callback }: IntervalCheckAccessPayload) => {
if (!offerId && offers?.[0]) {
offerId = offers[0];
}
intervalRef.current = window.setInterval(async () => {
const hasAccess = await accountController.checkEntitlements(offerId);

Finally, we call it checkAccessForAsset using the offerId.

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


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
Copy link
Collaborator

royschut commented May 31, 2024

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
Copy link
Collaborator Author

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical-debt Things that could use a little refactoring...
Projects
None yet
Development

No branches or pull requests

2 participants