Skip to content

Commit 1c3f0fc

Browse files
[Snapshot & Restore] fix pre existing policy with no existing repository (#76861)
* implement fix and add callout with copy * added test * make callout a danger callout and revise copy * block the form if we have a repo, but it does not exist * added test to assert that form wizard blocks on validation for not found repo * fix types and add a doc comment * move callout to above the form Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent 60294cb commit 1c3f0fc

File tree

5 files changed

+103
-4
lines changed

5 files changed

+103
-4
lines changed

x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/policy_form.helpers.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,6 @@ export type PolicyFormTestSubjects =
5656
| 'showAdvancedCronLink'
5757
| 'snapshotNameInput'
5858
| 'dataStreamBadge'
59+
| 'repositoryNotFoundWarning'
60+
| 'repositorySelect'
5961
| 'submitButton';

x-pack/plugins/snapshot_restore/__jest__/client_integration/policy_add.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ describe('<PolicyAdd />', () => {
6767
expect(find('nextButton').props().disabled).toBe(true);
6868
});
6969

70+
test('should not show repository-not-found warning', () => {
71+
const { exists } = testBed;
72+
expect(exists('repositoryNotFoundWarning')).toBe(false);
73+
});
74+
7075
describe('form validation', () => {
7176
describe('logistics (step 1)', () => {
7277
test('should require a policy name', async () => {

x-pack/plugins/snapshot_restore/__jest__/client_integration/policy_edit.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,44 @@ describe('<PolicyEdit />', () => {
5252
expect(find('pageTitle').text()).toEqual('Edit policy');
5353
});
5454

55+
describe('policy with pre-existing repository that was deleted', () => {
56+
beforeEach(async () => {
57+
httpRequestsMockHelpers.setGetPolicyResponse({ policy: POLICY_EDIT });
58+
httpRequestsMockHelpers.setLoadIndicesResponse({
59+
indices: ['my_index'],
60+
dataStreams: ['my_data_stream'],
61+
});
62+
httpRequestsMockHelpers.setLoadRepositoriesResponse({
63+
repositories: [{ name: 'this-is-a-new-repository' }],
64+
});
65+
66+
testBed = await setup();
67+
68+
await act(async () => {
69+
await nextTick();
70+
testBed.component.update();
71+
});
72+
});
73+
74+
test('should show repository-not-found warning', () => {
75+
const { exists, find } = testBed;
76+
expect(exists('repositoryNotFoundWarning')).toBe(true);
77+
// The select should be an empty string to allow users to select a new repository
78+
expect(find('repositorySelect').props().value).toBe('');
79+
});
80+
81+
describe('validation', () => {
82+
test('should block navigating to next step', () => {
83+
const { exists, find, actions } = testBed;
84+
actions.clickNextButton();
85+
// Assert that we are still on the repository configuration step
86+
expect(exists('repositoryNotFoundWarning')).toBe(true);
87+
// The select should be an empty string to allow users to select a new repository
88+
expect(find('repositorySelect').props().value).toBe('');
89+
});
90+
});
91+
});
92+
5593
/**
5694
* As the "edit" policy component uses the same form underneath that
5795
* the "create" policy, we won't test it again but simply make sure that

x-pack/plugins/snapshot_restore/public/application/components/policy_form/steps/step_logistics.tsx

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import {
1818
EuiLink,
1919
EuiSpacer,
2020
EuiText,
21+
EuiCallOut,
22+
EuiCode,
2123
} from '@elastic/eui';
2224

2325
import { Repository } from '../../../../../common/types';
@@ -54,6 +56,10 @@ export const PolicyStepLogistics: React.FunctionComponent<StepProps> = ({
5456

5557
const { i18n, history } = useServices();
5658

59+
const [showRepositoryNotFoundWarning, setShowRepositoryNotFoundWarning] = useState<boolean>(
60+
false
61+
);
62+
5763
// State for touched inputs
5864
const [touched, setTouched] = useState({
5965
name: false,
@@ -256,13 +262,26 @@ export const PolicyStepLogistics: React.FunctionComponent<StepProps> = ({
256262
}
257263
}
258264

265+
const doesRepositoryExist =
266+
!!policy.repository &&
267+
repositories.some((r: { name: string }) => r.name === policy.repository);
268+
269+
if (!doesRepositoryExist && !errors.repository) {
270+
updatePolicy(policy, { repositoryDoesNotExist: true });
271+
}
272+
273+
if (showRepositoryNotFoundWarning !== !doesRepositoryExist) {
274+
setShowRepositoryNotFoundWarning(!doesRepositoryExist);
275+
}
276+
259277
return (
260278
<EuiSelect
261279
options={repositories.map(({ name }: Repository) => ({
262280
value: name,
263281
text: name,
264282
}))}
265-
value={policy.repository || repositories[0].name}
283+
hasNoInitialSelection={!doesRepositoryExist}
284+
value={!doesRepositoryExist ? '' : policy.repository}
266285
onBlur={() => setTouched({ ...touched, repository: true })}
267286
onChange={(e) => {
268287
updatePolicy(
@@ -541,8 +560,31 @@ export const PolicyStepLogistics: React.FunctionComponent<StepProps> = ({
541560
</EuiButtonEmpty>
542561
</EuiFlexItem>
543562
</EuiFlexGroup>
544-
<EuiSpacer size="l" />
545563

564+
{showRepositoryNotFoundWarning && (
565+
<>
566+
<EuiSpacer size="m" />
567+
<EuiCallOut
568+
data-test-subj="repositoryNotFoundWarning"
569+
title={
570+
<FormattedMessage
571+
id="xpack.snapshotRestore.policyForm.stepLogistics.selectRepository.policyRepositoryNotFoundTitle"
572+
defaultMessage="Repository not found"
573+
/>
574+
}
575+
color="danger"
576+
iconType="alert"
577+
>
578+
<FormattedMessage
579+
id="xpack.snapshotRestore.policyForm.stepLogistics.selectRepository.policyRepositoryNotFoundDescription"
580+
defaultMessage="Repository {repo} does not exist. Please select an existing repository."
581+
values={{ repo: <EuiCode>{policy.repository}</EuiCode> }}
582+
/>
583+
</EuiCallOut>
584+
</>
585+
)}
586+
587+
<EuiSpacer size="l" />
546588
{renderNameField()}
547589
{renderSnapshotNameField()}
548590
{renderRepositoryField()}

x-pack/plugins/snapshot_restore/public/application/services/validation/validate_policy.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ export interface ValidatePolicyHelperData {
4141
* are not configuring this value - like when they are on a previous step.
4242
*/
4343
validateIndicesCount?: boolean;
44+
45+
/**
46+
* A policy might be configured with a repository that no longer exists. We want the form to
47+
* block in this case because just having a repository configured is not enough for validity.
48+
*/
49+
repositoryDoesNotExist?: boolean;
4450
}
4551

4652
export const validatePolicy = (
@@ -50,7 +56,13 @@ export const validatePolicy = (
5056
const i18n = textService.i18n;
5157

5258
const { name, snapshotName, schedule, repository, config, retention } = policy;
53-
const { managedRepository, isEditing, policyName, validateIndicesCount } = validationHelperData;
59+
const {
60+
managedRepository,
61+
isEditing,
62+
policyName,
63+
validateIndicesCount,
64+
repositoryDoesNotExist,
65+
} = validationHelperData;
5466

5567
const validation: PolicyValidation = {
5668
isValid: true,
@@ -99,7 +111,7 @@ export const validatePolicy = (
99111
);
100112
}
101113

102-
if (isStringEmpty(repository)) {
114+
if (isStringEmpty(repository) || repositoryDoesNotExist) {
103115
validation.errors.repository.push(
104116
i18n.translate('xpack.snapshotRestore.policyValidation.repositoryRequiredErrorMessage', {
105117
defaultMessage: 'Repository is required.',

0 commit comments

Comments
 (0)