use union of strings instead of enum #62493

mshustov · 2020-04-03T17:47:49Z

Summary

Part of #62263
Prevents licensing plugin code leakage into plugin bundles.
Before:

find x-pack/  -type f -name "*[a-zA-Z].plugin.js" -exec du -ch {} + | grep total$
 36M	total

After:

find x-pack/  -type f -name "*[a-zA-Z].plugin.js" -exec du -ch {} + | grep total$
 33M	total

elasticmachine · 2020-04-03T17:47:51Z

Pinging @elastic/kibana-platform (Team:Platform)

spalger

LGTM

mikecote

Alerting changes LGTM

spalger · 2020-04-03T20:45:24Z

@elasticmachine merge upstream

cjcenizal

ES UI apps look good but I have a question about DX.

Checked these apps:

Painless Lab
Remote Clusters
Search Profiler
Snapshot Restore
Watcher

In terms of DX, how are developers expected to discover the valid values of this string union? If I make a typo, the TS checker gives me this error:

      x-pack/plugins/painless_lab/server/services/license.ts:40:34 - error TS2367: This condition will always return 'false' since the types 'LicenseCheckState' and '"vaild"' have no overlap.

      40       const hasRequiredLicense = state === 'vaild';

What is the expected experience for a developer who is trying to fix this, or trying to compare against a different value?

cjcenizal · 2020-04-03T21:25:01Z

x-pack/plugins/painless_lab/server/services/license.ts

