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

use individual route for each KP app #65977

Merged
merged 3 commits into from
May 13, 2020

Conversation

pgayvallet
Copy link
Contributor

@pgayvallet pgayvallet commented May 11, 2020

Summary

Fix #59190

  • Create a <Route> for each KP app instead of relying on the /app/appId catch-all route to remove the distinction between apps having a custom appRoute and the others.
  • Fix the bug causing apps with custom appRoute not being properly handled when kibana was using an empty basePath and when the custom route was prefixed by /app/{id}/

This will also ease legacy cleanup as we should be able to just remove the catch-all handler and use a 'dumb' default 404 route/component instead.

Checklist

@pgayvallet pgayvallet added this to Pending Review in kibana-core [DEPRECATED] via automation May 11, 2020
@pgayvallet pgayvallet moved this from Pending Review to In Progress in kibana-core [DEPRECATED] May 11, 2020
@pgayvallet pgayvallet added Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels May 11, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@pgayvallet pgayvallet added v7.9.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels May 11, 2020
@pgayvallet pgayvallet marked this pull request as ready for review May 11, 2020 12:37
@pgayvallet pgayvallet requested a review from a team as a code owner May 11, 2020 12:37
@pgayvallet pgayvallet moved this from In Progress to Pending Review in kibana-core [DEPRECATED] May 11, 2020
@pgayvallet pgayvallet merged commit 2117ef1 into elastic:master May 13, 2020
kibana-core [DEPRECATED] automation moved this from Pending Review to Done (7.9) May 13, 2020
pgayvallet added a commit to pgayvallet/kibana that referenced this pull request May 13, 2020
* use individual route for each KP app

* cleanup
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

pgayvallet added a commit that referenced this pull request May 13, 2020
* use individual route for each KP app

* cleanup
pgayvallet added a commit to pgayvallet/kibana that referenced this pull request Jun 3, 2020
* use individual route for each KP app

* cleanup
pgayvallet added a commit to pgayvallet/kibana that referenced this pull request Jun 3, 2020
* use individual route for each KP app

* cleanup
# Conflicts:
#	src/core/public/application/ui/app_container.test.tsx
#	src/core/public/application/ui/app_router.tsx
pgayvallet added a commit that referenced this pull request Jun 3, 2020
* use individual route for each KP app

* cleanup
pgayvallet added a commit that referenced this pull request Jun 3, 2020
* use individual route for each KP app

* cleanup
# Conflicts:
#	src/core/public/application/ui/app_container.test.tsx
#	src/core/public/application/ui/app_router.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.9.0 v8.0.0
Projects
Development

Successfully merging this pull request may close these issues.

Plugins cannot register appRoute: /app/{name}
4 participants