[Feature] Dashboards List Extensibility #2149

pjfitzgibbons · 2022-08-16T21:50:56Z

Description

Extend OSD Dashboard List to allow other plugins to supply their own list of "dashboards" to be appended to the OSD Dashboard List.

"Edit" column on Dashboard List availability is set per-row, dependent upon presence of editUrl value on each input row.

Issues WIP

[FEATURE] Integrate Observability Panels/Apps lists into OpenSearch-Dashboards main List View

Related PR

Observability Plugin : [WIP] [FEATURE] Integrate Dashboards/Observability

Check List

Convert dashboards lists legacy_app.js to TS Signed-off-by: Peter Fitzgibbons <[email protected]>

Moved types to ./types.ts modules. Separated dependency of types between OSD and Observability Plugin Signed-off-by: Peter Fitzgibbons <[email protected]>

Integrated Observability Panel List Provider - added Type column to Dashboard List - enabled Search on Dashboad List for all columns/records Signed-off-by: Peter Fitzgibbons <[email protected]>

- Observability Apps integrated to Dashboard List - "Edit" column availability per-row dependent upon item editUrl presence Signed-off-by: Peter Fitzgibbons <[email protected]>

ashwin-pc

The approach looks good. A few comments/questions.

Since this modifies a core part of the dashboard experience, can we add some tests to ensure that the new functionality does not break anything?
Can you add some screenshots of how the new experience will look like since you seem to be adding a new column to the Dashboards listing table
Existing tests are failing for the dashboard listing component.

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

ashwin-pc · 2022-08-17T01:18:51Z

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

+export interface DashboardListItem {
+  id: string;
+  title: string;
+  type: string;
+  description: string;
+  url: string;
+  listType: string;
+}
+
+export type DashboardListItems = DashboardListItem[];
+export type DashboardListProviderFn = () => Observable<DashboardListItems>;


Why do we have this defined in 2 places?

ashwin-pc · 2022-08-17T01:20:03Z

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

+export interface DashboardListSource {
+  name: string;
+  listProviderFn: () => Observable<DashboardListItem>;
+}
+
+export type DashboardListSources = DashboardListSource[];


Why do we define this in multiple places?

Fixed in newest rev.

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

Update dashboard_listing.test.js and snap to match added component fields Signed-off-by: Peter Fitzgibbons <[email protected]>

Remove duplicated declarations for DashboardListSource/DashboardListSources types Signed-off-by: Peter Fitzgibbons <[email protected]>

ashwin-pc · 2022-08-24T00:42:09Z

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

 import { Storage } from '../../../opensearch_dashboards_utils/public';
 // @ts-ignore
-import { initDashboardApp } from './legacy_app';
+import { DashboardListItem, DashboardListProviderFn, initDashboardApp } from './legacy_app';


I see this import in the diff but DashboardListItem && DashboardListProviderFn dont seem to be used anywhere in this file

ashwin-pc · 2022-08-24T00:45:20Z

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

