Dashboard Lists Integration #3090

pjfitzgibbons · 2022-12-15T20:50:58Z

Description

Configure types for registration of Dashboard List "Providers"

These changes enable the interactions demonstrated in this animation:

Audio starts at 0:05. Unmute to listen.

Dashboard-integration-demo2.mov

References : observability#1308

Issues WIP

[FEATURE] Dashboards List Extensibility & Integrate Observability Apps/Panels/EventAnalytics

includes alternate solutions, example animation, and further explanation of this solution.

Testing Failures vs UX/CX of Linking "integrated" Plugins

Using EUILink w/ href=

Pros :

Works in test suite w/o change.

Cons :

Causes browser-reload in production due to operation of React Router / SPA and navigating "between" SPA's

Using EUILink w/ onClick=(() => navigateToApp(app, path) )

Pros :

Runs on browser production without reload when navigating "between" SPA's

Cons :

Test suite fails on Dashboard-Clone test, when trying to click on Dashboard-List for any existing Dashboard item.
http://.../app/dashboard?_t=123456#/list?_g=...

Check List

ashwin-pc · 2022-12-15T21:28:02Z

@pjfitzgibbons is this the same as #3000?

pjfitzgibbons · 2022-12-16T17:14:18Z

Yes, this time basebranch "main"

…

On Thu, Dec 15, 2022 at 1:28 PM Ashwin P Chandran ***@***.***> wrote: @pjfitzgibbons <https://github.com/pjfitzgibbons> is this the same as #3000 <#3000>? — Reply to this email directly, view it on GitHub <#3090 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACYNSPO5Z7GORXUIDJHUTWNOEO3ANCNFSM6AAAAAATAFFESE> . You are receiving this because you were mentioned.Message ID: ***@***.*** com>

ashwin-pc

I didnt get too far into the review but I just pulled down the PR and ran it when i came across a few issues.

On clicking that button I get the following error.

config/opensearch_dashboards.yml

src/plugins/dashboard/common/types.ts

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

src/plugins/dashboard/public/plugin.tsx

src/plugins/opensearch_dashboards_react/public/context/types.ts

seanneumann · 2023-01-05T18:22:35Z

How are we doing on this PR?

pjfitzgibbons · 2023-01-05T19:01:10Z

@seanneumann Ready for continuing review. Previous quesitons addressed.

seanneumann · 2023-01-05T19:08:27Z

@pjfitzgibbons cool. Can you also have someone in your group review it too? Would love to see their feedback, too if possible.

ashwin-pc · 2023-01-05T21:07:14Z

@pjfitzgibbons I'm still not able to make this feature work when i clone it locally and run it. I still see the same error that I called out in my last review. Do you have a different OSD setup? do you have any plugins installed?

pjfitzgibbons · 2023-01-06T18:09:32Z

@ashwin-pc Error on default Dashboard-create fixed now.

anirudha · 2023-01-06T19:38:58Z

@pjfitzgibbons can we also have josh/ eric review this

src/plugins/dashboard/public/plugin.tsx

AMoo-Miki · 2023-01-06T21:00:50Z

Test snapshots need to be updated and it would be great if the PR was rebased.

pjfitzgibbons · 2023-01-06T21:12:37Z

@AMoo-Miki Working on test updates now...

pjfitzgibbons · 2023-01-06T21:38:41Z

Test snapshots need to be updated and it would be great if the PR was rebased.

Updating tests momentarily...

pjfitzgibbons · 2023-01-06T22:35:07Z

Cypress tests are failing from upstream :
dashboard sample data validation -- adding sample data -- checking Visualize
All other cypress tests pass.

codecov-commenter · 2023-01-06T22:40:10Z

Codecov Report

Merging #3090 (5e23be7) into main (53047b6) will decrease coverage by 0.01%.
The diff coverage is 52.27%.

❗ Current head 5e23be7 differs from pull request most recent head 1bc9c2c. Consider uploading reports for the commit 1bc9c2c to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #3090      +/-   ##
==========================================
- Coverage   66.45%   66.44%   -0.01%     
==========================================
  Files        3209     3210       +1     
  Lines       61633    61670      +37     
  Branches     9506     9514       +8     
==========================================
+ Hits        40960    40979      +19     
- Misses      18396    18412      +16     
- Partials     2277     2279       +2

