Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dashboard De-Angular] Initial clean up and linter fix #4511

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Bump OUI to `1.1.2` to make `anomalyDetection` icon available ([#4408](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4408))
- Add `color-scheme` to the root styling ([#4477](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4477))
- [Multiple DataSource] Frontend support for adding sample data ([#4412](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4412))
- Dashboard De-Angularization ([#4502](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4502))

### 🐛 Bug Fixes

Expand Down
7 changes: 1 addition & 6 deletions src/plugins/dashboard/public/application/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,12 @@
*/

import './app.scss';
import { AppMountParameters } from 'opensearch-dashboards/public';
import React from 'react';
import { Route, Switch } from 'react-router-dom';
import { DashboardConstants, createDashboardEditUrl } from '../dashboard_constants';
import { DashboardEditor, DashboardListing, DashboardNoMatch } from './components';

export interface DashboardAppProps {
onAppLeave: AppMountParameters['onAppLeave'];
}

export const DashboardApp = ({ onAppLeave }: DashboardAppProps) => {
export const DashboardApp = () => {
return (
<Switch>
<Route path={[DashboardConstants.CREATE_NEW_DASHBOARD_URL, createDashboardEditUrl(':id')]}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const DashboardEditor = () => {
const { id: dashboardIdFromUrl } = useParams<{ id: string }>();
const { services } = useOpenSearchDashboards<DashboardServices>();
const { chrome } = services;
const isChromeVisible = useChromeVisibility(services.chrome);
const isChromeVisible = useChromeVisibility(chrome);
const [eventEmitter] = useState(new EventEmitter());

const { savedDashboard: savedDashboardInstance, dashboard } = useSavedDashboardInstance(
Expand All @@ -41,8 +41,6 @@ export const DashboardEditor = () => {

const { dashboardContainer, indexPatterns } = useDashboardContainer(
services,
isChromeVisible,
eventEmitter,
dashboard,
savedDashboardInstance,
appState
Expand All @@ -58,23 +56,23 @@ export const DashboardEditor = () => {
);

useEffect(() => {
if (appState && dashboard) {
if (currentAppState && dashboard) {
if (savedDashboardInstance?.id) {
chrome.setBreadcrumbs(
setBreadcrumbsForExistingDashboard(
savedDashboardInstance.title,
appState?.getState().viewMode,
currentAppState.viewMode,
dashboard.isDirty
)
);
chrome.docTitle.change(savedDashboardInstance.title);
} else {
chrome.setBreadcrumbs(
setBreadcrumbsForNewDashboard(appState?.getState().viewMode, dashboard.isDirty)
setBreadcrumbsForNewDashboard(currentAppState.viewMode, dashboard.isDirty)
);
}
}
}, [appState?.getState(), savedDashboardInstance, chrome]);
}, [currentAppState, savedDashboardInstance, chrome, dashboard]);

useEffect(() => {
// clean up all registered listeners if any is left
Expand All @@ -83,18 +81,6 @@ export const DashboardEditor = () => {
};
}, [eventEmitter]);

console.log('savedDashboardInstance', savedDashboardInstance);
console.log('dashboard', dashboard);
console.log('appState', appState);
console.log('appStateData', appState?.getState());
console.log('currentAppState', currentAppState);
console.log('isEmbeddableRendered', isEmbeddableRendered);
if (dashboard) {
console.log('isDirty', dashboard.isDirty);
}
console.log('dashboardContainer', dashboardContainer);
console.log('indexPatterns', indexPatterns);

return (
<div>
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import React, { memo, useState, useEffect } from 'react';
import { Filter, IndexPattern } from 'src/plugins/data/public';
import { IndexPattern } from 'src/plugins/data/public';
import { useCallback } from 'react';
import { useLocation } from 'react-router-dom';
import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
Expand Down Expand Up @@ -102,7 +102,7 @@ const TopNav = ({
stateContainer,
isEmbeddableRendered,
dashboard,
dashboardIdFromUrl
dashboardIdFromUrl,
]);

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
&:hover,
&:focus {
// TODO: this is a sass error, $embEditingModeHoverColor is undefined, comment it out for now
//background-color: $embEditingModeHoverColor;
// background-color: $embEditingModeHoverColor;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

import 'react-grid-layout/css/styles.css';
import 'react-resizable/css/styles.css';
import './_dashboard_grid.scss'
import './_dashboard_grid.scss';

// @ts-ignore
import sizeMe from 'react-sizeme';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
*/

import { i18n } from '@osd/i18n';
import { AppMountParameters } from 'opensearch-dashboards/public';
import { ViewMode } from '../../embeddable_plugin';
import { TopNavIds } from './top_nav_ids';
import { NavAction } from '../../types';
Expand All @@ -43,8 +42,7 @@ import { NavAction } from '../../types';
export function getTopNavConfig(
dashboardMode: ViewMode,
actions: { [key: string]: NavAction },
hideWriteControls: boolean,
onAppLeave?: AppMountParameters['onAppLeave']
hideWriteControls: boolean
) {
switch (dashboardMode) {
case ViewMode.VIEW:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ export const getDashboardInstance = async (

// Get the existing dashboard/default new dashboard from saved object loader
const savedDashboard: SavedObjectDashboard = await savedDashboards.get(opts);

// Serialized the saved object dashboard
const serializedDashboard = convertToSerializedDashboard(savedDashboard);

// Create a Dashboard class using the serialized dashboard
// const dashboard = (await dashboards.createDashboard(serializedDashboard)) as Dashboard;
const dashboard = new Dashboard(serializedDashboard);
await dashboard.setState(serializedDashboard);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import React, { useState } from 'react';
import { cloneDeep, isEqual, uniqBy } from 'lodash';
import { EMPTY, Observable, Subscription, merge, of, pipe } from 'rxjs';
import { EMPTY, Observable, Subscription, merge, pipe } from 'rxjs';
import {
catchError,
distinctUntilChanged,
Expand All @@ -16,7 +16,6 @@ import {
switchMap,
} from 'rxjs/operators';
import deepEqual from 'fast-deep-equal';
import { EventEmitter } from 'stream';
import { useEffect } from 'react';
import { i18n } from '@osd/i18n';
import _ from 'lodash';
Expand Down Expand Up @@ -55,8 +54,6 @@ import { Dashboard } from '../../../dashboard';

export const useDashboardContainer = (
services: DashboardServices,
isChromeVisible: boolean,
eventEmitter: EventEmitter,
dashboard?: Dashboard,
savedDashboardInstance?: SavedObjectDashboard,
appState?: DashboardAppStateContainer
Expand Down Expand Up @@ -134,7 +131,6 @@ const createDashboardEmbeddable = (
} = dashboardServices;
const { query: queryService } = data;
const filterManager = queryService.filterManager;
const timefilter = queryService.timefilter.timefilter;
const queryStringManager = queryService.queryString;
const { visualizeCapabilities, mapsCapabilities } = embeddableCapabilities;
const dashboardFactory = embeddable.getEmbeddableFactory<
Expand Down Expand Up @@ -237,7 +233,7 @@ const createDashboardEmbeddable = (
getShouldShowViewHelp(appStateData) ||
shouldShowUnauthorizedEmptyState(appStateData),
useMargins: appStateData.options.useMargins,
lastReloadRequestTime, // TODO
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added related logics to lastReloadRequestTime

lastReloadRequestTime,
title: appStateData.title,
description: appStateData.description,
expandedPanelId: appStateData.expandedPanelId,
Expand Down Expand Up @@ -339,8 +335,6 @@ const createDashboardEmbeddable = (
return Object.values(differences).length === 0 ? undefined : cloneDeep(differences);
};

// TODO: handle dashboard container input and output subsciptions
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved

// issue:
outputSubscription = merge(
// output of dashboard container itself
dashboardContainer.getOutput$(),
Expand Down Expand Up @@ -381,13 +375,6 @@ const createDashboardEmbeddable = (
) {
// Add filters modifies the object passed to it, hence the clone deep.
filterManager.addFilters(cloneDeep(container.getInput().filters));

// TODO: investigate if this is needed
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved, this is not needed here

/* dashboardStateManager.applyFilters(
$scope.model.query,
container.getInput().filters
);*/

appState.transitions.set('query', queryStringManager.getQuery());
}
// triggered when dashboard embeddable container has changes, and update the appState
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ export const useSavedDashboardInstance = (

const getSavedDashboardInstance = async () => {
try {
let savedDashboardInstance: any;
let savedDashboardObject: any;
if (history.location.pathname === '/create') {
try {
savedDashboardInstance = await getDashboardInstance(services);
savedDashboardObject = await getDashboardInstance(services);
} catch {
redirectWhenMissing({
history,
Expand All @@ -61,8 +61,8 @@ export const useSavedDashboardInstance = (
}
} else if (dashboardIdFromUrl) {
try {
savedDashboardInstance = await getDashboardInstance(services, dashboardIdFromUrl);
const { savedDashboard } = savedDashboardInstance;
savedDashboardObject = await getDashboardInstance(services, dashboardIdFromUrl);
const { savedDashboard } = savedDashboardObject;

// Update time filter to match the saved dashboard if time restore has been set to true when saving the dashboard
// We should only set the time filter according to time restore once when we are loading the dashboard
Expand Down Expand Up @@ -111,7 +111,7 @@ export const useSavedDashboardInstance = (
}
}

setSavedDashboardInstance(savedDashboardInstance);
setSavedDashboardInstance(savedDashboardObject);
} catch (error) {
toastNotifications.addWarning({
title: i18n.translate('dashboard.createDashboard.failedToLoadErrorMessage', {
Expand Down
62 changes: 3 additions & 59 deletions src/plugins/dashboard/public/dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,9 @@

import { cloneDeep } from 'lodash';
import { Filter, ISearchSource, Query, RefreshInterval } from '../../data/public';
import { DashboardPanelState } from './application';
import { EmbeddableInput } from './embeddable_plugin';
import { SavedDashboardPanel } from './types';

// export interface SerializedPanels {
// [panelId: string]: DashboardPanelState<EmbeddableInput & { [k: string]: unknown }>;
// }

// TODO: This class can be revisited and clean up more
export interface SerializedDashboard {
id?: string;
timeRestore: boolean;
Expand All @@ -35,8 +30,8 @@ export interface SerializedDashboard {
useMargins: boolean;
};
uiState?: string;
lastSavedTitle: string; // TODO: DO WE STILL NEED THIS?
refreshInterval?: RefreshInterval; // TODO: SHOULD THIS NOT BE OPTIONAL?
lastSavedTitle: string;
refreshInterval?: RefreshInterval;
searchSource?: ISearchSource;
query: Query;
filters: Filter[];
Expand All @@ -63,7 +58,6 @@ export class Dashboard<TDashboardParams = DashboardParams> {
public query: Query;
public filters: Filter[];
public title?: string;
// TODO: dashboardNew - pass version to dashboard class
public version = '3.0.0';
public isDirty = false;

Expand All @@ -90,8 +84,6 @@ export class Dashboard<TDashboardParams = DashboardParams> {
this.description = state.description;
}
if (state.panels) {
// this panels is only JSON.parse() panels, we should convert them into the same type as app state panels
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve

// app state store only JSON.parse() panels too
this.panels = cloneDeep(state.panels);
}
if (state.options) {
Expand Down Expand Up @@ -124,52 +116,4 @@ export class Dashboard<TDashboardParams = DashboardParams> {
private getRefreshInterval(refreshInterval: RefreshInterval) {
return cloneDeep(refreshInterval ?? {});
}

// private getQuery(query: Query): Query {
// return cloneDeep(query ?? ({} as Query));
// }

// private getFilters(filters: Filter[]) {
// return cloneDeep(filters ?? ({} as Filter[]));
// }

// private getPanels(panels?: SerializedPanels) {
// return cloneDeep(panels ?? ({} as SerializedPanels));
// }

/* clone() {
const serializedDashboard = this.serialize();
const dashboard = new Dashboard(serializedDashboard);
dashboard.setState(serializedDashboard);
return dashboard;
}*/

/* serialize(): SerializedDashboard {
return {
id: this.id,
timeRestore: this.timeRestore!,
timeTo: this.timeTo,
timeFrom: this.timeFrom,
description: this.description,
panels: this.serializePanels(),
options: cloneDeep(this.options) as any,
uiState: this.uiState,
lastSavedTitle: this.lastSavedTitle,
refreshInterval: this.refreshInterval,
searchSource: this.searchSource,
query: this.query,
filters: this.filters,
title: this.title!,
};
}*/

/* serializePanels(): SerializedPanels {
const embeddablesMap: {
[key: string]: DashboardPanelState;
} = {};
this.panels.forEach((panel: SavedDashboardPanel) => {
embeddablesMap[panel.panelIndex] = convertSavedDashboardPanelToPanelState(panel);
});
return embeddablesMap;
}*/
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ test('dashboard migration 7.3.0 migrates filters to query on search source', ()
"panelsJSON": "[{\\"id\\":\\"1\\",\\"type\\":\\"visualization\\",\\"foo\\":true},{\\"id\\":\\"2\\",\\"type\\":\\"visualization\\",\\"bar\\":true}]",
"timeRestore": false,
"title": "hi",
"uiStateJSON": "{}",
"useMargins": true,
"version": 1,
},
Expand Down