Specialty bot routing fixes

[bobpskier]

Issue #, if available: 674

Description of changes:

Integrate original lexv2 bug fixes from first PR for speciltyBotRouter.js and implement additional fixes as necessary. Address the following merge and feature issues:

• specialtyBotRouter.js will throw an exception as LexV2 will sometimes not return an intent as expected at lexv2response.sessionState.intent. The revised code is not handling this condition.

• specialtyBotRouter.js will throw an exception in getDialogState() for the same undefined lexv2response.sessionState.intent.

• bot router termination is not preserving the last message from the specialty bot in endUseOfSpecialtyBot().

• merging of specialty bot session attributes for supervisory bot response does not handle special cases for "appContext" and "qnabotcontext" when the specialty bot is another QnaBot.

• The matching QID's answer/markdown which initiated bot routing is not provided to the user. The original answer is ignored when it should be prepended as part the response message when bot routing starts.

• The superivsory bot does not terminate bot routing when the specialty LexV2 bot reaches fulfillment or is closed. Incorrect states are being checked. Note that a specialty QnABot (also LexV2) target is a special case where bot routing should not be terminated when fulfillment is reached as each answer represents a fulfilled state. A specialty QnABot requires special processing to terminate bot routing either through a defined session attribute or by the user entering the termination phrase.

• getRespCard() is incorrectly defining several attributes that should be left undefined. One example is an image url in the response card. When an image url is defined in the response card as an empty string which the present code is setting, the LexV2 service handling the fulfillment response from the supervisory bot will return an error and the UI will display an error.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[bobpskier]

@abhirpat Any updates on merging this pull request?

[abhirpat]

Hi @bobpskier , thank you for your effort and patience. Currently, I am reviewing this PR and would like to clarify some of these changes with you.

1. Could you please confirm that all of the improvements you suggested in #674 are included in this PR?

2. After integration this PR, I noticed that the message type was not set to SSML and it didn't met the condition for checking ssmlMessage here. It appears to be related to ssmlMessage that was removed (see deleted line 356 in PR changes). Is this intended behavior? If so, could you please elaborate on reasons for it?

There are unit tests available in the the file if you would like to validate this.

To run specialityBotRouter.test.js -

cd lambda/fulfillment
npm i --dev
npm test test/lib/middleware/specialtyBotRouter.test.js

3. After integration this PR, I noticed that the initial response is being concatenated differently. I noted this while manually testing Routing.002 created through functional tests. Is this intended behavior?

Before PR:

After PR:

4. After integration this PR, I noticed that the "exit" is not returning welcoming message as it used to before (see above screenshots as well). Have you observed on this your end as well? I noted this while testing functional test test_attribute_received_from_specialty_bot_and_chaining to test this.

Please let me know if you have noticed these changes (2 - 4 above) in application behavior on your end as well or if any questions.

Sincerely, Abhishek

[bobpskier]

@abhirpat Hi. Just responding to your questions above.

1. Could you please confirm that all of the improvements you suggested in https://github.com/aws-solutions/qnabot-on-aws/issues/674 are included in this PR?

Yes, the items in 674 are included in this PR which targets a single file: lambda/fulfillment/lib/middleware/specialtyBotRouter.js

2. After integration this PR, I noticed that the message type was not set to SSML and it didn't met the condition for checking ssmlMessage here. It appears to be related to ssmlMessage that was removed (see deleted line 356 in PR changes). Is this intended behavior? If so, could you please elaborate on reasons for it?

content in altMessages.ssml from the specialtyBot should also be appended to any existing ssml content in a similar fahsion. Let me know if you want me to add this in.

3. After integration this PR, I noticed that the initial response is being concatenated differently. I noted this while manually testing Routing.002 created through functional tests. Is this intended behavior?

The behavior depends on how the question that is initiating bot routing is setup. If this question provides an answer or markdown answer, this should be visible in the response.

4. After integration this PR, I noticed that the "exit" is not returning welcoming message as it used to before (see above screenshots as well). Have you observed on this your end as well? I noted this while testing functional test test_attribute_received_from_specialty_bot_and_chaining to test this.

Exit should always just play the welcome back message.

Can you send or point me at the file specialtyBotRouter.js that contains the results of the PR integration and I can investigate?

[abhirpat]

Thank you for reply, Bob. I have following response to your comments (in block quotes) -

content in altMessages.ssml from the specialtyBot should also be appended to any existing ssml content in a similar fahsion. Let me know if you want me to add this in.

• Yes please, if you could add this that will be great. I wasn't entirely sure if it should follow the same condition as altMessages.markdown i.e to check in both appContext and originalAppContext as well .

Exit should always just play the welcome back message.

• It appears to have been changed due to the modifications in the PR (_.get(botResp,'dialogState', "") === 'Fulfilled' || _.get(botResp,'dialogState', "") === 'Close' )

I can fix it by changing this line const resp = endUseOfSpecialtyBot(req, res, undefined) to const resp = endUseOfSpecialtyBot(req, res, welcomeBackMessage). This should address both 3 and 4 that I mentioned earlier. Please let me know if that's what you intended or if we need to have a call for further discussion.