-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Guided onboarding] Added Guided onboarding to the Fleet plugin #142185
[Guided onboarding] Added Guided onboarding to the Fleet plugin #142185
Conversation
Pinging @elastic/platform-onboarding (Team:Journey/Onboarding) |
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.
Nice work @yuliacech! It's so exciting to start seeing this come together 🎉. I tested following the steps you provided and everything worked as expected.
I left a couple nits in the code, and one question around the footer button label for the tour. Otherwise, I'm happy with the changes once CI is green. Going to approve to not block you.
@@ -0,0 +1,27 @@ | |||
/* |
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 for adding this mock!
* An observable with the boolean value if the guided onboarding is currently active for the integration. | ||
* Returns true, if the passed integration is used in the current guide's step. | ||
* Returns false otherwise. | ||
* @param {string} guideID the integration ID (package key) to check for in the guided onboarding config |
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.
typo here I think
* @param {string} guideID the integration ID (package key) to check for in the guided onboarding config | |
* @param {string} the integration ID (package key) to check for in the guided onboarding config |
: of(false) | ||
); | ||
useEffect(() => { | ||
setResult(!!isGuidedOnboardingActiveForIntegration); |
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.
Is the !!
necessary? Shouldn't this always return a boolean?
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 think useObservable
adds an undefined
to the types even when the original Observable is of type boolean
.
footerAction={ | ||
<EuiButton onClick={() => setIsGuidedOnboardingTourOpen(false)} size="s" color="success"> | ||
{i18n.translate('xpack.fleet.guidedOnboardingTour.nextButtonLabel', { | ||
defaultMessage: 'Next', |
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.
Should this be "End tour" or "Complete" instead when there is only one step?
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 would need to confirm with design and copy, but given that the tour doesn't complete the step but helps the user to move along, I think 'Next' could work here.
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.
If clicking the button returns them to the setup guide, then I think "Next" makes perfect sense here.
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 a lot, @kellyemurphy! Yes, the tour is used to point out the button. If it's closed, the guide stays in the same step until the user clicks the "Add integration" button and competes the installation.
Pinging @elastic/fleet (Team:Fleet) |
…t available in fleet
…older in the fleet plugin, also fixed tests
016e6bb
to
e74b69c
Compare
Thanks a lot for the review, @alisonelizabeth 🎉 |
...ications/integrations/sections/epm/screens/detail/components/with_guided_onboarding_tour.tsx
Outdated
Show resolved
Hide resolved
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 locally and the code looks great. Thanks for this! 🚀
…/epm/screens/detail/components/with_guided_onboarding_tour.tsx Co-authored-by: Kelly Murphy <[email protected]>
Thanks a lot for your review, @hop-dev! 👍 |
…-ref HEAD~1..HEAD --fix'
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
…tic#142185) * [Guided onboarding] Added a EuiTour for guided onboarding in Integrations * [Guided onboarding] Added data step completion for Elastic Defend * [Guided onboarding] Added tests for api * [Guided onboarding] Fixed jest tests * [Guided onboarding] Added a fail safe for guided onboarding plugin not available in fleet * [Guided onboarding] Moved the guided onboarding hook to a different folder in the fleet plugin, also fixed tests * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' * [Guided onboarding] Updates after the merge conflicts * [Guided onboarding] Fixed typos * [Guided onboarding] Fixed types error * Update x-pack/plugins/fleet/public/applications/integrations/sections/epm/screens/detail/components/with_guided_onboarding_tour.tsx Co-authored-by: Kelly Murphy <[email protected]> Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Kelly Murphy <[email protected]>
…tic#142185) * [Guided onboarding] Added a EuiTour for guided onboarding in Integrations * [Guided onboarding] Added data step completion for Elastic Defend * [Guided onboarding] Added tests for api * [Guided onboarding] Fixed jest tests * [Guided onboarding] Added a fail safe for guided onboarding plugin not available in fleet * [Guided onboarding] Moved the guided onboarding hook to a different folder in the fleet plugin, also fixed tests * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' * [Guided onboarding] Updates after the merge conflicts * [Guided onboarding] Fixed typos * [Guided onboarding] Fixed types error * Update x-pack/plugins/fleet/public/applications/integrations/sections/epm/screens/detail/components/with_guided_onboarding_tour.tsx Co-authored-by: Kelly Murphy <[email protected]> Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Kelly Murphy <[email protected]>
Summary
Fixes #140610, #138646
Partially addresses #139712
This PR is a part of the Guided onboarding work for 8.6, specifically these changes are important for the Security guide, the "Add data" step. When this step is active, the Integrations detail page for Elastic Defend will display a EUI tour pointing out the "Add integration" button. Also this button will redirect the user to the new Agent flow not only for the first agent install, but for repeat install as well. Other conditions for the new Agent flow are unchanged and it seems safe to redirect the user to the new flow even if they already have an Agent installed. The last change is on the data confirmation page. If the data step is active, the confirmation page will send a signal to the Guided onboarding plugin to mark the step as completed when the incoming data is detected.
The e2e UI tests will be addressed in a follow up PR (see #138568).
How to test
In Kibana, navigate to
http://localhost:5601/app/guidedOnboardingExample
and start the Security guide. Make sure the green guided onboarding button is displayed in the header and the first step is active.In the guided onboarding checklist panel, click the "Start" button in the first step. Or alternatively navigate to
http://localhost:5601/app/integrations/browse/security
. Click the Elastic Defend card to open the details page.On the Elastic Defend details page, make sure the EUI tour is displayed.
When clicking on the "Add integration" button, make sure the new Agent flow is used.
Complete the flow and make sure, when the incoming data is detected, the guided onboarding step is marked completed.
Delete the integration and restart the Security guide, but don't delete the installed Agent. Make sure the new flow is still used when installing the integration.
Screenshots
EUI tour on the details page
The data step is marked completed
Checklist