[Enterprise Search] Move LicenseContext to Kea #78231

cee-chen · 2020-09-22T22:56:19Z

Summary

Follow up to #78167 - part 2 electric boogaloo of our grand adventure moving all our old React Context to Kea logic.

This PR converts our old LicenseContext/<LicenseProvider /> to a new shiny LicensingLogic. I got to learn a few new things about Observables while doing, and I think our tests are more robust than before - fun times!

As always, I recommend following along by commit. As a heads up, the first commit (450c6b7) is a bit of a separate tech-debt/refactor - I noticed while working on this that the license we were using was technically deprecated and that we were supposed to calling license$ from PluginsStart, not PluginsSetup. That commit addresses that + cleans up the (getting hard to read) number of args we were passing to renderApp.

QA/Regression testing

Confirm that platinum/trial licenses can view meta engines as before
Confirm that basic licenses do not see meta engines or the support.elastic.co URL

Checklist

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

- I noticed my IDE complaining that we were using LicensingPluginSetup (deprecated) instead of LicensingPluginStart, and decided to factor plugin.ts to DRY out / ensure all the dependencies we were passing on app mount were start services and not setup - The number of args we were passing to renderApp was getting a little ridiculous, so I created small helpers to group them up by type (Kibana's args (dependencies/services) vs our plugin's args (data, config, etc.) + bonus remove unused CoreStart type/arg

cee-chen

As always, I recommend following along by commit. As a heads up, the first commit (450c6b7) is a bit of a separate tech-debt/refactor - I noticed while working on this that the license we were using was technically deprecated and that we were supposed to calling license$ from PluginsStart, not PluginsSetup. That commit addresses that + cleans up the (getting hard to read) number of args we were passing to renderApp.

cee-chen · 2020-09-22T22:59:34Z

x-pack/plugins/enterprise_search/public/plugin.ts

+ private getPluginData() {
+ // Small helper for grouping plugin data related args together
+ return { config: this.config, data: this.data };
+ }


This seems vaguely useless right now, but as a heads up I'll be adding another this.store to this in the near-ish future (required change in order for our main app and our header menu actions to be able to share logic).

cee-chen · 2020-09-22T23:00:18Z

x-pack/plugins/enterprise_search/public/plugin.ts


 const { renderApp } = await import('./applications');
 const { EnterpriseSearch } = await import('./applications/enterprise_search');

- return renderApp(EnterpriseSearch, params, coreStart, plugins, this.config, this.data);
+ return renderApp(EnterpriseSearch, kibanaDeps, pluginData);


Hopefully this is now easier to read/conceptualize than the old row of args that got away from me, haha.

cee-chen · 2020-09-22T23:02:41Z

x-pack/plugins/enterprise_search/public/plugin.ts

@@ -17,7 +16,7 @@ import {
 FeatureCatalogueCategory,
 HomePublicPluginSetup,
 } from '../../../../src/plugins/home/public';
-import { LicensingPluginSetup } from '../../licensing/public';
+import { LicensingPluginStart } from '../../licensing/public';


Extra context for this change:

kibana/x-pack/plugins/licensing/public/types.ts

Lines 12 to 17 in 5f781dc

export interface LicensingPluginSetup {

/**

* Steam of licensing information {@link ILicense}.

* @deprecated in favour of the counterpart provided from start contract

*/

license$: Observable<ILicense>;

I noticed it because VSCode was showing the old license$ with a strikethrough through it because of the @deprecated. Pretty cool!

cee-chen · 2020-09-22T23:03:24Z

x-pack/plugins/enterprise_search/public/applications/shared/licensing/licensing_logic.test.ts

+
+describe('LicensingLogic', () => {
+ const mockLicense = licensingMock.createLicense();
+ const mockLicense$ = new BehaviorSubject(mockLicense);


TIL I (sorta) learned how to mock observables

EDIT: TIL how to be redundant when saying TIL

cee-chen · 2020-09-22T23:04:12Z

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

+ <Router history={params.history}>
+ <App {...initialData} />
+ </Router>
+ </Provider>
 </KibanaContext.Provider>


Just one more wrapper to go! 🔥

- replaces useObservable with a manual subscription that updates the license value/state - moves hasXLicense checks to selectors vs helper functions

- Add mockLicensingValues to basic kea mock - Minor comment update to mockAllValues obj that I forgot to add in a previous PR

kibanamachine · 2020-09-23T01:09:02Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 6fa43f9

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
enterpriseSearch	271	-4	275

async chunks size

id	value	diff	baseline
enterpriseSearch	503.6KB	-6.6KB	510.2KB

page load bundle size

id	value	diff	baseline
enterpriseSearch	23.9KB	+742.0B	23.2KB

History

💚 Build #76374 succeeded 4f3d0db

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

scottybollinger

This is 🔥

byronhulcher · 2020-09-23T18:09:46Z

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

 </KibanaContext.Provider>
 </I18nProvider>,
 params.element
 );
 return () => {
 ReactDOM.unmountComponentAtNode(params.element);
+ unmountLicensingLogic();


If this list gets much longer I might try to think of a better solution than losting all of these inputs.

const configs = [ LicenseLogic: {<LicenseLogic props>}, HttpLogic: {<HttpLogic props}, etc ] const unmount = Object.entries(config).map((Logic, props) => { Logic(props); return Logic.mount() }) return () => { ReactDOM.unmountComponentAtNode(params.element); unmounts.forEach(unmount => unmount()) }

then we'd never forget to unmount!

For sure! FWIW, it stops at 4 total (by the end of my series of PRs) but I can definitely see a case for pulling all our mounts out to a separate file/helper and returning an array of unmounts. I think 4 is just on the border to not a completely warrant a separate helper, but let me know if you disagree 🤔

byronhulcher

LGTM

* Fix licensing to use start service + refactor - I noticed my IDE complaining that we were using LicensingPluginSetup (deprecated) instead of LicensingPluginStart, and decided to factor plugin.ts to DRY out / ensure all the dependencies we were passing on app mount were start services and not setup - The number of args we were passing to renderApp was getting a little ridiculous, so I created small helpers to group them up by type (Kibana's args (dependencies/services) vs our plugin's args (data, config, etc.) + bonus remove unused CoreStart type/arg * Add LicensingLogic + mount - replaces useObservable with a manual subscription that updates the license value/state - moves hasXLicense checks to selectors vs helper functions * Update components w/ license checks to use LicensingLogic * Update tests for components now calling LicensingLogic - Add mockLicensingValues to basic kea mock - Minor comment update to mockAllValues obj that I forgot to add in a previous PR * 🔥 Remove old LicensingContext

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

cee-chen requested review from a team September 22, 2020 22:56

cee-chen commented Sep 22, 2020

View reviewed changes

cee-chen added 4 commits September 22, 2020 16:33

Add LicensingLogic + mount

169d8bb

- replaces useObservable with a manual subscription that updates the license value/state - moves hasXLicense checks to selectors vs helper functions

Update components w/ license checks to use LicensingLogic

06293f0

Update tests for components now calling LicensingLogic

d4dd185

- Add mockLicensingValues to basic kea mock - Minor comment update to mockAllValues obj that I forgot to add in a previous PR

🔥 Remove old LicensingContext

6fa43f9

cee-chen force-pushed the context-to-kea branch from 4f3d0db to 6fa43f9 Compare September 22, 2020 23:33

scottybollinger approved these changes Sep 23, 2020

View reviewed changes

byronhulcher reviewed Sep 23, 2020

View reviewed changes

byronhulcher approved these changes Sep 23, 2020

View reviewed changes

cee-chen merged commit 0db3159 into elastic:master Sep 23, 2020

cee-chen deleted the context-to-kea branch September 23, 2020 18:36

cee-chen mentioned this pull request Sep 23, 2020

[7.x] [Enterprise Search] Move LicenseContext to Kea (#78231) #78335

Merged

cee-chen mentioned this pull request Sep 24, 2020

[Enterprise Search] Move externalUrl helper out of React Context #78368

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enterprise Search] Move LicenseContext to Kea #78231

[Enterprise Search] Move LicenseContext to Kea #78231

cee-chen commented Sep 22, 2020 •

edited

Loading

cee-chen left a comment

cee-chen Sep 22, 2020 •

edited

Loading

cee-chen Sep 22, 2020

cee-chen Sep 22, 2020

cee-chen Sep 22, 2020 •

edited

Loading

cee-chen Sep 22, 2020

kibanamachine commented Sep 23, 2020

scottybollinger left a comment

byronhulcher Sep 23, 2020

cee-chen Sep 23, 2020 •

edited

Loading

byronhulcher left a comment

	export interface LicensingPluginSetup {
	/**
	* Steam of licensing information {@link ILicense}.
	* @deprecated in favour of the counterpart provided from start contract
	*/
	license$: Observable<ILicense>;

[Enterprise Search] Move LicenseContext to Kea #78231

[Enterprise Search] Move LicenseContext to Kea #78231

Conversation

cee-chen commented Sep 22, 2020 • edited Loading

Summary

QA/Regression testing

Checklist

cee-chen left a comment

Choose a reason for hiding this comment

cee-chen Sep 22, 2020 • edited Loading

Choose a reason for hiding this comment

cee-chen Sep 22, 2020

Choose a reason for hiding this comment

cee-chen Sep 22, 2020

Choose a reason for hiding this comment

cee-chen Sep 22, 2020 • edited Loading

Choose a reason for hiding this comment

cee-chen Sep 22, 2020

Choose a reason for hiding this comment

kibanamachine commented Sep 23, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

async chunks size

page load bundle size

History

scottybollinger left a comment

Choose a reason for hiding this comment

byronhulcher Sep 23, 2020

Choose a reason for hiding this comment

cee-chen Sep 23, 2020 • edited Loading

Choose a reason for hiding this comment

byronhulcher left a comment

Choose a reason for hiding this comment

cee-chen commented Sep 22, 2020 •

edited

Loading

cee-chen Sep 22, 2020 •

edited

Loading

cee-chen Sep 22, 2020 •

edited

Loading

cee-chen Sep 23, 2020 •

edited

Loading