-
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
[Enterprise Search] Migrate shared Indexing Status component #84571
Merged
scottybollinger
merged 11 commits into
elastic:master
from
scottybollinger:scottybollinger/shared-indexing-status
Dec 1, 2020
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
92db929
Add react-motion package
scottybollinger d435cf4
Add shared interface
scottybollinger 6822405
Migrate IndexingStatusContent component
scottybollinger 569eb59
Migrate IndexingStatusErrors component
scottybollinger 7661303
Migrate IndexingStatus component
scottybollinger 5bf73de
Migrate IndexingStatusFetcher component
scottybollinger 730ac38
Add i18n
scottybollinger e0411e3
Revert "Add react-motion package"
scottybollinger e34f038
Remove motion dependency
scottybollinger beb9589
Update copy
scottybollinger db239d7
Refactor per code review
scottybollinger File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
28 changes: 28 additions & 0 deletions
28
x-pack/plugins/enterprise_search/public/applications/shared/indexing_status/constants.ts
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import { i18n } from '@kbn/i18n'; | ||
|
||
export const INDEXING_STATUS_PROGRESS_TITLE = i18n.translate( | ||
'xpack.enterpriseSearch.indexingStatus.progress.title', | ||
{ | ||
defaultMessage: 'Indexing progress', | ||
} | ||
); | ||
|
||
export const INDEXING_STATUS_HAS_ERRORS_TITLE = i18n.translate( | ||
'xpack.enterpriseSearch.indexingStatus.hasErrors.title', | ||
{ | ||
defaultMessage: 'Several documents have field conversion errors.', | ||
} | ||
); | ||
|
||
export const INDEXING_STATUS_HAS_ERRORS_BUTTON = i18n.translate( | ||
'xpack.enterpriseSearch.indexingStatus.hasErrors.button', | ||
{ | ||
defaultMessage: 'View errors', | ||
} | ||
); |
7 changes: 7 additions & 0 deletions
7
x-pack/plugins/enterprise_search/public/applications/shared/indexing_status/index.ts
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
export { IndexingStatus } from './indexing_status'; |
51 changes: 51 additions & 0 deletions
51
...ins/enterprise_search/public/applications/shared/indexing_status/indexing_status.test.tsx
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { shallow } from 'enzyme'; | ||
|
||
import { EuiPanel } from '@elastic/eui'; | ||
|
||
import { IndexingStatusContent } from './indexing_status_content'; | ||
import { IndexingStatusErrors } from './indexing_status_errors'; | ||
import { IndexingStatusFetcher } from './indexing_status_fetcher'; | ||
import { IndexingStatus } from './indexing_status'; | ||
|
||
describe('IndexingStatus', () => { | ||
const getItemDetailPath = jest.fn(); | ||
const getStatusPath = jest.fn(); | ||
const onComplete = jest.fn(); | ||
const setGlobalIndexingStatus = jest.fn(); | ||
|
||
const props = { | ||
percentageComplete: 50, | ||
numDocumentsWithErrors: 1, | ||
activeReindexJobId: 12, | ||
viewLinkPath: '/path', | ||
itemId: '1', | ||
getItemDetailPath, | ||
getStatusPath, | ||
onComplete, | ||
setGlobalIndexingStatus, | ||
}; | ||
|
||
it('renders', () => { | ||
const wrapper = shallow(<IndexingStatus {...props} />); | ||
const fetcher = wrapper.find(IndexingStatusFetcher).prop('children')( | ||
props.percentageComplete, | ||
props.numDocumentsWithErrors | ||
); | ||
|
||
expect(shallow(fetcher).find(EuiPanel)).toHaveLength(1); | ||
expect(shallow(fetcher).find(IndexingStatusContent)).toHaveLength(1); | ||
}); | ||
|
||
it('renders errors', () => { | ||
const wrapper = shallow(<IndexingStatus {...props} percentageComplete={100} />); | ||
const fetcher = wrapper.find(IndexingStatusFetcher).prop('children')(100, 1); | ||
expect(shallow(fetcher).find(IndexingStatusErrors)).toHaveLength(1); | ||
}); | ||
}); |
44 changes: 44 additions & 0 deletions
44
.../plugins/enterprise_search/public/applications/shared/indexing_status/indexing_status.tsx
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React from 'react'; | ||
|
||
import { EuiPanel, EuiSpacer } from '@elastic/eui'; | ||
|
||
import { IndexingStatusContent } from './indexing_status_content'; | ||
import { IndexingStatusErrors } from './indexing_status_errors'; | ||
import { IndexingStatusFetcher } from './indexing_status_fetcher'; | ||
|
||
import { IIndexingStatus } from '../types'; | ||
|
||
export interface IIndexingStatusProps extends IIndexingStatus { | ||
viewLinkPath: string; | ||
itemId: string; | ||
getItemDetailPath?(itemId: string): string; | ||
getStatusPath(itemId: string, activeReindexJobId: number): string; | ||
onComplete(numDocumentsWithErrors: number): void; | ||
setGlobalIndexingStatus?(activeReindexJob: IIndexingStatus): void; | ||
} | ||
|
||
export const IndexingStatus: React.FC<IIndexingStatusProps> = (props) => ( | ||
<IndexingStatusFetcher {...props}> | ||
{(percentageComplete, numDocumentsWithErrors) => ( | ||
<div> | ||
{percentageComplete < 100 && ( | ||
<EuiPanel paddingSize="l" hasShadow> | ||
<IndexingStatusContent percentageComplete={percentageComplete} /> | ||
</EuiPanel> | ||
)} | ||
{percentageComplete === 100 && numDocumentsWithErrors > 0 && ( | ||
<> | ||
<EuiSpacer /> | ||
<IndexingStatusErrors viewLinkPath={props.viewLinkPath} /> | ||
</> | ||
)} | ||
</div> | ||
)} | ||
</IndexingStatusFetcher> | ||
); |
21 changes: 21 additions & 0 deletions
21
...rprise_search/public/applications/shared/indexing_status/indexing_status_content.test.tsx
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { shallow } from 'enzyme'; | ||
|
||
import { EuiProgress, EuiTitle } from '@elastic/eui'; | ||
|
||
import { IndexingStatusContent } from './indexing_status_content'; | ||
|
||
describe('IndexingStatusContent', () => { | ||
it('renders', () => { | ||
const wrapper = shallow(<IndexingStatusContent percentageComplete={50} />); | ||
|
||
expect(wrapper.find(EuiTitle)).toHaveLength(1); | ||
expect(wrapper.find(EuiProgress)).toHaveLength(1); | ||
}); | ||
}); |
27 changes: 27 additions & 0 deletions
27
.../enterprise_search/public/applications/shared/indexing_status/indexing_status_content.tsx
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React from 'react'; | ||
|
||
import { EuiProgress, EuiSpacer, EuiTitle } from '@elastic/eui'; | ||
|
||
import { INDEXING_STATUS_PROGRESS_TITLE } from './constants'; | ||
|
||
interface IIndexingStatusContentProps { | ||
percentageComplete: number; | ||
} | ||
|
||
export const IndexingStatusContent: React.FC<IIndexingStatusContentProps> = ({ | ||
percentageComplete, | ||
}) => ( | ||
<div data-test-subj="IndexingStatusProgressMeter"> | ||
<EuiTitle size="s"> | ||
<h3>{INDEXING_STATUS_PROGRESS_TITLE}</h3> | ||
</EuiTitle> | ||
<EuiSpacer size="s" /> | ||
<EuiProgress color="primary" size="m" value={percentageComplete} max={100} /> | ||
</div> | ||
); |
25 changes: 25 additions & 0 deletions
25
...erprise_search/public/applications/shared/indexing_status/indexing_status_errors.test.tsx
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { shallow } from 'enzyme'; | ||
|
||
import { EuiCallOut, EuiButton } from '@elastic/eui'; | ||
|
||
import { EuiLinkTo } from '../react_router_helpers'; | ||
|
||
import { IndexingStatusErrors } from './indexing_status_errors'; | ||
|
||
describe('IndexingStatusErrors', () => { | ||
it('renders', () => { | ||
const wrapper = shallow(<IndexingStatusErrors viewLinkPath="/path" />); | ||
|
||
expect(wrapper.find(EuiButton)).toHaveLength(1); | ||
expect(wrapper.find(EuiCallOut)).toHaveLength(1); | ||
expect(wrapper.find(EuiLinkTo)).toHaveLength(1); | ||
expect(wrapper.find(EuiLinkTo).prop('to')).toEqual('/path'); | ||
}); | ||
}); |
31 changes: 31 additions & 0 deletions
31
...s/enterprise_search/public/applications/shared/indexing_status/indexing_status_errors.tsx
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React from 'react'; | ||
|
||
import { EuiButton, EuiCallOut } from '@elastic/eui'; | ||
|
||
import { EuiLinkTo } from '../react_router_helpers'; | ||
|
||
import { INDEXING_STATUS_HAS_ERRORS_TITLE, INDEXING_STATUS_HAS_ERRORS_BUTTON } from './constants'; | ||
|
||
interface IIndexingStatusErrorsProps { | ||
viewLinkPath: string; | ||
} | ||
|
||
export const IndexingStatusErrors: React.FC<IIndexingStatusErrorsProps> = ({ viewLinkPath }) => ( | ||
<EuiCallOut | ||
color="danger" | ||
iconType="cross" | ||
title="There was an error" | ||
data-test-subj="IndexingStatusErrors" | ||
> | ||
<p>{INDEXING_STATUS_HAS_ERRORS_TITLE}</p> | ||
<EuiButton color="danger" fill={true} size="s" data-test-subj="ViewErrorsButton"> | ||
<EuiLinkTo to={viewLinkPath}>{INDEXING_STATUS_HAS_ERRORS_BUTTON}</EuiLinkTo> | ||
</EuiButton> | ||
</EuiCallOut> | ||
); |
64 changes: 64 additions & 0 deletions
64
.../enterprise_search/public/applications/shared/indexing_status/indexing_status_fetcher.tsx
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React, { useEffect, useState, useRef } from 'react'; | ||
|
||
import { HttpLogic } from '../http'; | ||
import { flashAPIErrors } from '../flash_messages'; | ||
|
||
interface IIndexingStatusFetcherProps { | ||
activeReindexJobId: number; | ||
itemId: string; | ||
percentageComplete: number; | ||
numDocumentsWithErrors: number; | ||
onComplete?(numDocumentsWithErrors: number): void; | ||
getStatusPath(itemId: string, activeReindexJobId: number): string; | ||
children(percentageComplete: number, numDocumentsWithErrors: number): JSX.Element; | ||
} | ||
|
||
export const IndexingStatusFetcher: React.FC<IIndexingStatusFetcherProps> = ({ | ||
activeReindexJobId, | ||
children, | ||
getStatusPath, | ||
itemId, | ||
numDocumentsWithErrors, | ||
onComplete, | ||
percentageComplete = 0, | ||
}) => { | ||
const [indexingStatus, setIndexingStatus] = useState({ | ||
numDocumentsWithErrors, | ||
percentageComplete, | ||
}); | ||
const pollingInterval = useRef<number>(); | ||
|
||
useEffect(() => { | ||
pollingInterval.current = window.setInterval(async () => { | ||
try { | ||
const response = await HttpLogic.values.http.get(getStatusPath(itemId, activeReindexJobId)); | ||
if (response.percentageComplete >= 100) { | ||
clearInterval(pollingInterval.current); | ||
} | ||
setIndexingStatus({ | ||
percentageComplete: response.percentageComplete, | ||
numDocumentsWithErrors: response.numDocumentsWithErrors, | ||
}); | ||
if (response.percentageComplete >= 100 && onComplete) { | ||
onComplete(response.numDocumentsWithErrors); | ||
} | ||
} catch (e) { | ||
flashAPIErrors(e); | ||
} | ||
}, 3000); | ||
|
||
return () => { | ||
if (pollingInterval.current) { | ||
clearInterval(pollingInterval.current); | ||
} | ||
}; | ||
}, []); | ||
|
||
return children(indexingStatus.percentageComplete, indexingStatus.numDocumentsWithErrors); | ||
}; | ||
11 changes: 11 additions & 0 deletions
11
x-pack/plugins/enterprise_search/public/applications/shared/types.ts
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
export interface IIndexingStatus { | ||
percentageComplete: number; | ||
numDocumentsWithErrors: number; | ||
activeReindexJobId: number; | ||
} |
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.
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.
Do we plan on writing a test for this file?
Also, just curious - not a blocking request for this PR of course (unless people really like the idea), but at some point do we think it would be worth it to rewrite this file as a Kea logic file instead of as a render prop, and have this be an
actions.fetchIndexingStatus
action that updatesvalues.percentageComplete
andvalues.numDocumentsWithErrors
?Doing so has the advantage of not requiring
useRef
forpollingInterval
, for example.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.
Yeah, I've been toying with doing that but wondered if I had the time. I think I'll just go ahead and do it. I want to get this merged in so I can test it in Kibana though. Everything but this file has tests and my changes will be a fast-follower to this PR
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.
Sweet, sounds good - thanks Scotty!