-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[App Search] Implement various Relevance Tuning states and form actions #92644
Conversation
5750cc1
to
b3de21d
Compare
..._search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.ts
Show resolved
Hide resolved
..._search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.ts
Show resolved
Hide resolved
b3de21d
to
86f1fe2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some test mock comments
..._search/public/applications/app_search/components/relevance_tuning/relevance_tuning.test.tsx
Outdated
Show resolved
Hide resolved
...ch/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.test.ts
Outdated
Show resolved
Hide resolved
...tions/app_search/components/relevance_tuning/relevance_tuning_form/relevance_tuning_form.tsx
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
...prise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.tsx
Outdated
Show resolved
Hide resolved
...prise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.tsx
Outdated
Show resolved
Hide resolved
...prise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.tsx
Outdated
Show resolved
Hide resolved
...prise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.tsx
Outdated
Show resolved
Hide resolved
..._search/public/applications/app_search/components/relevance_tuning/relevance_tuning.test.tsx
Outdated
Show resolved
Hide resolved
..._search/public/applications/app_search/components/relevance_tuning/relevance_tuning.test.tsx
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
…h/components/relevance_tuning/relevance_tuning.test.tsx Co-authored-by: Constance <[email protected]>
…h/components/relevance_tuning/relevance_tuning_form/relevance_tuning_form.tsx Co-authored-by: Constance <[email protected]>
…h/components/relevance_tuning/relevance_tuning.tsx Co-authored-by: Constance <[email protected]>
…h/components/relevance_tuning/relevance_tuning.test.tsx Co-authored-by: Constance <[email protected]>
@constancecchen Removed that old comment: 9c20faf |
@constancecchen Cherry-picked 55e9ebcc1e946a1210905c63628c9eb9c39c60a0 |
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look fantastic! Really love the new Layout and Callout components 😍
<RelevanceTuningLayout engineBreadcrumb={engineBreadcrumb}> | ||
<UnsavedChangesPrompt hasUnsavedChanges={unsavedChanges} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[optional, not a blocker] I think you could probably bake UnsavedChangesPrompt
into the RelevanceTuningLayout
, unless you think that's too hidden or confusing. Up to you!
if (dataLoading) { | ||
return <Loading />; | ||
} | ||
|
||
if (!engineHasSchemaFields) { | ||
return <EmptyCallout />; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also optional, but I think Scotty stated a preference for single line returns. I'm fine with whatever though lol, not a big deal to me, we can yolo it and come back to returns later
expect(mockEngineActions.initializeEngine).toHaveBeenCalled(); | ||
}); | ||
|
||
it('will re-fetch the current engine after settings are updated if there were unconfirmed search fieldds', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
damnit, only just noticed this, sorry! optional though, feel free to fix in a future PR instead if you'd rather save time on CI cycles
it('will re-fetch the current engine after settings are updated if there were unconfirmed search fieldds', async () => { | |
it('will re-fetch the current engine after settings are updated if there were unconfirmed search fields', async () => { |
* master: (42 commits) [Lens] Introduces new chart switcher (elastic#91844) [Lens] fix selection when dragging (elastic#93034) Converts usage collection README to .mdx (elastic#92982) Fix expanding document when using saved search data grid (elastic#92999) [SECURITY SOLUTIONS] Bug case connector (elastic#93104) [Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON (elastic#92025) [Alerting][Docs] Changed alerting documentation to point to a single source of explaining the configurations. (elastic#92942) [APM] Fix hidden search bar in error pages while loading (elastic#84476) (elastic#93139) [DOCS] Fixes links for machine learning alerts (elastic#92744) [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (elastic#92748) [SecuritySolution][Case] Disable cases on detections in read-only mode (elastic#93010) [Security Solution][Case][Bug] Prevent closing collection when pushing (elastic#93095) [Security Solution][Detections][7.12] Critical Threshold Rule Fixes (elastic#92667) Bump ems landing page to 7.12 (elastic#93065) [App Search] Implement various Relevance Tuning states and form actions (elastic#92644) [actions] for simplistic email servers, set rejectUnauthorized to false (elastic#91760) [Security Solution][Case] Migrate category & subcategory fields of ServiceNow ITSM connector (elastic#93092) Hide instances latency distribution chart (elastic#92869) [Maps] fix MapboxDraw import from pointing to dist just pointing to folder (elastic#93087) [Maps] fix results trimmed tooltip message doubles feature count for line and polygon features (elastic#92932) ...
… playwright-ftr-e2e * 'playwright-ftr-e2e' of github.com:shahzad31/kibana: (38 commits) [chore] Enable core's eslint rule: `@ts-expect-error` (elastic#93086) [Lens] Introduces new chart switcher (elastic#91844) [Lens] fix selection when dragging (elastic#93034) Converts usage collection README to .mdx (elastic#92982) Fix expanding document when using saved search data grid (elastic#92999) [SECURITY SOLUTIONS] Bug case connector (elastic#93104) [Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON (elastic#92025) [Alerting][Docs] Changed alerting documentation to point to a single source of explaining the configurations. (elastic#92942) [APM] Fix hidden search bar in error pages while loading (elastic#84476) (elastic#93139) [DOCS] Fixes links for machine learning alerts (elastic#92744) [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (elastic#92748) [SecuritySolution][Case] Disable cases on detections in read-only mode (elastic#93010) [Security Solution][Case][Bug] Prevent closing collection when pushing (elastic#93095) [Security Solution][Detections][7.12] Critical Threshold Rule Fixes (elastic#92667) Bump ems landing page to 7.12 (elastic#93065) [App Search] Implement various Relevance Tuning states and form actions (elastic#92644) [actions] for simplistic email servers, set rejectUnauthorized to false (elastic#91760) [Security Solution][Case] Migrate category & subcategory fields of ServiceNow ITSM connector (elastic#93092) Hide instances latency distribution chart (elastic#92869) [Maps] fix MapboxDraw import from pointing to dist just pointing to folder (elastic#93087) ...
…ns (#92644) (#93135) Co-authored-by: Jason Stoltzfus <[email protected]>
… ilm/rollup-v2-action * 'ilm/rollup-v2-action' of github.com:elastic/kibana: (30 commits) Fix expanding document when using saved search data grid (#92999) [SECURITY SOLUTIONS] Bug case connector (#93104) [Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON (#92025) [Alerting][Docs] Changed alerting documentation to point to a single source of explaining the configurations. (#92942) [APM] Fix hidden search bar in error pages while loading (#84476) (#93139) [DOCS] Fixes links for machine learning alerts (#92744) [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748) [SecuritySolution][Case] Disable cases on detections in read-only mode (#93010) [Security Solution][Case][Bug] Prevent closing collection when pushing (#93095) [Security Solution][Detections][7.12] Critical Threshold Rule Fixes (#92667) Bump ems landing page to 7.12 (#93065) [App Search] Implement various Relevance Tuning states and form actions (#92644) [actions] for simplistic email servers, set rejectUnauthorized to false (#91760) [Security Solution][Case] Migrate category & subcategory fields of ServiceNow ITSM connector (#93092) Hide instances latency distribution chart (#92869) [Maps] fix MapboxDraw import from pointing to dist just pointing to folder (#93087) [Maps] fix results trimmed tooltip message doubles feature count for line and polygon features (#92932) [Security Solution][Detecttions] Indicator enrichment tweaks (#92989) [Maps] fix fit to data on heatmap not working (#92697) [Security Solution][Endpoint][Admin] Fixes policy sticky footer save test (#92919) ...
Summary
The commits in this PR are atomic, you should be able to follow along commit by commit.
Added Save and Reset buttons
To test:
Added the
UnsavedChangesPrompt
to prevent users from leaving the form if they have unsaved changesTo test:
Re-arranged the header to use the new
description
,body
, andaction
fieldsAdded an Invalid boosts error message
To test:
Added an unconfirmed fields message
To test:
Note that I moved the position of this callout to be located with the other callouts, and removed the "dismiss" button, which seemed like overkill.
Add an empty state
To test:
Checklist
Delete any items that are not applicable to this PR.
For maintainers