-
Notifications
You must be signed in to change notification settings - Fork 70
feat: support selecting photos from tags #537
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| using NUnit.Framework; | ||
| using Moq; | ||
| using ImmichFrame.Core.Api; | ||
| using ImmichFrame.Core.Interfaces; | ||
| using ImmichFrame.Core.Logic.Pool; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Threading.Tasks; | ||
| using System.Threading; | ||
|
|
||
| namespace ImmichFrame.Core.Tests.Logic.Pool; | ||
|
|
||
| [TestFixture] | ||
| public class TagAssetsPoolTests | ||
| { | ||
| private Mock<IApiCache> _mockApiCache; | ||
| private Mock<ImmichApi> _mockImmichApi; | ||
| private Mock<IAccountSettings> _mockAccountSettings; | ||
| private TestableTagAssetsPool _tagAssetsPool; | ||
|
|
||
| private class TestableTagAssetsPool : TagAssetsPool | ||
| { | ||
| public TestableTagAssetsPool(IApiCache apiCache, ImmichApi immichApi, IAccountSettings accountSettings) | ||
| : base(apiCache, immichApi, accountSettings) { } | ||
|
|
||
| public Task<IEnumerable<AssetResponseDto>> TestLoadAssets(CancellationToken ct = default) | ||
| { | ||
| return base.LoadAssets(ct); | ||
| } | ||
| } | ||
|
|
||
| [SetUp] | ||
| public void Setup() | ||
| { | ||
| _mockApiCache = new Mock<IApiCache>(); | ||
| _mockImmichApi = new Mock<ImmichApi>(null, null); | ||
| _mockAccountSettings = new Mock<IAccountSettings>(); | ||
| _tagAssetsPool = new TestableTagAssetsPool(_mockApiCache.Object, _mockImmichApi.Object, _mockAccountSettings.Object); | ||
|
|
||
| _mockAccountSettings.SetupGet(s => s.Tags).Returns(new List<Guid>()); | ||
| } | ||
|
|
||
| private AssetResponseDto CreateAsset(string id) => new AssetResponseDto { Id = id, Type = AssetTypeEnum.IMAGE, Tags = new List<TagResponseDto>() }; | ||
| private SearchResponseDto CreateSearchResult(List<AssetResponseDto> assets, int total) => | ||
| new SearchResponseDto { Assets = new SearchAssetResponseDto { Items = assets, Total = total } }; | ||
|
|
||
| [Test] | ||
| public async Task LoadAssets_CallsSearchAssetsForEachTag_AndPaginates() | ||
| { | ||
| // Arrange | ||
| var tag1Id = Guid.NewGuid(); | ||
| var tag2Id = Guid.NewGuid(); | ||
| _mockAccountSettings.SetupGet(s => s.Tags).Returns(new List<Guid> { tag1Id, tag2Id }); | ||
|
|
||
| var batchSize = 1000; // From TagAssetsPool.cs | ||
| var t1AssetsPage1 = Enumerable.Range(0, batchSize).Select(i => CreateAsset($"t1_p1_{i}")).ToList(); | ||
| var t1AssetsPage2 = Enumerable.Range(0, 30).Select(i => CreateAsset($"t1_p2_{i}")).ToList(); | ||
| var t2AssetsPage1 = Enumerable.Range(0, 20).Select(i => CreateAsset($"t2_p1_{i}")).ToList(); | ||
|
|
||
| // Tag 1 - Page 1 | ||
| _mockImmichApi.Setup(api => api.SearchAssetsAsync(It.Is<MetadataSearchDto>(d => d.TagIds.Contains(tag1Id) && d.Page == 1), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(CreateSearchResult(t1AssetsPage1, batchSize)); | ||
| // Tag 1 - Page 2 | ||
| _mockImmichApi.Setup(api => api.SearchAssetsAsync(It.Is<MetadataSearchDto>(d => d.TagIds.Contains(tag1Id) && d.Page == 2), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(CreateSearchResult(t1AssetsPage2, 30)); | ||
| // Tag 2 - Page 1 | ||
| _mockImmichApi.Setup(api => api.SearchAssetsAsync(It.Is<MetadataSearchDto>(d => d.TagIds.Contains(tag2Id) && d.Page == 1), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(CreateSearchResult(t2AssetsPage1, 20)); | ||
|
|
||
| // Act | ||
| var result = (await _tagAssetsPool.TestLoadAssets()).ToList(); | ||
|
|
||
| // Assert | ||
| Assert.That(result.Count, Is.EqualTo(batchSize + 30 + 20)); | ||
| Assert.That(result.Any(a => a.Id == "t1_p1_0")); | ||
| Assert.That(result.Any(a => a.Id == "t1_p2_29")); | ||
| Assert.That(result.Any(a => a.Id == "t2_p1_19")); | ||
|
|
||
| _mockImmichApi.Verify(api => api.SearchAssetsAsync(It.Is<MetadataSearchDto>(d => d.TagIds.Contains(tag1Id) && d.Page == 1), It.IsAny<CancellationToken>()), Times.Once); | ||
| _mockImmichApi.Verify(api => api.SearchAssetsAsync(It.Is<MetadataSearchDto>(d => d.TagIds.Contains(tag1Id) && d.Page == 2), It.IsAny<CancellationToken>()), Times.Once); | ||
| _mockImmichApi.Verify(api => api.SearchAssetsAsync(It.Is<MetadataSearchDto>(d => d.TagIds.Contains(tag2Id) && d.Page == 1), It.IsAny<CancellationToken>()), Times.Once); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task LoadAssets_NoTagsConfigured_ReturnsEmpty() | ||
| { | ||
| _mockAccountSettings.SetupGet(s => s.Tags).Returns(new List<Guid>()); | ||
| var result = (await _tagAssetsPool.TestLoadAssets()).ToList(); | ||
| Assert.That(result, Is.Empty); | ||
| _mockImmichApi.Verify(api => api.SearchAssetsAsync(It.IsAny<MetadataSearchDto>(), It.IsAny<CancellationToken>()), Times.Never); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task LoadAssets_TagHasNoAssets_DoesNotAffectOthers() | ||
| { | ||
| var tag1Id = Guid.NewGuid(); // Has assets | ||
| var tag2Id = Guid.NewGuid(); // No assets | ||
| _mockAccountSettings.SetupGet(s => s.Tags).Returns(new List<Guid> { tag1Id, tag2Id }); | ||
|
|
||
| var t1Assets = Enumerable.Range(0, 10).Select(i => CreateAsset($"t1_{i}")).ToList(); | ||
| _mockImmichApi.Setup(api => api.SearchAssetsAsync(It.Is<MetadataSearchDto>(d => d.TagIds.Contains(tag1Id)), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(CreateSearchResult(t1Assets, 10)); | ||
| _mockImmichApi.Setup(api => api.SearchAssetsAsync(It.Is<MetadataSearchDto>(d => d.TagIds.Contains(tag2Id)), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(CreateSearchResult(new List<AssetResponseDto>(), 0)); | ||
|
|
||
| var result = (await _tagAssetsPool.TestLoadAssets()).ToList(); | ||
| Assert.That(result.Count, Is.EqualTo(10)); | ||
| Assert.That(result.All(a => a.Id.StartsWith("t1_"))); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task LoadAssets_PassesCorrectMetadataSearchParameters() | ||
| { | ||
| var tagId = Guid.NewGuid(); | ||
| _mockAccountSettings.SetupGet(s => s.Tags).Returns(new List<Guid> { tagId }); | ||
|
|
||
| var assets = Enumerable.Range(0, 5).Select(i => CreateAsset($"asset_{i}")).ToList(); | ||
| _mockImmichApi.Setup(api => api.SearchAssetsAsync(It.IsAny<MetadataSearchDto>(), It.IsAny<CancellationToken>())) | ||
| .ReturnsAsync(CreateSearchResult(assets, 5)); | ||
|
|
||
| await _tagAssetsPool.TestLoadAssets(); | ||
|
|
||
| _mockImmichApi.Verify(api => api.SearchAssetsAsync( | ||
| It.Is<MetadataSearchDto>(d => | ||
| d.TagIds.Contains(tagId) && | ||
| d.Page == 1 && | ||
| d.Size == 1000 && | ||
| d.Type == AssetTypeEnum.IMAGE && | ||
| d.WithExif == true && | ||
| d.WithPeople == true | ||
| ), It.IsAny<CancellationToken>()), Times.Once); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| using ImmichFrame.Core.Api; | ||
| using ImmichFrame.Core.Interfaces; | ||
|
|
||
| namespace ImmichFrame.Core.Logic.Pool; | ||
|
|
||
| public class TagAssetsPool(IApiCache apiCache, ImmichApi immichApi, IAccountSettings accountSettings) : CachingApiAssetsPool(apiCache, immichApi, accountSettings) | ||
| { | ||
| protected override async Task<IEnumerable<AssetResponseDto>> LoadAssets(CancellationToken ct = default) | ||
| { | ||
| var tagAssets = new List<AssetResponseDto>(); | ||
|
|
||
| foreach (var tagId in accountSettings.Tags) | ||
| { | ||
| int page = 1; | ||
| int batchSize = 1000; | ||
| int total; | ||
| do | ||
| { | ||
| var metadataBody = new MetadataSearchDto | ||
| { | ||
| Page = page, | ||
| Size = batchSize, | ||
| TagIds = [tagId], | ||
| Type = AssetTypeEnum.IMAGE, | ||
| WithExif = true, | ||
| WithPeople = true | ||
| }; | ||
|
|
||
| var tagInfo = await immichApi.SearchAssetsAsync(metadataBody, ct); | ||
|
|
||
| total = tagInfo.Assets.Total; | ||
|
|
||
| // Fetch full asset details to get tag information | ||
| foreach (var asset in tagInfo.Assets.Items) | ||
| { | ||
| if (asset.Tags == null) | ||
| { | ||
| var assetInfo = await immichApi.GetAssetInfoAsync(new Guid(asset.Id), null, ct); | ||
| asset.Tags = assetInfo.Tags; | ||
| asset.ExifInfo = assetInfo.ExifInfo; | ||
| asset.People = assetInfo.People; | ||
| } | ||
| } | ||
|
|
||
| tagAssets.AddRange(tagInfo.Assets.Items); | ||
| page++; | ||
| } while (total == batchSize); | ||
| } | ||
|
|
||
| return tagAssets; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,9 @@ | |
| ], | ||
| "People": [ | ||
| "UUID" | ||
| ], | ||
| "Tags": [ | ||
| "UUID" | ||
| ] | ||
| } | ||
| ] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,3 +52,5 @@ Accounts: | |
| - UUID | ||
| People: | ||
| - UUID | ||
| Tags: | ||
| - UUID | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,6 +74,8 @@ General: | |
| ShowImageDesc: true # boolean | ||
| # Displays a comma separated list of names of all the people that are assigned in immich. | ||
| ShowPeopleDesc: true # boolean | ||
| # Displays a comma separated list of names of all the tags that are assigned in immich. | ||
| ShowTagsDesc: true # boolean | ||
| # Displays a comma separated list of names of all the albums for an image. | ||
| ShowAlbumName: true # boolean | ||
| # Displays the location of the current image. | ||
|
|
@@ -130,16 +132,21 @@ Accounts: | |
| # UUID of People | ||
| People: # string[] | ||
| - UUID | ||
| # UUID of Tags | ||
| Tags: # string[] | ||
| - UUID | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the easiest way to get a tag UUID for a user? Wouldn't
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe? I guess there could be issues when renaming tags. Mostly was trying to stay consistent with people and albums. If we used value for tags, then why not use a value for people and albums as well? But I'm happy to change it to value if you prefer
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Re: easiest way, I didn't see a way to get it outside the API. But maybe I didn't look hard enough. Personally I'm fine with using the API, but you're right that for most users, that would be a barrier. Probably worth mentioning that tags aren't enabled by default, so they're already power user territory.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
People and Albums could have the same With I'd suggest using the tags Please let me know what you think about this. :) I know that it would be easier to use the UUID for you, but for the end user it's better to have the value, since there won't be tag duplicates (afaik) |
||
|
|
||
| ``` | ||
| ### Security | ||
| Basic authentication can be added via `AuthenticationSecret`. It is **NOT** recommended to expose immichFrame to the public web, if you still choose to do so, you can set this to a secure secret. Every client needs to authenticate itself with this secret. This can be done in the Webclient via input field or via URL-Parameter. The URL-Parameter will look like this: `?authsecret=[MYSECRET]` | ||
|
|
||
| If this is enabled, the web api required the `Authorization`-Header with `Bearer [MYSECRET]`. | ||
|
|
||
| ### Filtering on Albums or People | ||
| ### Filtering on Albums, People, or Tags | ||
| You can get the UUIDs from the URL of the album/person. For this URL: `https://demo.immich.app/albums/85c85b29-c95d-4a8b-90f7-c87da1d518ba` this is the UUID: `85c85b29-c95d-4a8b-90f7-c87da1d518ba` | ||
|
|
||
| To get Tag UUIDs, you can use the Immich API: `GET /api/tags` with your API key, which returns all tags with their IDs. | ||
|
|
||
| ### Weather | ||
| Weather is enabled by entering an API key. Get yours free from [OpenWeatherMap][openweathermap-url] | ||
|
|
||
|
|
@@ -176,6 +183,7 @@ For full ImmichFrame functionality, the API key being used needs the following p | |
| - `memory.read` | ||
| - `person.read` | ||
| - `person.statistics` | ||
| - `tag.read` | ||
|
|
||
|
|
||
| ### Custom CSS | ||
|
|
||
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.
🛠️ Refactor suggestion | 🟠 Major
Add test coverage for the GetAssetInfoAsync code path.
The
CreateAssethelper always initializesTagsas an empty list (not null), so the tests never exercise the code path inTagAssetsPool.LoadAssetswhereasset.Tags == nulltriggers a call toGetAssetInfoAsyncto fetch full asset details. This is a critical path that should be tested.🔎 Suggested test case for null Tags scenario
Add a test method to verify the GetAssetInfoAsync behavior:
🤖 Prompt for AI Agents