-
Notifications
You must be signed in to change notification settings - Fork 4
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
Partner Aware Onboarding - Customizations for Square #1426
Conversation
Size Change: +197 B (0%) Total Size: 197 kB
ℹ️ View Unchanged
|
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.
Tested well! I have some comments:
Inspect a request to /wp-json/wc-admin/payment-gateway-suggestions
Confirm the response doesn't have woocommerce_payment and woocommerce_payments:with-in-person-payments
I get an empty array with this request. Is this expected? For context, the request URL was /wp-json/wc-admin/payment-gateway-suggestions?_locale=user
, missing force_default_suggestion=true
when comparing to the query in payments task.
It seems the task does not have a header.
cc @verofasulo would you be able to suggest a task header design to use for Get paid with Square
? Perhaps reusing the payments header? I think we'll also need the copy specific for Square.
![image](https://private-user-images.githubusercontent.com/3747241/301054242-45e8476e-fe46-45b2-b859-31f91d9d6c70.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjYxNTgyNTksIm5iZiI6MTcyNjE1Nzk1OSwicGF0aCI6Ii8zNzQ3MjQxLzMwMTA1NDI0Mi00NWU4NDc2ZS1mZTQ2LTQ1YjItYjg1OS0zMWY5MWQ5ZDZjNzAucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDkxMiUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA5MTJUMTYxOTE5WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NzkwOTM5NzIyNDc3NWFjYzA2M2UwOTk0MTAxMzgwOGMxNDFjY2NiN2M5ODJlNTM0NDA5NGM3MTI1ODQ2N2E4NiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.-Lnqoz_aXgYSX0L4nxNThbz3-SQnrQPMVqohnRugF2M)
includes/tasks/class-wc-calypso-task-setup-woocommerce-square.php
Outdated
Show resolved
Hide resolved
includes/free-trial/partners/class-wc-calypso-bridge-partner-square.php
Outdated
Show resolved
Hide resolved
* | ||
* @return void | ||
*/ | ||
private function remove_woo_payments_from_core_profiler_plugin_suggestions() { |
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.
I wonder if this is really needed since Core profiler is technically not in the flow for hosted Woo in WPCOM, but doesn't hurt to have it.
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.
Good point 👍 Let's remove it.
src/index.js
Outdated
// Setup Square task fill (Partner Aware Onboarding). | ||
registerPlugin( 'wc-calypso-bridge-task-setup-woocommerce-square', { | ||
scope: 'woocommerce-tasks', | ||
render: SetupWooCommerceSquareFill, |
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.
Since Square is optional (and probably is a very small subset of hosted Woo), I think it'll be beneficial to make this a lazy loaded fill. What if we check for square_connect_url
to render it?
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.
Good idea! I'll make the change.
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.
Updated in 5f86d28
includes/free-trial/partners/class-wc-calypso-bridge-partner-square.php
Outdated
Show resolved
Hide resolved
includes/free-trial/partners/class-wc-calypso-bridge-partner-square.php
Outdated
Show resolved
Hide resolved
includes/tasks/class-wc-calypso-task-setup-woocommerce-square.php
Outdated
Show resolved
Hide resolved
Hey @ilyasfoo, thanks for tagging me!
Please reuse the header we use for generic payment methods (the one in place when we don't have WooPayments by default):
|
…quare.php Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com>
…quare.php Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com>
Thank you @verofasulo @ilyasfoo The task is now placed at the 3rd position with cf2a894 commit. I've also added the task header with 9f95778 |
@ilyasfoo I've tested it with a fresh site and wasn't able to reproduce it. Could you please provide a screenshot of the request and response? |
@moon0326 Here's the request and response ![]() ![]() |
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.
Tested well, but I have a few nitpicky comments:
includes/free-trial/partners/class-wc-calypso-bridge-partner-square.php
Outdated
Show resolved
Hide resolved
includes/free-trial/partners/class-wc-calypso-bridge-partner-square.php
Outdated
Show resolved
Hide resolved
includes/free-trial/partners/class-wc-calypso-bridge-partner-square.php
Outdated
Show resolved
Hide resolved
…quare.php Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com>
…quare.php Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com>
…quare.php Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com>
Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com>
Thank you for the commits 🙇 |
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.
Thanks, @moon0326 ! Would you like to see if the response (#1426 (comment)) is expected? Otherwise, I think this is good to merge. Pre-approving
Also, just noting that we have a follow-up before launching this:
Regarding the header description, we can wait for Square to provide a copy.
Thank you! I'll try to reproduce it again.
I think we can request it and update it in a follow-up PR. I want everyone to start testing it easily on production soon. |
Thank you! Could you check the following options? The endpoint returns an empty array when |
@moon0326 That's it, I had this set to no. Now it's testing well, thanks! |
Changes proposed in this Pull Request:
Closes #1422
This PR removes Woo Payments from the following REST API endpoints when
woocommerce_onboarding_profile.partner
issquare
It also adds
Get paid with Square
task.How to test the changes in this Pull Request:
Feed Removal
woocommerce_onboarding_profile
option value witha:10:{s:15:"business_choice";s:28:"im_just_starting_my_business";s:21:"selling_online_answer";N;s:17:"selling_platforms";N;s:20:"is_store_country_set";b:1;s:8:"industry";a:1:{i:0;N;}s:18:"is_agree_marketing";b:0;s:11:"store_email";s:17:"test@gmail.com";s:9:"completed";b:1;s:23:"is_plugins_page_skipped";b:1;s:7:"partner";s:6:"square";}
It includespartner=square
Network
tab.Tell us a bit about your store
page, clickContinue
button./wp-json/wc-admin/onboarding/free-extensions
obw/core-profiler
.plugins
does not havewoocommerce-payments
WooCommerce -> Settings -> Payments
/wp-json/wc-admin/payment-gateway-suggestions
woocommerce_payment
andwoocommerce_payments:with-in-person-payments
Get paid with Square task
WooCommerce -> Home
Get paid with Square
task is the first task.Other information:
FOR PR REVIEWER ONLY: