Fix: loading forms in Firefox private browsing mode - #324
Conversation
There was a problem hiding this comment.
Verified:
- test cases cover every attempt at accessing the store in the context of last-saved
- only applies in online mode (since offline mode requires store for offline things)
To confirm -- offline mode in Firefox private mode always has and continues to fail with a cryptic error message?
| * @param {IsLastSaveEnabledOptions} [options] | ||
| */ | ||
| export const isLastSaveEnabled = ( survey ) => { | ||
| export const isLastSaveEnabled = ( survey, options = { checkStoreAvailability: true } ) => { |
There was a problem hiding this comment.
This has now changed from "does the form specify that last-saved is enabled" to "Enketo should use last-saved functionality". I think that's ok and the naming is still accurate enough but did want to flag that the change in semantics gave me pause.
There was a problem hiding this comment.
I'm honestly not thrilled about this signature change, this was mostly a pragmatic way to a quick fix. How would you feel about breaking this into two functions?
hasLastSavedInstance: the previousisLastSavedEnabledbehaviorisLastSavedEnabled:store.available && hasLastSavedInstance(survey)
There was a problem hiding this comment.
Hm, now that I look at it, I think the encryption check should be in isLastSavedEnabled rather than hasLastSavedInstance as well. It doesn't make sense for an encrypted survey to reference a last-saved instance, but in the event it does that presumably should also be populated with the model defaults.
There was a problem hiding this comment.
Not a huge deal but that latest suggestion does seem nicer to me!
|
@seadowg I see you're also lined up to review. I'm pretty confident that the approach is reasonable but happy for you to take a look if you want to make sure! |
|
@lognaturel don't want to hold this up, and I might not get a chance to review before the end of my day today. I'll remove myself as a reviewer. |
|
@eyelidlessness Remember that this PR needs to be QAed, so let's merge it into a new QA branch when you are ready. |
|
@yanokwa so you want a branch on mainline in order for an image to be built, right? If so, might I suggest @eyelidlessness simply push his branch to upstream? Then this PR can be merged to master once it's validated. |
|
Ordinarily, that'd work, but in this case we want to QA all the other changes that are coming in from transformer. I supposed we could add them to this PR, but seems do stuff in a QA branch. |
Also tests, and ensures, that last-saved instances are populated with model defaults on encrypted surveys
cb86018 to
127b099
Compare
4 participants
Fixes enketo/enketo#1050. In online mode, IndexedDB initialization failure is ignored. In the event of failure, references to
last-savedfall back to the form's model, the same way it would if IndexedDB was available but no last-saved record were stored.