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.

Sign up for GitHub

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

Jump to bottom

Adaptive ForEachElement does not exit when child action CancelAllDialogs is called #6430

Open
Rich-biomni opened this issue Aug 4, 2022 · 9 comments · Fixed by #6452
Open

Adaptive ForEachElement does not exit when child action CancelAllDialogs is called #6430

Rich-biomni opened this issue Aug 4, 2022 · 9 comments · Fixed by #6452
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository. ExemptFromDailyDRIReport Use this label to exclude the issue from the DRI report.

Comments

@Rich-biomni
Copy link

Version 4.16.1

Describe the bug

When a ForEachElement Child Action is CancelAllDialogs the ForEachElement Loops again rather than cancelling
This is also the case when there are nested ForEachElement actions aand at the lowest point a CancelAllDialogs is ran

To Reproduce

Create and adaptive dalog
Create a ForEachElement
In the action add a message, question, CancelAllDialogs, message
Run the dialog

--
Create and adaptive dalog
Create a ForEachElement with a sub foreachElement
In the action add a message, question, CancelAllDialogs, message
Run the dialog

Expected behavior

All foreachElement should cancel but instead it loops

Additional context

Add Zip with repoduction

SimpleBot.zip
@Rich-biomni Rich-biomni added bug Indicates an unexpected problem or an unintended behavior. needs-triage The issue has just been created and it has not been reviewed by the team. labels Aug 4, 2022
@stevkan stevkan added customer-reported Issue is created by anyone that is not a collaborator in the repository. Bot Services Required for internal Azure reporting. Do not delete. Do not change color. labels Aug 4, 2022
@stevkan
Copy link
Collaborator

stevkan commented Aug 4, 2022

Related to #6428

@stevkan stevkan removed the needs-triage The issue has just been created and it has not been reviewed by the team. label Aug 4, 2022
@stevkan stevkan self-assigned this Aug 4, 2022
@Rich-biomni Rich-biomni changed the title Adaptive ForEachElement does not exist when child action CancelAllDialogs is called Adaptive ForEachElement does not exit when child action CancelAllDialogs is called Aug 5, 2022
@Rich-biomni
Copy link
Author

Adding the sample bot again, since on a different bug it was mentioned, the sample could not be run / reproduced because of an out dated dependenc
SimpleBot.zip

@stevkan
Copy link
Collaborator

stevkan commented Aug 9, 2022

@Rich-biomni - I was able to repro the issue using your sample. Unfortunately, I was unable to find a simple solution / workaround to help mitigate this for you.

Escalating to @ceciliaavila for continued support on this.

@stevkan stevkan assigned ceciliaavila and unassigned stevkan Aug 9, 2022
@stevkan stevkan added the customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. label Aug 9, 2022
@tracyboehrer
Copy link
Member

Is this repeatable in 4.17.1?

@Rich-biomni
Copy link
Author

@tracyboehrer

Issue exists in 4.17.1 as well

@ceciliaavila ceciliaavila mentioned this issue Aug 25, 2022
Merged
tracyboehrer pushed a commit that referenced this issue Aug 29, 2022
@ceciliaavila
[#6430] Adaptive ForEachElement does not exit when child action Cance…
c572be3 
…lAllDialogs is called (#6452)

* Consider canceled status to end the dialog

* Add unit test
tracyboehrer added a commit that referenced this issue Sep 1, 2022
@tracyboehrer @ceciliaavila
@sw-joelmut @swagatmishra2007 @dependabot
4.1.7.2 Patch Release (#6450)
ec7a879 
* 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 issue Sep 7, 2022
Revert "[#6430] Adaptive ForEachElement does not exit when child acti…
eae7c1b 
…on CancelAllDialogs is called (#6452)"

This reverts commit c572be3.
tracyboehrer pushed a commit that referenced this issue Sep 7, 2022
Revert "[#6430] Adaptive ForEachElement does not exit when child acti…
07c8e34 
…on CancelAllDialogs is called (#6452)"

This reverts commit c572be3.
tracyboehrer added a commit that referenced this issue Sep 7, 2022
@tracyboehrer
Revert "[#6430] Adaptive ForEachElement does not exit when child acti…
9b2b6d6 
…on CancelAllDialogs is called (#6452)" (#6466)

This reverts commit c572be3.

Co-authored-by: Tracy Boehrer <trboehre@microsoft.com>
tracyboehrer added a commit that referenced this issue Sep 7, 2022
@tracyboehrer
Revert "[#6430] Adaptive ForEachElement does not exit when child acti…
0966055 
…on CancelAllDialogs is called (#6452)" (#6466)

This reverts commit c572be3.

Co-authored-by: Tracy Boehrer <trboehre@microsoft.com>
tracyboehrer added a commit that referenced this issue Sep 7, 2022
@tracyboehrer @sw-joelmut
4.17.2 Patch Update - Reversion of breaking change (#6467)
8307e64 
* Revert "[#6430] Adaptive ForEachElement does not exit when child action CancelAllDialogs is called (#6452)" (#6466)

This reverts commit c572be3.

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

* Add cancellation token to TeamsOperation class (#6458)

* Fix typing indicator shown after bot has replied (#6460)

Co-authored-by: Tracy Boehrer <trboehre@microsoft.com>
Co-authored-by: Joel Mut <62260472+sw-joelmut@users.noreply.github.com>
@ceciliaavila ceciliaavila mentioned this issue Sep 8, 2022
Closed
@tracyboehrer
Copy link
Member

This PR for this was reverted due to a breaking change. We will need to revisit.

Notes for SDK: This breaks Bot Designers (PVA) "SlotFillingTest" and some "IntentSwitchingTests". We will need to extract the tests to reproduce outside PVA.

@tracyboehrer tracyboehrer reopened this Sep 8, 2022
@ceciliaavila
Copy link
Collaborator

We need information on the broken PVA tests to reproduce on the SDK side.

@johnataylor johnataylor added the ExemptFromDailyDRIReport Use this label to exclude the issue from the DRI report. label Oct 6, 2022
@johnataylor
Copy link
Member

we have a PR so marking this as exempt from the daily DRI report

@tracyboehrer
Copy link
Member

Actually... this is still outstanding. See the note above about the PR being reverted until we can duplicate outside of PVA scenario tests in SDK. Still fine to exempt for now though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository. ExemptFromDailyDRIReport Use this label to exclude the issue from the DRI report.
Projects
None yet
Milestone
No milestone
6 participants
@JuanAr @tracyboehrer @johnataylor @stevkan @Rich-biomni @ceciliaavila