From 4e8a8f2aabc3f801a93b9fee04b9400bc7f2c761 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Sun, 25 Aug 2024 16:05:55 +0100 Subject: [PATCH 1/6] tests and implemetnation for error pages tasklist lists --- .../features/tasks/pages/tasks_list_page.dart | 14 ++- app/test/features/tasks/error_pages_test.dart | 87 +++++++++++++++++++ app/test/helpers/mock_tasks_providers.dart | 23 +++++ 3 files changed, 120 insertions(+), 4 deletions(-) create mode 100644 app/test/features/tasks/error_pages_test.dart create mode 100644 app/test/helpers/mock_tasks_providers.dart diff --git a/app/lib/features/tasks/pages/tasks_list_page.dart b/app/lib/features/tasks/pages/tasks_list_page.dart index 96affd268850..87039fdfa253 100644 --- a/app/lib/features/tasks/pages/tasks_list_page.dart +++ b/app/lib/features/tasks/pages/tasks_list_page.dart @@ -2,6 +2,7 @@ import 'dart:math'; import 'package:acter/common/providers/space_providers.dart'; import 'package:acter/common/toolkit/buttons/primary_action_button.dart'; +import 'package:acter/common/toolkit/errors/error_page.dart'; import 'package:acter/common/widgets/acter_search_widget.dart'; import 'package:acter/common/widgets/add_button_with_can_permission.dart'; import 'package:acter/common/widgets/empty_state_widget.dart'; @@ -108,10 +109,15 @@ class _TasksListPageConsumerState extends ConsumerState { Expanded( child: tasklistsLoader.when( data: (tasklists) => _buildTasklists(tasklists), - error: (e, s) { - _log.severe('Failed to search tasklists in space', e, s); - return Center( - child: Text(L10n.of(context).searchingFailed(e)), + error: (error, stack) { + _log.severe('Failed to search tasklists in space', error, stack); + return ErrorPage( + background: const TasksListSkeleton(), + error: error, + stack: stack, + onRetryTap: () { + ref.invalidate(allTasksListsProvider); + }, ); }, loading: () => const TasksListSkeleton(), diff --git a/app/test/features/tasks/error_pages_test.dart b/app/test/features/tasks/error_pages_test.dart new file mode 100644 index 000000000000..bbb1966db724 --- /dev/null +++ b/app/test/features/tasks/error_pages_test.dart @@ -0,0 +1,87 @@ +import 'package:acter/common/providers/room_providers.dart'; +import 'package:acter/common/providers/space_providers.dart'; +import 'package:acter/common/widgets/acter_search_widget.dart'; +import 'package:acter/features/tasks/pages/tasks_list_page.dart'; +import 'package:acter/features/tasks/providers/tasklists_providers.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../../helpers/error_helpers.dart'; +import '../../helpers/mock_tasks_providers.dart'; +import '../../helpers/test_util.dart'; + +void main() { + group('TaskList List Error Pages', () { + testWidgets('full list', (tester) async { + final mockedTaskListNotifier = MockAsyncAllTaskListsNotifier(); + await tester.pumpProviderWidget( + overrides: [ + allTasksListsProvider.overrideWith(() => mockedTaskListNotifier), + hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), + ], + child: const TasksListPage(), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + testWidgets('full list with search', (tester) async { + final mockedTaskListNotifier = MockAsyncAllTaskListsNotifier(); + + await tester.pumpProviderWidget( + overrides: [ + searchValueProvider + .overrideWith((_) => 'some string'), // set a search string + allTasksListsProvider.overrideWith(() => mockedTaskListNotifier), + hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), + ], + child: const TasksListPage(), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + + testWidgets('space list', (tester) async { + final mockedTaskListNotifier = MockAsyncAllTaskListsNotifier(); + await tester.pumpProviderWidget( + overrides: [ + roomDisplayNameProvider.overrideWith((a, b) => 'test'), + allTasksListsProvider.overrideWith(() => mockedTaskListNotifier), + hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), + ], + child: const TasksListPage( + spaceId: '!test', + ), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + + testWidgets('space list', (tester) async { + final mockedTaskListNotifier = MockAsyncAllTaskListsNotifier(); + await tester.pumpProviderWidget( + overrides: [ + roomDisplayNameProvider.overrideWith((a, b) => 'test'), + allTasksListsProvider.overrideWith(() => mockedTaskListNotifier), + hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), + ], + child: const TasksListPage( + spaceId: '!test', + ), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + + testWidgets('space list with search', (tester) async { + final mockedTaskListNotifier = MockAsyncAllTaskListsNotifier(); + await tester.pumpProviderWidget( + overrides: [ + roomDisplayNameProvider.overrideWith((a, b) => 'test'), + searchValueProvider + .overrideWith((_) => 'some search'), // set a search string + allTasksListsProvider.overrideWith(() => mockedTaskListNotifier), + hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), + ], + child: const TasksListPage( + spaceId: '!test', + ), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + }); +} diff --git a/app/test/helpers/mock_tasks_providers.dart b/app/test/helpers/mock_tasks_providers.dart new file mode 100644 index 000000000000..bc55e8b560fc --- /dev/null +++ b/app/test/helpers/mock_tasks_providers.dart @@ -0,0 +1,23 @@ +import 'dart:async'; +import 'package:acter/features/tasks/providers/notifiers.dart'; +import 'package:acter_flutter_sdk/acter_flutter_sdk_ffi.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:mockito/mockito.dart'; +import 'package:riverpod/riverpod.dart'; + +class MockAsyncAllTaskListsNotifier extends AsyncNotifier> + with Mock + implements AsyncAllTaskListsNotifier { + bool shouldFail = true; + + @override + Future> build() async { + if (shouldFail) { + // toggle failure so the retry works + shouldFail = !shouldFail; + throw 'Expected fail'; + } + + return []; + } +} From 79bb2aa2c219d0cd857c08dbc1e2829b8eb73ee9 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Sun, 25 Aug 2024 16:15:02 +0100 Subject: [PATCH 2/6] Error Page for TaskList Details --- .../tasks/pages/task_list_details_page.dart | 14 +++++++-- app/test/features/tasks/error_pages_test.dart | 14 +++++++++ app/test/helpers/mock_tasks_providers.dart | 31 +++++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/app/lib/features/tasks/pages/task_list_details_page.dart b/app/lib/features/tasks/pages/task_list_details_page.dart index 680efb45689d..68e4595fd380 100644 --- a/app/lib/features/tasks/pages/task_list_details_page.dart +++ b/app/lib/features/tasks/pages/task_list_details_page.dart @@ -1,5 +1,6 @@ import 'package:acter/common/actions/redact_content.dart'; import 'package:acter/common/actions/report_content.dart'; +import 'package:acter/common/toolkit/errors/error_page.dart'; import 'package:acter/common/widgets/edit_html_description_sheet.dart'; import 'package:acter/common/widgets/edit_title_sheet.dart'; import 'package:acter/common/widgets/render_html.dart'; @@ -132,9 +133,16 @@ class _TaskListPageState extends ConsumerState { final tasklistLoader = ref.watch(taskListItemProvider(widget.taskListId)); return tasklistLoader.when( data: (tasklist) => _buildTaskListData(tasklist), - error: (e, s) { - _log.severe('Failed to load tasklist', e, s); - return Text(L10n.of(context).loadingFailed(e)); + error: (error, stack) { + _log.severe('Failed to load tasklist', error, stack); + return ErrorPage( + background: Text(L10n.of(context).loading), + error: error, + stack: stack, + onRetryTap: () { + ref.invalidate(taskListItemProvider(widget.taskListId)); + }, + ); }, loading: () => Text(L10n.of(context).loading), ); diff --git a/app/test/features/tasks/error_pages_test.dart b/app/test/features/tasks/error_pages_test.dart index bbb1966db724..c127e92d6a84 100644 --- a/app/test/features/tasks/error_pages_test.dart +++ b/app/test/features/tasks/error_pages_test.dart @@ -1,6 +1,7 @@ import 'package:acter/common/providers/room_providers.dart'; import 'package:acter/common/providers/space_providers.dart'; import 'package:acter/common/widgets/acter_search_widget.dart'; +import 'package:acter/features/tasks/pages/task_list_details_page.dart'; import 'package:acter/features/tasks/pages/tasks_list_page.dart'; import 'package:acter/features/tasks/providers/tasklists_providers.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -84,4 +85,17 @@ void main() { await tester.ensureErrorPageWithRetryWorks(); }); }); + group('TaskList Details Error Pages', () { + testWidgets('body error page', (tester) async { + final mockedNotifier = MockTaskListItemNotifier(); + await tester.pumpProviderWidget( + overrides: [ + taskListItemProvider.overrideWith(() => mockedNotifier), + hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), + ], + child: const TaskListDetailPage(taskListId: 'taskListId'), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + }); } diff --git a/app/test/helpers/mock_tasks_providers.dart b/app/test/helpers/mock_tasks_providers.dart index bc55e8b560fc..3254d5846cc7 100644 --- a/app/test/helpers/mock_tasks_providers.dart +++ b/app/test/helpers/mock_tasks_providers.dart @@ -21,3 +21,34 @@ class MockAsyncAllTaskListsNotifier extends AsyncNotifier> return []; } } + +class MockTaskListItemNotifier extends FamilyAsyncNotifier + with Mock + implements TaskListItemNotifier { + bool shouldFail = true; + + @override + Future build(String arg) async { + if (shouldFail) { + // toggle failure so the retry works + shouldFail = !shouldFail; + throw 'Expected fail'; + } + + return MockTaskList(); + } +} + +class MockTaskList extends Fake implements TaskList { + @override + String name() => 'Test'; + @override + MsgContent? description() => null; + + @override + Future attachments() => + Completer().future; + + @override + Future comments() => Completer().future; +} From 1530441096588820363319fc007c5f7ab397a6e1 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Sun, 25 Aug 2024 16:29:32 +0100 Subject: [PATCH 3/6] Task Item details page error pages support --- .../tasks/pages/task_item_detail_page.dart | 16 +++++-- app/test/features/tasks/error_pages_test.dart | 19 ++++++++ app/test/helpers/mock_tasks_providers.dart | 47 ++++++++++++++++++- 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/app/lib/features/tasks/pages/task_item_detail_page.dart b/app/lib/features/tasks/pages/task_item_detail_page.dart index ae479cf0e0fc..9e47986a01b0 100644 --- a/app/lib/features/tasks/pages/task_item_detail_page.dart +++ b/app/lib/features/tasks/pages/task_item_detail_page.dart @@ -4,6 +4,7 @@ import 'package:acter/common/actions/redact_content.dart'; import 'package:acter/common/actions/report_content.dart'; import 'package:acter/common/providers/room_providers.dart'; import 'package:acter/common/toolkit/buttons/inline_text_button.dart'; +import 'package:acter/common/toolkit/errors/error_page.dart'; import 'package:acter/common/utils/utils.dart'; import 'package:acter/common/widgets/edit_html_description_sheet.dart'; import 'package:acter/common/widgets/edit_title_sheet.dart'; @@ -187,9 +188,18 @@ class TaskItemDetailPage extends ConsumerWidget { ) { return taskLoader.when( data: (task) => taskData(context, task, ref), - error: (e, s) { - _log.severe('Failed to load task', e, s); - return Text(L10n.of(context).loadingFailed(e)); + error: (error, stack) { + _log.severe('Failed to load task', error, stack); + return ErrorPage( + background: const TaskItemDetailPageSkeleton(), + error: error, + stack: stack, + onRetryTap: () { + ref.invalidate( + taskItemProvider((taskListId: taskListId, taskId: taskId)), + ); + }, + ); }, loading: () => const TaskItemDetailPageSkeleton(), ); diff --git a/app/test/features/tasks/error_pages_test.dart b/app/test/features/tasks/error_pages_test.dart index c127e92d6a84..f327a9f8df87 100644 --- a/app/test/features/tasks/error_pages_test.dart +++ b/app/test/features/tasks/error_pages_test.dart @@ -1,8 +1,10 @@ import 'package:acter/common/providers/room_providers.dart'; import 'package:acter/common/providers/space_providers.dart'; import 'package:acter/common/widgets/acter_search_widget.dart'; +import 'package:acter/features/tasks/pages/task_item_detail_page.dart'; import 'package:acter/features/tasks/pages/task_list_details_page.dart'; import 'package:acter/features/tasks/pages/tasks_list_page.dart'; +import 'package:acter/features/tasks/providers/task_items_providers.dart'; import 'package:acter/features/tasks/providers/tasklists_providers.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -98,4 +100,21 @@ void main() { await tester.ensureErrorPageWithRetryWorks(); }); }); + group('Task Details Error Pages', () { + testWidgets('body error page', (tester) async { + final mockedNotifier = MockTaskListItemNotifier(shouldFail: false); + await tester.pumpProviderWidget( + overrides: [ + notifierTaskProvider.overrideWith(() => MockTaskItemNotifier()), + taskListItemProvider.overrideWith(() => mockedNotifier), + hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), + ], + child: const TaskItemDetailPage( + taskListId: 'taskListId', + taskId: 'taskid', + ), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + }); } diff --git a/app/test/helpers/mock_tasks_providers.dart b/app/test/helpers/mock_tasks_providers.dart index 3254d5846cc7..7bfb3eee57c5 100644 --- a/app/test/helpers/mock_tasks_providers.dart +++ b/app/test/helpers/mock_tasks_providers.dart @@ -8,7 +8,9 @@ import 'package:riverpod/riverpod.dart'; class MockAsyncAllTaskListsNotifier extends AsyncNotifier> with Mock implements AsyncAllTaskListsNotifier { - bool shouldFail = true; + bool shouldFail; + + MockAsyncAllTaskListsNotifier({this.shouldFail = true}); @override Future> build() async { @@ -25,7 +27,9 @@ class MockAsyncAllTaskListsNotifier extends AsyncNotifier> class MockTaskListItemNotifier extends FamilyAsyncNotifier with Mock implements TaskListItemNotifier { - bool shouldFail = true; + bool shouldFail; + + MockTaskListItemNotifier({this.shouldFail = true}); @override Future build(String arg) async { @@ -39,7 +43,17 @@ class MockTaskListItemNotifier extends FamilyAsyncNotifier } } +class MockTaskItemNotifier extends FamilyAsyncNotifier + with Mock + implements TaskItemNotifier { + @override + Future build(Task arg) async { + return arg; + } +} + class MockTaskList extends Fake implements TaskList { + bool shouldFail = true; @override String name() => 'Test'; @override @@ -51,4 +65,33 @@ class MockTaskList extends Fake implements TaskList { @override Future comments() => Completer().future; + + @override + Future task(String taskId) async { + if (shouldFail) { + shouldFail = false; + + throw 'Expected fail'; + } + + return MockTask(); + } +} + +class MockTask extends Fake implements Task { + @override + String title() => 'Test'; + @override + MsgContent? description() => null; + @override + String? dueDate() => null; + @override + bool isAssignedToMe() => false; + + @override + Future attachments() => + Completer().future; + + @override + Future comments() => Completer().future; } From e4ecfe53f9cbadfb953b6d5115992524bc70ed27 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Sun, 25 Aug 2024 16:51:01 +0100 Subject: [PATCH 4/6] Tests for EventList Error Pages --- .../events/pages/event_list_page.dart | 30 ++++-- .../events/providers/event_providers.dart | 4 +- .../features/events/error_pages_test.dart | 100 ++++++++++++++++++ 3 files changed, 122 insertions(+), 12 deletions(-) create mode 100644 app/test/features/events/error_pages_test.dart diff --git a/app/lib/features/events/pages/event_list_page.dart b/app/lib/features/events/pages/event_list_page.dart index 1878480733f6..f8949d8804d9 100644 --- a/app/lib/features/events/pages/event_list_page.dart +++ b/app/lib/features/events/pages/event_list_page.dart @@ -2,6 +2,7 @@ import 'dart:math'; import 'package:acter/common/providers/space_providers.dart'; import 'package:acter/common/toolkit/buttons/primary_action_button.dart'; +import 'package:acter/common/toolkit/errors/error_page.dart'; import 'package:acter/common/utils/routes.dart'; import 'package:acter/common/widgets/acter_search_widget.dart'; import 'package:acter/common/widgets/add_button_with_can_permission.dart'; @@ -34,7 +35,7 @@ class _EventListPageState extends ConsumerState { String get searchValue => ref.watch(searchValueProvider); - EventFilters get eventFilterValue => ref.watch(eventFilerProvider); + EventFilters get eventFilterValue => ref.watch(eventFilterProvider); @override Widget build(BuildContext context) { @@ -85,10 +86,19 @@ class _EventListPageState extends ConsumerState { Expanded( child: calEventsLoader.when( data: (calEvents) => _buildEventList(calEvents), - error: (e, s) { - _log.severe('Failed to search events in space', e, s); - return Center( - child: Text(L10n.of(context).searchingFailed(e)), + error: (error, stack) { + _log.severe('Failed to search events in space', error, stack); + return ErrorPage( + background: const EventListSkeleton(), + error: error, + stack: stack, + onRetryTap: () { + ref.invalidate( + eventListSearchFilterProvider( + (spaceId: widget.spaceId, searchText: searchValue), + ), + ); + }, ); }, loading: () => const EventListSkeleton(), @@ -109,7 +119,7 @@ class _EventListPageState extends ConsumerState { selected: eventFilterValue == EventFilters.all, label: Text(L10n.of(context).all), onSelected: (value) => ref - .read(eventFilerProvider.notifier) + .read(eventFilterProvider.notifier) .state = EventFilters.all, ), const SizedBox(width: 10), @@ -117,7 +127,7 @@ class _EventListPageState extends ConsumerState { selected: eventFilterValue == EventFilters.bookmarked, label: Text(L10n.of(context).bookmarked), onSelected: (value) => ref - .read(eventFilerProvider.notifier) + .read(eventFilterProvider.notifier) .state = EventFilters.bookmarked, ), const SizedBox(width: 10), @@ -125,7 +135,7 @@ class _EventListPageState extends ConsumerState { selected: eventFilterValue == EventFilters.ongoing, label: Text(L10n.of(context).happeningNow), onSelected: (value) => ref - .read(eventFilerProvider.notifier) + .read(eventFilterProvider.notifier) .state = EventFilters.ongoing, ), const SizedBox(width: 10), @@ -133,7 +143,7 @@ class _EventListPageState extends ConsumerState { selected: eventFilterValue == EventFilters.upcoming, label: Text(L10n.of(context).upcoming), onSelected: (value) => ref - .read(eventFilerProvider.notifier) + .read(eventFilterProvider.notifier) .state = EventFilters.upcoming, ), const SizedBox(width: 10), @@ -141,7 +151,7 @@ class _EventListPageState extends ConsumerState { selected: eventFilterValue == EventFilters.past, label: Text(L10n.of(context).past), onSelected: (value) => ref - .read(eventFilerProvider.notifier) + .read(eventFilterProvider.notifier) .state = EventFilters.past, ), ], diff --git a/app/lib/features/events/providers/event_providers.dart b/app/lib/features/events/providers/event_providers.dart index e7e52e0f1c6d..4fe22f4c7716 100644 --- a/app/lib/features/events/providers/event_providers.dart +++ b/app/lib/features/events/providers/event_providers.dart @@ -125,7 +125,7 @@ enum EventFilters { past, } -final eventFilerProvider = +final eventFilterProvider = StateProvider.autoDispose((ref) => EventFilters.all); //SEARCH EVENTS @@ -138,7 +138,7 @@ final eventListSearchFilterProvider = FutureProvider.autoDispose List filteredEventList = []; //Filter events based on the selection - EventFilters eventFilter = ref.watch(eventFilerProvider); + EventFilters eventFilter = ref.watch(eventFilterProvider); switch (eventFilter) { case EventFilters.bookmarked: { diff --git a/app/test/features/events/error_pages_test.dart b/app/test/features/events/error_pages_test.dart new file mode 100644 index 000000000000..88ddb517a10a --- /dev/null +++ b/app/test/features/events/error_pages_test.dart @@ -0,0 +1,100 @@ +import 'package:acter/common/providers/room_providers.dart'; +import 'package:acter/common/providers/space_providers.dart'; +import 'package:acter/common/widgets/acter_search_widget.dart'; +import 'package:acter/features/events/pages/event_list_page.dart'; +import 'package:acter/features/events/providers/event_providers.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../../helpers/error_helpers.dart'; +import '../../helpers/test_util.dart'; + +void main() { + group('Event List Error Pages', () { + testWidgets('full list', (tester) async { + bool shouldFail = true; + await tester.pumpProviderWidget( + overrides: [ + eventListSearchFilterProvider.overrideWith((a, b) { + if (shouldFail) { + // toggle failure so the retry works + shouldFail = !shouldFail; + throw 'Expected fail: Space not loaded'; + } + return []; + }), + hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), + ], + child: const EventListPage(), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + testWidgets('full list with search', (tester) async { + bool shouldFail = true; + + await tester.pumpProviderWidget( + overrides: [ + searchValueProvider + .overrideWith((_) => 'some string'), // set a search string + + eventListSearchFilterProvider.overrideWith((a, b) { + if (shouldFail) { + // toggle failure so the retry works + shouldFail = !shouldFail; + throw 'Expected fail: Space not loaded'; + } + return []; + }), + hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), + ], + child: const EventListPage(), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + + testWidgets('space list', (tester) async { + bool shouldFail = true; + await tester.pumpProviderWidget( + overrides: [ + roomDisplayNameProvider.overrideWith((a, b) => 'test'), + eventListSearchFilterProvider.overrideWith((a, b) { + if (shouldFail) { + // toggle failure so the retry works + shouldFail = !shouldFail; + throw 'Expected fail: Space not loaded'; + } + return []; + }), + hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), + ], + child: const EventListPage( + spaceId: '!test', + ), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + + testWidgets('space list with search', (tester) async { + bool shouldFail = true; + await tester.pumpProviderWidget( + overrides: [ + roomDisplayNameProvider.overrideWith((a, b) => 'test'), + searchValueProvider + .overrideWith((_) => 'some search'), // set a search string + eventListSearchFilterProvider.overrideWith((a, b) { + if (shouldFail) { + // toggle failure so the retry works + shouldFail = !shouldFail; + throw 'Expected fail: Space not loaded'; + } + return []; + }), + hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), + ], + child: const EventListPage( + spaceId: '!test', + ), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + }); +} From daf3417777f22bad8794e487e157997bf72a87a3 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Sun, 25 Aug 2024 16:51:52 +0100 Subject: [PATCH 5/6] Remove duplicate test --- app/test/features/tasks/error_pages_test.dart | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/app/test/features/tasks/error_pages_test.dart b/app/test/features/tasks/error_pages_test.dart index f327a9f8df87..5d5a7422cf61 100644 --- a/app/test/features/tasks/error_pages_test.dart +++ b/app/test/features/tasks/error_pages_test.dart @@ -55,21 +55,6 @@ void main() { await tester.ensureErrorPageWithRetryWorks(); }); - testWidgets('space list', (tester) async { - final mockedTaskListNotifier = MockAsyncAllTaskListsNotifier(); - await tester.pumpProviderWidget( - overrides: [ - roomDisplayNameProvider.overrideWith((a, b) => 'test'), - allTasksListsProvider.overrideWith(() => mockedTaskListNotifier), - hasSpaceWithPermissionProvider.overrideWith((_, ref) => false), - ], - child: const TasksListPage( - spaceId: '!test', - ), - ); - await tester.ensureErrorPageWithRetryWorks(); - }); - testWidgets('space list with search', (tester) async { final mockedTaskListNotifier = MockAsyncAllTaskListsNotifier(); await tester.pumpProviderWidget( From 2b5bbd41b223d515ab4a6c764339117ccc72f0f2 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Sun, 25 Aug 2024 17:34:42 +0100 Subject: [PATCH 6/6] Implement ErrorPage for Event Details, fixup RSVP provider into typed variant --- .../features/calendar_sync/calendar_sync.dart | 6 +- .../providers/events_to_sync_provider.dart | 7 +- .../events/pages/event_details_page.dart | 43 ++++------ .../events/providers/event_providers.dart | 8 +- .../providers/notifiers/rsvp_notifier.dart | 13 ++- .../features/events/widgets/event_item.dart | 36 ++++----- .../event_details_skeleton_widget.dart | 81 ++++++++----------- .../features/events/error_pages_test.dart | 24 ++++++ app/test/helpers/mock_event_providers.dart | 65 +++++++++++++++ 9 files changed, 172 insertions(+), 111 deletions(-) create mode 100644 app/test/helpers/mock_event_providers.dart diff --git a/app/lib/features/calendar_sync/calendar_sync.dart b/app/lib/features/calendar_sync/calendar_sync.dart index 7375031152f2..9f4eb30b8cbb 100644 --- a/app/lib/features/calendar_sync/calendar_sync.dart +++ b/app/lib/features/calendar_sync/calendar_sync.dart @@ -188,7 +188,7 @@ Future _refreshCalendar( Future _updateEventDetails( CalendarEvent acterEvent, - String? rsvp, + RsvpStatusTag? rsvp, Event localEvent, ) async { localEvent.title = acterEvent.title(); @@ -203,8 +203,8 @@ Future _updateEventDetails( UTC, ); localEvent.status = switch (rsvp) { - 'yes' => EventStatus.Confirmed, - 'maybe' => EventStatus.Tentative, + RsvpStatusTag.Yes => EventStatus.Confirmed, + RsvpStatusTag.Maybe => EventStatus.Tentative, _ => EventStatus.None }; return localEvent; diff --git a/app/lib/features/calendar_sync/providers/events_to_sync_provider.dart b/app/lib/features/calendar_sync/providers/events_to_sync_provider.dart index 6e901663e866..4ddf40171515 100644 --- a/app/lib/features/calendar_sync/providers/events_to_sync_provider.dart +++ b/app/lib/features/calendar_sync/providers/events_to_sync_provider.dart @@ -4,7 +4,7 @@ import 'package:acter/features/events/providers/event_providers.dart'; import 'package:acter_flutter_sdk/acter_flutter_sdk_ffi.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; -typedef EventAndRsvp = ({CalendarEvent event, String? rsvp}); +typedef EventAndRsvp = ({CalendarEvent event, RsvpStatusTag? rsvp}); final eventsToSyncProvider = FutureProvider.autoDispose((ref) async { // fetch all from all spaces @@ -19,13 +19,12 @@ final eventsToSyncProvider = FutureProvider.autoDispose((ref) async { for (final event in upcomingAndOngoing) { final eventId = event.eventId().toString(); final myRsvpStatus = await ref.watch(myRsvpStatusProvider(eventId).future); - final rsvpStatus = myRsvpStatus.statusStr(); - if (rsvpStatus != 'no') { + if (myRsvpStatus == RsvpStatusTag.No) { // we sync all that aren't denied yet final event = await ref.watch( calendarEventProvider(eventId).future, ); // ensure we are listening to updates of the events themselves - toSync.add((event: event, rsvp: rsvpStatus)); + toSync.add((event: event, rsvp: myRsvpStatus)); } } return toSync; diff --git a/app/lib/features/events/pages/event_details_page.dart b/app/lib/features/events/pages/event_details_page.dart index 6c807cc55126..4aef0d60e57f 100644 --- a/app/lib/features/events/pages/event_details_page.dart +++ b/app/lib/features/events/pages/event_details_page.dart @@ -4,6 +4,7 @@ import 'package:acter/common/actions/redact_content.dart'; import 'package:acter/common/actions/report_content.dart'; import 'package:acter/common/providers/common_providers.dart'; import 'package:acter/common/providers/room_providers.dart'; +import 'package:acter/common/toolkit/errors/error_page.dart'; import 'package:acter/common/utils/utils.dart'; import 'package:acter/common/widgets/edit_html_description_sheet.dart'; import 'package:acter/common/widgets/edit_title_sheet.dart'; @@ -72,9 +73,17 @@ class _EventDetailPageConsumerState extends ConsumerState { ], ); }, - error: (e, s) { - _log.severe('Failed to load cal event', e, s); - return Text(L10n.of(context).errorLoadingEventDueTo(e)); + error: (error, stack) { + _log.severe('Failed to load cal event', error, stack); + return ErrorPage( + background: const EventDetailsSkeleton(), + error: error, + stack: stack, + textBuilder: L10n.of(context).errorLoadingEventDueTo, + onRetryTap: () { + ref.invalidate(calendarEventProvider(widget.calendarId)); + }, + ); }, loading: () => const EventDetailsSkeleton(), ), @@ -366,27 +375,7 @@ class _EventDetailPageConsumerState extends ConsumerState { } Widget _buildEventRsvpActions(CalendarEvent calendarEvent) { - final myRsvpStatus = ref.watch(myRsvpStatusProvider(widget.calendarId)); - Set rsvp = {null}; - myRsvpStatus.maybeWhen( - data: (data) { - final status = data.statusStr(); - if (status != null) { - switch (status) { - case 'yes': - rsvp = {RsvpStatusTag.Yes}; - break; - case 'maybe': - rsvp = {RsvpStatusTag.Maybe}; - break; - case 'no': - rsvp = {RsvpStatusTag.No}; - break; - } - } - }, - orElse: () => null, - ); + final rsvp = ref.watch(myRsvpStatusProvider(widget.calendarId)).valueOrNull; return Container( color: Theme.of(context).colorScheme.surface, @@ -400,7 +389,7 @@ class _EventDetailPageConsumerState extends ConsumerState { iconData: Icons.check, actionName: L10n.of(context).going, rsvpStatusColor: Theme.of(context).colorScheme.secondary, - isSelected: rsvp.single == RsvpStatusTag.Yes, + isSelected: rsvp == RsvpStatusTag.Yes, ), _buildVerticalDivider(), _buildEventRsvpActionItem( @@ -410,7 +399,7 @@ class _EventDetailPageConsumerState extends ConsumerState { iconData: Icons.close, actionName: L10n.of(context).notGoing, rsvpStatusColor: Theme.of(context).colorScheme.error, - isSelected: rsvp.single == RsvpStatusTag.No, + isSelected: rsvp == RsvpStatusTag.No, ), _buildVerticalDivider(), _buildEventRsvpActionItem( @@ -420,7 +409,7 @@ class _EventDetailPageConsumerState extends ConsumerState { iconData: Icons.question_mark, actionName: L10n.of(context).maybe, rsvpStatusColor: Colors.white, - isSelected: rsvp.single == RsvpStatusTag.Maybe, + isSelected: rsvp == RsvpStatusTag.Maybe, ), ], ), diff --git a/app/lib/features/events/providers/event_providers.dart b/app/lib/features/events/providers/event_providers.dart index 4fe22f4c7716..8b9de574bfde 100644 --- a/app/lib/features/events/providers/event_providers.dart +++ b/app/lib/features/events/providers/event_providers.dart @@ -15,7 +15,7 @@ final calendarEventProvider = AsyncNotifierProvider.autoDispose //MY RSVP STATUS PROVIDER final myRsvpStatusProvider = AsyncNotifierProvider.autoDispose - .family( + .family( () => AsyncRsvpStatusNotifier(), ); @@ -57,7 +57,7 @@ final myOngoingEventListProvider = FutureProvider.autoDispose for (final event in allOngoingEventList) { final myRsvpStatus = await ref .watch(myRsvpStatusProvider(event.eventId().toString()).future); - if (myRsvpStatus.statusStr() == 'yes') { + if (myRsvpStatus == ffi.RsvpStatusTag.Yes) { myOngoingEventList.add(event); } } @@ -83,7 +83,7 @@ final myUpcomingEventListProvider = FutureProvider.autoDispose for (final event in allUpcomingEventList) { final myRsvpStatus = await ref .watch(myRsvpStatusProvider(event.eventId().toString()).future); - if (myRsvpStatus.statusStr() == 'yes') { + if (myRsvpStatus == ffi.RsvpStatusTag.Yes) { myUpcomingEventList.add(event); } } @@ -109,7 +109,7 @@ final myPastEventListProvider = FutureProvider.autoDispose for (final event in allPastEventList) { final myRsvpStatus = await ref .watch(myRsvpStatusProvider(event.eventId().toString()).future); - if (myRsvpStatus.statusStr() == 'yes') { + if (myRsvpStatus == ffi.RsvpStatusTag.Yes) { myPastEventList.add(event); } } diff --git a/app/lib/features/events/providers/notifiers/rsvp_notifier.dart b/app/lib/features/events/providers/notifiers/rsvp_notifier.dart index b18b11bdf607..f28c577e44db 100644 --- a/app/lib/features/events/providers/notifiers/rsvp_notifier.dart +++ b/app/lib/features/events/providers/notifiers/rsvp_notifier.dart @@ -4,17 +4,22 @@ import 'package:acter_flutter_sdk/acter_flutter_sdk_ffi.dart' as ffi; import 'package:riverpod/riverpod.dart'; class AsyncRsvpStatusNotifier - extends AutoDisposeFamilyAsyncNotifier { + extends AutoDisposeFamilyAsyncNotifier { late Stream _listener; - Future _getMyResponse() async { + Future _getMyResponse() async { final client = ref.read(alwaysClientProvider); final calEvent = await client.waitForCalendarEvent(arg, null); - return await calEvent.respondedByMe(); + return switch ((await calEvent.respondedByMe()).statusStr()) { + 'yes' => ffi.RsvpStatusTag.Yes, + 'no' => ffi.RsvpStatusTag.No, + 'maybe' => ffi.RsvpStatusTag.Maybe, + _ => null, + }; } @override - Future build(String arg) async { + Future build(String arg) async { final client = ref.watch(alwaysClientProvider); _listener = client.subscribeStream('$arg::rsvp'); // keep it resident in memory diff --git a/app/lib/features/events/widgets/event_item.dart b/app/lib/features/events/widgets/event_item.dart index ce307002afbc..373c1988b74a 100644 --- a/app/lib/features/events/widgets/event_item.dart +++ b/app/lib/features/events/widgets/event_item.dart @@ -6,7 +6,7 @@ import 'package:acter/features/events/actions/get_event_type.dart'; import 'package:acter/features/events/providers/event_providers.dart'; import 'package:acter/features/events/widgets/event_date_widget.dart'; import 'package:acter_flutter_sdk/acter_flutter_sdk_ffi.dart' - show CalendarEvent; + show CalendarEvent, RsvpStatusTag; import 'package:flutter/material.dart'; import 'package:flutter_gen/gen_l10n/l10n.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; @@ -110,8 +110,7 @@ class EventItem extends StatelessWidget { final eventId = event.eventId().toString(); final rsvpLoader = ref.watch(myRsvpStatusProvider(eventId)); return rsvpLoader.when( - data: (rsvp) { - final status = rsvp.statusStr(); // kebab-case + data: (status) { final widget = _getRsvpStatus(context, status); // kebab-case return widget ?? const SizedBox.shrink(); }, @@ -132,24 +131,19 @@ class EventItem extends StatelessWidget { ); } - Widget? _getRsvpStatus(BuildContext context, String? status) { - if (status != null) { - switch (status) { - case 'yes': - return Icon( - Icons.check_circle, - color: Theme.of(context).colorScheme.secondary, - ); - case 'no': - return Icon( - Icons.cancel, - color: Theme.of(context).colorScheme.error, - ); - case 'maybe': - return const Icon(Icons.question_mark_rounded); - } - } - return null; + Widget? _getRsvpStatus(BuildContext context, RsvpStatusTag? status) { + return switch (status) { + RsvpStatusTag.Yes => Icon( + Icons.check_circle, + color: Theme.of(context).colorScheme.secondary, + ), + RsvpStatusTag.No => Icon( + Icons.cancel, + color: Theme.of(context).colorScheme.error, + ), + RsvpStatusTag.Maybe => const Icon(Icons.question_mark_rounded), + null => null, + }; } Widget _buildHappeningIndication(BuildContext context) { diff --git a/app/lib/features/events/widgets/skeletons/event_details_skeleton_widget.dart b/app/lib/features/events/widgets/skeletons/event_details_skeleton_widget.dart index e5a0bcdc69a0..a98777d2f6f2 100644 --- a/app/lib/features/events/widgets/skeletons/event_details_skeleton_widget.dart +++ b/app/lib/features/events/widgets/skeletons/event_details_skeleton_widget.dart @@ -2,31 +2,23 @@ import 'package:flutter/material.dart'; import 'package:skeletonizer/skeletonizer.dart'; import 'package:flutter_gen/gen_l10n/l10n.dart'; -class EventDetailsSkeleton extends StatefulWidget { +class EventDetailsSkeleton extends StatelessWidget { const EventDetailsSkeleton({super.key}); - - @override - State createState() => _EventListSkeletonState(); -} - -class _EventListSkeletonState extends State { @override Widget build(BuildContext context) { - return Skeletonizer(child: _buildSkeletonUI()); - } - - Widget _buildSkeletonUI() { - return Column( - children: [ - _buildEventHeaderSkeletonUI(), - _buildEventBasicInfoSkeletonUI(), - _buildEventRsvpButtonsSkeletonUI(), - _buildEventAboutSkeletonUI(), - ], + return SingleChildScrollView( + child: Column( + children: [ + Skeletonizer(child: _buildEventHeaderSkeletonUI(context)), + Skeletonizer(child: _buildEventBasicInfoSkeletonUI(context)), + Skeletonizer(child: _buildEventRsvpButtonsSkeletonUI(context)), + Skeletonizer(child: _buildEventAboutSkeletonUI(context)), + ], + ), ); } - Widget _buildEventHeaderSkeletonUI() { + Widget _buildEventHeaderSkeletonUI(BuildContext context) { return Padding( padding: const EdgeInsets.all(10), child: Column( @@ -42,7 +34,7 @@ class _EventListSkeletonState extends State { ); } - Widget _buildEventBasicInfoSkeletonUI() { + Widget _buildEventBasicInfoSkeletonUI(BuildContext context) { return Padding( padding: const EdgeInsets.all(16), child: Row( @@ -53,14 +45,12 @@ class _EventListSkeletonState extends State { color: Colors.white, ), const SizedBox(width: 10), - Expanded( - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - Text(L10n.of(context).eventTitleData), - Text(L10n.of(context).eventDescriptionsData), - ], - ), + Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text(L10n.of(context).eventTitleData), + Text(L10n.of(context).eventDescriptionsData), + ], ), const SizedBox(width: 20), Text(L10n.of(context).rsvp), @@ -69,40 +59,35 @@ class _EventListSkeletonState extends State { ); } - Widget _buildEventRsvpButtonsSkeletonUI() { + Widget _buildEventRsvpButtonsSkeletonUI(BuildContext context) { return Padding( padding: const EdgeInsets.all(16), child: Row( + mainAxisAlignment: MainAxisAlignment.spaceEvenly, children: [ - Expanded( - child: Container( - height: 80, - width: 80, - color: Colors.white, - ), + Container( + height: 80, + width: 80, + color: Colors.white, ), const SizedBox(width: 20), - Expanded( - child: Container( - height: 80, - width: 80, - color: Colors.white, - ), + Container( + height: 80, + width: 80, + color: Colors.white, ), const SizedBox(width: 20), - Expanded( - child: Container( - height: 80, - width: 80, - color: Colors.white, - ), + Container( + height: 80, + width: 80, + color: Colors.white, ), ], ), ); } - Widget _buildEventAboutSkeletonUI() { + Widget _buildEventAboutSkeletonUI(BuildContext context) { return Padding( padding: const EdgeInsets.all(16), child: Container( diff --git a/app/test/features/events/error_pages_test.dart b/app/test/features/events/error_pages_test.dart index 88ddb517a10a..8568a7be0ce8 100644 --- a/app/test/features/events/error_pages_test.dart +++ b/app/test/features/events/error_pages_test.dart @@ -1,11 +1,15 @@ import 'package:acter/common/providers/room_providers.dart'; import 'package:acter/common/providers/space_providers.dart'; import 'package:acter/common/widgets/acter_search_widget.dart'; +import 'package:acter/features/bookmarks/providers/bookmarks_provider.dart'; +import 'package:acter/features/events/pages/event_details_page.dart'; import 'package:acter/features/events/pages/event_list_page.dart'; import 'package:acter/features/events/providers/event_providers.dart'; import 'package:flutter_test/flutter_test.dart'; import '../../helpers/error_helpers.dart'; +import '../../helpers/mock_event_providers.dart'; +import '../../helpers/mock_space_providers.dart'; import '../../helpers/test_util.dart'; void main() { @@ -97,4 +101,24 @@ void main() { await tester.ensureErrorPageWithRetryWorks(); }); }); + group('Event Details Error Pages', () { + testWidgets('body error page', (tester) async { + final mockedNofitier = MockAsyncCalendarEventNotifier(); + await tester.pumpProviderWidget( + overrides: [ + isBookmarkedProvider.overrideWith((a, b) => false), + roomAvatarInfoProvider + .overrideWith(() => MockRoomAvatarInfoNotifier()), + calendarEventProvider.overrideWith(() => mockedNofitier), + myRsvpStatusProvider + .overrideWith(() => MockAsyncRsvpStatusNotifier()), + roomMembershipProvider.overrideWith((a, b) => null), + ], + child: const EventDetailPage( + calendarId: '!asdf', + ), + ); + await tester.ensureErrorPageWithRetryWorks(); + }); + }); } diff --git a/app/test/helpers/mock_event_providers.dart b/app/test/helpers/mock_event_providers.dart new file mode 100644 index 000000000000..35002dcf1782 --- /dev/null +++ b/app/test/helpers/mock_event_providers.dart @@ -0,0 +1,65 @@ +import 'dart:async'; + +import 'package:acter/features/events/providers/notifiers/event_notifiers.dart'; +import 'package:acter/features/events/providers/notifiers/rsvp_notifier.dart'; +import 'package:acter_flutter_sdk/acter_flutter_sdk_ffi.dart'; +import 'package:mockito/mockito.dart'; +import 'package:riverpod/riverpod.dart'; + +class MockAsyncCalendarEventNotifier + extends AutoDisposeFamilyAsyncNotifier + with Mock + implements AsyncCalendarEventNotifier { + bool shouldFail; + + MockAsyncCalendarEventNotifier({this.shouldFail = true}); + + @override + Future build(String arg) async { + if (shouldFail) { + // toggle failure so the retry works + shouldFail = !shouldFail; + throw 'Expected fail: Space not loaded'; + } + return MockEvent(); + } +} + +class MockAsyncRsvpStatusNotifier + extends AutoDisposeFamilyAsyncNotifier + with Mock + implements AsyncRsvpStatusNotifier { + @override + Future build(String arg) async { + return null; + } +} + +class MockEvent extends Fake implements CalendarEvent { + @override + String roomIdStr() => 'testRoomId'; + @override + String title() => 'Fake Event'; + @override + TextMessageContent? description() => null; + @override + UtcDateTime utcStart() => FakeUtcDateTime(); + @override + UtcDateTime utcEnd() => FakeUtcDateTime(); + + @override + Future participants() => + Completer().future; + + @override + Future attachments() => + Completer().future; + + @override + Future comments() => Completer().future; +} + +class FakeUtcDateTime extends Fake implements UtcDateTime { + @override + int timestampMillis() => 10; +}