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

Add explicit save / revert buttons in search & ingest forms #361

Merged
merged 5 commits into from
Sep 10, 2024

Conversation

ohltyler
Copy link
Member

@ohltyler ohltyler commented Sep 10, 2024

Description

This PR reverts the autosave functionality due to its ambiguity on when cluster resources are updated. To handle saving, this adds an explicit save button, and corresponding undo, to revert to the last saved state. Buttons are enabled/disabled based on the form state. Under the hood, when users click save, it runs the update workflow API with reprovision=false. Only the config is saved, but not the underlying workflow template or any associated resources. For full updates, users still have the ingest/search buttons. Updated text to more explicitly state what is happening when those are executed, which is actually building out the resources and running ingest/search. When users click the revert button, the form and transient UI config is reverted to the saved/indexed config.

Details on the different form states:

  • all general form state is all handled by formik to detect value changes, when values are touched, etc.
  • when ingest/search processors are added/removed, rather than updating the form, we have to re-create the form and updated form schema. This is because we generate the form based on metadata in the UI config (the config field types, defaults, etc.), and can't go vice-versa. Note this is why we pass the setUiConfig param to components, so it can be triggered on these particular actions. Because we can't listen on formik value changes for this part, we need standalone hooks to listen on when the current uiConfig has updated.

Implementation details:

  • adds unsavedIngestProcessors/unsavedSearchProcessors flags to persist when there are form changes related to adding/removing ingest/search processors
  • adds hooks to listen when the uiConfig has updated, and detect processor config changes to set unsavedIngestProcessors/unsavedSearchProcessors
  • refactors the autosave logic to have a single updateWorkflowUiConfig() fn to update the UI config only, without any template or resource updates. Used in the new Save button
  • adds revert/save buttons for both ingest/search conditions

Demo video, showing the new buttons functionality for ingest/search:

screen-capture.16.webm

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
@saimedhi saimedhi merged commit eb21762 into opensearch-project:main Sep 10, 2024
6 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 10, 2024
* Remove autosave; add save/revert buttons (ingest)

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

* Add save/revert buttons (search)

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

* cleanup

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

* cleanup

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

* remove unnecessary update

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

---------

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
(cherry picked from commit eb21762)
@ohltyler ohltyler deleted the remove-autosave branch September 10, 2024 20:18
ohltyler added a commit that referenced this pull request Sep 10, 2024
)


Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
(cherry picked from commit eb21762)

Co-authored-by: Tyler Ohlsen <ohltyler@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants