-
-
Notifications
You must be signed in to change notification settings - Fork 45
Return to form entry after session expiration via SSD #1883
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
Conversation
… session expiration
|
Product Note: This will make it so that when form entry is interrupted by the automatic user session expiration, the user will be entered directly back into that form when they log back in, rather than having to figure out what happened and navigate to re-opening the saved form themselves. |
…he form at first; the problem is that onResume() does actually get called even if onCreate launches a new activity
|
@damagatchi retest this please |
|
@ctsims Ok I added the notion of wiping flags that shouldn't get attempted later if they don't happen on first login. Implementing this made me realize that the post-update sync flag was another good candidate for this paradigm, and accordingly belonged more in |
wpride
left a comment
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.
Looks good to me; unfortunately looks like another migration 22 has been merged :/
ctsims
left a comment
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.
Important change in the update class.
Other than that this looks good. One quick note that I still think there could be an alternative where instead of using the SSD to store the interrupted state, we could store, say, "interrupted_ssd_id" in shared preferences, since we only anticipate having one of those in-play at a time.
| SessionStateDescriptor.class, | ||
| new ConcreteAndroidDbHelper(c, db)); | ||
|
|
||
| for (SessionStateDescriptorV1 ssd : oldStorage) { |
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.
Important note: Within this file. all SSD references before this migration occurs (which is all of them for now) need to be on SSDV1, otherwise a Db update from, say, v5 to v22 could try to load the SSD it's manipulating as an SSDV2 and crash.
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.
@ctsims Very good catch, thank you.
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.
..Ah, I just realized your other suggestion makes this irrelevant. I think that is a better way to go so I'll do that refactor now
|
ok @ctsims went with your suggestion of storing the interrupted SSD's id in prefs instead of using the flag. Definitely simpler, and also has the advantage of allowing us to be more precise about which SSD we intend to retrieve |
|
@damagatchi retest this please |
ctsims
left a comment
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.
couple of quick changes.
| , "org.javarosa.core.model.actions.SendAction" | ||
|
|
||
| // Added in 2.41 | ||
| , "org.commcare.android.database.user.models.SessionStateDescriptorV1" |
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 no longer exists and is causing a test failure now
| } | ||
| SqlStorage<SessionStateDescriptor> sessionStorage = | ||
| CommCareApplication.instance().getUserStorage(SessionStateDescriptor.class); | ||
| SessionStateDescriptor interrupted = sessionStorage.read(idOfInterrupted); |
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.
Two quick things:
- I don't know precisely why this could fail after fixing Added support to media files validation which are not url encoded. #2 below (no record exists), but I think that if this record isn't in session storage we shouldn't fail.
- Shared preferences are per-app, not per-user, so right now this can bleed across userdbs (which could cause Automatic GPS capture #1, but could also just cause bizarre behavior). Easiest fix is to append the user ID to the shared preferences key in the HiddenPreferences changes
Addresses https://manage.dimagi.com/default.asp?247211 by using the stored
SessionStateDescriptorto re-load an incomplete form after it is interrupted by session expiration.@ctsims: This is a slightly different way of doing this than we discussed, but I think it's much cleaner and more reliable. When I started trying to utilize the session-saving infrastructure for this, the workflows I had to use felt both messy and brittle. I then noticed the paradigm that was being used in
AndroidSessionWrapper.getExistingIncompleteCaseDescriptor()to detect when the session for which the user is trying to load a form already has an incomplete form in storage, and realized that a similar strategy could be used here. The strategy I adopted does a check upon login for a storedSessionStateDescriptorthat has been marked as "interrupted" by a session expiration, and re-loads it and its correspondingFormRecordif so.1 question I have: By doing it this way, the default result is that the user gets dumped into the
FormHierarchyActivityfor their saved form, rather than directly into form entry. It is easy enough to change that if that seems undesirable, but I didn't have a strong opinion either way. Let me know if you do.