[ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page #74533

alvarezmelissa87 · 2020-08-06T14:21:27Z

Summary

Fixes #70574

Anomaly detection job management page:
- handles list of group ids passed in URL by filtering table for specified group ids.
- with mlManagement groupIds set in the URL
- still supports mlManagement jobId set in the URL

Kibana management page anomaly detection jobs table:
- group badges now link to job management page for selected groups
Updates getJobIdUrl to create urls for group ids as well as job ids and updates function name to getSelectedIdsUrl to be more general.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

elasticmachine · 2020-08-06T14:22:41Z

Pinging @elastic/ml-ui (:ml)

qn895 · 2020-08-06T21:03:12Z

Tested and functionality works well 👍

jgowdyelastic · 2020-08-10T09:49:42Z

x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js

-
-export function getSelectedJobIdFromUrl(url) {
+export function getSelectedIdFromUrl(url) {
+  const result = {};


total nit, result isn't really needed. there could be two return statements in this function.

jgowdyelastic · 2020-08-10T09:55:59Z

x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js

      const urlParams = getUrlVars(url);
      const decodedJson = rison.decode(urlParams.mlManagement);
-      return decodedJson.jobId;
+
+      result.ids = isGroup ? decodedJson.groupIds : decodedJson.jobId;


a simpler approach here might be to have this function return jobId: string or groupIds: string[]
as having a property called ids contain a single string is confusing.

edit after testing, i see that decodedJson.jobId is now a string array. I've added a comment below about how I think this probably should still be a single string, seeing as we can't filter on multiple job ids.

jgowdyelastic · 2020-08-10T10:12:15Z

x-pack/plugins/ml/public/application/util/get_selected_ids_url.ts

+
+export function getSelectedIdsUrl(
+  tabId: string,
+  ids: string | string[],


getSelectedIdsUrl is only ever called with a single group Id in an array.
Why does this need to support multiple group ids? should it also support multiple job ids?

After testing this I see that we do create a URL containing an array of job IDs, but using the property jobId.
IMO this should be a single job id.

rather than having a isGroup modifier parameter, we could have two functions getJobIdUrl and getGroupIdsUrl which both call a non-exported common function for creating the url

Thanks for taking a look @jgowdyelastic 🙏

Support for multiple group ids is requested in the related issue for this as the table also supports it.

I'd originally put the job Id in the array for consistency and possible support of multiple ids. Taking your feedback into account, I agree it should be just a string until we do support multiple ids (which I don't think we need to anytime soon).

To conclude, groupIds will be an array of ids and jobId will be a string of just one id. I agree that breaking it up into two functions instead of using the isGroup parameter would be cleaner. 👍

@jgowdyelastic - all suggestions added in 812cd04

jgowdyelastic · 2020-08-10T15:32:36Z

x-pack/plugins/ml/public/application/util/get_selected_ids_url.ts

+  ANOMALY_DETECTION = 'jobs',
+}
+
+function getSelectedIdsUrl(tabId, settings: any): string {


sorry to be nit-picky. for type-completeness we should give settings its real type here, even if it's basic like [key: string]: string | string[]

qn895 · 2020-08-10T15:50:02Z

Tested and LGTM! Although I also agree with Jame's comment to define the type of the settings in getSelectedIdsUrl.

jgowdyelastic

Tested, LGTM

darnautov · 2020-08-10T16:22:41Z

x-pack/plugins/ml/public/application/util/get_selected_ids_url.ts

@@ -0,0 +1,39 @@
+/*


Could you please implement it as a method in ML URL generator?

There is an existing issue for consolidating the url generating functions for 7.10 that @qn895 is working on so this will be addressed by that afaik.
#69265 (comment)

darnautov · 2020-08-10T16:25:02Z

...data_frame_analytics/pages/analytics_management/components/analytics_list/analytics_list.tsx

-        setSelectedJobIdFromUrlInitialized(true);
-        setSearchQueryText(selectedJobIdFromUrl);
+    if (selectedIdFromUrlInitialized === false && analytics.length > 0) {
+      const { jobId, groupIds } = getSelectedIdFromUrl(window.location.href);


We should not access window.location.href directly. Would be great to receive it from the router state instead

👍 To be done in a follow up.

kibanamachine · 2020-08-10T19:10:21Z

💚 Build Succeeded

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

Build metrics

async chunks size

id	value	diff	baseline
ml	7.9MB	+4.9KB	7.9MB

History

💔 Build #67257 failed 812cd04
💚 Build #66972 succeeded 7245d89a9958d01052a104a20ca3f9cc34684307
💚 Build #66920 succeeded 7db0c2c14ab81f90182eb46e1b3a67dac44f6e63

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

…o job management page (elastic#74533) * handle group id in url for anomaly detection * filter analytics list by group id. * handle list of groupIds * ensure analytics can handle jobid in url. rename util function * add tests for getSelectedIdFromUrl and getGroupQueryText * keep groupIds as array of strings and jobId as single string * fix tests and update types # Conflicts: # x-pack/plugins/ml/public/application/jobs/jobs_list/components/jobs_list/jobs_list.js

…o job management page (#74533) (#74713) * handle group id in url for anomaly detection * filter analytics list by group id. * handle list of groupIds * ensure analytics can handle jobid in url. rename util function * add tests for getSelectedIdFromUrl and getGroupQueryText * keep groupIds as array of strings and jobId as single string * fix tests and update types # Conflicts: # x-pack/plugins/ml/public/application/jobs/jobs_list/components/jobs_list/jobs_list.js

…-task * master: (42 commits) Allow any hostname for chromium proxy bypass (elastic#74693) [ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page (elastic#74533) [Metrics UI] Fix No Data preview pluralization (elastic#74399) [Bug][Security_Solution][Telemetry] Capitalize S in macOS (elastic#74688) Remove karma tests from legacy maps (elastic#74668) [Ingest Manager] stop creating events-* index pattern and placeholder index (elastic#74683) [Enterprise Search] Update the browser/document title on plugin navigation (elastic#74392) [visualizations] Add i18n translation for 'No results found' (elastic#74619) [maps] convert vector style properties to TS (elastic#74553) bump geckodriver binary to 0.27 (elastic#74638) fix: update apm agents to catch abort requests (elastic#74658) [Security Solution] Resolver children pagination (elastic#74603) add memoryStatus to df analytics page and analytics table in management (elastic#74570) [Ingest Manager] Allow prerelease in package version (elastic#74452) [App Arch]: remove legacy karma tests (elastic#74599) [i18n] revert reverted changes (elastic#74633) [Lens] Clear out all attribute properties before updating (elastic#74483) [Uptime] Fix full reloads while navigating to alert/ml (elastic#73796) Index pattern field class refactor (elastic#73180) [ML] Functional tests - stabilize DFA job type check (elastic#74631) ...

* master: (339 commits) [Ingest Node Pipelines] Sentence-case processor names (elastic#74645) Bump angular dependency from 1.7.9 to 1.8.0 (elastic#74482) [ML] Fixing schema for custom rule conditions (elastic#74676) [ML] Refactor in preparation for new es client (elastic#74552) [ML] Adding initial file analysis overrides (elastic#74376) Allow any hostname for chromium proxy bypass (elastic#74693) [ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page (elastic#74533) [Metrics UI] Fix No Data preview pluralization (elastic#74399) [Bug][Security_Solution][Telemetry] Capitalize S in macOS (elastic#74688) Remove karma tests from legacy maps (elastic#74668) [Ingest Manager] stop creating events-* index pattern and placeholder index (elastic#74683) [Enterprise Search] Update the browser/document title on plugin navigation (elastic#74392) [visualizations] Add i18n translation for 'No results found' (elastic#74619) [maps] convert vector style properties to TS (elastic#74553) bump geckodriver binary to 0.27 (elastic#74638) fix: update apm agents to catch abort requests (elastic#74658) [Security Solution] Resolver children pagination (elastic#74603) add memoryStatus to df analytics page and analytics table in management (elastic#74570) [Ingest Manager] Allow prerelease in package version (elastic#74452) [App Arch]: remove legacy karma tests (elastic#74599) ...

…nes/processor-forms-a-d * 'master' of github.com:elastic/kibana: (26 commits) [Telemetry][API Integration] size_in_bytes to be a number (elastic#74664) [ILM] Convert node details flyout to TS (elastic#73707) [Ingest Node Pipelines] Sentence-case processor names (elastic#74645) Bump angular dependency from 1.7.9 to 1.8.0 (elastic#74482) [ML] Fixing schema for custom rule conditions (elastic#74676) [ML] Refactor in preparation for new es client (elastic#74552) [ML] Adding initial file analysis overrides (elastic#74376) Allow any hostname for chromium proxy bypass (elastic#74693) [ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page (elastic#74533) [Metrics UI] Fix No Data preview pluralization (elastic#74399) [Bug][Security_Solution][Telemetry] Capitalize S in macOS (elastic#74688) Remove karma tests from legacy maps (elastic#74668) [Ingest Manager] stop creating events-* index pattern and placeholder index (elastic#74683) [Enterprise Search] Update the browser/document title on plugin navigation (elastic#74392) [visualizations] Add i18n translation for 'No results found' (elastic#74619) [maps] convert vector style properties to TS (elastic#74553) bump geckodriver binary to 0.27 (elastic#74638) fix: update apm agents to catch abort requests (elastic#74658) [Security Solution] Resolver children pagination (elastic#74603) add memoryStatus to df analytics page and analytics table in management (elastic#74570) ...

alvarezmelissa87 requested a review from a team as a code owner August 6, 2020 14:21

alvarezmelissa87 self-assigned this Aug 6, 2020

alvarezmelissa87 added :ml release_note:enhancement v7.10.0 v8.0.0 labels Aug 6, 2020

alvarezmelissa87 requested review from darnautov and jgowdyelastic August 6, 2020 15:06

jgowdyelastic reviewed Aug 10, 2020

View reviewed changes

alvarezmelissa87 added 5 commits August 10, 2020 09:34

handle group id in url for anomaly detection

f0e66c0

filter analytics list by group id.

296688b

handle list of groupIds

0e4f2ed

ensure analytics can handle jobid in url. rename util function

148ab2a

add tests for getSelectedIdFromUrl and getGroupQueryText

3e9c17b

alvarezmelissa87 force-pushed the ml-anomaly-detection-pass-group-in-url branch from 7245d89 to 3e9c17b Compare August 10, 2020 13:35

keep groupIds as array of strings and jobId as single string

812cd04

alvarezmelissa87 requested a review from qn895 August 10, 2020 14:42

jgowdyelastic reviewed Aug 10, 2020

View reviewed changes

qn895 approved these changes Aug 10, 2020

View reviewed changes

jgowdyelastic approved these changes Aug 10, 2020

View reviewed changes

darnautov reviewed Aug 10, 2020

View reviewed changes

fix tests and update types

e4d47f1

alvarezmelissa87 merged commit 4ee483b into elastic:master Aug 10, 2020

alvarezmelissa87 mentioned this pull request Aug 10, 2020

[7.x] [ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page (#74533) #74713

Merged

alvarezmelissa87 deleted the ml-anomaly-detection-pass-group-in-url branch August 10, 2020 21:27

formgeist mentioned this pull request Aug 12, 2020

[APM] Anomaly detection Settings page: Link directly to ML jobs management to filter for the select environment #74826

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page #74533

[ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page #74533

alvarezmelissa87 commented Aug 6, 2020 •

edited

Loading

elasticmachine commented Aug 6, 2020

qn895 commented Aug 6, 2020

jgowdyelastic Aug 10, 2020 •

edited

Loading

jgowdyelastic Aug 10, 2020

jgowdyelastic Aug 10, 2020

jgowdyelastic Aug 10, 2020

jgowdyelastic Aug 10, 2020

jgowdyelastic Aug 10, 2020

alvarezmelissa87 Aug 10, 2020

alvarezmelissa87 Aug 10, 2020

jgowdyelastic Aug 10, 2020 •

edited

Loading

qn895 commented Aug 10, 2020

jgowdyelastic left a comment

darnautov Aug 10, 2020

alvarezmelissa87 Aug 10, 2020

darnautov Aug 10, 2020

alvarezmelissa87 Aug 10, 2020

kibanamachine commented Aug 10, 2020

[ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page #74533

[ML] ML on Kibana Management: Add ability to pass a group ID filter to job management page #74533

Conversation

alvarezmelissa87 commented Aug 6, 2020 • edited Loading

Summary

Checklist

elasticmachine commented Aug 6, 2020

qn895 commented Aug 6, 2020

jgowdyelastic Aug 10, 2020 • 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

jgowdyelastic Aug 10, 2020 • edited Loading

Choose a reason for hiding this comment

qn895 commented Aug 10, 2020

jgowdyelastic 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

Choose a reason for hiding this comment

kibanamachine commented Aug 10, 2020

💚 Build Succeeded

Build metrics

async chunks size

History

alvarezmelissa87 commented Aug 6, 2020 •

edited

Loading

jgowdyelastic Aug 10, 2020 •

edited

Loading

jgowdyelastic Aug 10, 2020 •

edited

Loading