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
refactor(server): extract add/remove assets logic to utility function #8329
Conversation
20f6a22
to
80b368d
Compare
Deploying immich with Cloudflare Pages
|
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 really nice! Thanks!
ed11f3f
to
5378181
Compare
5378181
to
1f40862
Compare
|
||
const removedIds = results.filter(({ success }) => success).map(({ id }) => id); | ||
if (removedIds.length > 0) { | ||
await this.albumRepository.removeAssets(id, removedIds); |
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.
Same here
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 don't think it's possible here since we need to answer two questions: (1) Did any ids actually get removed? (2) Was the thumbnail asset id part of those removed ids?
server/src/utils/asset.util.ts
Outdated
|
||
const newAssetIds = results.filter(({ success }) => success).map(({ id }) => id); | ||
if (newAssetIds.length > 0) { | ||
await repository.addAssetIds(dto.id, dto.assetIds); |
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.
Is this a bug? Surely it should be the new asset IDs.
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.
Ooh, great catch
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.
Dang you're good
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.
LGTM!
fix tests chore: generate sql foo
1f40862
to
e0d0dc3
Compare
No description provided.