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.

Sign up for GitHub

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

Jump to bottom

[Ingest Pipeline] Allow to provide a redirect path and a tab in component template edit UI #134910

Merged
merged 12 commits into from
Jun 29, 2022
Merged

[Ingest Pipeline] Allow to provide a redirect path and a tab in component template edit UI #134910

merged 12 commits into from
Jun 29, 2022

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Jun 22, 2022 •
edited
Loading

Description

Related to #133740

Similar to #134776

We need to be able to link the component template edit UI from Fleet on a specific tab and to be redirect to Fleet when the user save.

For this I introduced two query params ?tab= and ?redirect_path to select a specific tab and to redirect on save/close.

How to test

While running Kibana without basepath
Go to /app/management/data/index_management/edit_component_template/.alerts-observability.logs.alerts-mappings?tab=mappings&redirect_path=/app/fleet

You should go directly to the mappings tab and on save be redirected to Fleet

Screen Shot 2022-06-22 at 9 43 39 AM

This should work from the view component template to /app/management/data/index_management/component_templates/.alerts-ecs-mappings?redirect_path=/app/fleet

@nchaulet nchaulet added 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.4.0 labels Jun 22, 2022
@nchaulet nchaulet requested a review from alisonelizabeth June 22, 2022 13:44
@nchaulet nchaulet requested a review from a team as a code owner June 22, 2022 13:44
@nchaulet nchaulet self-assigned this Jun 22, 2022
@elasticmachine
Copy link
Contributor

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

@nchaulet nchaulet mentioned this pull request Jun 22, 2022
Closed
18 tasks
@nchaulet nchaulet force-pushed the feature-allow-query-redirect branch from e1c2540 to c9b29e9 Compare June 22, 2022 15:34
@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@nchaulet nchaulet mentioned this pull request Jun 23, 2022
Merged
@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

alisonelizabeth
alisonelizabeth reviewed Jun 28, 2022
View reviewed changes
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.

Hi @nchaulet! Nice work. I left a few code comments, mostly around naming.

I tested the query params on edit and everything worked as expected.

This should work from the view component template to /app/management/data/index_management/component_templates/.alerts-ecs-mappings?redirect_path=/app/fleet

I'm not sure what the expected behavior here is. Can you explain?

Also, if the query param is set, I wonder if it should dynamically update when the user changes steps. For example, if mappings is set, but then I got to the next step, "Aliases", the mappings query param remains in the URL. WDYT?

Screen Shot 2022-06-28 at 8 42 01 AM

x-pack/plugins/index_management/public/application/mount_management_section.ts Outdated Show resolved Hide resolved
x-pack/plugins/index_management/public/application/hooks/redirect_path.test.tsx Outdated Show resolved Hide resolved
...nent_templates/component_template_wizard/component_template_form/component_template_form.tsx Outdated Show resolved Hide resolved
...nent_templates/component_template_wizard/component_template_edit/component_template_edit.tsx Outdated Show resolved Hide resolved
...templates/component_template_wizard/component_template_edit/component_template_edit.test.tsx Outdated Show resolved Hide resolved
@nchaulet
Copy link
Member Author

@alisonelizabeth Thanks for the review to be able to update the step on step change I had to do a change to the FormWizzard component let me know if it make sense

@nchaulet nchaulet requested a review from alisonelizabeth June 28, 2022 15:48
@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@alisonelizabeth
Copy link
Contributor

Jest test failure due to #135440

@alisonelizabeth
Copy link
Contributor

@elasticmachine merge upstream

alisonelizabeth
alisonelizabeth approved these changes Jun 29, 2022
View reviewed changes
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.

Latest LGTM. Thanks for addressing my feedback! Found one minor typo in the test, but won't block on that.

I'm also not sure about this comment. If there's anything else I need to test related to this, please let me know and I can take another look if you'd like.

This should work from the view component template to /app/management/data/index_management/component_templates/.alerts-ecs-mappings?redirect_path=/app/fleet

...templates/component_template_wizard/component_template_edit/component_template_edit.test.tsx Outdated Show resolved Hide resolved
@nchaulet @alisonelizabeth
Update x-pack/plugins/index_management/public/application/components/…
b39a173 
…component_templates/component_template_wizard/component_template_edit/component_template_edit.test.tsx

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

nchaulet commented Jun 29, 2022 •
edited
Loading

@alisonelizabeth for

This should work from the view component template to /app/management/data/index_management/component_templates/.alerts-ecs-mappings?redirect_path=/app/fleet

What I was meaning here if you visit that page and click cancel you will be redirected to redirect_path, I guess no need to manually test that again

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
indexManagement 480 481 +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 515.6KB 516.4KB +846.0B

Page load bundle

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

id before after diff
esUiShared 130.3KB 130.4KB +133.0B

History

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

cc @nchaulet

@nchaulet nchaulet merged commit 27c04bd into elastic:main Jun 29, 2022
@nchaulet nchaulet deleted the feature-allow-query-redirect branch June 29, 2022 19:49
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jun 29, 2022
yakhinvadim pushed a commit to yakhinvadim/kibana that referenced this pull request Jul 5, 2022
@nchaulet
[Ingest Pipeline] Allow to provide a redirect path and a tab in compo…
4eefd5d 
…nent template edit UI (elastic#134910)
@nchaulet nchaulet mentioned this pull request Jan 24, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alisonelizabeth alisonelizabeth alisonelizabeth approved these changes

Assignees

@nchaulet nchaulet

Labels
backport:skip This commit does not require backporting 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.4.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nchaulet @elasticmachine @alisonelizabeth @kibana-ci @kibanamachine