Fix for navigation of saved form #2197
Conversation
| @@ -392,7 +392,7 @@ private void showNextView(boolean resuming) { | |||
| } | |||
|
|
|||
| if (activity.currentPromptIsQuestion()) { | |||
| if (!activity.saveAnswersForCurrentScreen(FormEntryConstants.EVALUATE_CONSTRAINTS)) { | |||
| if (!activity.saveAnswersForCurrentScreen(!activity.mFormController.isFormReadOnly())) { | |||
There was a problem hiding this comment.
What was the old flag set for? I would have guessed this would at the very least be changed to
FormEntryConstants.EVALUATE_CONSTRAINTS and !activity.mFormController.isFormReadOnly()
There was a problem hiding this comment.
@ctsims FormEntryConstants.EVALUATE_CONSTRAINTS is a constant that equates to true, so it doesn't really make a difference. Do you want to include it more for semantics ?
There was a problem hiding this comment.
FYI @ctsims
There was a problem hiding this comment.
@shubham1g5 I think the best way to say it is that either the FormEntryConstants.EVALUATE_CONSTRAINTS does something (in which case we should leave it in the code) or that it's antiquated and we don't use it anymore, in which case we should remove the definition of the value in addition to its use here. I'm not comfortable with only doing one.
There was a problem hiding this comment.
That makes sense! I don't see any particular use of them and thought they were there just for better semantics. Definitely pro removing them. cc: @ShivamPokhriyal
|
Hm, I think this is kind of prime-time for us to be responsive to the global community, so I'd support a hotfix. We should make sure to publicly respond to the forum note in addition to any tickets if/when the hotfix is released |
|
Makes sense. That will mean we need to do a hotfix 2.48.1 on top of 2.48 and instead of incrementally rolling out 2.48 (halt rollout), do the incremental rollout for 2.48.1. Alternatively we can let 2.48 fully rollout over next week and do a non-incremental 2.48.1 rollout afterwards. I think this is a better option since in both cases it will take us a week to propogate the update to all users. |
|
Any of those options sounds fine to me. If we think we can't quite make an update available quickly OTA, we could also consider just pointing them to a pre-release. I'm not sure what their project size/scope is, but given that we just cut a built, Master is probably very clean, so being on that 2.49 dev branch build is less crazy than it would be normally. The big downside would be that devices with that build installed wouldn't have access to other 2.48 hotfixes until the full 2.49 release., so it's not ideal, but if this is a small project it might be worth the trade. |
|
thanks! @ShivamPokhriyal Would you be able to open up a hotfix branch and create this PR against that. This page describes all you should need though do reach out in case something breaks. I have bumped the release % on 2.48 to 50 today and On Tuesday instead of bumping 2.48 to 100 we can just roll out the 2.48.1 hotfix to 100% of our users. |
In case of validation condition on a non-required question, navigation of submitted form gives constraint violation error for validation error
ShivamPokhriyal
force-pushed
the
fix-saved-form-navigation
branch
from
e2dff04
to
5b343d4
Compare
https://forum.dimagi.com/t/a-validation-condition-in-the-preview/7333
In case of validation condition on a non-required question, navigation of submitted form gives constraint violation error for validation error
Product Note: Fixes a bug that causes validation errors to appear while navigating saved forms.