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

Harden Prompts #2246

Merged
merged 18 commits into from
Jul 28, 2020
Merged

Harden Prompts #2246

merged 18 commits into from
Jul 28, 2020

Conversation

mdrichardson
Copy link
Contributor

@mdrichardson mdrichardson commented May 18, 2020

Fixes microsoft/BotBuilder-Samples#1728
Fixes: #2271

Description

Some of the prompts break if user uploads an attachment instead of responding with text because of some of the string methods that get called on activity.text (which doesn't exist if an attachment is sent).

Specific Changes

Add

if (!utterance) {
  return result;
}

to:

  1. ChoicePrompt
  2. DatetimePrompt
  3. NumberPrompt

The original issue was only for DatetimePrompt, but I added the other two. Other prompts didn't seem relevant for this.

Testing

Tested with CoreBot and it fixed original issue.

Note

I'm currently checking to see if the other SDK languages need something like this.

@coveralls
Copy link

coveralls commented May 18, 2020

Pull Request Test Coverage Report for Build 151204

  • 6 of 7 (85.71%) changed or added relevant lines in 4 files are covered.
  • 9 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.07%) to 81.95%

Changes Missing Coverage Covered Lines Changed/Added Lines %
libraries/botbuilder-dialogs/src/prompts/numberPrompt.ts 1 2 50.0%
Files with Coverage Reduction New Missed Lines %
libraries/botframework-streaming/src/webSocket/webSocketTransport.ts 2 80.65%
libraries/botframework-streaming/src/webSocket/webSocketServer.ts 7 66.67%
Totals Coverage Status
Change from base Build 151130: -0.07%
Covered Lines: 14557
Relevant Lines: 16888

💛 - Coveralls

@mdrichardson mdrichardson requested a review from a team as a code owner June 17, 2020 23:43
@GeoffCoxMSFT GeoffCoxMSFT self-requested a review July 24, 2020 16:27
@mdrichardson
Copy link
Contributor Author

mdrichardson commented Jul 27, 2020

@GeoffCoxMSFT Good suggestions. Made the changes and am requesting re-review.

@stevengum stevengum merged commit 722bede into master Jul 28, 2020
@stevengum stevengum deleted the mdrichardson/promptFix branch July 28, 2020 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PORT] Hardened prompts TypeScript / NodeJS CoreBot Sample assumes user responds to date prompt with text
4 participants