From 4110058c448d8febee6caeddcc0a4c45263c1aeb Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Wed, 5 Aug 2026 16:05:27 +0600 Subject: [PATCH 1/3] wire download status to the ui and dismiss finished entries --- .../asset_viewer/download.provider.dart | 49 +++++++ .../lib/repositories/download.repository.dart | 3 + mobile/lib/services/download.service.dart | 7 + .../asset_viewer/download_provider_test.dart | 125 ++++++++++++++++++ 4 files changed, 184 insertions(+) create mode 100644 mobile/test/providers/asset_viewer/download_provider_test.dart diff --git a/mobile/lib/providers/asset_viewer/download.provider.dart b/mobile/lib/providers/asset_viewer/download.provider.dart index 2c4854bdb0..d96c52df30 100644 --- a/mobile/lib/providers/asset_viewer/download.provider.dart +++ b/mobile/lib/providers/asset_viewer/download.provider.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:background_downloader/background_downloader.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/models/download/download_state.model.dart'; @@ -14,9 +16,38 @@ class DownloadStateNotifier extends StateNotifier { taskProgress: {}, ), ) { + _downloadService.onImageDownloadStatus = _downloadStatusCallback; + _downloadService.onVideoDownloadStatus = _downloadStatusCallback; + _downloadService.onLivePhotoDownloadStatus = _downloadStatusCallback; _downloadService.onTaskProgress = _taskProgressCallback; } + void _updateDownloadStatus(String taskId, TaskStatus status) { + if (status == TaskStatus.canceled) { + return; + } + + state = state.copyWith( + taskProgress: {} + ..addAll(state.taskProgress) + ..addAll({ + taskId: DownloadInfo( + progress: state.taskProgress[taskId]?.progress ?? 0, + fileName: state.taskProgress[taskId]?.fileName ?? '', + status: status, + ), + }), + ); + } + + void _downloadStatusCallback(TaskStatusUpdate update) { + _updateDownloadStatus(update.task.taskId, update.status); + + if (update.status == TaskStatus.complete) { + _onDownloadComplete(update.task.taskId); + } + } + void _taskProgressCallback(TaskProgressUpdate update) { // Ignore if the task is canceled or completed if (update.progress == -2 || update.progress == -1) { @@ -37,6 +68,24 @@ class DownloadStateNotifier extends StateNotifier { ); } + void _onDownloadComplete(String id) { + Future.delayed(const Duration(seconds: 2), () { + if (!mounted) { + return; + } + + state = state.copyWith( + taskProgress: {} + ..addAll(state.taskProgress) + ..remove(id), + ); + + if (state.taskProgress.isEmpty) { + state = state.copyWith(showProgress: false); + } + }); + } + Future cancelDownload(String id) async { final isCanceled = await _downloadService.cancelDownload(id); diff --git a/mobile/lib/repositories/download.repository.dart b/mobile/lib/repositories/download.repository.dart index 855b8302a9..8504487860 100644 --- a/mobile/lib/repositories/download.repository.dart +++ b/mobile/lib/repositories/download.repository.dart @@ -28,6 +28,8 @@ class DownloadRepository { void Function(TaskStatusUpdate)? onVideoDownloadStatus; + void Function(TaskStatusUpdate)? onLivePhotoDownloadStatus; + void Function(TaskProgressUpdate)? onTaskProgress; // #29900: `taskStatusCallback` is called before the DB has been updated, causing a race between the two Live Photo tasks @@ -49,6 +51,7 @@ class DownloadRepository { _downloader.registerCallbacks( group: kDownloadGroupLivePhoto, + taskStatusCallback: (update) => onLivePhotoDownloadStatus?.call(update), taskProgressCallback: (update) => onTaskProgress?.call(update), ); diff --git a/mobile/lib/services/download.service.dart b/mobile/lib/services/download.service.dart index f38b20cc21..e46258361b 100644 --- a/mobile/lib/services/download.service.dart +++ b/mobile/lib/services/download.service.dart @@ -22,6 +22,7 @@ class DownloadService { final Logger _log = Logger("DownloadService"); void Function(TaskStatusUpdate)? onImageDownloadStatus; void Function(TaskStatusUpdate)? onVideoDownloadStatus; + void Function(TaskStatusUpdate)? onLivePhotoDownloadStatus; void Function(TaskProgressUpdate)? onTaskProgress; /// Active Live Photo IDs undergoing saving @@ -30,6 +31,7 @@ class DownloadService { DownloadService(this._fileMediaRepository, this._downloadRepository) { _downloadRepository.onImageDownloadStatus = _onImageDownloadCallback; _downloadRepository.onVideoDownloadStatus = _onVideoDownloadCallback; + _downloadRepository.onLivePhotoDownloadStatus = _onLivePhotoDownloadCallback; _downloadRepository.onTaskProgress = _onTaskProgressCallback; _downloadRepository.onLivePhotoRecordComplete = _onLivePhotoRecordComplete; @@ -65,6 +67,11 @@ class DownloadService { onVideoDownloadStatus?.call(update); } + // UI-only; saves stay on the DB record stream (#29900) + void _onLivePhotoDownloadCallback(TaskStatusUpdate update) { + onLivePhotoDownloadStatus?.call(update); + } + Future _onLivePhotoRecordComplete(TaskRecord record) async { final livePhotosId = LivePhotosMetadata.fromJson(record.task.metaData).id; await _saveLivePhotos(livePhotosId); diff --git a/mobile/test/providers/asset_viewer/download_provider_test.dart b/mobile/test/providers/asset_viewer/download_provider_test.dart new file mode 100644 index 0000000000..209e5fb808 --- /dev/null +++ b/mobile/test/providers/asset_viewer/download_provider_test.dart @@ -0,0 +1,125 @@ +import 'package:background_downloader/background_downloader.dart'; +import 'package:fake_async/fake_async.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/models/download/livephotos_medatada.model.dart'; +import 'package:immich_mobile/providers/asset_viewer/download.provider.dart'; +import 'package:immich_mobile/services/download.service.dart'; +import 'package:mocktail/mocktail.dart'; + +class MockDownloadService extends Mock implements DownloadService {} + +DownloadTask _task(String id, {String filename = 'photo.jpg', String metaData = ''}) => + DownloadTask(taskId: id, url: 'https://example.com/$filename', filename: filename, metaData: metaData); + +void main() { + late MockDownloadService service; + late DownloadStateNotifier notifier; + late void Function(TaskProgressUpdate) onProgress; + late void Function(TaskStatusUpdate) onImage; + late void Function(TaskStatusUpdate) onLivePhoto; + + setUp(() { + service = MockDownloadService(); + notifier = DownloadStateNotifier(service); + addTearDown(() { + if (notifier.mounted) { + notifier.dispose(); + } + }); + + onProgress = verify(() => service.onTaskProgress = captureAny()).captured.last as void Function(TaskProgressUpdate); + onImage = + verify(() => service.onImageDownloadStatus = captureAny()).captured.last as void Function(TaskStatusUpdate); + onLivePhoto = + verify(() => service.onLivePhotoDownloadStatus = captureAny()).captured.last as void Function(TaskStatusUpdate); + }); + + test('complete flips the entry then removes it after the delay', () { + fakeAsync((async) { + final task = _task('task-1'); + onProgress(TaskProgressUpdate(task, 0.4)); + onImage(TaskStatusUpdate(task, TaskStatus.complete)); + + expect(notifier.state.taskProgress['task-1']?.status, TaskStatus.complete); + expect(notifier.state.showProgress, isTrue); + + async.elapse(const Duration(seconds: 2)); + + expect(notifier.state.taskProgress, isEmpty); + expect(notifier.state.showProgress, isFalse); + }); + }); + + test('failed keeps the entry visible', () { + fakeAsync((async) { + final task = _task('task-1'); + onProgress(TaskProgressUpdate(task, 0.4)); + onImage(TaskStatusUpdate(task, TaskStatus.failed)); + + expect(notifier.state.taskProgress['task-1']?.status, TaskStatus.failed); + + async.elapse(const Duration(seconds: 5)); + + expect(notifier.state.taskProgress['task-1']?.status, TaskStatus.failed); + expect(notifier.state.showProgress, isTrue); + }); + }); + + test('a live photo part completion removes that part entry', () { + fakeAsync((async) { + final image = _task( + 'live-image', + metaData: LivePhotosMetadata(part: LivePhotosPart.image, id: 'live-1').toJson(), + ); + final video = _task( + 'live-video', + filename: 'photo.MOV', + metaData: LivePhotosMetadata(part: LivePhotosPart.video, id: 'live-1').toJson(), + ); + onProgress(TaskProgressUpdate(image, 0.9)); + onProgress(TaskProgressUpdate(video, 0.9)); + onLivePhoto(TaskStatusUpdate(image, TaskStatus.complete)); + + async.elapse(const Duration(seconds: 2)); + + expect(notifier.state.taskProgress.containsKey('live-image'), isFalse); + expect(notifier.state.taskProgress.containsKey('live-video'), isTrue); + expect(notifier.state.showProgress, isTrue); + }); + }); + + test('canceled does not resurrect or alter the entry', () { + fakeAsync((async) { + final task = _task('task-1'); + onProgress(TaskProgressUpdate(task, 0.4)); + onImage(TaskStatusUpdate(task, TaskStatus.canceled)); + + expect(notifier.state.taskProgress['task-1']?.status, TaskStatus.running); + expect(notifier.state.taskProgress['task-1']?.progress, 0.4); + + onImage(TaskStatusUpdate(_task('ghost'), TaskStatus.canceled)); + expect(notifier.state.taskProgress.containsKey('ghost'), isFalse); + }); + }); + + test('showProgress clears when the last entry is removed', () { + fakeAsync((async) { + final a = _task('a'); + final b = _task('b'); + onProgress(TaskProgressUpdate(a, 1.0)); + onProgress(TaskProgressUpdate(b, 1.0)); + onImage(TaskStatusUpdate(a, TaskStatus.complete)); + + async.elapse(const Duration(seconds: 2)); + + expect(notifier.state.showProgress, isTrue); + + onImage(TaskStatusUpdate(b, TaskStatus.complete)); + + async.elapse(const Duration(seconds: 2)); + + expect(notifier.state.taskProgress, isEmpty); + expect(notifier.state.showProgress, isFalse); + }); + }); +} From 78b352f16f10064e946656f16f6ba2838b67e3cd Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Wed, 5 Aug 2026 20:41:46 +0600 Subject: [PATCH 2/3] mark finished downloads with a checkmark instead of dismissing --- mobile/lib/pages/common/download_panel.dart | 12 +++++- .../asset_viewer/download.provider.dart | 24 ------------ .../pages/common/download_panel_test.dart | 24 ++++++++++++ .../asset_viewer/download_provider_test.dart | 37 ++++--------------- 4 files changed, 42 insertions(+), 55 deletions(-) create mode 100644 mobile/test/pages/common/download_panel_test.dart diff --git a/mobile/lib/pages/common/download_panel.dart b/mobile/lib/pages/common/download_panel.dart index 2015948fad..ccaac0c9e2 100644 --- a/mobile/lib/pages/common/download_panel.dart +++ b/mobile/lib/pages/common/download_panel.dart @@ -64,6 +64,8 @@ class DownloadTaskTile extends StatelessWidget { @override Widget build(BuildContext context) { + final isComplete = status == TaskStatus.complete; + final doneColor = context.isDarkTheme ? Colors.green[200] : Colors.green[400]; final progressPercent = (progress * 100).round(); String getStatusText() => switch (status) { @@ -88,9 +90,15 @@ class DownloadTaskTile extends StatelessWidget { leading: const Icon(Icons.video_file_outlined), title: Text(getStatusText(), style: context.textTheme.labelLarge), trailing: IconButton( - icon: Icon(Icons.close, color: context.colorScheme.onError), + icon: Icon( + isComplete ? Icons.download_done : Icons.close, + color: isComplete ? doneColor : context.colorScheme.onError, + size: isComplete ? 28 : null, + ), onPressed: onCancelDownload, - style: ElevatedButton.styleFrom(backgroundColor: context.colorScheme.error.withAlpha(200)), + style: isComplete + ? null + : ElevatedButton.styleFrom(backgroundColor: context.colorScheme.error.withAlpha(200)), ), subtitle: Column( crossAxisAlignment: CrossAxisAlignment.start, diff --git a/mobile/lib/providers/asset_viewer/download.provider.dart b/mobile/lib/providers/asset_viewer/download.provider.dart index d96c52df30..a55dcf4564 100644 --- a/mobile/lib/providers/asset_viewer/download.provider.dart +++ b/mobile/lib/providers/asset_viewer/download.provider.dart @@ -1,5 +1,3 @@ -import 'dart:async'; - import 'package:background_downloader/background_downloader.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/models/download/download_state.model.dart'; @@ -42,10 +40,6 @@ class DownloadStateNotifier extends StateNotifier { void _downloadStatusCallback(TaskStatusUpdate update) { _updateDownloadStatus(update.task.taskId, update.status); - - if (update.status == TaskStatus.complete) { - _onDownloadComplete(update.task.taskId); - } } void _taskProgressCallback(TaskProgressUpdate update) { @@ -68,24 +62,6 @@ class DownloadStateNotifier extends StateNotifier { ); } - void _onDownloadComplete(String id) { - Future.delayed(const Duration(seconds: 2), () { - if (!mounted) { - return; - } - - state = state.copyWith( - taskProgress: {} - ..addAll(state.taskProgress) - ..remove(id), - ); - - if (state.taskProgress.isEmpty) { - state = state.copyWith(showProgress: false); - } - }); - } - Future cancelDownload(String id) async { final isCanceled = await _downloadService.cancelDownload(id); diff --git a/mobile/test/pages/common/download_panel_test.dart b/mobile/test/pages/common/download_panel_test.dart new file mode 100644 index 0000000000..f2a6b09d88 --- /dev/null +++ b/mobile/test/pages/common/download_panel_test.dart @@ -0,0 +1,24 @@ +import 'package:background_downloader/background_downloader.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/pages/common/download_panel.dart'; + +import '../../widget_tester_extensions.dart'; + +void main() { + testWidgets('complete uses a checkmark and other statuses use close', (tester) async { + await tester.pumpConsumerWidget( + DownloadTaskTile(progress: 1, fileName: 'photo.jpg', status: TaskStatus.complete, onCancelDownload: () {}), + ); + + expect(find.byIcon(Icons.download_done), findsOneWidget); + expect(find.byIcon(Icons.close), findsNothing); + + await tester.pumpConsumerWidget( + DownloadTaskTile(progress: 1, fileName: 'photo.jpg', status: TaskStatus.failed, onCancelDownload: () {}), + ); + + expect(find.byIcon(Icons.download_done), findsNothing); + expect(find.byIcon(Icons.close), findsOneWidget); + }); +} diff --git a/mobile/test/providers/asset_viewer/download_provider_test.dart b/mobile/test/providers/asset_viewer/download_provider_test.dart index 209e5fb808..bfccd29080 100644 --- a/mobile/test/providers/asset_viewer/download_provider_test.dart +++ b/mobile/test/providers/asset_viewer/download_provider_test.dart @@ -34,7 +34,7 @@ void main() { verify(() => service.onLivePhotoDownloadStatus = captureAny()).captured.last as void Function(TaskStatusUpdate); }); - test('complete flips the entry then removes it after the delay', () { + test('complete flips the entry and keeps it visible', () { fakeAsync((async) { final task = _task('task-1'); onProgress(TaskProgressUpdate(task, 0.4)); @@ -43,10 +43,10 @@ void main() { expect(notifier.state.taskProgress['task-1']?.status, TaskStatus.complete); expect(notifier.state.showProgress, isTrue); - async.elapse(const Duration(seconds: 2)); + async.elapse(const Duration(seconds: 5)); - expect(notifier.state.taskProgress, isEmpty); - expect(notifier.state.showProgress, isFalse); + expect(notifier.state.taskProgress['task-1']?.status, TaskStatus.complete); + expect(notifier.state.showProgress, isTrue); }); }); @@ -65,7 +65,7 @@ void main() { }); }); - test('a live photo part completion removes that part entry', () { + test('a live photo part completion keeps both entries', () { fakeAsync((async) { final image = _task( 'live-image', @@ -80,10 +80,10 @@ void main() { onProgress(TaskProgressUpdate(video, 0.9)); onLivePhoto(TaskStatusUpdate(image, TaskStatus.complete)); - async.elapse(const Duration(seconds: 2)); + async.elapse(const Duration(seconds: 5)); - expect(notifier.state.taskProgress.containsKey('live-image'), isFalse); - expect(notifier.state.taskProgress.containsKey('live-video'), isTrue); + expect(notifier.state.taskProgress['live-image']?.status, TaskStatus.complete); + expect(notifier.state.taskProgress['live-video']?.status, TaskStatus.running); expect(notifier.state.showProgress, isTrue); }); }); @@ -101,25 +101,4 @@ void main() { expect(notifier.state.taskProgress.containsKey('ghost'), isFalse); }); }); - - test('showProgress clears when the last entry is removed', () { - fakeAsync((async) { - final a = _task('a'); - final b = _task('b'); - onProgress(TaskProgressUpdate(a, 1.0)); - onProgress(TaskProgressUpdate(b, 1.0)); - onImage(TaskStatusUpdate(a, TaskStatus.complete)); - - async.elapse(const Duration(seconds: 2)); - - expect(notifier.state.showProgress, isTrue); - - onImage(TaskStatusUpdate(b, TaskStatus.complete)); - - async.elapse(const Duration(seconds: 2)); - - expect(notifier.state.taskProgress, isEmpty); - expect(notifier.state.showProgress, isFalse); - }); - }); } From 4706888d101eb793fd52f41c88981f4c10e53070 Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Tue, 11 Aug 2026 12:49:23 +0600 Subject: [PATCH 3/3] use the theme color for the done icon and guard status updates on a known task --- mobile/lib/pages/common/download_panel.dart | 4 +--- .../asset_viewer/download.provider.dart | 21 +++++++------------ mobile/lib/services/download.service.dart | 1 - .../asset_viewer/download_provider_test.dart | 9 ++++++++ 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/mobile/lib/pages/common/download_panel.dart b/mobile/lib/pages/common/download_panel.dart index ccaac0c9e2..596e6913d0 100644 --- a/mobile/lib/pages/common/download_panel.dart +++ b/mobile/lib/pages/common/download_panel.dart @@ -65,7 +65,6 @@ class DownloadTaskTile extends StatelessWidget { @override Widget build(BuildContext context) { final isComplete = status == TaskStatus.complete; - final doneColor = context.isDarkTheme ? Colors.green[200] : Colors.green[400]; final progressPercent = (progress * 100).round(); String getStatusText() => switch (status) { @@ -92,8 +91,7 @@ class DownloadTaskTile extends StatelessWidget { trailing: IconButton( icon: Icon( isComplete ? Icons.download_done : Icons.close, - color: isComplete ? doneColor : context.colorScheme.onError, - size: isComplete ? 28 : null, + color: isComplete ? context.colorScheme.primary : context.colorScheme.onError, ), onPressed: onCancelDownload, style: isComplete diff --git a/mobile/lib/providers/asset_viewer/download.provider.dart b/mobile/lib/providers/asset_viewer/download.provider.dart index a55dcf4564..ba72ef60c5 100644 --- a/mobile/lib/providers/asset_viewer/download.provider.dart +++ b/mobile/lib/providers/asset_viewer/download.provider.dart @@ -20,28 +20,23 @@ class DownloadStateNotifier extends StateNotifier { _downloadService.onTaskProgress = _taskProgressCallback; } - void _updateDownloadStatus(String taskId, TaskStatus status) { - if (status == TaskStatus.canceled) { + void _downloadStatusCallback(TaskStatusUpdate update) { + if (update.status == TaskStatus.canceled) { + return; + } + + final existing = state.taskProgress[update.task.taskId]; + if (existing == null) { return; } state = state.copyWith( taskProgress: {} ..addAll(state.taskProgress) - ..addAll({ - taskId: DownloadInfo( - progress: state.taskProgress[taskId]?.progress ?? 0, - fileName: state.taskProgress[taskId]?.fileName ?? '', - status: status, - ), - }), + ..addAll({update.task.taskId: existing.copyWith(status: update.status)}), ); } - void _downloadStatusCallback(TaskStatusUpdate update) { - _updateDownloadStatus(update.task.taskId, update.status); - } - void _taskProgressCallback(TaskProgressUpdate update) { // Ignore if the task is canceled or completed if (update.progress == -2 || update.progress == -1) { diff --git a/mobile/lib/services/download.service.dart b/mobile/lib/services/download.service.dart index e46258361b..09a56f0ce9 100644 --- a/mobile/lib/services/download.service.dart +++ b/mobile/lib/services/download.service.dart @@ -67,7 +67,6 @@ class DownloadService { onVideoDownloadStatus?.call(update); } - // UI-only; saves stay on the DB record stream (#29900) void _onLivePhotoDownloadCallback(TaskStatusUpdate update) { onLivePhotoDownloadStatus?.call(update); } diff --git a/mobile/test/providers/asset_viewer/download_provider_test.dart b/mobile/test/providers/asset_viewer/download_provider_test.dart index bfccd29080..2e2694475d 100644 --- a/mobile/test/providers/asset_viewer/download_provider_test.dart +++ b/mobile/test/providers/asset_viewer/download_provider_test.dart @@ -101,4 +101,13 @@ void main() { expect(notifier.state.taskProgress.containsKey('ghost'), isFalse); }); }); + + test('a status for an unknown task does not create an entry', () { + fakeAsync((async) { + onImage(TaskStatusUpdate(_task('ghost'), TaskStatus.complete)); + + expect(notifier.state.taskProgress, isEmpty); + expect(notifier.state.showProgress, isFalse); + }); + }); }