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.

Sign up for GitHub

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

Jump to bottom

feat: [UIE-8166] - DBaaS backup GA toggle and beta fixes #11048

Merged
merged 1 commit into from
Oct 8, 2024
Merged

feat: [UIE-8166] - DBaaS backup GA toggle and beta fixes #11048

merged 1 commit into from
Oct 8, 2024

Conversation

corya-akamai
Copy link
Contributor

Description 📝

Enhancements and fixes to the DBaaS backups tab

Changes 🔄

List any change relevant to the reviewer.

  • Add radio option for GA user to specify specify date time or "latest"
  • Add types to restore API call so we can read the returned database id and redirect to the summary tab f the new "forked" database
  • Refactor dbaas utilities to use isFeatureEnabledV2
  • Refactor DatabaseBackups to separate V1 and V2 features. All V1 features now under /legacy dir.
  • Disable date + times that are outside the backup window.

Target release date 🗓️

10/14

Preview 📷

backups-beta-disable-past

backups-beta-disable-future

ga-backup-newest

ga-backup-dateTime

How to test 🧪

Prerequisites

  • Managed Databases account capability
  • dbaasV2.beta set to false and dbaasV2.enabled set to true

Reproduction steps

  • Navigate to backups tab of a newly created V2 database

Verification steps

  • Radio button appears to selected a specific date or "latest"
  • Once you restore you will land on the summary tab of the "forked" db.

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@corya-akamai corya-akamai requested a review from a team as a code owner October 4, 2024 21:19
@corya-akamai corya-akamai requested review from carrillo-erik and coliu-akamai and removed request for a team October 4, 2024 21:19
@corya-akamai corya-akamai force-pushed the UIE-8166-dbaas-backups-time-selection branch from b6a7e50 to d8992c7 Compare October 4, 2024 21:21
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 4, 2024 •
edited
Loading

Coverage Report: ❌
Base Coverage: 86.95%
Current Coverage: 86.94%

@cpathipa cpathipa requested review from cpathipa and removed request for carrillo-erik October 7, 2024 13:27
@cpathipa cpathipa added the DBaaS Relates to Database as a Service label Oct 7, 2024
cpathipa
cpathipa requested changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@cpathipa cpathipa left a comment

Choose a reason for hiding this comment

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

Overall, it looks good. I have left feedback for properly handling database undefined scenarios and extracting the table body rendering logic into its own component.

...nager/src/features/Databases/DatabaseDetail/DatabaseBackups/legacy/DatabaseBackupsLegacy.tsx Outdated Show resolved Hide resolved
...nager/src/features/Databases/DatabaseDetail/DatabaseBackups/legacy/DatabaseBackupsLegacy.tsx Outdated Show resolved Hide resolved
...nager/src/features/Databases/DatabaseDetail/DatabaseBackups/legacy/DatabaseBackupsLegacy.tsx Outdated Show resolved Hide resolved
packages/manager/src/features/Databases/utilities.test.ts Outdated Show resolved Hide resolved
@corya-akamai corya-akamai force-pushed the UIE-8166-dbaas-backups-time-selection branch from d8992c7 to ede73a4 Compare October 8, 2024 14:52
cpathipa
cpathipa approved these changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@cpathipa cpathipa left a comment

Choose a reason for hiding this comment

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

@corya-akamai Thank you for addressing all the feedback.

@cpathipa cpathipa added the Add'tl Approval Needed Waiting on another approval! label Oct 8, 2024
coliu-akamai
coliu-akamai approved these changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

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

✅ confirmed new radio button options
✅ confirmed restoring lands on summary tab of new forked db

Thanks for the additional test coverage! 🎉

packages/api-v4/.changeset/pr-11048-changed-1728076842800.md
"@linode/api-v4": Changed
---

Specify the fork restore payload and return types ([#11048](https://github.com/linode/manager/pull/11048))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Specify the fork restore payload and return types ([#11048](https://github.com/linode/manager/pull/11048))
Specify DBaaS fork restore payload and return types ([#11048](https://github.com/linode/manager/pull/11048))

...ages/manager/src/features/Databases/DatabaseDetail/DatabaseBackups/DatabaseBackupsDialog.tsx
@@ -89,4 +91,4 @@ export const RestoreNewFromBackupDialog = (props: Props) => {
);
};

export default RestoreNewFromBackupDialog;
export default DatabaseBackupDialog;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we remove the default export (same with DatabaseBackupTableBody and DatabaseBackupsLegacy) if able to? We've been moving away from them in favor of named exports where possible

@coliu-akamai coliu-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Oct 8, 2024
@cpathipa cpathipa merged commit f51b555 into linode:develop Oct 8, 2024
20 checks passed
@cypress Cypress
Copy link

cypress bot commented Oct 8, 2024

Cloud Manager E2E    Run #6640

Run Properties:  status check passed Passed #6640  •  git commit f51b5553b7: fix: [UIE-8166] - DBaaS disabled invalid times based on oldest backup and select...
Project Cloud Manager E2E
Run status status check passed Passed #6640
Run duration 26m 41s
Commit git commit f51b5553b7: fix: [UIE-8166] - DBaaS disabled invalid times based on oldest backup and select...
Committer corya-akamai
View all properties for this run ↗︎
Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 5
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 429

hasyed-akamai pushed a commit to hasyed-akamai/manager that referenced this pull request Oct 9, 2024
@corya-akamai @hasyed-akamai
fix: [UIE-8166] - DBaaS disabled invalid times based on oldest backup…
89fa5ef 
… and selected date (linode#11048)
This was referenced Oct 10, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpathipa cpathipa cpathipa approved these changes

@coliu-akamai coliu-akamai coliu-akamai approved these changes

Assignees

@corya-akamai corya-akamai

Labels
Approved Multiple approvals and ready to merge! DBaaS Relates to Database as a Service
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@corya-akamai @cpathipa @coliu-akamai