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] [Sourcerer] Store and type cleanup #116640

Merged

Conversation

stephmilovic
Copy link
Contributor

@stephmilovic stephmilovic commented Oct 28, 2021

Summary

Let's merge this into elastic:sourcerer_kip_as to make life easier, double bug fixing is not ideal.

Closes https://github.com/elastic/security-team/issues/1730

  1. Moves data view related field state from sourcererScopes to kibanaDataViews
  2. enhances the useSourcererScope => now renamed to useSourcererDataView which combines the redux state of the sourcererScope and its corresponding kibanaDataView to get the active SourcererDataView
  3. Updates the deprecated IIndexPattern type to DataViewBase

@stephmilovic stephmilovic added release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting Security Solution Threat Hunting Team Team:Threat Hunting:Explore v8.0.0 and removed skip-ci labels Oct 29, 2021
@stephmilovic stephmilovic marked this pull request as ready for review October 29, 2021 14:34
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

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

showResults: indicesExist || defaultDataView.title.includes(`${signalIndexNameSourcerer}`),
}),
const isSignalIndexNeedsInit = useMemo(
() => !indicesExist && !defaultDataView.title.includes(`${signalIndexNameSourcerer}`),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

signalIndexNameSourcerer seems could be null, although I guess defaultDataView.title could never includes null string, it just a bit weird to covert null to string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes its for a type reason, cant check if a string includes null

@@ -74,7 +86,8 @@ export const useInitSourcerer = (
signalIndexName != null &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please give me an example of how it happen in UI between Line86-90?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I renamed signalIndexNameSelector to signalIndexNameSourcerer so maybe that will help.

if the signal index is not loading, the signalIndexName is defined in useSignalIndex, but the signalIndexNameSourcerer has not been defined. This can happen the first time signal index is created. We also check to make sure that the Timeline sourcerer isn't already initiated and that default exists. Does that help?

    if (
      !loadingSignalIndex &&
      signalIndexName != null &&
      signalIndexNameSourcerer == null &&
      (activeTimeline == null || activeTimeline.savedObjectId == null) &&
      initialTimelineSourcerer.current &&
      defaultDataView.id.length > 0
    )

@kibanamachine
Copy link
Contributor

💔 Build Failed

Failed CI Steps

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [8286a33]

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@stephmilovic stephmilovic merged commit ac0b022 into elastic:sourcerer_kip_as Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants