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

ui: add license change notification to db console #129420

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

dhartunian
Copy link
Collaborator

@dhartunian dhartunian commented Aug 21, 2024

This change adds a dismissable alert to the Overview page of DB
Console that informs users about upcoming license changes.

This popup is only shown if the cluster does not have an active
"Enterprise" license

The popup links to this page:
"https://www.cockroachlabs.com/enterprise-license-update/"

When the popup is dismissed, the dismissal is stored in the DB for
this user and they don't see this notification again.

Resolves: CRDB-40939

Release note (ui change): DB Console will show a notification alerting
customers without an Enterprise license, to upcoming license changes
with a link to more information.

@dhartunian dhartunian requested a review from a team as a code owner August 21, 2024 17:54
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@dhartunian dhartunian added backport-23.1.x Flags PRs that need to be backported to 23.1 backport-23.2.x Flags PRs that need to be backported to 23.2. backport-24.1.x Flags PRs that need to be backported to 24.1. backport-24.2.x Flags PRs that need to be backported to 24.2 labels Aug 21, 2024
@dhartunian dhartunian force-pushed the core-deprecation-notification branch from 780c20b to bf8c22f Compare August 21, 2024 17:55

const licenseUpdateDismissedPersistentLoadedSelector = createSelector(
(state: AdminUIState) => state.uiData,
uiData => uiData && has(uiData, LICENSE_UPDATE_DISMISSED_KEY),
Copy link
Member

Choose a reason for hiding this comment

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

nit: any reason we can't use native hasOwn or hasOwnProperty here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

Comment on lines 713 to 715
(uiData &&
uiData[LICENSE_UPDATE_DISMISSED_KEY] &&
uiData[LICENSE_UPDATE_DISMISSED_KEY].data &&
Copy link
Member

Choose a reason for hiding this comment

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

nit: i think this can be collapsed a little:

    return moment(uiData?.[LICENSE_UPDATE_DISMISSED_KEY]?.data ?? 0);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ooh nice. thanks!

Comment on lines 750 to 751
licenseUpdateDismissedPersistentLoaded &&
licenseUpdateDismissedPersistent ) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little confused by this case because based on the selector right above this one, doesn't licenseUpdateDismissedPersistent always return a moment as long as there is uiData loaded? Why don't we need to do the isAfter(moment(0)) check here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you are correct. added.

@dhartunian dhartunian force-pushed the core-deprecation-notification branch 2 times, most recently from a8d5aa8 to eb6ce6b Compare September 9, 2024 21:07
@dhartunian
Copy link
Collaborator Author

Notification example:

Screenshot 2024-09-09 at 17 13 44

Copy link
Member

@xinhaoz xinhaoz left a comment

Choose a reason for hiding this comment

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

:lgtm: !

This change adds a dismissable alert to the Overview page of DB
Console that informs users about upcoming license changes.

This popup is only shown if the cluster does not have an active
"Enterprise" license

The popup links to this page:
"https://www.cockroachlabs.com/enterprise-license-update/"

When the popup is dismissed, the dismissal is stored in the DB for
this user and they don't see this notification again.

Resolves: CRDB-40939

Release note (ui change): DB Console will show a notification alerting
customers without an Enterprise license, to upcoming license changes
with a link to more information.
@dhartunian
Copy link
Collaborator Author

TFTR! I added some tests too 😄

bors r=xinhaoz

@dhartunian dhartunian added the backport-23.1.26-rc FROZEN: requires ER request to thaw label Sep 9, 2024
@craig craig bot merged commit fd00996 into cockroachdb:master Sep 9, 2024
23 checks passed
Copy link

blathers-crl bot commented Sep 9, 2024

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from a597c17 to blathers/backport-release-23.1-129420: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.1.x failed. See errors above.


error creating merge commit from a597c17 to blathers/backport-release-23.2-129420: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.2.x failed. See errors above.


error creating merge commit from a597c17 to blathers/backport-release-24.1-129420: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 24.1.x failed. See errors above.


error creating merge commit from a597c17 to blathers/backport-release-24.2-129420: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 24.2.x failed. See errors above.


error creating merge commit from a597c17 to blathers/backport-release-23.1.26-rc-129420: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.1.26-rc failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-23.1.x Flags PRs that need to be backported to 23.1 backport-23.1.26-rc FROZEN: requires ER request to thaw backport-23.2.x Flags PRs that need to be backported to 23.2. backport-24.1.x Flags PRs that need to be backported to 24.1. backport-24.2.x Flags PRs that need to be backported to 24.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants