-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[App Search] API Logs: Add ApiLogsTable and NewApiEventsPrompt components #96008
Changes from all commits
e432834
91315db
0876999
220a1b3
427d480
cf897c4
a30b78b
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,5 @@ | ||
.apiLogDetailButton { | ||
// More closely mimics the regular line height of an EuiLink / | ||
// compresses table rows back to the standard height | ||
height: $euiSizeL !important; | ||
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. 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. If it were me, I'd say just let EUI do it's thing. However, if there's something unique about this table that is forcing the rows to be taller than usual, then I'd say this is a worthwhile change. It's up to you though. Is it worthwhile to drop a comment in the CSS about why you have added that or nah? To be clear, I'm good either way, I'll approve it regardless. 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. The first screenshot more closely matches the row heights in all their examples in their docs. The main difference is that their buttons are all icon buttons and not empty buttons 🤷 It does seem odd that EUI wouldn't have any affordance OOTB for EuiEmptyButton matching the same line height as an EuiLink. A comment makes a ton of sense! I'll add one here in a bit. 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. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { setMockValues, setMockActions, mountWithIntl } from '../../../../__mocks__'; | ||
|
||
// NOTE: We're mocking FormattedRelative here because it (currently) has | ||
// console warn issues, and it allows us to skip mocking dates | ||
jest.mock('@kbn/i18n/react', () => ({ | ||
...(jest.requireActual('@kbn/i18n/react') as object), | ||
FormattedRelative: jest.fn(() => '20 hours ago'), | ||
})); | ||
|
||
import React from 'react'; | ||
|
||
import { shallow } from 'enzyme'; | ||
|
||
import { EuiBasicTable, EuiBadge, EuiHealth, EuiButtonEmpty, EuiEmptyPrompt } from '@elastic/eui'; | ||
|
||
import { DEFAULT_META } from '../../../../shared/constants'; | ||
|
||
import { ApiLogsTable } from './'; | ||
|
||
describe('ApiLogsTable', () => { | ||
const apiLogs = [ | ||
{ | ||
timestamp: '1970-01-01T00:00:00.000Z', | ||
status: 404, | ||
http_method: 'GET', | ||
full_request_path: '/api/as/v1/test', | ||
}, | ||
{ | ||
timestamp: '1970-01-01T00:00:00.000Z', | ||
status: 500, | ||
http_method: 'DELETE', | ||
full_request_path: '/api/as/v1/test', | ||
}, | ||
{ | ||
timestamp: '1970-01-01T00:00:00.000Z', | ||
status: 200, | ||
http_method: 'POST', | ||
full_request_path: '/api/as/v1/engines/some-engine/search', | ||
}, | ||
]; | ||
|
||
const values = { | ||
dataLoading: false, | ||
apiLogs, | ||
meta: DEFAULT_META, | ||
}; | ||
const actions = { | ||
onPaginate: jest.fn(), | ||
}; | ||
|
||
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
setMockValues(values); | ||
setMockActions(actions); | ||
}); | ||
|
||
it('renders', () => { | ||
const wrapper = mountWithIntl(<ApiLogsTable />); | ||
const tableContent = wrapper.find(EuiBasicTable).text(); | ||
|
||
expect(tableContent).toContain('Method'); | ||
expect(tableContent).toContain('GET'); | ||
expect(tableContent).toContain('DELETE'); | ||
expect(tableContent).toContain('POST'); | ||
expect(wrapper.find(EuiBadge)).toHaveLength(3); | ||
|
||
expect(tableContent).toContain('Time'); | ||
expect(tableContent).toContain('20 hours ago'); | ||
|
||
expect(tableContent).toContain('Endpoint'); | ||
expect(tableContent).toContain('/api/as/v1/test'); | ||
expect(tableContent).toContain('/api/as/v1/engines/some-engine/search'); | ||
|
||
expect(tableContent).toContain('Status'); | ||
expect(tableContent).toContain('404'); | ||
expect(tableContent).toContain('500'); | ||
expect(tableContent).toContain('200'); | ||
expect(wrapper.find(EuiHealth)).toHaveLength(3); | ||
|
||
expect(wrapper.find(EuiButtonEmpty)).toHaveLength(3); | ||
wrapper.find('[data-test-subj="ApiLogsTableDetailsButton"]').first().simulate('click'); | ||
// TODO: API log details flyout | ||
Comment on lines
+88
to
+89
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. So it looks like you intend to test the flyout opening in the primary 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. Oops, just realized I forgot to respond to this - sorry Jason! Yeah the tables are a bit weird testing-wise and mount is a somewhat-expensive render so I figured just throw it in the main test. I can move it out though if we prefer |
||
}); | ||
|
||
it('renders an empty prompt if no items are passed', () => { | ||
setMockValues({ ...values, apiLogs: [] }); | ||
const wrapper = mountWithIntl(<ApiLogsTable />); | ||
const promptContent = wrapper.find(EuiEmptyPrompt).text(); | ||
|
||
expect(promptContent).toContain('No recent logs'); | ||
}); | ||
|
||
describe('hasPagination', () => { | ||
it('does not render with pagination by default', () => { | ||
const wrapper = shallow(<ApiLogsTable />); | ||
|
||
expect(wrapper.find(EuiBasicTable).prop('pagination')).toBeFalsy(); | ||
}); | ||
|
||
it('renders pagination if hasPagination is true', () => { | ||
const wrapper = shallow(<ApiLogsTable hasPagination />); | ||
|
||
expect(wrapper.find(EuiBasicTable).prop('pagination')).toBeTruthy(); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import React from 'react'; | ||
|
||
import { useValues, useActions } from 'kea'; | ||
|
||
import { | ||
EuiBasicTable, | ||
EuiBasicTableColumn, | ||
EuiBadge, | ||
EuiHealth, | ||
EuiButtonEmpty, | ||
EuiEmptyPrompt, | ||
} from '@elastic/eui'; | ||
import { i18n } from '@kbn/i18n'; | ||
import { FormattedRelative } from '@kbn/i18n/react'; | ||
|
||
import { convertMetaToPagination, handlePageChange } from '../../../../shared/table_pagination'; | ||
|
||
import { ApiLogsLogic } from '../index'; | ||
import { ApiLog } from '../types'; | ||
import { getStatusColor } from '../utils'; | ||
|
||
import './api_logs_table.scss'; | ||
|
||
interface Props { | ||
hasPagination?: boolean; | ||
} | ||
export const ApiLogsTable: React.FC<Props> = ({ hasPagination }) => { | ||
const { dataLoading, apiLogs, meta } = useValues(ApiLogsLogic); | ||
const { onPaginate } = useActions(ApiLogsLogic); | ||
|
||
const columns: Array<EuiBasicTableColumn<ApiLog>> = [ | ||
{ | ||
field: 'http_method', | ||
name: i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.methodTableHeading', { | ||
defaultMessage: 'Method', | ||
}), | ||
width: '100px', | ||
render: (method: string) => <EuiBadge color="primary">{method}</EuiBadge>, | ||
}, | ||
{ | ||
field: 'timestamp', | ||
name: i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.timeTableHeading', { | ||
defaultMessage: 'Time', | ||
}), | ||
width: '20%', | ||
render: (dateString: string) => <FormattedRelative value={new Date(dateString)} />, | ||
cee-chen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
{ | ||
field: 'full_request_path', | ||
name: i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.endpointTableHeading', { | ||
defaultMessage: 'Endpoint', | ||
}), | ||
width: '50%', | ||
truncateText: true, | ||
mobileOptions: { | ||
// @ts-ignore - EUI's typing is incorrect here | ||
width: '100%', | ||
}, | ||
}, | ||
{ | ||
field: 'status', | ||
name: i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.statusTableHeading', { | ||
defaultMessage: 'Status', | ||
}), | ||
dataType: 'number', | ||
width: '100px', | ||
render: (status: number) => <EuiHealth color={getStatusColor(status)}>{status}</EuiHealth>, | ||
}, | ||
{ | ||
width: '100px', | ||
align: 'right', | ||
render: (apiLog: ApiLog) => ( | ||
<EuiButtonEmpty | ||
size="s" | ||
className="apiLogDetailButton" | ||
data-test-subj="ApiLogsTableDetailsButton" | ||
// TODO: flyout onclick | ||
> | ||
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.detailsButtonLabel', { | ||
defaultMessage: 'Details', | ||
})} | ||
</EuiButtonEmpty> | ||
), | ||
}, | ||
]; | ||
|
||
const paginationProps = hasPagination | ||
? { | ||
pagination: { | ||
...convertMetaToPagination(meta), | ||
hidePerPageOptions: true, | ||
}, | ||
onChange: handlePageChange(onPaginate), | ||
} | ||
: {}; | ||
|
||
return ( | ||
<EuiBasicTable | ||
columns={columns} | ||
items={apiLogs} | ||
responsive | ||
loading={dataLoading} | ||
noItemsMessage={ | ||
<EuiEmptyPrompt | ||
iconType="clock" | ||
title={ | ||
<h3> | ||
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.emptyTitle', { | ||
defaultMessage: 'No recent logs', | ||
})} | ||
</h3> | ||
} | ||
body={ | ||
<p> | ||
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.emptyDescription', { | ||
defaultMessage: "Check back after you've performed some API calls.", | ||
})} | ||
</p> | ||
} | ||
/> | ||
} | ||
{...paginationProps} | ||
/> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
export { ApiLogsTable } from './api_logs_table'; | ||
export { NewApiEventsPrompt } from './new_api_events_prompt'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
.newApiEventsPrompt { | ||
padding: $euiSizeXS; | ||
padding-left: $euiSizeS; | ||
Comment on lines
+2
to
+3
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. FYI: without this custom padding, the height of the panel exceeds the height of the row height and causes the page to bounce around awkwardly, so I added it as a polish item |
||
display: flex; | ||
align-items: center; | ||
} |
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.
Everything inside here is mostly the same as before, I just added
EuipageContent
andEuiPageContentBody
wrappers - I recommend viewing with hidden whitespace changes for easier diffing