From c3030a8b5ee6b7aae46ed68d1668bf49dd15b9f1 Mon Sep 17 00:00:00 2001 From: Kawika Avilla Date: Tue, 11 Jul 2023 02:40:05 +0000 Subject: [PATCH 1/5] Set dashboard container after defining functions renderEmpty was not being set prior to the current container was being dispatched. Moved the set state to after the functions are defined. Signed-off-by: Kawika Avilla --- .../public/application/utils/use/use_dashboard_app_state.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx b/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx index d5af39cca4fd..e7d96dd83dd7 100644 --- a/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx +++ b/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx @@ -117,7 +117,6 @@ export const useDashboardAppAndGlobalState = ({ savedDashboard: savedDashboardInstance, appState: stateContainer, }); - setCurrentContainer(dashboardContainer); if (!dashboardContainer) { return; @@ -196,6 +195,8 @@ export const useDashboardAppAndGlobalState = ({ stopSyncingDashboardContainerOutputs(); subscriptions.unsubscribe(); }; + + setCurrentContainer(dashboardContainer); }; getDashboardContainer(); From e708d95f0b9acf1253139f55155890877a37c718 Mon Sep 17 00:00:00 2001 From: Kawika Avilla Date: Tue, 11 Jul 2023 03:28:14 +0000 Subject: [PATCH 2/5] place on top Signed-off-by: Kawika Avilla --- .../application/utils/use/use_dashboard_app_state.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx b/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx index e7d96dd83dd7..762a57a65668 100644 --- a/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx +++ b/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx @@ -117,14 +117,16 @@ export const useDashboardAppAndGlobalState = ({ savedDashboard: savedDashboardInstance, appState: stateContainer, }); + // Ensure empty state is attached to current container being dispatched + dashboardContainer!.renderEmpty = () => + renderEmpty(dashboardContainer!, stateContainer, services); + + setCurrentContainer(dashboardContainer); if (!dashboardContainer) { return; } - dashboardContainer.renderEmpty = () => - renderEmpty(dashboardContainer, stateContainer, services); - dashboardContainer.updateAppStateUrl = ({ replace, pathname, @@ -195,8 +197,6 @@ export const useDashboardAppAndGlobalState = ({ stopSyncingDashboardContainerOutputs(); subscriptions.unsubscribe(); }; - - setCurrentContainer(dashboardContainer); }; getDashboardContainer(); From 69fd6ba84c3d4d51a9ed3be97cde430514df0692 Mon Sep 17 00:00:00 2001 From: Kawika Avilla Date: Tue, 11 Jul 2023 09:46:26 +0000 Subject: [PATCH 3/5] set container after undefined check Signed-off-by: Kawika Avilla --- .../utils/use/use_dashboard_app_state.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx b/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx index 762a57a65668..e8655a889e4b 100644 --- a/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx +++ b/src/plugins/dashboard/public/application/utils/use/use_dashboard_app_state.tsx @@ -117,16 +117,15 @@ export const useDashboardAppAndGlobalState = ({ savedDashboard: savedDashboardInstance, appState: stateContainer, }); - // Ensure empty state is attached to current container being dispatched - dashboardContainer!.renderEmpty = () => - renderEmpty(dashboardContainer!, stateContainer, services); - - setCurrentContainer(dashboardContainer); - if (!dashboardContainer) { return; } + // Ensure empty state is attached to current container being dispatched + dashboardContainer.renderEmpty = () => + renderEmpty(dashboardContainer, stateContainer, services); + + // Ensure update app state in url is attached to current container being dispatched dashboardContainer.updateAppStateUrl = ({ replace, pathname, @@ -148,6 +147,8 @@ export const useDashboardAppAndGlobalState = ({ } }; + setCurrentContainer(dashboardContainer); + const stopSyncingDashboardContainerOutputs = handleDashboardContainerOutputs( services, dashboardContainer, From 9da7cb7c85604629dd9475277ed3fd892778127b Mon Sep 17 00:00:00 2001 From: Kawika Avilla Date: Tue, 11 Jul 2023 16:12:42 +0000 Subject: [PATCH 4/5] fix up license headers for new files Signed-off-by: Kawika Avilla --- src/plugins/dashboard/public/application/app.tsx | 8 +------- src/plugins/dashboard/public/application/index.tsx | 8 +------- .../dashboard/public/application/utils/breadcrumbs.ts | 8 +------- .../public/application/utils/get_dashboard_instance.tsx | 8 +------- .../public/application/utils/use/use_chrome_visibility.ts | 8 +------- src/plugins/dashboard/public/dashboard.ts | 8 +------- 6 files changed, 6 insertions(+), 42 deletions(-) diff --git a/src/plugins/dashboard/public/application/app.tsx b/src/plugins/dashboard/public/application/app.tsx index 1d9c6268c1e3..f60912054d72 100644 --- a/src/plugins/dashboard/public/application/app.tsx +++ b/src/plugins/dashboard/public/application/app.tsx @@ -1,12 +1,6 @@ /* + * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Any modifications Copyright OpenSearch Contributors. See - * GitHub history for details. */ import './app.scss'; diff --git a/src/plugins/dashboard/public/application/index.tsx b/src/plugins/dashboard/public/application/index.tsx index 6ac4a012b709..366366eb83d8 100644 --- a/src/plugins/dashboard/public/application/index.tsx +++ b/src/plugins/dashboard/public/application/index.tsx @@ -1,12 +1,6 @@ /* + * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Any modifications Copyright OpenSearch Contributors. See - * GitHub history for details. */ import React from 'react'; diff --git a/src/plugins/dashboard/public/application/utils/breadcrumbs.ts b/src/plugins/dashboard/public/application/utils/breadcrumbs.ts index dbedf389b07c..55934ead7f3d 100644 --- a/src/plugins/dashboard/public/application/utils/breadcrumbs.ts +++ b/src/plugins/dashboard/public/application/utils/breadcrumbs.ts @@ -1,12 +1,6 @@ /* + * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Any modifications Copyright OpenSearch Contributors. See - * GitHub history for details. */ import { i18n } from '@osd/i18n'; diff --git a/src/plugins/dashboard/public/application/utils/get_dashboard_instance.tsx b/src/plugins/dashboard/public/application/utils/get_dashboard_instance.tsx index 31c22b9e8048..4340b08fe7a6 100644 --- a/src/plugins/dashboard/public/application/utils/get_dashboard_instance.tsx +++ b/src/plugins/dashboard/public/application/utils/get_dashboard_instance.tsx @@ -1,12 +1,6 @@ /* + * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Any modifications Copyright OpenSearch Contributors. See - * GitHub history for details. */ import { Dashboard, DashboardParams } from '../../dashboard'; diff --git a/src/plugins/dashboard/public/application/utils/use/use_chrome_visibility.ts b/src/plugins/dashboard/public/application/utils/use/use_chrome_visibility.ts index 863c08be917b..b638d114666c 100644 --- a/src/plugins/dashboard/public/application/utils/use/use_chrome_visibility.ts +++ b/src/plugins/dashboard/public/application/utils/use/use_chrome_visibility.ts @@ -1,12 +1,6 @@ /* + * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Any modifications Copyright OpenSearch Contributors. See - * GitHub history for details. */ import { useState, useEffect } from 'react'; diff --git a/src/plugins/dashboard/public/dashboard.ts b/src/plugins/dashboard/public/dashboard.ts index 23b4bc72e19f..751837eb1ef5 100644 --- a/src/plugins/dashboard/public/dashboard.ts +++ b/src/plugins/dashboard/public/dashboard.ts @@ -1,12 +1,6 @@ /* + * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Any modifications Copyright OpenSearch Contributors. See - * GitHub history for details. */ /** From 1a01b223611acaa34dfb6cd1f4828d60aa5f0ff4 Mon Sep 17 00:00:00 2001 From: Kawika Avilla Date: Tue, 11 Jul 2023 17:02:21 +0000 Subject: [PATCH 5/5] add TODOs from PR Signed-off-by: Kawika Avilla --- .../public/application/components/dashboard_listing.tsx | 6 ++++++ .../application/utils/create_dashboard_app_state.tsx | 7 ++++++- .../application/utils/use/use_saved_dashboard_instance.ts | 2 ++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/plugins/dashboard/public/application/components/dashboard_listing.tsx b/src/plugins/dashboard/public/application/components/dashboard_listing.tsx index a407ee7ab1df..df3ff9099394 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_listing.tsx +++ b/src/plugins/dashboard/public/application/components/dashboard_listing.tsx @@ -155,6 +155,12 @@ export const DashboardListing = () => { [history, application] ); + // TODO: Currently, dashboard listing is using a href to view items. + // Dashboard listing should utilize a callback to take us away from using a href in favor + // of using onClick. + // + // https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3365 + // // const viewItem = useCallback( // ({ appId, viewUrl }: any) => { // if (appId === 'dashboard') { diff --git a/src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx b/src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx index fb3614891729..09b541a5e29b 100644 --- a/src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx +++ b/src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx @@ -105,13 +105,18 @@ export const createDashboardGlobalAndAppState = ({ ...state, }); } else { + // TODO: This logic was ported over this but can be handled more gracefully and intentionally + // Sync from state url should be refactored within this application. The app is syncing from + // the query state and the dashboard in different locations which can be handled better. + // https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3365 + // // Do nothing in case when state from url is empty, // this fixes: https://github.com/elastic/kibana/issues/57789 // There are not much cases when state in url could become empty: // 1. User manually removed `_a` from the url // 2. Browser is navigating away from the page and most likely there is no `_a` in the url. // In this case we don't want to do any state updates - // and just allow $scope.$on('destroy') fire later and clean up everything + // and just unmount later and clean up everything } }, }, diff --git a/src/plugins/dashboard/public/application/utils/use/use_saved_dashboard_instance.ts b/src/plugins/dashboard/public/application/utils/use/use_saved_dashboard_instance.ts index 1f09dc72c476..47c5b44fe7e5 100644 --- a/src/plugins/dashboard/public/application/utils/use/use_saved_dashboard_instance.ts +++ b/src/plugins/dashboard/public/application/utils/use/use_saved_dashboard_instance.ts @@ -93,6 +93,8 @@ export const useSavedDashboardInstance = ({ history.replace(DashboardConstants.LANDING_PAGE_PATH); }; + // TODO: handle try/catch as expected workflows instead of catching as an error + // https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3365 const getSavedDashboardInstance = async () => { try { let dashboardInstance: {