WPS migration: Transact onboarding #60238
Conversation
232d338 to
22c0739
Compare
| } | ||
|
|
||
| // If the merchant is already onboarded, nothing to do. | ||
| if ( $this->get_option( 'is_transact_onboarded', 'no' ) === 'yes' ) { |
There was a problem hiding this comment.
A simple bool option might fail. I think the best way would be to have a cache of the account data.
So we could have a method checks:
- Is Jetpack connected?
- If not, connect
- If yes, proceed
- Is there a merchant account in the cache and is it not expired?
- If not, fetch from the server, store it using
update_option(not transients) and some expiry time. Similar to what we do for Stripe (I hope) and WooPayments in the DatabaseCache class. - If fetch returns empty data, create a merchant account and then cache it
- If the account is there, proceed to the next step
- If not, fetch from the server, store it using
- Is PayPal Standard provider account onboarded?
- Same as above: fetch, cache, if it's there, we're onboarded
Normally it will be a method checking the local DB, but also with a fallback to the server in case any data gets outdated or goes missing.
| $jetpack_connection_manager = new Jetpack_Connection_Manager( 'woocommerce' ); | ||
| $is_connected = $jetpack_connection_manager->is_connected(); | ||
| return $is_connected; | ||
|
|
||
| if ( ! $is_connected ) { |
There was a problem hiding this comment.
Parts of the method I described above, especially around caching/fetching, could be DRY-ed and reused here.
Here's an example method like that from WooPayments, and the underlying cache/fetch method (we probably don't need anything as complicated here, though)
119fd7b to
38f3db0
Compare
38f3db0 to
e7b12e5
Compare
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
| // TODO: Do we need to pass this? If yes, do we need to handle scenario where the store changes domain? | ||
| 'store_url' => get_site_url(), |
There was a problem hiding this comment.
This is for the KYC reasons, which we don't really need here. I'd say let's make this field nullable in the server.
There was a problem hiding this comment.
Removed, thanks. It's already nullable in the server, too.
| 'settings' => array( | ||
| // TODO: Merchant account creation requires a statement descriptor, but we are not using it anywhere. | ||
| 'statement_descriptor' => get_bloginfo( 'name' ), |
There was a problem hiding this comment.
Same as above, let's make the field nullable and regenerate the server routes to make it not required.
There was a problem hiding this comment.
Removed. It was nullable after all. (I was getting some errors before, and misread this part that checks for statement descriptors).
| // TODO: We expect this flag to be true if the merchant can be migrated, | ||
| // i.e. does not need PayPal API keys, and they have accepted the ToS. | ||
|
|
||
| public function should_use_orders_v2() { |
There was a problem hiding this comment.
@Mayisha FYI that I'm moving this check from the helper to the gateway class, as it needs the gateway instance to perform the Transact checks.
I added a filter to handle the legacy settings logic.
Submission Review Guidelines:
Changes proposed in this Pull Request:
Towards PAYPAL-62
Merges into
feature/paypal-wps-migration.Client-side changes for Transact merchant and provider onboarding.
How to test the changes in this Pull Request:
wp option patch insert woocommerce_paypal_settings _should_load 'yes'(orpatch update) to enable PayPal Standard.wp option patch insert woocommerce_paypal_settings is_tos_accepted 'yes'(orpatch update) to simulate ToS accepted flag.Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment
This will be merged into a feature branch.