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(web): empty album stored #9771

Merged
merged 1 commit into from
Jun 2, 2024

Conversation

remshams
Copy link
Contributor

@remshams remshams commented May 26, 2024

I was annoyed by #9657 so I fixed it on may local version.

The idea is to remove and empty album (without assets) when navigating back (clicking the close button) from the album create page instead of cleaning up when navigating to the albums page.
However I'm just starting to get familiar with the code, so I don't know if there are any side effects I'm not aware of...

The alternative would be to do the same cleanup when e.g. when adding a a photo to an album (in the "photos" section).
The advantage of doing it on closing is that it only needs to done once and it also avoids the flickering when opening the "albums" section (the empty album is visible just before it is deleted).

Please let me know if it is useful for you.

@@ -6,7 +6,7 @@ import type { PageLoad } from './$types';
export const load = (async ({ params }) => {
await authenticate();
const [album, asset] = await Promise.all([
getAlbumInfo({ id: params.albumId, withoutAssets: true }),
getAlbumInfo({ id: params.albumId, withoutAssets: false }),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is necessary as otherwise the albums with assets get deleted when clicking the back button.

Not sure if this is a performance problem/how expensive the query is in the backend.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is and it is quite expensive. Album info should have the asset count in it though.

Copy link
Contributor Author

@remshams remshams May 26, 2024

Choose a reason for hiding this comment

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

Works perfectly, no idea why I missed the property, sorry and thx.

@remshams remshams force-pushed the fix/9657/emtpy-album-stored branch 4 times, most recently from 58904f1 to feec0c4 Compare May 26, 2024 17:32
@alextran1502 alextran1502 changed the title Fix/9657/emtpy album stored fix(web): emtpy album stored May 26, 2024
@remshams remshams force-pushed the fix/9657/emtpy-album-stored branch 5 times, most recently from 6b826db to bf5ceaa Compare May 30, 2024 16:59
@danieldietzler danieldietzler changed the title fix(web): emtpy album stored fix(web): empty album stored May 31, 2024
@remshams remshams force-pushed the fix/9657/emtpy-album-stored branch 2 times, most recently from 4f56d46 to d90552a Compare June 2, 2024 11:59
@remshams remshams force-pushed the fix/9657/emtpy-album-stored branch from d90552a to e7a99bb Compare June 2, 2024 12:22
@alextran1502 alextran1502 merged commit e7dc1f7 into immich-app:main Jun 2, 2024
22 checks passed
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.

5 participants