Replaces bundle usage in the editor with a local database#19747
Replaces bundle usage in the editor with a local database#19747
Conversation
|
| App Name | Jetpack |
|
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr19747-4e38515 | |
| Commit | 4e38515 | |
| Direct Download | jetpack-prototype-build-pr19747-4e38515.apk |
|
| App Name | WordPress |
|
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr19747-4e38515 | |
| Commit | 4e38515 | |
| Direct Download | wordpress-prototype-build-pr19747-4e38515.apk |
|
Thank you for putting the |
Thank you for testing and your feedback @justtwago 🙇 I also experimented with removing views from the Bundle. Since you are able to reproduce this consistently could you give the patch below a try? 🙇 |
Yeah, totally makes sense. I think that it's too extreme case that even crashes the app when I'm trying to put everything to a clipboard. 😄 I think that we can proceed with putting a revision to a local DB and monitoring the results on Sentry.
Thank you for the idea! I tried it out, and it looks like the app doesn't crash 😄 Anyway, I'm not sure that it's something that we want to have in the app since when the app destroys the activity in the background we can't retrieve the views when are back to the app so it causes a crash (it happens for me once but I couldn't for this and reproduce it later to catch a stack trace.) |
|
One more question is about putting/extracting a revision to/from DB: GitHub doesn't allow me to comment this block of code 😄 |
Great suggestion @justtwago 👍 I added the rest of the revision instances in the db with 3b61f82 |
This makes sense to me too @justtwago 👍 I prepared this PR for review to at least cover the data we add to the Bundle. |
Generated by 🚫 dangerJS |
Generated by 🚫 Danger |
justtwago
left a comment
There was a problem hiding this comment.
Antonis, thank you for the amazing work on the fix!
I smoke-tested the editor and also used the Don't keep activities flag, and the DB seems to work perfectly. Fingers crossed for seeing some progress in Sentry after releasing it.
I left some minor comments for cosmetics stuff, but I'd merge it today to include the fix in the nearest release.
In addition, we could consider extracting the persistence logic to a separate gradle module in the future.
libs/editor/src/main/java/org/wordpress/android/editor/savedinstance/SavedInstanceDatabase.kt
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/history/HistoryDetailFragment.kt
Show resolved
Hide resolved
|
Thank you for testing extensively and your feedback @justtwago 🙇
That indeed a good idea. I was on the edge of adding this but thought that it would extend the scope of the change and preferred to use the |
Yeah, totally agree that it's out of this PR's scope. 💯 |


Partially #9685 and #19157
Description
This PR tries to mitigate the
TransactionTooLargeExceptionerror in the editor by replacingBundleforParcelableobjects with a custom database implementation.To Test:
Regression Notes
Potential unintended areas of impact
Editor
What I did to test those areas of impact (or what existing automated tests I relied on)
Manual testing
What automated tests I added (or what prevented me from doing so)
Added tests for the added local db
PR Submission Checklist:
RELEASE-NOTES.txtif necessary.UI Changes Testing Checklist: