-
Notifications
You must be signed in to change notification settings - Fork 53
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
Fix / Cleeng PayPal payments not working #492
Fix / Cleeng PayPal payments not working #492
Conversation
Visit the preview URL for this PR (updated for commit b871a7c): https://ottwebapp--pr492-fix-cleeng-paypal-in-mz64wqn3.web.app (expires Fri, 10 May 2024 15:21:54 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: c198f8a3a199ba8747819f7f1e45cf602b777529 |
@@ -12,6 +12,6 @@ export const modalURLFromLocation = (location: Location, u: keyof AccountModals | |||
}; | |||
|
|||
// Create a full modal url including hostname, mostly for external use (e.g paypal successUrl) | |||
export const modalURLFromWindowLocation = (u: keyof AccountModals, queryParams?: QueryParamsArg) => { | |||
export const modalURLFromWindowLocation = (u: keyof AccountModals | null, queryParams?: QueryParamsArg) => { |
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 can also remove null
from here (the way we did in another PR).
@@ -217,7 +217,10 @@ export default class JWPCheckoutService extends CheckoutService { | |||
|
|||
getEntitlements: GetEntitlements = async ({ offerId }) => { | |||
try { | |||
const response = await InPlayer.Asset.checkAccessForAsset(parseInt(offerId)); | |||
// the offerId can be `S<assetId>_<pricingOptionId>` | |||
const [assetId] = offerId.replace(/^[A-Z]/, '').split('_'); |
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.
Should we also support it in getOffers
function?
const ppvOffers = ['ppv', 'ppv_custom']; | ||
const offerId = ppvOffers.includes(offer.access_type.name) ? `C${offer.id}` : `S${offer.id}`; | ||
const offerId = ppvOffers.includes(offer.access_type.name) ? `C${assetId}_${offer.id}` : `S${assetId}_${offer.id}`; |
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.
Does it make sense to include assetId in the Offer object and to use it where we need it?
I will close this PR for now. We need to make a plan first on how to deal with the asset ID VS offer ID VS pricing option ID not used consistently. But we don't have the time to do this soon, unfortunately... I will create a GH issue to keep on our radar. |
Description
The Cleeng Paypal flow didn't work anymore because a relative URL was used as
successUrl
. I tried cleaning it up a little because it was a bit difficult to differentiate between absolute and relative URLs (relative URLs are used for routing).I also fixed a bug that when buying access to a TVOD item, the waiting for payment screen will show a timeout error. This is because the logic doesn't check access for the TVOD offer but uses the SVOD offers instead. We should fix this soon to make the experience better and prevent having no access without refreshing first.
Edit
This problem might be bigger than I originally thought. We DO add the
offerId
to the waiting for payment URL, but the offer IDs are not consistent throughout the app. This is because Cleeng works with separate assets, whilst InPlayer uses a single asset with multiple pricing options.We use the offerId to check the access for SVOD and TVOD purchases, but because the
offerId
is filled only with the pricing option id, the check interval fails.In this PR, I attempt to workaround this issue by changing the JWP offerId to
S${assetId}_${optionId}
so it can be parsed later. This might look strange and does make the offerId inconsistent with the SVOD offer ID, which IS the assetId.Long story short, we have to look at this further and decide how to refactor.
Edit2
Since this PR originally tried to fix the Cleeng PayPal error, I will create a different, more simple PR for that without the refactoring.