System index templates can't be edited

[jkelastic]

Summary

Fixed # 52566 Removed the validator that checks for system indices and disable editing on index templates

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

• This was checked for cross-browser compatibility, including a check against IE11
• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for keyboard-only and screenreader accessibility

For maintainers

• This was checked for breaking API changes and was labeled appropriately
• This includes a feature addition or change that requires a release note and was labeled appropriately

[elasticmachine]

Pinging @elastic/es-ui (Team:Elasticsearch UI)

[jkelastic]

I think we still want this validation when creating an index template. By deleting this code, I think it will remove it from both creating and editing. I reached out to @sebelga to see the best way to approach this with the form lib.

copied previous comments over to this new branch

[jkelastic]

@alisonelizabeth I've recreated all 3 branches. Things look good now. Sorry for the inconvenience.

cc @sebelga

[sebelga]

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

[alisonelizabeth]

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))

[jkelastic]

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.

[sebelga]

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
    ...
  },
};

[jkelastic]

@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

[jkelastic]

@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!

[sebelga]

Code LGTM! I haven't tested locally. Thanks for making these changes!

[jkelastic]

@elasticmachine merge upstream

[jkelastic]

Code LGTM! I haven't tested locally. Thanks for making these changes!

No problem, happy to be able to contribute and make a difference!

[alisonelizabeth]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 7327860

History

• 💚 Build #22734 succeeded 08d3275
• 💚 Build #22196 succeeded d6d5fa6
• 💔 Build #22182 failed ad204c5
• 💔 Build #21166 failed 7868434

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[cjcenizal]

@alisonelizabeth @jkelastic @sebelga I just want to double-check -- has this been tested locally and verified to fix the issue?

[alisonelizabeth]

@cjcenizal I tested the PR targeting master.