-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat: user profile restore #2671
Conversation
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, and I was able to complete the same user flow as in your demo.
I can see how there is further discussion and work needed to expose the functionality to templating. But as I understand it, if we include a sign-in option currently, then add the ability to restore from the server based on auth ID later, then the feature should be available for users who have signed in as soon as they update to the latest app version
|
||
const currentUserId = this.localStorageService.getProtected("APP_USER_ID"); | ||
const restoreProfiles = authEntries | ||
.filter((v) => v.app_user_id !== currentUserId) |
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.
I can see how there might be a use case for restoring from the same ID (e.g. after factory resetting a single device). But it makes sense to focus on the requested use case for now
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.
I'm not sure how consistent this would be as some android devices generate per-installation IDs, some per-device and even some per-app (persists re-install), but agree it could be nice to enable users to restore their profile on the same device if they had to uninstall/reinstall the app, or clear cache data for any reason.
Although if enabling this would probably need to also keep better track of timestamps when synced to know whether the server version is the same as current or not.
I'll add a specific issue for follow-up as required: #2684
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
PR Checklist
Description
Author Notes
This PR adds the core methods and updates the user debug screen for testing purposes. Follow-up discussion and work will be required to determine how best to expose custom templating to manage user profile restoration
The user debug page can be found at
/user
Review Notes
The required server updates have been deployed so all should work when running locally from this branch. See demo below for example using 2 devices and restoring profile from first onto second.
Dev Notes
One additional knock-on was how best to handle protected fields, which are by default not imported/restored. I've opted for manually specifying a list of fields which we do want to restore (e.g. app theme, skin etc.), however in the future we may want a better solution. Also would be good to check the list to see if it makes sense.
Git Issues
This addresses the core of #2459, however does not close all the related-issues mentioned (not sure whether to close or not...)
Screenshots/Videos
Demo - Profile Restore
Uses one regular browser tab and one incognito to mimic multiple devices. Initial device is used with app, logged in and synced. Second device logs in and imports data to continue from where first device left off
Note 1: There is an additional profile for restore from previous testing
Note 2: Ignore the infinite mirrors. Had to use full desktop capture with OBS to show both windows and running on a single monitor :'(
2025-01-02.21-11-10.mp4