[Visualize] Shim with local application service by maryia-lapata · Pull Request #49891 · elastic/kibana

maryia-lapata · 2019-10-31T16:24:43Z

Summary

Shims Visualize plugin and uses local angular instance instead of references to global angular to be able to move this to the new platform without getting rid of angular first.

I didn't touch embeddable and saved_visualizations since they will be moved. For this reason centralizing dependencies for embeddable was reverted.

elasticmachine · 2019-12-03T12:13:44Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 843fbe3

elasticmachine · 2019-12-03T19:28:55Z

💚 Build Succeeded

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

flash1293

This looks good to me, tested in Chrome and everything seems to work as expected

src/legacy/core_plugins/kibana/public/visualize/plugin.ts

src/legacy/core_plugins/kibana/public/visualize/index.ts

flash1293 · 2019-12-05T14:22:57Z

src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts

      this.handler.update(this.expression, expressionParams);
    }
+
+    this.vis.emit('apply');


@ppisljar is this the right place to notify the app about changes going on in the embeddable? not super familiar with the control flow here

@ppisljar is OOO for a while now.
I'm going to take over this, lets take a look together and try to figure this out.

elasticmachine · 2019-12-06T01:26:24Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5c82113

crob611

Visualizations still work great when embedded in Canvas. 👍

# Conflicts: # src/legacy/core_plugins/kibana/public/visualize/editor/editor.js # src/legacy/core_plugins/kibana/public/visualize/kibana_services.ts

lizozom

Tested and working on Ubuntu / Chrome

However, I don't get why we in some places the getServices() pattern in removed this PR and replaced by passing down dependencies explicitly.

src/legacy/core_plugins/kibana/public/visualize/embeddable/get_index_pattern.ts

src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable_factory.tsx

lizozom · 2019-12-09T13:33:38Z

src/legacy/core_plugins/kibana/public/visualize/index.ts

