Skip to content

Commit

Permalink
Handle undefined dashboard settings
Browse files Browse the repository at this point in the history
  • Loading branch information
bmingles committed Jan 11, 2024
1 parent a3bea73 commit f251949
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 73 deletions.
21 changes: 18 additions & 3 deletions packages/code-studio/src/main/AppMainContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ interface AppMainContainerProps {
match: {
params: { notebookPath: string };
};
connection: IdeConnection;
connection?: IdeConnection;
session?: IdeSession;
sessionConfig?: SessionConfig;
setActiveTool: (tool: string) => void;
Expand Down Expand Up @@ -255,6 +255,10 @@ export class AppMainContainer extends Component<

initWidgets(): void {
const { connection } = this.props;
if (connection == null) {
return;
}

if (connection.subscribeToFieldUpdates == null) {
log.warn(
'subscribeToFieldUpdates not supported, not initializing widgets'
Expand Down Expand Up @@ -614,6 +618,10 @@ export class AppMainContainer extends Component<

startListeningForDisconnect(): void {
const { connection } = this.props;
if (connection == null) {
return;
}

connection.addEventListener(
dh.IdeConnection.EVENT_DISCONNECT,
this.handleDisconnect
Expand All @@ -630,6 +638,10 @@ export class AppMainContainer extends Component<

stopListeningForDisconnect(): void {
const { connection } = this.props;
if (connection == null) {
return;
}

connection.removeEventListener(
dh.IdeConnection.EVENT_DISCONNECT,
this.handleDisconnect
Expand All @@ -650,6 +662,7 @@ export class AppMainContainer extends Component<
): DehydratedDashboardPanelProps & { fetch?: () => Promise<unknown> } {
const { connection } = this.props;
const { metadata } = props;

if (
metadata?.type != null &&
(metadata?.id != null || metadata?.name != null)
Expand All @@ -666,12 +679,14 @@ export class AppMainContainer extends Component<
name: metadata.name,
title: metadata.name,
};

return {
fetch: () => connection.getObject(widget),
fetch: async () => connection?.getObject(widget),
...props,
localDashboardId: id,
};
}

return DashboardUtils.hydrate(props, id);
}

Expand All @@ -684,7 +699,7 @@ export class AppMainContainer extends Component<
const { connection } = this.props;
this.emitLayoutEvent(PanelEvent.OPEN, {
dragEvent,
fetch: () => connection.getObject(widget),
fetch: async () => connection?.getObject(widget),
widget,
});
}
Expand Down
41 changes: 18 additions & 23 deletions packages/dashboard-core-plugins/src/panels/ConsolePanel.test.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
import React from 'react';
import { render } from '@testing-library/react';
import { CommandHistoryStorage } from '@deephaven/console';
import type { Container } from '@deephaven/golden-layout';
import type { Container, EventEmitter } from '@deephaven/golden-layout';
import type { IdeConnection, IdeSession } from '@deephaven/jsapi-types';
import { dh } from '@deephaven/jsapi-shim';
import { SessionConfig, SessionWrapper } from '@deephaven/jsapi-utils';
import { TestUtils } from '@deephaven/utils';
import { ConsolePanel } from './ConsolePanel';

const mockConsole = jest.fn(() => null);
type IdeSessionConstructor = new (language: string) => IdeSession;

const mockConsole = jest.fn((_props: unknown) => null);
jest.mock('@deephaven/console', () => ({
...(jest.requireActual('@deephaven/console') as Record<string, unknown>),
Console: props => mockConsole(props),
default: props => mockConsole(props),
}));

function makeSession(language = 'TEST_LANG'): IdeSession {
return new dh.IdeSession(language) as unknown as IdeSession;
return new (dh.IdeSession as unknown as IdeSessionConstructor)(language);
}

function makeConnection({
Expand Down Expand Up @@ -42,31 +46,15 @@ function makeSessionWrapper({
return { session, connection, config, dh };
}

function makeEventHub() {
return {
emit: jest.fn(),
on: jest.fn(),
off: jest.fn(),
trigger: jest.fn(),
unbind: jest.fn(),
};
}

function makeGlContainer(): Container {
return {
emit: jest.fn(),
on: jest.fn(),
off: jest.fn(),
} as unknown as Container;
}

function makeCommandHistoryStorage(): CommandHistoryStorage {
return {} as CommandHistoryStorage;
}

function renderConsolePanel({
eventHub = makeEventHub(),
container = makeGlContainer(),
eventHub = TestUtils.createMockProxy<EventEmitter>(),
container = TestUtils.createMockProxy<Container>({
tab: undefined,
}),
commandHistoryStorage = makeCommandHistoryStorage(),
timeZone = 'MockTimeZone',
sessionWrapper = makeSessionWrapper(),
Expand All @@ -78,11 +66,18 @@ function renderConsolePanel({
commandHistoryStorage={commandHistoryStorage}
timeZone={timeZone}
sessionWrapper={sessionWrapper}
localDashboardId="mock-localDashboardId"
plugins={new Map()}
/>
);
}

beforeEach(() => {
// Mocking the Console component causes it to be treated as a functional
// component which causes React to log an error about passing refs. Disable
// logging to supress this
TestUtils.disableConsoleOutput('error');

mockConsole.mockClear();
});

Expand Down
16 changes: 14 additions & 2 deletions packages/dashboard-core-plugins/src/panels/ConsolePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ interface ConsolePanelProps extends DashboardPanelProps {

panelState?: PanelState;

sessionWrapper: SessionWrapper;
sessionWrapper?: SessionWrapper;

timeZone: string;
unzip?: (file: File) => Promise<JSZipObject[]>;
Expand Down Expand Up @@ -159,6 +159,10 @@ export class ConsolePanel extends PureComponent<

subscribeToFieldUpdates(): void {
const { sessionWrapper } = this.props;
if (sessionWrapper == null) {
return;
}

const { session } = sessionWrapper;

this.objectSubscriptionCleanup = session.subscribeToFieldUpdates(
Expand Down Expand Up @@ -244,6 +248,9 @@ export class ConsolePanel extends PureComponent<

handleOpenObject(object: VariableDefinition, forceOpen = true): void {
const { sessionWrapper } = this.props;
if (sessionWrapper == null) {
return;
}
const { session } = sessionWrapper;
const { root } = this.context;
const oldPanelId =
Expand Down Expand Up @@ -359,7 +366,7 @@ export class ConsolePanel extends PureComponent<
return <ObjectIcon type={type} />;
}

render(): ReactElement {
render(): ReactElement | null {
const {
commandHistoryStorage,
glContainer,
Expand All @@ -368,6 +375,11 @@ export class ConsolePanel extends PureComponent<
timeZone,
unzip,
} = this.props;

if (sessionWrapper == null) {
return null;
}

const { consoleSettings, error, objectMap } = this.state;
const { config, session, connection, details = {}, dh } = sessionWrapper;
const { workerName, processInfoId } = details;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const log = Log.module('FileExplorerPanel');

type StateProps = {
fileStorage: FileStorage;
language: string;
language?: string;
session?: IdeSession;
};

Expand Down
4 changes: 2 additions & 2 deletions packages/dashboard-core-plugins/src/panels/LogPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import { getDashboardSessionWrapper } from '../redux';
const log = Log.module('LogPanel');

interface LogPanelProps extends DashboardPanelProps {
session: IdeSession;
session?: IdeSession;
}

interface LogPanelState {
session: IdeSession;
session?: IdeSession;
}

class LogPanel extends PureComponent<LogPanelProps, LogPanelState> {
Expand Down
30 changes: 16 additions & 14 deletions packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ interface Metadata extends PanelMetadata {
id: string;
}
interface NotebookSetting {
isMinimapEnabled: boolean;
isMinimapEnabled?: boolean;
}

interface FileMetadata {
Expand All @@ -78,16 +78,21 @@ interface PanelState {
fileMetadata: FileMetadata | null;
}

interface NotebookPanelProps extends DashboardPanelProps {
interface NotebookPanelMappedProps {
defaultNotebookSettings: NotebookSetting;
fileStorage: FileStorage;
session?: IdeSession;
sessionLanguage?: string;
}

interface NotebookPanelProps
extends DashboardPanelProps,
NotebookPanelMappedProps {
isDashboardActive: boolean;
isPreview: boolean;
metadata: Metadata;
session: IdeSession;
sessionLanguage: string;
panelState: PanelState;
notebooksUrl: string;
defaultNotebookSettings: NotebookSetting;
updateSettings: (settings: Partial<WorkspaceSettings>) => void;
}

Expand Down Expand Up @@ -790,7 +795,7 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {
const { defaultNotebookSettings, updateSettings } = this.props;
const newSettings = {
defaultNotebookSettings: {
isMinimapEnabled: !defaultNotebookSettings.isMinimapEnabled,
isMinimapEnabled: !(defaultNotebookSettings.isMinimapEnabled ?? false),
},
};
updateSettings(newSettings);
Expand Down Expand Up @@ -1176,10 +1181,10 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {
const { defaultNotebookSettings } = this.props;
const { settings: initialSettings } = this.state;
return this.getOverflowActions(
defaultNotebookSettings.isMinimapEnabled,
defaultNotebookSettings.isMinimapEnabled ?? false,
this.getSettings(
initialSettings,
defaultNotebookSettings.isMinimapEnabled
defaultNotebookSettings.isMinimapEnabled ?? false
).wordWrap === 'on'
);
}
Expand Down Expand Up @@ -1216,7 +1221,7 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {
const isExistingItem = fileMetadata?.id != null;
const settings = this.getSettings(
initialSettings,
defaultNotebookSettings.isMinimapEnabled
defaultNotebookSettings.isMinimapEnabled ?? false
);
const isSessionConnected = session != null;
const isLanguageMatching = sessionLanguage === settings.language;
Expand Down Expand Up @@ -1428,10 +1433,7 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {
const mapStateToProps = (
state: RootState,
ownProps: { localDashboardId: string }
): Pick<
NotebookPanelProps,
'defaultNotebookSettings' | 'fileStorage' | 'session' | 'sessionLanguage'
> => {
): NotebookPanelMappedProps => {
const fileStorage = getFileStorage(state);
const defaultNotebookSettings = getDefaultNotebookSettings(state);
const sessionWrapper = getDashboardSessionWrapper(
Expand All @@ -1443,7 +1445,7 @@ const mapStateToProps = (
const { type: sessionLanguage } = sessionConfig ?? {};
return {
fileStorage,
defaultNotebookSettings: defaultNotebookSettings as NotebookSetting,
defaultNotebookSettings,
session,
sessionLanguage,
};
Expand Down
18 changes: 10 additions & 8 deletions packages/dashboard-core-plugins/src/redux/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { Link } from '../linker/LinkerUtils';
import { FilterSet } from '../panels';
import { ColumnSelectionValidator } from '../linker/ColumnSelectionValidator';

const EMPTY_OBJECT = Object.freeze({});

const EMPTY_MAP = new Map();

const EMPTY_ARRAY = Object.freeze([]);
Expand Down Expand Up @@ -107,10 +109,8 @@ export const getDashboardConsoleSettings = (
store: RootState,
dashboardId: string
): Record<string, unknown> =>
getDashboardData(store, dashboardId).consoleSettings as Record<
string,
unknown
>;
(getDashboardData(store, dashboardId).consoleSettings ??
EMPTY_OBJECT) as Record<string, unknown>;

/**
*
Expand All @@ -121,8 +121,8 @@ export const getDashboardConsoleSettings = (
export const getDashboardConnection = (
store: RootState,
dashboardId: string
): IdeConnection =>
getDashboardData(store, dashboardId).connection as IdeConnection;
): IdeConnection | undefined =>
getDashboardData(store, dashboardId).connection as IdeConnection | undefined;

/**
*
Expand All @@ -133,5 +133,7 @@ export const getDashboardConnection = (
export const getDashboardSessionWrapper = (
store: RootState,
dashboardId: string
): SessionWrapper =>
getDashboardData(store, dashboardId).sessionWrapper as SessionWrapper;
): SessionWrapper | undefined =>
getDashboardData(store, dashboardId).sessionWrapper as
| SessionWrapper
| undefined;
Loading

0 comments on commit f251949

Please sign in to comment.