Skip to content

Commit

Permalink
[Dashboard De-Angular] Initial clean up and linter fix (#4511)
Browse files Browse the repository at this point in the history
* Clean up linter issues

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

* Add changelog and other fixes

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

---------

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
  • Loading branch information
abbyhu2000 committed Jul 7, 2023
1 parent f8d5e82 commit 2ea2885
Show file tree
Hide file tree
Showing 12 changed files with 25 additions and 112 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- 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))
- Enable plugins to augment visualizations with additional data and context ([#4361](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4361))
- 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
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
// 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
/* 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
// 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

0 comments on commit 2ea2885

Please sign in to comment.