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

[Stateful sidenav] Don't fetch active space on unauthenticated routes #190408

Merged
merged 2 commits into from
Aug 13, 2024

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented Aug 13, 2024

This PR fixes a bug where the navigation plugin tries to fetch the active space on unauthenticated routes (e.g. the login page). This created noise in the log and could mis lead users that something was wrong.

For context, the navigation plugin fetches the active space on the client to decide which side navigation to render and on the server to decide which default route to redirect for the space home page.

Fixes #190135

@sebelga sebelga self-assigned this Aug 13, 2024
@sebelga sebelga added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Feature:Chrome Core's Chrome UI (sidenav, header, breadcrumbs) labels Aug 13, 2024
@sebelga
Copy link
Contributor Author

sebelga commented Aug 13, 2024

/ci

@sebelga sebelga marked this pull request as ready for review August 13, 2024 14:13
@sebelga sebelga requested a review from a team as a code owner August 13, 2024 14:13
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@sebelga
Copy link
Contributor Author

sebelga commented Aug 13, 2024

/ci

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

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

id before after diff
navigation 16.8KB 17.1KB +242.0B

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

cc @sebelga

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

LGTM. The tests look great!

@sebelga
Copy link
Contributor Author

sebelga commented Aug 13, 2024

Thanks for the quick review @tsullivan ! 👍

@sebelga sebelga merged commit 8dee365 into elastic:main Aug 13, 2024
29 checks passed
@kibanamachine kibanamachine added v8.16.0 backport:skip This commit does not require backporting labels Aug 13, 2024
@sebelga sebelga added v8.15.1 backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) and removed backport:skip This commit does not require backporting labels Aug 13, 2024
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.15 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 190408

Questions ?

Please refer to the Backport tool documentation

@sebelga
Copy link
Contributor Author

sebelga commented Aug 14, 2024

💚 All backports created successfully

Status Branch Result
8.15

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

sebelga added a commit to sebelga/kibana that referenced this pull request Aug 14, 2024
…elastic#190408)

(cherry picked from commit 8dee365)

# Conflicts:
#	src/plugins/navigation/public/plugin.test.ts
#	src/plugins/navigation/public/plugin.tsx
#	src/plugins/navigation/server/ui_settings.test.ts
#	src/plugins/navigation/server/ui_settings.ts
@sebelga sebelga deleted the stateful-sidenav/fetch-active-space-fix branch August 14, 2024 09:31
sebelga added a commit that referenced this pull request Aug 14, 2024
… routes (#190408) (#190487)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[Stateful sidenav] Don't fetch active space on unauthenticated routes
(#190408)](#190408)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Sébastien
Loix","email":"sebastien.loix@elastic.co"},"sourceCommit":{"committedDate":"2024-08-13T16:00:08Z","message":"[Stateful
sidenav] Don't fetch active space on unauthenticated routes
(#190408)","sha":"8dee365bda57a324d18fbd9405a965f74f1c047f","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:SharedUX","backport:prev-minor","Feature:Chrome","v8.16.0","v8.15.1"],"number":190408,"url":"https://github.com/elastic/kibana/pull/190408","mergeCommit":{"message":"[Stateful
sidenav] Don't fetch active space on unauthenticated routes
(#190408)","sha":"8dee365bda57a324d18fbd9405a965f74f1c047f"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/190408","number":190408,"mergeCommit":{"message":"[Stateful
sidenav] Don't fetch active space on unauthenticated routes
(#190408)","sha":"8dee365bda57a324d18fbd9405a965f74f1c047f"}},{"branch":"8.15","label":"v8.15.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) bug Fixes for quality problems that affect the customer experience Feature:Chrome Core's Chrome UI (sidenav, header, breadcrumbs) release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.15.1 v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Security exception in logs when visiting anonymous page (ex: login)
5 participants