-
Notifications
You must be signed in to change notification settings - Fork 8
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
Cypress tests for Events in site-features page #522
Conversation
it( 'Check to make sure Sidebar opens', () => { | ||
BasicSidebarCheck(); | ||
} ); | ||
}; | ||
} |
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.
Why have we removed ';' at the end?
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.
Its due to linting. I used command provided earlier.
npx wp-scripts lint-js <file-path> --no-ignore --fix
@@ -18,7 +18,11 @@ export const APIList = { | |||
basic_info: | |||
'/index.php?rest_route=%2Fnewfold-onboarding%2Fv1%2Fevents%2Fbatch&flow=wp-setup&_locale=user', | |||
basic_info_ecomm: | |||
'/index.php?rest_route=%2Fnewfold-onboarding%2Fv1%2Fevents%2Fbatch&flow=ecommerce&_locale=user' | |||
'/index.php?rest_route=%2Fnewfold-onboarding%2Fv1%2Fevents%2Fbatch&flow=ecommerce&_locale=user', |
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.
we are using the same API for general and ecomm flow, so I think it's better to have only 2 APIs, one for general and one for ecomm.
also, please update where the APIs have been used.
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.
Done
GA events test scripts for site-features step from both general onboarding and ecommerce onboarding