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 welcome event and fixes around speech sagas #1286

Merged
merged 27 commits into from
Jan 13, 2019

Conversation

compulim
Copy link
Contributor

@compulim compulim commented Nov 1, 2018

Fix #1371 and fix #1328.

Background

Since conversationUpdate was not designed for welcome scenario, it lacks of important features required for welcome, e.g. ability to send browser language and information to the bot as the first message.

This PR was designed to add a sample for send welcome event.

But during coding the sample, we talked to redux-saga team because one of their design decision is affecting our code. We settled with their design decision but that forced us to add an interim action (CONNECT_FULFILLING). We revisited all sagas to make sure they can handle successive actions properly.

While revisiting all sagas, we also identified few problematic areas in our sagas:

  • First incoming activity came from user will throw null reference exception in saga
  • Start/stop speech (TTS) dispatches were scattered across multiple sagas
    • sendEvent will stop speaking, behave same as other postActivity helpers
    • We will consolidate all stop speaking behavior in a new saga
  • via argument in setSendBox is never used, removed
  • Input hint was not honored in saga (Speech: Microphone not respecting input hint #1328)
    • This could potentially separated in another PR, but since this is a one-line change and we touched all sagas around speech, we will leave it in for now

Although this PR was originally targeted to add a sample with minor-but-broad change on all sagas, since we practically touched almost every sagas, we also revisited problematic areas to make sure they are working as expected.

CHANGELOG.md

Added

  • Core: Added sendEvent, in PR #1286
  • Core: Added CONNECT_FULFILLING action to workaround redux-saga design decision, in PR #1286

Fixed

  • Core: Some sagas missed handling successive actions, in PR #1286
  • Core: incomingActivitySaga may throw null-ref exception if the first activity is from user, in PR #1286
  • Core: Fix #1328. Should not start microphone if input hint is set to ignoringInput, in PR #1286

Samples

@compulim compulim added 4.2 p0 Must Fix. Release-blocker labels Nov 1, 2018
@compulim compulim added this to the v4.2 milestone Nov 1, 2018
@corinagum
Copy link
Contributor

Fixes #1371

@coveralls
Copy link

coveralls commented Dec 22, 2018

Pull Request Test Coverage Report for Build 748

  • 100 of 139 (71.94%) changed or added relevant lines in 32 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 45.443%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/core/src/actions/sendEvent.js 1 2 50.0%
packages/core/src/sagas/markAllAsSpokenOnStopSpeakActivitySaga.js 3 4 75.0%
packages/core/src/sagas/sendEventToPostActivitySaga.js 3 4 75.0%
packages/core/src/sagas/sendPostBackToPostActivitySaga.js 3 4 75.0%
packages/core/src/sagas/startSpeakActivityOnPostActivitySaga.js 3 4 75.0%
packages/component/src/Dictation.js 0 2 0.0%
packages/core/src/sagas/incomingActivitySaga.js 16 18 88.89%
packages/core/src/sagas/removeIncomingTypingAfterIntervalSaga.js 2 4 50.0%
packages/core/src/sagas/effects/whileSpeakIncomingActivity.js 3 6 50.0%
packages/core/src/sagas/markActivityForSpeakOnIncomingActivityFromOthersSaga.js 2 5 40.0%
Totals Coverage Status
Change from base Build 742: 0.5%
Covered Lines: 744
Relevant Lines: 1460

💛 - Coveralls

@compulim compulim changed the title [DRAFT] Send join event and various fixes Send join event and various fixes Dec 24, 2018
@compulim compulim changed the title Send join event and various fixes Send welcome event and fixes around speech sagas Dec 24, 2018
@compulim compulim removed their assignment Dec 24, 2018
packages/core/src/actions/postActivity.js Outdated Show resolved Hide resolved
packages/core/src/sagas/markActivityForSpeakSaga.js Outdated Show resolved Hide resolved
packages/core/src/sagas/sendEventToPostActivitySaga.js Outdated Show resolved Hide resolved
packages/core/src/sagas/stopDictateOnCardAction.js Outdated Show resolved Hide resolved
packages/core/src/sagas/stopSpeakActivityOnInputSaga.js Outdated Show resolved Hide resolved
samples/05.c.presentation-mode-styling/index.html Outdated Show resolved Hide resolved
samples/07.customization-timestamp-grouping/index.html Outdated Show resolved Hide resolved
@compulim compulim merged commit 849e563 into microsoft:master Jan 13, 2019
@compulim compulim deleted the sample-send-join-event branch January 13, 2019 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p0 Must Fix. Release-blocker size-s 1 days or less
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revert synthesized "conversationUpdate" activity Speech: Microphone not respecting input hint
4 participants