Skip to content
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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

stephmilovic
Copy link
Contributor

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

Screenshot 2025-01-08 at 3 04 15 PM

Installing

Screenshot 2025-01-08 at 3 04 23 PM

Installed

Screenshot 2025-01-08 at 3 07 04 PM

To test

  1. Visit the KB settings page and ensure no callout is present for installing product documentation
  2. Uninstall the product documentation from o11y assistant settings (app/management/kibana/observabilityAiAssistantManagement => Uninstall)
  3. Revisit the KB settings page and ensure the callout is present for installing product documentation
  4. Press install
  5. Once installation is complete, the callout should no longer be present

@stephmilovic stephmilovic added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:Security Generative AI Security Generative AI v8.18.0 labels Jan 8, 2025
@stephmilovic stephmilovic requested review from a team as code owners January 8, 2025 22:12
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Copy link
Member

@KDKHD KDKHD left a 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.

Comment on lines 23 to 24
const [isInstalled, setInstalled] = useState<boolean>(true);
const [isInstalling, setInstalling] = useState<boolean>(false);
Copy link
Member

@KDKHD KDKHD Jan 9, 2025

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:

Suggested change
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;
}

Copy link
Contributor Author

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

Copy link
Contributor Author

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.

@stephmilovic stephmilovic requested a review from a team as a code owner January 9, 2025 15:32
@jamesspi
Copy link

jamesspi commented Jan 9, 2025

Thanks for this @stephmilovic. For the text description, can we use the following instead?

The Elastic Documentation has been uninstalled. Please reinstall to ensure the most accurate results from the AI Assistant.

@stephmilovic stephmilovic requested a review from a team as a code owner January 9, 2025 20:36
@stephmilovic stephmilovic requested a review from xcrzx January 9, 2025 20:36
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 6536 6541 +5

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 22.2MB 22.2MB +3.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 88.2KB 88.2KB +32.0B

History

Copy link
Contributor

@xcrzx xcrzx left a 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:Security Generative AI Security Generative AI Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants