Skip to content

Commit

Permalink
fix: publish button and botStatus item should be disabled to click wh…
Browse files Browse the repository at this point in the history 
…en some bot is publishing (#5096)

* publish disabled

* icon color

* unit test

* get status loop

* set publishDisabled logic independently

Co-authored-by: Dong Lei <donglei@microsoft.com>
  • Loading branch information
@alanlong9278 @boydc2014
alanlong9278 and boydc2014 authored Dec 4, 2020
1 parent 1a28b60 commit 8cba0f5
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 36 deletions.
1 change: 1 addition & 0 deletions Composer/packages/client/__tests__/pages/publish/Publish.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ describe('publish page', () => {
changePublishTarget={jest.fn()}
items={[]}
projectId={rootState.projectId}
publishDisabled={false}
updateItems={jest.fn()}
updatePublishHistory={jest.fn()}
updateSelectedBots={jest.fn()}
Expand Down
10 changes: 9 additions & 1 deletion Composer/packages/client/src/pages/publish/BotStatusList.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export type IBotStatusListProps = {
items: IBotStatus[];
botPublishHistoryList: { projectId: string; publishHistory: { [key: string]: IStatus[] } }[];
botPublishTypesList: { projectId: string; publishTypes: PublishType[] }[];
publishDisabled: boolean;
updateItems: (items: IBotStatus[]) => void;
updatePublishHistory: (items: IStatus[], item: IBotStatus) => void;
updateSelectedBots: (items: IBotStatus[]) => void;
Expand All @@ -51,6 +52,7 @@ export const BotStatusList: React.FC<IBotStatusListProps> = (props) => {
items,
botPublishHistoryList,
botPublishTypesList,
publishDisabled,
updateItems,
updatePublishHistory,
changePublishTarget,
Expand Down Expand Up @@ -150,7 +152,13 @@ export const BotStatusList: React.FC<IBotStatusListProps> = (props) => {
onColumnClick: sortByName,
data: 'string',
onRender: (item: IBotStatus) => {
return <Checkbox label={item.name} onChange={(_, isChecked) => changeSelected(item, isChecked)} />;
return (
<Checkbox
disabled={publishDisabled}
label={item.name}
onChange={(_, isChecked) => changeSelected(item, isChecked)}
/>
);
},
isPadded: true,
},
Expand Down
98 changes: 63 additions & 35 deletions Composer/packages/client/src/pages/publish/Publish.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const Publish: React.FC<RouteComponentProps<{ projectId: string; targetName?: st
} = useRecoilValue(dispatcherState);

const [selectedBots, setSelectedBots] = useState<IBotStatus[]>([]);
const [publishDisabled, setPublishDisabled] = useState(false);

const [showNotifications, setShowNotifications] = useState<Record<string, boolean>>({});
// fill Settings, status, publishType, publish target for bot from botProjectMeta
Expand Down Expand Up @@ -121,13 +122,13 @@ const Publish: React.FC<RouteComponentProps<{ projectId: string; targetName?: st
element: (
<ActionButton
data-testid="publishPage-Toolbar-Publish"
disabled={selectedBots.length === 0}
disabled={publishDisabled || selectedBots.length === 0}
onClick={() => setPublishDialogHidden(false)}
>
<svg fill="none" height="15" viewBox="0 0 16 15" width="16" xmlns="http://www.w3.org/2000/svg">
<path
d="M16 4.28906V15H5V0H11.7109L16 4.28906ZM12 4H14.2891L12 1.71094V4ZM15 14V5H11V1H6V14H15ZM0 5H4V6H0V5ZM1 7H4V8H1V7ZM2 9H4V10H2V9Z"
fill={selectedBots.length > 0 ? '#0078D4' : 'rgb(161, 159, 157)'}
fill={selectedBots.length > 0 && !publishDisabled ? '#0078D4' : 'rgb(161, 159, 157)'}
/>
</svg>
<span css={{ marginLeft: '8px' }}>{formatMessage('Publish selected bots')}</span>
Expand All @@ -148,21 +149,41 @@ const Publish: React.FC<RouteComponentProps<{ projectId: string; targetName?: st
disabled: !isPullSupported,
},
];
const getUpdatedStatus = (target, botProjectId) => {
if (target) {
// TODO: this should use a backoff mechanism to not overload the server with requests
// OR BETTER YET, use a websocket events system to receive updates... (SOON!)
setTimeout(async () => {
getPublishStatus(botProjectId, target);
}, publishStatusInterval);
}
const [statusIntervals, setStatusIntervals] = useState<{ [key: string]: NodeJS.Timeout }[]>([]);
const getUpdatedStatus = (target, botProjectId): NodeJS.Timeout => {
// TODO: this should use a backoff mechanism to not overload the server with requests
// OR BETTER YET, use a websocket events system to receive updates... (SOON!)
return setInterval(async () => {
getPublishStatus(botProjectId, target);
}, publishStatusInterval);
};

const [pendingNotification, setPendingNotification] = useState<Notification>();
const [previousBotPublishHistoryList, setPreviousBotPublishHistoryList] = useState(botPublishHistoryList);
// check history to see if a 202 is found
useEffect(() => {
// most recent item is a 202, which means we should poll for updates...
// set publishDisabled
setPublishDisabled(
selectedBots.some((bot) => {
if (!(bot.publishTarget && bot.publishTargets)) {
return false;
}
const selectedTarget = bot.publishTargets.find((target) => target.name === bot.publishTarget);
const botProjectId = bot.id;
if (!selectedTarget) return false;
const botPublishHistory = botPublishHistoryList.find(
(publishHistory) => publishHistory.projectId === botProjectId
)?.publishHistory[bot.publishTarget];
if (!botPublishHistory || botPublishHistory.length === 0) {
return;
}
const latestPublishItem = botPublishHistory[0];
if (latestPublishItem.status === 202) {
return true;
}
})
);

selectedBots.forEach((bot) => {
if (!(bot.publishTarget && bot.publishTargets)) {
return;
Expand All @@ -180,9 +201,14 @@ const Publish: React.FC<RouteComponentProps<{ projectId: string; targetName?: st
return;
}
const latestPublishItem = botPublishHistory[0];
if (latestPublishItem.status === 202) {
getUpdatedStatus(selectedTarget, botProjectId);
} else if (latestPublishItem.status === 200 || latestPublishItem.status === 500) {
// stop polling if status is 200 or 500
if (latestPublishItem.status === 200 || latestPublishItem.status === 500) {
const interval = statusIntervals.find((i) => i[bot.id]);
if (interval) {
clearInterval(interval[bot.id]);
setStatusIntervals(statusIntervals.filter((i) => !i[botProjectId]));
}
// show result notifications
if (!isEqual(previousBotPublishHistory, botPublishHistory)) {
bot.status = latestPublishItem.status;
if (showNotifications[bot.id]) {
Expand All @@ -191,10 +217,6 @@ const Publish: React.FC<RouteComponentProps<{ projectId: string; targetName?: st
setShowNotifications({ ...showNotifications, [botProjectId]: false });
}
}
} else if (selectedTarget && selectedTarget.lastPublished && botPublishHistory.length === 0) {
// if the history is EMPTY, but we think we've done a publish based on lastPublished timestamp,
// we still poll for the results IF we see that a publish has happened previously
getPublishStatus(botProjectId, selectedTarget);
}
setBotStatusList(
botStatusList.map((item) => {
Expand Down Expand Up @@ -322,6 +344,7 @@ const Publish: React.FC<RouteComponentProps<{ projectId: string; targetName?: st
setSelectedBots(bots);
};
const publish = async (items: IBotStatus[]) => {
setPublishDisabled(true);
setPreviousBotPublishHistoryList(botPublishHistoryList);
// notifications
setShowNotifications(
Expand All @@ -335,31 +358,35 @@ const Publish: React.FC<RouteComponentProps<{ projectId: string; targetName?: st
addNotification(notification);

// publish to remote
const intervals: { [key: string]: NodeJS.Timeout }[] = [];
for (const bot of items) {
if (bot.publishTarget && bot.publishTargets) {
const selectedTarget = bot.publishTargets.find((target) => target.name === bot.publishTarget);
const botProjectId = bot.id;
const setting = botSettingList.find((botsetting) => botsetting.projectId === bot.id)?.setting;
if (setting && setting.publishTargets) {
setting.qna.subscriptionKey && (await setQnASettings(botProjectId, setting.qna.subscriptionKey));
const sensitiveSettings = getSensitiveProperties(setting);
await publishToTarget(botProjectId, selectedTarget, { comment: bot.comment }, sensitiveSettings);
if (!(setting && setting.publishTargets)) {
return;
}
setting.qna.subscriptionKey && (await setQnASettings(botProjectId, setting.qna.subscriptionKey));
const sensitiveSettings = getSensitiveProperties(setting);
await publishToTarget(botProjectId, selectedTarget, { comment: bot.comment }, sensitiveSettings);

// update the target with a lastPublished date
const updatedPublishTargets = setting.publishTargets.map((profile) => {
if (profile.name === selectedTarget?.name) {
return {
...profile,
lastPublished: new Date(),
};
} else {
return profile;
}
});
// update the target with a lastPublished date
const updatedPublishTargets = setting.publishTargets.map((profile) => {
if (profile.name === selectedTarget?.name) {
return {
...profile,
lastPublished: new Date(),
};
} else {
return profile;
}
});

await setPublishTargets(updatedPublishTargets, botProjectId);
}
await setPublishTargets(updatedPublishTargets, botProjectId);
intervals.push({ [botProjectId]: getUpdatedStatus(selectedTarget, botProjectId) });
}
setStatusIntervals(intervals);
setBotStatusList(
botStatusList.map((bot) => {
const item = items.find((i) => i.id === bot.id);
Expand Down Expand Up @@ -416,6 +443,7 @@ const Publish: React.FC<RouteComponentProps<{ projectId: string; targetName?: st
changePublishTarget={changePublishTarget}
items={botStatusList}
projectId={projectId}
publishDisabled={publishDisabled}
updateItems={updateBotStatusList}
updatePublishHistory={updatePublishHistory}
updateSelectedBots={updateSelectedBots}
Expand Down

0 comments on commit 8cba0f5

Please sign in to comment.