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

Integrate query into form (not persisted in config) #188

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

ohltyler
Copy link
Member

Description

Similar to the way we persist the ingest documents, we persist the query in the same form. This means:
1/ integrating with the overall form for schema/validation and temporary persistence
2/ not integrating with the underlying, persisted, indexed, config.

More details:

  • persist the query state at the top-level ResizableWorkspace like the ingest docs
  • update the query request form ConfigureSearchRequest to be similar to the ingest docs form, utilizing the JsonField component (which handles the auto-formatting as well)
  • update validateAndRunQuery() to ensure there is a valid, non-empty query, when attempting to click 'Run query' button
  • final updates to the formik and schema conversion fns to include the query field

Demo video showing validation, auto-formatting, persistence across page navigation, and finally the query execution:

screen-capture.39.webm

Issues Resolved

Makes progress on #23

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.

processorsSchemaObj[processorConfig.id] = yup.object(processorSchemaObj);
});

return yup.object(processorsSchemaObj);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I could be wrong as I'm not very familiar with the Yup library, had to look up the Yup library documentation about yup.object to understand what we're trying to do here - define the schema. I see there's another method - yup.object().shape, that defines the structure and validation rules for an object. Should we change to use yup.object().shape here as yup.object() just defines the structure without any constraints?

Copy link
Member Author

Choose a reason for hiding this comment

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

The parameters passed in object are where the constraints are set - see documentation. Passing verbosely with shape achieves the same thing, but prefer the short-form to keep it simpler.

@ohltyler ohltyler merged commit 8afac97 into opensearch-project:main Jun 20, 2024
10 checks passed
@ohltyler ohltyler deleted the search-updates branch June 20, 2024 20:58
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 20, 2024
Signed-off-by: Tyler Ohlsen <[email protected]>
(cherry picked from commit 8afac97)
ohltyler added a commit that referenced this pull request Jun 20, 2024
Signed-off-by: Tyler Ohlsen <[email protected]>
(cherry picked from commit 8afac97)

Co-authored-by: Tyler Ohlsen <[email protected]>
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