Always fetch initial results with a valid filter (#188)

Author: athomas

current_results_ui/lib/src/features/results_overview/data/results_repository.dart

@@ -32,12 +32,8 @@ abstract class QueryResultsBase extends ChangeNotifier {
   Counts resultCounts = Counts();
   int fetchedResultsCount = 0;
 
-  QueryResultsBase(
-    this._filter, {
-    bool fetchInitialResults = false,
-    this.supportsEmptyQuery = false,
-  }) {
-    if (fetchInitialResults) {
+  QueryResultsBase(this._filter, {this.supportsEmptyQuery = false}) {
+    if (_filter.terms.isNotEmpty || supportsEmptyQuery) {
       _fetchResults();
     }
   }

current_results_ui/lib/src/features/try_results/data/try_results_repository.dart

@@ -21,7 +21,7 @@ class TryQueryResults extends QueryResultsBase {
     required Filter filter,
     ResultsService? resultsService,
   }) : _resultsService = resultsService ?? ResultsService(),
-       super(filter, fetchInitialResults: true, supportsEmptyQuery: true);
+       super(filter, supportsEmptyQuery: true);
 
   @override
   Stream<Iterable<(ChangeInResult, Result)>> createResultsStream() async* {

current_results_ui/test/routing_test.dart

@@ -23,20 +23,30 @@ import 'package:provider/provider.dart';
 
 import 'routing_test.mocks.dart';
 
-class FakeQueryResults extends TryQueryResults {
-  FakeQueryResults({
-    super.cl = 0,
-    super.patchset = 0,
-    required super.filter,
-    super.resultsService,
-  });
+mixin FakeCreateResultsStreamMixin on QueryResultsBase {
+  bool createResultsStreamCalled = false;
 
   @override
   Stream<Iterable<(ChangeInResult, Result)>> createResultsStream() async* {
+    createResultsStreamCalled = true;
     yield [];
   }
 }
 
+class FakeQueryResults extends QueryResults with FakeCreateResultsStreamMixin {
+  FakeQueryResults(super.filter);
+}
+
+class FakeTryQueryResults extends TryQueryResults
+    with FakeCreateResultsStreamMixin {
+  FakeTryQueryResults({
+    required super.cl,
+    required super.patchset,
+    required super.filter,
+    super.resultsService,
+  });
+}
+
 @GenerateNiceMocks([
   MockSpec<AuthService>(),
   MockSpec<http.Client>(),
@@ -45,7 +55,8 @@ class FakeQueryResults extends TryQueryResults {
 void main() {
   late MockAuthService mockAuthService;
   late GoRouter router;
-  late TryQueryResults queryResults;
+  late FakeQueryResults queryResults;
+  late FakeTryQueryResults tryQueryResults;
   late MockResultsService mockResultsService;
 
   setUp(() {
@@ -55,13 +66,10 @@ void main() {
       (_) async => Review(id: '123', subject: 'Test Subject', patchsets: []),
     );
     router = createRouter(
-      queryResultsProvider: (filter) => queryResults = FakeQueryResults(
-        filter: filter,
-        resultsService: mockResultsService,
-      ),
+      queryResultsProvider: (filter) => queryResults = FakeQueryResults(filter),
       tryQueryResultsProvider:
           ({required cl, required patchset, required filter}) =>
-              queryResults = FakeQueryResults(
+              tryQueryResults = FakeTryQueryResults(
                 cl: cl,
                 patchset: patchset,
                 filter: filter,
@@ -92,6 +100,7 @@ void main() {
     final tabBar = tester.widget<TabBar>(find.byType(TabBar));
     expect(tabBar.controller?.index, equals(0));
     expect(find.byType(Instructions), findsNothing);
+    expect(queryResults.createResultsStreamCalled, isTrue);
   });
 
   testWidgets('Routing works for flaky parameter', (WidgetTester tester) async {
@@ -104,6 +113,7 @@ void main() {
     expect(resultsScreen.filter.terms, isEmpty);
     final tabBar = tester.widget<TabBar>(find.byType(TabBar));
     expect(tabBar.controller?.index, equals(1));
+    expect(queryResults.createResultsStreamCalled, isFalse);
   });
 
   testWidgets('Routing works for showAll parameter', (
@@ -118,6 +128,7 @@ void main() {
     expect(resultsScreen.filter.terms, isEmpty);
     final tabBar = tester.widget<TabBar>(find.byType(TabBar));
     expect(tabBar.controller?.index, equals(2));
+    expect(queryResults.createResultsStreamCalled, isFalse);
   });
 
   testWidgets('Routing works for combined parameters', (
@@ -132,6 +143,7 @@ void main() {
     expect(resultsScreen.filter.terms, equals(['test-filter']));
     final tabBar = tester.widget<TabBar>(find.byType(TabBar));
     expect(tabBar.controller?.index, equals(2));
+    expect(queryResults.createResultsStreamCalled, isTrue);
   });
 
   testWidgets('Routing works for default route', (WidgetTester tester) async {
@@ -145,6 +157,7 @@ void main() {
     final tabBar = tester.widget<TabBar>(find.byType(TabBar));
     expect(tabBar.controller?.index, equals(0));
     expect(find.byType(Instructions), findsOneWidget);
+    expect(queryResults.createResultsStreamCalled, isFalse);
   });
 
   testWidgets('Routing works for cl route', (WidgetTester tester) async {
@@ -155,9 +168,10 @@ void main() {
 
     final tabBar = tester.widget<TabBar>(find.byType(TabBar));
     expect(tabBar.controller?.index, equals(0));
-    expect(queryResults.cl, equals(1234));
-    expect(queryResults.patchset, equals(5));
-    expect(queryResults.filter.terms, isEmpty);
+    expect(tryQueryResults.cl, equals(1234));
+    expect(tryQueryResults.patchset, equals(5));
+    expect(tryQueryResults.filter.terms, isEmpty);
+    expect(tryQueryResults.createResultsStreamCalled, isTrue);
   });
 
   testWidgets('Routing works for cl route with filter', (
@@ -170,9 +184,10 @@ void main() {
 
     final tabBar = tester.widget<TabBar>(find.byType(TabBar));
     expect(tabBar.controller?.index, equals(0));
-    expect(queryResults.cl, equals(1234));
-    expect(queryResults.patchset, equals(5));
-    expect(queryResults.filter.terms, equals(['my-filter']));
+    expect(tryQueryResults.cl, equals(1234));
+    expect(tryQueryResults.patchset, equals(5));
+    expect(tryQueryResults.filter.terms, equals(['my-filter']));
+    expect(tryQueryResults.createResultsStreamCalled, isTrue);
   });
 
   testWidgets('Routing works for cl route with flaky', (
@@ -185,9 +200,10 @@ void main() {
 
     final tabBar = tester.widget<TabBar>(find.byType(TabBar));
     expect(tabBar.controller?.index, equals(1));
-    expect(queryResults.cl, equals(1234));
-    expect(queryResults.patchset, equals(5));
-    expect(queryResults.filter.terms, isEmpty);
+    expect(tryQueryResults.cl, equals(1234));
+    expect(tryQueryResults.patchset, equals(5));
+    expect(tryQueryResults.filter.terms, isEmpty);
+    expect(tryQueryResults.createResultsStreamCalled, isTrue);
   });
 
   testWidgets('Routing works for cl route with showAll', (
@@ -200,8 +216,9 @@ void main() {
 
     final tabBar = tester.widget<TabBar>(find.byType(TabBar));
     expect(tabBar.controller?.index, equals(2));
-    expect(queryResults.cl, equals(1234));
-    expect(queryResults.patchset, equals(5));
-    expect(queryResults.filter.terms, isEmpty);
+    expect(tryQueryResults.cl, equals(1234));
+    expect(tryQueryResults.patchset, equals(5));
+    expect(tryQueryResults.filter.terms, isEmpty);
+    expect(tryQueryResults.createResultsStreamCalled, isTrue);
   });
 }