[Enterprise Search] Convert our `public_url` route to `config_data` and collect initialAppData #75616

cee-chen · 2020-08-20T23:06:43Z

Summary

TL;DR of changes (or, follow along by commit):

Removes public_url API route in favor of new config_data route
Updates callEnterpriseSearchConfigAPI to account for new (expected) data
Passes down this fetched initial data as props to the WS & AS apps
Moves Kea <Provider> to a top-level wrapper and correctly resets state between AS<->WS navigation
Adds a very basic/initial AppLogic store in AS, mostly to confirm that Kea works as expected

QA

Go to http://localhost:5601/api/enterprise_search/config_data and confirm the new expected data loads (w/ falsey fallbacks, since that data doesn't exist yet)
Add a console.log('hasInitialized', hasInitialized); to public/applications/app_search/index.tsx (line 55 or so) and confirm that it updates to true and stays true when navigating within App Search pages

Checklist

Unit or functional tests were updated or added to match the most common scenarios

cee-chen · 2020-08-20T23:08:00Z

x-pack/plugins/enterprise_search/common/types/workplace_search.ts

+export interface IAccount {
+  id: string;
+  groups: string[];
+  isAdmin: boolean;
+  isCurated: boolean;
+  canCreatePersonalSources: boolean;
+  viewedOnboardingPage: boolean;
+}
+
+export interface IOrganization {
+  name: string;
+  defaultOrgName: string;
+}


@scottybollinger FYI that I moved Workplace Search's IAccount/IOrganization types here (since both public/ and server/ files use it).

Also note that I deleted IUser, since we're no longer showing current user data (Kibana does that for us)

cee-chen · 2020-08-20T23:10:09Z

...se_search/public/applications/workplace_search/components/overview/onboarding_steps.test.tsx

