Skip to content
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

[ILM] TS conversion of policies table #76006

Merged
merged 9 commits into from
Sep 2, 2020

Conversation

yuliacech
Copy link
Contributor

@yuliacech yuliacech commented Aug 26, 2020

Summary

This PR converts Policy table to TypeScript as an ongoing effort of converting the ILM plugin #74271. A more verbose loading indicator was added and a button to retry loading policies after failure.

Screenshot

loading_policies_table

@yuliacech
Copy link
Contributor Author

@elasticmachine merge upstream

@yuliacech yuliacech marked this pull request as ready for review August 27, 2020 15:05
@yuliacech yuliacech requested a review from a team as a code owner August 27, 2020 15:05
@yuliacech yuliacech added Feature:ILM Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.10.0 v8.0.0 labels Aug 27, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@yuliacech yuliacech added the release_note:skip Skip the PR/issue when compiling release notes label Aug 27, 2020
@yuliacech
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

@yuliacech this is looking really great! Happy to see us able to remove dependence on redux store! I did a local smoke test and the plugin seems to be working as expected!

Before approving I just wanted to get your input on the selected policies state that seems to not be in use in policy_table.tsx.

Otherwise looks GTG!

initUiMetric({ reportUiStats: () => {} });
// This expects HttpSetup but we're giving it AxiosInstance.
// @ts-ignore
initHttp(axios.create({ adapter: axiosXhrAdapter }));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not pass in HttpSetup here? We should be able to import and create a mock from src/core/public/mocks - similar to what we have for usageCollectionPluginMock :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason I couldn't get the test to work with coreMock but then I saw HttpService being used in some other tests, so I tried it and it worked.

@yuliacech
Copy link
Contributor Author

Hi @jloleysens , thanks a lot for your review! I deleted unused selectedPolicies state in policy_table.tsx. Could you have another look please?

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
indexLifecycleManagement 72 -87 159

async chunks size

id value diff baseline
indexLifecycleManagement 185.3KB -90.6KB 276.0KB

page load bundle size

id value diff baseline
indexLifecycleManagement 230.3KB +108.0B 230.2KB

distributable file count

id value diff baseline
total 44370 -3 44373

History

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

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Great work @yuliacech ! Thanks for addressing my feedback.

Did not retest locally, but the latest commits look good to me!

@yuliacech yuliacech merged commit 6686cff into elastic:master Sep 2, 2020
yuliacech added a commit to yuliacech/kibana that referenced this pull request Sep 2, 2020
* [ILM] TS conversion of policies table

* [ILM] Fix type check errors

* [ILM] Fix i18n check errors

* [ILM] Add types to linkedIndices

* [ILM] Add PR review fixes

* [ILM] Add PR review fixes (filter fn)

* [ILM] Fix i18n errors

Co-authored-by: Elastic Machine <[email protected]>
yuliacech added a commit that referenced this pull request Sep 2, 2020
* [ILM] TS conversion of policies table

* [ILM] Fix type check errors

* [ILM] Fix i18n check errors

* [ILM] Add types to linkedIndices

* [ILM] Add PR review fixes

* [ILM] Add PR review fixes (filter fn)

* [ILM] Fix i18n errors

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
@yuliacech yuliacech deleted the ilm_policies_table branch September 3, 2020 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ILM 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 v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants