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.

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

[Automatic Import] Bug: pressing Enter resets the process #198238

Closed
ilyannn opened this issue Oct 30, 2024 · 1 comment · Fixed by #199894
Closed

[Automatic Import] Bug: pressing Enter resets the process #198238

ilyannn opened this issue Oct 30, 2024 · 1 comment · Fixed by #199894
Assignees
Labels
8.17 candidate accessibility: keyboard navigation Accessibility keyboard navigation and focus bug Fixes for quality problems that affect the customer experience Feature:AutomaticImport Team:Security-Scalability Team label for Security Integrations Scalability Team

Comments

@ilyannn
Copy link
Contributor

ilyannn commented Oct 30, 2024

Pressing Enter in a field during the process resets the process by opening the URL of the form .../icu/app/integrations/create/assistant?name=postgres or .../icu/app/integrations/create/assistant?title=something&description=. depending on the field.

@ilyannn ilyannn added the Team:Security-Scalability Team label for Security Integrations Scalability Team label Oct 30, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-scalability (Team:Security-Scalability)

@ilyannn ilyannn added Feature:AutomaticImport bug Fixes for quality problems that affect the customer experience accessibility: keyboard navigation Accessibility keyboard navigation and focus labels Oct 30, 2024
@ilyannn ilyannn self-assigned this Nov 8, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Dec 11, 2024
## Release Notes

Fixes the bug where pressing Enter reloaded the Automatic Import.

## Summary

- Fixes elastic#198238
- Adds/fixes telemetry for CEL events.
- Refactors navigation functionality.
- Adds extensive unit tests and a Cypress test for it.

## Details

When the user presses the Enter inside our input field, the expected
action is to send the form, in this case completing the step. However,
previously the form submission would instead lead to reloading the whole
Automatic Import page.

In this PR we capture the form submission event and bubble it up as
`completeStep` to the main component. We also move the implementation
from the `Footer` up to this main component
`CreateIntegrationAssistant`. This helps collect all the details about
the step order in one place and refactor this logic.

As a result, pressing `Enter` in any field now either
 - Is processed by the field itself (in case of multi-line fields);
- Leads to the same action as pressing the "Next" button (desired
result); or
- Does nothing (e.g. in the inputs in the "Define data stream and upload
logs" group – the reason for this is unclear).

We add CEL-specific telemetry identifiers so that telemetry for step 5
is not always reported as `Deploy Step`.

We also rename a bunch of stuff that was named `...StepReady` into
`...StepReadyToComplete` as the previous name was ambiguous. To
demonstrate this ambiguity we've enlisted the help of GPT 4o:

<img width="832" alt="SCR-20241125-tiaa"
src="https://github.com/user-attachments/assets/ad6bcf7c-7cb2-41c2-ac6b-38924ce990d3">

## Testing

We provide a Cypress test for Enter behavior: pressing it on the
"integration title" input should let the flow proceed to the next step.
This test fails on `main`.

We also provide unit tests for all steps of navigation functionality in
`x-pack/plugins/integration_assistant/public/components/create_integration/create_integration_assistant/create_integration_assistant.test.tsx`:

Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit d8bb72e)
kibanamachine added a commit that referenced this issue Dec 11, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Automatic Import] Fix the enter bug
(#199894)](#199894)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ilya
Nikokoshev","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-11T15:04:45Z","message":"[Automatic
Import] Fix the enter bug (#199894)\n\n## Release Notes\r\n\r\nFixes the
bug where pressing Enter reloaded the Automatic Import.\r\n\r\n##
Summary\r\n\r\n- Fixes #198238\r\n- Adds/fixes telemetry for CEL
events.\r\n- Refactors navigation functionality.\r\n- Adds extensive
unit tests and a Cypress test for it.\r\n\r\n## Details\r\n\r\nWhen the
user presses the Enter inside our input field, the expected\r\naction is
to send the form, in this case completing the step.
However,\r\npreviously the form submission would instead lead to
reloading the whole\r\nAutomatic Import page.\r\n\r\nIn this PR we
capture the form submission event and bubble it up as\r\n`completeStep`
to the main component. We also move the implementation\r\nfrom the
`Footer` up to this main component\r\n`CreateIntegrationAssistant`. This
helps collect all the details about\r\nthe step order in one place and
refactor this logic.\r\n\r\nAs a result, pressing `Enter` in any field
now either\r\n - Is processed by the field itself (in case of multi-line
fields);\r\n- Leads to the same action as pressing the \"Next\" button
(desired\r\nresult); or\r\n- Does nothing (e.g. in the inputs in the
\"Define data stream and upload\r\nlogs\" group – the reason for this is
unclear).\r\n\r\nWe add CEL-specific telemetry identifiers so that
telemetry for step 5\r\nis not always reported as `Deploy
Step`.\r\n\r\nWe also rename a bunch of stuff that was named
`...StepReady` into\r\n`...StepReadyToComplete` as the previous name was
ambiguous. To\r\ndemonstrate this ambiguity we've enlisted the help of
GPT 4o:\r\n\r\n<img width=\"832\"
alt=\"SCR-20241125-tiaa\"\r\nsrc=\"https://github.com/user-attachments/assets/ad6bcf7c-7cb2-41c2-ac6b-38924ce990d3\">\r\n\r\n\r\n##
Testing\r\n\r\nWe provide a Cypress test for Enter behavior: pressing it
on the\r\n\"integration title\" input should let the flow proceed to the
next step.\r\nThis test fails on `main`.\r\n\r\nWe also provide unit
tests for all steps of navigation functionality
in\r\n`x-pack/plugins/integration_assistant/public/components/create_integration/create_integration_assistant/create_integration_assistant.test.tsx`:\r\n\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"d8bb72ebfdb1514f1fc03857d4c4e02f70fb7403","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","refactoring","Team:Fleet","v9.0.0","accessibility:
keyboard
navigation","backport:prev-minor","Team:Security-Scalability","Feature:AutomaticImport"],"title":"[Automatic
Import] Fix the enter
bug","number":199894,"url":"https://github.com/elastic/kibana/pull/199894","mergeCommit":{"message":"[Automatic
Import] Fix the enter bug (#199894)\n\n## Release Notes\r\n\r\nFixes the
bug where pressing Enter reloaded the Automatic Import.\r\n\r\n##
Summary\r\n\r\n- Fixes #198238\r\n- Adds/fixes telemetry for CEL
events.\r\n- Refactors navigation functionality.\r\n- Adds extensive
unit tests and a Cypress test for it.\r\n\r\n## Details\r\n\r\nWhen the
user presses the Enter inside our input field, the expected\r\naction is
to send the form, in this case completing the step.
However,\r\npreviously the form submission would instead lead to
reloading the whole\r\nAutomatic Import page.\r\n\r\nIn this PR we
capture the form submission event and bubble it up as\r\n`completeStep`
to the main component. We also move the implementation\r\nfrom the
`Footer` up to this main component\r\n`CreateIntegrationAssistant`. This
helps collect all the details about\r\nthe step order in one place and
refactor this logic.\r\n\r\nAs a result, pressing `Enter` in any field
now either\r\n - Is processed by the field itself (in case of multi-line
fields);\r\n- Leads to the same action as pressing the \"Next\" button
(desired\r\nresult); or\r\n- Does nothing (e.g. in the inputs in the
\"Define data stream and upload\r\nlogs\" group – the reason for this is
unclear).\r\n\r\nWe add CEL-specific telemetry identifiers so that
telemetry for step 5\r\nis not always reported as `Deploy
Step`.\r\n\r\nWe also rename a bunch of stuff that was named
`...StepReady` into\r\n`...StepReadyToComplete` as the previous name was
ambiguous. To\r\ndemonstrate this ambiguity we've enlisted the help of
GPT 4o:\r\n\r\n<img width=\"832\"
alt=\"SCR-20241125-tiaa\"\r\nsrc=\"https://github.com/user-attachments/assets/ad6bcf7c-7cb2-41c2-ac6b-38924ce990d3\">\r\n\r\n\r\n##
Testing\r\n\r\nWe provide a Cypress test for Enter behavior: pressing it
on the\r\n\"integration title\" input should let the flow proceed to the
next step.\r\nThis test fails on `main`.\r\n\r\nWe also provide unit
tests for all steps of navigation functionality
in\r\n`x-pack/plugins/integration_assistant/public/components/create_integration/create_integration_assistant/create_integration_assistant.test.tsx`:\r\n\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"d8bb72ebfdb1514f1fc03857d4c4e02f70fb7403"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199894","number":199894,"mergeCommit":{"message":"[Automatic
Import] Fix the enter bug (#199894)\n\n## Release Notes\r\n\r\nFixes the
bug where pressing Enter reloaded the Automatic Import.\r\n\r\n##
Summary\r\n\r\n- Fixes #198238\r\n- Adds/fixes telemetry for CEL
events.\r\n- Refactors navigation functionality.\r\n- Adds extensive
unit tests and a Cypress test for it.\r\n\r\n## Details\r\n\r\nWhen the
user presses the Enter inside our input field, the expected\r\naction is
to send the form, in this case completing the step.
However,\r\npreviously the form submission would instead lead to
reloading the whole\r\nAutomatic Import page.\r\n\r\nIn this PR we
capture the form submission event and bubble it up as\r\n`completeStep`
to the main component. We also move the implementation\r\nfrom the
`Footer` up to this main component\r\n`CreateIntegrationAssistant`. This
helps collect all the details about\r\nthe step order in one place and
refactor this logic.\r\n\r\nAs a result, pressing `Enter` in any field
now either\r\n - Is processed by the field itself (in case of multi-line
fields);\r\n- Leads to the same action as pressing the \"Next\" button
(desired\r\nresult); or\r\n- Does nothing (e.g. in the inputs in the
\"Define data stream and upload\r\nlogs\" group – the reason for this is
unclear).\r\n\r\nWe add CEL-specific telemetry identifiers so that
telemetry for step 5\r\nis not always reported as `Deploy
Step`.\r\n\r\nWe also rename a bunch of stuff that was named
`...StepReady` into\r\n`...StepReadyToComplete` as the previous name was
ambiguous. To\r\ndemonstrate this ambiguity we've enlisted the help of
GPT 4o:\r\n\r\n<img width=\"832\"
alt=\"SCR-20241125-tiaa\"\r\nsrc=\"https://github.com/user-attachments/assets/ad6bcf7c-7cb2-41c2-ac6b-38924ce990d3\">\r\n\r\n\r\n##
Testing\r\n\r\nWe provide a Cypress test for Enter behavior: pressing it
on the\r\n\"integration title\" input should let the flow proceed to the
next step.\r\nThis test fails on `main`.\r\n\r\nWe also provide unit
tests for all steps of navigation functionality
in\r\n`x-pack/plugins/integration_assistant/public/components/create_integration/create_integration_assistant/create_integration_assistant.test.tsx`:\r\n\r\n\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"d8bb72ebfdb1514f1fc03857d4c4e02f70fb7403"}}]}]
BACKPORT-->

