[ML] Add linking to dataframe from job management tab #65778

qn895 · 2020-05-07T21:40:05Z

Summary

This PR adds action links from Analytics jobs in Management app for viewing job config and exploration while still maintaining sortability

The job id column within the ML tab will stay the same without being hyperlinked

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
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

elasticmachine · 2020-05-07T21:40:07Z

Pinging @elastic/ml-ui (:ml)

peteharverson · 2020-05-11T12:09:52Z

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

@@ -191,12 +201,14 @@ export const getColumns = (
      'data-test-subj': 'mlAnalyticsTableRowDetailsToggle',
    },
    {
-      field: DataFrameAnalyticsListColumn.id,
+      // field: DataFrameAnalyticsListColumn.id,


Is this commented out line needed?

I'll fix that. Thanks.

This is now cleaned up in 1ec4fde

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

qn895 · 2020-05-14T17:02:40Z

@elasticmachine merge upstream

qn895 · 2020-05-14T17:14:31Z

I have updated the PR to now redirect to the DFA search page with the job_id filter.

Some thoughts for consideration:

I have refactored getJobIdUrl to take in the tab idea (e.g. jobs or data_frame_analytics). The function is is currently located in x-pack/plugins/ml/public/application/jobs/jobs_list/components/utils.js. Would it be better to move this utility function to a common file?
After the user navigates to the link with the job id, should we clear the window link to the default generic link for that tab?

So the URL at first would look something like
ml#/data_frame_analytics?mlManagement=(jobId:bank_classification_1)
would turn into
ml#/${data_frame_analytics}?_g=${urlParams._g}

alvarezmelissa87 · 2020-05-14T18:04:07Z

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

@@ -135,6 +138,10 @@ export const progressColumn = {
  'data-test-subj': 'mlAnalyticsTableColumnProgress',
 };

+export const getDFAnalyticsJobIdLink = (item: DataFrameAnalyticsListRow) => {


Nit: this could be implicit return

Changed this to implicit return in f3c8367#diff-5c7fbd428261fc2fd9567311e2e7046cL141.

alvarezmelissa87 · 2020-05-14T18:16:48Z

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

@@ -367,7 +367,7 @@ function jobProperty(job, prop) {
  return job[propMap[prop]];
 }

-export function getJobIdUrl(jobId) {
+export function getJobIdUrl(tabId, jobId) {


This could be moved to a common directory since it's shared (maybe public/application/util?).

Could just create a new ts file with the name of the function with only that function in that file (like job_selection_change.ts, has_matching_points.ts). Then you wouldn't have to worry about jobs_list/components/utils.js not being in ts yet.

I moved this getJobIdUrl function to x-pack/plugins/ml/public/application/util/get_job_id_url.ts in f3c8367. Much cleaner 😄 !

alvarezmelissa87 · 2020-05-14T18:17:10Z

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

@@ -408,3 +408,15 @@ export function clearSelectedJobIdFromUrl(url) {
    }
  }
 }
+
+export function resetMlJobUrl(tabId, url) {


I don't see this being used anywhere. Unless I'm missing something.

I originally did this in case we want to reset the window's url after navigating to the page, but since it's not needed, I have deleted this in f3c8367.

alvarezmelissa87 · 2020-05-14T18:27:15Z

Tested and functionality looks right. Looks good overall. Just a couple of suggestions around shared function placement. 😄

…qn895/kibana into add-df-links-to-management-list

peteharverson

Tested and LGTM.
The way you implemented it to leave the URL as e.g. ml#/data_frame_analytics?mlManagement=(jobId:bank_classification_1) looks good.
I think the screen capture in the PR description needs editing to show navigation to the jobs list.

alvarezmelissa87

Latest edits LGTM ⚡

qn895 · 2020-05-15T17:17:13Z

@elasticmachine merge upstream

kibanamachine · 2020-05-15T19:20:06Z

💚 Build Succeeded

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

History

💔 Build #48320 failed 32dbd1f
💔 Build #48088 failed f3c8367
💔 Build #48027 failed ea3af55
💔 Build #46479 failed e6b425e

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

…ine-editor * 'master' of github.com:elastic/kibana: (157 commits) [ML] fix url assertion (#66850) Skip failing lens test(s). #66779 [SOM] Preserve saved object references when saving the object (#66584) Use ES API from start contract (#66157) Reorganize Management apps into Ingest, Data, Alerts and Insights, Security, Kibana, and Stack groups (#65796) [Uptime] Fix flaky navigation to certs page in tests (#66806) [Maps] Do not check count for blended layers when layer is not visible (#66460) [SIEM] Fixes glob patterns from directory changes recently for GraphQL chore(NA): bump static-fs to 1.0.2 (#66775) [Maps] Handle cross cluster index _settings resp (#66797) [SIEM][Lists] Adds 90% of the REST API and client API for exception lists and exception items allow any type for customResponseHeaders config (#66689) [APM] Disable map layout animation (#66763) [ML] Add linking to dataframe from job management tab (#65778) [Maps] Get number of categories from palette (#66454) move oss features registration to KP (#66524) [kbn/plugin-helpers] typescript-ify (#66513) Add kibana-operations as codeowners for .ci/es-snapshots and vars/ (#66746) FTR: move basic services under common folder (#66563) Migrate Beats Management UI to KP (#65791) ... # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form_fields.tsx

* master: [ML] fix url assertion (elastic#66850) Skip failing lens test(s). elastic#66779 [SOM] Preserve saved object references when saving the object (elastic#66584) Use ES API from start contract (elastic#66157) Reorganize Management apps into Ingest, Data, Alerts and Insights, Security, Kibana, and Stack groups (elastic#65796) [Uptime] Fix flaky navigation to certs page in tests (elastic#66806) [Maps] Do not check count for blended layers when layer is not visible (elastic#66460) [SIEM] Fixes glob patterns from directory changes recently for GraphQL chore(NA): bump static-fs to 1.0.2 (elastic#66775) [Maps] Handle cross cluster index _settings resp (elastic#66797) [SIEM][Lists] Adds 90% of the REST API and client API for exception lists and exception items allow any type for customResponseHeaders config (elastic#66689) [APM] Disable map layout animation (elastic#66763) [ML] Add linking to dataframe from job management tab (elastic#65778)

* [ML] Add linking to dataframe from job management tab * [ML] Add linking to df analytics job list from management tab * [ML] Add ability to query text/job id from url for DFA page * [ML] Add linking to dataframe from job management tab * [ML] Add linking to df analytics job list from management tab * [ML] Add ability to query text/job id from url for DFA page * [ML] Refactor get_job_id_url util function & clean up * [ML] Add declaration file for job_list's utils Co-authored-by: Elastic Machine <[email protected]>

* [ML] Add linking to dataframe from job management tab * [ML] Add linking to df analytics job list from management tab * [ML] Add ability to query text/job id from url for DFA page * [ML] Add linking to dataframe from job management tab * [ML] Add linking to df analytics job list from management tab * [ML] Add ability to query text/job id from url for DFA page * [ML] Refactor get_job_id_url util function & clean up * [ML] Add declaration file for job_list's utils Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

[ML] Add linking to dataframe from job management tab

e6b425e

qn895 added release_note:enhancement :ml v8.0.0 Feature:Data Frame Analytics ML data frame analytics features v7.8.0 labels May 7, 2020

qn895 requested review from walterra, alvarezmelissa87 and peteharverson May 7, 2020 21:40

qn895 requested a review from a team as a code owner May 7, 2020 21:40

qn895 self-assigned this May 7, 2020

peteharverson added v7.9.0 and removed v7.8.0 labels May 11, 2020

peteharverson reviewed May 11, 2020

View reviewed changes

peteharverson requested changes May 11, 2020

View reviewed changes

...cation/data_frame_analytics/pages/analytics_management/components/analytics_list/columns.tsx Outdated Show resolved Hide resolved

qn895 added 5 commits May 12, 2020 15:52

[ML] Add linking to df analytics job list from management tab

56b740a

[ML] Add ability to query text/job id from url for DFA page

1ec4fde

[ML] Add linking to dataframe from job management tab

484b645

[ML] Add linking to df analytics job list from management tab

58f66a4

[ML] Add ability to query text/job id from url for DFA page

7f6ec89

Merge branch 'master' into add-df-links-to-management-list

ea3af55

alvarezmelissa87 reviewed May 14, 2020

View reviewed changes

qn895 added 2 commits May 14, 2020 14:16

Merge branch 'add-df-links-to-management-list' of https://github.com/…

0159072

…qn895/kibana into add-df-links-to-management-list

[ML] Refactor get_job_id_url util function & clean up

f3c8367

peteharverson approved these changes May 15, 2020

View reviewed changes

[ML] Add declaration file for job_list's utils

32dbd1f

alvarezmelissa87 approved these changes May 15, 2020

View reviewed changes

Merge branch 'master' into add-df-links-to-management-list

4004469

qn895 merged commit ae663eb into elastic:master May 15, 2020

qn895 mentioned this pull request May 18, 2020

[7.x] [ML] Add linking to dataframe from job management tab (#65778) #66887

Merged

qn895 deleted the add-df-links-to-management-list branch June 11, 2020 00:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Add linking to dataframe from job management tab #65778

[ML] Add linking to dataframe from job management tab #65778

qn895 commented May 7, 2020 •

edited

Loading

elasticmachine commented May 7, 2020

peteharverson May 11, 2020 •

edited

Loading

qn895 May 11, 2020

qn895 May 14, 2020

qn895 commented May 14, 2020

qn895 commented May 14, 2020

alvarezmelissa87 May 14, 2020

qn895 May 14, 2020

alvarezmelissa87 May 14, 2020

qn895 May 14, 2020

alvarezmelissa87 May 14, 2020

qn895 May 14, 2020

alvarezmelissa87 commented May 14, 2020

peteharverson left a comment

alvarezmelissa87 left a comment

qn895 commented May 15, 2020

kibanamachine commented May 15, 2020

[ML] Add linking to dataframe from job management tab #65778

[ML] Add linking to dataframe from job management tab #65778

Conversation

qn895 commented May 7, 2020 • edited Loading

Summary

Checklist

elasticmachine commented May 7, 2020

peteharverson May 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qn895 commented May 14, 2020

qn895 commented May 14, 2020

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

alvarezmelissa87 commented May 14, 2020

peteharverson left a comment

Choose a reason for hiding this comment

alvarezmelissa87 left a comment

Choose a reason for hiding this comment

qn895 commented May 15, 2020

kibanamachine commented May 15, 2020

💚 Build Succeeded

History

qn895 commented May 7, 2020 •

edited

Loading

peteharverson May 11, 2020 •

edited

Loading