bespoken / virtual-alexa

:robot: Easily test and debug Alexa skills programmatically
https://bespoken.io
Apache License 2.0
112 stars 35 forks source link

Intent confirmation for non-delegated dialogs #61

Closed jlw8ke closed 6 years ago

jlw8ke commented 6 years ago

Currently, Dialog.ConfirmIntent does not work for alexa skills using dialogs without delegating to Alexa.

For example...

myHandler.js

const MyIntentHandler = {
    canHandle(input) {
       return (
           AlexaUtils.getRequestType(input) === 'IntentRequest' &&
           AlexaUtils.getIntent(input).name === 'MyIntent'
        )
    },
    handle(input) {
        if (intent.confirmationStatus !== 'CONFIRMED') {
            if (intent.confirmationStatus !== 'DENIED') {
                return input.responseBuilder
                    .speak('Do yo want to do this?')
                    .addConfirmIntentDirective(intent)
                    .getResponse()
             }
             return input.responseBuilder.speak("Ok, I won't do this.").getResponse()
        }
        return input.responseBuilder.speak('Ok. I will do this').getResponse()
    }
}

myHandler.test.js

it('should respond with a denial message on intent deny', async () => {
   await alexa.utter('Do my intent')
   const { response } = await alexa.utter('no')
   expect(response.outputSpeech.ssml).to.contain("Ok, I won't do this.")
})

Expected Outcome: The intent's confirmationStatus to DENIED Actual Outcome: The intent's confirmationStatus stays undefined

I think the cause of this is https://github.com/bespoken/virtual-alexa/blob/a170f4f622fd8264a4ba8ba97ae1ef3acecf09d3/src/dialog/DialogManager.ts#L179

It looks like the confirmationStatus is only set to NONE if you use Dialog.Delegate first, so the confirmationStatus is never updated in this case.

jkelvie commented 6 years ago

Hi, thank you for identifying this bug, and the very detailed and clear description of the issue. The test cases are also extremely helpful.

I took a crack at this with a slightly different approach: https://github.com/jkelvie/virtual-alexa/commit/b7698e090cc5bef597d17fb7438b489ab6844264

This is meant to hew more closely to the spec - which is that the confirmationStatus should be NONE while the dialog is active.

jlw8ke commented 6 years ago

Thanks for the quick response. I like this approach more since it's more in line with how a real Alexa device would behave.

jkelvie commented 6 years ago

Do you want to update your PR to work like this? I would prefer to give you credit for the contribution.

jlw8ke commented 6 years ago

Sure thing! I'll go ahead and update it.

jkelvie commented 6 years ago

Closed with #62 - thank you for this great PR!