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

fix: [UIE-8194] dbaas maintenance pending updates state should display when any pending updates are available #11387

Merged
merged 1 commit into from
Dec 12, 2024
Merged

fix: [UIE-8194] dbaas maintenance pending updates state should display when any pending updates are available #11387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-11387-fixed-1733763957768.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Fixed
---

dbaas settings maintenance does not display review state and allows version upgrade when updates are available ([#11387](https://github.com/linode/manager/pull/11387))
54 changes: 50 additions & 4 deletions ...c/features/Databases/DatabaseDetail/DatabaseSettings/DatabaseSettingsMaintenance.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,19 @@ describe('Database Settings Maintenance', () => {
expect(button).toBeDisabled();
});

it('should enable upgrade version modal button when there are upgrades available', async () => {
it('should disable upgrade version modal button when there are upgrades available, but there are still updates available', async () => {
const database = databaseFactory.build({
engine: 'postgresql',
version: '13',
updates: {
pending: [
{
deadline: null,
description: 'Log configuration options changes required',
planned_for: null,
},
],
},
});

const onReviewUpdates = vi.fn();
Expand All @@ -94,11 +103,17 @@ describe('Database Settings Maintenance', () => {

const button = await findByRole('button', { name: UPGRADE_VERSION });

expect(button).toBeEnabled();
expect(button).toBeDisabled();
});

it('should show review text and modal button when there are updates', async () => {
const database = databaseFactory.build();
it('should enable upgrade version modal button when there are upgrades available, and there are no pending updates', async () => {
const database = databaseFactory.build({
engine: 'postgresql',
version: '13',
updates: {
pending: [],
},
});

const onReviewUpdates = vi.fn();
const onUpgradeVersion = vi.fn();
Expand All @@ -118,6 +133,37 @@ describe('Database Settings Maintenance', () => {
expect(button).toBeEnabled();
});

it('should show review text and modal button when there are updates ', async () => {
const database = databaseFactory.build({
updates: {
pending: [
{
deadline: null,
description: 'Log configuration options changes required',
planned_for: null,
},
],
},
});

const onReviewUpdates = vi.fn();
const onUpgradeVersion = vi.fn();

const { queryByRole } = renderWithTheme(
<DatabaseSettingsMaintenance
databaseEngine={database.engine}
databasePendingUpdates={database.updates.pending}
databaseVersion={database.version}
onReviewUpdates={onReviewUpdates}
onUpgradeVersion={onUpgradeVersion}
/>
);

const button = queryByRole('button', { name: 'Click to review' });

expect(button).toBeInTheDocument();
});

it('should not show review text and modal button when there are no updates', async () => {
const database = databaseFactory.build({
updates: {
Expand Down
17 changes: 15 additions & 2 deletions ...er/src/features/Databases/DatabaseDetail/DatabaseSettings/DatabaseSettingsMaintenance.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { StyledLinkButton, Typography } from '@linode/ui';
import { StyledLinkButton, TooltipIcon, Typography } from '@linode/ui';
import { Grid, styled } from '@mui/material';
import * as React from 'react';

Expand Down Expand Up @@ -40,11 +40,24 @@ export const DatabaseSettingsMaintenance = (props: Props) => {
<StyledTypography>{engineVersion}</StyledTypography>
<StyledLinkButton
data-testid="upgrade"
disabled={!versions?.length}
disabled={!versions?.length || hasUpdates}
onClick={onUpgradeVersion}
>
Upgrade Version
</StyledLinkButton>
{hasUpdates && (
<TooltipIcon
Copy link
Contributor Author

@smans-akamai smans-akamai Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@cpathipa The update was to have a Tooltip icon display when there are pending updates disabling the Upgrade Version button. Just pushed up another change to adjust the condition and I've updated the description.

sxTooltipIcon={{
padding: '0px 8px',
}}
text={
<Typography>
Upgrades are disabled while maintenance updates are in progress.
</Typography>
}
status="help"
/>
)}
</Grid>
{/*
TODO Uncomment and provide value when the EOL is returned by the API.
Expand Down
34 changes: 33 additions & 1 deletion packages/manager/src/features/Databases/utilities.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from 'src/factories';
import {
getDatabasesDescription,
hasPendingUpdates,
isDateOutsideBackup,
isDefaultDatabase,
isLegacyDatabase,
Expand All @@ -19,7 +20,12 @@ import {
import { HttpResponse, http, server } from 'src/mocks/testServer';
import { wrapWithTheme } from 'src/utilities/testHelpers';

import type { AccountCapability, Database, Engine } from '@linode/api-v4';
import type {
AccountCapability,
Database,
Engine,
PendingUpdates,
} from '@linode/api-v4';
import type { TimeOption } from 'src/features/Databases/DatabaseDetail/DatabaseBackups/DatabaseBackups';

const setup = (capabilities: AccountCapability[], flags: any) => {
Expand Down Expand Up @@ -419,6 +425,32 @@ describe('getDatabasesDescription', () => {
});
});

describe('hasPendingUpdates', () => {
it('should return false when there are no pending updates provided', () => {
expect(hasPendingUpdates()).toBe(false);
});

it('should return false when pendingUpdates param is undefined', () => {
expect(hasPendingUpdates(undefined)).toBe(false);
});

it('should return false when pending updates is an empty array', () => {
const updates: PendingUpdates[] = [];
expect(hasPendingUpdates(updates)).toBe(false);
});

it('should return true when there are pending updates', () => {
const updates: PendingUpdates[] = [
{
deadline: null,
description: 'Log configuration options changes required',
planned_for: null,
},
];
expect(hasPendingUpdates(updates)).toBe(true);
});
});

describe('isDefaultDatabase', () => {
it('should return true for default platform database', () => {
const db: Database = databaseFactory.build({
Expand Down
4 changes: 1 addition & 3 deletions packages/manager/src/features/Databases/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,7 @@ export const getDatabasesDescription = (
};

export const hasPendingUpdates = (pendingUpdates?: PendingUpdates[]) =>
Boolean(
pendingUpdates?.some((update) => update.deadline || update.planned_for)
);
Boolean(pendingUpdates && pendingUpdates?.length > 0);
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
Boolean(pendingUpdates && pendingUpdates?.length > 0);
Boolean(pendingUpdates?.length > 0);

Copy link
Contributor Author

@smans-akamai smans-akamai Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@abailly-akamai The pendingUpdates param is optional and can be undefined. So this change throws a "possibly undefined" typescript error without the added check. I believe the function was intentionally written this way so I'd prefer to leave it the as is.

Also, for the components using this method, the property that gets provided for pendingUpdates is optional. Perhaps because the data might not be available right away before making this check.

Since this just a fix for the existing behavior, can we leave it as is?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure thing, I just assumed the optional chaining was taking care of that 👍


export const isDefaultDatabase = (
database: Pick<DatabaseInstance, 'platform'>
Expand Down
Loading