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

fix: [#4325] Skip Storage properties from BotState class #4326

Merged
merged 4 commits into from
Sep 19, 2022

Conversation

sw-joelmut
Copy link
Collaborator

Fixes #4325

Description

This PR fixes an issue where the Host calls a Skill using the BeginSkill action, and right after ending the conversation with the skill a TextInput is prompt.
Moreover, after the input was prompted and provided the text response, the Host fails, because it tries to call the Skill again.
Furthermore, the Host was trying to call the Skill because the ConversationState was being updated with old information due to the BeginSkill class was saving into the Storage the ConversationIdFactory and ConversationState instances (that also have a Storage).
By skipping the two properties, like DotNet does, solved the issue.

Specific Changes

  • Added a functionality to skip properties into the BotState class, iterating the state object properties to find and skip the desired properties from outside the BotState scope, to then compare them in the hash functionality.
  • Added the skip properties functionality into the BeginSkill.beginDialog method.
  • Added two unit tests to address the BotState and BeginSkill changes.

Testing

The following image shows the comparison between the DotNet's Storage saved information and the one used in JavaScript.
imagen

@sw-joelmut sw-joelmut requested a review from a team as a code owner September 19, 2022 13:14
@coveralls
Copy link

coveralls commented Sep 19, 2022

Pull Request Test Coverage Report for Build 3082687836

  • 29 of 29 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 84.611%

Totals Coverage Status
Change from base Build 3045824559: 0.06%
Covered Lines: 19923
Relevant Lines: 22308

💛 - Coveralls

@tracyboehrer tracyboehrer merged commit 8bd9dba into main Sep 19, 2022
@tracyboehrer tracyboehrer deleted the southworks/add/botstate-skipproperties branch September 19, 2022 15:18
tracyboehrer pushed a commit that referenced this pull request Nov 17, 2023
* Add skip storage properties functionality to BotState

* Add unit tests for BotState skipProperties

* Fix test:compat

* test for linux
# Conflicts:
#	libraries/botbuilder-dialogs-adaptive-testing/tests/action.test.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants