From 35e0adaf0bc3d300641d3646701d53eba0e1c69d Mon Sep 17 00:00:00 2001 From: FuelBoard Contributor Date: Wed, 12 Aug 2026 08:31:08 +0100 Subject: [PATCH] Favourites are fuel-scoped: starring pins the station for one fuel only Fixing the bug where favouriting an Unleaded entry also created a Diesel favourite. Favourites are now [FavouriteEntry] = (station, fuel) pairs: - Starring a row pins it only for the fuel being viewed - Favourites tabs show only fuels that actually have favourites - Geofence monitor gives priority slots only to favourites matching the monitored fuel - Tests updated + new fuel-scoping test (31/31 passing) --- FuelBoard/ContentView.swift | 18 ++++---- FuelBoard/FavouritesView.swift | 34 +++++++------- FuelBoard/ProximityMonitor.swift | 18 +++++--- FuelBoard/StationsView.swift | 4 +- .../FuelBoardSharedTests/FuelBoardTests.swift | 23 +++++++--- Shared/FuelStore.swift | 45 ++++++++++++------- 6 files changed, 88 insertions(+), 54 deletions(-) diff --git a/FuelBoard/ContentView.swift b/FuelBoard/ContentView.swift index bb63582..a3c89e2 100644 --- a/FuelBoard/ContentView.swift +++ b/FuelBoard/ContentView.swift @@ -10,7 +10,7 @@ struct ContentView: View { @State private var sortMode: SortMode = FuelStore.loadSortMode() @State private var stationLimit: Int = FuelStore.loadStationLimit() @State private var distanceUnit: DistanceUnit = FuelStore.loadDistanceUnit() - @State private var favourites: [FuelStation] = FuelStore.loadFavourites() + @State private var favourites: [FavouriteEntry] = FuelStore.loadFavourites() @State private var alertsEnabled: Bool = FuelStore.loadAlertsEnabled() @State private var alertsRadius: Double = FuelStore.loadAlertsRadius() @State private var location: Coordinate? = { @@ -95,11 +95,13 @@ struct ContentView: View { } } + /// Station IDs favourited for the CURRENTLY selected fuel — the star on a + /// Stations-tab row reflects the fuel being viewed (fuel-scoped favourites). private var favouriteIDs: Set { - Set(favourites.map(\.id)) + Set(favourites.filter { $0.fuel == selectedFuel }.map(\.station.id)) } - private var refreshedFavourites: [FuelStation] { + private var refreshedFavourites: [FavouriteEntry] { FuelStore.refreshedFavourites(favourites, from: stations) } @@ -127,7 +129,6 @@ struct ContentView: View { selectedFuel: selectedFuel, location: location, distanceUnit: distanceUnit, - favouriteIDs: favouriteIDs, onToggleFavourite: toggleFavourite ) .tabItem { Label("Favourites", systemImage: "star.fill") } @@ -222,11 +223,12 @@ struct ContentView: View { } } - private func toggleFavourite(_ station: FuelStation) { - if favouriteIDs.contains(station.id) { - favourites.removeAll { $0.id == station.id } + private func toggleFavourite(_ station: FuelStation, fuel: FuelType) { + let key = FavouriteEntry(station: station, fuel: fuel) + if favourites.contains(where: { $0.id == key.id }) { + favourites.removeAll { $0.id == key.id } } else { - favourites.append(station) + favourites.append(key) } FuelStore.saveFavourites(favourites) WidgetCenter.shared.reloadAllTimelines() diff --git a/FuelBoard/FavouritesView.swift b/FuelBoard/FavouritesView.swift index 22c4f2b..b27253b 100644 --- a/FuelBoard/FavouritesView.swift +++ b/FuelBoard/FavouritesView.swift @@ -3,20 +3,21 @@ import SwiftUI /// Favourites tab — starred stations, split by fuel type. Each fuel type /// with at least one favourite appears as its own tab; that tab's favourites /// are ranked cheapest-first for that fuel, with the cheapest called out. +/// Favourites are fuel-scoped entries — starring a station only pins it for +/// the fuel you were viewing, so an Unleaded favourite never shows as Diesel. struct FavouritesView: View { - let favourites: [FuelStation] + let favourites: [FavouriteEntry] /// The app's currently selected fuel — used only as the initial tab. let selectedFuel: FuelType let location: Coordinate? let distanceUnit: DistanceUnit - let favouriteIDs: Set - var onToggleFavourite: (FuelStation) -> Void = { _ in } + var onToggleFavourite: (FuelStation, FuelType) -> Void = { _, _ in } - /// Fuel types that currently have at least one favourite selling them — - /// these are the only tabs shown. + /// Fuel types that currently have at least one favourite — these are the + /// only tabs shown (a fuel with no favourites gets no tab). private var availableFuels: [FuelType] { FuelType.allCases.filter { fuel in - favourites.contains { $0.prices[fuel] != nil } + favourites.contains { $0.fuel == fuel } } } @@ -28,9 +29,9 @@ struct FavouritesView: View { availableFuels.contains(fuel) ? fuel : (availableFuels.first ?? .e10) } - /// Favourites that sell the active fuel, cheapest first (distance tiebreak). + /// Stations favourited for the active fuel, cheapest first (distance tiebreak). private var ranked: [FuelStation] { - let available = favourites.filter { $0.prices[activeFuel] != nil } + let available = favourites.filter { $0.fuel == activeFuel }.map(\.station) return available.sorted { lhs, rhs in let lPrice = lhs.prices[activeFuel]! let rPrice = rhs.prices[activeFuel]! @@ -41,20 +42,23 @@ struct FavouritesView: View { } } + /// IDs favourited for the active fuel — the star state on each row. + private var activeFuelFavouriteIDs: Set { + Set(favourites.filter { $0.fuel == activeFuel }.map(\.station.id)) + } + private var cheapest: FuelStation? { ranked.first } private var cheapestPrice: Double? { cheapest?.prices[activeFuel] } - init(favourites: [FuelStation], + init(favourites: [FavouriteEntry], selectedFuel: FuelType, location: Coordinate?, distanceUnit: DistanceUnit, - favouriteIDs: Set, - onToggleFavourite: @escaping (FuelStation) -> Void = { _ in }) { + onToggleFavourite: @escaping (FuelStation, FuelType) -> Void = { _, _ in }) { self.favourites = favourites self.selectedFuel = selectedFuel self.location = location self.distanceUnit = distanceUnit - self.favouriteIDs = favouriteIDs self.onToggleFavourite = onToggleFavourite _fuel = State(initialValue: selectedFuel) } @@ -70,7 +74,7 @@ struct FavouritesView: View { .foregroundStyle(.secondary) Text("No favourites yet") .font(.headline) - Text("Tap the star on any station in the Stations tab to pin it here, ranked by which is cheapest.") + Text("Tap the star on any station in the Stations tab to pin it here, ranked by which is cheapest for that fuel.") .font(.caption) .foregroundStyle(.secondary) .multilineTextAlignment(.center) @@ -108,8 +112,8 @@ struct FavouritesView: View { distanceUnit: distanceUnit, baselinePrice: cheapestPrice, isTopResult: index == 0, - isFavourite: favouriteIDs.contains(station.id), - onToggleFavourite: { onToggleFavourite(station) } + isFavourite: activeFuelFavouriteIDs.contains(station.id), + onToggleFavourite: { onToggleFavourite(station, activeFuel) } ) } } diff --git a/FuelBoard/ProximityMonitor.swift b/FuelBoard/ProximityMonitor.swift index 026de85..10f1779 100644 --- a/FuelBoard/ProximityMonitor.swift +++ b/FuelBoard/ProximityMonitor.swift @@ -31,15 +31,20 @@ final class ProximityMonitor: NSObject, ObservableObject, @preconcurrency CLLoca fuel = FuelStore.loadSelectedFuel() radiusKM = FuelStore.loadAlertsRadius() stations = FuelStore.loadStations() - favourites = FuelStore.loadFavourites() + favourites = FuelStore.loadFavourites().filter { $0.fuel == fuel }.map(\.station) } /// Re-registers geofences. Call whenever stations/favourites/settings change. - func update(stations: [FuelStation], favourites: [FuelStation], fuel: FuelType, radiusKM: Double) { + /// Favourites are fuel-scoped entries; only entries for the monitored fuel + /// get priority slots (a Diesel favourite must not grab a slot while + /// monitoring Unleaded). + func update(stations: [FuelStation], favourites: [FavouriteEntry], fuel: FuelType, radiusKM: Double) { // Fall back to the shared cache when called before the first fetch // completes (launch, background region-event wake). self.stations = stations.isEmpty ? FuelStore.loadStations() : stations - self.favourites = favourites.isEmpty ? FuelStore.loadFavourites() : favourites + self.favourites = favourites.isEmpty + ? FuelStore.loadFavourites().filter { $0.fuel == fuel }.map(\.station) + : favourites.filter { $0.fuel == fuel }.map(\.station) self.fuel = fuel self.radiusKM = radiusKM @@ -51,8 +56,9 @@ final class ProximityMonitor: NSObject, ObservableObject, @preconcurrency CLLoca guard enabled else { return } // Favourites first (guaranteed slots), then closest stations, max 18. - var candidates: [FuelStation] = favourites - let favIDs = Set(favourites.map(\.id)) + // `self.favourites` is already filtered to the monitored fuel. + var candidates: [FuelStation] = self.favourites + let favIDs = Set(self.favourites.map(\.id)) let location = FuelStore.loadLocation() let others = stations .filter { !favIDs.contains($0.id) } @@ -88,7 +94,7 @@ final class ProximityMonitor: NSObject, ObservableObject, @preconcurrency CLLoca } else { requestPermissions() // Re-register geofences from restored cache immediately. - update(stations: stations, favourites: favourites, fuel: fuel, radiusKM: radiusKM) + update(stations: stations, favourites: FuelStore.loadFavourites(), fuel: fuel, radiusKM: radiusKM) } } diff --git a/FuelBoard/StationsView.swift b/FuelBoard/StationsView.swift index 6535781..15e1fce 100644 --- a/FuelBoard/StationsView.swift +++ b/FuelBoard/StationsView.swift @@ -14,7 +14,7 @@ struct StationsView: View { let topStationID: String? let location: Coordinate? let favouriteIDs: Set - var onToggleFavourite: (FuelStation) -> Void = { _ in } + var onToggleFavourite: (FuelStation, FuelType) -> Void = { _, _ in } var onRefresh: () async -> Void = {} /// Pagination: one page = 10 rows, reset whenever the underlying list @@ -107,7 +107,7 @@ struct StationsView: View { baselinePrice: baselinePrice, isTopResult: station.id == topStationID, isFavourite: favouriteIDs.contains(station.id), - onToggleFavourite: { onToggleFavourite(station) } + onToggleFavourite: { onToggleFavourite(station, selectedFuel) } ) } diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift index dd13e21..c22fffa 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift @@ -217,18 +217,29 @@ final class FuelTypeLabelTests: XCTestCase { final class FavouriteRefreshTests: XCTestCase { func testRefreshedFavouritesApplyFreshPrices() { - let fav = FuelStation(id: "s1", name: "OLD NAME", brand: "X", address: "", postcode: "", lat: 0, lng: 0, prices: [.e10: 140.0], priceUpdated: nil) + let fav = FavouriteEntry(station: FuelStation(id: "s1", name: "OLD NAME", brand: "X", address: "", postcode: "", lat: 0, lng: 0, prices: [.e10: 140.0], priceUpdated: nil), fuel: .e10) let fresh = FuelStation(id: "s1", name: "Fresh Station", brand: "X", address: "", postcode: "", lat: 0, lng: 0, prices: [.e10: 132.9], priceUpdated: nil) let updated = FuelStore.refreshedFavourites([fav], from: [fresh]) XCTAssertEqual(updated.count, 1) - XCTAssertEqual(updated[0].name, "Fresh Station") - XCTAssertEqual(updated[0].prices[.e10], 132.9) + XCTAssertEqual(updated[0].station.name, "Fresh Station") + XCTAssertEqual(updated[0].station.prices[.e10], 132.9) + XCTAssertEqual(updated[0].fuel, .e10, "fuel scoping survives refresh") } func testRefreshedFavouritesKeepUnmatchedSnapshot() { - let fav = FuelStation(id: "s1", name: "Cached", brand: "X", address: "", postcode: "", lat: 0, lng: 0, prices: [.e10: 140.0], priceUpdated: nil) + let fav = FavouriteEntry(station: FuelStation(id: "s1", name: "Cached", brand: "X", address: "", postcode: "", lat: 0, lng: 0, prices: [.e10: 140.0], priceUpdated: nil), fuel: .diesel) let updated = FuelStore.refreshedFavourites([fav], from: []) - XCTAssertEqual(updated[0].name, "Cached", "unmatched favourite keeps its snapshot") - XCTAssertEqual(updated[0].prices[.e10], 140.0) + XCTAssertEqual(updated[0].station.name, "Cached", "unmatched favourite keeps its snapshot") + XCTAssertEqual(updated[0].station.prices[.e10], 140.0) + XCTAssertEqual(updated[0].fuel, .diesel, "fuel scoping survives unmatched refresh") + } + + func testFavouriteEntryIDIsFuelScoped() { + let station = FuelStation(id: "s1", name: "X", brand: "X", address: "", postcode: "", lat: 0, lng: 0, prices: [.e10: 140.0, .diesel: 150.0], priceUpdated: nil) + let unleaded = FavouriteEntry(station: station, fuel: .e10) + let diesel = FavouriteEntry(station: station, fuel: .diesel) + XCTAssertNotEqual(unleaded.id, diesel.id, "same station favourited for two fuels is two distinct favourites") + XCTAssertTrue(unleaded.id.hasSuffix("|s1")) + XCTAssertTrue(diesel.id.hasPrefix("diesel|")) } } diff --git a/Shared/FuelStore.swift b/Shared/FuelStore.swift index 0315ef6..d37df42 100644 --- a/Shared/FuelStore.swift +++ b/Shared/FuelStore.swift @@ -17,6 +17,16 @@ struct Coordinate: Equatable, Codable { let lng: Double } +/// A favourite = a station pinned for ONE fuel type. Starring a row while +/// viewing Unleaded only creates an Unleaded favourite, so the same station +/// can be favourite for Diesel independently (or not at all). +struct FavouriteEntry: Identifiable, Codable, Equatable { + var station: FuelStation + let fuel: FuelType + + var id: String { "\(fuel.rawValue)|\(station.id)" } +} + enum SortMode: String, Codable, CaseIterable, Identifiable { case cheapest case closest @@ -234,7 +244,7 @@ struct FuelStore { static let sortModeKey = "fuelboard.sortMode" // SortMode raw value static let stationLimitKey = "fuelboard.stationLimitMiles" // Int miles (5/10/15) static let distanceUnitKey = "fuelboard.distanceUnit" // DistanceUnit raw value - static let favouritesKey = "fuelboard.favourites" // [FuelStation] JSON + static let favouritesKey = "fuelboard.favourites" // [FavouriteEntry] JSON static let alertsEnabledKey = "fuelboard.alertsEnabled" // Bool static let alertsRadiusKey = "fuelboard.alertsRadius" // Double km static let onboardingCompletedKey = "fuelboard.onboardingCompleted" // Bool @@ -348,24 +358,27 @@ struct FuelStore { // MARK: Favourites - static func loadFavourites() -> [FuelStation] { + /// A favourite pins a station FOR ONE fuel type. Starring a row while + /// viewing Unleaded only creates an Unleaded favourite — the station is + /// not automatically favourited for Premium or Diesel. + static func loadFavourites() -> [FavouriteEntry] { if let data = keychainData(service: favouritesKey), - let favs = try? JSONDecoder().decode([FuelStation].self, from: data) { - return favs.map { fav in - var f = fav - f.name = fav.name.sanitizedStationTitle - return f + let entries = try? JSONDecoder().decode([FavouriteEntry].self, from: data) { + return entries.map { entry in + var e = entry + e.station.name = entry.station.name.sanitizedStationTitle + return e } } if let defaults = UserDefaults(suiteName: appGroupSuite), let data = defaults.data(forKey: favouritesKey), - let favs = try? JSONDecoder().decode([FuelStation].self, from: data) { - return favs + let entries = try? JSONDecoder().decode([FavouriteEntry].self, from: data) { + return entries } return [] } - static func saveFavourites(_ favourites: [FuelStation]) { + static func saveFavourites(_ favourites: [FavouriteEntry]) { if let data = try? JSONEncoder().encode(favourites) { UserDefaults(suiteName: appGroupSuite)?.set(data, forKey: favouritesKey) writeKeychain(data: data, service: favouritesKey) @@ -374,14 +387,12 @@ struct FuelStore { /// Returns favourites with fresh prices applied from the given station list /// (favourites keep their cached snapshot when not in the current results). - static func refreshedFavourites(_ favourites: [FuelStation], from stations: [FuelStation]) -> [FuelStation] { - var updated = favourites - for (i, fav) in favourites.enumerated() { - if let fresh = stations.first(where: { $0.id == fav.id }) { - updated[i] = fresh - } + /// The fuel scoping is preserved per entry. + static func refreshedFavourites(_ favourites: [FavouriteEntry], from stations: [FuelStation]) -> [FavouriteEntry] { + favourites.map { entry in + guard let fresh = stations.first(where: { $0.id == entry.station.id }) else { return entry } + return FavouriteEntry(station: fresh, fuel: entry.fuel) } - return updated } // MARK: Alerts