-
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
[7.x] [SIEM] Implement NP Plugin Setup (#54030) #54219
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Set up our react app in the NP way * Defines the setup() method for our UI plugin * Renders the app in the NP way within our setup() method * Defines a legacy file that invokes the plugin manually Things seem to be mostly working; the app mounts with no immediate errors, at least. * Move files into NP structure Our plugin function and class are both direct children of siem/public. The app folder contains both our React app and the function to render it. * Register SIEM in the feature catalogue via NP format Unfortunately, this can't live in the plugin for now because it doesn't get invoked when we need it. For now, it's going to live in the same spot, and once we're a real NP plugin we can move it. * Eliminate usage of timezoneBrowser UI setting This seems to be redundant with dateFormat:tz except that it always returns a real timezone, not just a preference. By wrapping that logic in our own hook, useTimeZone, we can remove this weird usage and stick to the standard dateFormat and dateFormat:tz. * Clean up tests for FormattedDate components Mocks our simpler wrapping hooks rather than the entire UI Settings module. * Remove remaining uses of UI Settings mocks These remaining tests can mock settings directly, or otherwise were misusing the settings mocks to retrieve assertion values. * Remove unnecessary intermediate `describe` blocks They were not adding any information to the tests. * Remove use of kibana version in client requests We were previously passing this version all over the place for the sake of our framework-specific request header. The sole advantage of supplying such a header is that the client will receive an informative error modal in the case of a version mismatch between the client and server. We can successfully perform these requests with the `kbn-xsrf` header instead. Long-term, we can use core.http.fetch to perform the requests and auto-populate the version header, but it would be nicer to abstract those requests to the framework level rather than threading the HTTP client throughout the application. * Remove newly added uses of kbnVersion These happened on master in the meantime. * Use helper to generate test assertion Allows us to change the implementation of the empty string without breaking the test. * Remove guard from date formatting component We're always going to get back usable values from these hooks; while the user can unset the dateFormat in their settings, we'll still get an empty string which is effectively the same as no formatting (as evidenced in the tests). * Remove default from byte formatting component If the user has deleted this default, they presumably meant to do so and we shouldn't supersede it. * Refactor bytes formatting to allow use in our charts We need a formatting function to use with our charts, so this splits out a hook from the original react component, allowing our charts to be formatted as specified in the user's UI settings. * Refer to our constant for APP_ID * Explicit return values for some UI Settings hooks This forces accidental changes to the return value to be explicit. * Remove use of ui/chrome in request header This is an unnecessary use: kibana works the same no matter what contents the `kbn-xsrf` header contains (as long as it's there). * Mock UI Settings values in our TestProvider When using our TestProvider components, we were previously relying on platform's UISettings mocks instead of our own, more comprehensive ones. This worked for the most part, and when we needed real settings we would mock the UI Settings client manually. When we removed some app code that defaulted UI Settings values when the client did not return a value, tests that used TestProviders but also relied on those defaults broke. This adds that behavior back, and obviates the need for manual calls to jest.mock except when we're a) not using TestProviders but b) overriding the platform mocks. Also removes some of those unneeded uses. * Remove unused import Co-authored-by: Elastic Machine <[email protected]>
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backports the following commits to 7.x: