Merge pull request #274 from mgoodness/fix-sac-by-tank-role

Fix SAC by Tank Role

Author: ericgriffin

lib/features/statistics/data/repositories/statistics_repository.dart

@@ -311,10 +311,63 @@ class StatisticsRepository {
     }
   }
 
-  /// Get average SAC by tank role (back gas, stage, deco, etc.)
+  /// Get volume-based average SAC by tank role (back gas, stage, deco, etc.)
+  ///
+  /// Returns a map of tank role to average SAC in L/min.
+  /// Requires tank volume data.
+  Future<Map<String, double>> getSacVolumeByTankRole({String? diverId}) async {
+    try {
+      final diverFilter = diverId != null ? 'AND d.diver_id = ?' : '';
+      final params = diverId != null ? [diverId] : [];
+
+      final results = await _db.customSelect('''
+        SELECT
+          t.tank_role,
+          AVG(
+            CASE
+              WHEN COALESCE(d.runtime, d.bottom_time) > 0 AND d.avg_depth > 0 AND t.start_pressure > t.end_pressure THEN
+                ((t.start_pressure - t.end_pressure) * t.volume) / (COALESCE(d.runtime, d.bottom_time) / 60.0) / ((d.avg_depth / 10.0) + 1)
+              ELSE NULL
+            END
+          ) AS avg_sac
+        FROM dives d
+        INNER JOIN dive_tanks t ON t.dive_id = d.id
+        WHERE t.start_pressure IS NOT NULL
+          AND t.end_pressure IS NOT NULL
+          AND COALESCE(d.runtime, d.bottom_time) > 0
+          AND d.avg_depth > 0
+          AND t.volume > 0
+          $diverFilter
+        GROUP BY t.tank_role
+        HAVING avg_sac IS NOT NULL
+        ORDER BY avg_sac ASC
+        ''', variables: params.map((p) => Variable(p)).toList()).get();
+
+      final Map<String, double> sacByRole = {};
+      for (final row in results) {
+        final role = row.read<String>('tank_role');
+        final sac = row.read<double>('avg_sac');
+        sacByRole[role] = sac;
+      }
+
+      return sacByRole;
+    } catch (e, stackTrace) {
+      _log.error(
+        'Failed to get SAC in volume by tank role',
+        error: e,
+        stackTrace: stackTrace,
+      );
+      return {};
+    }
+  }
+
+  /// Get pressure-based average SAC by tank role (back gas, stage, deco, etc.)
   ///
   /// Returns a map of tank role to average SAC in bar/min.
-  Future<Map<String, double>> getSacByTankRole({String? diverId}) async {
+  /// Does not require tank volume.
+  Future<Map<String, double>> getSacPressureByTankRole({
+    String? diverId,
+  }) async {
     try {
       final diverFilter = diverId != null ? 'AND d.diver_id = ?' : '';
       final params = diverId != null ? [diverId] : [];
@@ -351,7 +404,7 @@ class StatisticsRepository {
       return sacByRole;
     } catch (e, stackTrace) {
       _log.error(
-        'Failed to get SAC by tank role',
+        'Failed to get SAC in pressure by tank role',
         error: e,
         stackTrace: stackTrace,
       );

lib/features/statistics/presentation/providers/statistics_providers.dart

@@ -73,7 +73,13 @@ final sacByTankRoleProvider = FutureProvider<Map<String, double>>((ref) async {
   _keepAliveWithExpiry(ref);
   final repository = ref.watch(statisticsRepositoryProvider);
   final currentDiverId = ref.watch(currentDiverIdProvider);
-  return repository.getSacByTankRole(diverId: currentDiverId);
+  final sacUnit = ref.watch(sacUnitProvider);
+
+  if (sacUnit == SacUnit.litersPerMin) {
+    return repository.getSacVolumeByTankRole(diverId: currentDiverId);
+  } else {
+    return repository.getSacPressureByTankRole(diverId: currentDiverId);
+  }
 });
 
 // ============================================================================

test/features/media/data/services/network_credentials_service_test.mocks.dart

@@ -407,4 +407,13 @@ class MockNetworkCredentialsRepository extends _i1.Mock
             returnValueForMissingStub: _i4.Future<void>.value(),
           )
           as _i4.Future<void>);
+
+  @override
+  _i4.Future<void> updateDisplayName(String? id, String? displayName) =>
+      (super.noSuchMethod(
+            Invocation.method(#updateDisplayName, [id, displayName]),
+            returnValue: _i4.Future<void>.value(),
+            returnValueForMissingStub: _i4.Future<void>.value(),
+          )
+          as _i4.Future<void>);
 }

test/features/media/data/services/network_scan_service_test.mocks.dart

@@ -305,6 +305,15 @@ class MockNetworkCredentialsService extends _i1.Mock
             ),
           )
           as _i5.Future<List<_i8.NetworkCredentialHost>>);
+
+  @override
+  _i5.Future<void> updateDisplayName(String? id, String? displayName) =>
+      (super.noSuchMethod(
+            Invocation.method(#updateDisplayName, [id, displayName]),
+            returnValue: _i5.Future<void>.value(),
+            returnValueForMissingStub: _i5.Future<void>.value(),
+          )
+          as _i5.Future<void>);
 }
 
 /// A class which mocks [ManifestSubscriptionRepository].
@@ -450,6 +459,23 @@ class MockManifestSubscriptionRepository extends _i1.Mock
           )
           as _i5.Future<void>);
 
+  @override
+  _i5.Future<void> updateUrlAndDisplayName(
+    String? id, {
+    required String? manifestUrl,
+    required String? displayName,
+  }) =>
+      (super.noSuchMethod(
+            Invocation.method(
+              #updateUrlAndDisplayName,
+              [id],
+              {#manifestUrl: manifestUrl, #displayName: displayName},
+            ),
+            returnValue: _i5.Future<void>.value(),
+            returnValueForMissingStub: _i5.Future<void>.value(),
+          )
+          as _i5.Future<void>);
+
   @override
   _i5.Future<void> deleteById(String? id) =>
       (super.noSuchMethod(

test/features/media/presentation/widgets/url_tab_test.mocks.dart

@@ -139,6 +139,15 @@ class MockNetworkCredentialsService extends _i1.Mock
             ),
           )
           as _i4.Future<List<_i7.NetworkCredentialHost>>);
+
+  @override
+  _i4.Future<void> updateDisplayName(String? id, String? displayName) =>
+      (super.noSuchMethod(
+            Invocation.method(#updateDisplayName, [id, displayName]),
+            returnValue: _i4.Future<void>.value(),
+            returnValueForMissingStub: _i4.Future<void>.value(),
+          )
+          as _i4.Future<void>);
 }
 
 /// A class which mocks [MediaRepository].

test/features/statistics/data/repositories/statistics_repository_error_test.dart

@@ -62,7 +62,8 @@ void main() {
       expect(sacPressureRecords.worst, isNull);
 
       // Methods that return empty map
-      expect(await repository.getSacByTankRole(), isEmpty);
+      expect(await repository.getSacVolumeByTankRole(), isEmpty);
+      expect(await repository.getSacPressureByTankRole(), isEmpty);
 
       // Methods that return tuple defaults
       final soloVsBuddy = await repository.getSoloVsBuddyCount();

test/features/statistics/data/repositories/statistics_repository_sac_test.dart

@@ -251,11 +251,11 @@ void main() {
   });
 
   // ---------------------------------------------------------------------------
-  // SAC By Tank Role
+  // SAC In Volume By Tank Role
   // ---------------------------------------------------------------------------
 
-  group('getSacByTankRole', () {
-    test('groups SAC by tank role using runtime', () async {
+  group('getSacVolumeByTankRole', () {
+    test('groups SAC in volume by tank role using runtime', () async {
       await insertDiveWithTank(
         id: 'dive-back',
         bottomTimeSeconds: 35 * 60,
@@ -264,9 +264,10 @@ void main() {
         startPressure: 200,
         endPressure: 50,
         tankRole: 'backGas',
+        volume: 12.0,
       );
 
-      final sacByRole = await repository.getSacByTankRole();
+      final sacByRole = await repository.getSacVolumeByTankRole();
 
       expect(sacByRole, isNotEmpty);
       expect(sacByRole.containsKey('backGas'), isTrue);
@@ -282,9 +283,68 @@ void main() {
         startPressure: 200,
         endPressure: 50,
         tankRole: 'backGas',
+        volume: 12.0,
       );
 
-      final sacByRole = await repository.getSacByTankRole();
+      final sacByRole = await repository.getSacVolumeByTankRole();
+
+      expect(sacByRole, isNotEmpty);
+      expect(sacByRole['backGas']!, greaterThan(0));
+    });
+
+    test('excludes tanks with zero volume', () async {
+      await insertDiveWithTank(
+        id: 'dive-zerovol',
+        bottomTimeSeconds: 40 * 60,
+        runtimeSeconds: 42 * 60,
+        avgDepth: 20.0,
+        startPressure: 200,
+        endPressure: 50,
+        tankRole: 'backGas',
+        volume: 0,
+      );
+
+      final sacByRole = await repository.getSacVolumeByTankRole();
+
+      expect(sacByRole, isEmpty);
+    });
+  });
+
+  // ---------------------------------------------------------------------------
+  // SAC In Pressure By Tank Role
+  // ---------------------------------------------------------------------------
+
+  group('getSacPressureByTankRole', () {
+    test('groups SAC in pressure by tank role using runtime', () async {
+      await insertDiveWithTank(
+        id: 'dive-back',
+        bottomTimeSeconds: 35 * 60,
+        runtimeSeconds: 42 * 60,
+        avgDepth: 20.0,
+        startPressure: 200,
+        endPressure: 50,
+        tankRole: 'backGas',
+      );
+
+      final sacByRole = await repository.getSacPressureByTankRole();
+
+      expect(sacByRole, isNotEmpty);
+      expect(sacByRole.containsKey('backGas'), isTrue);
+      expect(sacByRole['backGas']!, greaterThan(0));
+    });
+
+    test('falls back to bottom_time when runtime null', () async {
+      await insertDiveWithTank(
+        id: 'dive-norunt',
+        bottomTimeSeconds: 40 * 60,
+        runtimeSeconds: null,
+        avgDepth: 20.0,
+        startPressure: 200,
+        endPressure: 50,
+        tankRole: 'backGas',
+      );
+
+      final sacByRole = await repository.getSacPressureByTankRole();
 
       expect(sacByRole, isNotEmpty);
       expect(sacByRole['backGas']!, greaterThan(0));