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

fix(mobile): fix forgetting backup albums (#3108) #3244

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

madadam
Copy link
Contributor

@madadam madadam commented Jul 13, 2023

Fix/workaround for #3108. It seems the bug is caused by a weird behaviour / bug in photo_manager: Sometimes AssetPathEntity.fromId throws even when called with a valid asset id (say one returned from a previous call to PhotoManager.getAssetPathList).

This PR works around that by looking the assets up in a previously obtained map instead of calling AssetPathEntity.fromId.

How has this been tested

Manually, by deploying the patched app to my phone with flutter run and then going through the repro steps.

@vercel
Copy link

vercel bot commented Jul 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
immich ⬜️ Ignored (Inspect) Jul 13, 2023 8:24pm

@alextran1502
Copy link
Contributor

Can you add a few lines to the PR description on how this change has been tested

@madadam
Copy link
Contributor Author

madadam commented Jul 14, 2023

Can you add a few lines to the PR description on how this change has been tested

Added. Is it sufficient?

Copy link
Contributor

@fyfrey fyfrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me!

@alextran1502 alextran1502 enabled auto-merge (squash) July 17, 2023 03:05
@alextran1502 alextran1502 merged commit e287b18 into immich-app:main Jul 17, 2023
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.

4 participants