fix(mobile): mark finished downloads complete instead of leaving them stuck (#29023)
* wire download status to the ui and dismiss finished entries * mark finished downloads with a checkmark instead of dismissing * use the theme color for the done icon and guard status updates on a known task
This commit is contained in:
@@ -64,6 +64,7 @@ class DownloadTaskTile extends StatelessWidget {
|
|||||||
|
|
||||||
@override
|
@override
|
||||||
Widget build(BuildContext context) {
|
Widget build(BuildContext context) {
|
||||||
|
final isComplete = status == TaskStatus.complete;
|
||||||
final progressPercent = (progress * 100).round();
|
final progressPercent = (progress * 100).round();
|
||||||
|
|
||||||
String getStatusText() => switch (status) {
|
String getStatusText() => switch (status) {
|
||||||
@@ -88,9 +89,14 @@ class DownloadTaskTile extends StatelessWidget {
|
|||||||
leading: const Icon(Icons.video_file_outlined),
|
leading: const Icon(Icons.video_file_outlined),
|
||||||
title: Text(getStatusText(), style: context.textTheme.labelLarge),
|
title: Text(getStatusText(), style: context.textTheme.labelLarge),
|
||||||
trailing: IconButton(
|
trailing: IconButton(
|
||||||
icon: Icon(Icons.close, color: context.colorScheme.onError),
|
icon: Icon(
|
||||||
|
isComplete ? Icons.download_done : Icons.close,
|
||||||
|
color: isComplete ? context.colorScheme.primary : context.colorScheme.onError,
|
||||||
|
),
|
||||||
onPressed: onCancelDownload,
|
onPressed: onCancelDownload,
|
||||||
style: ElevatedButton.styleFrom(backgroundColor: context.colorScheme.error.withAlpha(200)),
|
style: isComplete
|
||||||
|
? null
|
||||||
|
: ElevatedButton.styleFrom(backgroundColor: context.colorScheme.error.withAlpha(200)),
|
||||||
),
|
),
|
||||||
subtitle: Column(
|
subtitle: Column(
|
||||||
crossAxisAlignment: CrossAxisAlignment.start,
|
crossAxisAlignment: CrossAxisAlignment.start,
|
||||||
|
|||||||
@@ -14,9 +14,29 @@ class DownloadStateNotifier extends StateNotifier<DownloadState> {
|
|||||||
taskProgress: <String, DownloadInfo>{},
|
taskProgress: <String, DownloadInfo>{},
|
||||||
),
|
),
|
||||||
) {
|
) {
|
||||||
|
_downloadService.onImageDownloadStatus = _downloadStatusCallback;
|
||||||
|
_downloadService.onVideoDownloadStatus = _downloadStatusCallback;
|
||||||
|
_downloadService.onLivePhotoDownloadStatus = _downloadStatusCallback;
|
||||||
_downloadService.onTaskProgress = _taskProgressCallback;
|
_downloadService.onTaskProgress = _taskProgressCallback;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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: <String, DownloadInfo>{}
|
||||||
|
..addAll(state.taskProgress)
|
||||||
|
..addAll({update.task.taskId: existing.copyWith(status: update.status)}),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
void _taskProgressCallback(TaskProgressUpdate update) {
|
void _taskProgressCallback(TaskProgressUpdate update) {
|
||||||
// Ignore if the task is canceled or completed
|
// Ignore if the task is canceled or completed
|
||||||
if (update.progress == -2 || update.progress == -1) {
|
if (update.progress == -2 || update.progress == -1) {
|
||||||
|
|||||||
@@ -28,6 +28,8 @@ class DownloadRepository {
|
|||||||
|
|
||||||
void Function(TaskStatusUpdate)? onVideoDownloadStatus;
|
void Function(TaskStatusUpdate)? onVideoDownloadStatus;
|
||||||
|
|
||||||
|
void Function(TaskStatusUpdate)? onLivePhotoDownloadStatus;
|
||||||
|
|
||||||
void Function(TaskProgressUpdate)? onTaskProgress;
|
void Function(TaskProgressUpdate)? onTaskProgress;
|
||||||
|
|
||||||
// #29900: `taskStatusCallback` is called before the DB has been updated, causing a race between the two Live Photo tasks
|
// #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(
|
_downloader.registerCallbacks(
|
||||||
group: kDownloadGroupLivePhoto,
|
group: kDownloadGroupLivePhoto,
|
||||||
|
taskStatusCallback: (update) => onLivePhotoDownloadStatus?.call(update),
|
||||||
taskProgressCallback: (update) => onTaskProgress?.call(update),
|
taskProgressCallback: (update) => onTaskProgress?.call(update),
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ class DownloadService {
|
|||||||
final Logger _log = Logger("DownloadService");
|
final Logger _log = Logger("DownloadService");
|
||||||
void Function(TaskStatusUpdate)? onImageDownloadStatus;
|
void Function(TaskStatusUpdate)? onImageDownloadStatus;
|
||||||
void Function(TaskStatusUpdate)? onVideoDownloadStatus;
|
void Function(TaskStatusUpdate)? onVideoDownloadStatus;
|
||||||
|
void Function(TaskStatusUpdate)? onLivePhotoDownloadStatus;
|
||||||
void Function(TaskProgressUpdate)? onTaskProgress;
|
void Function(TaskProgressUpdate)? onTaskProgress;
|
||||||
|
|
||||||
/// Active Live Photo IDs undergoing saving
|
/// Active Live Photo IDs undergoing saving
|
||||||
@@ -30,6 +31,7 @@ class DownloadService {
|
|||||||
DownloadService(this._fileMediaRepository, this._downloadRepository) {
|
DownloadService(this._fileMediaRepository, this._downloadRepository) {
|
||||||
_downloadRepository.onImageDownloadStatus = _onImageDownloadCallback;
|
_downloadRepository.onImageDownloadStatus = _onImageDownloadCallback;
|
||||||
_downloadRepository.onVideoDownloadStatus = _onVideoDownloadCallback;
|
_downloadRepository.onVideoDownloadStatus = _onVideoDownloadCallback;
|
||||||
|
_downloadRepository.onLivePhotoDownloadStatus = _onLivePhotoDownloadCallback;
|
||||||
_downloadRepository.onTaskProgress = _onTaskProgressCallback;
|
_downloadRepository.onTaskProgress = _onTaskProgressCallback;
|
||||||
_downloadRepository.onLivePhotoRecordComplete = _onLivePhotoRecordComplete;
|
_downloadRepository.onLivePhotoRecordComplete = _onLivePhotoRecordComplete;
|
||||||
|
|
||||||
@@ -65,6 +67,10 @@ class DownloadService {
|
|||||||
onVideoDownloadStatus?.call(update);
|
onVideoDownloadStatus?.call(update);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void _onLivePhotoDownloadCallback(TaskStatusUpdate update) {
|
||||||
|
onLivePhotoDownloadStatus?.call(update);
|
||||||
|
}
|
||||||
|
|
||||||
Future<void> _onLivePhotoRecordComplete(TaskRecord record) async {
|
Future<void> _onLivePhotoRecordComplete(TaskRecord record) async {
|
||||||
final livePhotosId = LivePhotosMetadata.fromJson(record.task.metaData).id;
|
final livePhotosId = LivePhotosMetadata.fromJson(record.task.metaData).id;
|
||||||
await _saveLivePhotos(livePhotosId);
|
await _saveLivePhotos(livePhotosId);
|
||||||
|
|||||||
@@ -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);
|
||||||
|
});
|
||||||
|
}
|
||||||
@@ -0,0 +1,113 @@
|
|||||||
|
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 and keeps it visible', () {
|
||||||
|
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: 5));
|
||||||
|
|
||||||
|
expect(notifier.state.taskProgress['task-1']?.status, TaskStatus.complete);
|
||||||
|
expect(notifier.state.showProgress, isTrue);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
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 keeps both entries', () {
|
||||||
|
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: 5));
|
||||||
|
|
||||||
|
expect(notifier.state.taskProgress['live-image']?.status, TaskStatus.complete);
|
||||||
|
expect(notifier.state.taskProgress['live-video']?.status, TaskStatus.running);
|
||||||
|
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('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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user