[App Search] Update EngineRouter & EngineNav to use EngineLogic #83138

cee-chen · 2020-11-11T01:26:41Z

Summary

Adds server-side route for HTTP API call
Updates EngineRouter & EngineNav with EngineLogic instead of previously static setup.

Screencaps

QA

Router tests

Engine initialization
- Open devtools and track XHR calls
- Click on any engine in the engines table
- Confirm you see a call to (e.g.) http://localhost:5601/api/app_search/engines/{ENGINE} in your logs and that its response returns the JSON you'd expect to see from our internal engine API
404 testing
- Go to http://localhost:5601/app/enterprise_search/app_search/engines/404engine (or whatever non-existent engine string)
- Confirm that you're redirected back to the Engines table with a "Could not find engine" error message

Engine nav tests

Invalid boosts
- [Enterprise Search] Create a sample engine. Then go to Schema and change the Visitors field from "number" to "text"
- [Kibana] Click into the sample engine/refresh.
- Confirm you see the "SAMPLE ENGINE" badge underneath the engine name
- Confirm you see a yellow alert icon next to Relevance Tuning nav link. Hovering over the icon should reveal the title "Invalid boosts"
Unconfirmed schema fields
- [Enterprise Search] Create a brand new engine. Populate it with documents from the paste sample JSON prompt
- [Kibana] Click into the new engine/refresh
- Confirm you see a blue info icon next to the Schema nav link. Hovering over the icon should reveal the title "New unconfirmed fields"
Unsearched unconfirmed schema fields
- [Enterprise Search] Go back to the sample engine and go to Documents. Index a new document with a new JSON field, e.g. add "test": "test", in the sample JSON
- [Kibana] Click into the sample engine/refresh
- Confirm you see a new yellow alert icon in the Relevance Tuning nav link (it should stack with the invalid boosts icon).
Meta engine schema conflicts
- [Enterprise Search] Start a platinum trial (can be done in /ent/select). Then create a meta engine with the sample engine and your new engine as sources
- [Kibana] Click into the meta engine/refresh.
- Confirm you see the "META ENGINE" badge under the engine name
- Confirm you see a yellow alert icon in the Schema nav link.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

- useEffect init - redirect/data loading - setQueuedErrorMessage helper

- logic values - icon alerts - update tests w/ real conditional testing + remove use of mount, mount combined with shallow_useeffect leads to weirdness

...ck/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx

cee-chen · 2020-11-11T01:33:52Z

...ck/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx

+          <EuiFlexGroup justifyContent="spaceBetween" gutterSize="none">
+            <EuiFlexItem>{RELEVANCE_TUNING_TITLE}</EuiFlexItem>
+            <EuiFlexItem className="appSearchNavIcons">


I debated pulling this out into a separate component called EngineNavAlerts or something, but eventually decided against it for now - it felt like a pre-optimization and tests felt easier to write and more clear w/o it. We can definitely do so if we start adding more engine nav icons in the future though.

cee-chen · 2020-11-11T01:34:35Z

x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts

@@ -107,4 +107,30 @@ describe('engine routes', () => {
      });
    });
  });
