-
Notifications
You must be signed in to change notification settings - Fork 21
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
Change to require Google Ads connection during the onboarding #2180
Change to require Google Ads connection during the onboarding #2180
Conversation
…ake its setup card always visible.
…e Ads account is connected.
…king PR #2174. Ref: - #2153 - #2174 (review)
…tion in step 4 of the onboarding.
…he page to load to proceed with assertions.
@@ -48,7 +48,7 @@ export default function MockupSearch( { product } ) { | |||
</ScaledText> | |||
<Placeholder stroke="thinner" width="79" color="blue" /> | |||
</div> | |||
<Flex justfy="space-between" align="stretch"> | |||
<Flex align="stretch"> |
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.
The "justfy" is a typo. Removed it because the default value of justify
is already the "space-between".
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 @eason9487 for working on this, the code looks good and I ran different tests with different scenarios and everything works well. There was one scenario that was not mentioned in the PR but the code manages the situation nicely.
When you try to link an older Ads account without billing set up, the API shows the account as connected (because the last incomplete step is not set). However, the UI will still prompt the billing setup card, which works fine in this case.
Also the E2E tests are now passing 👍
Changes proposed in this Pull Request:
Project thread: pcTzPl-1YY-p2
This PR changes the step 4 of the onboarding:
checkFAQExpandable
doesn't wait for the page to load to proceed with assertions.It also fixes #2172 as a few related E2E tests failed.
Closes #2172
Screenshots:
📹 Connect and disconnect Google Ads, complete onboarding without creating a campaign
Kapture.2023-12-13.at.16.27.59.mp4
📹 Complete onboarding along with creating a campaign
Kapture.2023-12-13.at.16.33.18.mp4
Detailed test instructions:
📌 Event tracking
💡 Testing this with the UI part together would save some time.
localStorage.setItem( 'debug', 'wc-admin:*' )
in the Console tab of DevTool, and refresh page to make it effective.gla_onboarding_complete_button_click
for A and C with these properties:opened_paid_ads_setup
: yes | nogoogle_ads_account_status
: connected | disconnected | incompletebilling_method_status
: unknown | pending | approved | cancelledcampaign_form_validation
: unknown | valid | invalidgla_onboarding_open_paid_ads_setup_button_click
for Bgla_onboarding_complete_with_paid_ads_button_click
for Dbudget
: should be the same as the entered valueaudiences
: country codes of the campaign audience countries, e.g.'US,JP,AU'
📌 UI
Changelog entry