Skip to content

[Index Management] Index details page: loading index data#163955

Merged
yuliacech merged 21 commits intoelastic:mainfrom
yuliacech:im/details_page/api_index_details
Aug 23, 2023
Merged

[Index Management] Index details page: loading index data#163955
yuliacech merged 21 commits intoelastic:mainfrom
yuliacech:im/details_page/api_index_details

Conversation

@yuliacech
Copy link
Copy Markdown
Contributor

@yuliacech yuliacech commented Aug 15, 2023

Summary

Follow up to #163521

  • A new internal api endpoint to load index data for a single index specified by the index name
  • An api service to send a request to the new endpoint
  • A loading and an error state on the new index details page
  • A "back to all indices" button
  • A button with the link to Discover
  • A context menu with index actions (only popover with options is displayed, but the actions are not implemented yet)

Screenshots

Screenshot 2023-08-23 at 14 31 26 Screenshot 2023-08-22 at 17 43 13 Screenshot 2023-08-22 at 17 42 30

How to test

  1. Add xpack.index_management.dev.enableIndexDetailsPage: true to your /config/kibana.dev.yml file
  2. Start ES and Kibana with yarn es snapshot and yarn start
  3. Add at least 1 sample data test
  4. Navigate to Index Management and click the name of any index
  5. Check that the button "back to indices" works (navigates back to the indices list)
  6. Check that the button "discover index" works (navigates to Discover)
  7. Check that the button "manage index" opens the context menu (note: index actions are not implemented yet)
  8. Check that there is a loading indicator (by adding throttle in the Network tab of Chrome Dev tools)
  9. Check that the error section is displayed and the "reload" button resends a request (by blocking the request in the Network tab of Chrome Dev Tools)

Checklist

testBed.component.update();
});

describe('error section', () => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not testing the loading indicator here, because I think I could re-use what @sabarasaba implemented for enrich policies here. So I'm planning to add the test later when enrich policies are merged into main.

expect(testBed.routerMock.history.push).toHaveBeenCalledWith('/indices');
});

it('renders a link to discover', () => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only testing that the button is displayed, since the discover link component has its own tests

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to add a comment to this effect in the code?

css={{ margin: '0 0.3em' }}
/>
);
if (asButton) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This component is mostly renamed from render_discover_link.tsx with a new option of rendering the link as a button (previously it was only an icon).

expect(component.exists('[data-test-subj="discoverButtonLink"]')).toBe(false);
});

