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: include live images in person view count #16116

Merged
merged 4 commits into from
Feb 17, 2025

Conversation

immangat
Copy link
Contributor

@immangat immangat commented Feb 15, 2025

Description

This PR fixes an issue where the total image count in the person view excluded live images. The query now correctly accounts for all relevant assets by removing the condition that filtered out assets with a livePhotoVideoId.

Root Cause

The issue occurs because the original query included a condition to exclude assets with a livePhotoVideoId (i.e., live images), which led to an inaccurate image count. As a result, only static images were included in the count under a person’s name.

Solution

  1. Removed the condition .on('assets.livePhotoVideoId', 'is', null) from the LEFT JOIN clause.

This fix ensures that both live and static images are considered when counting the total number of images under a person’s profile.

Fixes #16094

How Has This Been Tested?

This PR has been tested on the following platforms:

  • ✅ Web

Before the Fix:

  • The image count under a person’s profile incorrectly showed only static images, missing live images from the count.

After the Fix:

  • The image count now correctly includes both live and static images.

Further Considerations

⚠️ Note: While this fix resolves the issue for now, need more information on why live photos were not included in the first place lest some other bug is introduced.

Checklist:

  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation, if applicable.
  • I have no unrelated changes in the PR.
  • I have confirmed that any new dependencies are strictly necessary.
  • I have followed naming conventions and patterns in the surrounding code.

Fixed an issue where the total image count in the person view excluded live images.
The query now correctly accounts for all relevant assets by removing the condition
that filtered out assets with a livePhotoVideoId.

Issue:
- Image count under a person’s name was inaccurate, showing only static images.

Fix:
- Removed `.on('assets.livePhotoVideoId', 'is', null)` from the LEFT JOIN condition.

Tested on:
- Web

Ran PR checklist
@immangat immangat marked this pull request as ready for review February 15, 2025 08:25
@danieldietzler
Copy link
Member

Please update the generated SQL by running make sql.

@immangat
Copy link
Contributor Author

@danieldietzler I ran make sql.

FYI: I had to run npm run build in /server before running make sql for it to work.

Also, I did not see this step in the PR checklist in the developer docs.

@alextran1502 alextran1502 enabled auto-merge (squash) February 17, 2025 15:45
@alextran1502 alextran1502 merged commit 1689cec into immich-app:main Feb 17, 2025
34 of 36 checks passed
@immangat immangat deleted the fix/total-image-count branch February 17, 2025 17:27
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.

Live photos not included in total image count for people
3 participants