@@ -24,7 +24,6 @@ const account = {
  isAdmin: true,
  canCreatePersonalSources: true,
  groups: [],
-  supportEligible: true,


@scottybollinger Another heads up that I also deleted this, it doesn't seem to be in the initial AppLogic API spec and I don't see it used anywhere in ent-search either

cee-chen · 2020-08-20T23:11:00Z

x-pack/plugins/enterprise_search/public/applications/workplace_search/index.tsx

-import { Provider } from 'react-redux';
-import { Store } from 'redux';
-import { getContext, resetContext } from 'kea';
-
-resetContext({ createStore: true });
-
-const store = getContext().store as Store;


Another heads up to WS that this was moved to the top-level public/applications/index.tsx

- DRYs out repeated code - This will be used by an upcoming server/ endpoint change, hence why it's in common

- In preparation for upcoming server logic that will need to reuse these types + DRY out and clean up workplace_search types - remove unused supportEligible - remove currentUser - unneeded in Kibana

…data

… API from public/plugin + set returned initialData in this.data

- resetContext at the top level only gets called once total on first plugin load and never after, causing navigating between WS and AS to crash when both have Kea - this fixes the issue - moves redux Provider to top level app as well

… constant

kibanamachine · 2020-08-21T01:06:55Z

💚 Build Succeeded

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

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
enterpriseSearch	197	+1	196

async chunks size

id	value	diff	baseline
enterpriseSearch	342.5KB	-16.9KB	359.4KB

page load bundle size

id	value	diff	baseline
enterpriseSearch	21.8KB	+561.0B	21.2KB

History

💚 Build #69569 succeeded 9e96af7

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

JasonStoltz

This all seems reasonable to me. I have no concerns.

JasonStoltz · 2020-08-21T12:50:26Z

x-pack/plugins/enterprise_search/public/applications/__mocks__/kea.mock.ts

+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+jest.mock('kea', () => ({


Good call making this a common mock

JasonStoltz · 2020-08-21T12:52:07Z

x-pack/plugins/enterprise_search/public/applications/__mocks__/shallow_usecontext.mock.ts

@@ -14,6 +14,7 @@ import { mockLicenseContext } from './license_context.mock';
 jest.mock('react', () => ({
  ...(jest.requireActual('react') as object),
  useContext: jest.fn(() => ({ ...mockKibanaContext, ...mockLicenseContext })),
+  useEffect: jest.fn((fn) => fn()), // Calls on mount/every update - use mount for more complex behavior


Could you explain why you are mocking useEffect for shallow rendering?

So that these lines in app_search/index.tsx run in the test:

kibana/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx

Lines 52 to 54 in f4d815e

useEffect(() => {

if (!hasInitialized) initializeAppData(props);

}, [hasInitialized]);

Unfortunately Enzyme doesn't yet call useEffect() in shallow() so we have to kind of stub it ourselves. It's not perfect however since it ignores the [changeOnThisVar] array that you can pass into useEffect - if you want to really granularly test that, you'll have to use mount().

It's OK however in this case because I just want to check that initializeAppData was called on mount.

cee-chen · 2020-08-21T16:01:55Z

@scottybollinger - I know you're out today so apologies if I'm merging anything too quickly with changes that WS didn't want - feel free to let me know next week and I can revert or make changes as needed!

…nd collect initialAppData (#75616) (#75667) * [Setup] DRY out stripTrailingSlash helper - DRYs out repeated code - This will be used by an upcoming server/ endpoint change, hence why it's in common * [Setup] DRY out initial app data types to common/types - In preparation for upcoming server logic that will need to reuse these types + DRY out and clean up workplace_search types - remove unused supportEligible - remove currentUser - unneeded in Kibana * Update callEnterpriseSearchConfigAPI to parse and fetch new expected data * Remove /public_url API for /config_data * Remove getPublicUrl in favor of directly calling the new /config_data API from public/plugin + set returned initialData in this.data * Set up product apps to be passed initial data as props * Fix for Kea/redux state not resetting between AS<->WS nav - resetContext at the top level only gets called once total on first plugin load and never after, causing navigating between WS and AS to crash when both have Kea - this fixes the issue - moves redux Provider to top level app as well * Add very basic Kea logic file to App Search * Finish AppSearchConfigured tests & set up kea+useEffect mocks * [Cleanup] DRY out repeated mock initialAppData to a reusable defaults constant

* master: (71 commits) [Lens] Show 'No data for this field' for empty field in accordion (elastic#73772) Skip failing lens test Configure ScopedHistory consistenty regardless of URL used to mount app (elastic#75074) Fix returned payload by "search" usage collector (elastic#75340) [Security Solution] Fix missing key error (elastic#75576) Upgrade EUI to v27.4.1 (elastic#75240) Update datasets UI copy to data streams (elastic#75618) [Lens] Register saved object references (elastic#74523) [DOCS] Update links to Beats documentation (elastic#70380) [Enterprise Search] Convert our `public_url` route to `config_data` and collect initialAppData (elastic#75616) [Usage Collection Schemas] Remove Legacy entries (elastic#75652) [Dashboard First] Lens Originating App Breadcrumb (elastic#75470) Improve login UI error message. (elastic#75642) [Security Solution] modify circular deps checker to output images of circular deps graphs (elastic#75579) [Data Telemetry] Add index pattern to identify "meow" attacks (elastic#75163) Migrate CSP usage collector to `kibana_usage_collection` plugin (elastic#75536) [Console] Get ES Config from core (elastic#75406) [Uptime] Add delay in telemetry test (elastic#75162) [Lens] Use index pattern service instead saved object client (elastic#74654) Embeddable input (elastic#73033) ...

scottybollinger · 2020-08-24T17:30:20Z

x-pack/plugins/enterprise_search/common/types/index.ts

+export interface IInitialAppData {
+  readOnlyMode?: boolean;
+  ilmEnabled?: boolean;
+  configuredLimits?: IConfiguredLimits;


Why is this top-level and not nested under appSearch?

I believe Oleksiy mentioned that Workplace Search either uses configurable limits as well (possibly in the back-end) or may, in the future, add more configurable limits to the front-end.

cee-chen added Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Aug 20, 2020

cee-chen requested review from JasonStoltz and scottybollinger August 20, 2020 23:06

cee-chen commented Aug 20, 2020

View reviewed changes

cee-chen added 10 commits August 20, 2020 16:20

[Setup] DRY out stripTrailingSlash helper

6ff2100

- DRYs out repeated code - This will be used by an upcoming server/ endpoint change, hence why it's in common

[Setup] DRY out initial app data types to common/types

f784009

- In preparation for upcoming server logic that will need to reuse these types + DRY out and clean up workplace_search types - remove unused supportEligible - remove currentUser - unneeded in Kibana

Update callEnterpriseSearchConfigAPI to parse and fetch new expected …

7081be9

…data

Remove /public_url API for /config_data

181890b

Remove getPublicUrl in favor of directly calling the new /config_data…

7cd2140

… API from public/plugin + set returned initialData in this.data

Set up product apps to be passed initial data as props

4c836ac

Add very basic Kea logic file to App Search

1f77660

Finish AppSearchConfigured tests & set up kea+useEffect mocks

46990ef

[Cleanup] DRY out repeated mock initialAppData to a reusable defaults…

f4d815e

… constant

cee-chen force-pushed the public-url-to-config-data branch from 9e96af7 to f4d815e Compare August 20, 2020 23:20

JasonStoltz approved these changes Aug 21, 2020

View reviewed changes

cee-chen merged commit 172c464 into elastic:master Aug 21, 2020

cee-chen deleted the public-url-to-config-data branch August 21, 2020 16:02

cee-chen mentioned this pull request Aug 21, 2020

[7.x] [Enterprise Search] Convert our public_url route to config_data and collect initialAppData (#75616) #75667

Merged

scottybollinger reviewed Aug 24, 2020

View reviewed changes

cee-chen mentioned this pull request Sep 8, 2020

[Enterprise Search] Update config data endpoint to v2 #76970

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enterprise Search] Convert our `public_url` route to `config_data` and collect initialAppData #75616

[Enterprise Search] Convert our `public_url` route to `config_data` and collect initialAppData #75616

cee-chen commented Aug 20, 2020

cee-chen Aug 20, 2020

cee-chen Aug 20, 2020

cee-chen Aug 20, 2020

kibanamachine commented Aug 21, 2020

JasonStoltz left a comment

JasonStoltz Aug 21, 2020

JasonStoltz Aug 21, 2020

cee-chen Aug 21, 2020

cee-chen commented Aug 21, 2020

scottybollinger Aug 24, 2020

cee-chen Aug 24, 2020

	useEffect(() => {
	if (!hasInitialized) initializeAppData(props);
	}, [hasInitialized]);

[Enterprise Search] Convert our public_url route to config_data and collect initialAppData #75616

[Enterprise Search] Convert our public_url route to config_data and collect initialAppData #75616

Conversation

cee-chen commented Aug 20, 2020

Summary

QA

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Aug 21, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

async chunks size

page load bundle size

History

JasonStoltz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cee-chen commented Aug 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

[Enterprise Search] Convert our `public_url` route to `config_data` and collect initialAppData #75616

[Enterprise Search] Convert our `public_url` route to `config_data` and collect initialAppData #75616