-
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
Changes from 7 commits
92db929
d435cf4
6822405
569eb59
7661303
5bf73de
730ac38
e0411e3
e34f038
beb9589
db239d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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', | ||
scottybollinger marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
); |
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'; |
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); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,48 @@ | ||||||||||||||
/* | ||||||||||||||
* 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) => { | ||||||||||||||
const statusPanel = (percentComplete: number) => ( | ||||||||||||||
<EuiPanel paddingSize="l" hasShadow> | ||||||||||||||
<IndexingStatusContent percentageComplete={percentComplete} /> | ||||||||||||||
</EuiPanel> | ||||||||||||||
); | ||||||||||||||
|
||||||||||||||
return ( | ||||||||||||||
<IndexingStatusFetcher {...props}> | ||||||||||||||
{(percentageComplete, numDocumentsWithErrors) => ( | ||||||||||||||
<div className="c-stui-indexing-status-wrapper"> | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question here re: CSS classes
Suggested change
|
||||||||||||||
{percentageComplete < 100 && statusPanel(percentageComplete)} | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very minor comment, but I don't see the advantage of pulling out
Suggested change
Also, do we just show nothing if |
||||||||||||||
{percentageComplete === 100 && numDocumentsWithErrors > 0 && ( | ||||||||||||||
<> | ||||||||||||||
<EuiSpacer /> | ||||||||||||||
<IndexingStatusErrors viewLinkPath={props.viewLinkPath} /> | ||||||||||||||
</> | ||||||||||||||
)} | ||||||||||||||
</div> | ||||||||||||||
)} | ||||||||||||||
</IndexingStatusFetcher> | ||||||||||||||
); | ||||||||||||||
}; |
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); | ||
}); | ||
}); |
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> | ||
); |
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'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,32 @@ | ||||
/* | ||||
* 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 | ||||
className="c-stui-indexing-status-errors" | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we using these CSS classes? If not, let's lose them for now and re-add them as needed and w/ correct casing
Suggested change
|
||||
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> | ||||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
/* | ||
* 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 { Motion, spring } from 'react-motion'; | ||
|
||
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; | ||
} | ||
|
||
interface IIndexingStatus { | ||
numDocumentsWithErrors: number; | ||
percentageComplete: number; | ||
} | ||
|
||
interface IMotionStatusProps { | ||
defaultStatus: IIndexingStatus; | ||
currentStatus: IIndexingStatus; | ||
children(percentageComplete: number, numDocumentsWithErrors: number): JSX.Element; | ||
} | ||
|
||
const MotionStatus: React.FC<IMotionStatusProps> = ({ defaultStatus, currentStatus, children }) => ( | ||
<Motion | ||
defaultStyle={{ | ||
percentageComplete: defaultStatus.percentageComplete, | ||
}} | ||
style={{ | ||
percentageComplete: spring(currentStatus.percentageComplete), | ||
numDocumentsWithErrors: currentStatus.numDocumentsWithErrors, | ||
}} | ||
> | ||
{({ percentageComplete, numDocumentsWithErrors }) => | ||
children(percentageComplete, numDocumentsWithErrors) | ||
} | ||
</Motion> | ||
); | ||
|
||
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 ( | ||
<MotionStatus | ||
currentStatus={indexingStatus} | ||
defaultStatus={{ percentageComplete, numDocumentsWithErrors }} | ||
children={children} | ||
/> | ||
); | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Doing so has the advantage of not requiring There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Sweet, sounds good - thanks Scotty! |
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; | ||
} |
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.
I don't think we should bring this library to Kibana. The latest version was released 3 years ago. The library seems to be abandoned: chenglou/react-motion#569