Merge pull request #94 from flutter-news-app-full-source-code/fix-headlines-feed

Fix headlines feed

Author: fulleni

lib/headlines-feed/bloc/headlines_feed_bloc.dart

@@ -86,11 +86,10 @@ class HeadlinesFeedBloc extends Bloc<HeadlinesFeedEvent, HeadlinesFeedState> {
         r'$in': filter.eventCountries!.map((c) => c.id).toList(),
       };
     }
-    if (filter.sourceCountries?.isNotEmpty ?? false) {
-      queryFilter['source.headquarters.id'] = {
-        r'$in': filter.sourceCountries!.map((c) => c.id).toList(),
-      };
-    }
+    // Note: The `selectedSourceCountryIsoCodes` and `selectedSourceSourceTypes`
+    // fields are used exclusively for UI-side filtering on the `SourceFilterPage`
+    // and are not included in the backend query for headlines. Source filtering
+    // is performed solely by `source.id` when specific sources are selected.
     return queryFilter;
   }
 

lib/headlines-feed/bloc/sources_filter_bloc.dart

@@ -42,16 +42,17 @@ class SourcesFilterBloc extends Bloc<SourcesFilterEvent, SourcesFilterState> {
           .map((s) => s.id)
           .toSet();
 
-      // Use the passed-in initial capsule selections directly
-      final initialSelectedCountryIsoCodes =
-          event.initialSelectedCountryIsoCodes;
-      final initialSelectedSourceTypes = event.initialSelectedSourceTypes;
+      // The initial country and source type capsule selections are ephemeral
+      // to the UI of the SourceFilterPage and are not passed via the event.
+      // They are initialized as empty sets here, meaning the filter starts
+      // with all countries and source types selected by default in the UI.
+      final initialSelectedCountryIsoCodes = <String>{};
+      final initialSelectedSourceTypes = <SourceType>{};
 
       final allAvailableSources = (await _sourcesRepository.readAll()).items;
 
       // Initially, display all sources. Capsules are visually set but don't filter the list yet.
       // Filtering will occur if a capsule is manually toggled.
-      // However, if initial capsule filters ARE provided, we should respect them for the initial display.
       final displayableSources = _getFilteredSources(
         allSources: allAvailableSources,
         selectedCountries: initialSelectedCountryIsoCodes,

lib/headlines-feed/bloc/sources_filter_event.dart

@@ -9,22 +9,27 @@ abstract class SourcesFilterEvent extends Equatable {
   List<Object?> get props => [];
 }
 
+/// {@template load_source_filter_data}
+/// Event triggered to load the initial data for the source filter page.
+///
+/// This event is dispatched when the `SourceFilterPage` is initialized.
+/// It fetches all available countries and sources, and initializes the
+/// internal state with any `initialSelectedSources` passed from the
+/// `HeadlinesFilterPage`. The country and source type capsule selections
+/// are ephemeral to the `SourceFilterPage` and are not passed via this event.
+/// {@endtemplate}
 class LoadSourceFilterData extends SourcesFilterEvent {
+  /// {@macro load_source_filter_data}
   const LoadSourceFilterData({
     this.initialSelectedSources = const [],
-    this.initialSelectedCountryIsoCodes = const {},
-    this.initialSelectedSourceTypes = const {},
   });
 
+  /// The list of sources that were initially selected on the previous page.
   final List<Source> initialSelectedSources;
-  final Set<String> initialSelectedCountryIsoCodes;
-  final Set<SourceType> initialSelectedSourceTypes;
 
   @override
   List<Object?> get props => [
     initialSelectedSources,
-    initialSelectedCountryIsoCodes,
-    initialSelectedSourceTypes,
   ];
 }
 

lib/headlines-feed/models/headline_filter.dart

@@ -9,43 +9,39 @@ class HeadlineFilter extends Equatable {
   const HeadlineFilter({
     this.topics,
     this.sources,
-    this.selectedSourceCountryIsoCodes,
-    this.selectedSourceSourceTypes,
     this.eventCountries,
-    this.sourceCountries,
     this.isFromFollowedItems = false,
   });
 
   /// The list of selected topics to filter headlines by.
   final List<Topic>? topics;
 
   /// The list of selected sources to filter headlines by.
+  ///
+  /// Note: The `SourceFilterPage` uses internal UI state (country and source
+  /// type capsules) to refine the list of sources presented to the user.
+  /// However, only the *explicitly selected* sources from that refined list
+  /// are passed back and stored here. The country and source type selections
+  /// themselves are *not* part of this filter model, as they are purely for
+  /// UI-side filtering on the `SourceFilterPage` and should not affect the
+  /// backend query for headlines.
   final List<Source>? sources;
 
-  /// The set of ISO codes for countries selected to filter sources by their
-  /// headquarters.
-  final Set<String>? selectedSourceCountryIsoCodes;
-
-  /// The set of source types selected to filter sources by.
-  final Set<SourceType>? selectedSourceSourceTypes;
-
   /// The list of selected event countries to filter headlines by.
   final List<Country>? eventCountries;
 
-  /// The list of selected source headquarters countries to filter headlines by.
-  final List<Country>? sourceCountries;
-
   /// Whether the filter is based on the user's followed items.
+  ///
+  /// When `true`, the `topics` and `sources` fields will be populated based
+  /// on the user's followed items, and manual selections for these categories
+  /// will be ignored.
   final bool isFromFollowedItems;
 
   @override
   List<Object?> get props => [
     topics,
     sources,
-    selectedSourceCountryIsoCodes,
-    selectedSourceSourceTypes,
     eventCountries,
-    sourceCountries,
     isFromFollowedItems,
   ];
 
@@ -54,21 +50,13 @@ class HeadlineFilter extends Equatable {
   HeadlineFilter copyWith({
     List<Topic>? topics,
     List<Source>? sources,
-    Set<String>? selectedSourceCountryIsoCodes,
-    Set<SourceType>? selectedSourceSourceTypes,
     List<Country>? eventCountries,
-    List<Country>? sourceCountries,
     bool? isFromFollowedItems,
   }) {
     return HeadlineFilter(
       topics: topics ?? this.topics,
       sources: sources ?? this.sources,
-      selectedSourceCountryIsoCodes:
-          selectedSourceCountryIsoCodes ?? this.selectedSourceCountryIsoCodes,
-      selectedSourceSourceTypes:
-          selectedSourceSourceTypes ?? this.selectedSourceSourceTypes,
       eventCountries: eventCountries ?? this.eventCountries,
-      sourceCountries: sourceCountries ?? this.sourceCountries,
       isFromFollowedItems: isFromFollowedItems ?? this.isFromFollowedItems,
     );
   }

lib/headlines-feed/view/headlines_feed_page.dart

@@ -140,7 +140,6 @@ class _HeadlinesFeedPageState extends State<HeadlinesFeedPage> {
                     (state.filter.topics?.isNotEmpty ?? false) ||
                     (state.filter.sources?.isNotEmpty ?? false) ||
                     (state.filter.eventCountries?.isNotEmpty ?? false) ||
-                    (state.filter.sourceCountries?.isNotEmpty ?? false) ||
                     state.filter.isFromFollowedItems;
                 return Stack(
                   children: [
@@ -205,7 +204,10 @@ class _HeadlinesFeedPageState extends State<HeadlinesFeedPage> {
             if (state.status == HeadlinesFeedStatus.failure &&
                 state.feedItems.isEmpty) {
               return FailureStateWidget(
-                exception: state.error!,
+                //TODO(fulleni): l10n.
+                exception:
+                    state.error ??
+                    const UnknownException('Failed to load headlines feed.'),
                 onRetry: () => context.read<HeadlinesFeedBloc>().add(
                   HeadlinesFeedRefreshRequested(
                     adThemeStyle: AdThemeStyle.fromTheme(Theme.of(context)),

lib/headlines-feed/view/headlines_filter_page.dart

@@ -16,8 +16,6 @@ import 'package:ui_kit/ui_kit.dart';
 
 // Keys for passing data to/from SourceFilterPage
 const String keySelectedSources = 'selectedSources';
-const String keySelectedCountryIsoCodes = 'selectedCountryIsoCodes';
-const String keySelectedSourceTypes = 'selectedSourceTypes';
 
 /// {@template headlines_filter_page}
 /// A full-screen dialog page for selecting headline filters.
@@ -42,10 +40,7 @@ class _HeadlinesFilterPageState extends State<HeadlinesFilterPage> {
   /// and are only applied back to the BLoC when the user taps 'Apply'.
   late List<Topic> _tempSelectedTopics;
   late List<Source> _tempSelectedSources;
-  late Set<String> _tempSelectedSourceCountryIsoCodes;
-  late Set<SourceType> _tempSelectedSourceSourceTypes;
   late List<Country> _tempSelectedEventCountries;
-  late List<Country> _tempSelectedSourceCountries;
 
   // New state variables for the "Apply my followed items" feature
   bool _useFollowedFilters = false;
@@ -63,16 +58,7 @@ class _HeadlinesFilterPageState extends State<HeadlinesFilterPage> {
     final currentFilter = headlinesFeedState.filter;
     _tempSelectedTopics = List.from(currentFilter.topics ?? []);
     _tempSelectedSources = List.from(currentFilter.sources ?? []);
-    _tempSelectedSourceCountryIsoCodes = Set.from(
-      currentFilter.selectedSourceCountryIsoCodes ?? {},
-    );
-    _tempSelectedSourceSourceTypes = Set.from(
-      currentFilter.selectedSourceSourceTypes ?? {},
-    );
     _tempSelectedEventCountries = List.from(currentFilter.eventCountries ?? []);
-    _tempSelectedSourceCountries = List.from(
-      currentFilter.sourceCountries ?? [],
-    );
 
     _useFollowedFilters = currentFilter.isFromFollowedItems;
     _isLoadingFollowedFilters = false;
@@ -129,7 +115,6 @@ class _HeadlinesFilterPageState extends State<HeadlinesFilterPage> {
           _tempSelectedTopics = [];
           _tempSelectedSources = [];
           _tempSelectedEventCountries = [];
-          _tempSelectedSourceCountries = [];
         });
         if (mounted) {
           ScaffoldMessenger.of(context)
@@ -168,7 +153,6 @@ class _HeadlinesFilterPageState extends State<HeadlinesFilterPage> {
         _tempSelectedTopics = [];
         _tempSelectedSources = [];
         _tempSelectedEventCountries = [];
-        _tempSelectedSourceCountries = [];
         _isLoadingFollowedFilters = false;
         _useFollowedFilters = false;
       });
@@ -208,10 +192,6 @@ class _HeadlinesFilterPageState extends State<HeadlinesFilterPage> {
       _tempSelectedTopics = [];
       _tempSelectedSources = [];
       _tempSelectedEventCountries = [];
-      _tempSelectedSourceCountries = [];
-      // Keep source country/type filters as they are not part of this quick filter
-      // _tempSelectedSourceCountryIsoCodes = {};
-      // _tempSelectedSourceSourceTypes = {};
     });
   }
 
@@ -226,14 +206,13 @@ class _HeadlinesFilterPageState extends State<HeadlinesFilterPage> {
   /// (e.g., `SourceFilterPage`) via the `extra` parameter of `context.pushNamed`.
   /// Updates the temporary state via the [onResult] callback when the
   /// criterion page pops with a result (the user tapped 'Apply' on that page).
-  Widget _buildFilterTile({
+  Widget _buildFilterTile<T>({
     required BuildContext context,
     required String title,
     required int selectedCount,
     required String routeName,
-    // For sources, currentSelection will be a Map
-    required dynamic currentSelectionData,
-    required void Function(dynamic)? onResult,
+    required List<T> currentSelectionData,
+    required void Function(List<T>)? onResult,
     bool enabled = true,
   }) {
     final l10n = AppLocalizationsX(context).l10n;
@@ -252,7 +231,7 @@ class _HeadlinesFilterPageState extends State<HeadlinesFilterPage> {
       onTap:
           enabled // Only allow tap if enabled
           ? () async {
-              final result = await context.pushNamed<dynamic>(
+              final result = await context.pushNamed<List<T>>(
                 routeName,
                 extra: currentSelectionData,
               );
@@ -308,20 +287,9 @@ class _HeadlinesFilterPageState extends State<HeadlinesFilterPage> {
                 sources: _tempSelectedSources.isNotEmpty
                     ? _tempSelectedSources
                     : null,
-                selectedSourceCountryIsoCodes:
-                    _tempSelectedSourceCountryIsoCodes.isNotEmpty
-                    ? _tempSelectedSourceCountryIsoCodes
-                    : null,
-                selectedSourceSourceTypes:
-                    _tempSelectedSourceSourceTypes.isNotEmpty
-                    ? _tempSelectedSourceSourceTypes
-                    : null,
                 eventCountries: _tempSelectedEventCountries.isNotEmpty
                     ? _tempSelectedEventCountries
                     : null,
-                sourceCountries: _tempSelectedSourceCountries.isNotEmpty
-                    ? _tempSelectedSourceCountries
-                    : null,
                 isFromFollowedItems: _useFollowedFilters,
               );
               context.read<HeadlinesFeedBloc>().add(
@@ -379,54 +347,37 @@ class _HeadlinesFilterPageState extends State<HeadlinesFilterPage> {
               ),
             ),
           const Divider(),
-          _buildFilterTile(
+          _buildFilterTile<Topic>(
             context: context,
             title: l10n.headlinesFeedFilterTopicLabel,
             enabled: !_useFollowedFilters && !_isLoadingFollowedFilters,
             selectedCount: _tempSelectedTopics.length,
             routeName: Routes.feedFilterTopicsName,
             currentSelectionData: _tempSelectedTopics,
             onResult: (result) {
-              if (result is List<Topic>) {
-                setState(() => _tempSelectedTopics = result);
-              }
+              setState(() => _tempSelectedTopics = result);
             },
           ),
-          _buildFilterTile(
+          _buildFilterTile<Source>(
             context: context,
             title: l10n.headlinesFeedFilterSourceLabel,
             enabled: !_useFollowedFilters && !_isLoadingFollowedFilters,
             selectedCount: _tempSelectedSources.length,
             routeName: Routes.feedFilterSourcesName,
-            currentSelectionData: {
-              keySelectedSources: _tempSelectedSources,
-              keySelectedCountryIsoCodes: _tempSelectedSourceCountryIsoCodes,
-              keySelectedSourceTypes: _tempSelectedSourceSourceTypes,
-            },
+            currentSelectionData: _tempSelectedSources,
             onResult: (result) {
-              if (result is Map<String, dynamic>) {
-                setState(() {
-                  _tempSelectedSources =
-                      result[keySelectedSources] as List<Source>? ?? [];
-                  _tempSelectedSourceCountryIsoCodes =
-                      result[keySelectedCountryIsoCodes] as Set<String>? ?? {};
-                  _tempSelectedSourceSourceTypes =
-                      result[keySelectedSourceTypes] as Set<SourceType>? ?? {};
-                });
-              }
+              setState(() => _tempSelectedSources = result);
             },
           ),
-          _buildFilterTile(
+          _buildFilterTile<Country>(
             context: context,
             title: l10n.headlinesFeedFilterEventCountryLabel,
             enabled: !_useFollowedFilters && !_isLoadingFollowedFilters,
             selectedCount: _tempSelectedEventCountries.length,
             routeName: Routes.feedFilterEventCountriesName,
             currentSelectionData: _tempSelectedEventCountries,
             onResult: (result) {
-              if (result is List<Country>) {
-                setState(() => _tempSelectedEventCountries = result);
-              }
+              setState(() => _tempSelectedEventCountries = result);
             },
           ),
         ],