Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send postBack and messageBack even if text/value is empty #3120

Merged
merged 2 commits into from
Apr 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Associate the activity text with its attachments, by adding a `role="region"` to the activity DOM element.
- Fixes [#3074](https://github.com/microsoft/BotFramework-WebChat/issues/3074). Keep `props.locale` when sending to the bot, by [@compulim](https://github.com/compulim) in PR [#3095](https://github.com/microsoft/BotFramework-WebChat/issue/3095)
- Fixes [#3096](https://github.com/microsoft/BotFramework-WebChat/issues/3096). Use `<ScreenReaderText>` instead of `aria-label` for message bubbles, by [@compulim](https://github.com/compulim) in PR [#3097](https://github.com/microsoft/BotFramework-WebChat/issue/3097)
- Fixes [#2876](https://github.com/microsoft/BotFramework-WebChat/issues/2876). `messageBack` and `postBack` should send even if both `text` and `value` is falsy or `undefined`, by [@compulim](https://github.com/compulim) in PR [#3120](https://github.com/microsoft/BotFramework-WebChat/issues/3120)

### Changed

Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
90 changes: 0 additions & 90 deletions __tests__/heroCardActions.js

This file was deleted.

15 changes: 15 additions & 0 deletions __tests__/html/__jest__/parseURLParams.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export default function parseURLParams(search) {
return search
.replace(/^[#\?]/, '')
.split('&')
.reduce((params, keyValue) => {
const [key, value] = keyValue.split('=');
const decodedKey = decodeURIComponent(key);

if (key && key !== '__proto__' && key !== 'constructor' && key !== 'prototype') {
params[decodedKey] = decodeURIComponent(value);
}

return params;
}, {});
}
24 changes: 18 additions & 6 deletions __tests__/html/__jest__/setupRunHTMLTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import fetch from 'node-fetch';

import indent from './indent';
import mergeCoverageMap from './mergeCoverageMap';

import parseURLParams from './parseURLParams';
import runPageProcessor from './runPageProcessor';

global.runHTMLTest = async (
Expand All @@ -31,15 +31,27 @@ global.runHTMLTest = async (
.build();

const sessionId = (await driver.getSession()).getId();
const params = parseURLParams(new URL(url, 'http://webchat2/').hash);

try {
// For unknown reason, if we use ?wd=1, it will be removed.
// But when we use #wd=1, it kept.
await driver.get(
global.docker
? new URL(`#wd=1`, new URL(url, 'http://webchat2/'))
: new URL(url, `http://localhost:${global.webServerPort}/`)
);

if (global.docker) {
params.wd = 1;
}

const baseURL = global.docker
? new URL(url, 'http://webchat2/')
: new URL(url, `http://localhost:${global.webServerPort}/`);

const hash =
'#' +
Object.entries(params)
.map(([name, value]) => `${encodeURIComponent(name)}=${encodeURIComponent(value)}`)
.join('&');

await driver.get(new URL(hash, baseURL));

await runPageProcessor(driver, { ignoreConsoleError, ignorePageError });

Expand Down
54 changes: 54 additions & 0 deletions __tests__/html/heroCard.actions.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<!DOCTYPE html>
<html lang="en-US">
<head>
<script crossorigin="anonymous" src="https://unpkg.com/@babel/standalone@7/babel.min.js"></script>
<script crossorigin="anonymous" src="https://unpkg.com/regenerator-runtime/runtime.js"></script>
<script crossorigin="anonymous" src="https://unpkg.com/[email protected]/umd/react.development.js"></script>
<script crossorigin="anonymous" src="https://unpkg.com/[email protected]/umd/react-dom.development.js"></script>
<script
crossorigin="anonymous"
src="https://unpkg.com/[email protected]/umd/react-dom-test-utils.development.js"
></script>
<script crossorigin="anonymous" src="/__dist__/testharness.js"></script>
<link href="/assets/index.css" rel="stylesheet" type="text/css" />
</head>
<body>
<div id="webchat"></div>
<script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
<script type="text/babel" data-presets="es2015,stage-3">
const { conditions, createStore, host, pageObjects, timeouts, token } = window.WebChatTest;

(async function() {
const store = createStore();
const directLine = window.WebChat.createDirectLine({ token: await token.fetchDirectLineToken() });

window.WebChat.renderWebChat(
{
directLine,
store
},
document.getElementById('webchat')
);

await pageObjects.wait(conditions.uiConnected(), timeouts.directLine);
await pageObjects.sendMessageViaSendBox('herocardactions', { waitForSend: true });
await pageObjects.wait(conditions.minNumActivitiesShown(2), timeouts.directLine);

const buttonIndex = host.param('btn');
const numActivities = host.param('exp');

document.querySelector(`.ac-actionSet button:nth-of-type(${buttonIndex})`).click();

await pageObjects.wait(conditions.minNumActivitiesShown(numActivities), timeouts.directLine);
await pageObjects.wait(conditions.scrollToBottomCompleted(), timeouts.directLine);

await host.snapshot();
await host.done();
})().catch(async err => {
console.error(err);

await host.error(err);
});
</script>
</body>
</html>
14 changes: 14 additions & 0 deletions __tests__/html/heroCard.actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* @jest-environment ./__tests__/html/__jest__/WebChatEnvironment.js
*/

describe('hero card actions', () => {
test('imBack', () => runHTMLTest('heroCard.actions.html#btn=1&exp=4'));
test('postBack (string)', () => runHTMLTest('heroCard.actions.html#btn=2&exp=3'));
test('postBack (JSON)', () => runHTMLTest('heroCard.actions.html#btn=3&exp=3'));
test('messageBack (displayText + text + value)', () => runHTMLTest('heroCard.actions.html#btn=4&exp=4'));
test('messageBack (displayText + text)', () => runHTMLTest('heroCard.actions.html#btn=5&exp=4'));
test('messageBack (value)', () => runHTMLTest('heroCard.actions.html#btn=6&exp=3'));
test('postBack (empty)', () => runHTMLTest('heroCard.actions.html#btn=7&exp=3'));
test('messageBack (empty)', () => runHTMLTest('heroCard.actions.html#btn=8&exp=3'));
});
27 changes: 13 additions & 14 deletions packages/core/src/sagas/sendMessageBackToPostActivitySaga.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,20 @@ import { SEND_MESSAGE_BACK } from '../actions/sendMessageBack';
import postActivity from '../actions/postActivity';
import whileConnected from './effects/whileConnected';

// https://github.com/microsoft/botframework-sdk/blob/master/specs/botframework-activity/botframework-activity.md#message-back
function* postActivityWithMessageBack({ payload: { displayText, text, value } }) {
if (text || value) {
yield put(
postActivity({
channelData: {
messageBack: {
displayText
}
},
text,
type: 'message',
value
})
);
}
yield put(
postActivity({
channelData: {
messageBack: {
displayText
}
},
text,
type: 'message',
value
})
);
}

function* sendMessageBackToPostActivity() {
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/sagas/sendPostBackToPostActivitySaga.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { SEND_POST_BACK } from '../actions/sendPostBack';
import postActivity from '../actions/postActivity';
import whileConnected from './effects/whileConnected';

// https://github.com/microsoft/botframework-sdk/blob/master/specs/botframework-activity/botframework-activity.md#post-back
function* postActivityWithPostBack({ payload: { value } }) {
yield put(
postActivity({
Expand All @@ -21,7 +22,7 @@ function* postActivityWithPostBack({ payload: { value } }) {
}

function* sendPostBackToPostActivity() {
yield takeEvery(({ payload, type }) => type === SEND_POST_BACK && payload.value, postActivityWithPostBack);
yield takeEvery(SEND_POST_BACK, postActivityWithPostBack);
}

export default function* sendPostBackToPostActivitySaga() {
Expand Down
2 changes: 2 additions & 0 deletions packages/testharness/src/conditions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import actionDispatched from './actionDispatched';
import allOutgoingActivitiesSent from './allOutgoingActivitiesSent';
import connectivityStatusShown from './connectivityStatusShown';
import minNumActivitiesShown from './minNumActivitiesShown';
import scrollToBottomCompleted from './scrollToBottomCompleted';
import toastShown from './toastShown';
import uiConnected from './uiConnected';
import webChatRendered from './webChatRendered';
Expand All @@ -11,6 +12,7 @@ export {
allOutgoingActivitiesSent,
connectivityStatusShown,
minNumActivitiesShown,
scrollToBottomCompleted,
toastShown,
uiConnected,
webChatRendered
Expand Down
25 changes: 25 additions & 0 deletions packages/testharness/src/conditions/scrollToBottomCompleted.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import getTranscriptScrollableElement from '../elements/transcriptScrollable';

function completed() {
const scrollable = getTranscriptScrollableElement();

return scrollable && scrollable.offsetHeight + scrollable.scrollTop === scrollable.scrollHeight;
}

export default function scrollToBottomCompleted() {
return {
message: 'scroll to bottom completed',
fn: async () => {
// Browser may keep rendering content. Wait until 5 consecutive completion checks are all truthy.
for (let count = 0; count < 5; count++) {
if (!completed()) {
return false;
}

await new Promise(resolve => setTimeout(resolve, 100));
}

return true;
}
};
}
12 changes: 11 additions & 1 deletion packages/testharness/src/elements/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,15 @@ import sendBoxTextBox from './sendBoxTextBox';
import toastDismissButtons from './toastDismissButtons';
import toasterHeader from './toasterHeader';
import toasts from './toasts';
import transcript from './transcript';
import transcriptScrollable from './transcriptScrollable';

export { connectivityStatus, sendBoxTextBox, toasterHeader, toastDismissButtons, toasts };
export {
connectivityStatus,
sendBoxTextBox,
toasterHeader,
toastDismissButtons,
toasts,
transcript,
transcriptScrollable
};
3 changes: 3 additions & 0 deletions packages/testharness/src/elements/transcript.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function transcript() {
return document.querySelector('[role="log"]:not(.webchat__toaster)');
}
5 changes: 5 additions & 0 deletions packages/testharness/src/elements/transcriptScrollable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import getTranscriptElement from './transcript';

export default function transcriptScrollable() {
return getTranscriptElement().querySelector('*');
}
3 changes: 2 additions & 1 deletion packages/testharness/src/host/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import done from './done';
import error from './error';
import pageError from './pageError';
import param from './param';
import snapshot from './snapshot';

export { done, error, pageError, snapshot };
export { done, error, pageError, param, snapshot };
7 changes: 7 additions & 0 deletions packages/testharness/src/host/param.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import parseURLParams from '../utils/parseURLParams';

export default function param(name) {
const value = parseURLParams(document.location.hash)[name];

return /^\d+$/.test(value) ? +value : value;
}
17 changes: 1 addition & 16 deletions packages/testharness/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import * as pageObjects from './pageObjects/index';
import * as token from './token/index';
import createStore, { getActionHistory } from './utils/createStore';
import pageError from './host/pageError';
import parseURLParams from './utils/parseURLParams';
import runAsyncInterval from './utils/runAsyncInterval';
import shareObservable from './utils/shareObservable';
import sleep from './utils/sleep';
Expand All @@ -33,22 +34,6 @@ export {

const log = console.log.bind(console);

function parseURLParams(search) {
return search
.replace(/^[#\?]/, '')
.split('&')
.reduce((params, keyValue) => {
const [key, value] = keyValue.split('=');
const decodedKey = decodeURIComponent(key);

if (key && key !== '__proto__' && key !== 'constructor' && key !== 'prototype') {
params[decodedKey] = decodeURIComponent(value);
}

return params;
}, {});
}

// If not running under WebDriver, we handle all jobs here.
const webDriverMode = 'wd' in parseURLParams(location.hash);

Expand Down
Loading