[Compass App] Cleanup and extra tests (#2437)

miquelbeltran · web-flow · commit 799ce7f54846 · 2024-09-16T08:52:08.000+02:00
Some changes to make the project ready to merge to main - Fix formatting. - Add project to the `stable` channel CI (potentially could be added for `beta` and `master`) - CI only runs when targeting `main`, so I run the command locally and it worked. - Add unit tests for the `ApiClient` and `AuthApiClient` which had no test coverage outside of integration tests. - Resolved some TODOs I left before. ## Pre-launch Checklist - [x] I read the [Flutter Style Guide] _recently_, and have followed its advice. - [x] I signed the [CLA]. - [x] I read the [Contributors Guide]. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-devrel channel on [Discord].  [Flutter Style Guide]: https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md [CLA]: https://cla.developers.google.com/ [Discord]: https://github.com/flutter/flutter/blob/master/docs/contributing/Chat.md [Contributors Guide]: https://github.com/flutter/samples/blob/main/CONTRIBUTING.md
diff --git a/compass_app/app/.gitignore b/compass_app/app/.gitignore
@@ -41,3 +41,6 @@ app.*.map.json
 /android/app/debug
 /android/app/profile
 /android/app/release
+
+# Coverage test report
+coverage/
diff --git a/compass_app/app/lib/data/repositories/destination/destination_repository.dart b/compass_app/app/lib/data/repositories/destination/destination_repository.dart
@@ -5,6 +5,4 @@ import '../../../utils/result.dart';
 abstract class DestinationRepository {
   /// Get complete list of destinations
   Future<Result<List<Destination>>> getDestinations();
-
-  // TODO: Consider creating getByContinent instead of filtering in ViewModel
 }
diff --git a/compass_app/app/lib/data/services/api/api_client.dart b/compass_app/app/lib/data/services/api/api_client.dart
@@ -11,9 +11,18 @@ import 'model/user/user_api_model.dart';
 /// Adds the `Authentication` header to a header configuration.
 typedef AuthHeaderProvider = String? Function();
 
-// TODO: Configurable baseurl/host/port
 class ApiClient {
-  ApiClient();
+  ApiClient({
+    String? host,
+    int? port,
+    HttpClient Function()? clientFactory,
+  })  : _host = host ?? 'localhost',
+        _port = port ?? 8080,
+        _clientFactory = clientFactory ?? (() => HttpClient());
+
+  final String _host;
+  final int _port;
+  final HttpClient Function() _clientFactory;
 
   AuthHeaderProvider? _authHeaderProvider;
 
@@ -29,9 +38,9 @@ class ApiClient {
   }
 
   Future<Result<List<Continent>>> getContinents() async {
-    final client = HttpClient();
+    final client = _clientFactory();
     try {
-      final request = await client.get('localhost', 8080, '/continent');
+      final request = await client.get(_host, _port, '/continent');
       await _authHeader(request.headers);
       final response = await request.close();
       if (response.statusCode == 200) {
@@ -50,9 +59,9 @@ class ApiClient {
   }
 
   Future<Result<List<Destination>>> getDestinations() async {
-    final client = HttpClient();
+    final client = _clientFactory();
     try {
-      final request = await client.get('localhost', 8080, '/destination');
+      final request = await client.get(_host, _port, '/destination');
       await _authHeader(request.headers);
       final response = await request.close();
       if (response.statusCode == 200) {
@@ -71,10 +80,10 @@ class ApiClient {
   }
 
   Future<Result<List<Activity>>> getActivityByDestination(String ref) async {
-    final client = HttpClient();
+    final client = _clientFactory();
     try {
       final request =
-          await client.get('localhost', 8080, '/destination/$ref/activity');
+          await client.get(_host, _port, '/destination/$ref/activity');
       await _authHeader(request.headers);
       final response = await request.close();
       if (response.statusCode == 200) {
@@ -94,9 +103,9 @@ class ApiClient {
   }
 
   Future<Result<List<BookingApiModel>>> getBookings() async {
-    final client = HttpClient();
+    final client = _clientFactory();
     try {
-      final request = await client.get('localhost', 8080, '/booking');
+      final request = await client.get(_host, _port, '/booking');
       await _authHeader(request.headers);
       final response = await request.close();
       if (response.statusCode == 200) {
@@ -116,9 +125,9 @@ class ApiClient {
   }
 
   Future<Result<BookingApiModel>> getBooking(int id) async {
-    final client = HttpClient();
+    final client = _clientFactory();
     try {
-      final request = await client.get('localhost', 8080, '/booking/$id');
+      final request = await client.get(_host, _port, '/booking/$id');
       await _authHeader(request.headers);
       final response = await request.close();
       if (response.statusCode == 200) {
@@ -136,9 +145,9 @@ class ApiClient {
   }
 
   Future<Result<BookingApiModel>> postBooking(BookingApiModel booking) async {
-    final client = HttpClient();
+    final client = _clientFactory();
     try {
-      final request = await client.post('localhost', 8080, '/booking');
+      final request = await client.post(_host, _port, '/booking');
       await _authHeader(request.headers);
       request.write(jsonEncode(booking));
       final response = await request.close();
@@ -157,9 +166,9 @@ class ApiClient {
   }
 
   Future<Result<UserApiModel>> getUser() async {
-    final client = HttpClient();
+    final client = _clientFactory();
     try {
-      final request = await client.get('localhost', 8080, '/user');
+      final request = await client.get(_host, _port, '/user');
       await _authHeader(request.headers);
       final response = await request.close();
       if (response.statusCode == 200) {
diff --git a/compass_app/app/lib/data/services/api/auth_api_client.dart b/compass_app/app/lib/data/services/api/auth_api_client.dart
@@ -5,12 +5,23 @@ import '../../../utils/result.dart';
 import 'model/login_request/login_request.dart';
 import 'model/login_response/login_response.dart';
 
-// TODO: Configurable baseurl/host/port
 class AuthApiClient {
+  AuthApiClient({
+    String? host,
+    int? port,
+    HttpClient Function()? clientFactory,
+  })  : _host = host ?? 'localhost',
+        _port = port ?? 8080,
+        _clientFactory = clientFactory ?? (() => HttpClient());
+
+  final String _host;
+  final int _port;
+  final HttpClient Function() _clientFactory;
+
   Future<Result<LoginResponse>> login(LoginRequest loginRequest) async {
-    final client = HttpClient();
+    final client = _clientFactory();
     try {
-      final request = await client.post('localhost', 8080, '/login');
+      final request = await client.post(_host, _port, '/login');
       request.write(jsonEncode(loginRequest));
       final response = await request.close();
       if (response.statusCode == 200) {
diff --git a/compass_app/app/lib/ui/core/themes/text_styles.dart b/compass_app/app/lib/ui/core/themes/text_styles.dart
diff --git a/compass_app/app/lib/ui/home/widgets/home_screen.dart b/compass_app/app/lib/ui/home/widgets/home_screen.dart
@@ -84,7 +84,6 @@ class HomeScreen extends StatelessWidget {
   }
 }
 
-
 class _Booking extends StatelessWidget {
   const _Booking({
     super.key,
diff --git a/compass_app/app/lib/ui/results/widgets/result_card.dart b/compass_app/app/lib/ui/results/widgets/result_card.dart
@@ -1,8 +1,8 @@
 import 'package:cached_network_image/cached_network_image.dart';
 import 'package:flutter/material.dart';
+import 'package:google_fonts/google_fonts.dart';
 import '../../../domain/models/destination/destination.dart';
 import '../../../utils/image_error_listener.dart';
-import '../../core/themes/text_styles.dart';
 import '../../core/ui/tag_chip.dart';
 
 class ResultCard extends StatelessWidget {
@@ -38,7 +38,7 @@ class ResultCard extends StatelessWidget {
               children: [
                 Text(
                   destination.name.toUpperCase(),
-                  style: TextStyles.cardTitleStyle,
+                  style: _cardTitleStyle,
                 ),
                 const SizedBox(
                   height: 6,
@@ -67,3 +67,19 @@ class ResultCard extends StatelessWidget {
     );
   }
 }
+
+final _cardTitleStyle = GoogleFonts.rubik(
+  textStyle: const TextStyle(
+    fontWeight: FontWeight.w800,
+    fontSize: 15.0,
+    color: Colors.white,
+    letterSpacing: 1,
+    shadows: [
+      // Helps to read the text a bit better
+      Shadow(
+        blurRadius: 3.0,
+        color: Colors.black,
+      )
+    ],
+  ),
+);
diff --git a/compass_app/app/test/data/services/api/api_client_test.dart b/compass_app/app/test/data/services/api/api_client_test.dart
@@ -0,0 +1,72 @@
+import 'package:compass_app/data/services/api/api_client.dart';
+import 'package:compass_app/domain/models/continent/continent.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+import '../../../../testing/mocks.dart';
+import '../../../../testing/models/activity.dart';
+import '../../../../testing/models/booking.dart';
+import '../../../../testing/models/destination.dart';
+import '../../../../testing/models/user.dart';
+
+void main() {
+  group('ApiClient', () {
+    late MockHttpClient mockHttpClient;
+    late ApiClient apiClient;
+
+    setUp(() {
+      mockHttpClient = MockHttpClient();
+      apiClient = ApiClient(clientFactory: () => mockHttpClient);
+    });
+
+    test('should get continents', () async {
+      final continents = [const Continent(name: 'NAME', imageUrl: 'URL')];
+      mockHttpClient.mockGet('/continent', continents);
+      final result = await apiClient.getContinents();
+      expect(result.asOk.value, continents);
+    });
+
+    test('should get activities by destination', () async {
+      final activites = [kActivity];
+      mockHttpClient.mockGet(
+        '/destination/${kDestination1.ref}/activity',
+        activites,
+      );
+      final result =
+          await apiClient.getActivityByDestination(kDestination1.ref);
+      expect(result.asOk.value, activites);
+    });
+
+    test('should get booking', () async {
+      mockHttpClient.mockGet(
+        '/booking/${kBookingApiModel.id}',
+        kBookingApiModel,
+      );
+      final result = await apiClient.getBooking(kBookingApiModel.id!);
+      expect(result.asOk.value, kBookingApiModel);
+    });
+
+    test('should get bookings', () async {
+      mockHttpClient.mockGet('/booking', [kBookingApiModel]);
+      final result = await apiClient.getBookings();
+      expect(result.asOk.value, [kBookingApiModel]);
+    });
+
+    test('should get destinations', () async {
+      mockHttpClient.mockGet('/destination', [kDestination1]);
+      final result = await apiClient.getDestinations();
+      expect(result.asOk.value, [kDestination1]);
+    });
+
+    test('should get user', () async {
+      mockHttpClient.mockGet('/user', userApiModel);
+      final result = await apiClient.getUser();
+      expect(result.asOk.value, userApiModel);
+    });
+
+    test('should post booking', () async {
+      mockHttpClient.mockPost('/booking', kBookingApiModel);
+      final result = await apiClient.postBooking(kBookingApiModel);
+      expect(result.asOk.value, kBookingApiModel);
+    });
+  });
+}
diff --git a/compass_app/app/test/data/services/api/auth_api_client_test.dart b/compass_app/app/test/data/services/api/auth_api_client_test.dart
@@ -0,0 +1,37 @@
+import 'package:compass_app/data/services/api/auth_api_client.dart';
+import 'package:compass_app/data/services/api/model/login_request/login_request.dart';
+import 'package:compass_app/data/services/api/model/login_response/login_response.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+import '../../../../testing/mocks.dart';
+
+void main() {
+  group('AuthApiClient', () {
+    late MockHttpClient mockHttpClient;
+    late AuthApiClient apiClient;
+
+    setUp(() {
+      mockHttpClient = MockHttpClient();
+      apiClient = AuthApiClient(clientFactory: () => mockHttpClient);
+    });
+
+    test('should post login', () async {
+      const loginResponse = LoginResponse(
+        token: 'TOKEN',
+        userId: '123',
+      );
+      mockHttpClient.mockPost(
+        '/login',
+        loginResponse,
+        200,
+      );
+      final result = await apiClient.login(
+        const LoginRequest(
+          email: 'EMAIL',
+          password: 'PASSWORD',
+        ),
+      );
+      expect(result.asOk.value, loginResponse);
+    });
+  });
+}
diff --git a/compass_app/app/test/ui/results/results_viewmodel_test.dart b/compass_app/app/test/ui/results/results_viewmodel_test.dart
@@ -21,7 +21,6 @@ void main() {
 
     // perform a simple test
     // verifies that the list of items is properly loaded
-    // TODO: Verify loading state and calls to search method
     test('should load items', () async {
       expect(viewModel.destinations.length, 2);
     });
diff --git a/compass_app/app/testing/mocks.dart b/compass_app/app/testing/mocks.dart
@@ -1,4 +1,43 @@
+import 'dart:convert';
+import 'dart:io';
+
 import 'package:go_router/go_router.dart';
 import 'package:mocktail/mocktail.dart';
 
 class MockGoRouter extends Mock implements GoRouter {}
+
+class MockHttpClient extends Mock implements HttpClient {}
+
+class MockHttpHeaders extends Mock implements HttpHeaders {}
+
+class MockHttpClientRequest extends Mock implements HttpClientRequest {}
+
+class MockHttpClientResponse extends Mock implements HttpClientResponse {}
+
+extension HttpMethodMocks on MockHttpClient {
+  void mockGet(String path, Object object) {
+    when(() => get(any(), any(), path)).thenAnswer((invocation) {
+      final request = MockHttpClientRequest();
+      final response = MockHttpClientResponse();
+      when(() => request.close()).thenAnswer((_) => Future.value(response));
+      when(() => request.headers).thenReturn(MockHttpHeaders());
+      when(() => response.statusCode).thenReturn(200);
+      when(() => response.transform(utf8.decoder))
+          .thenAnswer((_) => Stream.value(jsonEncode(object)));
+      return Future.value(request);
+    });
+  }
+
+  void mockPost(String path, Object object, [int statusCode = 201]) {
+    when(() => post(any(), any(), path)).thenAnswer((invocation) {
+      final request = MockHttpClientRequest();
+      final response = MockHttpClientResponse();
+      when(() => request.close()).thenAnswer((_) => Future.value(response));
+      when(() => request.headers).thenReturn(MockHttpHeaders());
+      when(() => response.statusCode).thenReturn(statusCode);
+      when(() => response.transform(utf8.decoder))
+          .thenAnswer((_) => Stream.value(jsonEncode(object)));
+      return Future.value(request);
+    });
+  }
+}
diff --git a/compass_app/server/test/server_test.dart b/compass_app/server/test/server_test.dart
@@ -1,7 +1,6 @@
 import 'dart:convert';
 import 'dart:io';
 
-// TODO: Remove the compass_model and replace by a server-side model
 import 'package:compass_server/config/constants.dart';
 import 'package:compass_server/model/activity/activity.dart';
 import 'package:compass_server/model/booking/booking.dart';
diff --git a/tool/flutter_ci_script_stable.sh b/tool/flutter_ci_script_stable.sh
@@ -23,6 +23,8 @@ declare -ar PROJECT_NAMES=(
     "code_sharing/client"
     "code_sharing/server"
     "code_sharing/shared"
+    "compass_app/app"
+    "compass_app/server"
     "context_menus"
     "deeplink_store_example"
     "desktop_photo_search/fluent_ui"

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,4 @@ import '../../../utils/result.dart';`
`5`	`5`	`abstract class DestinationRepository {`
`6`	`6`	`/// Get complete list of destinations`
`7`	`7`	`Future<Result<List<Destination>>> getDestinations();`
`8`		`-`
`9`		`- // TODO: Consider creating getByContinent instead of filtering in ViewModel`
`10`	`8`	`}`
Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,6 @@ class HomeScreen extends StatelessWidget {`
`84`	`84`	`}`
`85`	`85`	`}`
`86`	`86`
`87`		`-`
`88`	`87`	`class _Booking extends StatelessWidget {`
`89`	`88`	`const _Booking({`
`90`	`89`	`super.key,`