[Home] Adding file upload to add data page #100863

jgowdyelastic · 2021-05-28T11:39:13Z

Adds a registry to the Home plugin to allow other plugins to add tabs to the Add Data page.
Embeds the File Data Visualizer in a new tab.

Closes #98652

cc @grabowskit

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
This was checked for breaking API changes and was labeled appropriately

jgowdyelastic · 2021-06-01T07:30:08Z

@elasticmachine merge upstream

jgowdyelastic · 2021-06-01T15:49:51Z

@elasticmachine merge upstream

elasticmachine · 2021-06-02T09:26:33Z

Pinging @elastic/ml-ui (:ml)

pgayvallet

home plugin changes LGTM.

A few comments

pgayvallet · 2021-06-02T10:13:17Z

src/plugins/home/public/application/components/tutorial_directory.js

+    if (tab?.content !== undefined) {
+      return tab.content;


NIT: if (tab?.content)

Updated in b532118

pgayvallet · 2021-06-02T10:16:02Z

src/plugins/home/public/services/add_data/add_data_service.ts

+  private addDataTabs: {
+    [key: string]: AddDataTab;
+  } = {};


NIT: Record<string, AddDataTab>

Updated in b532118

pgayvallet · 2021-06-02T10:18:51Z

x-pack/plugins/file_data_visualizer/kibana.json

+    "fileUpload",
+    "home"


I think home should be an optional dependency here.

Updated in b532118

pgayvallet · 2021-06-02T10:22:38Z

x-pack/plugins/file_data_visualizer/public/lazy_load_bundle/component_wrapper.tsx

+  useEffect(() => {
+    if (FileDataVisualizerComponent === null) {
+      import('../application/file_datavisualizer').then(({ FileDataVisualizer }) => {
+        setFileDataVisualizerComponent(FileDataVisualizer);


NIT: could use react.lazy instead of a manual effect.

good idea, that is much nicer.

Updated in b532118

nreese · 2021-06-02T12:37:16Z

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

@@ -40,7 +43,11 @@ export class FileDataVisualizerPlugin
      FileDataVisualizerSetupDependencies,
      FileDataVisualizerStartDependencies
    > {
-  public setup() {}
+  public setup(core: CoreSetup, plugins: FileDataVisualizerSetupDependencies) {
+    if (plugins.home) {


Should there be a permission check here to only register the file upload tab if the user has the minimum permissions needed to analyze a file?

Would this be a good time to remove access:fileUpload:analyzeFile and access:fileUpload:import tags from file upload routes?

I think this would be a good time to decouple file data visualizer permissions from ML so that users can upload a file and not have kibana access to ml or any ml user roles.

Should there be a permission check here to only register the file upload tab if the user has the minimum permissions needed to analyze a file?

ML has run in to race condition issues before when registering links in home and the management app where, due to the time it takes to check the license and in this case maybe the capabilities, the registration happens too late for the links to actually appear.

I'll take a look to see if this is possible, but we might just have to always register and perform the capabilities check later when the user navigates to the "Upload file" tab.

Would this be a good time to remove access:fileUpload:analyzeFile and access:fileUpload:import tags from file upload routes?

I might have missed something, why would we want to remove the capabilities checks from the routes?

nreese

Thanks for putting this PR up, its great that users can upload files without leaving the home application. Would it be possible to change the url for the "Upload a file" card on the home page to file data visualizer add data tab? That way, users will never be directed to ML from home and both locations take you to the same place.

kibanamachine · 2021-06-02T12:46:07Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`fileDataVisualizer`	179	181	+2
`home`	68	70	+2
total			+4

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`home`	64	65	+1

Async chunks

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

id	before	after	diff
`fileDataVisualizer`	1.0MB	1.0MB	+695.0B
`home`	165.0KB	165.3KB	+311.0B
total			+1006.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`home`	4	5	+1

Page load bundle

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

id	before	after	diff
`fileDataVisualizer`	11.8KB	12.7KB	+934.0B
`home`	14.8KB	15.4KB	+613.0B
total			+1.5KB

Unknown metric groups

API count

id	before	after	diff
`home`	88	89	+1

async chunk count

id	before	after	diff
`fileDataVisualizer`	1	2	+1

References to deprecated APIs

id	before	after	diff
`canvas`	29	25	-4
`crossClusterReplication`	8	6	-2
`fleet`	22	20	-2
`globalSearch`	4	2	-2
`indexManagement`	12	7	-5
`infra`	256	146	-110
`lens`	67	45	-22
`licensing`	18	15	-3
`lists`	239	236	-3
`maps`	286	208	-78
`ml`	121	115	-6
`monitoring`	109	56	-53
`securitySolution`	390	346	-44
`stackAlerts`	101	95	-6
total			-340

History

💚 Build #128423 succeeded 79a5440
💔 Build #128414 failed a7b490e
💚 Build #128260 succeeded cd1388f
💛 Build #128101 was flaky 3cb64d2
💔 Build #127870 failed 0dbb1f4

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

cc @jgowdyelastic

jgowdyelastic · 2021-06-02T13:04:40Z

Would it be possible to change the url for the "Upload a file" card on the home page to file data visualizer add data tab?

@nreese yes, I looked into this. It isn't possible to navigate straight to a specific tab in the Add Data page. If we're happy just linking the general "Add Data" page, where the user then has to click to the correct tab, I can change this link and move the registration of the link to the file data viz plugin.

Adding some URL state to the Add Data page to allow tab selection I think is outside of the scope of this PR. I'll raise an issue if we think this is needed.

nreese · 2021-06-02T13:45:29Z

I might have missed something, why would we want to remove the capabilities checks from the routes?

Currently, users have to have ML kibana privileges to gain access to the tags access:fileUpload:analyzeFile and access:fileUpload:import.

Limiting file data visualizer access to ML kibana privileges is an artifact of were the code was historically located. Why do users have to have ML kibana privilege to upload files?

It would make sense to decouple ML kibana privileges from file data visualizer so users without ML kibana privileges can upload files. Removing the capabilities tag was one implementation idea for this decoupling.

jgowdyelastic · 2021-06-02T13:56:13Z

Removing the capabilities tag was one implementation idea for this decoupling.

Ok cool. I assumed we'd move these capabilities to somewhere else. But I agree that removing them entirely is probably easiest plus the actual import UI is protected behind a separate security check.

nreese

Functionally looks great. Permissions need to be sorted out but that can be tackled in another effort.

LGTM
code review, tested in chrome

* [Home] Adding file upload to add data page * updating comment * tiny refactor * attempting to reduce bundle size * reverting to original home register import * lazy load tab contents * changes based on review Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-06-02T14:36:47Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

jgowdyelastic · 2021-06-02T16:08:00Z

Follow on permission work #101169

* master: (68 commits) Unskip advanced settings a11y test (elastic#100558) [App Search] Crawler Landing Page (elastic#100822) [DOCS] Clarify when to use kbn clean (elastic#101155) change label behavior (elastic#100991) skip flaky suite (elastic#101126) Fix cases plugin ownership (elastic#101073) [Home] Adding file upload to add data page (elastic#100863) [ML] Functional tests - reenable categorization tests (elastic#101137) [DOCS] Adds server.uuid to settings docs (elastic#101121) Fix newsfeed unread notifications always on when reloading Kibana (elastic#100357) [Lens] Time shift metrics (elastic#98781) [Deprecations service] make `correctiveActions.manualSteps` required (elastic#100997) Add "Risk Matrix" section to the PR template (elastic#100649) [Maps] spatially filter by all geo fields (elastic#100735) [Security Solution] Add Ransomware canary advanced policy option (elastic#101068) [Exploratory view] Core web vitals (elastic#100320) [Security solution][Endpoint] Add unit tests for fleet event filters/trusted apps cards (elastic#101034) [Lens] Use a setter function for the dimension panel (elastic#101123) [Index Patterns] Fix return saved index pattern object (elastic#101051) [CI] For PRs, build TS refs before public api docs check (elastic#100791) ...

* [Home] Adding file upload to add data page * updating comment * tiny refactor * attempting to reduce bundle size * reverting to original home register import * lazy load tab contents * changes based on review Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: James Gowdy <jgowdy@elastic.co>

* [ML] Add index visualizer * [ML] Readd support for global state * [ML] Add time buckets & fix dependencies * [ML] Working ver * [ML] Add back and boolean support * [ML] Remove old files inside ml * [ML] Rename files * [ML] Move field type icon * [ML] Create new folder structure * [ML] Organize index_data_visualizer * [ML] Move types into index_data_visualizer folder * [ML] Move more files into file_data_visualizer * [ML] Move more files into index_data_visualizer * [ML] Add new data visualizer model * [ML] Remove getVisualizerFieldStats which is not used by dv * [ML] Delete redundant folder * [ML] Copy old data visualizer routes to new plugin * [ML] Remove old routes * [ML] Disable for ml job cards tests for now * [ML] Remove todos * [ML] Move the toast error to the UI component * [ML] Fix map styling * [ML] Add runtime_mappings for internal/file_upload/time_field_range * [ML] Move routes into folder * [ML] Update permissions * [ML] Update texts * [ML] Update schemas import and api get_field_stats * [ML] Reorg folders into common * [ML] Update types & tests * [ML] Update internal/data_visualizer permissions and action panel tests * [ML] Update imports after #100863 * [ML] Fix CI * [ML] Rename folder from file_data_visualizer to data_visualizer * [ML] Rename i18n ids * [ML] Update fileDataVisualizer -> dataVisualizer dependency name in ml plugin * [ML] Remove ml prefix in data test subjs * [ML] Fix settings and docs * [ML] Update plugin description * [ML] Remove mlContext dependency completely * [ML] Set query to optional * Revert "[ML] Update plugin description" This reverts commit 4ab1a25 * [ML] Update plugins list docs * [ML] Fix types and i18n * [ML] Revert ml data test subj/class name changes * [ML] Split up data visualizer model, remove Logger * [ML] Remove empty file and indexPatternFieldEditor * [ML] Move imports of file_upload * [ML] Update plugin dependencies * Re-add missing data_visualizer.json * Remove capabilities in data_visualizer * Fix test subjs * Update ownership for data_visualizer and file_upload code to be ml * Update estypes after 98266 Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

jgowdyelastic added 4 commits May 28, 2021 12:37

[Home] Adding file upload to add data page

1350425

updating comment

7908c10

tiny refactor

0dbb1f4

attempting to reduce bundle size

3cb64d2

Merge branch 'master' into adding-file-upload-tab-to-home-add-data-page

cd1388f

kibanamachine and others added 3 commits June 1, 2021 11:49

Merge branch 'master' into adding-file-upload-tab-to-home-add-data-page

c16ce24

reverting to original home register import

a7b490e

lazy load tab contents

79a5440

jgowdyelastic requested a review from joshdover June 2, 2021 09:04

jgowdyelastic self-assigned this Jun 2, 2021

jgowdyelastic added :ml Feature:File and Index Data Viz ML file and index data visualizer release_note:enhancement review v7.14.0 v8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 2, 2021

jgowdyelastic requested a review from nreese June 2, 2021 09:05

jgowdyelastic marked this pull request as ready for review June 2, 2021 09:05

jgowdyelastic requested a review from a team as a code owner June 2, 2021 09:05

pgayvallet approved these changes Jun 2, 2021

View reviewed changes

changes based on review

b532118

nreese reviewed Jun 2, 2021

View reviewed changes

nreese approved these changes Jun 2, 2021

View reviewed changes

jgowdyelastic merged commit ad09cdc into elastic:master Jun 2, 2021

jgowdyelastic deleted the adding-file-upload-tab-to-home-add-data-page branch June 2, 2021 14:33

kibanamachine mentioned this pull request Jun 2, 2021

[7.x] [Home] Adding file upload to add data page (#100863) #101166

Merged

qn895 added a commit to qn895/kibana that referenced this pull request Jun 2, 2021

[ML] Update imports after elastic#100863

5e81e92

nreese mentioned this pull request Jun 4, 2021

Change file data visualizer links to point to new location in home application #101393

Merged

lcawl mentioned this pull request Jun 8, 2021

[DOCS] Adding file upload to add data page #101674

Merged

jgowdyelastic mentioned this pull request Jun 21, 2021

[ML] Update file data visualizer permissions #101169

Merged

4 tasks

lcawl mentioned this pull request Aug 10, 2021

[DOCS] Updates file upload details for geospatial data #107985

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Home] Adding file upload to add data page #100863

[Home] Adding file upload to add data page #100863

jgowdyelastic commented May 28, 2021 •

edited

Loading

jgowdyelastic commented Jun 1, 2021

jgowdyelastic commented Jun 1, 2021

elasticmachine commented Jun 2, 2021

pgayvallet left a comment

pgayvallet Jun 2, 2021

jgowdyelastic Jun 2, 2021

pgayvallet Jun 2, 2021

jgowdyelastic Jun 2, 2021

pgayvallet Jun 2, 2021

jgowdyelastic Jun 2, 2021

pgayvallet Jun 2, 2021

jgowdyelastic Jun 2, 2021

jgowdyelastic Jun 2, 2021

nreese Jun 2, 2021

nreese Jun 2, 2021 •

edited

Loading

jgowdyelastic Jun 2, 2021

nreese left a comment

kibanamachine commented Jun 2, 2021

API count

async chunk count

References to deprecated APIs

jgowdyelastic commented Jun 2, 2021

nreese commented Jun 2, 2021

jgowdyelastic commented Jun 2, 2021

nreese left a comment

kibanamachine commented Jun 2, 2021

jgowdyelastic commented Jun 2, 2021

[Home] Adding file upload to add data page #100863

[Home] Adding file upload to add data page #100863

Conversation

jgowdyelastic commented May 28, 2021 • edited Loading

jgowdyelastic commented Jun 1, 2021

jgowdyelastic commented Jun 1, 2021

elasticmachine commented Jun 2, 2021

pgayvallet 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

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

nreese Jun 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nreese left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 2, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

async chunk count

References to deprecated APIs

History

jgowdyelastic commented Jun 2, 2021

nreese commented Jun 2, 2021

jgowdyelastic commented Jun 2, 2021

nreese left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 2, 2021

💚 Backport successful

jgowdyelastic commented Jun 2, 2021

jgowdyelastic commented May 28, 2021 •

edited

Loading

nreese Jun 2, 2021 •

edited

Loading