[ML] Transforms: Adds a link to discover from the transform list to the actions menu. #97805

walterra · 2021-04-21T12:19:37Z

Summary

Adds a link to discover from the transform list to the actions menu.

Conditions for the link to be enabled:

Kibana index pattern must be available
Transform must have been started once and done some progress so there's the destination index available

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

For maintainers

This was checked for breaking API changes and was labeled appropriately

…enu.

elasticmachine · 2021-04-22T10:44:44Z

Pinging @elastic/ml-ui (:ml)

afharo

I think that the changes in src/core/public/saved_objects/saved_objects_service.mock.ts are not needed.

afharo · 2021-04-22T12:50:36Z

src/core/public/saved_objects/saved_objects_service.mock.ts

@@ -16,7 +16,7 @@ const createStartContractMock = () => {
      bulkUpdate: jest.fn(),
      delete: jest.fn(),
      bulkGet: jest.fn(),
-      find: jest.fn(),
+      find: jest.fn().mockResolvedValue({ savedObjects: [] }),


Do you mind setting the mock if needed on your own tests?

I refactored the code to apply that mock in the transform plugin code (171a65c)

darnautov · 2021-04-22T14:54:28Z

x-pack/plugins/transform/public/app/common/navigation.tsx

+export function getDiscoverUrl(indexPatternId: string, baseUrl: string): string {
+  return `${baseUrl}/app/discover#?${getDiscoverUrlState(indexPatternId)}`;
 }


discover plugin provides a URL generator

kibana/src/plugins/discover/public/url_generator.ts

Line 91 in 4584a8b

export class DiscoverUrlGenerator

Good find, we hadn't used those generators yet in the transform plugin. I refactored to make use of it, the result is that these utils are not necessary anymore 🥳 - related update in bb30f3f.

pheyos

Good to see functional test coming as part of this PR 🎉
Just left a few suggestions.

x-pack/test/functional/services/transform/discover.ts

x-pack/test/functional/services/transform/transform_table.ts

...public/app/sections/transform_management/components/action_discover/discover_action_name.tsx

peteharverson · 2021-04-23T11:43:17Z

.../public/app/sections/transform_management/components/action_discover/use_action_discover.tsx

+  const action: TransformListAction = useMemo(
+    () => ({
+      name: (item: TransformListRow) => <DiscoverActionName items={[item]} />,
+      enabled: (item: TransformListRow) => {


If the action is disabled, the text seems to have a disabled style but not the icon. Do we have any control over the icon state? The Start icon in contrast does get a disabled style

EUI's app icons don't support a disabled state yet. As discussed I changed it to use the visTable icon instead (189cbb2).

Created an issue in EUI here: elastic/eui#4755

.../public/app/sections/transform_management/components/action_discover/use_action_discover.tsx

x-pack/test/functional/services/transform/discover.ts

peteharverson

Tested latest changes and LGTM

afharo

No-longer touching anything on the Core side. LGTM :)

Thanks for doing the changes @walterra

jgowdyelastic

LGTM

[ML] Fix assertion. Co-authored-by: Robert Oskamp <[email protected]>

pheyos

Functional tests LGTM

kibanamachine · 2021-04-27T14:49:18Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`transform`	221	224	+3

Async chunks

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

id	before	after	diff
`transform`	903.8KB	908.6KB	+4.8KB

Page load bundle

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

id	before	after	diff
`transform`	19.5KB	19.5KB	-69.0B

History

💚 Build #122416 succeeded 2616008
💚 Build #122390 succeeded a150df4
💔 Build #122342 failed dc8e95b
💚 Build #122317 succeeded 189cbb2
💔 Build #122300 failed bb30f3f

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

cc @walterra

…he actions menu. (elastic#97805) Adds a link to discover from the transform list to the actions menu. Conditions for the link to be enabled: - Kibana index pattern must be available - Transform must have been started once and done some progress so there's the destination index available

kibanamachine · 2021-04-27T14:51:46Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…he actions menu. (#97805) (#98491) Adds a link to discover from the transform list to the actions menu. Conditions for the link to be enabled: - Kibana index pattern must be available - Transform must have been started once and done some progress so there's the destination index available Co-authored-by: Walter Rafelsberger <[email protected]>

walterra added release_note:enhancement :ml v8.0.0 Feature:Transforms ML transforms v7.14.0 labels Apr 21, 2021

walterra self-assigned this Apr 21, 2021

walterra added 3 commits April 22, 2021 10:35

[ML] Adds a link to discover from the transform list to the actions m…

453057d

…enu.

[ML] Jest tests.

92ae38b

[ML] Functional tests to assert transform destination index in Discover.

7882f2c

walterra force-pushed the ml-transform-link-to-discover branch from 97e7a2e to 7882f2c Compare April 22, 2021 10:40

walterra marked this pull request as ready for review April 22, 2021 10:44

walterra requested review from a team as code owners April 22, 2021 10:44

walterra requested review from peteharverson, pheyos, jgowdyelastic and darnautov April 22, 2021 10:45

walterra mentioned this pull request Apr 22, 2021

[ML] Increase Test Coverage 7.14 #97982

Closed

15 tasks

afharo requested changes Apr 22, 2021

View reviewed changes

darnautov reviewed Apr 22, 2021

View reviewed changes

pheyos reviewed Apr 23, 2021

View reviewed changes

x-pack/test/functional/services/transform/discover.ts Outdated Show resolved Hide resolved

x-pack/test/functional/services/transform/discover.ts Outdated Show resolved Hide resolved

x-pack/test/functional/services/transform/transform_table.ts Outdated Show resolved Hide resolved

peteharverson reviewed Apr 23, 2021

View reviewed changes

walterra added 7 commits April 26, 2021 11:15

Merge branch 'master' into ml-transform-link-to-discover

21244e7

[ML] Don't change mock in Kibana core.

171a65c

[ML] Refactor to use url generator to link to Discover.

bb30f3f

[ML] Make Discover first letter uppercase.

69fabdc

[ML] Fix share plugin mock.

78b5939

[ML] Fix Discover action icon.

189cbb2

[ML] Fix tooltips.

dc8e95b

walterra added 2 commits April 26, 2021 17:25

[ML] Fix jest tests.

a150df4

[ML] Tweak functional tests.

2616008

pheyos reviewed Apr 27, 2021

View reviewed changes

x-pack/test/functional/services/transform/discover.ts Outdated Show resolved Hide resolved

peteharverson approved these changes Apr 27, 2021

View reviewed changes

afharo requested review from afharo and removed request for afharo April 27, 2021 11:19

afharo approved these changes Apr 27, 2021

View reviewed changes

jgowdyelastic approved these changes Apr 27, 2021

View reviewed changes

Update x-pack/test/functional/services/transform/discover.ts

233b0b4

[ML] Fix assertion. Co-authored-by: Robert Oskamp <[email protected]>

pheyos approved these changes Apr 27, 2021

View reviewed changes

walterra enabled auto-merge (squash) April 27, 2021 14:26

walterra added the auto-backport Deprecated - use backport:version if exact versions are needed label Apr 27, 2021

walterra merged commit 18d9d43 into elastic:master Apr 27, 2021

walterra deleted the ml-transform-link-to-discover branch April 27, 2021 14:50

kibanamachine mentioned this pull request Apr 27, 2021

[7.x] [ML] Transforms: Adds a link to discover from the transform list to the actions menu. (#97805) #98491

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Transforms: Adds a link to discover from the transform list to the actions menu. #97805

[ML] Transforms: Adds a link to discover from the transform list to the actions menu. #97805

walterra commented Apr 21, 2021 •

edited

Loading

elasticmachine commented Apr 22, 2021

afharo left a comment

afharo Apr 22, 2021

walterra Apr 26, 2021

darnautov Apr 22, 2021

walterra Apr 26, 2021 •

edited

Loading

pheyos left a comment

peteharverson Apr 23, 2021

walterra Apr 26, 2021

peteharverson left a comment

afharo left a comment

jgowdyelastic left a comment

pheyos left a comment

kibanamachine commented Apr 27, 2021

kibanamachine commented Apr 27, 2021

[ML] Transforms: Adds a link to discover from the transform list to the actions menu. #97805

[ML] Transforms: Adds a link to discover from the transform list to the actions menu. #97805

Conversation

walterra commented Apr 21, 2021 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Apr 22, 2021

afharo left a comment

Choose a reason for hiding this comment

afharo Apr 22, 2021

Choose a reason for hiding this comment

walterra Apr 26, 2021

Choose a reason for hiding this comment

darnautov Apr 22, 2021

Choose a reason for hiding this comment

walterra Apr 26, 2021 • edited Loading

Choose a reason for hiding this comment

pheyos left a comment

Choose a reason for hiding this comment

peteharverson Apr 23, 2021

Choose a reason for hiding this comment

walterra Apr 26, 2021

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

afharo left a comment

Choose a reason for hiding this comment

jgowdyelastic left a comment

Choose a reason for hiding this comment

pheyos left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 27, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

kibanamachine commented Apr 27, 2021

💚 Backport successful

walterra commented Apr 21, 2021 •

edited

Loading

walterra Apr 26, 2021 •

edited

Loading