[Snapshot & Restore] fix pre existing policy with no existing reposit…

…ory (#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 <[email protected]>
elastic · Sep 9, 2020 · 1c3f0fc · 1c3f0fc
1 parent 60294cb
commit 1c3f0fc
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 4 deletions.
diff --git a/x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/policy_form.helpers.ts b/x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/policy_form.helpers.ts
@@ -56,4 +56,6 @@ export type PolicyFormTestSubjects =
   | 'showAdvancedCronLink'
   | 'snapshotNameInput'
   | 'dataStreamBadge'
+  | 'repositoryNotFoundWarning'
+  | 'repositorySelect'
   | 'submitButton';
diff --git a/x-pack/plugins/snapshot_restore/__jest__/client_integration/policy_add.test.ts b/x-pack/plugins/snapshot_restore/__jest__/client_integration/policy_add.test.ts
@@ -67,6 +67,11 @@ describe('<PolicyAdd />', () => {
       expect(find('nextButton').props().disabled).toBe(true);
     });
 
+    test('should not show repository-not-found warning', () => {
+      const { exists } = testBed;
+      expect(exists('repositoryNotFoundWarning')).toBe(false);
+    });
+
     describe('form validation', () => {
       describe('logistics (step 1)', () => {
         test('should require a policy name', async () => {

diff --git a/x-pack/plugins/snapshot_restore/__jest__/client_integration/policy_edit.test.ts b/x-pack/plugins/snapshot_restore/__jest__/client_integration/policy_edit.test.ts
@@ -52,6 +52,44 @@ describe('<PolicyEdit />', () => {
       expect(find('pageTitle').text()).toEqual('Edit policy');
     });
 
+    describe('policy with pre-existing repository that was deleted', () => {
+      beforeEach(async () => {
+        httpRequestsMockHelpers.setGetPolicyResponse({ policy: POLICY_EDIT });
+        httpRequestsMockHelpers.setLoadIndicesResponse({
+          indices: ['my_index'],
+          dataStreams: ['my_data_stream'],
+        });
+        httpRequestsMockHelpers.setLoadRepositoriesResponse({
+          repositories: [{ name: 'this-is-a-new-repository' }],
+        });
+
+        testBed = await setup();
+
+        await act(async () => {
+          await nextTick();
+          testBed.component.update();
+        });
+      });
+
+      test('should show repository-not-found warning', () => {
+        const { exists, find } = testBed;
+        expect(exists('repositoryNotFoundWarning')).toBe(true);
+        // The select should be an empty string to allow users to select a new repository
+        expect(find('repositorySelect').props().value).toBe('');
+      });
+
+      describe('validation', () => {
+        test('should block navigating to next step', () => {
+          const { exists, find, actions } = testBed;
+          actions.clickNextButton();
+          // Assert that we are still on the repository configuration step
+          expect(exists('repositoryNotFoundWarning')).toBe(true);
+          // The select should be an empty string to allow users to select a new repository
+          expect(find('repositorySelect').props().value).toBe('');
+        });
+      });
+    });
+
     /**
      * As the "edit" policy component uses the same form underneath that
      * the "create" policy, we won't test it again but simply make sure that

diff --git a/...ugins/snapshot_restore/public/application/components/policy_form/steps/step_logistics.tsx b/...ugins/snapshot_restore/public/application/components/policy_form/steps/step_logistics.tsx
@@ -18,6 +18,8 @@ import {
   EuiLink,
   EuiSpacer,
   EuiText,
+  EuiCallOut,
+  EuiCode,
 } from '@elastic/eui';
 
 import { Repository } from '../../../../../common/types';
@@ -54,6 +56,10 @@ export const PolicyStepLogistics: React.FunctionComponent<StepProps> = ({
 
   const { i18n, history } = useServices();
 
+  const [showRepositoryNotFoundWarning, setShowRepositoryNotFoundWarning] = useState<boolean>(
+    false
+  );
+
   // State for touched inputs
   const [touched, setTouched] = useState({
     name: false,
@@ -256,13 +262,26 @@ export const PolicyStepLogistics: React.FunctionComponent<StepProps> = ({
       }
     }
 
+    const doesRepositoryExist =
+      !!policy.repository &&
+      repositories.some((r: { name: string }) => r.name === policy.repository);
+
+    if (!doesRepositoryExist && !errors.repository) {
+      updatePolicy(policy, { repositoryDoesNotExist: true });
+    }
+
+    if (showRepositoryNotFoundWarning !== !doesRepositoryExist) {
+      setShowRepositoryNotFoundWarning(!doesRepositoryExist);
+    }
+
     return (
       <EuiSelect
         options={repositories.map(({ name }: Repository) => ({
           value: name,
           text: name,
         }))}
-        value={policy.repository || repositories[0].name}
+        hasNoInitialSelection={!doesRepositoryExist}
+        value={!doesRepositoryExist ? '' : policy.repository}
         onBlur={() => setTouched({ ...touched, repository: true })}
         onChange={(e) => {
           updatePolicy(
@@ -541,8 +560,31 @@ export const PolicyStepLogistics: React.FunctionComponent<StepProps> = ({
           </EuiButtonEmpty>
         </EuiFlexItem>
       </EuiFlexGroup>
-      <EuiSpacer size="l" />
 
+      {showRepositoryNotFoundWarning && (
+        <>
+          <EuiSpacer size="m" />
+          <EuiCallOut
+            data-test-subj="repositoryNotFoundWarning"
+            title={
+              <FormattedMessage
+                id="xpack.snapshotRestore.policyForm.stepLogistics.selectRepository.policyRepositoryNotFoundTitle"
+                defaultMessage="Repository not found"
+              />
+            }
+            color="danger"
+            iconType="alert"
+          >
+            <FormattedMessage
+              id="xpack.snapshotRestore.policyForm.stepLogistics.selectRepository.policyRepositoryNotFoundDescription"
+              defaultMessage="Repository {repo} does not exist. Please select an existing repository."
+              values={{ repo: <EuiCode>{policy.repository}</EuiCode> }}
+            />
+          </EuiCallOut>
+        </>
+      )}
+
+      <EuiSpacer size="l" />
       {renderNameField()}
       {renderSnapshotNameField()}
       {renderRepositoryField()}

diff --git a/x-pack/plugins/snapshot_restore/public/application/services/validation/validate_policy.ts b/x-pack/plugins/snapshot_restore/public/application/services/validation/validate_policy.ts
@@ -41,6 +41,12 @@ export interface ValidatePolicyHelperData {
    * are not configuring this value - like when they are on a previous step.
    */
   validateIndicesCount?: boolean;
+
+  /**
+   * A policy might be configured with a repository that no longer exists. We want the form to
+   * block in this case because just having a repository configured is not enough for validity.
+   */
+  repositoryDoesNotExist?: boolean;
 }
 
 export const validatePolicy = (
@@ -50,7 +56,13 @@ export const validatePolicy = (
   const i18n = textService.i18n;
 
   const { name, snapshotName, schedule, repository, config, retention } = policy;
-  const { managedRepository, isEditing, policyName, validateIndicesCount } = validationHelperData;
+  const {
+    managedRepository,
+    isEditing,
+    policyName,
+    validateIndicesCount,
+    repositoryDoesNotExist,
+  } = validationHelperData;
 
   const validation: PolicyValidation = {
     isValid: true,
@@ -99,7 +111,7 @@ export const validatePolicy = (
     );
   }
 
-  if (isStringEmpty(repository)) {
+  if (isStringEmpty(repository) || repositoryDoesNotExist) {
     validation.errors.repository.push(
       i18n.translate('xpack.snapshotRestore.policyValidation.repositoryRequiredErrorMessage', {
         defaultMessage: 'Repository is required.',