added point in time table #3264
Conversation
Signed-off-by: Ajay Gupta <[email protected]>
|
|
||
| // TODO: use APIs to fetch PITs and update the table and message | ||
| const [pits] = useState([]); | ||
| const [message] = useState(<EmptyState />); |
There was a problem hiding this comment.
Why is this a component - not sure if we should use state to save components, can we use a boolean to either show zero state or table instead ?
There was a problem hiding this comment.
So we will be adding and using setMessage to update the message based on the response from the get PIT API. There can be other states like error fetching data.
There was a problem hiding this comment.
I think we don't need to bring extra state management. In our core service, we provide an embedded PersistedState class.
This class is an implementation of a persistent state management system. It manages and persists the state of an application across reloads and page transitions.
The PersistedState class has get, set, and setSilent methods. You can use the set method to update the state of the PersistedState object.
| <EuiPageContentBody> | ||
| <EuiInMemoryTable | ||
| items={pits} | ||
| itemId="name" |
There was a problem hiding this comment.
I see itemId="id" used in other pages - can we use the same for consistency?
There was a problem hiding this comment.
Will update this in following PR based on the property name we have in the PIT object.
| sort: string; | ||
| } | ||
|
|
||
| const PITTable = ({ history }: RouteComponentProps) => { |
There was a problem hiding this comment.
Can we please add unit tests for this component ?
There was a problem hiding this comment.
Will add tests in the following PR when we add logic to fetch the PITs
joshuarrrr
added
the
Skip-Changelog
|
@joshuarrrr @kavilla @ashwin-pc can you please review this change? Thanks |
There was a problem hiding this comment.
The "Point in Time Management" plugin adds a new functionality to OpenSearch Dashboards, which allows users to take a snapshot of their data at a specific point in time and then perform queries against that snapshot. The changes made in the pull request seem to be related to the plugin's UI, specifically in the src/plugins/point_in_time_management/public directory.
Have talked to @ajygupta and the project is targeting v2.8 release. Please feel free to add more pull request to my tracking issue: #3589
Thank you!
|
|
||
| export async function mountManagementSection( | ||
| getStartServices: StartServicesAccessor<PointInTimeManagementStartDependencies>, | ||
| params: ManagementAppMountParams | ||
| ) { | ||
| const [{ chrome, application }] = await getStartServices(); | ||
| const [{ chrome, application, savedObjects, notifications }] = await getStartServices(); | ||
| const deps: PointInTimeManagementContext = { |
There was a problem hiding this comment.
NIT: Instead of passing params object to multiple functions and extracting its properties, maybe consider destructuring it at the beginning of the function to make the code cleaner.
const { setBreadcrumbs, history, element } = params;
|
|
||
| // TODO: use APIs to fetch PITs and update the table and message | ||
| const [pits] = useState([]); | ||
| const [message] = useState(<EmptyState />); |
There was a problem hiding this comment.
I think we don't need to bring extra state management. In our core service, we provide an embedded PersistedState class.
This class is an implementation of a persistent state management system. It manages and persists the state of an application across reloads and page transitions.
The PersistedState class has get, set, and setSilent methods. You can use the set method to update the state of the PersistedState object.
| sort: string; | ||
| } | ||
|
|
||
| const PITTable = ({ history }: RouteComponentProps) => { |
|
@ajygupta could you check my comments and make changes? thanks |
|
@ajygupta We'll move this to draft until you're able to get back to the changes requested. |
Signed-off-by: Ajay Gupta [email protected]
Description
Updated the Empty state with the point in time table. Added routing, breadcrumbs and data source selection support.
There is integration with the backend APIs and saved objects to fetch PITs and populated the table which would be done in following PRs.
Issues Resolved
#1998
Check List
yarn test:jestyarn test:jest_integrationyarn test:ftr