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

[Index Management] Fix encoding issue on index details page #166882

Merged
merged 18 commits into from
Sep 22, 2023

Conversation

yuliacech
Copy link
Contributor

@yuliacech yuliacech commented Sep 20, 2023

Summary

Fixes #166100

This PR adds a workaround fix for the new index details page when opening for index names with special characters, for example test_index%. Because of how encoding/decoding works, we can't use the index name as a part of the url like /indices/${indexName} (see for more details). Instead we have to pass the index name in a query parameter like `/indices/index_details?indexName=${indexName}. The downside of this workaround is that the url semantics doesn't reflect that the index name is mandatory for the page to work. Once #132600 is resolved, we should revert this workaround and use the index name as a url segment again.

Note for reviewers: The jest tests for this fix are part of #165705

How to test

  1. Add xpack.index_management.dev.enableIndexDetailsPage: true to the file config/kibana.dev.yml to enable the new index details page
  2. Navigate to Index Management and use the "create index" button
  3. Type a name with special characters, for example test%
  4. Click the new index name in the list and check that the details page and all tabs work
  5. Also reload the page completely and check that the page still loads correctly

…y param instead of a path parameter for index name to avoid issues with encoded characters
@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 release_note:skip Skip the PR/issue when compiling release notes v8.11.0 labels Sep 20, 2023
* 2.0.
*/

export enum Section {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to extract these constants to a separate file in common folder so that it could be imported without increasing the bundle size.

@yuliacech yuliacech marked this pull request as ready for review September 21, 2023 18:51
@yuliacech yuliacech requested a review from a team as a code owner September 21, 2023 18:52
@elasticmachine
Copy link
Contributor

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

Copy link
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.

Great work! I'm happy we were able to find a temporary workaround for this 🎉

The downside of this workaround is that the url semantics doesn't reflect that the index name is mandatory for the page to work.

Does it make sense to add a check that the index was specified as a query param before making the request to fetch the index? We might also consider rendering a slightly different error message in that scenario. Here's the current behavior:

Screenshot 2023-09-21 at 8 10 50 PM

…me/index_list/details_page/details_page.tsx

Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>
@yuliacech
Copy link
Contributor Author

Thank you for the review, @alisonelizabeth! Great point about the empty index name, definitely adding a better error message for this case and will also check if I can prevent the unnecessary request as well.

@yuliacech
Copy link
Contributor Author

I added a message when no index name is provided and stopped an unnecessary http request (d3591e6).
Screenshot 2023-09-22 at 13 11 57

@yuliacech yuliacech enabled auto-merge (squash) September 22, 2023 11:14
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #41 / serverless observability UI Observability Log Explorer Dataset Selector with installed integrations and uncategorized data streams when open on the integrations tab should display a list of installed integrations

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
indexManagement 605 606 +1

Async chunks

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

id before after diff
indexManagement 601.6KB 601.3KB -216.0B

Page load bundle

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

id before after diff
indexManagement 33.1KB 33.6KB +532.0B

History

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

@yuliacech yuliacech merged commit 213ef56 into elastic:main Sep 22, 2023
20 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 22, 2023
joemcelroy pushed a commit to joemcelroy/kibana that referenced this pull request Sep 25, 2023
…166882)

## Summary
Fixes elastic#166100

This PR adds a workaround fix for the new index details page when
opening for index names with special characters, for example
`test_index%`. Because of how encoding/decoding works, we can't use the
index name as a part of the url like `/indices/${indexName}` (see for
more details). Instead we have to pass the index name in a query
parameter like `/indices/index_details?indexName=${indexName}. The
downside of this workaround is that the url semantics doesn't reflect
that the index name is mandatory for the page to work. Once
elastic#132600 is resolved, we should
revert this workaround and use the index name as a url segment again.

Note for reviewers: The jest tests for this fix are part of
elastic#165705

### How to test
1. Add `xpack.index_management.dev.enableIndexDetailsPage: true` to the
file `config/kibana.dev.yml` to enable the new index details page
2. Navigate to Index Management and use the "create index" button 
3. Type a name with special characters, for example `test%`
4. Click the new index name in the list and check that the details page
and all tabs work
5. Also reload the page completely and check that the page still loads
correctly

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>
@yuliacech yuliacech deleted the im/details_page/fix_encoding 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 commit 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 v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Index Management] Index details page doesn't work for indices with special characters
5 participants