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

Allow for switching between Orders and Subscriptions #44

Closed
JasonTheAdams opened this issue Aug 25, 2020 · 19 comments
Closed

Allow for switching between Orders and Subscriptions #44

JasonTheAdams opened this issue Aug 25, 2020 · 19 comments
Assignees

Comments

@JasonTheAdams
Copy link

Greetings!

I work with givewp.com and we've been working with an integration manager at PayPal since June to provide integration for PayPal Commerce.

We've recently run into a problem with this SDK wherein the vault=true parameter must be set at the time that the SDK script is loaded. This means that the SDK is locked into either creating Orders or Subscriptions once it's loaded.

This is a problem for us as our product provides organizations with a donation form to put on their website. During realtime, the donor may decide whether they're making a one-time or recurring donation. In fact, the donor could even start willing out CC details, then decide they want to make it recurring, tick a box, and continue. You can see a couple demos:

Is there currently a way of switching between Orders and Subscriptions at runtime? If not, we need that for compatibility of integration.

Thank you!

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label feature_request to this issue, with a confidence of 0.77. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@gregjopa
Copy link
Contributor

Is there currently a way of switching between Orders and Subscriptions at runtime?

Hi @JasonTheAdams, the JS SDK can be reloaded on the page for handling query parameter changes. Here's a codepen to demonstrate: https://codepen.io/gregjopa/pen/jOqBGJy.

Screen Shot 2020-08-25 at 2 06 38 PM

This demo is using paypal-js to simplify the loading of the sdk script. At the end of the day this is just inserting the new sdk script like so document.head.insertBefore(newScript, document.head.firstElementChild) which will load and take priority and cleanup the old sdk script on the page.

Would this proposed solution work for your use case?

@JasonTheAdams
Copy link
Author

Thanks, @gregjopa! I appreciate you taking the time to put that together! Obviously, not having to reload the script and re-render — causing the flash — would be ideal for a future feature. But this seems like it could be a solid workaround.

We'll give this a shot this week and see if it works within our system and I'll report back to this issue with our success.

Thank you!

@gregjopa
Copy link
Contributor

@JasonTheAdams happy to help! I'll leave this issue open. Please let us know how the integration goes.

@ravinderk
Copy link

@gregjopa When I define a components property of PayPal SDK then click on the radio button does not overwrite old button and the number of button count keeps increase with a javascript error. Can you review this pen: https://codepen.io/ravinderk/pen/wvGevRg

image

paypa-sdk

image

@gregjopa
Copy link
Contributor

@ravinderk that is by design. Our motto is you can never have too many paypal buttons on a web page.

Just kidding 😆. To avoid showing multiple paypal buttons, you need to clean up the old before rendering the new. There's a helpful close() function to doing this cleanup work. Ex:

  let buttons;

  function render(options) {
    if (buttons) {
      buttons.close();
    }
    buttons = paypal.Buttons(options);
    buttons.render(document.querySelector('#paypal-button-container'));
  }

Here's a pen demoing this cleanup behavior: https://codepen.io/gregjopa/pen/RwagNWR?editors=1000

@ravinderk
Copy link

ravinderk commented Aug 28, 2020

@gregjopa I am still seeing this error. Can you check it?

First:
image

Second:
image

@gregjopa
Copy link
Contributor

gregjopa commented Aug 28, 2020

@ravinderk That first error seems to related to hosted fields. The error does not happen when using components=buttons for both cases. I updated the codepen to reflect this: https://codepen.io/gregjopa/pen/RwagNWR?editors=1000.

My guess is you also need to cleanup hosted fields in the same manner as the buttons:

if (fields) {
  fields.close();
}

fields = window.paypal.HostedFields({ 
  // your config here 
});

And the second error seems to be a side effect of the first error.

@ravinderk
Copy link

ravinderk commented Aug 28, 2020

@gregjopa Is window.paypal.HostedFields is a function? How will I get access to it?

image

@gregjopa
Copy link
Contributor

@ravinderk whoops I was wrong. HostedFields is not a function. It's an object and your screenshot is correct. You will need to use it's render() function like so:

paypal.HostedFields.render({ /* ... */ })
    .then(function (hostedFieldsInstance) { /* ... */ })

https://developer.paypal.com/docs/business/checkout/reference/style-guide/#style-reference-for-advanced-credit-and-debit-card-payments.

Note that hostedFields is kind of off topic with this issue. Please open up a new issue if you have additional questions about implementing hostedFields.

@JasonTheAdams
Copy link
Author

@gregjopa From our perspective this is all the same thing. We're implementing both the Smart Buttons and Advanced Fields within our forms. We're not having a problem with this combination except in the context of switching between one-time and recurring donations.

We could rename this Issue to "Allow for switching Smart Buttons between Orders and Subscriptions" and then create another "Allow for switching Hosted Fields between Orders and Subscriptions", if you would like. For us, though, we can't proceed until both are working.

@gregjopa
Copy link
Contributor

@JasonTheAdams thanks for clarifying. I didn't realize you and @ravinderk are working together 😄 Let's keep this one issue.
I'm checking with the team about how to clean up hosted-fields. I'll follow up soon.

@JasonTheAdams
Copy link
Author

Oh yeah, I guess there's no reason you'd have known that. Hahah! Thank you! 😄

@gregjopa gregjopa self-assigned this Sep 2, 2020
@JasonTheAdams
Copy link
Author

Hey @gregjopa! Any word on this? Just checking in. Thanks!

@gregjopa
Copy link
Contributor

gregjopa commented Sep 3, 2020

Hey @JasonTheAdams, thanks for your patience. We've reviewed this issue internally and there is a bug with hosted-fields "destroy" hook for the reload use case. A fix has been submitted and should be released by the end of next week.

I've updated this codepen example for verifying this fix. Currently it throws the following error when switching between Orders and Subscriptions:

Uncaught Error: Request listener already exists for zoid_allow_delegate_payments_sdk_contingency_handler on domain * for wildcard window

This error should go away after the fix is released next week. I'll follow up once the fix is in prod.

@JasonTheAdams
Copy link
Author

Roger that! Thanks for the update and pending fix!

@gregjopa
Copy link
Contributor

Uncaught Error: Request listener already exists for zoid_allow_delegate_payments_sdk_contingency_handler on domain * for wildcard window

Hi @JasonTheAdams and @ravinderk, I'm happy to share that the above error has been fixed in yesterday's v5.0.164 release!

Thanks for bringing this to our attention. I'm going to close this one out.

@JasonTheAdams
Copy link
Author

Thank you, @gregjopa! We appreciate you and your team taking the time to look into this and resolve the issue. It will be a few weeks, likely, before we're able to implement this, but from the looks of your codepen it seems to be working great. 👍

@ravinderk
Copy link

ravinderk commented Sep 28, 2020

@gregjopa I find out that subscription with smart button works fine even I set intent to capture. subscription value is not mentioned in allowed values: https://developer.paypal.com/docs/checkout/reference/customize-sdk/#intent

I able to process subscription but I see a warning in Chrome console tab.
image

  • Are you planning to make intent=subscirption required in the future?
  • I want to mention at the same time webpage can contain two forms that can process subscription and order both., so I am trying to find a way to skip reloading the script just for the intent param.
  • vault param is required for a subscription but I can also process the one-time payment with it. Am I missing something here?

As per testing, things are working fine but I am trying to make sure that the solution is future proof.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants