[NP] Graph migration by maryia-lapata · Pull Request #59409 · elastic/kibana

maryia-lapata · 2020-03-05T11:34:08Z

Fixes #57180.

Migration of Graph client side app from x-pack/legacy/plugins/graph to x-pack/plugins/graph.

Since we cannot use legacy imports in NP, the legacy KibanaParsedUrl was just inlined into generateDefaultTemplate.

Things that can't be migrated right now and be left behind in the legacy plugin:

saved object mappings
saved object migrations
xpack feature registration

Checklist

Unit or functional tests were updated or added to match the most common scenarios

maryia-lapata · 2020-03-06T12:19:53Z

@elasticmachine merge upstream

elasticmachine · 2020-03-06T12:19:55Z

merge conflict between base and head

maryia-lapata · 2020-03-06T12:36:32Z

x-pack/plugins/graph/public/angular/graph_client_workspace.js

    };

    //Determines if 2 nodes are connected via an edge
    this.areLinked = function(a, b) {


This function isn't used, but I'm not sure if we can remove it.

elasticmachine · 2020-03-06T15:32:16Z

Pinging @elastic/kibana-app (Team:KibanaApp)

maryia-lapata · 2020-03-06T15:34:44Z

x-pack/plugins/graph/public/application.ts

 import angular from 'angular';
 import { i18nDirective, i18nFilter, I18nProvider } from '@kbn/i18n/angular';
-
+import '../../../../webpackShims/ace';


It couldn't find the module ace, so I import webpackShims/ace.
@flash1293 can we use such import in NP?

maryia-lapata · 2020-03-06T15:39:18Z

@flash1293 maybe you know how we should replace legacy styles import

kibana/x-pack/plugins/graph/public/angular/templates/_graph.scss

Line 1 in 637a8a3

@import 'src/legacy/ui/public/chrome/variables';

flash1293

That looks pretty good already, left a few comments.

One thing I noticed is that it's not showing a proper title:

Looks like the legacy platform worked some magic there we have to replace.

The README from the legacy platform can also be moved now.

About the scss import thing:

@flash1293 maybe you know how we should replace legacy styles import

I think it's safe to remove that line, I couldn't find a usage of the variables defined in there, looks like a leftover

About the workspace js file:

This function isn't used, but I'm not sure if we can remove it.

Let's leave the cleanup in here for later iterations, all of this will move into redux reducers anyway soon.

x-pack/plugins/graph/public/components/app.tsx

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

flash1293 · 2020-03-10T15:06:30Z

x-pack/plugins/graph/public/state_management/url_templates.ts

 import { i18n } from '@kbn/i18n';
 import rison from 'rison-node';
 import { takeEvery, select } from 'redux-saga/effects';
+import { KibanaParsedUrl } from './kibana_parsed_url';


As this is used in just a single place here and is just taking a single code path through the whole thing, I think it's fine to condense the actual logic and inline it into generateDefaultTemplate directly using only the core services and utilities.

Something like (just for illustration):

const appPath = modifyUrl(`/discover`, parsed => { parsed.query['_g'] = rison.encode(...); }); const discoverUrl = core.http.basePath.prepend(`app/kibana#${appPath}`)

It's already making tons of assumptions about discover URLs anyway. I'll create a separate issue to switch to #57496 at some point, but for now let's make it as little code as possible in here

Thank you for suggestion. I updated the code.
I also left the call of parse function from url module. Since rison.encode wraps index value into quotes when id starts with a number (in our case e.g. datasource.id is 90943e30-9a47-11e8-b64d-95841ca0b247), to replace ' to %27 in the url I partially left the implementation of prependPath.

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

…to np-graph

maryia-lapata · 2020-03-11T11:34:46Z

jenkins, test this please

maryia-lapata · 2020-03-11T14:31:33Z

One thing I noticed is that it's not showing a proper title:

Looks like the legacy platform worked some magic there we have to replace.

It occurs if I switch to Graph after Logs or Metrics, so it seems like the Logs app is breaking tab title.

The README from the legacy platform can also be moved now.

I moved it and updated a bit.

flash1293 · 2020-03-11T17:07:27Z

@elasticmachine merge upstream

flash1293

Tested and couldn't find any issues, LGTM 🚀

I just merged a PR fixing config issues, there might be slight conflicts with this PR, but should be easy to fix ultimately.

…to np-graph

cchaos

SASS moves look ok

kertal · 2020-03-16T05:54:19Z

@elasticmachine merge upstream

kibanamachine · 2020-03-16T07:34:12Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 04e17e1

History

💚 Build #33009 succeeded c9e874e
💚 Build #32498 succeeded c626f4f
💔 Build #32418 failed 4fc604f
💚 Build #32370 succeeded 74ee0bc
💚 Build #31664 succeeded 24f36a6

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

* Move graph to NP * Styles * Clean up * Fix eslint * Fix ESlint * Fix path * Fix container height * Clean up * Update index.ts * Update graph_client_workspace.js * Refactoring * Remove unused methods * Update graph_client_workspace.test.js * Rename npData to data * Move Readme * Inline parsing discover url * Remove import of legacy styles * Update README Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

…o alerting/view-in-app * 'alerting/view-in-app' of github.com:gmmorris/kibana: (33 commits) [SIEM] Adds 'Closes and opens signals' Cypress test (elastic#59950) [NP] Graph migration (elastic#59409) [ML] Clone analytics job (elastic#59791) Move VALUE_CLICK_TRIGGER and APPLY_FILTER_TRIGGER to ui_action… (elastic#60202) Handle improperly defined Watcher Logging Action text parameter. (elastic#60169) Move select range trigger to uiActions (elastic#60168) [SIEM][CASES] Configure cases: Final (elastic#59358) Closes elastic#59784. Sets xpack.apm.serviceMapEnabled default value true. (elastic#60153) [siem/cypress] update junit filename to be picked up by runbld (elastic#60156) OSS logic added to test environment (elastic#60134) Move canvas setup into app mount (elastic#59926) enable triggers_actions_ui plugin by default (elastic#60137) Expose Elasticsearch from start and deprecate from setup (elastic#59886) [SIEM] [Case] Insert timeline bugfix and limit 25 cases (elastic#60136) [ML] Client side cut over (elastic#60100) Disable query bar on service map routes (elastic#60118) Move subscribe_with_scope to kibana_legacy (elastic#59781) [Ingest] Agent Config create with sys monitoring (elastic#60111) [Watcher UI] Not possible to edit a watch that was created with the API if the ID contains a dot (elastic#59383) Actually fetch functionbeat fields needed for telemetry (elastic#60054) ...

* master: (40 commits) skips 'config_open.ts' files from linter check (elastic#60248) [Searchprofiler] Spacing between rendered shards (elastic#60238) Add UiSettings validation & Kibana default route redirection (elastic#59694) [SIEM][CASE] Change configuration button (elastic#60229) [Step 1][App Arch] Saved object migrations from kibana plugin to new platform (elastic#59044) adds new test (elastic#60064) [Uptime] Index Status API to Rest (elastic#59657) [SIEM] Adds 'Closes and opens signals' Cypress test (elastic#59950) [NP] Graph migration (elastic#59409) [ML] Clone analytics job (elastic#59791) Move VALUE_CLICK_TRIGGER and APPLY_FILTER_TRIGGER to ui_action… (elastic#60202) Handle improperly defined Watcher Logging Action text parameter. (elastic#60169) Move select range trigger to uiActions (elastic#60168) [SIEM][CASES] Configure cases: Final (elastic#59358) Closes elastic#59784. Sets xpack.apm.serviceMapEnabled default value true. (elastic#60153) [siem/cypress] update junit filename to be picked up by runbld (elastic#60156) OSS logic added to test environment (elastic#60134) Move canvas setup into app mount (elastic#59926) enable triggers_actions_ui plugin by default (elastic#60137) Expose Elasticsearch from start and deprecate from setup (elastic#59886) ...

maryia-lapata added 5 commits March 3, 2020 16:41

Merge remote-tracking branch 'upstream/master' into np-graph

bd4e263

Move graph to NP

d94d251

Styles

47e9068

Merge remote-tracking branch 'upstream/master' into np-graph

93ae891

Merge branch 'master' into np-graph

d3daf2e

maryia-lapata added WIP Work in progress v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Mar 5, 2020

maryia-lapata added 8 commits March 5, 2020 15:54

Clean up

ae8e156

Fix eslint

5212a64

Fix ESlint

d87c2ba

Fix path

61c5cca

Fix container height

d396330

Clean up

fe7e61a

Update index.ts

69f7ed2

Update graph_client_workspace.js

0a38c7f

Merge branch 'master' into np-graph

0ea6bae

maryia-lapata commented Mar 6, 2020

View reviewed changes

maryia-lapata added 2 commits March 6, 2020 16:57

Refactoring

4efe3d1

Remove unused methods

637a8a3

maryia-lapata changed the title ~~[WIP] [NP] Graph migration~~ [NP] Graph migration Mar 6, 2020

maryia-lapata added Feature:Graph Graph application feature Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// and removed WIP Work in progress labels Mar 6, 2020

maryia-lapata commented Mar 6, 2020

View reviewed changes

flash1293 reviewed Mar 10, 2020

View reviewed changes

maryia-lapata added 6 commits March 11, 2020 10:03

Merge branch 'master' into np-graph

18bc813

Merge branch 'np-graph' of https://github.com/maryia-lapata/kibana in…

e008927

…to np-graph

Rename npData to data

6b46f1b

Move Readme

3fc7d7f

Inline parsing discover url

a2dd603

Remove import of legacy styles

74ee0bc

Update README

4fc604f

maryia-lapata marked this pull request as ready for review March 11, 2020 14:31

maryia-lapata requested a review from a team March 11, 2020 14:31

maryia-lapata requested a review from a team as a code owner March 11, 2020 14:31

Merge branch 'master' into np-graph

c626f4f

flash1293 approved these changes Mar 12, 2020

View reviewed changes

maryia-lapata added 2 commits March 13, 2020 10:53

Merge branch 'master' into np-graph

dadd1eb

Merge branch 'np-graph' of https://github.com/maryia-lapata/kibana in…

c9e874e

…to np-graph

flash1293 mentioned this pull request Mar 13, 2020

Meta: Kibana app NP cutover #60097

Closed

69 tasks

cchaos approved these changes Mar 13, 2020

View reviewed changes

Merge branch 'master' into np-graph

04e17e1

maryia-lapata merged commit 7d12b76 into elastic:master Mar 16, 2020

maryia-lapata deleted the np-graph branch March 16, 2020 07:36

maryia-lapata mentioned this pull request Mar 16, 2020

[7.x] [NP] Graph migration (#59409) #60225

Merged

maryia-lapata added the backported label Mar 16, 2020

Conversation

maryia-lapata commented Mar 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

maryia-lapata commented Mar 6, 2020

Uh oh!

elasticmachine commented Mar 6, 2020

Uh oh!

maryia-lapata Mar 6, 2020

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Mar 6, 2020

Uh oh!

maryia-lapata Mar 6, 2020

Choose a reason for hiding this comment

Uh oh!

maryia-lapata commented Mar 6, 2020

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

flash1293 Mar 10, 2020

Choose a reason for hiding this comment

Uh oh!

maryia-lapata Mar 11, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maryia-lapata commented Mar 11, 2020

Uh oh!

maryia-lapata commented Mar 11, 2020

Uh oh!

flash1293 commented Mar 11, 2020

Uh oh!

flash1293 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

kertal commented Mar 16, 2020

Uh oh!

kibanamachine commented Mar 16, 2020

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

maryia-lapata commented Mar 5, 2020 •

edited

Loading

flash1293 left a comment •

edited

Loading