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

ATO 207 run form validation on form activation #11326

Merged
merged 7 commits into from
Jul 18, 2022

Conversation

indam23
Copy link
Contributor

@indam23 indam23 commented Jul 13, 2022

Proposed changes:

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@indam23 indam23 changed the base branch from main to 3.2.x July 13, 2022 10:53
@indam23 indam23 requested a review from losterloh July 13, 2022 10:57
@indam23 indam23 changed the title ATO 207 form validation activation ATO 207 run form validation on form activation Jul 13, 2022
@indam23 indam23 marked this pull request as ready for review July 13, 2022 13:08
@indam23 indam23 requested a review from a team as a code owner July 13, 2022 13:08
@indam23 indam23 removed the request for review from a team July 14, 2022 06:55
Copy link
Member

@ancalita ancalita left a comment

Choose a reason for hiding this comment

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

Looks good, an additional q I have is to double-check there was no need to add this check back into _get_slot_extractions for loop which was removed in PR 10295?

if not tracker.active_loop:
  break

tests/core/actions/test_forms.py Show resolved Hide resolved
tests/core/actions/test_forms.py Show resolved Hide resolved
tests/core/actions/test_forms.py Outdated Show resolved Hide resolved
@indam23
Copy link
Contributor Author

indam23 commented Jul 14, 2022

double-check there was no need to add this check back into _get_slot_extractions for loop which was removed in PR 10295?

Looked into this, but it doesn't seem to make any difference whether the check is there or not; all forms tests pass either way 🤔

@indam23 indam23 requested a review from ancalita July 14, 2022 10:40
Copy link
Member

@ancalita ancalita left a comment

Choose a reason for hiding this comment

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

Whoop whoop 🎉
Left one tiny comment to make sure we have that wondering assert back into test_activate too, otherwise good to merge.

tests/core/actions/test_forms.py Show resolved Hide resolved
Copy link
Contributor

@losterloh losterloh left a comment

Choose a reason for hiding this comment

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

🚀🚀🚀

@github-actions
Copy link
Contributor

🚀 A preview of the docs have been deployed at the following URL: https://11326--rasahq-docs-rasa-v2.netlify.app/docs/rasa

@XiaofeiQian
Copy link
Contributor

Can we merge and release this soon? thanks.

@indam23 indam23 merged commit cfc687c into 3.2.x Jul 18, 2022
@indam23 indam23 deleted the ATO-207-form-validation-action-first-run branch July 18, 2022 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

required_slots function in form is not called
4 participants