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

change: [UIE-7308] - Notifications for database resize events #10262

Conversation

mpolotsk-akamai
Copy link
Contributor

@mpolotsk-akamai mpolotsk-akamai commented Mar 6, 2024

Description πŸ“

Needs to be tested on DEV/ALPHA

Bell icon / Alert changes:
System notify users when database resizing starts, display a progress bar which disappears upon completion, update status from 'Active' to 'Resizing', show resizing progress in percentage, and revert to 'Active' status when finished.
Option to switch between dedicated and shared plans, with the exception of switching from dedicated plan to a shared plan of the same size.

Changes πŸ”„

  • Notification / Event of resizing Start
  • Notification progress bar in Bell icon Alert
  • Notification complete – bar disappears
  • Shared to dedicated and back plan change
  • Status Changes from Active to Resizing
  • Rendering progress % numbers in UI
  • Changing back to Active when done

Target release date πŸ—“οΈ

Release date 3/18/24

Preview πŸ“·

How to test πŸ§ͺ

Needs to be tested in Alpha.
Open application, navigate to /databases, select the database cluster. Go to the Resize tab, choose a plan, and click the 'Resize Database Cluster' button. In the popup window, confirm the resizing by entering the name of the database cluster, then click 'Resize Cluster' button. You can monitor the resizing status in the Summary and Resize tabs, as well as in a table on the Databases page. Additionally, notification about scheduled resizing and a progress bar which disappears upon completion in bell icon dropdown.

@mpolotsk-akamai mpolotsk-akamai requested a review from a team as a code owner March 6, 2024 16:19
@mpolotsk-akamai mpolotsk-akamai requested review from hana-linode and abailly-akamai and removed request for a team March 6, 2024 16:19
@carrillo-erik
Copy link
Contributor

carrillo-erik commented Mar 6, 2024

Could you please share any insight with the following..

When an action is successful we denote the toast notifications with green color.
Screenshot 2024-03-06 at 9 08 55 AM

Here is an example for volume being resized.
Screenshot 2024-03-06 at 9 12 18 AM

Also, there is a discrepancy with the copy of the toast notification. Normally, only notifications for account or profile actions will start the message with Your ...

@bnussman-akamai bnussman-akamai added the DBaaS Relates to Database as a Service label Mar 7, 2024
@mpolotsk-akamai
Copy link
Contributor Author

mpolotsk-akamai commented Mar 7, 2024

Could you please share any insight with the following..

When an action is successful we denote the toast notifications with green color. Screenshot 2024-03-06 at 9 08 55 AM

Here is an example for volume being resized. Screenshot 2024-03-06 at 9 12 18 AM

Also, there is a discrepancy with the copy of the toast notification. Normally, only notifications for account or profile actions will start the message with Your ...

We have a blue notification (info) indicating that the database cluster will be resized, similar to the behavior of Linode Resize.
A green notification (success) means, for example, that the database was successfully resized, indication successful completion, while red notification indicates that the action was not successful.

Regarding the text in the notification I will update it to "Database cluster {cluster name} is being resized."

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

Thanks for the solid PR!

a few things overall:

  • make sure to use strict equality
  • You have unit test failures (run yarn test or look at GH action report
  • this is lacking coverage (unit test in this PR, and is there any plan to add e2e coverage?)
  • You need to rebase this PR to get the latest changes from develop
  • Make sure to add screenshots about what I am expected to see in the UI in your PR description
    Am not seeing the progress bar. Is it only in alpha? If so, please add that the PR needs to be tested with alpha
    Screenshot 2024-03-07 at 09 37 01

@mpolotsk-akamai
Copy link
Contributor Author

@abailly-akamai thank you for your review! I'll make the necessary changes.

@mpolotsk-akamai mpolotsk-akamai force-pushed the feature/UIE-7308-cloud-manager-changes-for-DBAAS branch from c7fcb73 to b6f35f6 Compare March 8, 2024 14:53
Copy link

github-actions bot commented Mar 11, 2024

Coverage Report: ❌
Base Coverage: 81.45%
Current Coverage: 81.42%

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

Thanks @mpolotsk-akamai the PR is looking good.

  • Please add to the description that this needs to be tested on DEV/ALPHA (I imagine this is not going live for a while and still behind a feature flag?
  • Left some code clean up comments

Was able to confirm the events and the progress bar however, I am getting an error in the console about an unknown API event received, which seems to be directly related to the resize event. Can you look into it?

Screenshot 2024-03-13 at 09 46 11

sizeTooSmall: boolean,
planIsDisabled?: boolean,
disabledToolTip?: string
): string | undefined => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: above 2 arguments, use named args

@mpolotsk-akamai
Copy link
Contributor Author

Thanks @mpolotsk-akamai the PR is looking good.

  • Please add to the description that this needs to be tested on DEV/ALPHA (I imagine this is not going live for a while and still behind a feature flag?
  • Left some code clean up comments

Was able to confirm the events and the progress bar however, I am getting an error in the console about an unknown API event received, which seems to be directly related to the resize event. Can you look into it?

Screenshot 2024-03-13 at 09 46 11

@abailly-akamai, thank you for the review!

I encountered the same issue. It occurs when we have old events in the dropdown menu. Sam Klock provided an explanation:
"If you're referring to an event for a job that occurred before we enabled the new event types in Alpha, then this behavior is expected; the new types aren't applied retroactively."

One of the ways to resolve this is by keeping the copy defined for "database_scale" in the code, as there will be users who have event records for it in production. So, I will add database_scale to eventMessageGenerator.

@hana-linode hana-linode changed the title change: [UIE-7308] - notification for resizing database change: [UIE-7308] - Notifications for database resize events Mar 14, 2024
Copy link
Contributor

@hana-linode hana-linode left a comment

Choose a reason for hiding this comment

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

Can we extend the width of the loading bar to match the container?

image

example from the MSW
image

Generally, settings are positioned as the last tab content instead of the middle. Can we move the Resize tab next to Backups instead of at the end?
image

examples
image
image

@hana-linode hana-linode changed the base branch from develop to release-v1.115.0 March 18, 2024 16:11
@hana-linode hana-linode changed the base branch from release-v1.115.0 to develop March 18, 2024 16:27
@hana-linode
Copy link
Contributor

@mpolotsk-akamai looks like there are merge conflicts that need to be addressed

…hich disappears after resize complete, update status from Active to Resizing, show resizing progress in percentage, and revert to Active status when finished. Option to switch between dedicated and shared plans, with the exception of switching from dedicated plan to a shared plan of the same size.
@mpolotsk-akamai mpolotsk-akamai force-pushed the feature/UIE-7308-cloud-manager-changes-for-DBAAS branch from c135d39 to 7b980d7 Compare March 18, 2024 20:29
@mpolotsk-akamai
Copy link
Contributor Author

@hana-linode , merge conflicts have been resolved

@hana-linode hana-linode added the Approved Multiple approvals and ready to merge! label Mar 19, 2024
@carrillo-erik
Copy link
Contributor

Thanks for making the changes.

@hana-linode hana-linode merged commit 874584c into linode:develop Mar 21, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! DBaaS Relates to Database as a Service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants