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

Refactor UA Overview to support step-completion #111243

Merged
merged 20 commits into from
Sep 8, 2021
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
7f9becd
Refactor UA Overview to store step-completion state at the root and d…
cjcenizal Sep 6, 2021
554bcdc
[Upgrade Assistant] Support completion state on overview page (#26)
sabarasaba Sep 7, 2021
642e540
Merge branch '7.x' into ua/7.x/step-completion-state
cjcenizal Sep 7, 2021
66b72a6
Fix typo in test descriptions.
cjcenizal Sep 7, 2021
a979937
Simplify Fix Deprecation Issues step state management.
cjcenizal Sep 7, 2021
25d0491
Rename DeprecationsCountCheckpoint setHasNoDeprecationLogs prop and t…
cjcenizal Sep 7, 2021
27d9f08
Add logic to set Back Up Step to incomplete when it receives an error…
cjcenizal Sep 7, 2021
768cd1a
Merge branch '7.x' into ua/7.x/step-completion-state
cjcenizal Sep 7, 2021
22436ca
Don't export const flushPromiseJobQueue.
cjcenizal Sep 7, 2021
810c03f
Rename OverviewStepsProp to OverviewStepProps.
cjcenizal Sep 7, 2021
0a14a6c
Fix merge and TS errors.
cjcenizal Sep 7, 2021
3340c42
Merge branch '7.x' into ua/7.x/step-completion-state
kibanamachine Sep 8, 2021
0e2dc7f
Merge branch '7.x' into ua/7.x/step-completion-state
kibanamachine Sep 8, 2021
9be3802
Fix typos in test descriptions.
cjcenizal Sep 8, 2021
7ccb6ab
Memoize callbacks and define exhaustive deps for hooks.
cjcenizal Sep 8, 2021
5b4f0d5
Remove unused prop from FixIssuesStep and refine props for FixLogsStep.
cjcenizal Sep 8, 2021
6e3bbea
Add comment explaining that fixed means 0 critical issues.
cjcenizal Sep 8, 2021
b5b082b
Merge branch '7.x' into ua/7.x/step-completion-state
cjcenizal Sep 8, 2021
f40b250
Define exhaustive deps.
cjcenizal Sep 8, 2021
416aa3b
Revert use of exhaustive deps to avoid infinite loop.
cjcenizal Sep 8, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@
*/

export { setupEnvironment, WithAppDependencies } from './setup_environment';
export { advanceTime } from './time_manipulation';
export { kibanaDeprecationsServiceHelpers } from './kibana_deprecations_service.mock';
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { act } from 'react-dom/test-utils';

/**
* These helpers are intended to be used in conjunction with jest.useFakeTimers().
*/

const flushPromiseJobQueue = async () => {
// See https://stackoverflow.com/questions/52177631/jest-timer-and-promise-dont-work-well-settimeout-and-async-function
await Promise.resolve();
};

export const advanceTime = async (ms: number) => {
await act(async () => {
jest.advanceTimersByTime(ms);
await flushPromiseJobQueue();
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 think the flakiness in #111255 was due to not flushing this promise queue.

});
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
* 2.0.
*/

import { act } from 'react-dom/test-utils';

import { setupEnvironment } from '../../helpers';
import { CLOUD_BACKUP_STATUS_POLL_INTERVAL_MS } from '../../../../common/constants';
import { setupEnvironment, advanceTime } from '../../helpers';
import { OverviewTestBed, setupOverviewPage } from '../overview.helpers';

describe('Overview - Backup Step', () => {
Expand All @@ -34,6 +33,11 @@ describe('Overview - Backup Step', () => {
expect(exists('snapshotRestoreLink')).toBe(true);
expect(find('snapshotRestoreLink').props().href).toBe('snapshotAndRestoreUrl');
});

test('renders step as incomplete ', () => {
const { exists } = testBed;
expect(exists('backupStep-incomplete')).toBe(true);
});
});

describe('On Cloud', () => {
Expand Down Expand Up @@ -104,6 +108,11 @@ describe('Overview - Backup Step', () => {
expect(exists('cloudSnapshotsLink')).toBe(true);
expect(find('dataBackedUpStatus').text()).toContain('Last snapshot created on');
});

test('renders step as complete ', () => {
const { exists } = testBed;
expect(exists('backupStep-complete')).toBe(true);
});
});

describe(`when data isn't backed up`, () => {
Expand All @@ -121,24 +130,47 @@ describe('Overview - Backup Step', () => {
expect(exists('dataNotBackedUpStatus')).toBe(true);
expect(exists('cloudSnapshotsLink')).toBe(true);
});

test('renders step as incomplete ', () => {
const { exists } = testBed;
expect(exists('backupStep-incomplete')).toBe(true);
});
});
});