it('renders the link as a button if the prop is set', async () => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are mostly from the renamed file render_discover_link.test.ts with an addition of new tests for rendering the link as a button (not only as an icon)

@@ -1,29 +0,0 @@
/*
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random unused code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, angular 😅 Glad to see this cleaned up!

*/
import SemVer from 'semver/classes/semver';

import { MAJOR_VERSION } from '../../../common';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the check for kibanaVersion.major < 8 since these changes won't be backported

indexStatusByName,
indices: getIndicesByName(state, indexNames),
isSystemIndexByName: getIsSystemIndexByName(indexNames, allIndices),
hasSystemIndex: hasSystemIndex(indexNames, allIndices),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed system indices that are not needed in version 8.x

showStats,
showSettings,
detailPanel,
isOnListView,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed the prop to be more "descriptive": when the context menu is displayed in the indices list, it has more options

// @ts-ignore this component needs to be refactored into TS
import { IndexActionsContextMenu } from './index_actions_context_menu';

export interface ReduxProps {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This component is a wrapper to initialize the context menu with index actions without using redux. For now all functions are empty and they will be implemented in a follow up PR.

import { RouteDependencies } from '../../../types';
import { addInternalBasePath } from '..';

export function registerGetRoute({
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This route is not hidden behind the feature flag, since I believe this is not affecting end users. But let me know what you think.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I think this is OK as well

@yuliacech yuliacech added Feature:Index Management Index and index templates UI Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes v8.11.0 labels Aug 22, 2023
@yuliacech yuliacech marked this pull request as ready for review August 22, 2023 19:12
@yuliacech yuliacech requested a review from a team as a code owner August 22, 2023 19:12
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

Copy link
Copy Markdown
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @yuliacech! It's really great to see some of the old code starting to get cleaned up 🎉 Verified changes locally. I left a few comments, but nothing blocking.

iconType="discoverApp"
aria-label="Discover"
data-test-subj="discoverIconLink"
css={{ margin: '0 0.3em' }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to add the custom CSS here? I see now that this existed previously. I'll leave it up to you if it makes sense to revisit or not.

@@ -1,29 +0,0 @@
/*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, angular 😅 Glad to see this cleaned up!

import { FormattedMessage } from '@kbn/i18n-react';
import { KibanaPageTemplate } from '@kbn/shared-ux-page-kibana-template';

export const DetailsPageLoading = () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to use the section_loading component from es_ui_shared?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it was still using "deprecated" eui page still but I see now it's actually already migrated. I'll switch to the re-usable one and delete the extra component: the less code, the better :)

import { RouteDependencies } from '../../../types';
import { addInternalBasePath } from '..';

export function registerGetRoute({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I think this is OK as well

export default function ({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
const es = getService('es');
const esDeleteAllIndices = getService('esDeleteAllIndices');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL 😄

};

const testIndex = 'test_index';
describe('index details', async () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add these tests for serverless as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see that you added the serverless tests for indices "fetch all" route already (in the stats PR)! That makes it much easier for me, I'll add my tests there as well

expect(testBed.routerMock.history.push).toHaveBeenCalledWith('/indices');
});

it('renders a link to discover', () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to add a comment to this effect in the code?

<EuiPageTemplate.EmptyPrompt
data-test-subj="indexDetailsErrorLoadingDetails"
color="danger"
iconType="error"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're using the warning icon in a lot of our other plugins for error states; I wonder if we should change that 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! I'll change to warning for consistency

@yuliacech
Copy link
Copy Markdown
Contributor Author

Thanks a lot for the review, @alisonelizabeth!

@kibana-ci
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #61 / Cases - group 1 Create case "before each" hook for "creates a case from the stack management page"
  • [job] [logs] FTR Configs #58 / saved objects security and spaces enabled _bulk_create user with all at other space within the default space "before all" hook for "should return 403 forbidden [isolatedtype/defaultspace-isolatedtype-id]"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
indexManagement 495 498 +3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
indexManagement 525.2KB 523.6KB -1.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
indexManagement 32.7KB 32.6KB -21.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@yuliacech yuliacech merged commit 79178ca into elastic:main Aug 23, 2023
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Aug 23, 2023
yuliacech added a commit that referenced this pull request Aug 28, 2023
## Summary

Addresses #164546

Follow up to #163521 and
#163955

This PR re-implements index actions in the context menu on the index
details page. The actions are implemented without redux which is used in
the old index details flyout (to be removed when this work is complete)
and in the indices list. The PR introduces a declaration file to list
all props of the component `IndexActionsContextMenu` written in JS.
There is also a new component `ManageIndexButton` that implements index
actions specifically to be executed on the new index details page. In
the future most of the code in the component `ManageIndexButton` can be
re-used when more refactorings will be made (switching to TS and not
using redux in the indices list).

All index actions are async and I added a loading indicator to the
context menu button to indicate that requests are in flight updating the
index.

### Screen recordings


https://github.com/elastic/kibana/assets/6585477/c39f1450-b495-4c50-b4ca-8989a2259ed5

Add/remove ILM policy actions with a confirmation modal



https://github.com/elastic/kibana/assets/6585477/964931c9-b926-4ed4-aa5c-218f52881131





### How to test
1. Add `xpack.index_management.dev.enableIndexDetailsPage: true` to your
`/config/kibana.dev.yml` file
7. Start ES and Kibana with `yarn es snapshot` and `yarn start`
8. Add several indices to test with the command `PUT /test_index` in Dev
Tools Console
9. Navigate to Index Management and click the name of any index
10. Check index actions: 

- [x] Close index
- [x] Open index
- [x] Force merge index
- [x] Refresh index
- [x] Clear index cache
- [x] Flush index
- [ ] Unfreeze index (not sure how to add a frozen index)
- [x] Delete index
- [x] ILM: add lifecycle policy
- [x] ILM: remove lifecycle policy
- [x] ILM: retry lifecycle policy (add any built-in policy and wait a
couple of minutes until the rollover fails)

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Ikuni17 pushed a commit that referenced this pull request Sep 27, 2023
## Summary
This PR removes the feature flag and enables the new index details page
by default. The index details page was implemented in following PRs:
- #163521
- #163955
- #164741
- #165027
- #165038
- #165456

In this PR we completely remove now obsolete code for the old index
details flyout: react components and corresponding redux code. All
related tests are updated and cleaned up. The config value for Index
Management plugin `xpack.index_management.dev.enableIndexDetailsPage` is
deprecated as unused and can be removed in v9.0.


### How to test
1. Start ES and Kibana with `yarn es snapshot` and `yarn start`
3. Navigate to Index Management and create an index
4. Click the index name in the table and check the tabs of the details
page
### Screenshots
#### Stateful
Overview 
<img width="1387" alt="Screenshot 2023-09-27 at 14 41 57"
src="https://github.com/elastic/kibana/assets/6585477/e58b15e7-d10c-4473-873c-d0f128392404">


Mappings
<img width="1392" alt="Screenshot 2023-09-27 at 14 42 05"
src="https://github.com/elastic/kibana/assets/6585477/441157cb-5a26-47c3-8da0-b4df51ebec5d">


Settings 
<img width="1385" alt="Screenshot 2023-09-27 at 14 42 13"
src="https://github.com/elastic/kibana/assets/6585477/da66a2eb-1f21-44c1-9356-484c66caab88">


Statistics
<img width="1380" alt="Screenshot 2023-09-27 at 14 42 22"
src="https://github.com/elastic/kibana/assets/6585477/ec93d85c-e754-4c21-88ab-0124dc114fc9">


Error loading data
<img width="1333" alt="Screenshot 2023-09-26 at 19 05 37"
src="https://github.com/elastic/kibana/assets/6585477/fc1804b3-6aa0-4019-bae6-e7bb40113b28">
<img width="1327" alt="Screenshot 2023-09-26 at 19 06 07"
src="https://github.com/elastic/kibana/assets/6585477/ca711697-cc74-4ba8-b17c-ec9b01f3026e">
<img width="1329" alt="Screenshot 2023-09-26 at 19 06 28"
src="https://github.com/elastic/kibana/assets/6585477/0cb46b09-8542-452a-8845-40d060057e95">
<img width="1331" alt="Screenshot 2023-09-26 at 19 06 48"
src="https://github.com/elastic/kibana/assets/6585477/87de8d3d-b6e5-4e8f-b27c-18a1c6e950d8">


Error saving index settings
<img width="1332" alt="Screenshot 2023-09-26 at 19 07 31"
src="https://github.com/elastic/kibana/assets/6585477/e6e4b3d0-c237-4d0a-995a-4562bc78f88e">


### Serverless
Overview 
<img width="1336" alt="Screenshot 2023-09-26 at 19 51 47"
src="https://github.com/elastic/kibana/assets/6585477/6c76c23b-4be6-4ab3-ae1d-c7ae751e100d">


Mappings
<img width="1336" alt="Screenshot 2023-09-26 at 19 23 51"
src="https://github.com/elastic/kibana/assets/6585477/625fa703-506f-4389-9df0-86441a655074">


Settings
<img width="1332" alt="Screenshot 2023-09-26 at 19 24 02"
src="https://github.com/elastic/kibana/assets/6585477/c496ab09-f2db-4c1b-9fb6-1e9b64b1c142">


# Release note
Index details can now be viewed on a new index details page in Index
Management.
<img width="1387" alt="Screenshot 2023-09-27 at 14 41 57"
src="https://github.com/elastic/kibana/assets/6585477/b90c706d-8b15-49e4-8f6a-cb66f3ed1822">

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
@yuliacech yuliacech deleted the im/details_page/api_index_details branch February 15, 2024 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Index Management Index and index templates UI release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v8.11.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants