-
Notifications
You must be signed in to change notification settings - Fork 497
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
Handle RCS messages properly, and improve group MMS #1097
base: master
Are you sure you want to change the base?
Conversation
…droid Studio (everything else)
…, plus the subject.
Sometimes you would see both of these subjects in gmail: 123-123-1234/Dad 123-123-1234/Dad/Dad
…reads, due to grabbing the contact ID from a random person in each thread
@locofocos Thank you so much for doing this; it will close a large number of outstanding tickets. I will review in detail shortly. (It will still be up to @jberkel to publish to Google Play.) |
I'm really happy with the separation of Again, many thanks. |
// Tip - if you want to try different strategies for computing mmsThreadId against the same messages, | ||
// make sure to generate a new MESSAGE_ID as well. It seems like gmail caches REFERENCES if you reuse the same MESSAGE_ID. |
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.
Some context, from what I learned doing this: MESSAGE_ID is the unique message on gmail's end. REFERENCES is the field (on gmail's end) that determines which email an email is replying to.
For the last message grouping fix I built, I wasn't seeing it take effect, even after I deleted all my sms backups from gmail and ran the back again. I was able to determine that gmail was caching the REFERENCES field by clicking "show original" in gmail and looking at the message header. I had to force the app to generate new MESSAGE_IDs. Basically there's some string we pass into the hash function that generates MESSAGE_ID. You can just concatenate "v2", "v3", etc. to the hash input. It's almost like bumping a version number. Once I generated a new MESSAGE_ID for those messages I had already backed up before, then my last fix started working.
However, this has repercussions for releasing this fix. I tried backing up all the messages on my phone with a clean build of this current PR. I'm using a different gmail label (SMS-testing
instead of the usual SMS
) to keep things clean, but the app was still generating the same MESSAGE_ID as before. So when I checked on some older (already backed up) messages, they weren't grouped correctly 😢
I can vet this out, but I believe if we version/change the hash input (like we start concatenating "v2"), that will force gmail to recognize these as new messages. So then users will be able to reset the app state and re-backup their existing messages, and then older threads should get fixed 🎉 I expect the old messages would linger around in gmail as well, so there may some duplicates... but users will only get this if they choose to re-backup their existing messages.
This will also be a useful opportunity for me to check against more test messages. I'll check this out some day soon!
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.
Good news! Versioning the hash input ab80cc0 fixes this problem nicely. This will be helpful to force gmail to recognize these messages as new threads and group them correctly.
@kurahaupo you're welcome! Thanks for the initial review. I'll get this cleaned up and tested further some time soon, I hope. |
…MS (some messages have images)
…s not broken into 2 threads
…is worked before, but now avoids a warning log
@kurahaupo I've given this some solid testing on my phone, and I'm happy with the result. Is the travis CI build running against this PR? I was looking for the build output so I could at least see which tests I need to update. edit - I might need some some more time to look over the results and debug more. All of those test cases look good. But I did a another huge backup last night, and I'm seeing some ungrouped messages. |
I'll check on the CI when I get home to my PC |
Hi |
@hubono I debated whether I should post a copy of my latest APK build, just in case life happens and we don't get this PR merged. Caveats, if this wasn't obvious:
However, I've been using it for months. No promises, but I'm happy with the results. So here is the APK I built locally from ac71d55 |
Fixes #1055 and #1027
This fixes RCS importing 🎉
It also improves how group MMS threads are imported. Basically it includes all the recipients in the "to" field, and puts everyone's name in the subject, so that gmail does a much better job mimicking the group threads on your phone.
I've been toying around with it on my Pixel 3a running Android 10. I've tested with a mix of SMS, group MMS chats, and RCS chats.
I'm completely open to the idea that I was a little heavy-handed with my refactor. I ran with what made sense. I'm fine if someone else wants to take this as as proof-of-concept and run in a slightly different direction to make this mergeable.
TODO
Testing
Tips for testing:
app/src/main/java/com/zegoggles/smssync/service/BackupQueryBuilder.java
. https://www.epochconverter.com/ is your friend here.Some test cases that I've gone through on my phone, and verified that the backup looks correct in Gmail:
BackupItemsFetcher.getItemsForDataType
andConsts.MMS_PROVIDER
/SMS_PROVIDER
. So we would have to build some intermediate caching mechanism that gathers all the SMSs, gathers all the MMSs, interleaves them, then feeds them into the existing logic.Adopting this new version
Here is my 2 cents for how affected users could get their backups into a clean-ish state:
old-backups
. Download the new app version. Reset the backup state and backup everything again (to your favorite label, likeSMS
). For whatever time period you still have on your phone, delete emails from that time period with labelold-backups
. Verify that you have a nice continuity when looking at bothold-backups
andSMS
. Then change all theold-backups
emails over to theSMS
label.