Skip to content
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

Don't include session in backup/transfer #2747

Merged
merged 1 commit into from Aug 3, 2022

Conversation

jpelgrom
Copy link
Member

@jpelgrom jpelgrom commented Aug 2, 2022

Summary

A user on Discord mentioned that they were automatically logged in on a new device which surprised them. Session data is device specific and shouldn't be restored or transferred, so this PR excludes session data from the automatic backup/transfer.

Not including the session data will also show onboarding on start, which prevents issues with re-using the mobile app integration registration.

Screenshots

n/a

Link to pull request in Documentation repository

n/a

Any other notes

 - The session data is unique to the device and shouldn't be included in a backup or transfer
 - Add Android 12+ backup rules for Wear OS
@dshokouhi
Copy link
Member

Should we just disable auto backup? It seems to cause more headaches than anything.

@jpelgrom
Copy link
Member Author

jpelgrom commented Aug 2, 2022

Should we just disable auto backup? It seems to cause more headaches than anything.

Backups are nice to have though, and it's not impossible to make them work.

If we decide to remove auto backup, that also closes feature request #2462? It doesn't make sense to me to only support manual backup.

@dshokouhi
Copy link
Member

Should we just disable auto backup? It seems to cause more headaches than anything.

Backups are nice to have though, and it's not impossible to make them work.

If we decide to remove auto backup, that also closes feature request #2462? It doesn't make sense to me to only support manual backup.

So for me the only time I find it works well the way it is today is if I upgraded my device and stopped using my old phone immediately. In those cases the app starts up all data is there and I don't even have to login or enable additional sensors. I don't see that as a bad thing like the user in discord did. It becomes an issue when a user does not factory reset old phone and continues to open and use the app on the old phone. Now with that said things like widgets may not function well and I am pretty sure need to be recreated as the ID's change during a device transfer. I also don't think app shortcuts get added back either.

@jpelgrom
Copy link
Member Author

jpelgrom commented Aug 2, 2022

We can't dictate what users do after restoring a backup re. the 'old' session, so we have to support all scenarios. Keeping your settings / sensors is good to have! Why remove those advantages by completely disabling backup if it also can be solved by asking users to log in again?

Now with that said things like widgets may not function well and I am pretty sure need to be recreated as the ID's change during a device transfer.

Last time I checked IDs will be migrated over to a new device.

@dshokouhi
Copy link
Member

We can't dictate what users do after restoring a backup re. the 'old' session, so we have to support all scenarios. Keeping your settings / sensors is good to have! Why remove those advantages by completely disabling backup if it also can be solved by asking users to log in again?

Well when auto backup does its restore its more obvious that it restored by the user not having to login. If we remove the session then the user still needs to complete onboarding right? At that point how does a user know that everything else was restored? Do we show the old device name and check off location tracking to show we still have that data during onboarding? It becomes a bit more hidden that there was a restore at this point.

@jpelgrom
Copy link
Member Author

jpelgrom commented Aug 2, 2022

If we remove the session then the user still needs to complete onboarding right? (...) Do we show the old device name and check off location tracking to show we still have that data during onboarding?

Yes the user would need to complete onboarding again, and it will look just like a fresh install during that process. It currently does not show stored data.

It becomes a bit more hidden that there was a restore at this point.

Is that a bad thing?

As the feature request I linked to indicates, there is a demand to be able to restore settings. The current approach tries to keep everything which doesn't work in all scenarios, and completely removing backups feels like overreacting.

@dshokouhi
Copy link
Member

It becomes a bit more hidden that there was a restore at this point.

Is that a bad thing?

not until another user complains lol

As the feature request I linked to indicates, there is a demand to be able to restore settings. The current approach tries to keep everything which doesn't work in all scenarios

The current request is actually more a bug report as they say its not working at all but this PR contradicts that. Then later on they request for manual import and export but that is actually not related to android auto backup because we can't manually force it to upload. Auto backup has its own schedule and constraints. I honestly took that request as the app having its own thing for users to export and save elsewhere to restore.

@jpelgrom
Copy link
Member Author

jpelgrom commented Aug 2, 2022

The current request is actually more a bug report as they say its not working at all but this PR contradicts that.

While I don't know why the auto backup didn't work for them, I can personally confirm it works (it restores old data for the debug app frequently). The fact that they made a bug report still indicates the demand.

Then later on they request for manual import and export but that is actually not related to android auto backup (...)

Yes it is a bug report turned feature request, and even if it's not strictly related if we can't support the auto backup I don't see why a manual backup would be much different - it's the same settings!

@dshokouhi
Copy link
Member

While I don't know why the auto backup didn't work for them, I can personally confirm it works (it restores old data for the debug app frequently). The fact that they made a bug report still indicates the demand.

yup same here it works for me too. Getting logs from something like that will be difficult too, not sure how to proceed there.

@JBassett JBassett merged commit bddb3a6 into home-assistant:master Aug 3, 2022
@jpelgrom jpelgrom deleted the fix-session-in-backup branch August 4, 2022 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants