From 62578bdcf4adc9c3ed798f399193efbb2a0539cf Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Fri, 26 Aug 2022 15:25:51 +0200 Subject: [PATCH] [ML] Fix navigation for the Basic licence (#139469) * fix redirect to the data vis * update jest tests * remove window.location.href from the clone action * assert the redirect * update power user tests * use plugin id const, return promise reject --- x-pack/plugins/ml/public/application/app.tsx | 2 +- .../jobs/jobs_list/components/utils.js | 6 ++-- .../application/license/check_license.tsx | 8 +++-- .../license/ml_client_license.test.ts | 7 ++-- .../application/license/ml_client_license.ts | 35 +++++++++++-------- .../apps/ml/permissions/full_ml_access.ts | 7 +++- .../apps/ml/permissions/read_ml_access.ts | 7 +++- 7 files changed, 49 insertions(+), 23 deletions(-) diff --git a/x-pack/plugins/ml/public/application/app.tsx b/x-pack/plugins/ml/public/application/app.tsx index 53c50893bcdfb..8e9a7d67d10ac 100644 --- a/x-pack/plugins/ml/public/application/app.tsx +++ b/x-pack/plugins/ml/public/application/app.tsx @@ -146,7 +146,7 @@ export const renderApp = ( appMountParams.onAppLeave((actions) => actions.default()); - const mlLicense = setLicenseCache(deps.licensing, [ + const mlLicense = setLicenseCache(deps.licensing, coreStart.application, [ () => ReactDOM.render( , diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js index 364cdd1be55db..430244c52e69c 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js @@ -13,7 +13,7 @@ import { getToastNotificationService, toastNotificationServiceProvider, } from '../../../services/toast_notification_service'; -import { getToastNotifications } from '../../../util/dependency_cache'; +import { getApplication, getToastNotifications } from '../../../util/dependency_cache'; import { ml } from '../../../services/ml_api_service'; import { stringMatch } from '../../../util/string_utils'; import { getDataViewNames } from '../../../util/index_utils'; @@ -22,6 +22,8 @@ import { JOB_ACTION } from '../../../../../common/constants/job_actions'; import { parseInterval } from '../../../../../common/util/parse_interval'; import { mlCalendarService } from '../../../services/calendar_service'; import { isPopulatedObject } from '@kbn/ml-is-populated-object'; +import { ML_PAGES } from '../../../../../common/constants/locator'; +import { PLUGIN_ID } from '../../../../../common/constants/app'; export function loadFullJob(jobId) { return new Promise((resolve, reject) => { @@ -287,7 +289,7 @@ export async function cloneJob(jobId) { ); } - window.location.href = '#/jobs/new_job'; + getApplication().navigateToApp(PLUGIN_ID, { path: ML_PAGES.ANOMALY_DETECTION_CREATE_JOB }); } catch (error) { getToastNotificationService().displayErrorToast( error, diff --git a/x-pack/plugins/ml/public/application/license/check_license.tsx b/x-pack/plugins/ml/public/application/license/check_license.tsx index 569746a1dab27..9436614effe3c 100644 --- a/x-pack/plugins/ml/public/application/license/check_license.tsx +++ b/x-pack/plugins/ml/public/application/license/check_license.tsx @@ -5,7 +5,8 @@ * 2.0. */ -import { LicensingPluginSetup } from '@kbn/licensing-plugin/public'; +import type { LicensingPluginSetup } from '@kbn/licensing-plugin/public'; +import type { CoreStart } from '@kbn/core/public'; import { MlLicense } from '../../../common/license'; import { MlClientLicense } from './ml_client_license'; @@ -16,13 +17,16 @@ let mlLicense: MlClientLicense | null = null; * * @export * @param {LicensingPluginSetup} licensingSetup + * @param application + * @param postInitFunctions * @returns {MlClientLicense} */ export function setLicenseCache( licensingSetup: LicensingPluginSetup, + application: CoreStart['application'], postInitFunctions?: Array<(lic: MlLicense) => void> ) { - mlLicense = new MlClientLicense(); + mlLicense = new MlClientLicense(application); mlLicense.setup(licensingSetup.license$, postInitFunctions); return mlLicense; } diff --git a/x-pack/plugins/ml/public/application/license/ml_client_license.test.ts b/x-pack/plugins/ml/public/application/license/ml_client_license.test.ts index 02a38f152a3ea..ea7aa5d1a4b99 100644 --- a/x-pack/plugins/ml/public/application/license/ml_client_license.test.ts +++ b/x-pack/plugins/ml/public/application/license/ml_client_license.test.ts @@ -9,10 +9,13 @@ import { Observable, Subject } from 'rxjs'; import { ILicense } from '@kbn/licensing-plugin/common/types'; import { MlClientLicense } from './ml_client_license'; +import { applicationServiceMock } from '@kbn/core/public/application/application_service.mock'; describe('MlClientLicense', () => { + const startApplicationContractMock = applicationServiceMock.createStartContract(); + test('should miss the license update when initialized without postInitFunction', () => { - const mlLicense = new MlClientLicense(); + const mlLicense = new MlClientLicense(startApplicationContractMock); // upon instantiation the full license doesn't get set expect(mlLicense.isFullLicense()).toBe(false); @@ -35,7 +38,7 @@ describe('MlClientLicense', () => { }); test('should not miss the license update when initialized with postInitFunction', (done) => { - const mlLicense = new MlClientLicense(); + const mlLicense = new MlClientLicense(startApplicationContractMock); // upon instantiation the full license doesn't get set expect(mlLicense.isFullLicense()).toBe(false); diff --git a/x-pack/plugins/ml/public/application/license/ml_client_license.ts b/x-pack/plugins/ml/public/application/license/ml_client_license.ts index 055ad7b641398..37488b8448aef 100644 --- a/x-pack/plugins/ml/public/application/license/ml_client_license.ts +++ b/x-pack/plugins/ml/public/application/license/ml_client_license.ts @@ -5,19 +5,36 @@ * 2.0. */ +import type { CoreStart } from '@kbn/core/public'; +import { ML_PAGES } from '../../../common/constants/locator'; import { MlLicense } from '../../../common/license'; import { showExpiredLicenseWarning } from './expired_warning'; +import { PLUGIN_ID } from '../../../common/constants/app'; export class MlClientLicense extends MlLicense { - fullLicenseResolver() { + constructor(private application: CoreStart['application']) { + super(); + } + + private redirectToKibana() { + this.application.navigateToApp('home'); + return Promise.reject(); + } + + private redirectToBasic() { + this.application.navigateToApp(PLUGIN_ID, { path: ML_PAGES.DATA_VISUALIZER }); + return Promise.reject(); + } + + fullLicenseResolver(): Promise { if (this.isMlEnabled() === false || this.isMinimumLicense() === false) { // ML is not enabled or the license isn't at least basic - return redirectToKibana(); + return this.redirectToKibana(); } if (this.isFullLicense() === false) { // ML is enabled, but only with a basic or gold license - return redirectToBasic(); + return this.redirectToBasic(); } // ML is enabled @@ -30,7 +47,7 @@ export class MlClientLicense extends MlLicense { basicLicenseResolver() { if (this.isMlEnabled() === false || this.isMinimumLicense() === false) { // ML is not enabled or the license isn't at least basic - return redirectToKibana(); + return this.redirectToKibana(); } // ML is enabled @@ -40,13 +57,3 @@ export class MlClientLicense extends MlLicense { return Promise.resolve(); } } - -function redirectToKibana() { - window.location.href = '/'; - return Promise.reject(); -} - -function redirectToBasic() { - window.location.href = '#/datavisualizer'; - return Promise.reject(); -} diff --git a/x-pack/test/functional_basic/apps/ml/permissions/full_ml_access.ts b/x-pack/test/functional_basic/apps/ml/permissions/full_ml_access.ts index b90b97ca87a57..0ff4eca82a5cf 100644 --- a/x-pack/test/functional_basic/apps/ml/permissions/full_ml_access.ts +++ b/x-pack/test/functional_basic/apps/ml/permissions/full_ml_access.ts @@ -5,13 +5,14 @@ * 2.0. */ +import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../ftr_provider_context'; - import { USER } from '../../../../functional/services/ml/security_common'; export default function ({ getService }: FtrProviderContext) { const esArchiver = getService('esArchiver'); const ml = getService('ml'); + const browser = getService('browser'); const testUsers = [ { user: USER.ML_POWERUSER, discoverAvailable: true }, @@ -57,6 +58,10 @@ export default function ({ getService }: FtrProviderContext) { await ml.testExecution.logTestStep('should load the ML app'); await ml.navigation.navigateToMl(); + await ml.testExecution.logTestStep('should redirect to the "Data Visualizer" page'); + const browserURl = await browser.getCurrentUrl(); + expect(browserURl).to.contain('/ml/datavisualizer'); + await ml.testExecution.logTestStep('should display the disabled "Overview" tab'); await ml.navigation.assertOverviewTabEnabled(false); diff --git a/x-pack/test/functional_basic/apps/ml/permissions/read_ml_access.ts b/x-pack/test/functional_basic/apps/ml/permissions/read_ml_access.ts index fc293defceb86..c7a3f13d23ed7 100644 --- a/x-pack/test/functional_basic/apps/ml/permissions/read_ml_access.ts +++ b/x-pack/test/functional_basic/apps/ml/permissions/read_ml_access.ts @@ -5,13 +5,14 @@ * 2.0. */ +import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../ftr_provider_context'; - import { USER } from '../../../../functional/services/ml/security_common'; export default function ({ getService }: FtrProviderContext) { const esArchiver = getService('esArchiver'); const ml = getService('ml'); + const browser = getService('browser'); const testUsers = [ { user: USER.ML_VIEWER, discoverAvailable: true }, @@ -57,6 +58,10 @@ export default function ({ getService }: FtrProviderContext) { await ml.testExecution.logTestStep('should load the ML app'); await ml.navigation.navigateToMl(); + await ml.testExecution.logTestStep('should redirect to the "Data Visualizer" page'); + const browserURl = await browser.getCurrentUrl(); + expect(browserURl).to.contain('/ml/datavisualizer'); + await ml.testExecution.logTestStep('should display the disabled "Overview" tab'); await ml.navigation.assertOverviewTabEnabled(false);