[NP] Dashboard #61895

sulemanof · 2020-03-30T21:28:08Z

Summary

Move the Dashboard plugin to NP.

Note: src/legacy/core_plugins/kibana/public/dashboard/migrations was only left in legacy and will be moved right after this PR is merged.

Changes are mostly related to update imports, files structure and i18n ids.
Other changes:

remove all jest.mock('ui/...'), so completely removed all references to legacy;
in file kbn_url_tracker.ts the createHashHistory() were moved down into onMountApp func to avoid creating a hash history before an app is mounted (the hash history starts adding # ending to the actual url);
since the share plugin is optional, the Share button was disabled if share contract is not exposed

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
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
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

For maintainers

This was checked for breaking API changes and was labeled appropriately

…a_parsed_url

…a_parsed_url # Conflicts: # x-pack/legacy/plugins/lens/public/plugin.tsx

…a_parsed_url # Conflicts: # x-pack/legacy/plugins/lens/public/legacy_imports.ts

# Conflicts: # src/legacy/core_plugins/kibana/public/dashboard/legacy_imports.ts

# Conflicts: # src/legacy/core_plugins/kibana/public/visualize/legacy_imports.ts

# Conflicts: # src/plugins/dashboard/public/dashboard_app.tsx # src/plugins/dashboard/public/dashboard_app_controller.tsx # src/plugins/dashboard/public/lib/embeddable_saved_object_converters.test.ts

flash1293

Added some comments, but this is looking great already! Big step forward 🎉

src/plugins/dashboard/public/application/application.ts

src/plugins/dashboard/public/plugin.tsx

x-pack/plugins/translations/translations/ja-JP.json

flash1293 · 2020-04-02T13:58:11Z

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_constants.ts

- * under the License.
- */
-
-export const DashboardConstants = {


Wow, I wasn't aware we had duplicate code for this stuff, it's good we can finally get rid of it :)

# Conflicts: # src/plugins/dashboard/public/application/actions/expand_panel_action.test.tsx # src/plugins/dashboard/public/application/actions/replace_panel_action.test.tsx # src/plugins/dashboard/public/application/dashboard_app_controller.tsx # src/plugins/dashboard/public/application/embeddable/dashboard_container.test.tsx # src/plugins/dashboard/public/application/embeddable/grid/dashboard_grid.test.tsx # src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.test.tsx

# Conflicts: # src/plugins/dashboard/public/application/dashboard_state_manager.ts

Dosant · 2020-04-03T13:02:42Z

src/plugins/dashboard/public/application/dashboard_state_manager.ts


    // get state defaults from saved dashboard, make sure it is migrated
    this.stateDefaults = migrateAppState(
-      getAppStateDefaults(this.savedDashboard, this.hideWriteControls),
-      kibanaVersion
+      { ...getAppStateDefaults(this.savedDashboard, this.hideWriteControls) },


Wonder what has changed that before this spread wasn't needed?
Or is this fixes some existing bug?

Not sure what caused this, but TypeScript started hate the previous one

I tried to revert and seems like all is good. maybe you had some type issues in a process of moving staff around. Could you double check please?

Okay, that was my fault.
VSCode accidentally thought that I want to use the TypeScript version which it likes more 🙂

And throws me such an issue:

I reverted back those changes, but seems this will become actual again once ts 3.8.2 is merged

https://github.com/elastic/kibana/pull/57774/files#diff-2f67bccb7b0ca326ff2a7aec1b0ee965

Dosant · 2020-04-03T13:04:46Z

src/plugins/dashboard/public/application/legacy_app.js

@@ -94,7 +94,7 @@ export function initDashboardApp(app, deps) {
      .when(DashboardConstants.LANDING_PAGE_PATH, {
        ...defaults,
        template: dashboardListingTemplate,
-        controller($scope, kbnUrlStateStorage, history) {
+        controller: function($scope, kbnUrlStateStorage, history) {


this function changes something? or no need?

Ohhh, that's a very interesting point!
So, it was working fine in legacy platform, but stop working in the new platform.
The failure was something like: Angular can't bootstrap..., the reason was the controller is invoked as new controller() inside the angular and this particular function was not converted to a function declaration and acts like an arrow function.
So seems we have different babel configs for legacy and new platform (in legacy object methods are converted to function declarations, but in new platform they are not).
Maybe @joshdover could clarify this? 🙂

Dosant · 2020-04-03T13:33:00Z

src/plugins/dashboard/public/application/lib/migrate_app_state.ts

-} from '../types';
-import { migratePanelsTo730 } from '../../migrations/migrate_to_730_panels';
+} from '../../types';
+// should be moved in src/plugins/dashboard/common right after https://github.com/elastic/kibana/pull/61895 is merged


Could you please clarify? this is a link to the same pr 🤔
circular dependency :D

Ah, yeah, awkward.
An infinite loop 😆
That actually means a separate PR will be created with moving those right after this one is merged 😃

Dosant

Tested in chrome. Was focused on navigation between apps. And _g preserving between navigations.
Didn't notice anything

flash1293

Tested in Chrome and everything works as expected, LGTM 👍
(Assuming the i18n thing is fixed, should be a really small change)

sulemanof · 2020-04-04T13:09:37Z

@elasticmachine merge upstream

kibanamachine · 2020-04-04T14:45:53Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 1886a6e

History

💔 Build #38478 failed 2a32285
💔 Build #38376 failed 2296e2b
💚 Build #38356 succeeded 3ab2a67
💚 Build #38201 succeeded d1c9446
💚 Build #38174 succeeded 3bd0afd

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

snide

Sass file moves typical of NP migration. Did not check the JS level.

Dosant

Code lgtm, didn't retest after previous comment

* Remove absoluteToParsedUrl reference in dashboard * Remove KibanaParsedUrl from visualize * Fix tests * Add tests * Fix saved dashboard * Fix empty line after resolving conflicts * Move dashboard to np * Move migrations back to legacy * Make it works * Other fixes * Move into application folder * FIx translations * Make share & home plugins otional * FIx kbn url tracking, jest tests * Import from dashboard_constants in FT * Fix translations order * Use getStartServices for start plugin deps * Path fixes * i18n fix Co-authored-by: Elastic Machine <[email protected]>

* Remove absoluteToParsedUrl reference in dashboard * Remove KibanaParsedUrl from visualize * Fix tests * Add tests * Fix saved dashboard * Fix empty line after resolving conflicts * Move dashboard to np * Move migrations back to legacy * Make it works * Other fixes * Move into application folder * FIx translations * Make share & home plugins otional * FIx kbn url tracking, jest tests * Import from dashboard_constants in FT * Fix translations order * Use getStartServices for start plugin deps * Path fixes * i18n fix Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

sulemanof added 24 commits March 24, 2020 16:57

Remove absoluteToParsedUrl reference in dashboard

4ffa207

Remove KibanaParsedUrl from visualize

901c13a

Fix tests

860f6b7

Merge remote-tracking branch 'upstream/master' into shim/remove_kiban…

95412fa

…a_parsed_url

Add tests

fe0afc0

Merge remote-tracking branch 'upstream/master' into shim/remove_kiban…

9d1a9a9

…a_parsed_url # Conflicts: # x-pack/legacy/plugins/lens/public/plugin.tsx

Fix saved dashboard

64bea6e

Merge remote-tracking branch 'upstream/master' into shim/remove_kiban…

b994157

…a_parsed_url # Conflicts: # x-pack/legacy/plugins/lens/public/plugin.tsx

Merge remote-tracking branch 'upstream/master' into shim/remove_kiban…

d709340

…a_parsed_url # Conflicts: # x-pack/legacy/plugins/lens/public/legacy_imports.ts

Fix empty line after resolving conflicts

922c9c1

Move dashboard to np

f71e6e2

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

799f5b8

# Conflicts: # src/legacy/core_plugins/kibana/public/dashboard/legacy_imports.ts

Move migrations back to legacy

7547af3

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

474b906

# Conflicts: # src/legacy/core_plugins/kibana/public/visualize/legacy_imports.ts

Make it works

81f62ba

Other fixes

275ab26

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

a081cc4

# Conflicts: # src/plugins/dashboard/public/dashboard_app.tsx # src/plugins/dashboard/public/dashboard_app_controller.tsx # src/plugins/dashboard/public/lib/embeddable_saved_object_converters.test.ts

Move into application folder

e71c658

FIx translations

3296e9a

Make share & home plugins otional

7b19c92

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

aaa4d7d

FIx kbn url tracking, jest tests

6e2d552

Import from dashboard_constants in FT

24e7518

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

21ac319

sulemanof marked this pull request as ready for review April 1, 2020 12:27

sulemanof requested a review from a team April 1, 2020 12:27

sulemanof requested review from a team as code owners April 1, 2020 12:27

sulemanof added Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes labels Apr 1, 2020

flash1293 added the v7.8.0 label Apr 2, 2020

flash1293 reviewed Apr 2, 2020

View reviewed changes

sulemanof added 5 commits April 2, 2020 17:23

Merge branch 'master' into np/dashboard

5e51637

Fix translations order

3bd0afd

Use getStartServices for start plugin deps

d1c9446

Path fixes

3ab2a67

botelastic bot added the Feature:Embedding Embedding content via iFrame label Apr 3, 2020

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

2296e2b

# Conflicts: # src/plugins/dashboard/public/application/dashboard_state_manager.ts

Dosant assigned Dosant and unassigned Dosant Apr 3, 2020

Dosant self-requested a review April 3, 2020 11:19

Dosant reviewed Apr 3, 2020

View reviewed changes

flash1293 approved these changes Apr 3, 2020

View reviewed changes

sulemanof added 2 commits April 3, 2020 19:33

i18n fix

f732503

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

2a32285

Merge branch 'master' into np/dashboard

1886a6e

snide approved these changes Apr 6, 2020

View reviewed changes

Dosant approved these changes Apr 6, 2020

View reviewed changes

sulemanof merged commit 104b490 into elastic:master Apr 6, 2020

sulemanof deleted the np/dashboard branch April 6, 2020 09:11

sulemanof mentioned this pull request Apr 6, 2020

[7.x] [NP] Dashboard (#61895) #62574

Merged

sulemanof added the backported label Apr 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NP] Dashboard #61895

[NP] Dashboard #61895

sulemanof commented Mar 30, 2020 •

edited

Loading

flash1293 left a comment

flash1293 Apr 2, 2020

Dosant Apr 3, 2020

sulemanof Apr 3, 2020

Dosant Apr 3, 2020

sulemanof Apr 3, 2020

Dosant Apr 3, 2020

sulemanof Apr 3, 2020

Dosant Apr 3, 2020

sulemanof Apr 3, 2020

Dosant left a comment

flash1293 left a comment

sulemanof commented Apr 4, 2020

kibanamachine commented Apr 4, 2020

snide left a comment

Dosant left a comment

[NP] Dashboard #61895

[NP] Dashboard #61895

Conversation

sulemanof commented Mar 30, 2020 • edited Loading

Summary

Checklist

For maintainers

flash1293 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

Dosant left a comment

Choose a reason for hiding this comment

flash1293 left a comment

Choose a reason for hiding this comment

sulemanof commented Apr 4, 2020

kibanamachine commented Apr 4, 2020

💚 Build Succeeded

History

snide left a comment

Choose a reason for hiding this comment

Dosant left a comment

Choose a reason for hiding this comment

sulemanof commented Mar 30, 2020 •

edited

Loading