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

add allowloop to repeat dialog and test #3778

Merged
merged 4 commits into from
Apr 29, 2020

Conversation

xieofxie
Copy link
Contributor

Close #3742

Copy link
Contributor

@tomlm tomlm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕐

@tomlm tomlm added changes required Reviews has requested changes to be made before approval R9 Release 9 - May 15th, 2020 labels Apr 24, 2020
@luhan2017 luhan2017 removed the changes required Reviews has requested changes to be made before approval label Apr 27, 2020
@xieofxie
Copy link
Contributor Author

OK, it seems tests passed but no result for coverage..

@@ -58,10 +70,16 @@ public override async Task<DialogTurnResult> BeginDialogAsync(DialogContext dc,
var repeatedIds = dc.State.GetValue<List<string>>(TurnPath.RepeatedIds, () => new List<string>());
if (repeatedIds.Contains(targetDialogId))
{
throw new ArgumentException($"Recursive loop detected, {targetDialogId} cannot be repeated twice in one turn.");
if (this.AllowLoop == null || this.AllowLoop.GetValue(dc.State) == false)
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move this conidtion around the entire repeatedIds section, no need to track ids if you aren't using them.

Copy link
Contributor

@tomlm tomlm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R9 Release 9 - May 15th, 2020
Projects
None yet
Development

Successfully merging this pull request may close these issues.

enable action RepeatDialog to be able called more than once in one turn
3 participants