diff --git a/app/models.py b/app/models.py index 9ae8830..bcb8595 100644 --- a/app/models.py +++ b/app/models.py @@ -600,45 +600,36 @@ class FuelLog(db.Model): def get_consumption(self, consumption_unit=None, volume_unit='L'): """Calculate consumption for this fill-up. - For full-tank fill-ups, sum every litre poured between the previous - full tank and this one (inclusive) and divide by the distance covered - — the "fill-to-fill" method. Partial fills between two full tanks are - therefore counted (issue #169). If any of the intervening logs is - flagged ``is_missed``, the figure is unknowable and we return None. - - For partial fills, compare against the immediately preceding log of - any type to give an instantaneous "fuel added per km" estimate; this - is a rough indicator, not a true consumption figure. + Only meaningful for full-tank fills: sum every litre poured between + the previous full tank and this one (inclusive) and divide by the + distance covered — the "fill-to-fill" method. Partial fills between + two full tanks are therefore counted in the next full tank's figure + (issue #169). If any of the intervening logs is flagged ``is_missed``, + the figure is unknowable and we return None. + + Partial fills return None: the litres added in a top-up tell you + nothing about consumption over the preceding distance, and surfacing + a number there is misleading (issue #194). """ - if not self.volume: + if not self.volume or not self.is_full_tank: return None - if self.is_full_tank: - prev_full = FuelLog.query.filter( - FuelLog.vehicle_id == self.vehicle_id, - FuelLog.odometer < self.odometer, - FuelLog.is_full_tank == True, - ).order_by(FuelLog.odometer.desc()).first() - if not prev_full: - return None - distance = self.odometer - prev_full.odometer - between = FuelLog.query.filter( - FuelLog.vehicle_id == self.vehicle_id, - FuelLog.odometer > prev_full.odometer, - FuelLog.odometer <= self.odometer, - ).all() - if any(log.is_missed for log in between): - return None - volume_native = sum(log.volume for log in between if log.volume) - else: - prev_log = FuelLog.query.filter( - FuelLog.vehicle_id == self.vehicle_id, - FuelLog.odometer < self.odometer, - ).order_by(FuelLog.odometer.desc()).first() - if not prev_log: - return None - distance = self.odometer - prev_log.odometer - volume_native = self.volume + prev_full = FuelLog.query.filter( + FuelLog.vehicle_id == self.vehicle_id, + FuelLog.odometer < self.odometer, + FuelLog.is_full_tank == True, + ).order_by(FuelLog.odometer.desc()).first() + if not prev_full: + return None + distance = self.odometer - prev_full.odometer + between = FuelLog.query.filter( + FuelLog.vehicle_id == self.vehicle_id, + FuelLog.odometer > prev_full.odometer, + FuelLog.odometer <= self.odometer, + ).all() + if any(log.is_missed for log in between): + return None + volume_native = sum(log.volume for log in between if log.volume) if distance > 0 and volume_native > 0: odometer_unit = self.vehicle.get_effective_odometer_unit() diff --git a/tests/test_fuel.py b/tests/test_fuel.py index 13d048b..9664640 100644 --- a/tests/test_fuel.py +++ b/tests/test_fuel.py @@ -96,7 +96,8 @@ def test_delete_fuel_log(self, auth_client, sample_fuel_log): class TestPartialFillConsumption: - """#122 — consumption should be calculated for partial fills.""" + """#194 — partial fills return no consumption; the next full fill + captures the partial volume over the whole interval (#169).""" def test_full_tank_consumption_unchanged(self, app, test_user, sample_vehicle): log1 = FuelLog(vehicle_id=sample_vehicle.id, user_id=test_user.id, @@ -108,17 +109,14 @@ def test_full_tank_consumption_unchanged(self, app, test_user, sample_vehicle): # 42L / 500km * 100 = 8.4 L/100km assert abs(log2.get_consumption() - 8.4) < 0.01 - def test_partial_fill_returns_consumption(self, app, test_user, sample_vehicle): + def test_partial_fill_returns_none(self, app, test_user, sample_vehicle): log1 = FuelLog(vehicle_id=sample_vehicle.id, user_id=test_user.id, date=date(2024, 1, 1), odometer=10000, volume=40, is_full_tank=True) log2 = FuelLog(vehicle_id=sample_vehicle.id, user_id=test_user.id, date=date(2024, 1, 10), odometer=10200, volume=20, is_full_tank=False) db.session.add_all([log1, log2]) db.session.commit() - # 20L / 200km * 100 = 10.0 L/100km - consumption = log2.get_consumption() - assert consumption is not None - assert abs(consumption - 10.0) < 0.01 + assert log2.get_consumption() is None def test_partial_fill_no_previous_log_returns_none(self, app, test_user, sample_vehicle): log = FuelLog(vehicle_id=sample_vehicle.id, user_id=test_user.id, @@ -134,21 +132,27 @@ def test_no_volume_returns_none(self, app, test_user, sample_vehicle): db.session.commit() assert log.get_consumption() is None - def test_partial_fill_uses_any_previous_log(self, app, test_user, sample_vehicle): - """Partial fill looks back to the nearest log regardless of fill type.""" - log1 = FuelLog(vehicle_id=sample_vehicle.id, user_id=test_user.id, - date=date(2024, 1, 1), odometer=10000, volume=40, is_full_tank=True) - log2 = FuelLog(vehicle_id=sample_vehicle.id, user_id=test_user.id, - date=date(2024, 1, 8), odometer=10300, volume=15, is_full_tank=False) - log3 = FuelLog(vehicle_id=sample_vehicle.id, user_id=test_user.id, - date=date(2024, 1, 15), odometer=10500, volume=20, is_full_tank=False) - db.session.add_all([log1, log2, log3]) + def test_issue_194_partial_then_full(self, app, test_user, sample_vehicle): + """#194 — Steve's reported scenario: + full (62.8L) → partial 3.8L (no consumption) → full 62.1L (real figure + spans the whole interval and includes the 3.8L top-up). + """ + full_a = FuelLog(vehicle_id=sample_vehicle.id, user_id=test_user.id, + date=date(2026, 5, 11), odometer=10000, volume=62.8, + is_full_tank=True) + partial = FuelLog(vehicle_id=sample_vehicle.id, user_id=test_user.id, + date=date(2026, 5, 20), odometer=10557, volume=3.8, + is_full_tank=False) + full_b = FuelLog(vehicle_id=sample_vehicle.id, user_id=test_user.id, + date=date(2026, 5, 21), odometer=10600, volume=62.1, + is_full_tank=True) + db.session.add_all([full_a, partial, full_b]) db.session.commit() - # log3 looks back to log2 (nearest), not log1 - # 20L / 200km * 100 = 10.0 L/100km - consumption = log3.get_consumption() + assert partial.get_consumption() is None + # (3.8 + 62.1) / 600 * 100 = 10.983 L/100km + consumption = full_b.get_consumption() assert consumption is not None - assert abs(consumption - 10.0) < 0.01 + assert abs(consumption - 10.983) < 0.01 def test_full_tank_sums_intervening_partials(self, app, test_user, sample_vehicle): """#169 — full tank consumption must sum partial fills since the previous full."""