Co-authored-by: Ilya Nikokoshev <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this issue Dec 12, 2024
## Release Notes

Fixes the bug where pressing Enter reloaded the Automatic Import.

## Summary

- Fixes elastic#198238
- Adds/fixes telemetry for CEL events.
- Refactors navigation functionality.
- Adds extensive unit tests and a Cypress test for it.

## Details

When the user presses the Enter inside our input field, the expected
action is to send the form, in this case completing the step. However,
previously the form submission would instead lead to reloading the whole
Automatic Import page.

In this PR we capture the form submission event and bubble it up as
`completeStep` to the main component. We also move the implementation
from the `Footer` up to this main component
`CreateIntegrationAssistant`. This helps collect all the details about
the step order in one place and refactor this logic.

As a result, pressing `Enter` in any field now either
 - Is processed by the field itself (in case of multi-line fields);
- Leads to the same action as pressing the "Next" button (desired
result); or
- Does nothing (e.g. in the inputs in the "Define data stream and upload
logs" group – the reason for this is unclear).

We add CEL-specific telemetry identifiers so that telemetry for step 5
is not always reported as `Deploy Step`.

We also rename a bunch of stuff that was named `...StepReady` into
`...StepReadyToComplete` as the previous name was ambiguous. To
demonstrate this ambiguity we've enlisted the help of GPT 4o:

<img width="832" alt="SCR-20241125-tiaa"
src="https://github.com/user-attachments/assets/ad6bcf7c-7cb2-41c2-ac6b-38924ce990d3">


## Testing

We provide a Cypress test for Enter behavior: pressing it on the
"integration title" input should let the flow proceed to the next step.
This test fails on `main`.

We also provide unit tests for all steps of navigation functionality in
`x-pack/plugins/integration_assistant/public/components/create_integration/create_integration_assistant/create_integration_assistant.test.tsx`:


Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.17 candidate accessibility: keyboard navigation Accessibility keyboard navigation and focus bug Fixes for quality problems that affect the customer experience Feature:AutomaticImport Team:Security-Scalability Team label for Security Integrations Scalability Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants