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

[#6433] Error in AdaptiveDialog.ContinueActionAsync with native dialog SDK #6444

Merged
merged 3 commits into from
Aug 30, 2022

Conversation

sw-joelmut
Copy link
Collaborator

@sw-joelmut sw-joelmut commented Aug 18, 2022

Fixes #6433

Description

This PR fixes an issue where the ComponentDialog cannot be found in the Dialogs stack and in the ResourceExplorer when using AdaptiveDialogs, adding a condition to evaluate the resource existence before trying to load it into the dialogs stack.
Additionally, it reworks the solution to load the dialog resource from the DialogContext instead when it cannot find the dialog (more information can be found in this comment).

Specific Changes

  • Adds a pre-condition before trying to load the dialog from the ResourceExplorer.
  • Added a unit test validating the new condition when using ComponentDialogs with AdaptiveDialogs.
  • Added FindDialog method in the AdaptiveDialog class, that detects when there is no Dialog in the stack, it loads it from the ResourceExplorer.
  • Added unit test that handles bot reconnection and resumes the conversation.
  • Updated the DialogContext.FindDialog method to detect when the AdaptiveDialog cannot be found, delegate the search to the AdaptiveDialog class.
  • Updated the AdaptiveDialog_LoadDialogFromProperty.test.dialog unit test, validating the new condition.
  • Removed previous implementation in BaseInvokeDialog and AdaptiveDialog related to the PRs (# 6338, # 6365).

Testing

The following image shows the new unit tests passing and the conversation samples detecting the LUIS Activity.
image

@sw-joelmut sw-joelmut added the Automation: No parity PR does not need to be applied to other languages. label Aug 18, 2022
@sw-joelmut sw-joelmut requested a review from a team as a code owner August 18, 2022 18:56
@sw-joelmut
Copy link
Collaborator Author

Hi @MatteoMilanese, @EricDahlvang, @johnataylor, @sbiaudet, @ramfattah, @tracyboehrer.

The past few days, we've been looking for a way to solve the issue described by @MatteoMilanese in this comment.
We were able to reproduce the issue described in the comment, and the solution we found is to let the DialogContext class detect when the AdaptiveDialog couldn't be found in the dialogs stack and allow the Parent dialog to find it.
Moreover, the AdaptiveDialog class will include a second FindDialog method, so in case the Dialog cannot be found, gather it from the ResourceExplorer.

Additionally, we already pushed the changes and updated the PR's description.

Note: Previous implementations made in the PRs (#6338, #6365) have been removed (except unit tests), as they are no longer being evaluated.

Thanks

@BruceHaley
Copy link
Contributor

✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Dialogs.dll

@tracyboehrer tracyboehrer merged commit 14e5ca0 into main Aug 30, 2022
@tracyboehrer tracyboehrer deleted the southworks/fix/adaptive-native-dialog branch August 30, 2022 13:44
tracyboehrer pushed a commit that referenced this pull request Sep 1, 2022
…g SDK (#6444)

* Add condition before loading the resource

* Add unit tests

* Load AdaptiveDialogs dynamically on DialogContext
tracyboehrer added a commit that referenced this pull request Sep 1, 2022
* README version to 4.18 (#6425)

Co-authored-by: Tracy Boehrer <trboehre@microsoft.com>

* [#6434] Priority broken for RegexRecognizer (#6435)

* Consider priority in OnRecognizeAsync method

* Add unit test

* Update Bool function to use Convert.ToBoolean (#6431)

* Update IsMatch empty string values (#6426)

* [#6428] Adaptive ForEachElement loop's incorrectly when changes cause ContinueDialogAsync to be recalled (#6448)

* Reacquire childDialogState after ContinueDialogAsync

* Add unit test

* Support passing sas token url's for token service (#6449)

Co-authored-by: Swagat Mishra <swagatm@microsoft.com>

* Bump @actions/core from 1.6.0 to 1.9.1 in /actions/verify-pr-labels (#6445)

Bumps [@actions/core](https://github.com/actions/toolkit/tree/HEAD/packages/core) from 1.6.0 to 1.9.1.
- [Release notes](https://github.com/actions/toolkit/releases)
- [Changelog](https://github.com/actions/toolkit/blob/main/packages/core/RELEASES.md)
- [Commits](https://github.com/actions/toolkit/commits/HEAD/packages/core)

---
updated-dependencies:
- dependency-name: "@actions/core"
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Support passing sas token url's for token service (#6449)

Co-authored-by: Swagat Mishra <swagatm@microsoft.com>

* [#6428] Adaptive ForEachElement loop's incorrectly when changes cause ContinueDialogAsync to be recalled (#6448)

* Reacquire childDialogState after ContinueDialogAsync

* Add unit test

* Delete vnext code which is dead (#6440)

* Fix PublishToCoveralls.ps1 (#6439)

* Remove failing Powershell patch download

* Upgrade coveralls.net to 4.0.1

* [#6430] Adaptive ForEachElement does not exit when child action CancelAllDialogs is called (#6452)

* Consider canceled status to end the dialog

* Add unit test

* [#6432] TeamsInfo.GetMemberAsync(...) doesn't work properly in Skill Bot scenario, it returns http 405 error (#6443)

* Implement GetMemberAsync for skills

* Add unit test

* [#6433] Error in AdaptiveDialog.ContinueActionAsync with native dialog SDK (#6444)

* Add condition before loading the resource

* Add unit tests

* Load AdaptiveDialogs dynamically on DialogContext

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Tracy Boehrer <trboehre@microsoft.com>
Co-authored-by: Cecilia Avila <44245136+ceciliaavila@users.noreply.github.com>
Co-authored-by: Joel Mut <62260472+sw-joelmut@users.noreply.github.com>
Co-authored-by: swagat mishra <swagatmishra2007@gmail.com>
Co-authored-by: Swagat Mishra <swagatm@microsoft.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: BruceHaley <v-brucehaley@microsoft.com>
tracyboehrer pushed a commit that referenced this pull request Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Automation: No parity PR does not need to be applied to other languages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error in AdaptiveDialog.ContinueActionAsync with native dialog SDK
3 participants