Skip to content

Commit

Permalink
[ML] Fix PDF and PNG export with ML embeddables (#128897)
Browse files Browse the repository at this point in the history
* set up renderComplete callbacks from the swim lane embeddable

* set up renderComplete callbacks from the anomaly charts embeddable

* update output

* set attribute

* update jest tests
  • Loading branch information
darnautov authored Mar 31, 2022
1 parent d101d08 commit c2db728
Show file tree
Hide file tree
Showing 10 changed files with 159 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,28 @@ export class AnomalyChartsEmbeddable extends Embeddable<
}
}

public onLoading() {
this.renderComplete.dispatchInProgress();
this.updateOutput({ loading: true, error: undefined });
}

public onError(error: Error) {
this.renderComplete.dispatchError();
this.updateOutput({ loading: false, error: { name: error.name, message: error.message } });
}

public onRenderComplete() {
this.renderComplete.dispatchComplete();
this.updateOutput({ loading: false, error: undefined });
}

public render(node: HTMLElement) {
super.render(node);
this.node = node;

// required for the export feature to work
this.node.setAttribute('data-shared-item', '');

const I18nContext = this.services[0].i18n.Context;
const theme$ = this.services[0].theme.theme$;

Expand All @@ -114,6 +132,9 @@ export class AnomalyChartsEmbeddable extends Embeddable<
refresh={this.reload$.asObservable()}
onInputChange={this.updateInput.bind(this)}
onOutputChange={this.updateOutput.bind(this)}
onRenderComplete={this.onRenderComplete.bind(this)}
onLoading={this.onLoading.bind(this)}
onError={this.onError.bind(this)}
/>
</Suspense>
</KibanaContextProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ describe('EmbeddableAnomalyChartsContainer', () => {

const onInputChange = jest.fn();
const onOutputChange = jest.fn();
const onRenderComplete = jest.fn();
const onLoading = jest.fn();
const onError = jest.fn();

const mockedInput = {
viewMode: 'view',
Expand Down Expand Up @@ -145,6 +148,9 @@ describe('EmbeddableAnomalyChartsContainer', () => {
refresh={refresh}
onInputChange={onInputChange}
onOutputChange={onOutputChange}
onLoading={onLoading}
onRenderComplete={onRenderComplete}
onError={onError}
/>,
defaultOptions
);
Expand Down Expand Up @@ -172,6 +178,9 @@ describe('EmbeddableAnomalyChartsContainer', () => {
refresh={refresh}
onInputChange={onInputChange}
onOutputChange={onOutputChange}
onLoading={onLoading}
onRenderComplete={onRenderComplete}
onError={onError}
/>,
defaultOptions
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ export interface EmbeddableAnomalyChartsContainerProps {
refresh: Observable<any>;
onInputChange: (input: Partial<AnomalyChartsEmbeddableInput>) => void;
onOutputChange: (output: Partial<AnomalyChartsEmbeddableOutput>) => void;
onRenderComplete: () => void;
onLoading: () => void;
onError: (error: Error) => void;
}

export const EmbeddableAnomalyChartsContainer: FC<EmbeddableAnomalyChartsContainerProps> = ({
Expand All @@ -48,6 +51,9 @@ export const EmbeddableAnomalyChartsContainer: FC<EmbeddableAnomalyChartsContain
refresh,
onInputChange,
onOutputChange,
onRenderComplete,
onError,
onLoading,
}) => {
const [chartWidth, setChartWidth] = useState<number>(0);
const [severity, setSeverity] = useState(
Expand Down Expand Up @@ -94,7 +100,8 @@ export const EmbeddableAnomalyChartsContainer: FC<EmbeddableAnomalyChartsContain
refresh,
services,
chartWidth,
severity.val
severity.val,
{ onRenderComplete, onError, onLoading }
);
const resizeHandler = useCallback(
throttle((e: { width: number; height: number }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ describe('useAnomalyChartsInputResolver', () => {
const start = moment().subtract(1, 'years');
const end = moment();

const renderCallbacks = {
onRenderComplete: jest.fn(),
onLoading: jest.fn(),
onError: jest.fn(),
};

beforeEach(() => {
jest.useFakeTimers();

Expand Down Expand Up @@ -116,21 +122,27 @@ describe('useAnomalyChartsInputResolver', () => {
refresh,
services,
1000,
0
0,
renderCallbacks
)
);

expect(result.current.chartsData).toBe(undefined);
expect(result.current.error).toBe(undefined);
expect(result.current.isLoading).toBe(true);
expect(renderCallbacks.onLoading).toHaveBeenCalledTimes(0);

jest.advanceTimersByTime(501);

expect(renderCallbacks.onLoading).toHaveBeenCalledTimes(1);

const explorerServices = services[2];

expect(explorerServices.anomalyDetectorService.getJobs$).toHaveBeenCalledTimes(1);
expect(explorerServices.anomalyExplorerService.getAnomalyData$).toHaveBeenCalledTimes(1);

expect(renderCallbacks.onRenderComplete).toHaveBeenCalledTimes(1);

embeddableInput.next({
id: 'test-explorer-charts-embeddable',
jobIds: ['anotherJobId'],
Expand All @@ -144,8 +156,14 @@ describe('useAnomalyChartsInputResolver', () => {
});
jest.advanceTimersByTime(501);

expect(renderCallbacks.onLoading).toHaveBeenCalledTimes(2);

expect(explorerServices.anomalyDetectorService.getJobs$).toHaveBeenCalledTimes(2);
expect(explorerServices.anomalyExplorerService.getAnomalyData$).toHaveBeenCalledTimes(2);

expect(renderCallbacks.onRenderComplete).toHaveBeenCalledTimes(2);

expect(renderCallbacks.onError).toHaveBeenCalledTimes(0);
});

test.skip('should not complete the observable on error', async () => {
Expand All @@ -156,7 +174,8 @@ describe('useAnomalyChartsInputResolver', () => {
refresh,
services,
1000,
1
1,
renderCallbacks
)
);

Expand All @@ -168,5 +187,6 @@ describe('useAnomalyChartsInputResolver', () => {
} as Partial<AnomalyChartsEmbeddableInput>);

expect(result.current.error).toBeDefined();
expect(renderCallbacks.onError).toHaveBeenCalledTimes(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ export function useAnomalyChartsInputResolver(
refresh: Observable<any>,
services: [CoreStart, MlStartDependencies, AnomalyChartsServices],
chartWidth: number,
severity: number
severity: number,
renderCallbacks: {
onRenderComplete: () => void;
onLoading: () => void;
onError: (error: Error) => void;
}
): {
chartsData: ExplorerChartsData | undefined;
isLoading: boolean;
Expand All @@ -61,6 +66,9 @@ export function useAnomalyChartsInputResolver(
.pipe(
tap(setIsLoading.bind(null, true)),
debounceTime(FETCH_RESULTS_DEBOUNCE_MS),
tap(() => {
renderCallbacks.onLoading();
}),
switchMap(([explorerJobs, input, embeddableContainerWidth, severityValue]) => {
if (!explorerJobs) {
// couldn't load the list of jobs
Expand Down Expand Up @@ -118,6 +126,8 @@ export function useAnomalyChartsInputResolver(
setError(null);
setChartsData(results);
setIsLoading(false);

renderCallbacks.onRenderComplete();
}
});

Expand All @@ -134,5 +144,11 @@ export function useAnomalyChartsInputResolver(
severity$.next(severity);
}, [severity]);

useEffect(() => {
if (error) {
renderCallbacks.onError(error);
}
}, [error]);

return { chartsData, isLoading, error };
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,28 @@ export class AnomalySwimlaneEmbeddable extends Embeddable<
);
}

public onLoading() {
this.renderComplete.dispatchInProgress();
this.updateOutput({ loading: true, error: undefined });
}

public onError(error: Error) {
this.renderComplete.dispatchError();
this.updateOutput({ loading: false, error: { name: error.name, message: error.message } });
}

public onRenderComplete() {
this.renderComplete.dispatchComplete();
this.updateOutput({ loading: false, error: undefined });
}

public render(node: HTMLElement) {
super.render(node);
this.node = node;

// required for the export feature to work
this.node.setAttribute('data-shared-item', '');

const I18nContext = this.services[0].i18n.Context;
const theme$ = this.services[0].theme.theme$;

Expand All @@ -76,6 +94,9 @@ export class AnomalySwimlaneEmbeddable extends Embeddable<
refresh={this.reload$.asObservable()}
onInputChange={this.updateInput.bind(this)}
onOutputChange={this.updateOutput.bind(this)}
onRenderComplete={this.onRenderComplete.bind(this)}
onLoading={this.onLoading.bind(this)}
onError={this.onError.bind(this)}
/>
</Suspense>
</KibanaContextProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ describe('ExplorerSwimlaneContainer', () => {

const onInputChange = jest.fn();
const onOutputChange = jest.fn();
const onRenderComplete = jest.fn();
const onLoading = jest.fn();
const onError = jest.fn();

beforeEach(() => {
embeddableContext = { id: 'test-id' } as AnomalySwimlaneEmbeddable;
Expand Down Expand Up @@ -102,6 +105,9 @@ describe('ExplorerSwimlaneContainer', () => {
refresh={refresh}
onInputChange={onInputChange}
onOutputChange={onOutputChange}
onLoading={onLoading}
onRenderComplete={onRenderComplete}
onError={onError}
/>,
defaultOptions
);
Expand Down Expand Up @@ -141,6 +147,9 @@ describe('ExplorerSwimlaneContainer', () => {
refresh={refresh}
onInputChange={onInputChange}
onOutputChange={onOutputChange}
onLoading={onLoading}
onRenderComplete={onRenderComplete}
onError={onError}
/>,
defaultOptions
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ export interface ExplorerSwimlaneContainerProps {
refresh: Observable<any>;
onInputChange: (input: Partial<AnomalySwimlaneEmbeddableInput>) => void;
onOutputChange: (output: Partial<AnomalySwimlaneEmbeddableOutput>) => void;
onRenderComplete: () => void;
onLoading: () => void;
onError: (error: Error) => void;
}

export const EmbeddableSwimLaneContainer: FC<ExplorerSwimlaneContainerProps> = ({
Expand All @@ -45,6 +48,9 @@ export const EmbeddableSwimLaneContainer: FC<ExplorerSwimlaneContainerProps> = (
refresh,
onInputChange,
onOutputChange,
onRenderComplete,
onLoading,
onError,
}) => {
const [chartWidth, setChartWidth] = useState<number>(0);

Expand All @@ -61,7 +67,8 @@ export const EmbeddableSwimLaneContainer: FC<ExplorerSwimlaneContainerProps> = (
refresh,
services,
chartWidth,
fromPage
fromPage,
{ onRenderComplete, onError, onLoading }
);

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ describe('useSwimlaneInputResolver', () => {
let services: [CoreStart, MlStartDependencies, AnomalySwimlaneServices];
let onInputChange: jest.Mock;

const renderCallbacks = {
onRenderComplete: jest.fn(),
onLoading: jest.fn(),
onError: jest.fn(),
};

beforeEach(() => {
jest.useFakeTimers();

Expand Down Expand Up @@ -78,6 +84,7 @@ describe('useSwimlaneInputResolver', () => {
];
onInputChange = jest.fn();
});

afterEach(() => {
jest.useRealTimers();
jest.clearAllMocks();
Expand All @@ -91,7 +98,8 @@ describe('useSwimlaneInputResolver', () => {
refresh,
services,
1000,
1
1,
renderCallbacks
)
);

Expand All @@ -106,6 +114,9 @@ describe('useSwimlaneInputResolver', () => {
expect(services[2].anomalyDetectorService.getJobs$).toHaveBeenCalledTimes(1);
expect(services[2].anomalyTimelineService.loadOverallData).toHaveBeenCalledTimes(1);

expect(renderCallbacks.onLoading).toHaveBeenCalledTimes(1);
expect(renderCallbacks.onRenderComplete).toHaveBeenCalledTimes(1);

await act(async () => {
embeddableInput.next({
id: 'test-swimlane-embeddable',
Expand All @@ -121,6 +132,9 @@ describe('useSwimlaneInputResolver', () => {
expect(services[2].anomalyDetectorService.getJobs$).toHaveBeenCalledTimes(2);
expect(services[2].anomalyTimelineService.loadOverallData).toHaveBeenCalledTimes(2);

expect(renderCallbacks.onLoading).toHaveBeenCalledTimes(2);
expect(renderCallbacks.onRenderComplete).toHaveBeenCalledTimes(2);

await act(async () => {
embeddableInput.next({
id: 'test-swimlane-embeddable',
Expand All @@ -135,6 +149,9 @@ describe('useSwimlaneInputResolver', () => {

expect(services[2].anomalyDetectorService.getJobs$).toHaveBeenCalledTimes(2);
expect(services[2].anomalyTimelineService.loadOverallData).toHaveBeenCalledTimes(3);

expect(renderCallbacks.onLoading).toHaveBeenCalledTimes(3);
expect(renderCallbacks.onRenderComplete).toHaveBeenCalledTimes(3);
});

test('should not complete the observable on error', async () => {
Expand All @@ -145,7 +162,8 @@ describe('useSwimlaneInputResolver', () => {
refresh,
services,
1000,
1
1,
renderCallbacks
)
);

Expand All @@ -160,5 +178,7 @@ describe('useSwimlaneInputResolver', () => {
});

expect(result.current[6]?.message).toBe('Invalid job');

expect(renderCallbacks.onError).toHaveBeenCalledTimes(1);
});
});
Loading

0 comments on commit c2db728

Please sign in to comment.