Skip to content

Commit

Permalink
PR comment fixes
Browse files Browse the repository at this point in the history
Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>
  • Loading branch information
ashwin-pc committed Aug 24, 2023
1 parent d0d2ec9 commit a92e483
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 53 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 19 additions & 21 deletions src/plugins/data_explorer/public/components/app_container.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

import React from 'react';
import { AppContainer } from './app_container';
import { render } from '@testing-library/react';
import { View } from '../services/view_service/view';
import { ViewMountParameters } from '../services/view_service';
import { AppMountParameters } from '../../../../core/public';
import { render } from 'test_utils/testing_lib_helpers';

describe('DataExplorerApp', () => {
const createView = () => {
Expand All @@ -16,32 +16,30 @@ describe('DataExplorerApp', () => {
title: 'Test View',
defaultPath: '/test-path',
appExtentions: {} as any,
mount: async ({ canvasElement, panelElement }: ViewMountParameters) => {
const canvasContent = document.createElement('div');
const panelContent = document.createElement('div');
canvasContent.innerHTML = 'canvas-content';
panelContent.innerHTML = 'panel-content';
canvasElement.appendChild(canvasContent);
panelElement.appendChild(panelContent);
return () => {
canvasContent.remove();
panelContent.remove();
};
},
Canvas: (() => <div>canvas</div>) as any,
Panel: (() => <div>panel</div>) as any,
Context: (() => <div>Context</div>) as any,
});
};

const params: AppMountParameters = {
element: document.createElement('div'),
history: {} as any,
onAppLeave: jest.fn(),
setHeaderActionMenu: jest.fn(),
appBasePath: '',
};

it('should render NoView when a non existent view is selected', () => {
const { container } = render(<AppContainer />);
const { container } = render(<AppContainer params={params} />);

expect(container).toContainHTML('View not found');
});

// TODO: Complete once state management is in place
// it('should render the canvas and panel when selected', () => {
// const view = createView();
// const { container } = render(<AppContainer view={view} />);
it('should render the canvas and panel when selected', () => {
const view = createView();
const { container } = render(<AppContainer view={view} params={params} />);

// expect(container).toMatchSnapshot();
// });
expect(container).toMatchSnapshot();
});
});
12 changes: 10 additions & 2 deletions src/plugins/data_explorer/public/components/no_view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import React from 'react';
import { EuiPageTemplate, EuiEmptyPrompt } from '@elastic/eui';
import { FormattedMessage } from '@osd/i18n/react';

export const NoView = () => {
return (
Expand All @@ -20,10 +21,17 @@ export const NoView = () => {
<EuiEmptyPrompt
iconType="alert"
iconColor="danger"
title={<h2>View not found</h2>}
title={
<h2>
<FormattedMessage id="dataExplorer.noView.title" defaultMessage="View not found" />
</h2>
}
body={
<p>
The view you are trying to access does not exist. Please check the URL and try again.
<FormattedMessage
id="dataExplorer.noView.body"
defaultMessage="The view you are trying to access does not exist. Please check the URL and try again."
/>
</p>
}
/>
Expand Down
9 changes: 0 additions & 9 deletions src/plugins/data_explorer/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ export class DataExplorerPlugin
{ data }: DataExplorerPluginSetupDependencies
): DataExplorerPluginSetup {
const viewService = this.viewService;
// TODO: Remove this before merge to main
// eslint-disable-next-line no-console
console.log('data_explorer: Setup');

const { appMounted, appUnMounted, stop: stopUrlTracker } = createOsdUrlTracker({
baseUrl: core.http.basePath.prepend(`/app/${PLUGIN_ID}`),
Expand Down Expand Up @@ -87,9 +84,6 @@ export class DataExplorerPlugin
title: PLUGIN_NAME,
navLinkStatus: AppNavLinkStatus.hidden,
mount: async (params: AppMountParameters) => {
// TODO: Remove this before merge to main
// eslint-disable-next-line no-console
console.log('data_explorer: Mounted');
// Load application bundle
const { renderApp } = await import('./application');

Expand Down Expand Up @@ -135,9 +129,6 @@ export class DataExplorerPlugin
}

public start(core: CoreStart): DataExplorerPluginStart {
// TODO: Remove this before merge to main
// eslint-disable-next-line no-console
console.log('data_explorer: Started');
return {};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ export class ViewService implements CoreService<ViewServiceSetup, ViewServiceSta
private views: Record<string, View> = {};

private registerView(view: View) {
if (this.views[view.id]) {
throw new Error(`A view with this the id ${view.id} already exists!`);
if (view.id in this.views) {
throw new Error(`A view with the id ${view.id} already exists!`);
}
this.views[view.id] = view;
}
Expand Down Expand Up @@ -73,7 +73,7 @@ export class ViewService implements CoreService<ViewServiceSetup, ViewServiceSta
* returns all registered Views
*/
all: (): View[] => {
return [...Object.values(this.views)];
return Object.values(this.views);
},
};
}
Expand Down
23 changes: 14 additions & 9 deletions src/plugins/data_explorer/public/utils/state_management/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,21 @@ export const getPreloadedState = async (

const { defaults } = view.ui;

// defaults can be a function or an object
const preloadedState = typeof defaults === 'function' ? await defaults() : defaults;
rootState[view.id] = preloadedState.state;
try {
// defaults can be a function or an object
const preloadedState = typeof defaults === 'function' ? await defaults() : defaults;
rootState[view.id] = preloadedState.state;

// if the view wants to override the root state, we do that here
if (preloadedState.root) {
rootState = {
...rootState,
...preloadedState.root,
};
// if the view wants to override the root state, we do that here
if (preloadedState.root) {
rootState = {
...rootState,
...preloadedState.root,
};
}
} catch (e) {
// eslint-disable-next-line no-console
console.error(`Error initializing view ${view.id}: ${e}`);
}
});
await Promise.all(promises);
Expand Down
9 changes: 0 additions & 9 deletions src/plugins/discover/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,6 @@ export class DiscoverPlugin
private initializeServices?: () => { core: CoreStart; plugins: DiscoverStartPlugins };

setup(core: CoreSetup<DiscoverStartPlugins, DiscoverStart>, plugins: DiscoverSetupPlugins) {
// TODO: Remove this before merge to main
// eslint-disable-next-line no-console
console.log('DiscoverPlugin.setup()');
const baseUrl = core.http.basePath.prepend('/app/discover');

if (plugins.share) {
Expand Down Expand Up @@ -249,9 +246,6 @@ export class DiscoverPlugin
defaultPath: '#/',
category: DEFAULT_APP_CATEGORIES.opensearchDashboards,
mount: async (params: AppMountParameters) => {
// TODO: Remove this before merge to main
// eslint-disable-next-line no-console
console.log('DiscoverPlugin.mount()');
if (!this.initializeServices) {
throw Error('Discover plugin method initializeServices is undefined');
}
Expand Down Expand Up @@ -352,9 +346,6 @@ export class DiscoverPlugin
}

start(core: CoreStart, plugins: DiscoverStartPlugins) {
// TODO: Remove this before merge to main
// eslint-disable-next-line no-console
console.log('DiscoverPlugin.start()');
setUiActions(plugins.uiActions);

this.initializeServices = () => {
Expand Down

0 comments on commit a92e483

Please sign in to comment.