-
Notifications
You must be signed in to change notification settings - Fork 26
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
🐛 [#4579] Ensure triggerFromStep uses the correct current_step #4967
🐛 [#4579] Ensure triggerFromStep uses the correct current_step #4967
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4967 +/- ##
=======================================
Coverage 96.62% 96.62%
=======================================
Files 760 760
Lines 25838 25838
Branches 3386 3386
=======================================
Hits 24965 24965
Misses 608 608
Partials 265 265 ☔ View full report in Codecov by Sentry. |
5bbd7ea
to
b837307
Compare
previously the current step was inferred by looking at the last completed step and considering the step after that the current step. This does not work however if you complete a step and go back to the previous step, so instead we explicitly pass the current form step via the serializer context
51886a3
to
e7c4ef3
Compare
@@ -198,7 +200,7 @@ def check_submission_logic( | |||
if not submission_state.form_steps: | |||
return | |||
|
|||
rules = get_rules_to_evaluate(submission) | |||
rules = get_rules_to_evaluate(submission, current_step) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was surprised (and suspicious) about this fix, is there a reason why previously the current_step
was inferred for this flow, instead of explicitly passed to get_rules_to_evaluate
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function exists because the logic for the submission as a whole still needs to be evaluated during registration/processing after it's completed. In those contexts, there isn't a submission step, and it's also not relevant for the "trigger_from_step", because the assumption is that this only gets called for completed submissions that have gone through all the steps anyway.
Clearly this was a wrong assumption - as this does get called in the logic check serialization too which is in the context of a particular step. Nice find!
previously the current step was inferred by looking at the last completed step and considering the step after that the current step. This does not work however if you complete a step and go back to the previous step, so instead we explicitly pass the current form step via the serializer context Backport-of: #4967
previously the current step was inferred by looking at the last completed step and considering the step after that the current step. This does not work however if you complete a step and go back to the previous step, so instead we explicitly pass the current form step via the serializer context Backport-of: #4967
previously the current step was inferred by looking at the last completed step and considering the step after that the current step. This does not work however if you complete a step and go back to the previous step, so instead we explicitly pass the current form step via the serializer context
Closes #4579
Changes
Checklist
Check off the items that are completed or not relevant.
Impact on features
Release management
I have updated the translations assets (you do NOT need to provide translations)
./bin/makemessages_js.sh
./bin/compilemessages_js.sh
Dockerfile/scripts
./bin
folderCommit hygiene