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

Update cluster status partial to component #11680

Merged
merged 5 commits into from
May 27, 2021
Merged

Update cluster status partial to component #11680

merged 5 commits into from
May 27, 2021

Conversation

arnav28
Copy link
Contributor

@arnav28 arnav28 commented May 20, 2021

No description provided.

@arnav28 arnav28 added the ui label May 20, 2021
@vercel vercel bot temporarily deployed to Preview – vault May 20, 2021 22:53 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook May 20, 2021 22:57 Inactive
@tracked
fakeRenew = false;

get isRenewing() {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

{{#if (and activeCluster.unsealed auth.currentToken)}}
{{#unless cluster.dr.isSecondary}}
{{#unless this.version.isOSS}}
{{#if (and @cluster.unsealed this.auth.currentToken)}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on looking at the code, I'm not sure activeCluster as defined on (old) status-menu.js is exactly swappable with currentCluster.cluster which is what's being passed into the component here. My guess is that with the full model (activeCluster) we get the replication info and we may not on the cluster service version, but we'll need to investigate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added back the active cluster for now.
In meantime, I will dig further and if required get in touch with Matthew. We can clean up later if required.

Copy link
Contributor

@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

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

One other check, in case you haven't already, is to run yarn run test -f="enterprise" locally. When we push a branch to OSS it skips all the enterprise test. This area seems to touch some enterprise feature so it's good practice to run those test just in case.

Added back activeCluster
Updated changelog
@arnav28 arnav28 merged commit 55ad812 into master May 27, 2021
@arnav28 arnav28 deleted the ui/status-partials branch May 27, 2021 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants