[ML] Maps integration: add empty state prompt when no supported jobs exist #125878

alvarezmelissa87 · 2022-02-16T22:01:58Z

Summary

Related meta issue: #123492

When there are no ML jobs available for selection in the wizard shows a prompt with a link to ML job management page.

Checklist

Delete any items that are not applicable to this PR.

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 was checked for cross-browser compatibility

elasticmachine · 2022-02-16T22:02:01Z

Pinging @elastic/ml-ui (:ml)

qn895 · 2022-02-17T16:13:49Z

x-pack/plugins/ml/public/maps/anomaly_layer_wizard_factory.tsx

@@ -45,18 +46,23 @@ export class AnomalyLayerWizardFactory {
    this.canGetJobs = canGetJobs;
  }

-  private async getServices(): Promise<{ mlJobsService: MlApiServices['jobs'] }> {
-    const [coreStart] = await this.getStartServices();
+  private async getServices(): Promise<{ mlJobsService: MlApiServices['jobs']; mlLocator: any }> {


I think we can update mlLocator here to use MlLocator type from ml/common/types/locator. Note that locators.get() can return undefined as well 👀

Updated type from any in cb6ff5f3e2f81603a2a34444a7f777206e504896

qn895 · 2022-02-17T16:15:31Z

x-pack/plugins/ml/public/maps/anomaly_layer_wizard_factory.tsx

  }

  public async create(): Promise<LayerWizard> {
-    const { mlJobsService } = await this.getServices();
+    const { mlJobsService, mlLocator } = await this.getServices();
+    const jobsManagementPath = await mlLocator.getUrl({


Since locators.get() can return undefined, would be good to handle the scenario for when mlLocator is undefined. Suggestion to console.error/throw error if that's the case.

Also should we be checking if user has permission to create jobs or not before showing the Create jobs prompt?

Good call - added canCreateJob check and disabled the creation prompt if user doesn't have creation permission in cb6ff5f3e2f81603a2a34444a7f777206e504896

Also added undefined mlLocator check in this change. Now the user will only see the empty prompt if the locator is defined.

lcawl

UI text LGTM

alvarezmelissa87 · 2022-02-17T17:59:46Z

This has been updated and is ready for another look when you get a chance. 🙏 cc @qn895, @jgowdyelastic

qn895 · 2022-02-17T18:21:16Z

Tested and LGTM 🎉 Unrelated to this PR but I realized after navigating to our ML job management, it wasn't clear/obvious right away where the Create job is since the button is to the right and is quite small. Perhaps we can bring the button to more front and center, or redirect to the create job right away based on the selected index pattern selected in Maps. Just thoughts for possible future enhancements :)

jgowdyelastic

Added some small comments, on the whole LGTM

jgowdyelastic · 2022-02-21T09:18:03Z

x-pack/plugins/ml/public/maps/anomaly_job_selector.tsx

-    }
-
-    return (
+    const supportedJobsExist = this.state.jobIdList?.length && this.state.jobIdList?.length > 0;


nit pick, this could be !!this.state.jobIdList?.length

jgowdyelastic · 2022-02-21T09:18:54Z

x-pack/plugins/ml/public/maps/anomaly_job_selector_empty_state.tsx

+      </h2>
+    }
+    body={
+      <>


this fragment isn't needed

jgowdyelastic · 2022-02-21T09:32:22Z

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

@@ -177,7 +177,8 @@ export class MlPlugin implements Plugin<MlPluginSetup, MlPluginStart> {
      if (pluginsSetup.maps) {
        // Pass capabilites.ml.canGetJobs as minimum permission to show anomalies card in maps layers
        const canGetJobs = capabilities.ml?.canGetJobs === true || false;
-        await registerMapExtension(pluginsSetup.maps, core, canGetJobs);
+        const canCreateJobs = capabilities.ml?.canCreateJob === true || false;


it might be worth changing both of these checks to use ??

kibana-ci · 2022-02-23T21:30:45Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	1612	1613	+1

Async chunks

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

id	before	after	diff
`ml`	3.5MB	3.5MB	+1.6KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`ml`	39.1KB	39.2KB	+86.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`ml`	97	98	+1

Total ESLint disabled count

id	before	after	diff
`ml`	99	100	+1

History

💚 Build #25940 succeeded ffd40cb
💚 Build #25107 succeeded 212fb405a6f6cc582199998a222f9d07d364dd3f
💔 Build #25074 failed cb6ff5f3e2f81603a2a34444a7f777206e504896
💚 Build #24853 succeeded 34325b33985f4e08b7aec53a952ccf1c9b6a8b4b
💔 Build #24832 failed bbe139cca99c31edbfe34564e446b7b168fefd8f

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

cc @alvarezmelissa87

kibanamachine · 2022-02-25T22:16:18Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 125878 or prevent reminders by adding the backport:skip label.

kibanamachine · 2022-02-28T22:26:18Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 125878 or prevent reminders by adding the backport:skip label.

…exist (elastic#125878) * add empty state prompt when no supported jobs exist * fix formatting * check creation permission. update types * remove unnecessary fragment and update permission check

alvarezmelissa87 added :ml release_note:skip Skip the PR/issue when compiling release notes v8.2.0 labels Feb 16, 2022

alvarezmelissa87 requested a review from a team as a code owner February 16, 2022 22:01

alvarezmelissa87 self-assigned this Feb 16, 2022

alvarezmelissa87 mentioned this pull request Feb 16, 2022

[Maps][ML] Integration meta issue #123492

Closed

8 tasks

alvarezmelissa87 requested review from lcawl, qn895 and jgowdyelastic February 17, 2022 00:55

qn895 reviewed Feb 17, 2022

View reviewed changes

lcawl added the ui-copy Review of UI copy with docs team is recommended label Feb 17, 2022

lcawl approved these changes Feb 17, 2022

View reviewed changes

qn895 approved these changes Feb 17, 2022

View reviewed changes

alvarezmelissa87 force-pushed the ml-maps-integration-updates branch from cb6ff5f to 212fb40 Compare February 17, 2022 19:43

jgowdyelastic approved these changes Feb 21, 2022

View reviewed changes

alvarezmelissa87 added 3 commits February 23, 2022 09:11

add empty state prompt when no supported jobs exist

9a7f7e3

fix formatting

c7b4775

check creation permission. update types

ffd40cb

alvarezmelissa87 force-pushed the ml-maps-integration-updates branch from 212fb40 to ffd40cb Compare February 23, 2022 16:12

remove unnecessary fragment and update permission check

e080847

alvarezmelissa87 merged commit c5a1a90 into elastic:main Feb 23, 2022

alvarezmelissa87 deleted the ml-maps-integration-updates branch February 23, 2022 22:11

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 25, 2022

peteharverson added the backport:skip This commit does not require backporting label Mar 1, 2022

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Maps integration: add empty state prompt when no supported jobs exist #125878

[ML] Maps integration: add empty state prompt when no supported jobs exist #125878

alvarezmelissa87 commented Feb 16, 2022 •

edited

Loading

elasticmachine commented Feb 16, 2022

qn895 Feb 17, 2022

alvarezmelissa87 Feb 17, 2022

qn895 Feb 17, 2022

alvarezmelissa87 Feb 17, 2022

lcawl left a comment

alvarezmelissa87 commented Feb 17, 2022

qn895 commented Feb 17, 2022

jgowdyelastic left a comment

jgowdyelastic Feb 21, 2022

jgowdyelastic Feb 21, 2022

jgowdyelastic Feb 21, 2022

kibana-ci commented Feb 23, 2022

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented Feb 25, 2022

kibanamachine commented Feb 28, 2022

[ML] Maps integration: add empty state prompt when no supported jobs exist #125878

[ML] Maps integration: add empty state prompt when no supported jobs exist #125878

Conversation

alvarezmelissa87 commented Feb 16, 2022 • edited Loading

Summary

Checklist

elasticmachine commented Feb 16, 2022

qn895 Feb 17, 2022

Choose a reason for hiding this comment

alvarezmelissa87 Feb 17, 2022

Choose a reason for hiding this comment

qn895 Feb 17, 2022

Choose a reason for hiding this comment

alvarezmelissa87 Feb 17, 2022

Choose a reason for hiding this comment

lcawl left a comment

Choose a reason for hiding this comment

alvarezmelissa87 commented Feb 17, 2022

qn895 commented Feb 17, 2022

jgowdyelastic left a comment

Choose a reason for hiding this comment

jgowdyelastic Feb 21, 2022

Choose a reason for hiding this comment

jgowdyelastic Feb 21, 2022

Choose a reason for hiding this comment

jgowdyelastic Feb 21, 2022

Choose a reason for hiding this comment

kibana-ci commented Feb 23, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented Feb 25, 2022

kibanamachine commented Feb 28, 2022

alvarezmelissa87 commented Feb 16, 2022 •

edited

Loading