-
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
[Security solution] Reinstall product documentation callout #205975
base: main
Are you sure you want to change the base?
Conversation
Pinging @elastic/security-solution (Team: SecuritySolution) |
…into product_doc_install
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 looks good. Left a comment but it is a nitpick.
const [isInstalled, setInstalled] = useState<boolean>(true); | ||
const [isInstalling, setInstalling] = useState<boolean>(false); |
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.
Are the additional states isInstalled
and isInstalling
necessary? I believe they could be removed with something like this which would prevent a re-render:
const [isInstalled, setInstalled] = useState<boolean>(true); | |
const [isInstalling, setInstalling] = useState<boolean>(false); | |
const { mutateAsync: installProductDoc, isLoading: isInstalling } = useInstallProductDoc(); | |
const { status, isLoading: isStatusLoading } = useGetProductDocStatus(); | |
const isInstalledOrUnknown = status == undefined ? true : status.overall === 'installed' | |
... | |
if (isInstalledOrUnknown) { | |
return null; | |
} |
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.
This does not track if isInstalling
, we need to show loading indicator
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 went ahead and combined state values to avoid separate state updates triggering multiple renders.
Thanks for this @stephmilovic. For the text description, can we use the following instead?
|
…into product_doc_install
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
|
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.
Rules area test changes LGTM
Summary
Product documentation is installed when the assistant is initialized. This PR gives a path for the user to reinstall the product documentation in case it gets deleted (via o11y assistant settings)
Not installed
Installing
Installed
To test
app/management/kibana/observabilityAiAssistantManagement
=> Uninstall)