@@ -37,7 +37,7 @@ export class License {
  ) {
    licensing.license$.subscribe(license => {
      const { state, message } = license.check(pluginId, minimumLicenseType);
-      const hasRequiredLicense = state === LICENSE_CHECK_STATE.Valid;
+      const hasRequiredLicense = state === 'valid';


cjcenizal · 2020-04-03T21:25:12Z

x-pack/plugins/painless_lab/public/plugin.tsx

@@ -82,7 +81,7 @@ export class PainlessLabUIPlugin implements Plugin<void, void, PluginDependencie
          PLUGIN.id,
          PLUGIN.minimumLicenseType
        );
-        const isValidLicense = state === LICENSE_CHECK_STATE.Valid;
+        const isValidLicense = state === 'valid';


cjcenizal · 2020-04-03T21:29:48Z

x-pack/plugins/remote_clusters/server/plugin.ts

@@ -54,7 +53,7 @@ export class RemoteClustersServerPlugin implements Plugin<void, void, any, any>

    licensing.license$.subscribe(license => {
      const { state, message } = license.check(PLUGIN.getI18nName(), PLUGIN.minimumLicenseType);
-      const hasRequiredLicense = state === LICENSE_CHECK_STATE.Valid;
+      const hasRequiredLicense = state === 'valid';


cjcenizal · 2020-04-03T21:30:12Z

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

@@ -50,7 +49,7 @@ export class SearchProfilerUIPlugin implements Plugin<void, void, AppPublicPlugi
        const license = await licensing.license$.pipe(first()).toPromise();
        const { state, message } = license.check(PLUGIN.id, PLUGIN.minimumLicenseType);
        const initialLicenseStatus =
-          state === LICENSE_CHECK_STATE.Valid ? { valid: true } : { valid: false, message };
+          state === 'valid' ? { valid: true } : { valid: false, message };


cjcenizal · 2020-04-03T21:30:18Z

x-pack/plugins/searchprofiler/server/plugin.ts

@@ -33,7 +31,7 @@ export class SearchProfilerServerPlugin implements Plugin {

    licensing.license$.subscribe(license => {
      const { state, message } = license.check(PLUGIN.id, PLUGIN.minimumLicenseType);
-      const hasRequiredLicense = state === LICENSE_CHECK_STATE.Valid;
+      const hasRequiredLicense = state === 'valid';


cjcenizal · 2020-04-03T21:31:13Z

x-pack/plugins/snapshot_restore/server/services/license.ts

@@ -38,7 +37,7 @@ export class License {
  ) {
    licensing.license$.subscribe(license => {
      const { state, message } = license.check(pluginId, minimumLicenseType);
-      const hasRequiredLicense = state === LICENSE_CHECK_STATE.Valid;
+      const hasRequiredLicense = state === 'valid';


cjcenizal · 2020-04-03T21:31:26Z

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

 import { TimeBuckets } from './legacy';
 import { PLUGIN } from '../common/constants';
 import { Dependencies } from './types';

 const licenseToLicenseStatus = (license: ILicense): LicenseStatus => {
  const { state, message } = license.check(PLUGIN.ID, PLUGIN.MINIMUM_LICENSE_REQUIRED);
  return {
-    valid: state === LICENSE_CHECK_STATE.Valid && license.getFeature(PLUGIN.ID).isAvailable,
+    valid: state === 'valid' && license.getFeature(PLUGIN.ID).isAvailable,


cjcenizal · 2020-04-03T21:31:31Z

x-pack/plugins/watcher/server/plugin.ts

@@ -73,7 +72,7 @@ export class WatcherServerPlugin implements Plugin<void, void, any, any> {

    licensing.license$.subscribe(async license => {
      const { state, message } = license.check(PLUGIN.ID, PLUGIN.MINIMUM_LICENSE_REQUIRED);
-      const hasMinimumLicense = state === LICENSE_CHECK_STATE.Valid;
+      const hasMinimumLicense = state === 'valid';


walterra

ML/transform changes LGTM.

legrego

Security changes LGTM

thomasneirynck

maps/* lgtm, code review

mshustov · 2020-04-06T07:49:44Z

What is the expected experience for a developer who is trying to fix this, or trying to compare against a different value?

@cjcenizal

The error message contains a reference to the correct type LicenseCheckState.
IDE handles a union of strings and provides autocomplete.
Union of strings doesn't require from developer to know what enum to import.

mshustov · 2020-04-06T07:54:11Z

@elasticmachine merge upstream

kibanamachine · 2020-04-06T09:40:18Z

💚 Build Succeeded

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

History

💚 Build #38584 succeeded e915218
💛 Build #38514 was flaky 9e59953

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

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

use union of strings instead of enum

9e59953

mshustov added chore Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Apr 3, 2020

mshustov requested review from a team as code owners April 3, 2020 17:47

spalger approved these changes Apr 3, 2020

View reviewed changes

mikecote approved these changes Apr 3, 2020

View reviewed changes

rudolf approved these changes Apr 3, 2020

View reviewed changes

spalger mentioned this pull request Apr 3, 2020

7.7.0 general performance problems, blocking IE11 #62238

Closed

Merge branch 'master' into issue-62263-no-enum-licensing

e915218

cjcenizal self-requested a review April 3, 2020 21:07

cjcenizal approved these changes Apr 3, 2020

View reviewed changes

walterra approved these changes Apr 4, 2020

View reviewed changes

legrego approved these changes Apr 4, 2020

View reviewed changes

thomasneirynck approved these changes Apr 5, 2020

View reviewed changes

Merge branch 'master' into issue-62263-no-enum-licensing

e17c703

pgayvallet approved these changes Apr 6, 2020

View reviewed changes

mshustov merged commit bdf628d into elastic:master Apr 6, 2020

mshustov deleted the issue-62263-no-enum-licensing branch April 6, 2020 10:31

mshustov added a commit to mshustov/kibana that referenced this pull request Apr 6, 2020

use union of strings instead of enum (elastic#62493)

8a76a61

Co-authored-by: Elastic Machine <[email protected]>

mshustov mentioned this pull request Apr 6, 2020

[7.x] use union of strings instead of enum (#62493) #62581

Merged

mshustov added a commit to mshustov/kibana that referenced this pull request Apr 6, 2020

use union of strings instead of enum (elastic#62493)

76cba73

Co-authored-by: Elastic Machine <[email protected]>

mshustov mentioned this pull request Apr 6, 2020

[7.7] use union of strings instead of enum (#62493) #62582

Merged

mshustov added a commit that referenced this pull request Apr 6, 2020

use union of strings instead of enum (#62493) (#62581)

307338f

Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

mshustov added a commit that referenced this pull request Apr 6, 2020

use union of strings instead of enum (#62493) (#62582)

379af6d

Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use union of strings instead of enum #62493

use union of strings instead of enum #62493

mshustov commented Apr 3, 2020 •

edited

Loading

elasticmachine commented Apr 3, 2020

spalger left a comment

mikecote left a comment

spalger commented Apr 3, 2020

cjcenizal left a comment •

edited

Loading

cjcenizal Apr 3, 2020

cjcenizal Apr 3, 2020

cjcenizal Apr 3, 2020

cjcenizal Apr 3, 2020

cjcenizal Apr 3, 2020

cjcenizal Apr 3, 2020

cjcenizal Apr 3, 2020

cjcenizal Apr 3, 2020

walterra left a comment

legrego left a comment

thomasneirynck left a comment

mshustov commented Apr 6, 2020 •

edited

Loading

mshustov commented Apr 6, 2020

kibanamachine commented Apr 6, 2020

use union of strings instead of enum #62493

use union of strings instead of enum #62493

Conversation

mshustov commented Apr 3, 2020 • edited Loading

Summary

elasticmachine commented Apr 3, 2020

spalger left a comment

Choose a reason for hiding this comment

mikecote left a comment

Choose a reason for hiding this comment

spalger commented Apr 3, 2020

cjcenizal left a comment • edited Loading

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

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

Choose a reason for hiding this comment

walterra left a comment

Choose a reason for hiding this comment

legrego left a comment

Choose a reason for hiding this comment

thomasneirynck left a comment

Choose a reason for hiding this comment

mshustov commented Apr 6, 2020 • edited Loading

mshustov commented Apr 6, 2020

kibanamachine commented Apr 6, 2020

💚 Build Succeeded

History

mshustov commented Apr 3, 2020 •

edited

Loading

cjcenizal left a comment •

edited

Loading

mshustov commented Apr 6, 2020 •

edited

Loading