[Index Management] Fix bug in mappings fields

[ElenaStoeva]

Fixes #156202

Summary

This PR fixes the bug in Mappings fields from the Component template wizard by changing the Subtype parameter so that it handles the case when the value in the Subtype dropdown menu is cleared, similarly to what is done in the Type parameter.

How to test:

1. Navigate to Index Management, go to Component templates tab.
2. Start the wizard to create a new component template.
3. Go to the Mappings step and add a field of type Numeric or Range, and click on the Subtype dropdown menu.
4. Verify that pressing backspace in the Subtype dropdown menu doesn't make the page to crash.
5. After adding the mapping field, click the pencil icon to edit this field and in the flyout click in the Numeric/Range type dropdown.
6. Verify that pressing backspace doesn't make the page to crash.

[elasticmachine]

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

[yuliacech]

Thanks a lot for fixing the form, @ElenaStoeva! Would it be possible to add a test for this bug?

[ElenaStoeva]

Thanks a lot for fixing the form, @ElenaStoeva! Would it be possible to add a test for this bug?

Thanks for the review @yuliacech!

Regarding the tests, I see that there are already integration tests for the Numeric and Range type fields, but I'm not sure how exactly we would write a test for this bug. We would need to simulate typing a backspace but I still don't know if that would be possible and how much effort it would require. Do you have any idea?

[yuliacech]

Hi @ElenaStoeva, I think you would need to add a new action to the template_form_helpers file here. To simulate the backspace, it would look something like this find("input-data-test-subj").simulate('keydown', { key: 'Backspace' }). There is a similar test for example here and the correct key for the backspace press can be found on this page.

[ElenaStoeva]

Hi @ElenaStoeva, I think you would need to add a new action to the template_form_helpers file here. To simulate the backspace, it would look something like this find("input-data-test-subj").simulate('keydown', { key: 'Backspace' }). There is a similar test for example here and the correct key for the backspace press can be found on this page.

Thanks for the suggestion @yuliacech!
I tried adding the following integration test in x-pack/plugins/index_management/public/application/components/mappings_editor/__jest__/client_integration/datatypes/date_range_datatype.test.tsx:

  test('should not allow clearing the Data Range subtype dropdown', async () => {
    const defaultMappings = {
      properties: {
        myField: {
          type: 'double_range',
        },
      },
    };

    await act(async () => {
      testBed = setup({ value: defaultMappings, onChange: onChangeHandler });
    });
    testBed.component.update();

    const {
      component,
      find,
      exists,
      actions: { startEditField },
    } = testBed;

    // Open the flyout to edit the field
    await startEditField('myField');

    // Press Backspace at the Subtype dropdown
    await act(async () => {
      find('fieldSubType').simulate('keydown', { key: 'Backspace' });
    });
    component.update();

    // Verify that page didn't crash
    expect(exists('mappingsEditorFieldEdit.editFieldUpdateButton'));

    // Verify that Update button is disabled - no change has been made
    expect(find('mappingsEditorFieldEdit.editFieldUpdateButton').props().disabled).toBe(true);
  });

but I noticed that it always passes - even when the fix for the bug is not present, so this test might not be very useful.

I also tried adding a functional test but this turned out to be tricky - for some reason the webdriver under the hood is not able to interact with the SubType dropdown menu when I try to click on it or set its value:

ElementNotInteractableError: element not interactable
   (Session info: chrome=114.0.5735.106)
       at Object.throwDecodedError (node_modules/selenium-webdriver/lib/error.js:524:15)
       at parseHttpResponse (node_modules/selenium-webdriver/lib/http.js:601:13)
       at Executor.execute (node_modules/selenium-webdriver/lib/http.js:529:28)
       at processTicksAndRejections (node:internal/process/task_queues:96:5)
       at Task.exec (prevent_parallel_calls.ts:28:20)

I can spend some more time to try to find some way to simulate pressing Backspace but I wonder if it would be worth the time given that this is very specific case that we already tested manually and I'm not sure if it would be beneficial to add a whole test for it. What do you think?

[ElenaStoeva]

Hey @yuliacech, I found a workaround for the error in the functional tests so I added them with ea77b4b. Without the fix from this PR, these two tests are failing so they should be catching this bug in the future.
Let me know if you have an suggestion on the tests.

[ElenaStoeva]

@elasticmachine merge upstream

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: ecb1144

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
indexManagement	525.9KB	526.0KB	+22.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
enterpriseSearch	19	21	+2
securitySolution	413	417	+4
total			+6

Total ESLint disabled count

id	before	after	diff
enterpriseSearch	20	22	+2
securitySolution	497	501	+4
total			+6

History

• 💚 Build #132072 succeeded 91504ee

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

cc @ElenaStoeva

[yuliacech]

Amazing effort, @ElenaStoeva! Thanks a lot for looking into the tests for the bug as well 👍