fix: album sorting options (#5127)

* fix: album sort options

* fix: don't load assets

* pr feedback

* fix: albumStub

* fix(web): album shared without assets

* fix: tests

---------

Co-authored-by: Alex Tran <alex.tran1502@gmail.com>
This commit is contained in:
martin 2023-11-20 20:01:21 +01:00 committed by GitHub
parent 347e6191c5
commit 725f30c494
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 180 additions and 109 deletions

View file

@ -37,15 +37,6 @@ export const mapAlbum = (entity: AlbumEntity, withAssets: boolean): AlbumRespons
const hasSharedLink = entity.sharedLinks?.length > 0;
const hasSharedUser = sharedUsers.length > 0;
let startDate = assets.at(0)?.fileCreatedAt || undefined;
let endDate = assets.at(-1)?.fileCreatedAt || undefined;
// Swap dates if start date is greater than end date.
if (startDate && endDate && startDate > endDate) {
const temp = startDate;
startDate = endDate;
endDate = temp;
}
return {
albumName: entity.albumName,
description: entity.description,
@ -58,10 +49,10 @@ export const mapAlbum = (entity: AlbumEntity, withAssets: boolean): AlbumRespons
sharedUsers,
shared: hasSharedUser || hasSharedLink,
hasSharedLink,
startDate,
endDate,
startDate: entity.startDate ? entity.startDate : undefined,
endDate: entity.endDate ? entity.endDate : undefined,
assets: (withAssets ? assets : []).map((asset) => mapAsset(asset)),
assetCount: entity.assets?.length || 0,
assetCount: entity.assetCount,
isActivityEnabled: entity.isActivityEnabled,
};
};

View file

