Fix: restore population of deprecatedID in edit mode #345
Conversation
This was a regression in enketo#321. `record` here is populated by `_checkAutoSavedRecord`, which is only available in offline mode. Since that will always be falsey (undefined) in edit mode, `submitted` would be set to false. Since we should only set `deprecatedID` when editing submissions, it made more sense to assign it directly based on the `isEditing` property introduced in enketo#269 (which should also have been added to the JSDoc type at that time).
| // Make sure that Enketo Core won't do the instanceID --> deprecatedID move | ||
| data.submitted = false; | ||
| } | ||
| data.submitted = Boolean( data.isEditing ); |
There was a problem hiding this comment.
An added benefit of this approach, in my opinion, is that submitted is now always explicitly set, so we don't need to rely on the implicit behavior in enketo-core when it's not set (which unexpectedly defaults to true).
|
As far as process, once this is reviewed, it will be merged to master then we will cherry pick and put it on a long-lived 3.0.x branch and tag it. |
|
Like you say, it seems this captures the intent much more clearly. It feels very safe but then again, so did #321. 😭 sources of risk I can think of:
I’ve reviewed the types of URLs carefully and those risks really do feel low. But my confidence is shaken. We need to do a point release. We can either do it with this and it will get some QA exercise but not a full regression pass or revert #321 for the point release and then do a more thorough QA pass with this. I’ve gone back and forth but seeing this PR, I’m inclined to merge it for the point release. |
|
@yanokwa I believe the release can be tagged on master. |
|
@lognaturel We've merged #340 to master and it hasn't gone through QA. |
|
Sigh, ok, sorry. |
This was a regression in #321:
recordhere is populated by_checkAutoSavedRecord, which is only available in offline mode. Since that will always be falsey (undefined) in edit mode,submittedwould be set to false. Since we should only setdeprecatedIDwhen editing submissions, it made more sense to assign it directly based on theisEditingproperty introduced in #269 (which should also have been added to the JSDoc type at that time).