diff --git a/app/lib/common/toolkit/errors/error_dialog.dart b/app/lib/common/toolkit/errors/error_dialog.dart index 8975cf74d7f4..72736d618e1a 100644 --- a/app/lib/common/toolkit/errors/error_dialog.dart +++ b/app/lib/common/toolkit/errors/error_dialog.dart @@ -12,7 +12,7 @@ class ActerErrorDialog extends StatelessWidget { static const retryBtn = Key('error-dialog-retry-btn'); final Object error; - final StackTrace stack; + final StackTrace? stack; final VoidCallback? onRetryTap; final String? title; @@ -26,7 +26,7 @@ class ActerErrorDialog extends StatelessWidget { const ActerErrorDialog({ super.key, required this.error, - required this.stack, + this.stack, this.onRetryTap, this.title, this.text, @@ -39,7 +39,7 @@ class ActerErrorDialog extends StatelessWidget { /// BuildContext required BuildContext context, required Object error, - required StackTrace stack, + StackTrace? stack, /// Title of the dialog String? title, @@ -120,12 +120,12 @@ class ActerErrorDialog extends StatelessWidget { class _ActerErrorAlert extends QuickAlertContainer { final bool includeBugReportButton; final Object error; - final StackTrace stack; + final StackTrace? stack; const _ActerErrorAlert({ required super.options, required this.error, - required this.stack, + this.stack, this.includeBugReportButton = true, }); @@ -148,13 +148,18 @@ class _ActerErrorAlert extends QuickAlertContainer { top: 10, child: TextButton( child: Text(L10n.of(context).reportBug), - onPressed: () => openBugReport( - context, - queryParams: { + onPressed: () async { + final queryParams = { 'error': error.toString(), - 'stack': stack.toString(), - }, - ), + }; + if (stack != null) { + queryParams['stack'] = stack.toString(); + } + return openBugReport( + context, + queryParams: queryParams, + ); + }, ), ), ], diff --git a/app/lib/common/toolkit/errors/inline_error_button.dart b/app/lib/common/toolkit/errors/inline_error_button.dart new file mode 100644 index 000000000000..c21bee25ddba --- /dev/null +++ b/app/lib/common/toolkit/errors/inline_error_button.dart @@ -0,0 +1,95 @@ +import 'package:acter/common/toolkit/errors/error_dialog.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_gen/gen_l10n/l10n.dart'; + +/// InlineErrorButton for text inlined actions +/// +/// This is a ErrorButton that highlights the given text using the +/// `theme.inlineErrorTheme`. Thus this is super useful if you have some text +/// and want a specific part of it to be highlighted to the user indicating +/// it has an action. See [ErrorButton] for options. +class ActerInlineErrorButton extends StatelessWidget { + final Object error; + final StackTrace? stack; + final VoidCallback? onRetryTap; + final Icon? icon; + + final String? dialogTitle; + final String? text; + final String Function(Object error)? textBuilder; + final bool includeBugReportButton; + + const ActerInlineErrorButton({ + super.key, + required this.error, + this.icon, + this.stack, + this.dialogTitle, + this.text, + this.textBuilder, + this.onRetryTap, + this.includeBugReportButton = true, + }); + + @override + Widget build(BuildContext context) { + if (icon != null) { + return _buildWithIcon(context); + } + return TextButton( + onPressed: () async { + await ActerErrorDialog.show( + context: context, + error: error, + stack: stack, + title: dialogTitle, + text: text, + textBuilder: textBuilder, + onRetryTap: onRetryTap != null + ? () { + onRetryTap!(); + Navigator.pop(context); + } + : null, + includeBugReportButton: includeBugReportButton, + ); + }, + child: Text(L10n.of(context).fatalError), + ); + } + + const ActerInlineErrorButton.icon({ + super.key, + required this.error, + this.stack, + required this.icon, + this.dialogTitle, + this.text, + this.textBuilder, + this.onRetryTap, + this.includeBugReportButton = true, + }); + + Widget _buildWithIcon(BuildContext context) { + return IconButton( + icon: icon!, + onPressed: () async { + await ActerErrorDialog.show( + context: context, + error: error, + stack: stack, + title: dialogTitle, + text: text, + textBuilder: textBuilder, + onRetryTap: onRetryTap != null + ? () { + onRetryTap!(); + Navigator.pop(context); + } + : null, + includeBugReportButton: includeBugReportButton, + ); + }, + ); + } +} diff --git a/app/lib/features/chat/providers/notifiers/media_chat_notifier.dart b/app/lib/features/chat/providers/notifiers/media_chat_notifier.dart index 9c9c0e83df83..e630b1552849 100644 --- a/app/lib/features/chat/providers/notifiers/media_chat_notifier.dart +++ b/app/lib/features/chat/providers/notifiers/media_chat_notifier.dart @@ -33,14 +33,21 @@ class MediaChatNotifier extends StateNotifier { ); try { //Get media path if already downloaded - final mediaPath = await _convo!.mediaPath(messageInfo.messageId, false); - if (mediaPath.text() != null) { - final videoThumbnailFile = await getThumbnailData(mediaPath.text()!); + final mediaPath = + (await _convo!.mediaPath(messageInfo.messageId, false)).text(); + + if (mediaPath != null) { state = state.copyWith( - mediaFile: File(mediaPath.text()!), - videoThumbnailFile: videoThumbnailFile, + mediaFile: File(mediaPath), + videoThumbnailFile: null, mediaChatLoadingState: const MediaChatLoadingState.loaded(), ); + final videoThumbnailFile = await getThumbnailData(mediaPath); + if (videoThumbnailFile != null) { + if (state.mediaFile?.path == mediaPath) { + state = state.copyWith(videoThumbnailFile: videoThumbnailFile); + } + } } else { // FIXME: this does not react if yet if we switched the network ... if (await ref @@ -79,14 +86,19 @@ class MediaChatNotifier extends StateNotifier { null, tempDir.path, ); - String? mediaPath = result.text(); + final mediaPath = result.text(); if (mediaPath != null) { - final videoThumbnailFile = await getThumbnailData(mediaPath); state = state.copyWith( mediaFile: File(mediaPath), - videoThumbnailFile: videoThumbnailFile, + videoThumbnailFile: null, isDownloading: false, ); + final videoThumbnailFile = await getThumbnailData(mediaPath); + if (videoThumbnailFile != null) { + if (state.mediaFile?.path == mediaPath) { + state = state.copyWith(videoThumbnailFile: videoThumbnailFile); + } + } } } catch (e, s) { _log.severe('Error downloading media:', e, s); diff --git a/app/lib/features/chat/widgets/image_message_builder.dart b/app/lib/features/chat/widgets/image_message_builder.dart index c2a8531b2242..d6d3bcff4485 100644 --- a/app/lib/features/chat/widgets/image_message_builder.dart +++ b/app/lib/features/chat/widgets/image_message_builder.dart @@ -1,4 +1,5 @@ import 'package:acter/common/models/types.dart'; +import 'package:acter/common/toolkit/errors/inline_error_button.dart'; import 'package:acter/common/widgets/image_dialog.dart'; import 'package:acter/features/chat/models/media_chat_state/media_chat_state.dart'; import 'package:acter/features/chat/providers/chat_providers.dart'; @@ -7,6 +8,7 @@ import 'package:flutter_chat_types/flutter_chat_types.dart' as types; import 'package:flutter_chat_ui/flutter_chat_ui.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_gen/gen_l10n/l10n.dart'; +import 'package:phosphor_flutter/phosphor_flutter.dart'; class ImageMessageBuilder extends ConsumerWidget { final types.ImageMessage message; @@ -32,7 +34,7 @@ class ImageMessageBuilder extends ConsumerWidget { } else if (mediaState.mediaFile == null) { return imagePlaceholder(context, roomId, mediaState, ref); } else { - return imageUI(context, mediaState); + return imageUI(context, ref, mediaState); } } @@ -110,7 +112,11 @@ class ImageMessageBuilder extends ConsumerWidget { ); } - Widget imageUI(BuildContext context, MediaChatState mediaState) { + Widget imageUI( + BuildContext context, + WidgetRef ref, + MediaChatState mediaState, + ) { final size = MediaQuery.of(context).size; return InkWell( onTap: () { @@ -133,13 +139,17 @@ class ImageMessageBuilder extends ConsumerWidget { maxWidth: isReplyContent ? size.height * 0.2 : 300, maxHeight: isReplyContent ? size.width * 0.2 : 300, ), - child: imageFileView(context, mediaState), + child: imageFileView(context, ref, mediaState), ), ), ); } - Widget imageFileView(BuildContext context, MediaChatState mediaState) { + Widget imageFileView( + BuildContext context, + WidgetRef ref, + MediaChatState mediaState, + ) { return Image.file( mediaState.mediaFile!, frameBuilder: ( @@ -160,11 +170,20 @@ class ImageMessageBuilder extends ConsumerWidget { }, errorBuilder: ( BuildContext context, - Object url, - StackTrace? error, - ) { - return Text(L10n.of(context).couldNotLoadImage(error.toString())); - }, + Object error, + StackTrace? stack, + ) => + ActerInlineErrorButton.icon( + icon: Icon(PhosphorIcons.imageBroken()), + error: error, + stack: stack, + textBuilder: L10n.of(context).couldNotLoadImage, + onRetryTap: () { + final ChatMessageInfo messageInfo = + (messageId: message.id, roomId: roomId); + ref.invalidate(mediaChatStateProvider(messageInfo)); + }, + ), fit: BoxFit.cover, ); } diff --git a/app/test/features/chat/chat_message_test.dart b/app/test/features/chat/messages/chat_message_test.dart similarity index 100% rename from app/test/features/chat/chat_message_test.dart rename to app/test/features/chat/messages/chat_message_test.dart diff --git a/app/test/features/chat/messages/image_message_test.dart b/app/test/features/chat/messages/image_message_test.dart new file mode 100644 index 000000000000..002f28165fc6 --- /dev/null +++ b/app/test/features/chat/messages/image_message_test.dart @@ -0,0 +1,77 @@ +import 'package:acter/common/providers/chat_providers.dart'; +import 'package:acter/common/toolkit/errors/inline_error_button.dart'; +import 'package:acter/features/chat/providers/chat_providers.dart'; +import 'package:acter/features/chat/widgets/image_message_builder.dart'; +import 'package:acter_flutter_sdk/acter_flutter_sdk_ffi.dart'; +import 'package:convenient_test_dev/convenient_test_dev.dart'; +import 'package:flutter_chat_types/flutter_chat_types.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; + +import '../../../helpers/error_helpers.dart'; +import '../../../helpers/mock_chat_providers.dart'; +import '../../../helpers/test_util.dart'; + +class FixedOptionStringMock extends Mock implements OptionString { + final String? result; + + FixedOptionStringMock(this.result); + + @override + String? text() => result; +} + +/// Mocked version of ActerSdk for test purposes +class RetryMediaConvoMock extends Mock implements Convo { + bool shouldFail = true; + @override + Future mediaPath(String eventId, bool isThumb) async { + if (shouldFail) { + shouldFail = false; + return FixedOptionStringMock('bad_path'); + } + return FixedOptionStringMock(null); + } +} + +void main() { + group('Image Fails to Load Error', () { + testWidgets('shows error and retries', (tester) async { + const imageMessage = ImageMessage( + author: User( + id: 'sender', + firstName: 'userName', + ), + remoteId: 'eventItem.uniqueId()', + createdAt: 1234567, + height: 20, + id: 'eventId', + name: 'msgContent.body()', + size: 30, + uri: 'msgContent.source()!.url()', + width: 30, + ); + + await tester.pumpProviderWidget( + overrides: [ + // Provider first provides a broken path to trigger the error + // then null, so it would check for auto-download but not attempt + chatProvider.overrideWith( + () => MockAsyncConvoNotifier(retVal: RetryMediaConvoMock()), + ), + autoDownloadMediaProvider.overrideWith((a, b) => false), + ], + child: const ImageMessageBuilder( + message: imageMessage, + roomId: '!roomId', + messageWidth: 100, + ), + ); + + await tester.pumpWithRunAsyncUntil( + () => findsOne.matches(find.byType(ActerInlineErrorButton), {}), + ); + await tester.ensureInlineErrorWithRetryWorks(); + }); + }); +} diff --git a/app/test/helpers/error_helpers.dart b/app/test/helpers/error_helpers.dart index 80b27dccd854..e0285fbdb1d4 100644 --- a/app/test/helpers/error_helpers.dart +++ b/app/test/helpers/error_helpers.dart @@ -1,5 +1,6 @@ import 'package:acter/common/toolkit/errors/error_dialog.dart'; import 'package:acter/common/toolkit/errors/error_page.dart'; +import 'package:acter/common/toolkit/errors/inline_error_button.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'test_util.dart'; @@ -27,7 +28,7 @@ extension ErrorPageExtensions on WidgetTester { expect( find.byKey(ActerErrorDialog.retryBtn), findsOneWidget, - reason: 'Confirm Button not present', + reason: 'Retry Button not present', ); await tap(find.byKey(ActerErrorDialog.retryBtn)); @@ -46,4 +47,47 @@ extension ErrorPageExtensions on WidgetTester { rethrow; } } + + Future ensureInlineErrorWithRetryWorks({dumpOnError = true}) async { + await pumpProviderScopeOnce(); + try { + expect( + find.byType(ActerInlineErrorButton), + findsOneWidget, + reason: 'Inline Error Button not found', + ); + expect( + find.byType(ActerErrorDialog), + findsNothing, + reason: 'Acter Dialog showed too early', + ); + await tap(find.byType(ActerInlineErrorButton)); + await pumpProviderScopeOnce(); + expect( + find.byType(ActerErrorDialog), + findsOne, + reason: "Acter Dialog didn't open", + ); + + expect( + find.byKey(ActerErrorDialog.retryBtn), + findsOneWidget, + reason: 'Retry Button not present', + ); + + await tap(find.byKey(ActerErrorDialog.retryBtn)); + await pumpProviderScope(times: 10); + // dialog is gone on retry working + expect( + find.byType(ActerErrorDialog), + findsNothing, + reason: 'Error Dialog did not disappear', + ); + } catch (e) { + if (dumpOnError) { + debugDumpApp(); + } + rethrow; + } + } } diff --git a/app/test/helpers/mock_chat_providers.dart b/app/test/helpers/mock_chat_providers.dart index d880b2b009c9..16fcbf99c7af 100644 --- a/app/test/helpers/mock_chat_providers.dart +++ b/app/test/helpers/mock_chat_providers.dart @@ -31,11 +31,13 @@ class MockChatRoomNotifier extends StateNotifier final String roomId; MockChatRoomNotifier(this.roomId) - : super(const ChatRoomState( - hasMore: false, - messages: [], - loading: ChatRoomLoadingState.loaded(), - ),); + : super( + const ChatRoomState( + hasMore: false, + messages: [], + loading: ChatRoomLoadingState.loaded(), + ), + ); @override Future fetchMediaBinary(String? msgType, String eventId) { @@ -135,9 +137,13 @@ class MockRoomAvatarInfoNotifier extends FamilyNotifier class MockAsyncConvoNotifier extends FamilyAsyncNotifier with Mock implements AsyncConvoNotifier { + final Convo? retVal; + + MockAsyncConvoNotifier({this.retVal}); + @override FutureOr build(String arg) { - return null; + return retVal; } } diff --git a/app/test/helpers/test_util.dart b/app/test/helpers/test_util.dart index 1d451e050f77..6424777977c1 100644 --- a/app/test/helpers/test_util.dart +++ b/app/test/helpers/test_util.dart @@ -27,17 +27,14 @@ extension PumpUntilFound on WidgetTester { Matcher matcher, { Duration duration = const Duration(milliseconds: 100), int tries = 10, + dumpOnError = true, }) async { for (var i = 1; i <= tries; i++) { await pump(duration); - - try { - expect(finder, matches); - break; - } on TestFailure { - if (i == tries) { + if (!matcher.matches(finder, {}) && (i == tries)) { + if (dumpOnError) { debugDumpApp(); - rethrow; + expect(finder, matches); } } }