+export type DashboardListItems = DashboardListItem[];
+export type DashboardListProviderFn = () => Observable<DashboardListItems>;
+export interface DashboardDisplay {
+  hits: DashboardListItem[];


nit: You already have DashboardListItems. Id either remove that or use that here

ashwin-pc · 2022-08-24T00:50:42Z

src/plugins/dashboard/public/types.ts

+export interface DashboardListSource {
+  name: string;
+  listProviderFn: () => Observable<DashboardListItem>;
+}
+
+export type DashboardListSources = DashboardListSource[];
+
+export interface DashboardListItem {
+  id: string;
+  title: string;
+  type: string;
+  description: string;
+  url: string;
+  listType: string;
+}


Also defined in legacy_app.ts

ashwin-pc · 2022-08-27T01:55:20Z

Hey @pjfitzgibbons, thanks for addressing most of the comments. I dont see any changes since my latest review though. The 3 remaining callouts I had for the PR are:

There still seems to be a duplicate interface for DashboardListItem and DashboardListItems
Tests for the new functionality (You can just add to the existing functional tests for the component)
Fixing the existing Functional tests that are breaking in the CI runner

Correct sorting of Dashboard List by items' Title, Type, or Description. Signed-off-by: Peter Fitzgibbons <[email protected]>

seanneumann · 2022-09-26T20:52:42Z

Can we get an update?

pjfitzgibbons · 2022-09-26T22:29:23Z

Will have a new commit to post on Tue. Sep 27 Thanks and Kindest Regards, Peter Fitzgibbons (224) 307-9689

…

On Mon, Sep 26, 2022 at 1:52 PM Sean Neumann ***@***.***> wrote: Can we get an update? — Reply to this email directly, view it on GitHub <#2149 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACYNR3CB4OFIQUBFFCHZLWAIEKLANCNFSM56XLMXOA> . You are receiving this because you were mentioned.Message ID: ***@***.*** com>

Allow plugins to register as "Create provider", adding itself to the "Create" button of Dashboard List. Signed-off-by: Peter Fitzgibbons <[email protected]>

joshuarrrr · 2022-09-28T18:49:17Z

yarn.lock

+  version "1.0.30001407"
+  resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001407.tgz"
+  integrity sha512-4ydV+t4P7X3zH83fQWNDX/mQEzYomossfpViCOx9zHBSMV+rIe3LFqglHHtVyvNl1FhTNxPxs3jei82iqOW04w==


This file should be removed from this PR and made as a separate PR if needed (it's also causing a merge conflict at the moment). I've also created a general tracking issue for this problem here: #2429

joshuarrrr · 2022-10-04T18:13:47Z

@pjfitzgibbons Just following up to see if you're able to update the PR an resolve conflicts or remove the yarn.lock file.

…ct#1550) (opensearch-project#1707) * Fix WMS can't load when unable access maps services Signed-off-by: Junqiu Lei <[email protected]> (cherry picked from commit b2cfb6e)

…ct#1750) Signed-off-by: Tommy Markley <[email protected]> (cherry picked from commit 72a3424) Co-authored-by: Tommy Markley <[email protected]>

Signed-off-by: Zelin Hao <[email protected]>

) (opensearch-project#1774) Incorrectly set the date range of the library. This sets the value correctly and turns the dates into consts. Issue: n/a Signed-off-by: Kawika Avilla <[email protected]> (cherry picked from commit 4703abf) Co-authored-by: Kawika Avilla <[email protected]>

…ch-project#2107) Issue: opensearch-project#1873 This was happening because of a prop useDefaultGroupDomain. This prop was introduced into the elastic charts to allow clustering for the multiple series which will share a common Y domain i.e for example the stacked and non- stacked bars will have the same Y common domain/axis. As per the elastic/charts whenever the useDefaultGroupDomain is set to true, it will force to group the series/visualiations groupId's to a default group domain. But, TSVB is not a shared Y domain. So the prop is unneeded. Further insight: From the code: ``` const groupId = hasSeparateAxis || isStackedWithinSeries ? seriesGroup.id : mainAxisGroupId; ``` where ``` const mainAxisGroupId = yAxisIdGenerator('main_group'); ``` ... which indicates that the global default `groupId` is not used by TSVB. Hence, TSVB should have not been using `useDefaultGroupDomain` ever. Signed-off-by: AbhishekReddy1127 <[email protected]> (cherry picked from commit 55181d4) Co-authored-by: Abhishek Reddy <[email protected]>

…ns (opensearch-project#1946) (opensearch-project#1959) Resolves issue - opensearch-project#1895 Signed-off-by: Manasvini B Suryanarayana <[email protected]> (cherry picked from commit 2c428b8) Co-authored-by: Manasvini B Suryanarayana <[email protected]> Co-authored-by: Kawika Avilla <[email protected]>

…-project#2244) Enable filtering with custom health checks based on node attributes: ``` opensearch.optimizedHealthcheck.filters: { attribute_key: "attribute_value", } ``` Also, fixes issue that expects the response to array when it was a dictionary. Issue: opensearch-project#2214 opensearch-project#2203 Signed-off-by: Kawika Avilla <[email protected]> (cherry picked from commit e6bbb40) Co-authored-by: Kawika Avilla <[email protected]>

…ch-project#2277) (opensearch-project#2298) Original implementation incorrectly assumed the return list of nodes was an object array. This PR: opensearch-project#2232 Addressed the return but didn't catch the nodes.find in the return which is a function for an array. Also, refactors to return a list of node_ids because the original implementation indicated that it should but it can return node ids but it never did. It only returned `null` or `_local`, the problem with this approach is that it doesn't expect valid node version with different DIs or filter out nodes when we pass `null` as the node ID for the node info call because it was fan out the request to all nodes. Now this function will return `_local` if all the nodes share the same cluster_id using a greedy approach since we can assume it is all the same version. Will return node ids, if the cluster_id are different so it will pass a CSV to the node info call and return the info for those nodes. And null if no cluster_id is present, ie, fan out the response. Original issue: opensearch-project#2203 Signed-off-by: Kawika Avilla <[email protected]> Signed-off-by: Kawika Avilla <[email protected]>

…ch-project#1948) (opensearch-project#1955) (opensearch-project#2388) Issue: n/a Backport PRs will also run actions for pushs and PRs which is just unneeded double work everytime. So this prevents that. Also, BWC will run a complete array for every job in the matrix. Now we set the environment variable to prevents the running of the default versions. Finally updating date range of filter tests to be more inclusive for new test data. Signed-off-by: Kawika Avilla <[email protected]> (cherry picked from commit 57a751e) (cherry picked from commit dfe13d6) Co-authored-by: Kawika Avilla <[email protected]>

…roject#1823) (opensearch-project#2387) Maps zoom level were updated to be 14 as noted here: opensearch-project/maps#4 (comment) Issue: n/a Signed-off-by: Kawika Avilla <[email protected]> (cherry picked from commit e7362f9) Co-authored-by: Kawika Avilla <[email protected]>

…pensearch-project#2366) ### Description Updates `moment-timezone` Dependabot PR: opensearch-project#2229 ### Issues Resolved opensearch-project#2262 opensearch-project#2263 Signed-off-by: Ashwin Pc <[email protected]> (cherry picked from commit 77b6068) Co-authored-by: Ashwin P Chandran <[email protected]> Co-authored-by: Kawika Avilla <[email protected]>

…search-project#2352) (opensearch-project#2371) * Pass `options` to `vega.parse` to enable inclusion of parsed ASTs * Introduce the forced CSP-compliant interpreter that prevents evaluation of unsafe methods * Modified the consumed `leaflet-vega` package to one that honors `options` Signed-off-by: Miki <[email protected]> Signed-off-by: Miki <[email protected]> (cherry picked from commit bebbcca) Co-authored-by: Miki <[email protected]> Co-authored-by: Ashwin P Chandran <[email protected]> Co-authored-by: Kawika Avilla <[email protected]>

* Register App Analytics, Panels as "Dashboard List Providers" (requires opensearch-project/OpenSearch-Dashboards#2149) * Register App Analytics, Event Analytics as "Dashboard Left-Nav Links", using existing app.register functions of OSD. Create a new "top-level" Left-Nav section "Observability" Signed-off-by: Peter Fitzgibbons <[email protected]>

* Configure types for registration of Dashboard List "Providers" and "Creators" * Add Left-Nav "top-level" category for Observability Signed-off-by: Peter Fitzgibbons <[email protected]>

joshuarrrr · 2022-11-14T19:48:22Z

@pjfitzgibbons Should this PR be closed in favor of #2670, or are you still working on this? For now, I'm going to set it to draft since it's stale.

* Register App Analytics, Panels as "Dashboard List Providers" (requires opensearch-project/OpenSearch-Dashboards#2149) * Register App Analytics, Event Analytics as "Dashboard Left-Nav Links", using existing app.register functions of OSD. Create a new "top-level" Left-Nav section "Observability" Signed-off-by: Peter Fitzgibbons <[email protected]>

* Register App Analytics, Panels as "Dashboard List Providers" (requires opensearch-project#2149) * Register App Analytics, Event Analytics as "Dashboard Left-Nav Links", using existing app.register functions of OSD. Create a new "top-level" Left-Nav section "Observability" Signed-off-by: Peter Fitzgibbons <[email protected]>

Peter Fitzgibbons added 4 commits August 11, 2022 14:57

Dashboard List integration

9c98133

Convert dashboards lists legacy_app.js to TS Signed-off-by: Peter Fitzgibbons <[email protected]>

Dashboard List integration

fdbfef8

Moved types to ./types.ts modules. Separated dependency of types between OSD and Observability Plugin Signed-off-by: Peter Fitzgibbons <[email protected]>

Dashboard List integration

eb658b1

Integrated Observability Panel List Provider - added Type column to Dashboard List - enabled Search on Dashboad List for all columns/records Signed-off-by: Peter Fitzgibbons <[email protected]>

Dashboard List Extensibility - Completed Observability Integration

35a61cb

- Observability Apps integrated to Dashboard List - "Edit" column availability per-row dependent upon item editUrl presence Signed-off-by: Peter Fitzgibbons <[email protected]>

pjfitzgibbons requested a review from a team as a code owner August 16, 2022 21:50

pjfitzgibbons changed the title ~~Feature/integrate dashboard lists.2~~ [Feature] Dashboards List Extensibility Aug 16, 2022

pjfitzgibbons mentioned this pull request Aug 16, 2022

[Feature] Dashboard List Extensibility - Observability Integration opensearch-project/observability#941

Closed

6 tasks

ashwin-pc requested changes Aug 17, 2022

View reviewed changes

Peter Fitzgibbons added 2 commits August 23, 2022 12:39

Dashboard List Extensibility - Test cleanup

9ee7909

Update dashboard_listing.test.js and snap to match added component fields Signed-off-by: Peter Fitzgibbons <[email protected]>

Dashboard List Extensibility - Dependency management

1bbd9ab

Remove duplicated declarations for DashboardListSource/DashboardListSources types Signed-off-by: Peter Fitzgibbons <[email protected]>

ashwin-pc reviewed Aug 24, 2022

View reviewed changes

pjfitzgibbons requested a review from ashwin-pc August 25, 2022 22:52

Dashboard List Extensibility - Sorting

1eddeed

Correct sorting of Dashboard List by items' Title, Type, or Description. Signed-off-by: Peter Fitzgibbons <[email protected]>

Peter Fitzgibbons added 2 commits September 28, 2022 11:26

Dashboard List Extensibility - Additional Create providers

2446740

Allow plugins to register as "Create provider", adding itself to the "Create" button of Dashboard List. Signed-off-by: Peter Fitzgibbons <[email protected]>

Dashboard List Extensibility - Additional Create providers

e20cf92

Allow plugins to register as "Create provider", adding itself to the "Create" button of Dashboard List. Signed-off-by: Peter Fitzgibbons <[email protected]>

joshuarrrr reviewed Sep 28, 2022

View reviewed changes

joshuarrrr mentioned this pull request Sep 28, 2022

[Meta] Workflow to catch/flag unintended yarn.lock updates in PRs #2429

Open

opensearch-trigger-bot bot and others added 9 commits October 24, 2022 08:34

Fix WMS can't load when unable access maps services (opensearch-proje…

7ab24c5

…ct#1550) (opensearch-project#1707) * Fix WMS can't load when unable access maps services Signed-off-by: Junqiu Lei <[email protected]> (cherry picked from commit b2cfb6e)

Adds v2.0.1 release notes (opensearch-project#1744) (opensearch-proje…

0fb693d

…ct#1750) Signed-off-by: Tommy Markley <[email protected]> (cherry picked from commit 72a3424) Co-authored-by: Tommy Markley <[email protected]>

Bump the version to 2.0.2 (opensearch-project#1758)

3018a1e

Signed-off-by: Zelin Hao <[email protected]>

opensearch-trigger-bot bot and others added 3 commits October 24, 2022 08:34

pjfitzgibbons mentioned this pull request Oct 24, 2022

[Feature] Observability - Dashboard Lists Integration opensearch-project/observability#1180

Closed

6 tasks

Dashboard Lists Integration

370327e

* Configure types for registration of Dashboard List "Providers" and "Creators" * Add Left-Nav "top-level" category for Observability Signed-off-by: Peter Fitzgibbons <[email protected]>

joshuarrrr marked this pull request as draft November 14, 2022 19:48

anirudha mentioned this pull request Jan 3, 2023

[cohesion] As a user, I can view all observability menu items in the left main nav pane opensearch-project/dashboards-observability#113

Open

pjfitzgibbons closed this Nov 23, 2022

pjfitzgibbons mentioned this pull request Dec 1, 2022

Observability - Dashboard Lists Integration opensearch-project/observability#1314

Closed

6 tasks

pjfitzgibbons mentioned this pull request Jan 4, 2023

[Feature] Dashboards-List integrations opensearch-project/dashboards-observability#151

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Dashboards List Extensibility #2149

[Feature] Dashboards List Extensibility #2149

pjfitzgibbons commented Aug 16, 2022 •

edited

Loading

ashwin-pc left a comment

ashwin-pc Aug 17, 2022

ashwin-pc Aug 17, 2022

pjfitzgibbons Aug 23, 2022

ashwin-pc Aug 24, 2022

ashwin-pc Aug 24, 2022

ashwin-pc Aug 24, 2022

ashwin-pc commented Aug 27, 2022

seanneumann commented Sep 26, 2022

pjfitzgibbons commented Sep 26, 2022 via email

joshuarrrr Sep 28, 2022 •

edited

Loading

joshuarrrr commented Oct 4, 2022

joshuarrrr commented Nov 14, 2022

[Feature] Dashboards List Extensibility #2149

[Feature] Dashboards List Extensibility #2149

Conversation

pjfitzgibbons commented Aug 16, 2022 • edited Loading

Description

Issues WIP

Related PR

Check List

ashwin-pc left a comment

Choose a reason for hiding this comment

ashwin-pc Aug 17, 2022

Choose a reason for hiding this comment

ashwin-pc Aug 17, 2022

Choose a reason for hiding this comment

pjfitzgibbons Aug 23, 2022

Choose a reason for hiding this comment

ashwin-pc Aug 24, 2022

Choose a reason for hiding this comment

ashwin-pc Aug 24, 2022

Choose a reason for hiding this comment

ashwin-pc Aug 24, 2022

Choose a reason for hiding this comment

ashwin-pc commented Aug 27, 2022

seanneumann commented Sep 26, 2022

pjfitzgibbons commented Sep 26, 2022 via email

joshuarrrr Sep 28, 2022 • edited Loading

Choose a reason for hiding this comment

joshuarrrr commented Oct 4, 2022

joshuarrrr commented Nov 14, 2022

pjfitzgibbons commented Aug 16, 2022 •

edited

Loading

joshuarrrr Sep 28, 2022 •

edited

Loading