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

System index templates can't be edited #55229

Merged
merged 7 commits into from
Jan 30, 2020
Merged

System index templates can't be edited #55229

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7868434
Removed period validation
jkelastic Jan 17, 2020
c5a4d31
Add back period validation
jkelastic Jan 18, 2020
ad204c5
Fix EsLint
jkelastic Jan 23, 2020
ad0db5d
Undo Snapshot changes from Index Management bug
jkelastic Jan 23, 2020
d6d5fa6
Undo Capitalization
jkelastic Jan 23, 2020
08d3275
Merge branch 'master' into bugfix/index_mgmt_systemAllowIndices
elasticmachine Jan 27, 2020
7327860
Merge branch 'master' into bugfix/index_mgmt_systemAllowIndices
elasticmachine Jan 30, 2020
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
3 changes: 2 additions & 1 deletion ...acy/plugins/index_management/public/app/components/template_form/steps/step_logistics.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
} from '../../../../../../../../../src/plugins/es_ui_shared/static/forms/components';
import { documentationService } from '../../../services/documentation';
import { StepProps } from '../types';
import { schemas } from '../template_form_schemas';
import { schemas, nameConfig, nameConfigWithoutValidations } from '../template_form_schemas';

// Create or Form components with partial props that are common to all instances
const UseField = getUseField({ component: Field });
Expand Down Expand Up @@ -131,6 +131,7 @@ export const StepLogistics: React.FunctionComponent<StepProps> = ({
['data-test-subj']: name.testSubject,
euiFieldProps: { disabled: isEditing },
}}
config={isEditing ? nameConfigWithoutValidations : nameConfig}
/>
</FormRow>
{/* Index patterns */}
Expand Down
124 changes: 63 additions & 61 deletions ...cy/plugins/index_management/public/app/components/template_form/template_form_schemas.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
FormSchema,
FIELD_TYPES,
VALIDATION_TYPES,
FieldConfig,
} from '../../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib';