+  });
+  instance.start(npStart.core, {
+    ...npStart.plugins,
+    data,


index patterns were moved to NP already, so I think you can use only the NP version of data plugin (and call it plainly data)

I think you can delete both npData and data from this list.
data is defined when you deconstruct plugins

src/legacy/core_plugins/kibana/public/visualize/listing/visualize_listing.js

src/legacy/core_plugins/kibana/public/visualize/plugin.ts

tasks/config/run.js

src/legacy/core_plugins/kibana/public/visualize/global_state_sync.ts

lizozom · 2019-12-09T13:49:23Z

src/legacy/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable.ts

      this.handler.update(this.expression, expressionParams);
    }
+
+    this.vis.emit('apply');


@ppisljar is OOO for a while now.
I'm going to take over this, lets take a look together and try to figure this out.

# Conflicts: # src/legacy/core_plugins/kibana/public/visualize/editor/editor.js # src/legacy/core_plugins/kibana/public/visualize/index.js

maryia-lapata · 2019-12-09T15:35:19Z

@lizozom thank you for the comments.
The visualize embeddables will be moved out of visualize soon. That's why I reverted usage of kibana_services for the embeddable folder.

src/legacy/core_plugins/kibana/public/visualize/index.ts

src/legacy/core_plugins/kibana/public/visualize/kibana_services.ts

elasticmachine · 2019-12-10T09:10:54Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9016c3c

History

💔 Build #14274 failed 51e5691
💔 Build #14250 failed b09c332
💚 Build #14108 succeeded 86306a3

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

lizozom

LGTM

* Add dashboard updates * Use I18nProvider instead of I18nContext * remove unused dependencies * Centralizing and cleaning up legacy imports * Fix merge conflict * fix merge bugs and rename main dynamic entrypoint * Rename app to legacy_app * Clear deps * fix jest tests * fix saved object finder bug * Fix unit tests * Ignore TS * revert using stateless component for this PR * fix types * Fix merge conflicts * Update deps * Revert filter bar export * Revert ts-ignore * Clean up * Refactoring * Fix test * Remove global_state_sync * Refactoring * Remove uiExports/embeddableFactories * Trigger digest cycle in local angular when vis is changed. * Fix TS * Revert back syncOnMount * Add missed import * Revert import 'uiExports/embeddableFactories' * Update app navigation func test * Update app navigation func test * Update app navigation func test * Remove 'kibana-install-dir' arg in pluginFunctionalTestsRelease * Fix review comments * Fix code review comments * Rename alias * Fix indexPatterns * Use IndexPatternsContract interface

flash1293 and others added 30 commits October 18, 2019 09:24

centralize dependencies

62f3164

move more stuff into kibana_services

0bf00ae

fix tests and dependency

e72e51a

bootstrap home via local application service

b42f417

fix routing for home app

0d2521e

Merge remote-tracking branch 'upstream/master' into centralize-home

12fb70b

switch to dependency getter instead of direct exports

af3ad7f

Merge branch 'centralize-home' into local-application-service

a455322

clean up

a1295d3

add redirect functionality to local application service

082748a

return early from route change handlers on dummy wrapper route

2bf3bf6

fix jest tests

398299c

Start shimming dashboard

44bcd46

Merge remote-tracking branch 'upstream/master' into centralize-home

822cdac

fix broken tests

4ea2e63

Merge branch 'centralize-home' into local-application-service

22c5588

local application service

1bd6b3b

Merge remote-tracking branch 'upstream/master' into shim-dashboard

ae8dd12

wip

afafe10

wip

a7f2826

Merge branch 'local-application-service' into shim-dashboard

dbcf159

fix mounting home several times

c417060

Merge branch 'local-application-service' into shim-dashboard

0f2b23b

wip

03fd207

Centralize dependencies for Visualize plugin

160fe52

move handlers from addSetupWork into legacy_compat

eea7c62

pass dependencies into angular configurator

4f0c5b2

centralize dependencies and clean up shim

f1ff388

Merge branch 'master' into flash1293/local-application-service

48ecdf7

Centralizing dependencies in editor and embeddable

0b595a0

Remove 'kibana-install-dir' arg in pluginFunctionalTestsRelease

d58434b

flash1293 mentioned this pull request Dec 4, 2019

Embeddable tests turned off: Functional tests bundle ui/new_platform twice #43087

Closed

flash1293 approved these changes Dec 5, 2019

View reviewed changes

maryia-lapata added 2 commits December 6, 2019 03:01

Merge branch 'master' into vis-shim

0bcb116

Fix review comments

5c82113

crob611 approved these changes Dec 6, 2019

View reviewed changes

Merge remote-tracking branch 'kibana/master' into vis-shim

86306a3

# Conflicts: # src/legacy/core_plugins/kibana/public/visualize/editor/editor.js # src/legacy/core_plugins/kibana/public/visualize/kibana_services.ts

lizozom reviewed Dec 9, 2019

View reviewed changes

maryia-lapata added 3 commits December 9, 2019 18:19

Fix code review comments

3bd47af

Merge remote-tracking branch 'kibana/master' into vis-shim

b0c5243

# Conflicts: # src/legacy/core_plugins/kibana/public/visualize/editor/editor.js # src/legacy/core_plugins/kibana/public/visualize/index.js

Rename alias

b09c332

Fix indexPatterns

51e5691

lizozom reviewed Dec 9, 2019

View reviewed changes

src/legacy/core_plugins/kibana/public/visualize/index.ts Outdated Show resolved Hide resolved

src/legacy/core_plugins/kibana/public/visualize/kibana_services.ts Outdated Show resolved Hide resolved

Use IndexPatternsContract interface

9016c3c

lizozom approved these changes Dec 10, 2019

View reviewed changes

maryia-lapata merged commit 33989b0 into elastic:master Dec 10, 2019

maryia-lapata deleted the vis-shim branch December 10, 2019 10:51

maryia-lapata mentioned this pull request Dec 10, 2019

[7.x] [Visualize] Shim with local application service (#49891) #52611

Merged

maryia-lapata added backported and removed review labels Dec 10, 2019

stacey-gammon mentioned this pull request Dec 12, 2019

Fix and turn back on legacy embeddable tests #52945

Merged

sulemanof mentioned this pull request Dec 19, 2019

Fix vis editor layout #53292

Merged

7 tasks

maryia-lapata mentioned this pull request Jan 17, 2020

Back button doesn’t work in Visualize #55084

Closed

Conversation

maryia-lapata commented Oct 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

elasticmachine commented Dec 3, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Dec 3, 2019

💚 Build Succeeded

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

flash1293 Dec 5, 2019

Choose a reason for hiding this comment

Uh oh!

lizozom Dec 9, 2019

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Dec 6, 2019

💚 Build Succeeded

Uh oh!

crob611 left a comment

Choose a reason for hiding this comment

Uh oh!

lizozom left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lizozom Dec 9, 2019

Choose a reason for hiding this comment

Uh oh!

lizozom Dec 9, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lizozom Dec 9, 2019

Choose a reason for hiding this comment

Uh oh!

maryia-lapata commented Dec 9, 2019

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Dec 10, 2019

💚 Build Succeeded

History

Uh oh!

lizozom left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

maryia-lapata commented Oct 31, 2019 •

edited

Loading