-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Explained the need to support database downgrading #6366
Conversation
/** | ||
* Downgrading an app is generally not officially supported without first uninstalling | ||
* the current version. However, it may be possible in some cases, depending on the app | ||
* and device. Rooted devices, for example, might allow more direct downgrades. |
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.
Thanks for looking into this! Seeing this, I'm actually leaning towards the oppositive and just exploding if we get a call to onDowngrade
. I think the idea that data gets cleared out might be more dangerous than just crashing and letting the user export the data if they still need it or at least understand that they're in a bad state. Would be good to get input from @lognaturel on this as well.
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.
Interesting let's see what @lognaturel thinks.
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.
Here's an error message that I think would be helpful to someone in this state: "Downgrading ODK Collect is not supported. If you need the forms and data that ODK Collect currently manages, you must upgrade to the version you used previously or a newer version. If you don't need the data, you can uninstall this version and then reinstall it again."
Does that sound right? If so, let's go with something like that. It doesn't need to be translated.
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.
As discussed on a call, we're going to just throw an exception with a message like "App downgrade not supported" if onDowngrade
is called.
b6587b9
to
2089bb2
Compare
This exception is thrown by default by throw new SQLiteException("Can't downgrade database from version " +
oldVersion + " to " + newVersion); I think it's enough we don't need to change it. |
2089bb2
to
aed1892
Compare
Closes #6315
Why is this the best possible solution? Were any other approaches considered?
We have discussed the issue and agreed that since downgrading is not officially supported we should stop handling it in our codebase.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
Downgrading isn't officially supported, so we don't typically test for it. I believe we can skip testing in this case as well. A code review by @seadowg.
Do we need any specific form for testing your changes? If so, please attach one.
No.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No.
Before submitting this PR, please make sure you have:
./gradlew connectedAndroidTest
(or./gradlew testLab
) and confirmed all checks still passDateFormatsTest