import {
Expand All @@ -34,70 +35,71 @@ const {
const { toInt } = fieldFormatters;
const indexPatternInvalidCharacters = INVALID_INDEX_PATTERN_CHARS.join(' ');

export const schemas: Record<string, FormSchema> = {
logistics: {
name: {
type: FIELD_TYPES.TEXT,
label: i18n.translate('xpack.idxMgmt.templateForm.stepLogistics.fieldNameLabel', {
defaultMessage: 'Name',
export const nameConfig: FieldConfig = {
type: FIELD_TYPES.TEXT,
label: i18n.translate('xpack.idxMgmt.templateForm.stepLogistics.fieldNameLabel', {
defaultMessage: 'Name',
}),
validations: [
{
validator: emptyField(
i18n.translate('xpack.idxMgmt.templateValidation.templateNameRequiredError', {
defaultMessage: 'A template name is required.',
})
),
},
{
validator: containsCharsField({
chars: ' ',
message: i18n.translate('xpack.idxMgmt.templateValidation.templateNameSpacesError', {
defaultMessage: 'Spaces are not allowed in a template name.',
}),
}),
validations: [
{
validator: emptyField(
i18n.translate('xpack.idxMgmt.templateValidation.templateNameRequiredError', {
defaultMessage: 'A template name is required.',
})
),
},
{
validator: containsCharsField({
chars: ' ',
message: i18n.translate('xpack.idxMgmt.templateValidation.templateNameSpacesError', {
defaultMessage: 'Spaces are not allowed in a template name.',
}),
}),
},
{
validator: startsWithField({
char: '_',
message: i18n.translate(
'xpack.idxMgmt.templateValidation.templateNameUnderscoreError',
{
defaultMessage: 'A template name must not start with an underscore.',
}
),
}),
},
{
Copy link
Contributor

Choose a reason for hiding this comment

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

As from my comment on the other PR, we should keep this validation for when we create new template.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copying the original suggestion here for reference:

So here we have multiple options

  • Add a skipValidation prop to UseField (that would go along with the enabled: false
  • Be more granular and being able to provide validation IDs to be skipped
    validationsToSkip={['startWithDot']}
  • Extract and export the name config from the schema. Then add it on the UseField without the validations
import { schemas, nameConfig } from '../template_form_schemas';
const { validations, nameConfigWithoutValidations } = nameConfig;

// And then use that config when editing

<UseField
  path="name"
  componentProps={{
    idAria: name.idAria,
    ['data-test-subj']: name.testSubject,
    euiFieldProps: { disabled: isEditing },
  }}
  config={ isEditing ? nameConfigWithoutValidations : nameConfig }
/>

I would go with the last one as it is simpler and we don't need to touch the lib for now. It would be a nice enhancement though.

(#55145 (comment))

Copy link
Contributor Author

@jkelastic jkelastic Jan 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I put the above in step_logstics.tsx, but it says nameConfig doesn't exist in ../template_form_schemas . I must not be getting this correct.

Screen Shot 2020-01-22 at 3 28 48 PM

Screen Shot 2020-01-22 at 3 28 28 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

So you need to extract the config from the schema. Like this

// template_form_schema.tsx

import {
  FormSchema,
  FIELD_TYPES,
  VALIDATION_TYPES,
  FieldConfig, // Add this import
} from '../../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib';

// ...

export const nameConfig: FieldConfig = {
  type: FIELD_TYPES.TEXT,
  label: i18n.translate('xpack.idxMgmt.templateForm.stepLogistics.fieldNameLabel', {
    defaultMessage: 'Name',
  }),
  validations: [
    {
      validator: emptyField(
        i18n.translate('xpack.idxMgmt.templateValidation.templateNameRequiredError', {
          defaultMessage: 'A template name is required.',
        })
      ),
    },
    {
      validator: containsCharsField({
        chars: ' ',
        message: i18n.translate('xpack.idxMgmt.templateValidation.templateNameSpacesError', {
          defaultMessage: 'Spaces are not allowed in a template name.',
        }),
      }),
    },
    {
      validator: startsWithField({
        char: '_',
        message: i18n.translate('xpack.idxMgmt.templateValidation.templateNameUnderscoreError', {
          defaultMessage: 'A template name must not start with an underscore.',
        }),
      }),
    },
    {
      validator: startsWithField({
        char: '.',
        message: i18n.translate('xpack.idxMgmt.templateValidation.templateNamePeriodError', {
          defaultMessage: 'A template name must not start with a period.',
        }),
      }),
    },
    {
      validator: containsCharsField({
        chars: INVALID_TEMPLATE_NAME_CHARS,
        message: ({ charsFound }) =>
          i18n.translate('xpack.idxMgmt.templateValidation.templateNameInvalidaCharacterError', {
            defaultMessage: 'A template name must not contain the character "{invalidChar}"',
            values: { invalidChar: charsFound[0] },
          }),
      }),
    },
    {
      validator: lowerCaseStringField(
        i18n.translate('xpack.idxMgmt.templateValidation.templateNameLowerCaseRequiredError', {
          defaultMessage: 'The template name must be in lowercase.',
        })
      ),
    },
  ],
};

export const schemas: Record<string, FormSchema> = {
  logistics: {
    name: nameConfig, // use it here
    ...
  },
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sebelga thank you for all the detailed steps. I have a question, if const { validations, nameConfigWithoutValidations } = nameConfig; then nameConfigWithoutValidations would still always validate the system indices correct? We need to remove the period validation

    {
      validator: startsWithField({
        char: '.',
        message: i18n.translate('xpack.idxMgmt.templateValidation.templateNamePeriodError', {
          defaultMessage: 'A template name must not start with a period.',
        }),
      }),
    },

from nameConfig

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sebelga I've got some help from @alisonelizabeth and sorted out my mistakes and confusion. I've uploaded the modified code. Please help check my code again. Thank you!

validator: startsWithField({
char: '.',
message: i18n.translate('xpack.idxMgmt.templateValidation.templateNamePeriodError', {
defaultMessage: 'A template name must not start with a period.',
}),
}),
},
{
validator: containsCharsField({
chars: INVALID_TEMPLATE_NAME_CHARS,
message: ({ charsFound }) =>
i18n.translate(
'xpack.idxMgmt.templateValidation.templateNameInvalidaCharacterError',
{
defaultMessage: 'A template name must not contain the character "{invalidChar}"',
values: { invalidChar: charsFound[0] },
}
),
},
{
validator: startsWithField({
char: '_',
message: i18n.translate('xpack.idxMgmt.templateValidation.templateNameUnderscoreError', {
defaultMessage: 'A template name must not start with an underscore.',
}),
}),
},
{
validator: startsWithField({
char: '.',
message: i18n.translate('xpack.idxMgmt.templateValidation.templateNamePeriodError', {
defaultMessage: 'A template name must not start with a period.',
}),
}),
},
{
validator: containsCharsField({
chars: INVALID_TEMPLATE_NAME_CHARS,
message: ({ charsFound }) =>
i18n.translate('xpack.idxMgmt.templateValidation.templateNameInvalidaCharacterError', {
defaultMessage: 'A template name must not contain the character "{invalidChar}"',
values: { invalidChar: charsFound[0] },
}),
},
{
validator: lowerCaseStringField(
i18n.translate('xpack.idxMgmt.templateValidation.templateNameLowerCaseRequiredError', {
defaultMessage: 'The template name must be in lowercase.',
})
),
},
],
}),
},
{
validator: lowerCaseStringField(
i18n.translate('xpack.idxMgmt.templateValidation.templateNameLowerCaseRequiredError', {
defaultMessage: 'The template name must be in lowercase.',
})
),
},
],
};

export const nameConfigWithoutValidations: FieldConfig = {
...nameConfig,
validations: [],
};

export const schemas: Record<string, FormSchema> = {
logistics: {
name: nameConfig,
indexPatterns: {
type: FIELD_TYPES.COMBO_BOX,
defaultValue: [],
Expand Down