[Security Solution] fixes Data Quality dashboard errors when a basePath is configured
#156233
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
andrew-goldstein
added
bug
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
Files by Code Ownerelastic/security-threat-hunting-explore
|
stephmilovic
reviewed
May 1, 2023
| jest.clearAllMocks(); | ||
|
|
||
| render( | ||
| <TestProviders> | ||
| <DataQualityDetails {...defaultProps} /> | ||
| </TestProviders> | ||
| ); | ||
|
|
||
| await waitFor(() => {}); |
There was a problem hiding this comment.
Guessing it's to make sure the DataQuality dashboard renders? Either way, do you mind adding a comment for these @andrew-goldstein ? 😄
stephmilovic
reviewed
May 1, 2023
| @@ -200,46 +202,50 @@ const DataQualityComponent: React.FC = () => { | |||
| [createCaseFlyout] | |||
| ); | |||
|
|
|||
| if (isSourcererLoading) { | |||
There was a problem hiding this comment.
Suggested change
| if (isSourcererLoading) { | |
| if (isSourcererLoading || isSignalIndexNameLoading) { |
Why not move both conditions up?
stephmilovic
approved these changes
May 1, 2023
michaelolo24
reviewed
May 1, 2023
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| describe('happy path', () => { |
There was a problem hiding this comment.
nit: If you do end up adding another commit, can you re-label this to successful response from ilm endpoint or something.. This is fine too, just not the most descriptive 😄
michaelolo24
reviewed
May 1, 2023
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| describe('happy path', () => { |
There was a problem hiding this comment.
samesies if you end up adding a commit
michaelolo24
reviewed
May 1, 2023
| mockHttpFetch.mockResolvedValue(mockStatsGreenIndex); | ||
| }); | ||
|
|
||
| test('it returns the expected stats', async () => { |
There was a problem hiding this comment.
Side question. I've seen it debated on a few PR's. Do you consider a benefit to having separate test blocks vs multiple expects in a single test block? Or even just testing the whole result.current object in one test?
There was a problem hiding this comment.
Side question. I've seen it debated on a few PR's. Do you consider a benefit to having separate test blocks vs multiple expects in a single test block? Or even just testing the whole
result.currentobject in one test?
I generally prefer to read and write "one assertion per test". That said, these tests had redundant test setup code. Per our offline discussion, I refactored-out the redundant setup code, and applied this pattern to the other test files in this PR.
michaelolo24
approved these changes
May 1, 2023
…sePath` is configured This PR implements a fix for an [issue](elastic#156231) where the Data Quality dashboard displays errors when a `basePath` is configured, preventing indices from being checked. ### Desk testing - Verify the fix per the reporduction steps in <elastic#156231> - Also verify the page still behaves correctly when Kibana is started with no base path, via: ``` yarn start --no-base-path ``` ### Before / after screenshots **Before:**  _Above: Before the fix, errors occur when a `basePath` is configured_ **After:**  _Above: After the fix, errors do NOT occur_
andrew-goldstein
force-pushed
the
data-quality-base-path-fix
branch
from
f891f21
to
8ebf765
Compare
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
May 4, 2023
…ath` is configured (elastic#156233) ## [Security Solution] fixes Data Quality dashboard errors when a `basePath` is configured This PR implements a fix for an [issue](elastic#156231) where the Data Quality dashboard displays errors when a `basePath` is configured, preventing indices from being checked. ### Desk testing - Verify the fix per the reporduction steps in <elastic#156231> - Also verify the page still behaves correctly when Kibana is started with no base path, via: ``` yarn start --no-base-path ``` ### Before / after screenshots **Before:**  _Above: Before the fix, errors occur when a `basePath` is configured_ **After:**  _Above: After the fix, errors do NOT occur_ (cherry picked from commit 3389a2b)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
May 4, 2023
…`basePath` is configured (#156233) (#156730) # Backport This will backport the following commits from `main` to `8.8`: - [[Security Solution] fixes Data Quality dashboard errors when a `basePath` is configured (#156233)](#156233) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Andrew Macri","email":"andrew.macri@elastic.co"},"sourceCommit":{"committedDate":"2023-05-04T19:24:39Z","message":"[Security Solution] fixes Data Quality dashboard errors when a `basePath` is configured (#156233)\n\n## [Security Solution] fixes Data Quality dashboard errors when a `basePath` is configured\r\n\r\nThis PR implements a fix for an [issue](#156231) where the Data Quality dashboard displays errors when a `basePath` is configured, preventing indices from being checked.\r\n\r\n### Desk testing\r\n\r\n- Verify the fix per the reporduction steps in <https://github.com/elastic/kibana/issues/156231>\r\n- Also verify the page still behaves correctly when Kibana is started with no base path, via:\r\n\r\n```\r\nyarn start --no-base-path\r\n```\r\n\r\n### Before / after screenshots\r\n\r\n**Before:**\r\n\r\n\r\n\r\n_Above: Before the fix, errors occur when a `basePath` is configured_\r\n\r\n**After:**\r\n\r\n\r\n\r\n_Above: After the fix, errors do NOT occur_","sha":"3389a2b35bf65fbb32ab10769997d4a210e95607","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team: SecuritySolution","Team:Threat Hunting:Investigations","v8.8.0","v8.9.0"],"number":156233,"url":"#156233 Solution] fixes Data Quality dashboard errors when a `basePath` is configured (#156233)\n\n## [Security Solution] fixes Data Quality dashboard errors when a `basePath` is configured\r\n\r\nThis PR implements a fix for an [issue](#156231) where the Data Quality dashboard displays errors when a `basePath` is configured, preventing indices from being checked.\r\n\r\n### Desk testing\r\n\r\n- Verify the fix per the reporduction steps in <https://github.com/elastic/kibana/issues/156231>\r\n- Also verify the page still behaves correctly when Kibana is started with no base path, via:\r\n\r\n```\r\nyarn start --no-base-path\r\n```\r\n\r\n### Before / after screenshots\r\n\r\n**Before:**\r\n\r\n\r\n\r\n_Above: Before the fix, errors occur when a `basePath` is configured_\r\n\r\n**After:**\r\n\r\n\r\n\r\n_Above: After the fix, errors do NOT occur_","sha":"3389a2b35bf65fbb32ab10769997d4a210e95607"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"#156233 Solution] fixes Data Quality dashboard errors when a `basePath` is configured (#156233)\n\n## [Security Solution] fixes Data Quality dashboard errors when a `basePath` is configured\r\n\r\nThis PR implements a fix for an [issue](#156231) where the Data Quality dashboard displays errors when a `basePath` is configured, preventing indices from being checked.\r\n\r\n### Desk testing\r\n\r\n- Verify the fix per the reporduction steps in <https://github.com/elastic/kibana/issues/156231>\r\n- Also verify the page still behaves correctly when Kibana is started with no base path, via:\r\n\r\n```\r\nyarn start --no-base-path\r\n```\r\n\r\n### Before / after screenshots\r\n\r\n**Before:**\r\n\r\n\r\n\r\n_Above: Before the fix, errors occur when a `basePath` is configured_\r\n\r\n**After:**\r\n\r\n\r\n\r\n_Above: After the fix, errors do NOT occur_","sha":"3389a2b35bf65fbb32ab10769997d4a210e95607"}}]}] BACKPORT--> Co-authored-by: Andrew Macri <andrew.macri@elastic.co>
jloleysens
added a commit
that referenced
this pull request
May 5, 2023
\r\n\r\n_Above){kind=link}
\r\n\r\n_Above){kind=link}
* main: (153 commits)
[Security Solution] {{state.signals_count}} Object not working (#156472) (#156707)
[Synthetics] refresh data on visualization scrubbing (#156777)
[RAM] Docs for slack improvements (#153885)
[RAM] Alert search bar only KQL (#155947)
[ML] Functional tests - stabilize export job tests (#156586)
[Saved Search] Update saved search schema to allow empty `sort` arrays (#156769)
[ML] Rename `curated` model type to `elastic` (#156684)
[Discover] Enable sharing for text based languages (#156652)
[api-docs] 2023-05-05 Daily api_docs build (#156781)
Upgrade EUI to v77.2.2 (#155208)
[RAM][Maintenance Window][8.8]Fix window maintenance workflow (#156427)
[DOCS] Case file attachments (#156459)
[D4C] additional error handling for 'block' action added + policy editor UI fixes (#156629)
[Enterprise Search] refactor(SearchApplications): rename telemetry ids (#156733)
[Enterprise Search] Add telemetry to ELSER deployment buttons + error (#156545)
[Security Solution] fixes Data Quality dashboard errors when a `basePath` is configured (#156233)
[Logs onboarding] StepsFooter outside of main panel (#156686)
[Security Solution] Add a migration to unmute custom Security Solution rules (#156593)
[Enterprise Search][Behavioral Analytics] Update formulas (#156704)
Add API Events to Endpoint Security Advanced Policy (#156718)
...
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.
[Security Solution] fixes Data Quality dashboard errors when a
basePathis configuredThis PR implements a fix for an issue where the Data Quality dashboard displays errors when a
basePathis configured, preventing indices from being checked.Desk testing
basePathis configured #156231Before / after screenshots
Before:
Above: Before the fix, errors occur when a
basePathis configuredAfter:
Above: After the fix, errors do NOT occur