// FLAKY: https://github.com/elastic/kibana/issues/111255
test.skip('polls for new status', async () => {
// The behavior we're testing involves state changes over time, so we need finer control over
// timing.
describe('poll for new status', () => {
beforeEach(async () => {
jest.useFakeTimers();
testBed = await setupCloudOverviewPage();
expect(server.requests.length).toBe(4);

// Resolve the polling timeout.
await act(async () => {
jest.runAllTimers();
// First request will succeed.
httpRequestsMockHelpers.setLoadCloudBackupStatusResponse({
isBackedUp: true,
lastBackupTime: '2021-08-25T19:59:59.863Z',
});

expect(server.requests.length).toBe(5);
testBed = await setupCloudOverviewPage();
});

afterEach(() => {
jest.useRealTimers();
});

test('renders step as incomplete when a success state is followed by an error state', async () => {
const { exists } = testBed;
expect(exists('backupStep-complete')).toBe(true);

// Second request will error.
httpRequestsMockHelpers.setLoadCloudBackupStatusResponse(undefined, {
statusCode: 400,
message: 'error',
});

// Resolve the polling timeout.
await advanceTime(CLOUD_BACKUP_STATUS_POLL_INTERVAL_MS);
testBed.component.update();

expect(exists('backupStep-incomplete')).toBe(true);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,37 @@ describe('Overview - Fix deprecation issues step', () => {
server.restore();
});

describe('Step status', () => {
test(`It's complete when there are no critical deprecations`, async () => {
httpRequestsMockHelpers.setLoadEsDeprecationsResponse(esDeprecationsEmpty);

await act(async () => {
const deprecationService = deprecationsServiceMock.createStartContract();
deprecationService.getAllDeprecations = jest.fn().mockRejectedValue([]);

testBed = await setupOverviewPage({
services: {
core: {
deprecations: deprecationService,
},
},
});
});

const { exists, component } = testBed;

component.update();

expect(exists(`fixIssuesStep-complete`)).toBe(true);
});

test('Its incomplete when there are critical deprecations', async () => {
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
test('Its incomplete when there are critical deprecations', async () => {
test(`It's incomplete when there are critical deprecations`, async () => {

const { exists } = testBed;

expect(exists(`fixIssuesStep-incomplete`)).toBe(true);
});
});

describe('ES deprecations', () => {
test('Shows deprecation warning and critical counts', () => {
const { exists, find } = testBed;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,49 @@ describe('Overview - Fix deprecation logs step', () => {
server.restore();
});

describe('Step status', () => {
sabarasaba marked this conversation as resolved.
Show resolved Hide resolved
test(`It's complete when there are no deprecation logs since last checkpoint`, async () => {
httpRequestsMockHelpers.setUpdateDeprecationLoggingResponse(getLoggingResponse(true));

httpRequestsMockHelpers.setLoadDeprecationLogsCountResponse({
count: 0,
});

await act(async () => {
testBed = await setupOverviewPage();
});

const { exists, component } = testBed;

component.update();

expect(exists(`fixLogsStep-complete`)).toBe(true);
});

test('Its incomplete when there are deprecation logs since last checkpoint', async () => {
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
test('Its incomplete when there are deprecation logs since last checkpoint', async () => {
test(`It's incomplete when there are deprecation logs since last checkpoint`, async () => {

httpRequestsMockHelpers.setUpdateDeprecationLoggingResponse(getLoggingResponse(true));

httpRequestsMockHelpers.setLoadDeprecationLogsCountResponse({
count: 5,
});

await act(async () => {
testBed = await setupOverviewPage();
});

const { exists, component } = testBed;

component.update();

expect(exists(`fixLogsStep-incomplete`)).toBe(true);
});
});

describe('Step 1 - Toggle log writing and collecting', () => {
test('toggles deprecation logging', async () => {
const { find, actions } = testBed;

httpRequestsMockHelpers.setUpdateDeprecationLoggingResponse({
isDeprecationLogIndexingEnabled: false,
isDeprecationLoggingEnabled: false,
});
httpRequestsMockHelpers.setUpdateDeprecationLoggingResponse(getLoggingResponse(false));

expect(find('deprecationLoggingToggle').props()['aria-checked']).toBe(true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,33 +11,34 @@ import type { EuiStepProps } from '@elastic/eui/src/components/steps/step';

import type { CloudSetup } from '../../../../../../cloud/public';
import { OnPremBackup } from './on_prem_backup';
import { CloudBackup, CloudBackupStatusResponse } from './cloud_backup';
import { CloudBackup } from './cloud_backup';
import type { OverviewStepProps } from '../../types';

const title = i18n.translate('xpack.upgradeAssistant.overview.backupStepTitle', {
defaultMessage: 'Back up your data',
});

interface Props {
interface Props extends OverviewStepProps {
cloud?: CloudSetup;
cloudBackupStatusResponse?: CloudBackupStatusResponse;
}

export const getBackupStep = ({ cloud, cloudBackupStatusResponse }: Props): EuiStepProps => {
export const getBackupStep = ({ cloud, isComplete, setIsComplete }: Props): EuiStepProps => {
const status = isComplete ? 'complete' : 'incomplete';

if (cloud?.isCloudEnabled) {
return {
status,
title,
status: cloudBackupStatusResponse!.data?.isBackedUp ? 'complete' : 'incomplete',
'data-test-subj': `backupStep-${status}`,
children: (
<CloudBackup
cloudBackupStatusResponse={cloudBackupStatusResponse!}
cloudSnapshotsUrl={cloud!.snapshotsUrl!}
/>
<CloudBackup cloudSnapshotsUrl={cloud!.snapshotsUrl!} setIsComplete={setIsComplete} />
),
};
}

return {
title,
'data-test-subj': 'backupStep-incomplete',
status: 'incomplete',
children: <OnPremBackup />,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React from 'react';
import React, { useEffect } from 'react';
import moment from 'moment-timezone';
import { FormattedDate, FormattedTime, FormattedMessage } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';
Expand All @@ -20,22 +20,39 @@ import {
EuiCallOut,
} from '@elastic/eui';

import { CloudBackupStatus } from '../../../../../common/types';
import { UseRequestResponse } from '../../../../shared_imports';
import { ResponseError } from '../../../lib/api';

export type CloudBackupStatusResponse = UseRequestResponse<CloudBackupStatus, ResponseError>;
import { useAppContext } from '../../../app_context';

interface Props {
cloudBackupStatusResponse: UseRequestResponse<CloudBackupStatus, ResponseError>;
cloudSnapshotsUrl: string;
setIsComplete: (isComplete: boolean) => void;
}

export const CloudBackup: React.FunctionComponent<Props> = ({
cloudBackupStatusResponse,
cloudSnapshotsUrl,
setIsComplete,
}) => {
const { isInitialRequest, isLoading, error, data, resendRequest } = cloudBackupStatusResponse;
const {
services: { api },
} = useAppContext();

const {
isInitialRequest,
isLoading,
error,
data,
resendRequest,
} = api.useLoadCloudBackupStatus();

// Tell overview whether the step is complete or not.
useEffect(() => {
// Loading shouldn't invalidate the previous state.
if (!isLoading) {
// An error should invalidate the previous state.
setIsComplete((!error && data?.isBackedUp) ?? false);
}

// eslint-disable-next-line react-hooks/exhaustive-deps
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why this is needed?

}, [error, isLoading, data]);

if (isInitialRequest && isLoading) {
return <EuiLoadingContent data-test-subj="cloudBackupLoading" lines={3} />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React, { FunctionComponent } from 'react';
import React, { FunctionComponent, useEffect, useMemo } from 'react';
import { useHistory } from 'react-router-dom';

import {
Expand Down Expand Up @@ -60,18 +60,34 @@ const i18nTexts = {
}),
};

export const ESDeprecationStats: FunctionComponent = () => {
interface Props {
setIsFixed: (isFixed: boolean) => void;
}

export const ESDeprecationStats: FunctionComponent<Props> = ({ setIsFixed }) => {
const history = useHistory();
const {
services: { api },
} = useAppContext();

const { data: esDeprecations, isLoading, error } = api.useLoadEsDeprecations();

const warningDeprecations =
esDeprecations?.deprecations?.filter((deprecation) => deprecation.isCritical === false) || [];
const criticalDeprecations =
esDeprecations?.deprecations?.filter((deprecation) => deprecation.isCritical) || [];
const warningDeprecations = useMemo(
() =>
esDeprecations?.deprecations?.filter((deprecation) => deprecation.isCritical === false) || [],
[esDeprecations]
);
const criticalDeprecations = useMemo(
() => esDeprecations?.deprecations?.filter((deprecation) => deprecation.isCritical) || [],
[esDeprecations]
);

useEffect(() => {
if (!isLoading && !error) {
setIsFixed(criticalDeprecations.length === 0);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still needed?

}, [criticalDeprecations, isLoading, error]);

const hasWarnings = warningDeprecations.length > 0;
const hasCritical = criticalDeprecations.length > 0;
Expand Down
Loading