+
+  describe('GET /api/app_search/engines/{name}', () => {


I thought this made sense in the same file as the collective/plural /api/app_search/engines, but lmk if you disagree!

cee-chen · 2020-11-11T01:37:21Z

...ck/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx

+                />
+              )}
+              {unsearchedUnconfirmedFields && (
+                <EuiIcon


FYI - I don't super love the screen reader effect of these icons since it reads out (e.g.) "Relevance Tuning Invalid Boosts" all together without much pause, but I'm not 100% sure how to fix this without a lot more code. Happy to investigate later as tech debt though!

cee-chen · 2020-11-11T01:38:15Z

...ugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx

-    const label = wrapper.find('[data-test-subj="EngineLabel"]').last();
-    expect(label.text()).toEqual(expect.stringContaining('SOME-ENGINE'));
+    setMockValues({ ...values, isSampleEngine: true });
+    wrapper.setProps({}); // Re-render


I have a PR coming immediately after this one that creates a new rerender(wrapper); test helper so we don't need all these clarification comments.

...ugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx

JasonStoltz · 2020-11-11T15:50:32Z

[Enterprise Search] Create a brand new engine. Populate it with documents from the paste sample JSON prompt
[Kibana] Click into the new engine/refresh
Confirm you see a "Crawler" nav link (not present in sample or meta engines)
Confirm you see a blue info icon next to the Schema nav link. Hovering over the icon should reveal the title "New unconfirmed fields"

When I create a new engine and populate it with documents... i see neither of these things. I see neither a Crawler link nor a Schema icon

JasonStoltz · 2020-11-11T15:53:45Z

Confirm you see a new yellow alert icon in the Relevance Tuning nav link (it should stack with the invalid boosts icon).

Shouldn't this be next the the "Schema" nav item, not the "Relevance tuning" tab?

JasonStoltz

Just a couple of QA related concerns. Otherwise, I really like how this turned out, it looks and works great.

cee-chen · 2020-11-11T17:04:46Z

Aside: I wish GitHub let you thread remarks not just on code comments

Crawler link

Shoot sorry - I forgot to mention you need to have started ent-search locally with CRAWLER=true bin/enterprise-search locally to have the server send back the correct permission. I've been starting my local dev that way for a while since I've been helping Byron w/ code review so that totally slipped my mind, apologies.

Schema info icon

Make sure your local ent-search is on latest master. It should at the very minimum have https://github.com/elastic/ent-search/pull/2457 merged in

Shouldn't this be next the the "Schema" nav item, not the "Relevance tuning" tab?

No, this was moved in https://github.com/elastic/ent-search/pull/2457 as a UX enhancement - check out the screencaps in that thread for more context on the new callouts being shown. The reason is that the place to actually fix unsearched fields is in Relevance Tuning, not in Schema, so the icon should be shown over Relevance Tuning. That being said, Schema should still show an info icon and warning callout as well.

kibanamachine · 2020-11-11T18:47:06Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 2297900

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`enterpriseSearch`	463	464	+1

Async chunks

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

id	before	after	diff
`enterpriseSearch`	691.6KB	695.7KB	+4.1KB

History

💚 Build #87263 succeeded 9fc3ef8

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

JasonStoltz · 2020-11-11T19:50:44Z

Ah ok, I was able to confirm now, finally.

…) (#83207) * Add server-side route * Add EngineRouter logic - useEffect init - redirect/data loading - setQueuedErrorMessage helper * Add EngineNav logic - logic values - icon alerts - update tests w/ real conditional testing + remove use of mount, mount combined with shallow_useeffect leads to weirdness * Split out EngineRouter and EngineNav into separate files * [PR feedback] More explicit test cases

…ts-public * upstream/master: (57 commits) Remove unused asciidoc file (elastic#83228) [Lens] Remove background from lens embeddable (elastic#83061) [Discover] Unskip flaky tests based on discover fixture index pattern (elastic#82991) Removing unnecessary trailing slash in CODEOWNERS Trying to fix CODEOWNERS again, where was a non-existent team prior (elastic#83236) Trying to fix CODEOWERS, missing a starting slash (elastic#83233) skip flaky suite (elastic#83231) Add enzyme rerender test helper (elastic#83208) Move Elasticsearch type definitions out of APM (elastic#83081) [ts/checkTsProjects] produce a more useful error message (elastic#83209) [kbnClient] retry updating config if necessary (elastic#83205) I accidentally removed this line in a recent PR (elastic#83201) Don't make the caller do work the function can do (elastic#83180) [App Search] Update EngineRouter & EngineNav to use EngineLogic (elastic#83138) [Workplace Search] Add routes for Sources (elastic#83125) Update logstash pipeline management to use system index APIs (elastic#80405) [ML] Replace EuiBasicTable with EuiInMemoryTable (elastic#83057) [Metrics UI] Add basic interaction and shell for node details overlay (elastic#82013) [App Search] Added the log retention confirmation modal to the Settings page (elastic#83009) [docs] Fix create map title in import geospatial page (elastic#83172) ...

cee-chen added 3 commits November 10, 2020 16:58

Add server-side route

3d9725f

Add EngineRouter logic

12d7805

- useEffect init - redirect/data loading - setQueuedErrorMessage helper

Add EngineNav logic

9fc3ef8

- logic values - icon alerts - update tests w/ real conditional testing + remove use of mount, mount combined with shallow_useeffect leads to weirdness

cee-chen added Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Nov 11, 2020

cee-chen requested a review from JasonStoltz November 11, 2020 01:26

cee-chen commented Nov 11, 2020

View reviewed changes

...ck/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx Outdated Show resolved Hide resolved

cee-chen commented Nov 11, 2020

View reviewed changes

JasonStoltz reviewed Nov 11, 2020

View reviewed changes

...ugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx Outdated Show resolved Hide resolved

JasonStoltz requested changes Nov 11, 2020

View reviewed changes

cee-chen added 3 commits November 11, 2020 08:43

Split out EngineRouter and EngineNav into separate files

2556da7

[PR feedback] More explicit test cases

e10cc76

Merge remote-tracking branch 'upstream/master' into engine-logic

2297900

JasonStoltz approved these changes Nov 11, 2020

View reviewed changes

cee-chen merged commit e607078 into elastic:master Nov 11, 2020

cee-chen deleted the engine-logic branch November 11, 2020 19:58

cee-chen mentioned this pull request Nov 11, 2020

[7.x] [App Search] Update EngineRouter & EngineNav to use EngineLogic (#83138) #83207

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[App Search] Update EngineRouter & EngineNav to use EngineLogic #83138

[App Search] Update EngineRouter & EngineNav to use EngineLogic #83138

cee-chen commented Nov 11, 2020 •

edited

Loading

cee-chen Nov 11, 2020

cee-chen Nov 11, 2020

cee-chen Nov 11, 2020

cee-chen Nov 11, 2020

JasonStoltz commented Nov 11, 2020

JasonStoltz commented Nov 11, 2020 •

edited

Loading

JasonStoltz left a comment •

edited

Loading

cee-chen commented Nov 11, 2020 •

edited

Loading

kibanamachine commented Nov 11, 2020

JasonStoltz commented Nov 11, 2020

[App Search] Update EngineRouter & EngineNav to use EngineLogic #83138

[App Search] Update EngineRouter & EngineNav to use EngineLogic #83138

Conversation

cee-chen commented Nov 11, 2020 • edited Loading

Summary

Screencaps

QA

Router tests

Engine nav tests

Checklist

cee-chen Nov 11, 2020

Choose a reason for hiding this comment

cee-chen Nov 11, 2020

Choose a reason for hiding this comment

cee-chen Nov 11, 2020

Choose a reason for hiding this comment

cee-chen Nov 11, 2020

Choose a reason for hiding this comment

JasonStoltz commented Nov 11, 2020

JasonStoltz commented Nov 11, 2020 • edited Loading

JasonStoltz left a comment • edited Loading

Choose a reason for hiding this comment

cee-chen commented Nov 11, 2020 • edited Loading

kibanamachine commented Nov 11, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

JasonStoltz commented Nov 11, 2020

cee-chen commented Nov 11, 2020 •

edited

Loading

JasonStoltz commented Nov 11, 2020 •

edited

Loading

JasonStoltz left a comment •

edited

Loading

cee-chen commented Nov 11, 2020 •

edited

Loading