Skip to content

Commit

Permalink
fix: Include settings changes in persistence layer and flush changes …
Browse files Browse the repository at this point in the history
…before switching bot (#3117)

* fix: Wait for task queue flush before switching a bot

* update the action

* Include settings changes in persistence layer

* fix lint

* update the action

* update the error handler for open bot

* use flush method on workers to wait for queues to drain

* use includes instead of indexOf

* use loading spinner

* reset project state when open bot is pending

* show loading when creating new bot

* bind handleMessage when setting onmessage

Co-authored-by: Andy Brown <asbrown002@gmail.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
  • Loading branch information
3 people authored May 18, 2020
1 parent 8e0c795 commit 10f50a6
Show file tree
Hide file tree
Showing 19 changed files with 201 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,20 +88,20 @@ const CreationFlow: React.FC<CreationFlowProps> = () => {
);
};

const handleSubmit = formData => {
const handleSubmit = async formData => {
handleDismiss();
switch (creationFlowStatus) {
case CreationFlowStatus.NEW_FROM_SCRATCH:
case CreationFlowStatus.NEW_FROM_TEMPLATE:
handleCreateNew(formData);
await handleCreateNew(formData);
break;
case CreationFlowStatus.SAVEAS:
handleSaveAs(formData);
break;

default:
handleCreateNew(formData);
await handleCreateNew(formData);
}
handleDismiss();
};

const handleCreateNext = async data => {
Expand Down
1 change: 1 addition & 0 deletions Composer/packages/client/src/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const UNDO_LIMIT = 10;

export enum ActionTypes {
GET_PROJECT_SUCCESS = 'GET_PROJECT_SUCCESS',
GET_PROJECT_PENDING = 'GET_PROJECT_PENDING',
GET_PROJECT_FAILURE = 'GET_PROJECT_FAILURE',
REMOVE_PROJECT_SUCCESS = 'REMOVE_PROJECT_SUCCESS',
GET_RECENT_PROJECTS_SUCCESS = 'GET_RECENT_PROJECTS_SUCCESS',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@

/** @jsx jsx */
import { jsx } from '@emotion/core';
import { useState, useContext, useMemo } from 'react';
import { useState, useContext } from 'react';
import { JsonEditor } from '@bfc/code-editor';
import formatMessage from 'format-message';
import { ChoiceGroup } from 'office-ui-fabric-react/lib/ChoiceGroup';
import { Link } from 'office-ui-fabric-react/lib/Link';
import { RouteComponentProps } from '@reach/router';
import debounce from 'lodash/debounce';

import { StoreContext } from '../../../store';
import { isAbsHosted } from '../../../utils/envUtil';
Expand Down Expand Up @@ -56,13 +55,9 @@ export const DialogSettings: React.FC<RouteComponentProps> = () => {
}
};

const handleChange = useMemo(
() =>
debounce((result: any) => {
saveChangeResult(result);
}, 200),
[projectId]
);
const handleChange = (result: any) => {
saveChangeResult(result);
};

const hostedControl = () => (
<div css={hostedControls}>
Expand Down
13 changes: 11 additions & 2 deletions Composer/packages/client/src/router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ const Skills = React.lazy(() => import('./pages/skills'));
const BotCreationFlowRouter = React.lazy(() => import('./components/CreationFlow'));

const Routes = props => {
const { state } = useContext(StoreContext);
const { botOpening } = state;

return (
<div css={data}>
<Suspense fallback={<LoadingSpinner />}>
Expand All @@ -40,7 +43,6 @@ const Routes = props => {
/>
<Redirect from="/bot/:projectId/publish" to="/bot/:projectId/publish/all" noThrow />
<Redirect from="/" to={resolveToBasePath(BASEPATH, 'home')} noThrow />
{/* <Redirect from="/bot/:projectId" to="/bot/:projectId/dialogs/Main" noThrow /> */}
<ProjectRouter path="/bot/:projectId">
<DesignPage path="dialogs/:dialogId/*" />
<SettingPage path="settings/*" />
Expand All @@ -56,6 +58,13 @@ const Routes = props => {
<NotFound default />
</Router>
</Suspense>
{botOpening && (
<div
css={{ position: 'absolute', top: 0, left: 0, bottom: 0, right: 0, background: 'rgba(255, 255, 255, 0.6)' }}
>
<LoadingSpinner />
</div>
)}
</div>
);
};
Expand All @@ -79,7 +88,7 @@ const ProjectRouter: React.FC<RouteComponentProps<{ projectId: string }>> = prop
}
}, [props.projectId]);

if (props.projectId !== state.projectId) {
if (state.botOpening || props.projectId !== state.projectId) {
return <LoadingSpinner />;
}

Expand Down
7 changes: 4 additions & 3 deletions Composer/packages/client/src/store/action/lg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import { undoable } from '../middlewares/undo';
import { ActionCreator, State, Store } from '../types';
import LgWorker from '../parsers/lgWorker';

export const updateLgFile: ActionCreator = async (store, { id, content }) => {
export const updateLgFile: ActionCreator = async (store, { id, content, projectId }) => {
const result = (await LgWorker.parse(id, content, store.getState().lgFiles)) as LgFile;
store.dispatch({
type: ActionTypes.UPDATE_LG,
payload: { ...result },
payload: { ...result, projectId },
});
};

Expand All @@ -36,8 +36,9 @@ export const undoableUpdateLgFile = undoable(
(state: State, args: any[], isEmpty) => {
if (isEmpty) {
const id = args[0].id;
const projectId = args[0].projectId;
const content = clonedeep(state.lgFiles.find(lgFile => lgFile.id === id)?.content);
return [{ id, content }];
return [{ id, content, projectId }];
} else {
return args;
}
Expand Down
4 changes: 2 additions & 2 deletions Composer/packages/client/src/store/action/lu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import LuWorker from '../parsers/luWorker';
import httpClient from './../../utils/httpUtil';
import { ActionTypes } from './../../constants/index';

export const updateLuFile: ActionCreator = async (store, { id, content }) => {
export const updateLuFile: ActionCreator = async (store, { id, projectId, content }) => {
const result = (await LuWorker.parse(id, content)) as LuFile;
store.dispatch({
type: ActionTypes.UPDATE_LU,
payload: { ...result },
payload: { ...result, projectId },
});
};

Expand Down
27 changes: 22 additions & 5 deletions Composer/packages/client/src/store/action/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
import { navigate } from '@reach/router';

import { ActionCreator } from '../types';
import filePersistence from '../persistence/FilePersistence';
import lgWorker from '../parsers/lgWorker';
import luWorker from '../parsers/luWorker';

import { ActionTypes, BASEPATH, BotStatus } from './../../constants/index';
import { navigateTo } from './../../utils/navigation';
Expand All @@ -12,6 +15,19 @@ import settingStorage from './../../utils/dialogSettingStorage';
import luFileStatusStorage from './../../utils/luFileStatusStorage';
import httpClient from './../../utils/httpUtil';

const checkProjectUpdates = async () => {
const workers = [filePersistence, lgWorker, luWorker];

return Promise.all(workers.map(w => w.flush()));
};

export const setOpenPendingStatus: ActionCreator = async store => {
store.dispatch({
type: ActionTypes.GET_PROJECT_PENDING,
});
await checkProjectUpdates();
};

export const setCreationFlowStatus: ActionCreator = ({ dispatch }, creationFlowStatus) => {
dispatch({
type: ActionTypes.SET_CREATION_FLOW_STATUS,
Expand Down Expand Up @@ -105,6 +121,7 @@ export const openBotProject: ActionCreator = async (store, absolutePath) => {
storageId,
path: absolutePath,
};
await setOpenPendingStatus(store);
const response = await httpClient.put(`/projects/open`, data);
const files = response.data.files;
const projectId = response.data.id;
Expand All @@ -121,12 +138,11 @@ export const openBotProject: ActionCreator = async (store, absolutePath) => {
} else {
navigate(BASEPATH);
}
} catch (err) {
} catch (error) {
store.dispatch({
type: ActionTypes.SET_ERROR,
type: ActionTypes.GET_PROJECT_FAILURE,
payload: {
summary: 'Failed to open bot',
message: err.response.data.message,
error,
},
});
store.dispatch({
Expand All @@ -148,6 +164,7 @@ export const saveProjectAs: ActionCreator = async (store, projectId, name, descr
description,
location,
};
await setOpenPendingStatus(store);
const response = await httpClient.post(`/projects/${projectId}/project/saveAs`, data);
const files = response.data.files;
const newProjectId = response.data.id;
Expand Down Expand Up @@ -188,7 +205,7 @@ export const createProject: ActionCreator = async (
location,
schemaUrl,
};

await setOpenPendingStatus(store);
const response = await httpClient.post(`/projects`, data);
const files = response.data.files;
settingStorage.remove(name);
Expand Down
42 changes: 8 additions & 34 deletions Composer/packages/client/src/store/action/setting.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import get from 'lodash/get';
import has from 'lodash/has';
import { SensitiveProperties } from '@bfc/shared';

import { ActionCreator, DialogSetting } from '../types';
import settingsStorage from '../../utils/dialogSettingStorage';

import { ActionTypes } from './../../constants/index';
import { BotEnvironments } from './../../utils/envUtil';
Expand All @@ -16,36 +11,15 @@ export const setSettings: ActionCreator = async (
{ dispatch },
projectId: string,
botName: string,
settings: DialogSetting,
slot?: BotEnvironments
settings: DialogSetting
) => {
try {
// set value to store
dispatch({
type: ActionTypes.SYNC_ENV_SETTING,
payload: {
settings,
},
});
// set value in local storage
for (const property of SensitiveProperties) {
if (has(settings, property)) {
const propertyValue = get(settings, property, '');
settingsStorage.setField(botName, property, propertyValue);
}
}
// set value to server
const suffix = slot ? `/${slot}` : '';
await httpClient.post(`/projects/${projectId}/settings/${suffix}`, { settings });
} catch (err) {
dispatch({
type: ActionTypes.SET_ERROR,
payload: {
message: err.response && err.response.data.message ? err.response.data.message : err,
summary: 'SYNC CONFIG ERROR',
},
});
}
dispatch({
type: ActionTypes.SYNC_ENV_SETTING,
payload: {
projectId,
settings,
},
});
};

export const setDialogSettingsSlot = async ({ dispatch }, projectId: string, slot?: BotEnvironments) => {
Expand Down
1 change: 1 addition & 0 deletions Composer/packages/client/src/store/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export const initialState: State = {
showing: false,
status: AppUpdaterStatus.IDLE,
},
botOpening: false,
};

export interface StoreContextValue {
Expand Down
33 changes: 28 additions & 5 deletions Composer/packages/client/src/store/parsers/baseWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@

import uniqueId from 'lodash/uniqueId';

interface WorkerMsg {
data: {
id: string;
error?: any;
payload?: any;
};
}

// Wrapper class
export class BaseWorker {
private worker: Worker;
Expand All @@ -11,10 +19,10 @@ export class BaseWorker {

constructor(worker: Worker) {
this.worker = worker;
this.worker.onmessage = this.handleMsg;
this.worker.onmessage = this.handleMsg.bind(this);
}

sendMsg = <Payload>(payload: Payload) => {
public sendMsg<Payload>(payload: Payload) {
const msgId = uniqueId();
const msg = { id: msgId, payload };
return new Promise((resolve, reject) => {
Expand All @@ -23,10 +31,10 @@ export class BaseWorker {
this.rejects[msgId] = reject;
this.worker.postMessage(msg);
});
};
}

// Handle incoming calculation result
handleMsg = msg => {
public handleMsg(msg: WorkerMsg) {
const { id, error, payload } = msg.data;
if (error) {
const reject = this.rejects[id];
Expand All @@ -39,5 +47,20 @@ export class BaseWorker {
// purge used callbacks
delete this.resolves[id];
delete this.rejects[id];
};
}

public async flush(): Promise<boolean> {
return new Promise(resolve => {
const timer = setInterval(() => {
if (this.isEmpty()) {
clearInterval(timer);
resolve(true);
}
}, 100);
});
}

private isEmpty() {
return !Object.keys(this.resolves).length;
}
}
Loading

0 comments on commit 10f50a6

Please sign in to comment.