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

Wire in new status.condition property #13273

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

richard-cox
Copy link
Member

@richard-cox richard-cox commented Feb 3, 2025

  • blocker on steve bump to rancher/rancher (to get fix for BE filter bug)

Summary

Fixes #13268

Occurred changes and/or fixed issues

  • as per title, now that the v1 mgmt cluster status.connected property is supported wire it in

Areas or cases that should be tested

  • side nav cluster list should order clusters in
    • local cluster
    • running
    • name

Areas which could experience regressions

  • side nav

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

@richard-cox richard-cox added the ci/skip-e2e Forcibly skip E2E tests in the CI label Feb 3, 2025
@richard-cox richard-cox added this to the v2.11.0 milestone Feb 3, 2025
@richard-cox richard-cox self-assigned this Feb 3, 2025
@richard-cox richard-cox force-pushed the pagination-side-nav-sort branch from b33330d to 16284f6 Compare February 5, 2025 11:21
@richard-cox richard-cox removed the ci/skip-e2e Forcibly skip E2E tests in the CI label Feb 5, 2025
@richard-cox richard-cox force-pushed the pagination-side-nav-sort branch from 16284f6 to 9fcb8c3 Compare February 5, 2025 11:24
@richard-cox richard-cox marked this pull request as ready for review February 5, 2025 11:28
@richard-cox
Copy link
Member Author

Linking rancher/rancher#48993

// },
{
asc: false,
field: 'status.connected'
Copy link
Contributor

Choose a reason for hiding this comment

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

The title of this pr seems to think we're supposed to be using status.condition

@@ -37,7 +37,7 @@ type ProvCluster = {
}

/**
* Order
* Order of v1 mgmt clusters
* 1. local cluster - https://github.com/rancher/dashboard/issues/10975
* 2. working clusters
* 3. name
Copy link
Contributor

@codyrancher codyrancher Feb 5, 2025

Choose a reason for hiding this comment

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

The description of this pr also mentions something like:

  1. local cluster
  2. working clusters
  3. name

But judging from the issue I think 3. name is supposed to read 3. everything else

Copy link
Contributor

@codyrancher codyrancher left a comment

Choose a reason for hiding this comment

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

I don't actually see a difference between what I see in master vs what I see in this branch. Both seem to have the same order. But I don't see any harm in what's going on either so feel free to merge.

image

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.

Update side bar cluster list sort to work with new status.connected property
2 participants