From e5a5bce4e7b9badc0f8c11c97143daafbc383248 Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 22 Nov 2025 15:46:44 +0100 Subject: [PATCH 01/10] feat(account): add pagination support for in-app notifications - Add loadingMore status to InAppNotificationCenterStatus enum - Extend InAppNotificationCenterState with pagination properties: - breakingNewsHasMore, breakingNewsCursor, digestHasMore, digestCursor - Update InAppNotificationCenterState copyWith method to include new parameters - Add comments explaining the new pagination-related properties --- .../in_app_notification_center_state.dart | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/lib/account/bloc/in_app_notification_center_state.dart b/lib/account/bloc/in_app_notification_center_state.dart index ded68a2e..8af0a2b7 100644 --- a/lib/account/bloc/in_app_notification_center_state.dart +++ b/lib/account/bloc/in_app_notification_center_state.dart @@ -8,6 +8,9 @@ enum InAppNotificationCenterStatus { /// The state when notifications are being loaded. loading, + /// The state when more notifications are being loaded for pagination. + loadingMore, + /// The state when notifications have been successfully loaded. success, @@ -25,6 +28,10 @@ class InAppNotificationCenterState extends Equatable { this.breakingNewsNotifications = const [], this.digestNotifications = const [], this.currentTabIndex = 0, + this.breakingNewsHasMore = true, + this.breakingNewsCursor, + this.digestHasMore = true, + this.digestCursor, this.error, }); @@ -50,12 +57,28 @@ class InAppNotificationCenterState extends Equatable { /// An error that occurred during notification loading or processing. final HttpException? error; + /// A flag indicating if there are more breaking news notifications to fetch. + final bool breakingNewsHasMore; + + /// The cursor for fetching the next page of breaking news notifications. + final String? breakingNewsCursor; + + /// A flag indicating if there are more digest notifications to fetch. + final bool digestHasMore; + + /// The cursor for fetching the next page of digest notifications. + final String? digestCursor; + @override List get props => [ status, currentTabIndex, breakingNewsNotifications, digestNotifications, + breakingNewsHasMore, + breakingNewsCursor ?? Object(), + digestHasMore, + digestCursor ?? Object(), error ?? Object(), // Include error in props, handle nullability ]; @@ -67,6 +90,10 @@ class InAppNotificationCenterState extends Equatable { int? currentTabIndex, List? breakingNewsNotifications, List? digestNotifications, + bool? breakingNewsHasMore, + String? breakingNewsCursor, + bool? digestHasMore, + String? digestCursor, }) { return InAppNotificationCenterState( status: status ?? this.status, @@ -75,6 +102,10 @@ class InAppNotificationCenterState extends Equatable { breakingNewsNotifications: breakingNewsNotifications ?? this.breakingNewsNotifications, digestNotifications: digestNotifications ?? this.digestNotifications, + breakingNewsHasMore: breakingNewsHasMore ?? this.breakingNewsHasMore, + breakingNewsCursor: breakingNewsCursor ?? this.breakingNewsCursor, + digestHasMore: digestHasMore ?? this.digestHasMore, + digestCursor: digestCursor ?? this.digestCursor, ); } } From a1c530f9db0d4bdec02934fd32efeb24d0cefe4e Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 22 Nov 2025 15:46:52 +0100 Subject: [PATCH 02/10] feat(account): add event for fetching more notifications Add a new event InAppNotificationCenterFetchMoreRequested to be dispatched when the user scrolls to the end of a notification list and more data needs to be fetched. --- lib/account/bloc/in_app_notification_center_event.dart | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/account/bloc/in_app_notification_center_event.dart b/lib/account/bloc/in_app_notification_center_event.dart index f44ca1e7..e791a0d3 100644 --- a/lib/account/bloc/in_app_notification_center_event.dart +++ b/lib/account/bloc/in_app_notification_center_event.dart @@ -55,3 +55,10 @@ class InAppNotificationCenterMarkOneAsRead @override List get props => [notificationId]; } + +/// Dispatched when the user scrolls to the end of a notification list and +/// more data needs to be fetched. +class InAppNotificationCenterFetchMoreRequested + extends InAppNotificationCenterEvent { + const InAppNotificationCenterFetchMoreRequested(); +} From a1deefac3c237a3c048499a739aaa7d0e9b21cf9 Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 22 Nov 2025 15:47:15 +0100 Subject: [PATCH 03/10] feat(account): add pagination to in-app notifications - Implement fetch more functionality for breaking news and digest tabs - Add loading state for pagination - Update state to include hasMore and cursor for each notification type - Refactor notification filtering into a separate method - Use bloc_concurrency's droppable transformer for fetch more events --- .../bloc/in_app_notification_center_bloc.dart | 149 +++++++++++++++--- 1 file changed, 131 insertions(+), 18 deletions(-) diff --git a/lib/account/bloc/in_app_notification_center_bloc.dart b/lib/account/bloc/in_app_notification_center_bloc.dart index d840620e..58ac0e04 100644 --- a/lib/account/bloc/in_app_notification_center_bloc.dart +++ b/lib/account/bloc/in_app_notification_center_bloc.dart @@ -1,5 +1,6 @@ import 'dart:async'; +import 'package:bloc_concurrency/bloc_concurrency.dart'; import 'package:bloc/bloc.dart'; import 'package:collection/collection.dart'; import 'package:core/core.dart'; @@ -35,6 +36,10 @@ class InAppNotificationCenterBloc on(_onMarkAllAsRead); on(_onTabChanged); on(_onMarkOneAsRead); + on( + _onFetchMoreRequested, + transformer: droppable(), + ); } final DataRepository _inAppNotificationRepository; @@ -42,6 +47,8 @@ class InAppNotificationCenterBloc final Logger _logger; /// Handles the request to load all notifications for the current user. + /// This now only fetches the first page of notifications. Subsequent pages + /// are loaded by [_onFetchMoreRequested]. Future _onSubscriptionRequested( InAppNotificationCenterSubscriptionRequested event, Emitter emit, @@ -58,6 +65,8 @@ class InAppNotificationCenterBloc try { final response = await _inAppNotificationRepository.readAll( userId: userId, + // Fetch the first page with a defined limit. + pagination: const PaginationOptions(limit: 20), sort: [const SortOption('createdAt', SortOrder.desc)], ); @@ -65,31 +74,28 @@ class InAppNotificationCenterBloc final breakingNews = []; final digests = []; + _filterAndAddNotifications( + notifications: allNotifications, + breakingNewsList: breakingNews, + digestList: digests, + ); - // Filter notifications into their respective categories, prioritizing - // 'notificationType' from the backend, then falling back to 'contentType'. - for (final n in allNotifications) { - final notificationType = n.payload.data['notificationType'] as String?; - final contentType = n.payload.data['contentType'] as String?; - - if (notificationType == - PushNotificationSubscriptionDeliveryType.dailyDigest.name || - notificationType == - PushNotificationSubscriptionDeliveryType.weeklyRoundup.name || - contentType == 'digest') { - digests.add(n); - } else { - // All other types (including 'breakingOnly' notificationType, - // 'headline' contentType, or any unknown types) go to breaking news. - breakingNews.add(n); - } - } + // Since we are fetching all notifications together and then filtering, + // the pagination cursor and hasMore status will be the same for both lists. + // This assumes the backend doesn't support filtering by notification type + // in the query itself. + final hasMore = response.hasMore; + final cursor = response.cursor; emit( state.copyWith( status: InAppNotificationCenterStatus.success, breakingNewsNotifications: breakingNews, digestNotifications: digests, + breakingNewsHasMore: hasMore, + breakingNewsCursor: cursor, + digestHasMore: hasMore, + digestCursor: cursor, ), ); } on HttpException catch (e, s) { @@ -112,6 +118,89 @@ class InAppNotificationCenterBloc } } + /// Handles fetching the next page of notifications when the user scrolls. + Future _onFetchMoreRequested( + InAppNotificationCenterFetchMoreRequested event, + Emitter emit, + ) async { + final isBreakingNewsTab = state.currentTabIndex == 0; + final hasMore = isBreakingNewsTab + ? state.breakingNewsHasMore + : state.digestHasMore; + + if (state.status == InAppNotificationCenterStatus.loadingMore || !hasMore) { + return; + } + + emit(state.copyWith(status: InAppNotificationCenterStatus.loadingMore)); + + final userId = _appBloc.state.user?.id; + if (userId == null) { + _logger.warning( + 'Cannot fetch more notifications: user is not logged in.', + ); + emit(state.copyWith(status: InAppNotificationCenterStatus.failure)); + return; + } + + final cursor = isBreakingNewsTab + ? state.breakingNewsCursor + : state.digestCursor; + + try { + final response = await _inAppNotificationRepository.readAll( + userId: userId, + pagination: PaginationOptions(limit: 20, cursor: cursor), + sort: [const SortOption('createdAt', SortOrder.desc)], + ); + + final newNotifications = response.items; + final newBreakingNews = []; + final newDigests = []; + + _filterAndAddNotifications( + notifications: newNotifications, + breakingNewsList: newBreakingNews, + digestList: newDigests, + ); + + final nextCursor = response.cursor; + final nextHasMore = response.hasMore; + + emit( + state.copyWith( + status: InAppNotificationCenterStatus.success, + breakingNewsNotifications: [ + ...state.breakingNewsNotifications, + ...newBreakingNews, + ], + digestNotifications: [...state.digestNotifications, ...newDigests], + breakingNewsHasMore: nextHasMore, + breakingNewsCursor: nextCursor, + digestHasMore: nextHasMore, + digestCursor: nextCursor, + ), + ); + } on HttpException catch (e, s) { + _logger.severe('Failed to fetch more in-app notifications.', e, s); + emit( + state.copyWith(status: InAppNotificationCenterStatus.failure, error: e), + ); + } catch (e, s) { + _logger.severe( + 'An unexpected error occurred while fetching more in-app notifications.', + e, + s, + ); + emit( + state.copyWith( + status: InAppNotificationCenterStatus.failure, + error: UnknownException(e.toString()), + ), + ); + } + } + /// Handles the event to change the active tab. Future _onTabChanged( InAppNotificationCenterTabChanged event, @@ -280,4 +369,28 @@ class InAppNotificationCenterBloc ); } } + + /// A helper method to filter a list of notifications into "Breaking News" + /// and "Digests" categories and add them to the provided lists. + void _filterAndAddNotifications({ + required List notifications, + required List breakingNewsList, + required List digestList, + }) { + for (final n in notifications) { + final notificationType = n.payload.data['notificationType'] as String?; + final contentType = n.payload.data['contentType'] as String?; + + if (notificationType == + PushNotificationSubscriptionDeliveryType.dailyDigest.name || + notificationType == + PushNotificationSubscriptionDeliveryType.weeklyRoundup.name || + contentType == 'digest') { + digestList.add(n); + } else { + // All other types go to breaking news. + breakingNewsList.add(n); + } + } + } } From 38872182699e9435022d82e24b47a65db9a525ac Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 22 Nov 2025 15:47:23 +0100 Subject: [PATCH 04/10] feat(account): enhance in-app notification center - Add loading more indicator and functionality - Improve empty state handling - Optimize tab switching and notification loading logic - Refactor notification list to support infinite scrolling --- .../view/in_app_notification_center_page.dart | 194 ++++++++++++------ 1 file changed, 129 insertions(+), 65 deletions(-) diff --git a/lib/account/view/in_app_notification_center_page.dart b/lib/account/view/in_app_notification_center_page.dart index e8d8af8a..198933b1 100644 --- a/lib/account/view/in_app_notification_center_page.dart +++ b/lib/account/view/in_app_notification_center_page.dart @@ -36,8 +36,8 @@ class _InAppNotificationCenterPageState ..addListener(() { if (!_tabController.indexIsChanging) { context.read().add( - InAppNotificationCenterTabChanged(_tabController.index), - ); + InAppNotificationCenterTabChanged(_tabController.index), + ); } }); } @@ -56,18 +56,16 @@ class _InAppNotificationCenterPageState appBar: AppBar( title: Text(l10n.notificationCenterPageTitle), actions: [ - BlocBuilder< - InAppNotificationCenterBloc, - InAppNotificationCenterState - >( + BlocBuilder( builder: (context, state) { final hasUnread = state.notifications.any((n) => !n.isRead); return IconButton( onPressed: hasUnread ? () { context.read().add( - const InAppNotificationCenterMarkAllAsRead(), - ); + const InAppNotificationCenterMarkAllAsRead(), + ); } : null, icon: const Icon(Icons.done_all), @@ -83,73 +81,124 @@ class _InAppNotificationCenterPageState ], ), ), - body: - BlocConsumer< - InAppNotificationCenterBloc, - InAppNotificationCenterState - >( - listener: (context, state) { - if (state.status == InAppNotificationCenterStatus.failure && - state.error != null) { - ScaffoldMessenger.of(context) - ..hideCurrentSnackBar() - ..showSnackBar( - SnackBar( - content: Text(state.error!.message), - backgroundColor: Theme.of(context).colorScheme.error, - ), - ); - } - }, - builder: (context, state) { - if (state.status == InAppNotificationCenterStatus.loading) { - return LoadingStateWidget( - icon: Icons.notifications_none_outlined, - headline: l10n.notificationCenterLoadingHeadline, - subheadline: l10n.notificationCenterLoadingSubheadline, - ); - } - - if (state.status == InAppNotificationCenterStatus.failure) { - return FailureStateWidget( - exception: - state.error ?? - OperationFailedException( - l10n.notificationCenterFailureHeadline, - ), - onRetry: () { - context.read().add( + body: BlocConsumer( + listener: (context, state) { + if (state.status == InAppNotificationCenterStatus.failure && + state.error != null) { + ScaffoldMessenger.of(context) + ..hideCurrentSnackBar() + ..showSnackBar( + SnackBar( + content: Text(state.error!.message), + backgroundColor: Theme.of(context).colorScheme.error, + ), + ); + } + }, + builder: (context, state) { + if (state.status == InAppNotificationCenterStatus.loading && + state.breakingNewsNotifications.isEmpty && + state.digestNotifications.isEmpty) { + return LoadingStateWidget( + icon: Icons.notifications_none_outlined, + headline: l10n.notificationCenterLoadingHeadline, + subheadline: l10n.notificationCenterLoadingSubheadline, + ); + } + + if (state.status == InAppNotificationCenterStatus.failure && + state.breakingNewsNotifications.isEmpty && + state.digestNotifications.isEmpty) { + return FailureStateWidget( + exception: state.error ?? + OperationFailedException( + l10n.notificationCenterFailureHeadline, + ), + onRetry: () { + context.read().add( const InAppNotificationCenterSubscriptionRequested(), ); - }, - ); - } + }, + ); + } - return TabBarView( - controller: _tabController, - children: [ - _NotificationList( - notifications: state.breakingNewsNotifications, - ), - _NotificationList(notifications: state.digestNotifications), - ], - ); - }, - ), + return TabBarView( + controller: _tabController, + children: [ + _NotificationList( + status: state.status, + notifications: state.breakingNewsNotifications, + hasMore: state.breakingNewsHasMore, + ), + _NotificationList( + status: state.status, + notifications: state.digestNotifications, + hasMore: state.digestHasMore, + ), + ], + ); + }, + ), ); } } -class _NotificationList extends StatelessWidget { - const _NotificationList({required this.notifications}); +class _NotificationList extends StatefulWidget { + const _NotificationList({ + required this.notifications, + required this.hasMore, + required this.status, + }); + final InAppNotificationCenterStatus status; final List notifications; + final bool hasMore; + + @override + State<_NotificationList> createState() => _NotificationListState(); +} + +class _NotificationListState extends State<_NotificationList> { + final _scrollController = ScrollController(); + + @override + void initState() { + super.initState(); + _scrollController.addListener(_onScroll); + } + + @override + void dispose() { + _scrollController + ..removeListener(_onScroll) + ..dispose(); + super.dispose(); + } + + void _onScroll() { + final bloc = context.read(); + if (_isBottom && + widget.hasMore && + bloc.state.status != InAppNotificationCenterStatus.loadingMore) { + bloc.add(const InAppNotificationCenterFetchMoreRequested()); + } + } + + bool get _isBottom { + if (!_scrollController.hasClients) return false; + final maxScroll = _scrollController.position.maxScrollExtent; + final currentScroll = _scrollController.offset; + return currentScroll >= (maxScroll * 0.98); + } @override Widget build(BuildContext context) { final l10n = AppLocalizationsX(context).l10n; - if (notifications.isEmpty) { + // Show empty state only if not in the middle of an initial load. + if (widget.notifications.isEmpty && + widget.status != InAppNotificationCenterStatus.loading) { return InitialStateWidget( icon: Icons.notifications_off_outlined, headline: l10n.notificationCenterEmptyHeadline, @@ -158,16 +207,31 @@ class _NotificationList extends StatelessWidget { } return ListView.separated( - itemCount: notifications.length, + controller: _scrollController, + itemCount: widget.hasMore + ? widget.notifications.length + 1 + : widget.notifications.length, separatorBuilder: (context, index) => const Divider(height: 1), itemBuilder: (context, index) { - final notification = notifications[index]; + if (index >= widget.notifications.length) { + return widget.status == InAppNotificationCenterStatus.loadingMore + ? const Padding( + padding: EdgeInsets.symmetric( + vertical: AppSpacing.lg, + ), + child: Center( + child: CircularProgressIndicator(), + ), + ) + : const SizedBox.shrink(); + } + final notification = widget.notifications[index]; return InAppNotificationListItem( notification: notification, onTap: () async { context.read().add( - InAppNotificationCenterMarkedAsRead(notification.id), - ); + InAppNotificationCenterMarkedAsRead(notification.id), + ); final payload = notification.payload; final contentType = payload.data['contentType'] as String?; From 215def247261697144c00591a398caa4ce1d3795 Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 22 Nov 2025 16:09:50 +0100 Subject: [PATCH 05/10] feat(account): add pagination limit constant and update fetch logic - Introduce _notificationsFetchLimit constant for notification pagination - Update pagination limit from 20 to _notificationsFetchLimit in both initial fetch and load more operations - Improve code maintainability and readability by using a constant for pagination limit --- lib/account/bloc/in_app_notification_center_bloc.dart | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/account/bloc/in_app_notification_center_bloc.dart b/lib/account/bloc/in_app_notification_center_bloc.dart index 58ac0e04..3850e88a 100644 --- a/lib/account/bloc/in_app_notification_center_bloc.dart +++ b/lib/account/bloc/in_app_notification_center_bloc.dart @@ -22,6 +22,9 @@ part 'in_app_notification_center_state.dart'; /// {@endtemplate} class InAppNotificationCenterBloc extends Bloc { + /// The number of notifications to fetch per page. + static const _notificationsFetchLimit = 10; + /// {@macro in_app_notification_center_bloc} InAppNotificationCenterBloc({ required DataRepository inAppNotificationRepository, @@ -66,7 +69,7 @@ class InAppNotificationCenterBloc final response = await _inAppNotificationRepository.readAll( userId: userId, // Fetch the first page with a defined limit. - pagination: const PaginationOptions(limit: 20), + pagination: const PaginationOptions(limit: _notificationsFetchLimit), sort: [const SortOption('createdAt', SortOrder.desc)], ); @@ -150,7 +153,10 @@ class InAppNotificationCenterBloc try { final response = await _inAppNotificationRepository.readAll( userId: userId, - pagination: PaginationOptions(limit: 20, cursor: cursor), + pagination: PaginationOptions( + limit: _notificationsFetchLimit, + cursor: cursor, + ), sort: [const SortOption('createdAt', SortOrder.desc)], ); From 1740bb955f0ee63843b3a68fef8e01f628dae45e Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 22 Nov 2025 16:36:27 +0100 Subject: [PATCH 06/10] build(deps): update core dependency to latest commit - Update core dependency in pubspec.yaml and pubspec.lock - Change ref from b01f9b457d4faaa6fab58f67ebc09d93314e6b10 to 8a03af9af8ccf4920388ddce73062aca2fde1bc9 - This update ensures the project uses the latest version of the core dependency --- pubspec.lock | 4 ++-- pubspec.yaml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pubspec.lock b/pubspec.lock index 34844ee3..2a501857 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -185,8 +185,8 @@ packages: dependency: "direct main" description: path: "." - ref: b01f9b457d4faaa6fab58f67ebc09d93314e6b10 - resolved-ref: b01f9b457d4faaa6fab58f67ebc09d93314e6b10 + ref: "8a03af9af8ccf4920388ddce73062aca2fde1bc9" + resolved-ref: "8a03af9af8ccf4920388ddce73062aca2fde1bc9" url: "https://github.com/flutter-news-app-full-source-code/core.git" source: git version: "1.3.1" diff --git a/pubspec.yaml b/pubspec.yaml index a00c8646..5c701f37 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -145,7 +145,7 @@ dependency_overrides: core: git: url: https://github.com/flutter-news-app-full-source-code/core.git - ref: b01f9b457d4faaa6fab58f67ebc09d93314e6b10 + ref: 8a03af9af8ccf4920388ddce73062aca2fde1bc9 http_client: git: url: https://github.com/flutter-news-app-full-source-code/http-client.git From 5c279f74b3d7605b5d842de04f7bc5aa9b6ac42c Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 22 Nov 2025 16:57:35 +0100 Subject: [PATCH 07/10] fix(account): allow null values for notification cursors and error - Update InAppNotificationCenterState to handle null values for notification cursors - Enable explicit setting of error to null - Use nullable wrapper for breakingNewsCursor and digestCursor - Allow redundant argument value for error to support null assignment --- .../in_app_notification_center_state.dart | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/account/bloc/in_app_notification_center_state.dart b/lib/account/bloc/in_app_notification_center_state.dart index 8af0a2b7..95cb6b14 100644 --- a/lib/account/bloc/in_app_notification_center_state.dart +++ b/lib/account/bloc/in_app_notification_center_state.dart @@ -91,21 +91,30 @@ class InAppNotificationCenterState extends Equatable { List? breakingNewsNotifications, List? digestNotifications, bool? breakingNewsHasMore, - String? breakingNewsCursor, + // Use a nullable wrapper to explicitly set the cursor to null. + Object? breakingNewsCursor, bool? digestHasMore, - String? digestCursor, + Object? digestCursor, }) { return InAppNotificationCenterState( status: status ?? this.status, - error: error ?? this.error, + // Allow explicitly setting the error to null. + // ignore: avoid_redundant_argument_values + error: error, currentTabIndex: currentTabIndex ?? this.currentTabIndex, breakingNewsNotifications: breakingNewsNotifications ?? this.breakingNewsNotifications, digestNotifications: digestNotifications ?? this.digestNotifications, breakingNewsHasMore: breakingNewsHasMore ?? this.breakingNewsHasMore, - breakingNewsCursor: breakingNewsCursor ?? this.breakingNewsCursor, + breakingNewsCursor: breakingNewsCursor == null + ? this + .breakingNewsCursor // No change + : breakingNewsCursor as String?, // New value digestHasMore: digestHasMore ?? this.digestHasMore, - digestCursor: digestCursor ?? this.digestCursor, + digestCursor: digestCursor == null + ? this + .digestCursor // No change + : digestCursor as String?, // New value ); } } From b097e985c0525933b5f63e6386bd5d7b7aec815d Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 22 Nov 2025 16:57:50 +0100 Subject: [PATCH 08/10] refactor(account): improve in-app notification fetching logic - Separate fetching logic for "Breaking News" and "Digests" tabs - Implement concurrent fetching for both tabs on initial load - Use filtering to request specific notification types from the backend - Simplify notification fetching and error handling - Add droppable transformer to subscription request event handling --- .../bloc/in_app_notification_center_bloc.dart | 258 +++++++++--------- 1 file changed, 124 insertions(+), 134 deletions(-) diff --git a/lib/account/bloc/in_app_notification_center_bloc.dart b/lib/account/bloc/in_app_notification_center_bloc.dart index 3850e88a..411b7c75 100644 --- a/lib/account/bloc/in_app_notification_center_bloc.dart +++ b/lib/account/bloc/in_app_notification_center_bloc.dart @@ -27,6 +27,8 @@ class InAppNotificationCenterBloc /// {@macro in_app_notification_center_bloc} InAppNotificationCenterBloc({ + // The BLoC should not be responsible for creating its own dependencies. + // They should be provided from the outside. required DataRepository inAppNotificationRepository, required AppBloc appBloc, required Logger logger, @@ -34,7 +36,10 @@ class InAppNotificationCenterBloc _appBloc = appBloc, _logger = logger, super(const InAppNotificationCenterState()) { - on(_onSubscriptionRequested); + on( + _onSubscriptionRequested, + transformer: droppable(), + ); on(_onMarkedAsRead); on(_onMarkAllAsRead); on(_onTabChanged); @@ -49,79 +54,43 @@ class InAppNotificationCenterBloc final AppBloc _appBloc; final Logger _logger; - /// Handles the request to load all notifications for the current user. - /// This now only fetches the first page of notifications. Subsequent pages - /// are loaded by [_onFetchMoreRequested]. + /// Handles the initial subscription request to fetch notifications for both + /// tabs concurrently. Future _onSubscriptionRequested( InAppNotificationCenterSubscriptionRequested event, Emitter emit, ) async { emit(state.copyWith(status: InAppNotificationCenterStatus.loading)); - final userId = _appBloc.state.user?.id; if (userId == null) { - _logger.warning('Cannot fetch notifications: user is not logged in.'); + _logger.warning( + 'Cannot fetch more notifications: user is not logged in.', + ); emit(state.copyWith(status: InAppNotificationCenterStatus.failure)); return; } - try { - final response = await _inAppNotificationRepository.readAll( + // Fetch both tabs' initial data in parallel. + await Future.wait([ + _fetchNotifications( + emit: emit, userId: userId, - // Fetch the first page with a defined limit. - pagination: const PaginationOptions(limit: _notificationsFetchLimit), - sort: [const SortOption('createdAt', SortOrder.desc)], - ); - - final allNotifications = response.items; - - final breakingNews = []; - final digests = []; - _filterAndAddNotifications( - notifications: allNotifications, - breakingNewsList: breakingNews, - digestList: digests, - ); - - // Since we are fetching all notifications together and then filtering, - // the pagination cursor and hasMore status will be the same for both lists. - // This assumes the backend doesn't support filtering by notification type - // in the query itself. - final hasMore = response.hasMore; - final cursor = response.cursor; + filter: _breakingNewsFilter, + isInitialFetch: true, + ), + _fetchNotifications( + emit: emit, + userId: userId, + filter: _digestFilter, + isInitialFetch: true, + ), + ]); - emit( - state.copyWith( - status: InAppNotificationCenterStatus.success, - breakingNewsNotifications: breakingNews, - digestNotifications: digests, - breakingNewsHasMore: hasMore, - breakingNewsCursor: cursor, - digestHasMore: hasMore, - digestCursor: cursor, - ), - ); - } on HttpException catch (e, s) { - _logger.severe('Failed to fetch in-app notifications.', e, s); - emit( - state.copyWith(status: InAppNotificationCenterStatus.failure, error: e), - ); - } catch (e, s) { - _logger.severe( - 'An unexpected error occurred while fetching in-app notifications.', - e, - s, - ); - emit( - state.copyWith( - status: InAppNotificationCenterStatus.failure, - error: UnknownException(e.toString()), - ), - ); - } + // After both fetches are complete, set the status to success. + emit(state.copyWith(status: InAppNotificationCenterStatus.success)); } - /// Handles fetching the next page of notifications when the user scrolls. + /// Handles fetching the next page of notifications for the current tab. Future _onFetchMoreRequested( InAppNotificationCenterFetchMoreRequested event, Emitter emit, @@ -146,65 +115,20 @@ class InAppNotificationCenterBloc return; } + final filter = isBreakingNewsTab ? _breakingNewsFilter : _digestFilter; final cursor = isBreakingNewsTab ? state.breakingNewsCursor : state.digestCursor; - try { - final response = await _inAppNotificationRepository.readAll( - userId: userId, - pagination: PaginationOptions( - limit: _notificationsFetchLimit, - cursor: cursor, - ), - sort: [const SortOption('createdAt', SortOrder.desc)], - ); - - final newNotifications = response.items; - final newBreakingNews = []; - final newDigests = []; - - _filterAndAddNotifications( - notifications: newNotifications, - breakingNewsList: newBreakingNews, - digestList: newDigests, - ); - - final nextCursor = response.cursor; - final nextHasMore = response.hasMore; + await _fetchNotifications( + emit: emit, + userId: userId, + filter: filter, + cursor: cursor, + ); - emit( - state.copyWith( - status: InAppNotificationCenterStatus.success, - breakingNewsNotifications: [ - ...state.breakingNewsNotifications, - ...newBreakingNews, - ], - digestNotifications: [...state.digestNotifications, ...newDigests], - breakingNewsHasMore: nextHasMore, - breakingNewsCursor: nextCursor, - digestHasMore: nextHasMore, - digestCursor: nextCursor, - ), - ); - } on HttpException catch (e, s) { - _logger.severe('Failed to fetch more in-app notifications.', e, s); - emit( - state.copyWith(status: InAppNotificationCenterStatus.failure, error: e), - ); - } catch (e, s) { - _logger.severe( - 'An unexpected error occurred while fetching more in-app notifications.', - e, - s, - ); - emit( - state.copyWith( - status: InAppNotificationCenterStatus.failure, - error: UnknownException(e.toString()), - ), - ); - } + // After fetch, set status back to success. + emit(state.copyWith(status: InAppNotificationCenterStatus.success)); } /// Handles the event to change the active tab. @@ -212,6 +136,8 @@ class InAppNotificationCenterBloc InAppNotificationCenterTabChanged event, Emitter emit, ) async { + // If the tab is changed, we don't need to re-fetch data as it was + // already fetched on initial load. We just update the index. emit(state.copyWith(currentTabIndex: event.tabIndex)); } @@ -376,27 +302,91 @@ class InAppNotificationCenterBloc } } - /// A helper method to filter a list of notifications into "Breaking News" - /// and "Digests" categories and add them to the provided lists. - void _filterAndAddNotifications({ - required List notifications, - required List breakingNewsList, - required List digestList, + /// A generic method to fetch notifications based on a filter. + Future _fetchNotifications({ + required Emitter emit, + required String userId, + required Map filter, + String? cursor, + bool isInitialFetch = false, }) { - for (final n in notifications) { - final notificationType = n.payload.data['notificationType'] as String?; - final contentType = n.payload.data['contentType'] as String?; - - if (notificationType == - PushNotificationSubscriptionDeliveryType.dailyDigest.name || - notificationType == - PushNotificationSubscriptionDeliveryType.weeklyRoundup.name || - contentType == 'digest') { - digestList.add(n); - } else { - // All other types go to breaking news. - breakingNewsList.add(n); - } - } + final isBreakingNewsFilter = filter == _breakingNewsFilter; + + return _inAppNotificationRepository + .readAll( + userId: userId, + filter: filter, + pagination: PaginationOptions( + limit: _notificationsFetchLimit, + cursor: cursor, + ), + sort: [const SortOption('createdAt', SortOrder.desc)], + ) + .then((response) { + final newNotifications = response.items; + final nextCursor = response.cursor; + final hasMore = response.hasMore; + + if (isBreakingNewsFilter) { + emit( + state.copyWith( + breakingNewsNotifications: isInitialFetch + ? newNotifications + : [...state.breakingNewsNotifications, ...newNotifications], + breakingNewsHasMore: hasMore, + breakingNewsCursor: nextCursor, + ), + ); + } else { + emit( + state.copyWith( + digestNotifications: isInitialFetch + ? newNotifications + : [...state.digestNotifications, ...newNotifications], + digestHasMore: hasMore, + digestCursor: nextCursor, + ), + ); + } + }) + .catchError((Object e, StackTrace s) { + _logger.severe('Failed to fetch notifications.', e, s); + final httpException = e is HttpException + ? e + : UnknownException(e.toString()); + emit( + state.copyWith( + status: InAppNotificationCenterStatus.failure, + error: httpException, + ), + ); + }); } + + /// Filter for "Breaking News" notifications. + /// + /// This filter uses the `$nin` (not in) operator to exclude notifications + /// that are explicitly typed as digests. All other notifications are + /// considered "breaking news" for the purpose of this tab. + Map get _breakingNewsFilter => { + 'payload.data.notificationType': { + r'$nin': [ + PushNotificationSubscriptionDeliveryType.dailyDigest.name, + PushNotificationSubscriptionDeliveryType.weeklyRoundup.name, + ], + }, + }; + + /// Filter for "Digests" notifications. + /// + /// This filter uses the `$in` operator to select notifications that are + /// explicitly typed as either a daily or weekly digest. + Map get _digestFilter => { + 'payload.data.notificationType': { + r'$in': [ + PushNotificationSubscriptionDeliveryType.dailyDigest.name, + PushNotificationSubscriptionDeliveryType.weeklyRoundup.name, + ], + }, + }; } From 63b767f96a0579d5d7b38f92a7adf8281e08d4e6 Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 22 Nov 2025 17:06:18 +0100 Subject: [PATCH 09/10] refactor(account): improve error handling and state updates in notifications - Refactor notification fetching to use a centralized error handler - Optimize state updates by atomically updating both tabs' data - Improve pagination by updating the correct tab's state after fetching more notifications - Remove unnecessary parameters and simplify the _fetchNotifications method --- .../bloc/in_app_notification_center_bloc.dart | 170 +++++++++--------- 1 file changed, 90 insertions(+), 80 deletions(-) diff --git a/lib/account/bloc/in_app_notification_center_bloc.dart b/lib/account/bloc/in_app_notification_center_bloc.dart index 411b7c75..29d9b428 100644 --- a/lib/account/bloc/in_app_notification_center_bloc.dart +++ b/lib/account/bloc/in_app_notification_center_bloc.dart @@ -70,24 +70,31 @@ class InAppNotificationCenterBloc return; } - // Fetch both tabs' initial data in parallel. - await Future.wait([ - _fetchNotifications( - emit: emit, - userId: userId, - filter: _breakingNewsFilter, - isInitialFetch: true, - ), - _fetchNotifications( - emit: emit, - userId: userId, - filter: _digestFilter, - isInitialFetch: true, - ), - ]); + try { + // Fetch both tabs' initial data in parallel and wait for their results. + final results = await Future.wait([ + _fetchNotifications(userId: userId, filter: _breakingNewsFilter), + _fetchNotifications(userId: userId, filter: _digestFilter), + ]); + + final breakingNewsResponse = results[0]; + final digestResponse = results[1]; - // After both fetches are complete, set the status to success. - emit(state.copyWith(status: InAppNotificationCenterStatus.success)); + // Perform a single, atomic state update with both results. + emit( + state.copyWith( + status: InAppNotificationCenterStatus.success, + breakingNewsNotifications: breakingNewsResponse.items, + breakingNewsHasMore: breakingNewsResponse.hasMore, + breakingNewsCursor: breakingNewsResponse.cursor, + digestNotifications: digestResponse.items, + digestHasMore: digestResponse.hasMore, + digestCursor: digestResponse.cursor, + ), + ); + } catch (error, stackTrace) { + _handleFetchError(emit, error, stackTrace); + } } /// Handles fetching the next page of notifications for the current tab. @@ -120,15 +127,42 @@ class InAppNotificationCenterBloc ? state.breakingNewsCursor : state.digestCursor; - await _fetchNotifications( - emit: emit, - userId: userId, - filter: filter, - cursor: cursor, - ); + try { + final response = await _fetchNotifications( + userId: userId, + filter: filter, + cursor: cursor, + ); - // After fetch, set status back to success. - emit(state.copyWith(status: InAppNotificationCenterStatus.success)); + // Append the new items to the correct list. + if (isBreakingNewsTab) { + emit( + state.copyWith( + status: InAppNotificationCenterStatus.success, + breakingNewsNotifications: [ + ...state.breakingNewsNotifications, + ...response.items, + ], + breakingNewsHasMore: response.hasMore, + breakingNewsCursor: response.cursor, + ), + ); + } else { + emit( + state.copyWith( + status: InAppNotificationCenterStatus.success, + digestNotifications: [ + ...state.digestNotifications, + ...response.items, + ], + digestHasMore: response.hasMore, + digestCursor: response.cursor, + ), + ); + } + } catch (error, stackTrace) { + _handleFetchError(emit, error, stackTrace); + } } /// Handles the event to change the active tab. @@ -303,64 +337,22 @@ class InAppNotificationCenterBloc } /// A generic method to fetch notifications based on a filter. - Future _fetchNotifications({ - required Emitter emit, + Future> _fetchNotifications({ required String userId, required Map filter, String? cursor, - bool isInitialFetch = false, - }) { - final isBreakingNewsFilter = filter == _breakingNewsFilter; - - return _inAppNotificationRepository - .readAll( - userId: userId, - filter: filter, - pagination: PaginationOptions( - limit: _notificationsFetchLimit, - cursor: cursor, - ), - sort: [const SortOption('createdAt', SortOrder.desc)], - ) - .then((response) { - final newNotifications = response.items; - final nextCursor = response.cursor; - final hasMore = response.hasMore; - - if (isBreakingNewsFilter) { - emit( - state.copyWith( - breakingNewsNotifications: isInitialFetch - ? newNotifications - : [...state.breakingNewsNotifications, ...newNotifications], - breakingNewsHasMore: hasMore, - breakingNewsCursor: nextCursor, - ), - ); - } else { - emit( - state.copyWith( - digestNotifications: isInitialFetch - ? newNotifications - : [...state.digestNotifications, ...newNotifications], - digestHasMore: hasMore, - digestCursor: nextCursor, - ), - ); - } - }) - .catchError((Object e, StackTrace s) { - _logger.severe('Failed to fetch notifications.', e, s); - final httpException = e is HttpException - ? e - : UnknownException(e.toString()); - emit( - state.copyWith( - status: InAppNotificationCenterStatus.failure, - error: httpException, - ), - ); - }); + }) async { + // This method now simply fetches and returns the data, or throws on error. + // The responsibility of emitting state is moved to the event handlers. + return _inAppNotificationRepository.readAll( + userId: userId, + filter: filter, + pagination: PaginationOptions( + limit: _notificationsFetchLimit, + cursor: cursor, + ), + sort: [const SortOption('createdAt', SortOrder.desc)], + ); } /// Filter for "Breaking News" notifications. @@ -389,4 +381,22 @@ class InAppNotificationCenterBloc ], }, }; + + /// Centralized error handler for fetch operations. + void _handleFetchError( + Emitter emit, + Object error, + StackTrace stackTrace, + ) { + _logger.severe('Failed to fetch notifications.', error, stackTrace); + final httpException = error is HttpException + ? error + : UnknownException(error.toString()); + emit( + state.copyWith( + status: InAppNotificationCenterStatus.failure, + error: httpException, + ), + ); + } } From e29baf22f80f374f949e5c93ccc5d3f513bb5dee Mon Sep 17 00:00:00 2001 From: fulleni Date: Sat, 22 Nov 2025 17:21:40 +0100 Subject: [PATCH 10/10] style: format --- .../bloc/in_app_notification_center_bloc.dart | 10 +- .../in_app_notification_center_state.dart | 10 +- .../view/in_app_notification_center_page.dart | 140 +++++++++--------- 3 files changed, 79 insertions(+), 81 deletions(-) diff --git a/lib/account/bloc/in_app_notification_center_bloc.dart b/lib/account/bloc/in_app_notification_center_bloc.dart index 29d9b428..6d54b468 100644 --- a/lib/account/bloc/in_app_notification_center_bloc.dart +++ b/lib/account/bloc/in_app_notification_center_bloc.dart @@ -1,7 +1,7 @@ import 'dart:async'; -import 'package:bloc_concurrency/bloc_concurrency.dart'; import 'package:bloc/bloc.dart'; +import 'package:bloc_concurrency/bloc_concurrency.dart'; import 'package:collection/collection.dart'; import 'package:core/core.dart'; import 'package:data_repository/data_repository.dart'; @@ -22,13 +22,8 @@ part 'in_app_notification_center_state.dart'; /// {@endtemplate} class InAppNotificationCenterBloc extends Bloc { - /// The number of notifications to fetch per page. - static const _notificationsFetchLimit = 10; - /// {@macro in_app_notification_center_bloc} InAppNotificationCenterBloc({ - // The BLoC should not be responsible for creating its own dependencies. - // They should be provided from the outside. required DataRepository inAppNotificationRepository, required AppBloc appBloc, required Logger logger, @@ -50,6 +45,9 @@ class InAppNotificationCenterBloc ); } + /// The number of notifications to fetch per page. + static const _notificationsFetchLimit = 10; + final DataRepository _inAppNotificationRepository; final AppBloc _appBloc; final Logger _logger; diff --git a/lib/account/bloc/in_app_notification_center_state.dart b/lib/account/bloc/in_app_notification_center_state.dart index 95cb6b14..2aba115a 100644 --- a/lib/account/bloc/in_app_notification_center_state.dart +++ b/lib/account/bloc/in_app_notification_center_state.dart @@ -107,14 +107,12 @@ class InAppNotificationCenterState extends Equatable { digestNotifications: digestNotifications ?? this.digestNotifications, breakingNewsHasMore: breakingNewsHasMore ?? this.breakingNewsHasMore, breakingNewsCursor: breakingNewsCursor == null - ? this - .breakingNewsCursor // No change - : breakingNewsCursor as String?, // New value + ? this.breakingNewsCursor + : breakingNewsCursor as String?, digestHasMore: digestHasMore ?? this.digestHasMore, digestCursor: digestCursor == null - ? this - .digestCursor // No change - : digestCursor as String?, // New value + ? this.digestCursor + : digestCursor as String?, ); } } diff --git a/lib/account/view/in_app_notification_center_page.dart b/lib/account/view/in_app_notification_center_page.dart index 198933b1..c3bf325d 100644 --- a/lib/account/view/in_app_notification_center_page.dart +++ b/lib/account/view/in_app_notification_center_page.dart @@ -36,8 +36,8 @@ class _InAppNotificationCenterPageState ..addListener(() { if (!_tabController.indexIsChanging) { context.read().add( - InAppNotificationCenterTabChanged(_tabController.index), - ); + InAppNotificationCenterTabChanged(_tabController.index), + ); } }); } @@ -56,16 +56,18 @@ class _InAppNotificationCenterPageState appBar: AppBar( title: Text(l10n.notificationCenterPageTitle), actions: [ - BlocBuilder( + BlocBuilder< + InAppNotificationCenterBloc, + InAppNotificationCenterState + >( builder: (context, state) { final hasUnread = state.notifications.any((n) => !n.isRead); return IconButton( onPressed: hasUnread ? () { context.read().add( - const InAppNotificationCenterMarkAllAsRead(), - ); + const InAppNotificationCenterMarkAllAsRead(), + ); } : null, icon: const Icon(Icons.done_all), @@ -81,65 +83,69 @@ class _InAppNotificationCenterPageState ], ), ), - body: BlocConsumer( - listener: (context, state) { - if (state.status == InAppNotificationCenterStatus.failure && - state.error != null) { - ScaffoldMessenger.of(context) - ..hideCurrentSnackBar() - ..showSnackBar( - SnackBar( - content: Text(state.error!.message), - backgroundColor: Theme.of(context).colorScheme.error, - ), - ); - } - }, - builder: (context, state) { - if (state.status == InAppNotificationCenterStatus.loading && - state.breakingNewsNotifications.isEmpty && - state.digestNotifications.isEmpty) { - return LoadingStateWidget( - icon: Icons.notifications_none_outlined, - headline: l10n.notificationCenterLoadingHeadline, - subheadline: l10n.notificationCenterLoadingSubheadline, - ); - } + body: + BlocConsumer< + InAppNotificationCenterBloc, + InAppNotificationCenterState + >( + listener: (context, state) { + if (state.status == InAppNotificationCenterStatus.failure && + state.error != null) { + ScaffoldMessenger.of(context) + ..hideCurrentSnackBar() + ..showSnackBar( + SnackBar( + content: Text(state.error!.message), + backgroundColor: Theme.of(context).colorScheme.error, + ), + ); + } + }, + builder: (context, state) { + if (state.status == InAppNotificationCenterStatus.loading && + state.breakingNewsNotifications.isEmpty && + state.digestNotifications.isEmpty) { + return LoadingStateWidget( + icon: Icons.notifications_none_outlined, + headline: l10n.notificationCenterLoadingHeadline, + subheadline: l10n.notificationCenterLoadingSubheadline, + ); + } - if (state.status == InAppNotificationCenterStatus.failure && - state.breakingNewsNotifications.isEmpty && - state.digestNotifications.isEmpty) { - return FailureStateWidget( - exception: state.error ?? - OperationFailedException( - l10n.notificationCenterFailureHeadline, - ), - onRetry: () { - context.read().add( + if (state.status == InAppNotificationCenterStatus.failure && + state.breakingNewsNotifications.isEmpty && + state.digestNotifications.isEmpty) { + return FailureStateWidget( + exception: + state.error ?? + OperationFailedException( + l10n.notificationCenterFailureHeadline, + ), + onRetry: () { + context.read().add( const InAppNotificationCenterSubscriptionRequested(), ); - }, - ); - } + }, + ); + } - return TabBarView( - controller: _tabController, - children: [ - _NotificationList( - status: state.status, - notifications: state.breakingNewsNotifications, - hasMore: state.breakingNewsHasMore, - ), - _NotificationList( - status: state.status, - notifications: state.digestNotifications, - hasMore: state.digestHasMore, - ), - ], - ); - }, - ), + return TabBarView( + controller: _tabController, + children: [ + _NotificationList( + status: state.status, + notifications: state.breakingNewsNotifications, + hasMore: state.breakingNewsHasMore, + ), + _NotificationList( + status: state.status, + notifications: state.digestNotifications, + hasMore: state.digestHasMore, + ), + ], + ); + }, + ), ); } } @@ -216,12 +222,8 @@ class _NotificationListState extends State<_NotificationList> { if (index >= widget.notifications.length) { return widget.status == InAppNotificationCenterStatus.loadingMore ? const Padding( - padding: EdgeInsets.symmetric( - vertical: AppSpacing.lg, - ), - child: Center( - child: CircularProgressIndicator(), - ), + padding: EdgeInsets.symmetric(vertical: AppSpacing.lg), + child: Center(child: CircularProgressIndicator()), ) : const SizedBox.shrink(); } @@ -230,8 +232,8 @@ class _NotificationListState extends State<_NotificationList> { notification: notification, onTap: () async { context.read().add( - InAppNotificationCenterMarkedAsRead(notification.id), - ); + InAppNotificationCenterMarkedAsRead(notification.id), + ); final payload = notification.payload; final contentType = payload.data['contentType'] as String?;