From 77a362f0c0fa6f7757519ab664c68c1ba0253cf2 Mon Sep 17 00:00:00 2001 From: SGT Date: Fri, 13 Jun 2025 16:18:44 -0300 Subject: [PATCH 01/12] chore(server): replace usage of deprecated orderBy and remove unnecessary instruction (#19072) * replace usage of deprecated orderBy instruction remove unnecesarry extra order instruction update e2e test * rename symbols --- e2e/src/api/specs/person.e2e-spec.ts | 75 ++++++++++++++++++-- server/src/repositories/person.repository.ts | 3 +- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/e2e/src/api/specs/person.e2e-spec.ts b/e2e/src/api/specs/person.e2e-spec.ts index 1826002af6..a56118a68b 100644 --- a/e2e/src/api/specs/person.e2e-spec.ts +++ b/e2e/src/api/specs/person.e2e-spec.ts @@ -11,11 +11,24 @@ describe('/people', () => { let hiddenPerson: PersonResponseDto; let multipleAssetsPerson: PersonResponseDto; + let nameAlicePerson: PersonResponseDto; + let nameBobPerson: PersonResponseDto; + let nameCharliePerson: PersonResponseDto; + let nameNullPerson: PersonResponseDto; + beforeAll(async () => { await utils.resetDatabase(); admin = await utils.adminSetup(); - [visiblePerson, hiddenPerson, multipleAssetsPerson] = await Promise.all([ + [ + visiblePerson, + hiddenPerson, + multipleAssetsPerson, + nameCharliePerson, + nameBobPerson, + nameAlicePerson, + nameNullPerson, + ] = await Promise.all([ utils.createPerson(admin.accessToken, { name: 'visible_person', }), @@ -26,10 +39,24 @@ describe('/people', () => { utils.createPerson(admin.accessToken, { name: 'multiple_assets_person', }), + // --- Setup for the specific sorting test --- + utils.createPerson(admin.accessToken, { + name: 'Charlie', + }), + utils.createPerson(admin.accessToken, { + name: 'Bob', + }), + utils.createPerson(admin.accessToken, { + name: 'Alice', + }), + utils.createPerson(admin.accessToken, { + name: '', + }), ]); const asset1 = await utils.createAsset(admin.accessToken); const asset2 = await utils.createAsset(admin.accessToken); + const asset3 = await utils.createAsset(admin.accessToken); await Promise.all([ utils.createFace({ assetId: asset1.id, personId: visiblePerson.id }), @@ -37,6 +64,15 @@ describe('/people', () => { utils.createFace({ assetId: asset1.id, personId: multipleAssetsPerson.id }), utils.createFace({ assetId: asset1.id, personId: multipleAssetsPerson.id }), utils.createFace({ assetId: asset2.id, personId: multipleAssetsPerson.id }), + utils.createFace({ assetId: asset3.id, personId: multipleAssetsPerson.id }), + // Named persons + utils.createFace({ assetId: asset1.id, personId: nameCharliePerson.id }), // 1 asset + utils.createFace({ assetId: asset1.id, personId: nameBobPerson.id }), + utils.createFace({ assetId: asset2.id, personId: nameBobPerson.id }), // 2 assets + utils.createFace({ assetId: asset1.id, personId: nameAlicePerson.id }), // 1 asset + // Null-named person + utils.createFace({ assetId: asset1.id, personId: nameNullPerson.id }), + utils.createFace({ assetId: asset2.id, personId: nameNullPerson.id }), // 2 assets ]); }); @@ -51,26 +87,53 @@ describe('/people', () => { expect(status).toBe(200); expect(body).toEqual({ hasNextPage: false, - total: 3, + total: 7, hidden: 1, people: [ expect.objectContaining({ name: 'multiple_assets_person' }), + expect.objectContaining({ name: 'Bob' }), + expect.objectContaining({ name: 'Alice' }), + expect.objectContaining({ name: 'Charlie' }), expect.objectContaining({ name: 'visible_person' }), expect.objectContaining({ name: 'hidden_person' }), ], }); }); + it('should sort visible people by asset count (desc), then by name (asc, nulls last)', async () => { + const { status, body } = await request(app).get('/people').set('Authorization', `Bearer ${admin.accessToken}`); + + expect(status).toBe(200); + expect(body.hasNextPage).toBe(false); + expect(body.total).toBe(7); // All persons + expect(body.hidden).toBe(1); // 'hidden_person' + + const people = body.people as PersonResponseDto[]; + + expect(people.map((p) => p.id)).toEqual([ + multipleAssetsPerson.id, // name: 'multiple_assets_person', count: 3 + nameBobPerson.id, // name: 'Bob', count: 2 + nameAlicePerson.id, // name: 'Alice', count: 1 + nameCharliePerson.id, // name: 'Charlie', count: 1 + visiblePerson.id, // name: 'visible_person', count: 1 + ]); + + expect(people.some((p) => p.id === hiddenPerson.id)).toBe(false); + }); + it('should return only visible people', async () => { const { status, body } = await request(app).get('/people').set('Authorization', `Bearer ${admin.accessToken}`); expect(status).toBe(200); expect(body).toEqual({ hasNextPage: false, - total: 3, + total: 7, hidden: 1, people: [ expect.objectContaining({ name: 'multiple_assets_person' }), + expect.objectContaining({ name: 'Bob' }), + expect.objectContaining({ name: 'Alice' }), + expect.objectContaining({ name: 'Charlie' }), expect.objectContaining({ name: 'visible_person' }), ], }); @@ -80,12 +143,12 @@ describe('/people', () => { const { status, body } = await request(app) .get('/people') .set('Authorization', `Bearer ${admin.accessToken}`) - .query({ withHidden: true, page: 2, size: 1 }); + .query({ withHidden: true, page: 5, size: 1 }); expect(status).toBe(200); expect(body).toEqual({ hasNextPage: true, - total: 3, + total: 7, hidden: 1, people: [expect.objectContaining({ name: 'visible_person' })], }); @@ -128,7 +191,7 @@ describe('/people', () => { .set('Authorization', `Bearer ${admin.accessToken}`); expect(status).toBe(200); - expect(body).toEqual(expect.objectContaining({ assets: 2 })); + expect(body).toEqual(expect.objectContaining({ assets: 3 })); }); }); diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index 229a523c17..6633fb68ac 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -179,9 +179,8 @@ export class PersonRepository { ) .$if(!options?.closestFaceAssetId, (qb) => qb - .orderBy(sql`NULLIF(person.name, '') is null`, 'asc') .orderBy((eb) => eb.fn.count('asset_faces.assetId'), 'desc') - .orderBy(sql`NULLIF(person.name, '')`, sql`asc nulls last`) + .orderBy(sql`NULLIF(person.name, '') asc nulls last`) .orderBy('person.createdAt'), ) .$if(!options?.withHidden, (qb) => qb.where('person.isHidden', '=', false)) From 523fe5bef718140735a7d2847aef74620026b63f Mon Sep 17 00:00:00 2001 From: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com> Date: Sun, 15 Jun 2025 00:10:33 +0200 Subject: [PATCH 02/12] refactor: album options modal (#19177) --- .../album-page/album-options.svelte | 202 ------------------ web/src/lib/modals/AlbumOptionsModal.svelte | 193 +++++++++++++++++ .../[[assetId=id]]/+page.svelte | 85 +++----- 3 files changed, 218 insertions(+), 262 deletions(-) delete mode 100644 web/src/lib/components/album-page/album-options.svelte create mode 100644 web/src/lib/modals/AlbumOptionsModal.svelte diff --git a/web/src/lib/components/album-page/album-options.svelte b/web/src/lib/components/album-page/album-options.svelte deleted file mode 100644 index 3a20e10602..0000000000 --- a/web/src/lib/components/album-page/album-options.svelte +++ /dev/null @@ -1,202 +0,0 @@ - - -{#if !selectedRemoveUser} - - -
-
-

{$t('settings').toUpperCase()}

-
- {#if order} - - {/if} - -
-
-
-
{$t('people').toUpperCase()}
-
- - -
-
- -
-
{user.name}
-
{$t('owner')}
-
- - {#each album.albumUsers as { user, role } (user.id)} -
-
- -
-
{user.name}
- {#if role === AlbumUserRole.Viewer} - {$t('role_viewer')} - {:else} - {$t('role_editor')} - {/if} - {#if user.id !== album.ownerId} - - {#if role === AlbumUserRole.Viewer} - handleUpdateSharedUserRole(user, AlbumUserRole.Editor)} - text={$t('allow_edits')} - /> - {:else} - handleUpdateSharedUserRole(user, AlbumUserRole.Viewer)} - text={$t('disallow_edits')} - /> - {/if} - - handleMenuRemove(user)} text={$t('remove')} /> - - {/if} -
- {/each} -
-
-
- - -{/if} - -{#if selectedRemoveUser} - (confirmed ? handleRemoveUser() : (selectedRemoveUser = null))} - /> -{/if} diff --git a/web/src/lib/modals/AlbumOptionsModal.svelte b/web/src/lib/modals/AlbumOptionsModal.svelte new file mode 100644 index 0000000000..f9c9cab204 --- /dev/null +++ b/web/src/lib/modals/AlbumOptionsModal.svelte @@ -0,0 +1,193 @@ + + + onClose({ action: 'refreshAlbum' })} size="small"> + +
+
+

{$t('settings').toUpperCase()}

+
+ {#if order} + + {/if} + +
+
+
+
{$t('people').toUpperCase()}
+
+ + +
+
+ +
+
{user.name}
+
{$t('owner')}
+
+ + {#each album.albumUsers as { user, role } (user.id)} +
+
+ +
+
{user.name}
+ {#if role === AlbumUserRole.Viewer} + {$t('role_viewer')} + {:else} + {$t('role_editor')} + {/if} + {#if user.id !== album.ownerId} + + {#if role === AlbumUserRole.Viewer} + handleUpdateSharedUserRole(user, AlbumUserRole.Editor)} + text={$t('allow_edits')} + /> + {:else} + handleUpdateSharedUserRole(user, AlbumUserRole.Viewer)} + text={$t('disallow_edits')} + /> + {/if} + + handleRemoveUser(user)} text={$t('remove')} /> + + {/if} +
+ {/each} +
+
+
+ + diff --git a/web/src/routes/(user)/albums/[albumId=id]/[[photos=photos]]/[[assetId=id]]/+page.svelte b/web/src/routes/(user)/albums/[albumId=id]/[[photos=photos]]/[[assetId=id]]/+page.svelte index faaa7f6ff6..f21230a316 100644 --- a/web/src/routes/(user)/albums/[albumId=id]/[[photos=photos]]/[[assetId=id]]/+page.svelte +++ b/web/src/routes/(user)/albums/[albumId=id]/[[photos=photos]]/[[assetId=id]]/+page.svelte @@ -4,7 +4,6 @@ import CastButton from '$lib/cast/cast-button.svelte'; import AlbumDescription from '$lib/components/album-page/album-description.svelte'; import AlbumMap from '$lib/components/album-page/album-map.svelte'; - import AlbumOptions from '$lib/components/album-page/album-options.svelte'; import AlbumSummary from '$lib/components/album-page/album-summary.svelte'; import AlbumTitle from '$lib/components/album-page/album-title.svelte'; import ActivityStatus from '$lib/components/asset-viewer/activity-status.svelte'; @@ -38,6 +37,7 @@ import { modalManager } from '$lib/managers/modal-manager.svelte'; import { TimelineManager } from '$lib/managers/timeline-manager/timeline-manager.svelte'; import type { TimelineAsset } from '$lib/managers/timeline-manager/types'; + import AlbumOptionsModal from '$lib/modals/AlbumOptionsModal.svelte'; import AlbumShareModal from '$lib/modals/AlbumShareModal.svelte'; import AlbumUsersModal from '$lib/modals/AlbumUsersModal.svelte'; import QrCodeModal from '$lib/modals/QrCodeModal.svelte'; @@ -130,27 +130,6 @@ } }); - const handleToggleEnableActivity = async () => { - try { - const updateAlbum = await updateAlbumInfo({ - id: album.id, - updateAlbumDto: { - isActivityEnabled: !album.isActivityEnabled, - }, - }); - - album = { ...album, isActivityEnabled: updateAlbum.isActivityEnabled }; - - await refreshAlbum(); - notificationController.show({ - type: NotificationType.Info, - message: $t('activity_changed', { values: { enabled: album.isActivityEnabled } }), - }); - } catch (error) { - handleError(error, $t('errors.cant_change_activity', { values: { enabled: album.isActivityEnabled } })); - } - }; - const handleFavorite = async () => { try { await activityManager.toggleLike(); @@ -262,22 +241,6 @@ } }; - const handleRemoveUser = async (userId: string, nextViewMode: AlbumPageViewMode) => { - if (userId == 'me' || userId === $user.id) { - await goto(backUrl); - return; - } - - try { - await refreshAlbum(); - - // Dynamically set the view mode based on the passed argument - viewMode = album.albumUsers.length > 0 ? nextViewMode : AlbumPageViewMode.VIEW; - } catch (error) { - handleError(error, $t('errors.error_deleting_shared_user')); - } - }; - const handleDownloadAlbum = async () => { await downloadAlbum(album); }; @@ -453,6 +416,29 @@ album = await getAlbumInfo({ id: album.id, withoutAssets: true }); } }; + + const handleOptions = async () => { + const result = await modalManager.show(AlbumOptionsModal, { album, order: albumOrder, user: $user }); + + if (!result) { + return; + } + + switch (result.action) { + case 'changeOrder': { + albumOrder = result.order; + break; + } + case 'shareUser': { + await handleShare(); + break; + } + case 'refreshAlbum': { + await refreshAlbum(); + break; + } + } + };
@@ -697,11 +683,7 @@ text={$t('select_album_cover')} onClick={() => (viewMode = AlbumPageViewMode.SELECT_THUMBNAIL)} /> - (viewMode = AlbumPageViewMode.OPTIONS)} - /> + {/if} handleRemoveAlbum()} /> @@ -773,23 +755,6 @@ {/if}
-{#if viewMode === AlbumPageViewMode.OPTIONS && $user} - { - albumOrder = order; - await setModeToView(); - }} - onRemove={(userId) => handleRemoveUser(userId, AlbumPageViewMode.OPTIONS)} - onRefreshAlbum={refreshAlbum} - onClose={() => (viewMode = AlbumPageViewMode.VIEW)} - onToggleEnabledActivity={handleToggleEnableActivity} - onShowSelectSharedUser={handleShare} - /> -{/if} -