immich icon indicating copy to clipboard operation
immich copied to clipboard

Published 20 hours ago • verified publisher icon immich-app

refactor(mobile): db migration utils

Open shenlong-tanwen opened this issue 1 year ago • 7 comments

Changes made:

  • Moved migration and db related utils to a common DBUtils class

shenlong-tanwen avatar Feb 11 '24 01:02 shenlong-tanwen

Deploying immich with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6083045
Status: ✅  Deploy successful!
Preview URL: https://ef31cc75.immich.pages.dev
Branch Preview URL: https://refactor-mobile-clear-backup.immich.pages.dev

View logs

cloudflare-workers-and-pages[bot] avatar Feb 11 '24 01:02 cloudflare-workers-and-pages[bot]

So if a user is logout, the backup setting is clear, which means they will have to reselect them again, is this correct?

alextran1502 avatar Feb 18 '24 03:02 alextran1502

So if a user is logout, the backup setting is clear, which means they will have to reselect them again, is this correct?

Yes. If you have several instances / several users using the same device, using the same backup settings might be a problem since if background backup / foreground backup is enabled and albums are already selected for backup, assets will start backing up even after logging out and in with a different account.

shenlong-tanwen avatar Feb 18 '24 12:02 shenlong-tanwen

I think most users don't use multiple instances, so this would be an inconvenience for most people.

If we want to solve the root cause, the proper way is to save these settings under userId and the instance URL.

alextran1502 avatar Feb 21 '24 17:02 alextran1502

I think most users don't use multiple instances, so this would be an inconvenience for most people.

True, but it still might make sense to remove the albums selected for backup on logout. If we do not want that, I can update this PR and change it to a refactoring one instead where we group all migration related code inside DBUtils

shenlong-tanwen avatar Feb 22 '24 18:02 shenlong-tanwen

I think most users don't use multiple instances, so this would be an inconvenience for most people.

Reverted the backup settings clearing logic for now. We can add it back later using the instance URL + userID based handling as you've suggested in the future

shenlong-tanwen avatar Mar 02 '24 23:03 shenlong-tanwen

@alextran1502 I have merged in main, fixed up some linting issues and tested that the app works as expected. I tested that on logout the user backup preferences persist to the next login too. Imo this is good to go now

zackpollard avatar May 13 '24 11:05 zackpollard