diff --git a/.github/workflows/draft-release.yml b/.github/workflows/draft-release.yml index a090c4bff165a3..f17f7922aae538 100644 --- a/.github/workflows/draft-release.yml +++ b/.github/workflows/draft-release.yml @@ -201,9 +201,15 @@ jobs: latest=false fi + discussion=() + if [[ "${TAG}" =~ ^v[0-9]+\.[0-9]+\.0(-rc\.[0-9]+)?$ ]]; then + discussion=(--discussion-category Announcements) + fi + echo "publishing ${TAG} (latest=${latest}, prerelease=${PRERELEASE}, previous=${previous:-none})" gh release edit "${TAG}" \ --repo "${REPO}" \ --draft=false \ --latest="${latest}" \ - --prerelease="${PRERELEASE}" + --prerelease="${PRERELEASE}" \ + "${discussion[@]}" diff --git a/e2e/src/specs/server/api/shared-link.e2e-spec.ts b/e2e/src/specs/server/api/shared-link.e2e-spec.ts index 4e5988a167d590..6fec58c2415c90 100644 --- a/e2e/src/specs/server/api/shared-link.e2e-spec.ts +++ b/e2e/src/specs/server/api/shared-link.e2e-spec.ts @@ -330,6 +330,16 @@ describe('/shared-links', () => { }), ); }); + + it('should create an album shared link when the client sends an empty assetIds array', async () => { + const { status, body } = await request(app) + .post('/shared-links') + .set('Authorization', `Bearer ${user1.accessToken}`) + .send({ type: SharedLinkType.Album, albumId: album.id, assetIds: [] }); + + expect(status).toBe(201); + expect(body).toEqual(expect.objectContaining({ type: SharedLinkType.Album, userId: user1.userId })); + }); }); describe('PATCH /shared-links/:id', () => { diff --git a/mobile/lib/providers/websocket.provider.dart b/mobile/lib/providers/websocket.provider.dart index 32531c354b06de..243c22b54c6132 100644 --- a/mobile/lib/providers/websocket.provider.dart +++ b/mobile/lib/providers/websocket.provider.dart @@ -74,6 +74,7 @@ class WebsocketNotifier extends StateNotifier { socket.onConnect((_) { dPrint(() => "Established Websocket Connection"); state = WebsocketState(isConnected: true, socket: socket); + _refreshServerInfo(); }); socket.onDisconnect((_) { @@ -97,7 +98,7 @@ class WebsocketNotifier extends StateNotifier { socket.on('on_asset_restore', _handleRemoteChange); socket.on('on_asset_hidden', _handleRemoteChange); socket.on('on_asset_update', _handleRemoteChange); - socket.on('on_config_update', _handleOnConfigUpdate); + socket.on('on_config_update', _refreshServerInfo); socket.on('on_new_release', _handleReleaseUpdates); } catch (e) { dPrint(() => "[WEBSOCKET] Catch Websocket Error - $e"); @@ -135,7 +136,7 @@ class WebsocketNotifier extends StateNotifier { ); } - void _handleOnConfigUpdate(dynamic _) { + void _refreshServerInfo([_]) { unawaited(_ref.read(serverInfoProvider.notifier).getServerFeatures()); unawaited(_ref.read(serverInfoProvider.notifier).getServerConfig()); } diff --git a/mobile/lib/repositories/asset_media.repository.dart b/mobile/lib/repositories/asset_media.repository.dart index 320a123ddc5cde..f9a5de3a833929 100644 --- a/mobile/lib/repositories/asset_media.repository.dart +++ b/mobile/lib/repositories/asset_media.repository.dart @@ -22,7 +22,9 @@ import 'package:path/path.dart' as p; import 'package:photo_manager/photo_manager.dart'; import 'package:share_plus/share_plus.dart'; -typedef _ShareFile = ({File file, bool cleanup, String displayName}); +/// A file staged for the share sheet. [tempEntity] is the temp file or +/// directory to delete afterwards, null when [file] is a gallery original. +typedef _ShareFile = ({File file, FileSystemEntity? tempEntity, String displayName}); final assetMediaRepositoryProvider = Provider( (ref) => AssetMediaRepository(ref.watch(nativeSyncApiProvider), ref.watch(storageRepositoryProvider)), @@ -98,12 +100,15 @@ class AssetMediaRepository { } } - /// Deletes temporary files in parallel - Future _cleanupTempFiles(List tempFiles) async { + @protected + @visibleForTesting + Future cleanupTempFiles(List tempFiles) async { await Future.wait( tempFiles.map((file) async { try { - await file.delete(); + if (file.existsSync()) { + await file.delete(recursive: true); + } } catch (e) { _log.warning("Failed to delete temporary file: ${file.path}", e); } @@ -111,27 +116,44 @@ class AssetMediaRepository { ); } - String _sanitizeFilename(String filename) { - return filename.replaceAll(RegExp(r'[\\/]'), '_'); + static final RegExp _pathSeparators = RegExp(r'[\\/]'); + + static String _sanitizeFilename(String filename) { + return filename.replaceAll(_pathSeparators, '_'); + } + + static String _getOriginalShareFilename(BaseAsset asset) { + final hasUsableName = asset.name.replaceAll(_pathSeparators, '').isNotEmpty; + return hasUsableName ? _sanitizeFilename(asset.name) : _shareFallbackName(asset); } - String _getPreviewFilename(BaseAsset asset) { + static String _shareFallbackName(BaseAsset asset) => asset.remoteId ?? asset.localId ?? 'asset'; + + static String _getPreviewFilename(BaseAsset asset) { final sanitizedFilename = _sanitizeFilename(asset.name); final baseName = p.basenameWithoutExtension(sanitizedFilename); - final fallbackName = asset.remoteId ?? asset.localId ?? 'asset'; - return '${baseName.isEmpty ? fallbackName : baseName}-preview.jpg'; + return '${baseName.isEmpty ? _shareFallbackName(asset) : baseName}-preview.jpg'; } + static String _shareDisplayName(BaseAsset asset, ShareAssetType fileType) => + switch (asset.isVideo ? ShareAssetType.original : fileType) { + ShareAssetType.original => _getOriginalShareFilename(asset), + ShareAssetType.preview => _getPreviewFilename(asset), + }; + + static String _ordinalShareFilename(String filename, int occurrence) => + '${p.basenameWithoutExtension(filename)} ($occurrence)${p.extension(filename)}'; + bool _isCancelled(Completer? cancelCompleter) => cancelCompleter?.isCompleted ?? false; - Future<_ShareFile?> _getLocalOriginalShareFile(BaseAsset asset, String localId) async { + Future<_ShareFile?> _getLocalOriginalShareFile(BaseAsset asset, String localId, String displayName) async { final file = await _storageRepository.getFileForAsset(localId); if (file == null) { _log.warning("Local original file not found for sharing: $asset"); return null; } - return (file: file, cleanup: CurrentPlatform.isIOS, displayName: _sanitizeFilename(asset.name)); + return (file: file, tempEntity: CurrentPlatform.isIOS ? file : null, displayName: displayName); } Future<_ShareFile?> _downloadRemoteShareFile({ @@ -145,7 +167,8 @@ class AssetMediaRepository { taskId: taskId, url: url, headers: ApiService.getRequestHeaders(), - filename: '$taskId-$displayName', + filename: displayName, + directory: taskId, baseDirectory: BaseDirectory.temporary, group: kShareDownloadGroup, updates: Updates.statusAndProgress, @@ -162,14 +185,17 @@ class AssetMediaRepository { }, ); + final file = File(await task.filePath()); if (_isCancelled(cancelCompleter)) { + await cleanupTempFiles([file.parent]); return null; } if (statusUpdate.status == TaskStatus.complete) { - return (file: File(await task.filePath()), cleanup: true, displayName: displayName); + return (file: file, tempEntity: file.parent, displayName: displayName); } + await cleanupTempFiles([file.parent]); _log.severe("Download for $displayName failed with status ${statusUpdate.status}", statusUpdate.exception); return null; } @@ -177,13 +203,14 @@ class AssetMediaRepository { Future<_ShareFile?> _getRemoteOriginalShareFile( BaseAsset asset, String remoteId, { + required String displayName, Completer? cancelCompleter, required void Function(double progress) onProgress, }) { return _downloadRemoteShareFile( taskId: 'share-original-$remoteId-${DateTime.now().microsecondsSinceEpoch}', url: getOriginalUrlForRemoteId(remoteId, edited: asset.isEdited), - displayName: _sanitizeFilename(asset.name), + displayName: displayName, cancelCompleter: cancelCompleter, onProgress: onProgress, ); @@ -192,13 +219,14 @@ class AssetMediaRepository { Future<_ShareFile?> _getRemotePreviewShareFile( BaseAsset asset, String remoteId, { + required String displayName, Completer? cancelCompleter, required void Function(double progress) onProgress, }) { return _downloadRemoteShareFile( taskId: 'share-preview-$remoteId-${DateTime.now().microsecondsSinceEpoch}', url: getThumbnailUrlForRemoteId(remoteId, type: AssetMediaSize.preview, edited: asset.isEdited), - displayName: _getPreviewFilename(asset), + displayName: displayName, cancelCompleter: cancelCompleter, onProgress: onProgress, ); @@ -206,12 +234,13 @@ class AssetMediaRepository { Future<_ShareFile?> _getOriginalShareFile( BaseAsset asset, { + required String displayName, Completer? cancelCompleter, required void Function(double progress) onProgress, }) { final localId = asset.localId; if (localId != null && !asset.isEdited) { - return _getLocalOriginalShareFile(asset, localId); + return _getLocalOriginalShareFile(asset, localId, displayName); } final remoteId = asset.remoteId; @@ -220,11 +249,18 @@ class AssetMediaRepository { return Future.value(null); } - return _getRemoteOriginalShareFile(asset, remoteId, cancelCompleter: cancelCompleter, onProgress: onProgress); + return _getRemoteOriginalShareFile( + asset, + remoteId, + displayName: displayName, + cancelCompleter: cancelCompleter, + onProgress: onProgress, + ); } Future<_ShareFile?> _getPreviewShareFile( BaseAsset asset, { + required String displayName, Completer? cancelCompleter, required void Function(double progress) onProgress, }) async { @@ -233,6 +269,7 @@ class AssetMediaRepository { final remotePreview = await _getRemotePreviewShareFile( asset, remoteId, + displayName: displayName, cancelCompleter: cancelCompleter, onProgress: onProgress, ); @@ -243,13 +280,42 @@ class AssetMediaRepository { final localId = asset.localId; if (localId != null) { - return _getLocalOriginalShareFile(asset, localId); + return _getLocalOriginalShareFile(asset, localId, _shareDisplayName(asset, ShareAssetType.original)); } _log.warning("Asset has no local or remote ID for preview sharing: $asset"); return null; } + /// As of share_plus 10.1.4, sharing copies every file into a single cache + /// folder regardless of where it came from, and equal names overwrite each + /// other there. Downloads are renamed to their display name first since + /// receivers only see the on-disk filename, and a name already taken in the + /// batch gets the first free ` (n)` suffix because gallery originals cannot + /// be renamed. + Future _resolveShareFiles(List<_ShareFile> files) async { + final usedNames = { + for (final shareFile in files) + if (shareFile.tempEntity is! Directory) p.basename(shareFile.file.path), + }; + for (var index = 0; index < files.length; index++) { + final shareFile = files[index]; + if (shareFile.tempEntity is! Directory) { + continue; + } + + var occurrence = 0; + var displayName = shareFile.displayName; + while (usedNames.contains(displayName)) { + displayName = _ordinalShareFilename(shareFile.displayName, ++occurrence); + } + + usedNames.add(displayName); + final file = await shareFile.file.rename(p.join(shareFile.file.parent.path, displayName)); + files[index] = (file: file, tempEntity: shareFile.tempEntity, displayName: displayName); + } + } + Future shareAssets( List assets, BuildContext context, { @@ -257,8 +323,8 @@ class AssetMediaRepository { Completer? cancelCompleter, void Function(double progress)? onAssetDownloadProgress, }) async { - final downloadedXFiles = []; - final tempFiles = []; + final shareFiles = <_ShareFile>[]; + final tempFiles = []; final totalAssets = assets.length; var processedAssets = 0; @@ -277,27 +343,34 @@ class AssetMediaRepository { for (final asset in assets) { if (_isCancelled(cancelCompleter)) { - await _cleanupTempFiles(tempFiles); + await cleanupTempFiles(tempFiles); return 0; } final effectiveFileType = asset.isVideo ? ShareAssetType.original : fileType; + final displayName = _shareDisplayName(asset, fileType); final shareFile = switch (effectiveFileType) { ShareAssetType.original => await _getOriginalShareFile( asset, + displayName: displayName, cancelCompleter: cancelCompleter, onProgress: updateProgress, ), ShareAssetType.preview => await _getPreviewShareFile( asset, + displayName: displayName, cancelCompleter: cancelCompleter, onProgress: updateProgress, ), }; + final tempEntity = shareFile?.tempEntity; + if (tempEntity != null) { + tempFiles.add(tempEntity); + } if (_isCancelled(cancelCompleter)) { - await _cleanupTempFiles(tempFiles); + await cleanupTempFiles(tempFiles); return 0; } @@ -307,23 +380,33 @@ class AssetMediaRepository { continue; } - downloadedXFiles.add(XFile(shareFile.file.path, name: shareFile.displayName)); - if (shareFile.cleanup) { - tempFiles.add(shareFile.file); - } + shareFiles.add(shareFile); processedAssets++; updateProgress(); } - if (downloadedXFiles.isEmpty) { + if (shareFiles.isEmpty) { _log.warning("No asset can be retrieved for share"); return 0; } if (_isCancelled(cancelCompleter) || !context.mounted) { - await _cleanupTempFiles(tempFiles); + await cleanupTempFiles(tempFiles); + return 0; + } + + try { + await _resolveShareFiles(shareFiles); + } catch (e, s) { + _log.warning("Failed to prepare files for sharing", e, s); + await cleanupTempFiles(tempFiles); + return 0; + } + if (_isCancelled(cancelCompleter) || !context.mounted) { + await cleanupTempFiles(tempFiles); return 0; } + final downloadedXFiles = shareFiles.map((shareFile) => XFile(shareFile.file.path)).toList(); // we dont want to await the share result since the // "preparing" dialog will not disappear until @@ -332,8 +415,8 @@ class AssetMediaRepository { Share.shareXFiles( downloadedXFiles, sharePositionOrigin: Rect.fromPoints(Offset.zero, Offset(size.width / 3, size.height)), - ).then((result) async { - await _cleanupTempFiles(tempFiles); + ).whenComplete(() async { + await cleanupTempFiles(tempFiles); }), ); diff --git a/mobile/test/repositories/asset_media_repository_test.dart b/mobile/test/repositories/asset_media_repository_test.dart new file mode 100644 index 00000000000000..c94e5aed51d540 --- /dev/null +++ b/mobile/test/repositories/asset_media_repository_test.dart @@ -0,0 +1,262 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:background_downloader/background_downloader.dart'; +import 'package:drift/drift.dart' hide isNotNull, isNull; +import 'package:drift/native.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/constants/enums.dart'; +import 'package:immich_mobile/data/db/main/database.dart'; +import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; +import 'package:immich_mobile/domain/models/store.model.dart'; +import 'package:immich_mobile/domain/services/store.service.dart'; +import 'package:immich_mobile/entities/store.entity.dart'; +import 'package:immich_mobile/infrastructure/repositories/settings.repository.dart'; +import 'package:immich_mobile/infrastructure/repositories/storage.repository.dart'; +import 'package:immich_mobile/infrastructure/repositories/store.repository.dart'; +import 'package:immich_mobile/platform/native_sync_api.g.dart'; +import 'package:immich_mobile/repositories/asset_media.repository.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:path/path.dart' as p; + +import '../test_utils.dart'; + +class _MockNativeSyncApi extends Mock implements NativeSyncApi {} + +class _MockPersistentStorage extends Mock implements PersistentStorage {} + +class _MockStorageRepository extends Mock implements StorageRepository {} + +class _TestAssetMediaRepository extends AssetMediaRepository { + _TestAssetMediaRepository(super.nativeSyncApi, super.storageRepository, this.shareCall); + + final Completer> shareCall; + final cleanups = >[]; + final cleanupAfterShare = Completer>(); + + @override + Future cleanupTempFiles(List tempFiles) async { + cleanups.add(tempFiles); + if (shareCall.isCompleted) { + cleanupAfterShare.complete(tempFiles); + } + } +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late Drift db; + late StoreService store; + late Directory tempRoot; + late _MockStorageRepository storage; + late _TestAssetMediaRepository repository; + late List taskDirs; + late Set failedRemoteIds; + late Completer> shareCall; + + setUpAll(() async { + db = Drift(DatabaseConnection(NativeDatabase.memory(), closeStreamsSynchronously: true)); + store = await StoreService.init(storeRepository: StoreRepository(db), listenUpdates: false); + await SettingsRepository.ensureInitialized(db); + await Store.put(StoreKey.serverEndpoint, 'https://example.com/api'); + // the downloader keeps the storage it gets on its first call. the default one spawns an isolate + // that needs the real path_provider plugin, so a stub answers init and the resume data cleanup + final persistentStorage = _MockPersistentStorage(); + when(persistentStorage.initialize).thenAnswer((_) async {}); + when(() => persistentStorage.removeResumeData(any())).thenAnswer((_) async {}); + when(() => persistentStorage.removePausedTask(any())).thenAnswer((_) async {}); + await FileDownloader(persistentStorage: persistentStorage).ready; + }); + + tearDownAll(() async { + await SettingsRepository.reset(); + await store.dispose(); + await db.close(); + }); + + setUp(() { + tempRoot = Directory.systemTemp.createTempSync('immich-share-test'); + storage = _MockStorageRepository(); + shareCall = Completer>(); + repository = _TestAssetMediaRepository(_MockNativeSyncApi(), storage, shareCall); + taskDirs = []; + failedRemoteIds = {}; + + // keeps every temp path the code asks for inside this test's own folder + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler( + const MethodChannel('plugins.flutter.io/path_provider'), + (_) async => tempRoot.path, + ); + // captures the file paths the share sheet would receive + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler( + const MethodChannel('dev.fluttercommunity.plus/share'), + (call) async { + final arguments = Map.from(call.arguments as Map); + shareCall.complete((arguments['paths']! as List).cast()); + return 'success'; + }, + ); + // stands in for a real download, writes the file and reports the task status + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler( + const MethodChannel('com.bbflight.background_downloader'), + (call) async { + if (call.method != 'enqueue') { + return true; + } + final args = call.arguments! as List; + final task = Task.createFromJsonString(args.first as String) as DownloadTask; + final file = File(await task.filePath()); + await file.parent.create(recursive: true); + await file.writeAsString(task.taskId); + final status = failedRemoteIds.any((id) => task.taskId.contains('-$id-')) + ? TaskStatus.failed + : TaskStatus.complete; + if (status == TaskStatus.complete) { + taskDirs.add(file.parent.path); + } + FileDownloader().downloaderForTesting.processStatusUpdate(TaskStatusUpdate(task, status)); + return true; + }, + ); + }); + + tearDown(() async { + if (tempRoot.existsSync()) { + await tempRoot.delete(recursive: true); + } + }); + + Future<({int count, List names})> share( + WidgetTester tester, + List assets, { + ShareAssetType fileType = ShareAssetType.original, + Completer? cancelCompleter, + TargetPlatform platform = TargetPlatform.android, + }) async { + debugDefaultTargetPlatformOverride = platform; + try { + late BuildContext context; + await tester.pumpWidget( + MaterialApp( + home: Builder( + builder: (ctx) { + context = ctx; + return const SizedBox.shrink(); + }, + ), + ), + ); + + final result = await tester.runAsync(() async { + final count = await repository.shareAssets( + assets, + context, + fileType: fileType, + cancelCompleter: cancelCompleter, + ); + if (cancelCompleter?.isCompleted ?? false) { + return (count: count, names: []); + } + final paths = await shareCall.future; + final cleaned = await repository.cleanupAfterShare.future; + expect(cleaned.map((entity) => entity.path), taskDirs); + return (count: count, names: paths.map(p.basename).toList()); + }); + + return result!; + } finally { + debugDefaultTargetPlatformOverride = null; + } + } + + testWidgets('shares sanitized and fallback names then removes task directories', (tester) async { + final assets = [ + TestUtils.createRemoteAsset(id: 'remote-1').copyWith(name: 'holiday/Photo 1 名字.jpg'), + TestUtils.createRemoteAsset(id: 'remote-2').copyWith(name: ''), + TestUtils.createRemoteAsset(id: 'remote-3').copyWith(name: r'\/'), + ]; + + final result = await share(tester, assets); + + expect(result.count, 3); + expect(result.names, ['holiday_Photo 1 名字.jpg', 'remote-2', 'remote-3']); + }); + + testWidgets('keeps the first copy and uses the first free ordinal for the next', (tester) async { + final assets = [ + TestUtils.createRemoteAsset(id: 'remote-1').copyWith(name: 'IMG.jpg'), + TestUtils.createRemoteAsset(id: 'remote-2').copyWith(name: 'IMG (1).jpg'), + TestUtils.createRemoteAsset(id: 'remote-3').copyWith(name: 'IMG.jpg'), + ]; + + final result = await share(tester, assets); + + expect(result.count, 3); + expect(result.names, ['IMG.jpg', 'IMG (1).jpg', 'IMG (2).jpg']); + }); + + testWidgets('keeps mixed local and remote paths distinct', (tester) async { + final localFile = File(p.join(tempRoot.path, 'local', 'IMG.jpg')); + localFile.parent.createSync(); + localFile.writeAsStringSync('local'); + when(() => storage.getFileForAsset('local-1')).thenAnswer((_) async => localFile); + final assets = [ + TestUtils.createRemoteAsset(id: 'remote-1').copyWith(name: 'IMG.jpg'), + TestUtils.createLocalAsset(id: 'local-1').copyWith(name: 'IMG.jpg'), + ]; + + final result = await share(tester, assets); + + expect(result.count, 2); + expect(result.names, ['IMG (1).jpg', 'IMG.jpg']); + expect(localFile.existsSync(), isTrue); + }); + + testWidgets('does not count failed downloads when adding ordinals', (tester) async { + failedRemoteIds.add('remote-1'); + final assets = [ + TestUtils.createRemoteAsset(id: 'remote-1').copyWith(name: 'IMG.jpg'), + TestUtils.createRemoteAsset(id: 'remote-2').copyWith(name: 'IMG.jpg'), + ]; + + final result = await share(tester, assets); + + expect(result.count, 1); + expect(result.names, ['IMG.jpg']); + }); + + testWidgets('cleans the current iOS temp file when cancelled during retrieval', (tester) async { + final cancellation = Completer(); + final localFile = File(p.join(tempRoot.path, 'local', 'IMG.jpg')); + localFile.parent.createSync(); + localFile.writeAsStringSync('local'); + when(() => storage.getFileForAsset('local-1')).thenAnswer((_) async { + cancellation.complete(); + return localFile; + }); + final asset = TestUtils.createLocalAsset(id: 'local-1').copyWith(name: 'IMG.jpg'); + + final result = await share(tester, [asset], cancelCompleter: cancellation, platform: TargetPlatform.iOS); + + expect(result.count, 0); + expect(result.names, isEmpty); + expect(shareCall.isCompleted, isFalse); + expect(repository.cleanups.singleOrNull, [localFile]); + }); + + testWidgets('adds an ordinal when preview and video names match', (tester) async { + final assets = [ + TestUtils.createRemoteAsset(id: 'remote-1').copyWith(name: 'IMG.jpg'), + TestUtils.createRemoteAsset(id: 'remote-2').copyWith(name: 'IMG-preview.jpg', type: AssetType.video), + ]; + + final result = await share(tester, assets, fileType: ShareAssetType.preview); + + expect(result.count, 2); + expect(result.names, ['IMG-preview.jpg', 'IMG-preview (1).jpg']); + }); +} diff --git a/server/bin/immich-dev b/server/bin/immich-dev index 84c5eea8da00fb..6febb21481f59b 100755 --- a/server/bin/immich-dev +++ b/server/bin/immich-dev @@ -6,4 +6,5 @@ if [[ "$IMMICH_ENV" == "production" ]]; then fi cd /usr/src/app || exit +pnpm --filter @immich/plugin-sdk build pnpm --filter immich exec nest start --debug "0.0.0.0:9230" --watch -- "$@" diff --git a/server/src/controllers/shared-link.controller.spec.ts b/server/src/controllers/shared-link.controller.spec.ts index 856a74e340359b..ee5c3c69c647ef 100644 --- a/server/src/controllers/shared-link.controller.spec.ts +++ b/server/src/controllers/shared-link.controller.spec.ts @@ -77,6 +77,17 @@ describe(SharedLinkController.name, () => { ); expect(service.create).not.toHaveBeenCalled(); }); + + it('should allow an empty assetIds array for share type Album', async () => { + const albumId = newUuid(); + await request(ctx.getHttpServer()) + .post('/shared-links') + .send({ type: SharedLinkType.Album, albumId, assetIds: [] }); + expect(service.create).toHaveBeenCalledWith( + undefined, + expect.objectContaining({ type: SharedLinkType.Album, albumId }), + ); + }); }); describe('DELETE /shared-links/:id/assets', () => { diff --git a/server/src/dtos/shared-link.dto.ts b/server/src/dtos/shared-link.dto.ts index d22875f28637fc..a54b3afc4abec3 100644 --- a/server/src/dtos/shared-link.dto.ts +++ b/server/src/dtos/shared-link.dto.ts @@ -37,7 +37,7 @@ const SharedLinkCreateSchema = z if (!albumId) { ctx.addIssue(`albumId is required for type ${SharedLinkType.Album}`); } - if (assetIds) { + if (assetIds && assetIds.length > 0) { ctx.addIssue(`assetIds can only be used with type ${SharedLinkType.Individual}`); } return; diff --git a/server/src/queries/person.repository.sql b/server/src/queries/person.repository.sql index 8f07b918009338..821041c5c1e246 100644 --- a/server/src/queries/person.repository.sql +++ b/server/src/queries/person.repository.sql @@ -4,8 +4,11 @@ update "asset_face" set "personGroupId" = $1 +from + "asset" where - "asset_face"."personGroupId" = $2 + "asset_face"."assetId" = "asset"."id" + and "asset_face"."personGroupId" = $2 -- PersonRepository.unassignFaces update "asset_face" diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index caa3d79e527e54..3e6e6d13759078 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -100,17 +100,15 @@ export class PersonRepository { async reassignFaces({ oldPersonGroupId, faceIds, ownerId, newPersonGroupId }: UpdateFacesData): Promise { const result = await this.db .updateTable('asset_face') + .from('asset') + .whereRef('asset_face.assetId', '=', 'asset.id') .set({ personGroupId: newPersonGroupId }) .$if(!!oldPersonGroupId, (qb) => qb.where('asset_face.personGroupId', '=', oldPersonGroupId!)) .$if(!!faceIds, (qb) => qb.where('asset_face.id', 'in', faceIds!)) - .$if(!!ownerId, (qb) => - qb.where('asset_face.personGroupId', 'in', (eb) => - eb.selectFrom('person').select('person.personGroupId').where('person.ownerId', '=', ownerId!), - ), - ) + .$if(!!ownerId, (qb) => qb.where('asset.ownerId', '=', ownerId!)) .executeTakeFirst(); - return Number(result.numChangedRows ?? 0); + return Number(result.numUpdatedRows ?? 0); } @GenerateSql({ params: [{ sourceType: SourceType.MachineLearning, clusterGroupId: DummyValue.UUID }] }) diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index f4740a42bc2f81..dcb0c73beabfa8 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -441,7 +441,6 @@ export class PersonService extends BaseService { const { waiting } = await this.jobRepository.getJobCounts(QueueName.FacialRecognition); if (force) { - console.log('unassigning faces'); await this.personRepository.unassignFaces({ clusterGroupId, sourceType: SourceType.MachineLearning }); await this.handlePersonCleanup(); await this.personRepository.vacuum({ reindexVectors: false }); diff --git a/server/test/medium/specs/services/person.service.spec.ts b/server/test/medium/specs/services/person.service.spec.ts index 860406fa72055e..2adefe7e7a4b8b 100644 --- a/server/test/medium/specs/services/person.service.spec.ts +++ b/server/test/medium/specs/services/person.service.spec.ts @@ -182,6 +182,8 @@ describe(PersonService.name, () => { ownerId: user2.id, personGroupId: person2.personGroupId, }); + const { asset } = await ctx.newAsset({ ownerId: user2.id }); + await ctx.newAssetFace({ assetId: asset.id, personGroupId: person2.personGroupId }); storageMock.unlink.mockResolvedValue(); const auth = factory.auth({ user: user1 }); @@ -191,6 +193,9 @@ describe(PersonService.name, () => { const user2People = await Array.fromAsync(ctx.get(PersonRepository).getAll({ ownerId: user2.id })); expect(user1People).toEqual([expect.objectContaining({ personGroupId: person1.personGroupId })]); expect(user2People).toEqual([expect.objectContaining({ personGroupId: person1.personGroupId })]); + await expect(ctx.get(PersonRepository).getFaces(asset.id, { viewingUserId: asset.ownerId })).resolves.toEqual([ + expect.objectContaining({ personGroupId: person1.personGroupId }), + ]); }); it('should skip people with a different name', async () => { @@ -258,6 +263,33 @@ describe(PersonService.name, () => { ]), ); }); + + it('should not merge into person another user does not have', async () => { + const { sut, ctx } = setup(); + const storageMock = ctx.getMock(StorageRepository); + const { user: user1 } = await ctx.newUser(); + const { user: user2 } = await ctx.newUser({ clusterGroupId: user1.clusterGroupId }); + const { person: person1 } = await ctx.newPerson({ ownerId: user1.id }); + const { person: person2 } = await ctx.newPerson({ ownerId: user1.id }); + await ctx.newPerson({ + ownerId: user2.id, + personGroupId: person2.personGroupId, + }); + const { asset } = await ctx.newAsset({ ownerId: user2.id }); + await ctx.newAssetFace({ assetId: asset.id, personGroupId: person2.personGroupId }); + storageMock.unlink.mockResolvedValue(); + + const auth = factory.auth({ user: user1 }); + + await sut.mergePerson(auth, person1.personGroupId, { ids: [person2.personGroupId] }); + const user1People = await Array.fromAsync(ctx.get(PersonRepository).getAll({ ownerId: user1.id })); + const user2People = await Array.fromAsync(ctx.get(PersonRepository).getAll({ ownerId: user2.id })); + expect(user1People).toEqual([expect.objectContaining({ personGroupId: person1.personGroupId })]); + expect(user2People).toEqual([expect.objectContaining({ personGroupId: person2.personGroupId })]); + await expect(ctx.get(PersonRepository).getFaces(asset.id, { viewingUserId: asset.ownerId })).resolves.toEqual([ + expect.objectContaining({ personGroupId: person2.personGroupId }), + ]); + }); }); describe('createFace', () => { diff --git a/web/src/lib/components/shared-components/map/Map.svelte b/web/src/lib/components/shared-components/map/Map.svelte index 4092a4aa986784..abdc17c8ed5133 100644 --- a/web/src/lib/components/shared-components/map/Map.svelte +++ b/web/src/lib/components/shared-components/map/Map.svelte @@ -400,7 +400,7 @@ (viewportGridActive ? onViewportClose?.() : handleViewportSelect())}> - + @@ -414,7 +414,7 @@ - + diff --git a/web/src/lib/components/shared-components/search-bar/SearchBar.svelte b/web/src/lib/components/shared-components/search-bar/SearchBar.svelte index a126a7526dfa7c..0e9c18a423768d 100644 --- a/web/src/lib/components/shared-components/search-bar/SearchBar.svelte +++ b/web/src/lib/components/shared-components/search-bar/SearchBar.svelte @@ -11,7 +11,7 @@ import { t } from 'svelte-i18n'; import SearchFilters from './SearchFilters.svelte'; import { searchManager } from '$lib/managers/search-manager.svelte'; - import { getSearchTypePlaceholder } from './search-bar-utils'; + import { getSearchTypePlaceholder, isPopoverContent } from './search-bar-utils'; type Props = { grayTheme: boolean; @@ -67,10 +67,22 @@ searchStore.isSearchEnabled = true; }; - const onFocusOut = () => { + const onFocusOut = (event: FocusEvent) => { + if (isPopoverContent(event)) { + return; + } + searchStore.isSearchEnabled = false; }; + const onDropdownFocusOut = (event: FocusEvent) => { + if (isPopoverContent(event)) { + return; + } + + closeDropdown(); + }; + const onHistoryTermClick = async (searchTerm: string) => { searchManager.filter.query = searchTerm; await handleSearch(); @@ -138,7 +150,7 @@ onfocusin={onFocusIn} role="search" > -
+
({ + id, + name: value, + value, + createdAt: '2026-01-01T00:00:00Z', + updatedAt: '2026-01-01T00:00:00Z', +}); + +describe('SearchTagsSection component', () => { + beforeEach(() => { + vi.stubGlobal('IntersectionObserver', getIntersectionObserverMock()); + vi.stubGlobal('visualViewport', getVisualViewportMock()); + authManager.setUser(userAdminFactory.build()); + authManager.setPreferences(preferencesFactory.build({ tags: { enabled: true, sidebarWeb: true } })); + searchManager.setQuery({ tagIds: ['tag-1', 'tag-2'] }); + }); + + afterEach(() => { + authManager.reset(); + searchManager.reset(); + }); + + it('removes the tag when its chip is clicked', async () => { + const user = userEvent.setup(); + render(SearchTagsSection, { + props: { title: undefined, parentPromise: Promise.resolve([tag('tag-1', 'holiday'), tag('tag-2', 'family')]) }, + }); + + await user.click(await screen.findByRole('button', { name: 'holiday' })); + + expect(screen.queryByRole('button', { name: 'holiday' })).not.toBeInTheDocument(); + expect([...(searchManager.filter.tagIds ?? [])]).toEqual(['tag-2']); + }); + + // removing the focused chip would otherwise drop focus to the body, which the + // search bar treats as focus leaving the search panel + it('keeps focus inside the section when a chip is removed', async () => { + const user = userEvent.setup(); + const { container } = render(SearchTagsSection, { + props: { title: undefined, parentPromise: Promise.resolve([tag('tag-1', 'holiday'), tag('tag-2', 'family')]) }, + }); + + await user.click(await screen.findByRole('button', { name: 'holiday' })); + + expect(document.activeElement).not.toBe(document.body); + expect(container.contains(document.activeElement)).toBe(true); + }); +}); diff --git a/web/src/lib/components/shared-components/search-bar/SearchTagsSection.svelte b/web/src/lib/components/shared-components/search-bar/SearchTagsSection.svelte index 2579a13a21bf31..d2512554bb51da 100644 --- a/web/src/lib/components/shared-components/search-bar/SearchTagsSection.svelte +++ b/web/src/lib/components/shared-components/search-bar/SearchTagsSection.svelte @@ -17,6 +17,7 @@ // eslint-disable-next-line no-useless-assignment let { title = $bindable(), parentPromise }: Props = $props(); + let container = $state(); let selectedTags = $derived(searchManager.filter.tagIds); let allTags: TagResponseDto[] = $state([]); let tagMap = $derived(Object.fromEntries(allTags.map((tag) => [tag.id, tag]))); @@ -45,13 +46,15 @@ return; } + // Move focus back to the container so it doesn't fallback to the body and closes the search bar + container?.focus(); selectedTags.delete(tag); title = getSearchTagsTitle(allTags, selectedTags); }; {#if authManager.authenticated && authManager.preferences.tags.enabled} -
+
{$t('search_filter_tags_description')} { + const focusOutEventTo = (relatedTarget: EventTarget | null) => new FocusEvent('focusout', { relatedTarget }); + + const createCalendarPopup = () => { + const popup = document.createElement('div'); + popup.dataset.popoverContent = ''; + return popup; + }; + + it('returns true when focus moves to an element inside a calendar popup', () => { + const popup = createCalendarPopup(); + const dayButton = document.createElement('button'); + popup.append(dayButton); + + expect(isPopoverContent(focusOutEventTo(dayButton))).toBe(true); + }); + + it('returns true when focus moves to the calendar popup itself', () => { + const popup = createCalendarPopup(); + + expect(isPopoverContent(focusOutEventTo(popup))).toBe(true); + }); + + it('returns false when focus moves to an element outside a calendar popup', () => { + const button = document.createElement('button'); + + expect(isPopoverContent(focusOutEventTo(button))).toBe(false); + }); + + it('returns false when focus does not move to another element', () => { + expect(isPopoverContent(focusOutEventTo(null))).toBe(false); + }); +}); diff --git a/web/src/lib/components/shared-components/search-bar/search-bar-utils.ts b/web/src/lib/components/shared-components/search-bar/search-bar-utils.ts index 9ae4fe0df9ef27..7b56d1a2557bcd 100644 --- a/web/src/lib/components/shared-components/search-bar/search-bar-utils.ts +++ b/web/src/lib/components/shared-components/search-bar/search-bar-utils.ts @@ -179,3 +179,8 @@ export const getSearchTagsTitle = (tags: TagResponseDto[], selected: SvelteSet { + const element = event.relatedTarget; + return element instanceof Element && element.closest('[data-popover-content]') !== null; +};