@ -58,10 +58,6 @@ describe(AlbumService.name, () => {
describe('getAll', () => {
it('gets list of albums for auth user', async () => {
albumMock.getOwned.mockResolvedValue([albumStub.empty, albumStub.sharedWithUser]);
albumMock.getAssetCountForIds.mockResolvedValue([
{ albumId: albumStub.empty.id, assetCount: 0 },
{ albumId: albumStub.sharedWithUser.id, assetCount: 0 },
]);
albumMock.getInvalidThumbnail.mockResolvedValue([]);
const result = await sut.getAll(authStub.admin, {});
@ -72,7 +68,6 @@ describe(AlbumService.name, () => {
it('gets list of albums that have a specific asset', async () => {
albumMock.getByAssetId.mockResolvedValue([albumStub.oneAsset]);
albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.oneAsset.id, assetCount: 1 }]);
albumMock.getInvalidThumbnail.mockResolvedValue([]);
const result = await sut.getAll(authStub.admin, { assetId: albumStub.oneAsset.id });
@ -83,7 +78,6 @@ describe(AlbumService.name, () => {
it('gets list of albums that are shared', async () => {
albumMock.getShared.mockResolvedValue([albumStub.sharedWithUser]);
albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.sharedWithUser.id, assetCount: 0 }]);
albumMock.getInvalidThumbnail.mockResolvedValue([]);
const result = await sut.getAll(authStub.admin, { shared: true });
@ -94,7 +88,6 @@ describe(AlbumService.name, () => {
it('gets list of albums that are NOT shared', async () => {
albumMock.getNotShared.mockResolvedValue([albumStub.empty]);
albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.empty.id, assetCount: 0 }]);
albumMock.getInvalidThumbnail.mockResolvedValue([]);
const result = await sut.getAll(authStub.admin, { shared: false });
@ -106,7 +99,6 @@ describe(AlbumService.name, () => {
it('counts assets correctly', async () => {
albumMock.getOwned.mockResolvedValue([albumStub.oneAsset]);
albumMock.getAssetCountForIds.mockResolvedValue([{ albumId: albumStub.oneAsset.id, assetCount: 1 }]);
albumMock.getInvalidThumbnail.mockResolvedValue([]);
const result = await sut.getAll(authStub.admin, {});
@ -118,9 +110,6 @@ describe(AlbumService.name, () => {
it('updates the album thumbnail by listing all albums', async () => {
albumMock.getOwned.mockResolvedValue([albumStub.oneAssetInvalidThumbnail]);
albumMock.getAssetCountForIds.mockResolvedValue([
{ albumId: albumStub.oneAssetInvalidThumbnail.id, assetCount: 1 },
]);
albumMock.getInvalidThumbnail.mockResolvedValue([albumStub.oneAssetInvalidThumbnail.id]);
albumMock.update.mockResolvedValue(albumStub.oneAssetValidThumbnail);
assetMock.getFirstAssetForAlbumId.mockResolvedValue(albumStub.oneAssetInvalidThumbnail.assets[0]);
@ -134,9 +123,6 @@ describe(AlbumService.name, () => {
it('removes the thumbnail for an empty album', async () => {
albumMock.getOwned.mockResolvedValue([albumStub.emptyWithInvalidThumbnail]);
albumMock.getAssetCountForIds.mockResolvedValue([
{ albumId: albumStub.emptyWithInvalidThumbnail.id, assetCount: 1 },
]);
albumMock.getInvalidThumbnail.mockResolvedValue([albumStub.emptyWithInvalidThumbnail.id]);
albumMock.update.mockResolvedValue(albumStub.emptyWithValidThumbnail);
assetMock.getFirstAssetForAlbumId.mockResolvedValue(null);

View file

@ -66,21 +66,12 @@ export class AlbumService {
albums = await this.albumRepository.getOwned(ownerId);
}
// Get asset count for each album. Then map the result to an object:
// { [albumId]: assetCount }
const albumsAssetCount = await this.albumRepository.getAssetCountForIds(albums.map((album) => album.id));
const albumsAssetCountObj = albumsAssetCount.reduce((obj: Record<string, number>, { albumId, assetCount }) => {
obj[albumId] = assetCount;
return obj;
}, {});
return Promise.all(
albums.map(async (album) => {
const lastModifiedAsset = await this.assetRepository.getLastUpdatedAssetForAlbumId(album.id);
return {
...mapAlbumWithoutAssets(album),
sharedLinks: undefined,
assetCount: albumsAssetCountObj[album.id],
lastModifiedAssetTimestamp: lastModifiedAsset?.fileModifiedAt,
};
}),

View file

@ -30,7 +30,6 @@ export interface IAlbumRepository {
hasAsset(asset: AlbumAsset): Promise<boolean>;
removeAsset(assetId: string): Promise<void>;
removeAssets(assets: AlbumAssets): Promise<void>;
getAssetCountForIds(ids: string[]): Promise<AlbumAssetCount[]>;
getInvalidThumbnail(): Promise<string[]>;
getOwned(ownerId: string): Promise<AlbumEntity[]>;
getShared(ownerId: string): Promise<AlbumEntity[]>;

View file

@ -40,7 +40,7 @@ export function mapSharedLink(sharedLink: SharedLinkEntity): SharedLinkResponseD
createdAt: sharedLink.createdAt,
expiresAt: sharedLink.expiresAt,
assets: assets.map((asset) => mapAsset(asset)),
album: sharedLink.album ? mapAlbumWithoutAssets(sharedLink.album) : undefined,
album: sharedLink.album?.id ? mapAlbumWithoutAssets(sharedLink.album) : undefined,
allowUpload: sharedLink.allowUpload,
allowDownload: sharedLink.allowDownload,
showMetadata: sharedLink.showExif,

View file

@ -9,6 +9,7 @@ import {
OneToMany,
PrimaryGeneratedColumn,
UpdateDateColumn,
VirtualColumn,
} from 'typeorm';
import { AssetEntity } from './asset.entity';
import { SharedLinkEntity } from './shared-link.entity';
@ -59,4 +60,34 @@ export class AlbumEntity {
@Column({ default: true })
isActivityEnabled!: boolean;
@VirtualColumn({
query: (alias) => `
SELECT MIN(assets."fileCreatedAt")
FROM "assets" assets
JOIN "albums_assets_assets" aa ON aa."assetsId" = assets.id
WHERE aa."albumsId" = ${alias}.id
`,
})
startDate!: Date | null;
@VirtualColumn({
query: (alias) => `
SELECT MAX(assets."fileCreatedAt")
FROM "assets" assets
JOIN "albums_assets_assets" aa ON aa."assetsId" = assets.id
WHERE aa."albumsId" = ${alias}.id
`,
})
endDate!: Date | null;
@VirtualColumn({
query: (alias) => `
SELECT COUNT(assets."id")
FROM "assets" assets
JOIN "albums_assets_assets" aa ON aa."assetsId" = assets.id
WHERE aa."albumsId" = ${alias}.id
`,
})
assetCount!: number;
}

View file

@ -1,4 +1,4 @@
import { AlbumAsset, AlbumAssetCount, AlbumAssets, AlbumInfoOptions, IAlbumRepository } from '@app/domain';
import { AlbumAsset, AlbumAssets, AlbumInfoOptions, IAlbumRepository } from '@app/domain';
import { Injectable } from '@nestjs/common';
import { InjectDataSource, InjectRepository } from '@nestjs/typeorm';
import { DataSource, FindOptionsOrder, FindOptionsRelations, In, IsNull, Not, Repository } from 'typeorm';
@ -59,28 +59,6 @@ export class AlbumRepository implements IAlbumRepository {
});
}
async getAssetCountForIds(ids: string[]): Promise<AlbumAssetCount[]> {
// Guard against running invalid query when ids list is empty.
if (!ids.length) {
return [];
}
// Only possible with query builder because of GROUP BY.
const countByAlbums = await this.repository
.createQueryBuilder('album')
.select('album.id')
.addSelect('COUNT(albums_assets.assetsId)', 'asset_count')
.leftJoin('albums_assets_assets', 'albums_assets', 'albums_assets.albumsId = album.id')
.where('album.id IN (:...ids)', { ids })
.groupBy('album.id')
.getRawMany();
return countByAlbums.map<AlbumAssetCount>((albumCount) => ({
albumId: albumCount['album_id'],
assetCount: Number(albumCount['asset_count']),
}));
}
/**
* Returns the album IDs that have an invalid thumbnail, when:
* - Thumbnail references an asset outside the album

View file

@ -19,6 +19,9 @@ export const albumStub = {
sharedLinks: [],
sharedUsers: [],
isActivityEnabled: true,
startDate: null,
endDate: null,
assetCount: 0,
}),
sharedWithUser: Object.freeze<AlbumEntity>({
id: 'album-2',
@ -35,6 +38,9 @@ export const albumStub = {
sharedLinks: [],
sharedUsers: [userStub.user1],
isActivityEnabled: true,
startDate: null,
endDate: null,
assetCount: 0,
}),
sharedWithMultiple: Object.freeze<AlbumEntity>({
id: 'album-3',
@ -51,6 +57,9 @@ export const albumStub = {
sharedLinks: [],
sharedUsers: [userStub.user1, userStub.user2],
isActivityEnabled: true,
startDate: null,
endDate: null,
assetCount: 0,
}),
sharedWithAdmin: Object.freeze<AlbumEntity>({
id: 'album-3',
@ -67,6 +76,9 @@ export const albumStub = {
sharedLinks: [],
sharedUsers: [userStub.admin],
isActivityEnabled: true,
startDate: null,
endDate: null,
assetCount: 0,
}),
oneAsset: Object.freeze<AlbumEntity>({
id: 'album-4',
@ -83,6 +95,9 @@ export const albumStub = {
sharedLinks: [],
sharedUsers: [],
isActivityEnabled: true,
startDate: assetStub.image.fileCreatedAt,
endDate: assetStub.image.fileCreatedAt,
assetCount: 1,
}),
twoAssets: Object.freeze<AlbumEntity>({
id: 'album-4a',
@ -99,6 +114,9 @@ export const albumStub = {
sharedLinks: [],
sharedUsers: [],
isActivityEnabled: true,
startDate: assetStub.withLocation.fileCreatedAt,
endDate: assetStub.image.fileCreatedAt,
assetCount: 2,
}),
emptyWithInvalidThumbnail: Object.freeze<AlbumEntity>({
id: 'album-5',
@ -115,6 +133,9 @@ export const albumStub = {
sharedLinks: [],
sharedUsers: [],
isActivityEnabled: true,
startDate: null,
endDate: null,
assetCount: 0,
}),
emptyWithValidThumbnail: Object.freeze<AlbumEntity>({
id: 'album-5',
@ -131,6 +152,9 @@ export const albumStub = {
sharedLinks: [],
sharedUsers: [],
isActivityEnabled: true,
startDate: null,
endDate: null,
assetCount: 0,
}),
oneAssetInvalidThumbnail: Object.freeze<AlbumEntity>({
id: 'album-6',
@ -147,6 +171,9 @@ export const albumStub = {
sharedLinks: [],
sharedUsers: [],
isActivityEnabled: true,
startDate: assetStub.image.fileCreatedAt,
endDate: assetStub.image.fileCreatedAt,
assetCount: 1,
}),
oneAssetValidThumbnail: Object.freeze<AlbumEntity>({
id: 'album-6',
@ -163,5 +190,8 @@ export const albumStub = {
sharedLinks: [],
sharedUsers: [],
isActivityEnabled: true,
startDate: assetStub.image.fileCreatedAt,
endDate: assetStub.image.fileCreatedAt,
assetCount: 1,
}),
};

View file

@ -181,6 +181,9 @@ export const sharedLinkStub = {
sharedUsers: [],
sharedLinks: [],
isActivityEnabled: true,
startDate: today,
endDate: today,
assetCount: 1,
assets: [
{
id: 'id_1',

View file

@ -5,7 +5,6 @@ export const newAlbumRepositoryMock = (): jest.Mocked<IAlbumRepository> => {
getById: jest.fn(),
getByIds: jest.fn(),
getByAssetId: jest.fn(),
getAssetCountForIds: jest.fn(),
getInvalidThumbnail: jest.fn(),
getOwned: jest.fn(),
getShared: jest.fn(),

View file

@ -5,10 +5,10 @@
export let option: Sort;
const handleSort = () => {
if (albumViewSettings === option.sortTitle) {
if (albumViewSettings === option.title) {
option.sortDesc = !option.sortDesc;
} else {
albumViewSettings = option.sortTitle;
albumViewSettings = option.title;
}
};
</script>
@ -18,12 +18,12 @@
class="rounded-lg p-2 hover:bg-immich-dark-primary hover:dark:bg-immich-dark-primary/50"
on:click={() => handleSort()}
>
{#if albumViewSettings === option.sortTitle}
{#if albumViewSettings === option.title}
{#if option.sortDesc}
&#8595;
{:else}
&#8593;
{/if}
{/if}{option.table}</button
{/if}{option.title}</button
></th
>

View file

@ -7,6 +7,7 @@
import { createEventDispatcher } from 'svelte';
import { goto } from '$app/navigation';
import { mdiCircleEditOutline, mdiContentCopy, mdiDelete, mdiOpenInNew } from '@mdi/js';
import noThumbnailUrl from '$lib/assets/no-thumbnail.png';
export let link: SharedLinkResponseDto;
@ -60,18 +61,28 @@
class="flex w-full gap-4 border-b border-gray-200 py-4 transition-all hover:border-immich-primary dark:border-gray-600 dark:text-immich-gray dark:hover:border-immich-dark-primary"
>
<div>
{#await getAssetInfo()}
<LoadingSpinner />
{:then asset}
{#if (link?.album?.assetCount && link?.album?.assetCount > 0) || link.assets.length > 0}
{#await getAssetInfo()}
<LoadingSpinner />
{:then asset}
<img
id={asset.id}
src={api.getAssetThumbnailUrl(asset.id, ThumbnailFormat.Webp)}
alt={asset.id}
class="h-[100px] w-[100px] rounded-lg object-cover"
loading="lazy"
draggable="false"
/>
{/await}
{:else}
<img
id={asset.id}
src={api.getAssetThumbnailUrl(asset.id, ThumbnailFormat.Webp)}
alt={asset.id}
src={noThumbnailUrl}
alt={'Album without assets'}
class="h-[100px] w-[100px] rounded-lg object-cover"
loading="lazy"
draggable="false"
/>
{/await}
{/if}
</div>
<div class="flex flex-col justify-between">

View file

@ -1,9 +1,6 @@
<script lang="ts" context="module">
// table is the text printed in the table and sortTitle is the text printed in the dropDow menu
export interface Sort {
table: string;
sortTitle: string;
title: string;
sortDesc: boolean;
widthClass: string;
sortFn: (reverse: boolean, albums: AlbumResponseDto[]) => AlbumResponseDto[];
@ -54,46 +51,75 @@
let sortByOptions: Record<string, Sort> = {
albumTitle: {
table: 'Album title',
sortTitle: 'Album title',
title: 'Album title',
sortDesc: $albumViewSettings.sortDesc, // Load Sort Direction
widthClass: 'w-8/12 text-left sm:w-4/12 md:w-4/12 md:w-4/12 2xl:w-6/12',
widthClass: 'text-left w-8/12 sm:w-4/12 md:w-4/12 md:w-4/12 xl:w-[30%] 2xl:w-[40%]',
sortFn: (reverse, albums) => {
return orderBy(albums, 'albumName', [reverse ? 'desc' : 'asc']);
},
},
numberOfAssets: {
table: 'Assets',
sortTitle: 'Number of assets',
title: 'Number of assets',
sortDesc: $albumViewSettings.sortDesc,
widthClass: 'w-4/12 text-center sm:w-2/12 2xl:w-1/12',
widthClass: 'text-center w-4/12 m:w-2/12 md:w-2/12 xl:w-[15%] 2xl:w-[12%]',
sortFn: (reverse, albums) => {
return orderBy(albums, 'assetCount', [reverse ? 'desc' : 'asc']);
},
},
lastModified: {
table: 'Updated date',
sortTitle: 'Last modified',
title: 'Last modified',
sortDesc: $albumViewSettings.sortDesc,
widthClass: 'text-center hidden sm:block w-3/12 lg:w-2/12',
widthClass: 'text-center hidden sm:block w-3/12 xl:w-[15%] 2xl:w-[12%]',
sortFn: (reverse, albums) => {
return orderBy(albums, [(album) => new Date(album.updatedAt)], [reverse ? 'desc' : 'asc']);
},
},
mostRecent: {
table: 'Created date',
sortTitle: 'Most recent photo',
created: {
title: 'Created date',
sortDesc: $albumViewSettings.sortDesc,
widthClass: 'text-center hidden sm:block w-3/12 lg:w-2/12',
widthClass: 'text-center hidden sm:block w-3/12 xl:w-[15%] 2xl:w-[12%]',
sortFn: (reverse, albums) => {
return orderBy(albums, [(album) => new Date(album.createdAt)], [reverse ? 'desc' : 'asc']);
},
},
mostRecent: {
title: 'Most recent photo',
sortDesc: $albumViewSettings.sortDesc,
widthClass: 'text-center hidden xl:block xl:w-[15%] 2xl:w-[12%]',
sortFn: (reverse, albums) => {
return orderBy(
albums,
[
(album) =>
album.lastModifiedAssetTimestamp ? new Date(album.lastModifiedAssetTimestamp) : new Date(album.updatedAt),
],
[(album) => (album.endDate ? new Date(album.endDate) : '')],
[reverse ? 'desc' : 'asc'],
);
).sort((a, b) => {
if (a.endDate === undefined) {
return 1;
}
if (b.endDate === undefined) {
return -1;
}
return 0;
});
},
},
mostOld: {
title: 'Oldest photo',
sortDesc: $albumViewSettings.sortDesc,
widthClass: 'text-center hidden xl:block xl:w-[15%] 2xl:w-[12%]',
sortFn: (reverse, albums) => {
return orderBy(
albums,
[(album) => (album.startDate ? new Date(album.startDate) : null)],
[reverse ? 'desc' : 'asc'],
).sort((a, b) => {
if (a.startDate === undefined) {
return 1;
}
if (b.startDate === undefined) {
return -1;
}
return 0;
});
},
},
};
@ -144,16 +170,25 @@
};
$: {
const { sortBy } = $albumViewSettings;
for (const key in sortByOptions) {
if (sortByOptions[key].sortTitle === sortBy) {
if (sortByOptions[key].title === $albumViewSettings.sortBy) {
$albums = sortByOptions[key].sortFn(sortByOptions[key].sortDesc, $unsortedAlbums);
$albumViewSettings.sortDesc = sortByOptions[key].sortDesc; // "Save" sortDesc
$albumViewSettings.sortBy = sortByOptions[key].title;
break;
}
}
}
const test = (searched: string): Sort => {
for (const key in sortByOptions) {
if (sortByOptions[key].title === searched) {
return sortByOptions[key];
}
}
return sortByOptions[0];
};
const handleCreateAlbum = async () => {
const newAlbum = await createAlbum();
if (newAlbum) {
@ -220,19 +255,20 @@
<Dropdown
options={Object.values(sortByOptions)}
selectedOption={test($albumViewSettings.sortBy)}
render={(option) => {
return {
title: option.sortTitle,
title: option.title,
icon: option.sortDesc ? mdiArrowDownThin : mdiArrowUpThin,
};
}}
on:select={(event) => {
for (const key in sortByOptions) {
if (sortByOptions[key].sortTitle === event.detail.sortTitle) {
if (sortByOptions[key].title === event.detail.title) {
sortByOptions[key].sortDesc = !sortByOptions[key].sortDesc;
$albumViewSettings.sortBy = sortByOptions[key].title;
}
}
$albumViewSettings.sortBy = event.detail.sortTitle;
}}
/>
@ -271,7 +307,7 @@
{#each Object.keys(sortByOptions) as key (key)}
<TableHeader bind:albumViewSettings={$albumViewSettings.sortBy} bind:option={sortByOptions[key]} />
{/each}
<th class="hidden w-2/12 text-center text-sm font-medium lg:block 2xl:w-1/12">Action</th>
<th class="hidden text-center text-sm font-medium 2xl:block 2xl:w-[12%]">Action</th>
</tr>
</thead>
<tbody
@ -284,18 +320,34 @@
on:keydown={(event) => event.key === 'Enter' && goto(`albums/${album.id}`)}
tabindex="0"
>
<td class="text-md w-8/12 text-ellipsis text-left sm:w-4/12 md:w-4/12 2xl:w-6/12">{album.albumName}</td>
<td class="text-md w-4/12 text-ellipsis text-center sm:w-2/12 md:w-2/12 2xl:w-1/12">
{album.assetCount}
{album.assetCount == 1 ? `item` : `items`}
</td>
<td class="text-md hidden w-3/12 text-ellipsis text-center sm:block lg:w-2/12"
>{dateLocaleString(album.updatedAt)}</td
<td class="text-md text-ellipsis text-left w-8/12 sm:w-4/12 md:w-4/12 xl:w-[30%] 2xl:w-[40%]"
>{album.albumName}</td
>
<td class="text-md hidden w-3/12 text-ellipsis text-center sm:block lg:w-2/12"
<td class="text-md text-ellipsis text-center sm:w-2/12 md:w-2/12 xl:w-[15%] 2xl:w-[12%]">
{album.assetCount}
{album.assetCount > 1 ? `items` : `item`}
</td>
<td class="text-md hidden text-ellipsis text-center sm:block w-3/12 xl:w-[15%] 2xl:w-[12%]"
>{dateLocaleString(album.updatedAt)}
</td>
<td class="text-md hidden text-ellipsis text-center sm:block w-3/12 xl:w-[15%] 2xl:w-[12%]"
>{dateLocaleString(album.createdAt)}</td
>
<td class="text-md hidden w-2/12 text-ellipsis text-center lg:block 2xl:w-1/12">
<td class="text-md text-ellipsis text-center hidden xl:block xl:w-[15%] 2xl:w-[12%]">
{#if album.endDate}
{dateLocaleString(album.endDate)}
{:else}
&#10060;
{/if}</td
>
<td class="text-md text-ellipsis text-center hidden xl:block xl:w-[15%] 2xl:w-[12%]"
>{#if album.startDate}
{dateLocaleString(album.startDate)}
{:else}
&#10060;
{/if}</td
>
<td class="text-md hidden text-ellipsis text-center 2xl:block xl:w-[15%] 2xl:w-[12%]">
<button
on:click|stopPropagation={() => handleEdit(album)}
class="rounded-full bg-immich-primary p-3 text-gray-100 transition-all duration-150 hover:bg-immich-primary/75 dark:bg-immich-dark-primary dark:text-gray-700"