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

cluster-ui: derive app name from route parameter in cluster-ui #70999

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

xinhaoz
Copy link
Member

@xinhaoz xinhaoz commented Oct 1, 2021

Fixes: #70998

Release justification: category 2

Previously, we were deriving the selected app name from the
query string parameter in for the statements page in the
cluster-ui package. The selected app name should be derived from
the route parameter for the statements page.

Release note (bug fix): the selected app name in the statements page
is now derived from the route parameters.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@xinhaoz xinhaoz requested a review from a team October 1, 2021 16:58
@xinhaoz xinhaoz force-pushed the fix-app-filtering branch from 26621c8 to a6f99c0 Compare October 1, 2021 16:59
@Azhng
Copy link
Contributor

Azhng commented Oct 1, 2021

Wait I thought we switched to using query params from route params for app name?

@xinhaoz
Copy link
Member Author

xinhaoz commented Oct 1, 2021

@Azhng We did for the statement details page, this is for the statements page. I think we should really just use a query parameter here too, though.

@xinhaoz xinhaoz force-pushed the fix-app-filtering branch from a6f99c0 to 3dd506d Compare October 1, 2021 20:55
@maryliag
Copy link
Contributor

maryliag commented Oct 1, 2021

When I select the demo app, it shows the correct value on the details page, but if I select internal, it's showing some extra query on the details pag
Screen Shot 2021-10-01 at 4 58 24 PM
e

Fixes: cockroachdb#70998

Release justification: category 2

Previously, we were deriving the selected app name from the
query string parameter in for the statements page in the
cluster-ui package. The selected app name should be derived from
the route parameter for the statements page.

Release note (bug fix): the selected app name in the statements page
is now derived from the route parameters.
@xinhaoz xinhaoz force-pushed the fix-app-filtering branch from 3dd506d to cb31d8d Compare October 1, 2021 21:02
Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@xinhaoz
Copy link
Member Author

xinhaoz commented Oct 1, 2021

bors r=maryliag

@craig
Copy link
Contributor

craig bot commented Oct 1, 2021

Build succeeded:

@craig craig bot merged commit 3426e8d into cockroachdb:master Oct 1, 2021
@xinhaoz xinhaoz deleted the fix-app-filtering branch January 26, 2022 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cluster-ui: fix app filtering and selection
4 participants