Flag	Coverage Δ
Linux	`66.39% <52.27%> (-0.01%)`	⬇️
Windows	`66.39% <52.27%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/plugins/dashboard/public/plugin.tsx	`0.00% <0.00%> (ø)`
...s_react/public/table_list_view/table_list_view.tsx	`0.00% <0.00%> (ø)`
...board/public/application/listing/create_button.tsx	`91.66% <91.66%> (ø)`
...rd/public/application/listing/dashboard_listing.js	`76.92% <100.00%> (-1.65%)`	⬇️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

joshuarrrr

@pjfitzgibbons I haven't finished reviewing this yet, but I wanted to flag that I don't think we should have any dashboard-app-specific props in the generic react components like table_list_view. It also seems like we have a number of duplicated types that would be useful to consolidate.

CHANGELOG.md

src/plugins/dashboard/common/types.ts

src/plugins/opensearch_dashboards_react/public/table_list_view/table_list_view.tsx

ashwin-pc

I was able to successfully pull the code and verify that it works. I did not verify the behavior with additional dashboard creators though. Is there an easy way for me to verify that it works without installing the observability plugin?

src/plugins/dashboard/common/types.ts

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

src/plugins/dashboard/public/application/listing/__snapshots__/dashboard_listing.test.js.snap

src/plugins/dashboard/public/types.ts

src/plugins/opensearch_dashboards_react/public/context/types.ts

...plugins/opensearch_dashboards_react/public/table_list_view/components/create_button_menu.tsx

pjfitzgibbons · 2023-03-21T19:15:16Z

Sigh. TWO character commit.

pjfitzgibbons · 2023-03-28T17:44:07Z

Meeting notes from call 3/27/2023 3:30 PM
Attendees : @seanneumann @anirudha @kavilla @pjfitzgibbons @kgcreative

Discussed EUILink pros/cons as described in Description of this PR.

Final decision, supported by A. Jadhav, S.Neumann and K. Avilla, K. Garcia :

Change will be restored to use <EUILink href=...> which passes all tests locally (P.Fitzgibbons) from ciGroup5, ciGroup7, ciGroup9.
The meeting group acknowledged that browser navigation "between" plugins, including OSD's own plugins, cause a server-side request. This is due to the way React Router/HashRouter behave with SSR.
The browser-reload when navigating between plugins will be addressed in a subsequent PR to OSD, where the concern will be addressed at the platform level.

Final commits to this PR will be posted today and CI run is expected to be all-green. PR will then be ready for merge with support from K.Avilla and evidence of all previous reviews.

kavilla · 2023-03-28T23:43:32Z

test/functional/services/common/browser.ts

@@ -196,7 +196,7 @@ export async function BrowserProvider({ getService }: FtrProviderContext) {
     * @param {boolean} insertTimestamp Optional
     * @return {Promise<void>}
     */
-    public async get(url: string, insertTimestamp: boolean = true) {
+    public async get(url: string, insertTimestamp: boolean = false) {


the test is failing due to this change. could we revert this back to true?

* Plugins register their links with registerDashboardProvider API Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com>

Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com>

pjfitzgibbons · 2023-03-29T16:16:47Z

Cypress passes locally :

  34 passing (1m)


  (Results)

  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ Tests:        34                                                                               │
  │ Passing:      34                                                                               │
  │ Failing:      0                                                                                │
  │ Pending:      0                                                                                │
  │ Skipped:      0                                                                                │
  │ Screenshots:  0                                                                                │
  │ Video:        true                                                                             │
  │ Duration:     1 minute, 18 seconds                                                             │
  │ Spec Ran:     core-opensearch-dashboards/opensearch-dashboards/dashboard_sanity_test_spec.js   │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘


  (Video)

  -  Started processing:  Compressing to 32 CRF                                                     
  -  Finished processing: /Users/pjfitz/IdeaProjects/OpenSearch/opensearch-dashboards    (8 seconds)
                          -functional-test/cypress/videos/core-opensearch-dashboards/               
                          opensearch-dashboards/dashboard_sanity_test_spec.js.mp4

ARM appears to be a platform issue :

   │ debg Attempting download of https://mirrors.nodejs.org/dist/v14.21.3/node-v14.21.3-linux-arm64.tar.gz 044b7ec3fea04cd3815d26901ee37203dcc942688b72ee6eac96f6a1ca3cc63f
   │ debg Download failed: Request failed with status code 500

joshuarrrr · 2023-03-29T23:39:08Z

cypress test failures can be ignored - they're related to vis builder changes

joshuarrrr

Thanks!

CHANGELOG.md

joshuarrrr · 2023-03-30T23:01:25Z

src/plugins/opensearch_dashboards_react/public/table_list_view/README.md

nice, thanks for providing!

Signed-off-by: Josh Romero <rmerqg@amazon.com>

* Dashboard-List Integration * Plugins register their links with registerDashboardProvider API Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com> * Update CHANGELOG.md Signed-off-by: Josh Romero <rmerqg@amazon.com> --------- Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com> Signed-off-by: Josh Romero <rmerqg@amazon.com> Co-authored-by: Peter Fitzgibbons <pjfitz@amazon.com> Co-authored-by: Josh Romero <rmerqg@amazon.com> (cherry picked from commit d2bd318) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md

* Dashboard-List Integration * Plugins register their links with registerDashboardProvider API Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com> * Update CHANGELOG.md Signed-off-by: Josh Romero <rmerqg@amazon.com> --------- Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com> Signed-off-by: Josh Romero <rmerqg@amazon.com> Co-authored-by: Peter Fitzgibbons <pjfitz@amazon.com> Co-authored-by: Josh Romero <rmerqg@amazon.com> Signed-off-by: vagimeli <vagimeli@amazon.com>

* Dashboard-List Integration * Plugins register their links with registerDashboardProvider API Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com> * Update CHANGELOG.md Signed-off-by: Josh Romero <rmerqg@amazon.com> --------- Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com> Signed-off-by: Josh Romero <rmerqg@amazon.com> Co-authored-by: Peter Fitzgibbons <pjfitz@amazon.com> Co-authored-by: Josh Romero <rmerqg@amazon.com> (cherry picked from commit d2bd318) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Dashboard-List Integration * Plugins register their links with registerDashboardProvider API Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com> * Update CHANGELOG.md Signed-off-by: Josh Romero <rmerqg@amazon.com> --------- Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com> Signed-off-by: Josh Romero <rmerqg@amazon.com> Co-authored-by: Peter Fitzgibbons <pjfitz@amazon.com> Co-authored-by: Josh Romero <rmerqg@amazon.com> Signed-off-by: David Sinclair <david@sinclair.tech>

pjfitzgibbons requested a review from a team as a code owner December 15, 2022 20:50

pjfitzgibbons force-pushed the feature/dashboard-list-integration.7 branch from dfe9fe1 to ad46ccb Compare December 15, 2022 21:06

ashwin-pc reviewed Dec 17, 2022

View reviewed changes

ashwin-pc mentioned this pull request Dec 17, 2022

Dashboard Lists Integration #3000

Closed

8 tasks

pjfitzgibbons mentioned this pull request Jan 4, 2023

[FEATURE] Dashboards List Extensions #1866

Closed

joshuali925 reviewed Jan 6, 2023

View reviewed changes

src/plugins/dashboard/public/plugin.tsx Outdated Show resolved Hide resolved

joshuarrrr added v2.5.0 'Issues and PRs related to version v2.5.0' backport 2.x labels Jan 6, 2023

abbyhu2000 added the backport 2.5 label Jan 7, 2023

pjfitzgibbons requested review from ashwin-pc and joshuali925 and removed request for ashwin-pc and joshuali925 January 9, 2023 23:34

joshuarrrr previously requested changes Jan 10, 2023

View reviewed changes

ashwin-pc reviewed Jan 10, 2023

View reviewed changes

vagimeli mentioned this pull request Jan 10, 2023

[DOC] Dashboards list extensibility opensearch-project/documentation-website#2203

Closed

4 tasks

joshuarrrr mentioned this pull request Mar 23, 2023

2.7.0 Platform Upgrades for Plugins (O11y, MDS, Geospatial,+) #3674

Closed

23 tasks

pjfitzgibbons force-pushed the feature/dashboard-list-integration.7 branch 3 times, most recently from e517b11 to 72b9434 Compare March 25, 2023 00:15

kavilla mentioned this pull request Mar 27, 2023

Major.minor.patch definitions opensearch-project/opensearch-dashboards-sdk-js#41

Open

kavilla reviewed Mar 28, 2023

View reviewed changes

Peter Fitzgibbons added 3 commits March 29, 2023 04:06

Dashboard-List Integration

c9a25c0

* Plugins register their links with registerDashboardProvider API Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com>

Update Dashboard Listing test snapshots.

b1aab11

Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com>

Use href for Dashboard List "view" link.

5e23be7

Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com>

pjfitzgibbons force-pushed the feature/dashboard-list-integration.7 branch from 926d481 to 5e23be7 Compare March 29, 2023 11:06

Reset browser automation - get

1bc9c2c

Signed-off-by: Peter Fitzgibbons <pjfitz@amazon.com>

joshuarrrr linked an issue Mar 30, 2023 that may be closed by this pull request

[FEATURE] Dashboards List Extensions #1866

Closed

kavilla previously approved these changes Mar 30, 2023

View reviewed changes

joshuarrrr previously approved these changes Mar 30, 2023

View reviewed changes

Update CHANGELOG.md

d719339

Signed-off-by: Josh Romero <rmerqg@amazon.com>

joshuarrrr dismissed stale reviews from kavilla and themself via d719339 March 30, 2023 23:08

joshuarrrr approved these changes Mar 30, 2023

View reviewed changes

kavilla approved these changes Mar 30, 2023

View reviewed changes

joshuarrrr merged commit d2bd318 into opensearch-project:main Mar 30, 2023

opensearch-trigger-bot bot mentioned this pull request Mar 30, 2023

[Backport 2.x] Dashboard Lists Integration #3749

Merged

junqiu-lei mentioned this pull request Apr 17, 2023

Fix cypress test failed due to OSD dashboard create button updated opensearch-project/dashboards-maps#393

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dashboard Lists Integration #3090

Dashboard Lists Integration #3090

pjfitzgibbons commented Dec 15, 2022 •

edited

Loading

ashwin-pc commented Dec 15, 2022

pjfitzgibbons commented Dec 16, 2022 via email

ashwin-pc left a comment

seanneumann commented Jan 5, 2023

pjfitzgibbons commented Jan 5, 2023

seanneumann commented Jan 5, 2023

ashwin-pc commented Jan 5, 2023

pjfitzgibbons commented Jan 6, 2023

anirudha commented Jan 6, 2023

AMoo-Miki commented Jan 6, 2023

pjfitzgibbons commented Jan 6, 2023

pjfitzgibbons commented Jan 6, 2023

pjfitzgibbons commented Jan 6, 2023

codecov-commenter commented Jan 6, 2023 •

edited

Loading

joshuarrrr left a comment

ashwin-pc left a comment

pjfitzgibbons commented Mar 21, 2023

pjfitzgibbons commented Mar 28, 2023

kavilla Mar 28, 2023

pjfitzgibbons commented Mar 29, 2023

joshuarrrr commented Mar 29, 2023

joshuarrrr left a comment

joshuarrrr Mar 30, 2023

Dashboard Lists Integration #3090

Dashboard Lists Integration #3090

Conversation

pjfitzgibbons commented Dec 15, 2022 • edited Loading

Description

Issues WIP

Testing Failures vs UX/CX of Linking "integrated" Plugins

Using EUILink w/ href=

Using EUILink w/ onClick=(() => navigateToApp(app, path) )

Check List

ashwin-pc commented Dec 15, 2022

pjfitzgibbons commented Dec 16, 2022 via email

ashwin-pc left a comment

Choose a reason for hiding this comment

seanneumann commented Jan 5, 2023

pjfitzgibbons commented Jan 5, 2023

seanneumann commented Jan 5, 2023

ashwin-pc commented Jan 5, 2023

pjfitzgibbons commented Jan 6, 2023

anirudha commented Jan 6, 2023

AMoo-Miki commented Jan 6, 2023

pjfitzgibbons commented Jan 6, 2023

pjfitzgibbons commented Jan 6, 2023

pjfitzgibbons commented Jan 6, 2023

codecov-commenter commented Jan 6, 2023 • edited Loading

Codecov Report

joshuarrrr left a comment

Choose a reason for hiding this comment

ashwin-pc left a comment

Choose a reason for hiding this comment

pjfitzgibbons commented Mar 21, 2023

pjfitzgibbons commented Mar 28, 2023

kavilla Mar 28, 2023

Choose a reason for hiding this comment

pjfitzgibbons commented Mar 29, 2023

joshuarrrr commented Mar 29, 2023

joshuarrrr left a comment

Choose a reason for hiding this comment

joshuarrrr Mar 30, 2023

Choose a reason for hiding this comment

pjfitzgibbons commented Dec 15, 2022 •

edited

Loading

codecov-commenter commented Jan 6, 2023 •

edited

Loading