From 2e7b4aea798f9dc49e8ef860347f9a2ae239dbb3 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 29 Jul 2026 11:38:16 +0200 Subject: [PATCH] fix(client/apple): a pinned card belongs to its profile, not to its host's binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Grouping by profile asked each HOST which profile it was bound to, then handed the answer to that host's every card. So a card visibly wearing a "Gaming" chip — a pin, on a host bound to nothing — was filed under "No Profile" along with the rest. Every pinned card was, which is most of what the grouping exists to separate. Pins are not bindings. The arrangement works on CARDS now: a host's own card follows its binding, a pinned card follows its pin, and both can land in the same band when a host pinned the profile it is also bound to. The expansion moved out of the view and into the arrangement, because it is exactly what the grouping has to see. The regression is pinned by a test whose fixture is the shape that broke: a host bound to nothing, carrying two pins. Co-Authored-By: Claude Opus 5 (1M context) --- .../PunktfunkClient/Home/HomeView.swift | 26 ++----- .../PunktfunkShared/HostArrangement.swift | 76 +++++++++++++++---- .../SharedFoundationTests.swift | 55 +++++++++----- 3 files changed, 102 insertions(+), 55 deletions(-) diff --git a/clients/apple/Sources/PunktfunkClient/Home/HomeView.swift b/clients/apple/Sources/PunktfunkClient/Home/HomeView.swift index 76f9e61b..9605d6d2 100644 --- a/clients/apple/Sources/PunktfunkClient/Home/HomeView.swift +++ b/clients/apple/Sources/PunktfunkClient/Home/HomeView.swift @@ -63,8 +63,8 @@ struct HomeView: View { groupHeader(title, accent: group.accent) } LazyVGrid(columns: gridColumns, spacing: gridSpacing) { - ForEach(entries(in: group)) { entry in - hostCard(entry.host, pinned: entry.profile) + ForEach(group.cards) { card in + hostCard(card.host, pinned: card.pinned) } } } @@ -210,16 +210,9 @@ struct HomeView: View { // MARK: - Cards - /// One grid tile: a host's primary card, or one of its pinned host+profile cards. A pin is - /// presentation only — same record, same live status, so it is an extra ENTRY here rather than - /// a duplicated host (which would fork pairing, WoL and renames — design §5.2a). - private struct HostGridEntry: Identifiable { - let host: StoredHost - let profile: StreamProfile? - var id: String { "\(host.id.uuidString)#\(profile?.id ?? "")" } - } - - /// The grid's bands, ordered and divided per this device's preference (`HostArrangement`). + /// The grid's bands, ordered and divided per this device's preference — cards and all, so a + /// pinned card can be filed under the profile it connects with rather than under its host's + /// binding (`HostArrangement`). private var hostGroups: [HostGroup] { HostArrangement.groups( hosts: store.hosts, catalog: profiles.catalog, @@ -228,15 +221,6 @@ struct HomeView: View { grouping: HostGrouping(rawValue: groupingRaw) ?? .none) } - /// A band's hosts, each immediately followed by its pinned cards — so a pin reads as belonging - /// to its host rather than as a stray tile somewhere in the grid. - private func entries(in group: HostGroup) -> [HostGridEntry] { - group.hosts.flatMap { host in - [HostGridEntry(host: host, profile: nil)] - + profiles.pinned(for: host).map { HostGridEntry(host: host, profile: $0) } - } - } - /// Online = advertising on mDNS OR answered the reachability probe (a routed/VPN host never /// advertises). One definition, used by the cards and by the Status grouping alike. private func isOnline(_ host: StoredHost) -> Bool { diff --git a/clients/apple/Sources/PunktfunkShared/HostArrangement.swift b/clients/apple/Sources/PunktfunkShared/HostArrangement.swift index 616a618c..750e3a0a 100644 --- a/clients/apple/Sources/PunktfunkShared/HostArrangement.swift +++ b/clients/apple/Sources/PunktfunkShared/HostArrangement.swift @@ -1,10 +1,14 @@ // How the host grid is ordered and grouped. // -// Pure value logic, deliberately: which host lands where is the kind of thing that reads as +// Pure value logic, deliberately: which card lands where is the kind of thing that reads as // obviously right and behaves subtly wrong (an unstable sort reshuffling equal rows on every // redraw, a never-connected host sorting as if it were connected in 1970), so it lives apart from // the view and is tested. It sits in PunktfunkShared because everything it touches — `StoredHost`, // `ProfileCatalog` — already does. +// +// It arranges CARDS, not hosts. A host contributes its own card plus one per pinned profile +// (§5.2a), and under profile grouping those go to different bands — the pinned card belongs to +// the profile it connects with, which is the whole reason it exists. import Foundation @@ -38,8 +42,10 @@ public enum HostSort: String, CaseIterable, Identifiable, Sendable { /// What the grid is divided by. public enum HostGrouping: String, CaseIterable, Identifiable, Sendable { case none - /// The host's DEFAULT profile — the one a plain tap uses. Pinned cards stay with their host: - /// a pin is presentation of that host, not a host of its own. + /// The profile each CARD connects with: a host's own card follows its binding, a pinned card + /// follows its pin. Grouping by the binding alone would file every pinned card under "No + /// Profile" — pins are not bindings, and a pinned card is precisely the one that knows which + /// profile it means. case profile case status @@ -62,6 +68,21 @@ public enum HostGrouping: String, CaseIterable, Identifiable, Sendable { } } +/// One card in the grid: a host, and the profile that card connects with. +public struct HostCard: Identifiable, Equatable, Sendable { + public let host: StoredHost + /// nil = the host's own card, which follows whatever it is bound to. Set = a pinned + /// host+profile card (§5.2a). + public let pinned: StreamProfile? + + public var id: String { "\(host.id.uuidString)#\(pinned?.id ?? "")" } + + public init(host: StoredHost, pinned: StreamProfile? = nil) { + self.host = host + self.pinned = pinned + } +} + /// One rendered band of the grid. `title` is nil for the ungrouped case, which is what tells the /// view to draw no header at all rather than an empty one. public struct HostGroup: Identifiable, Equatable, Sendable { @@ -69,13 +90,13 @@ public struct HostGroup: Identifiable, Equatable, Sendable { public let title: String? /// `#RRGGBB` when the band is a profile — its chip colour, so the header matches the cards. public let accent: String? - public let hosts: [StoredHost] + public let cards: [HostCard] - public init(id: String, title: String?, accent: String? = nil, hosts: [StoredHost]) { + public init(id: String, title: String?, accent: String? = nil, cards: [HostCard]) { self.id = id self.title = title self.accent = accent - self.hosts = hosts + self.cards = cards } } @@ -120,26 +141,46 @@ public enum HostArrangement { }.map(\.host) } + /// A host's own card, then one per pinned profile — so a pin reads as belonging to its + /// host wherever the two sit together. + func cards(_ host: StoredHost) -> [HostCard] { + [HostCard(host: host)] + catalog.pinned(for: host).map { HostCard(host: host, pinned: $0) } + } + switch grouping { case .none: - return [HostGroup(id: "all", title: nil, hosts: ordered(ranked))] + return [HostGroup(id: "all", title: nil, cards: ordered(ranked).flatMap(cards))] case .profile: // Catalog order, so the bands sit in the same order the scope menu lists them, then - // the unbound hosts. A band with nothing in it isn't drawn. + // whatever belongs to no profile. A band with nothing in it isn't drawn. + let sorted = ordered(ranked) var groups: [HostGroup] = catalog.profiles.compactMap { profile in - let members = ranked.filter { catalog.binding(for: $0.host)?.id == profile.id } + let members: [HostCard] = sorted.flatMap { host -> [HostCard] in + var found: [HostCard] = [] + // A host BOUND to this profile brings its own card… + if catalog.binding(for: host)?.id == profile.id { + found.append(HostCard(host: host)) + } + // …and a host that PINNED it brings that pinned card, whatever it is bound + // to. Both can be true: a bound profile may also be pinned. + if let pin = catalog.pinned(for: host).first(where: { $0.id == profile.id }) { + found.append(HostCard(host: host, pinned: pin)) + } + return found + } guard !members.isEmpty else { return nil } return HostGroup( id: "profile-\(profile.id)", title: profile.name, accent: profile.accent, - hosts: ordered(members)) + cards: members) } - // A dangling binding resolves as none (§4.4), so it lands here rather than in a band - // named after a profile that no longer exists. - let unbound = ranked.filter { catalog.binding(for: $0.host) == nil } + // Hosts that a plain tap streams with the defaults. A dangling binding resolves as + // none (§4.4), so it lands here rather than in a band named after a profile that no + // longer exists. + let unbound = sorted.filter { catalog.binding(for: $0) == nil }.map { HostCard(host: $0) } if !unbound.isEmpty { groups.append( - HostGroup(id: "profile-none", title: "No Profile", hosts: ordered(unbound))) + HostGroup(id: "profile-none", title: "No Profile", cards: unbound)) } return groups @@ -148,11 +189,14 @@ public enum HostArrangement { let up = ranked.filter { online.contains($0.host.id) } let down = ranked.filter { !online.contains($0.host.id) } if !up.isEmpty { - groups.append(HostGroup(id: "status-online", title: "Online", hosts: ordered(up))) + groups.append( + HostGroup(id: "status-online", title: "Online", + cards: ordered(up).flatMap(cards))) } if !down.isEmpty { groups.append( - HostGroup(id: "status-offline", title: "Offline", hosts: ordered(down))) + HostGroup(id: "status-offline", title: "Offline", + cards: ordered(down).flatMap(cards))) } return groups } diff --git a/clients/apple/Tests/PunktfunkKitTests/SharedFoundationTests.swift b/clients/apple/Tests/PunktfunkKitTests/SharedFoundationTests.swift index 37a8fcec..42b2cee5 100644 --- a/clients/apple/Tests/PunktfunkKitTests/SharedFoundationTests.swift +++ b/clients/apple/Tests/PunktfunkKitTests/SharedFoundationTests.swift @@ -235,6 +235,8 @@ final class SharedFoundationTests: XCTestCase { var couch = StoredHost(name: "Couch", address: "10.0.0.4", addedAt: Date(timeIntervalSince1970: 300)) couch.lastConnected = Date(timeIntervalSince1970: 9_000) + // Couch is bound to nothing but has PINNED both profiles as their own cards. + couch.pinnedProfileIDs = ["111111111111", "222222222222"] return ([basement, desk, attic, couch], catalog) } @@ -246,51 +248,67 @@ final class SharedFoundationTests: XCTestCase { hosts: hosts, catalog: catalog, online: online, sort: sort, grouping: grouping) } + /// Card labels: the host, plus the pinned profile when the card carries one. + private func labels(_ group: HostGroup) -> [String] { + group.cards.map { card in + card.pinned.map { "\(card.host.name)·\($0.name)" } ?? card.host.name + } + } + /// The default is the order the grid had before it could sort at all — an update must not - /// rearrange anyone's hosts. + /// rearrange anyone's hosts. Each host is followed by its own pinned cards. /// /// Undated hosts sort FIRST, because having no date means predating the field means being /// older. In a real store they are a prefix (everything saved before the upgrade), so the /// result is the stored order exactly — which is the point. func testHostSortByDateAddedKeepsTheStoredOrder() { - let (hosts, _) = arrangementFixture() let group = arranged(.added)[0] XCTAssertNil(group.title, "ungrouped draws no header") - XCTAssertEqual(group.hosts.map(\.name), hosts.map(\.name)) - XCTAssertEqual(group.hosts.map(\.name), ["Basement", "Desk", "attic", "Couch"]) + XCTAssertEqual( + labels(group), ["Basement", "Desk", "attic", "Couch", "Couch·Game", "Couch·Work"]) } /// Case- and locale-insensitive, so "attic" doesn't sort after "Desk" the way a raw `<` on /// Strings would put every lowercase name below every uppercase one. func testHostSortByNameIgnoresCase() { XCTAssertEqual( - arranged(.name)[0].hosts.map(\.name), ["attic", "Basement", "Couch", "Desk"]) + arranged(.name)[0].cards.map(\.host.name), + ["attic", "Basement", "Couch", "Couch", "Couch", "Desk"]) } /// Most recent first — and a host you have NEVER connected to goes last, not first. Treating /// "never" as `.distantPast` would sort it as if it had been connected in 1970. func testHostSortByLastConnectedPutsNeverConnectedLast() { - let hosts = arranged(.lastConnected)[0].hosts - XCTAssertEqual(hosts.prefix(2).map(\.name), ["Couch", "Desk"]) + let names = arranged(.lastConnected)[0].cards.map(\.host.name) + XCTAssertEqual(names.prefix(4).map { $0 }, ["Couch", "Couch", "Couch", "Desk"]) // The two never-connected ones tie, so they keep their stored order — `sorted` is not // stable in Swift, and equal rows swapping between redraws is a visible bug. - XCTAssertEqual(hosts.suffix(2).map(\.name), ["Basement", "attic"]) + XCTAssertEqual(names.suffix(2).map { $0 }, ["Basement", "attic"]) } - /// Bands in catalog order, then the unbound ones. An empty band isn't drawn at all. - func testHostGroupingByProfile() { + /// ⭐ The regression this was written for: a PINNED card belongs to the profile it connects + /// with, not to whatever its host is bound to. Grouping on the binding alone filed every + /// pinned card under "No Profile" — including the ones visibly wearing a profile chip. + func testHostGroupingFilesPinnedCardsUnderTheirOwnProfile() { let groups = arranged(.name, .profile) XCTAssertEqual(groups.map(\.title), ["Game", "Work", "No Profile"]) + // Desk is BOUND to Game; Couch merely pinned it. Both belong in the Game band. + XCTAssertEqual(labels(groups[0]), ["Couch·Game", "Desk"]) + XCTAssertEqual(labels(groups[1]), ["attic", "Couch·Work"]) + // Couch's OWN card streams with the defaults, so that one is unbound. + XCTAssertEqual(labels(groups[2]), ["Basement", "Couch"]) XCTAssertEqual(groups[0].accent, "#ff8800", "the header matches its cards") - XCTAssertEqual(groups[0].hosts.map(\.name), ["Desk"]) - XCTAssertEqual(groups[2].hosts.map(\.name), ["Basement", "Couch"]) + } - // A profile nobody uses gets no band. - let unused = ProfileCatalog(profiles: [StreamProfile(name: "Travel", id: "333333333333")]) + /// A profile nobody uses gets no band at all. + func testHostGroupingSkipsEmptyBands() { let (hosts, _) = arrangementFixture() - let none = HostArrangement.groups( + let unused = ProfileCatalog(profiles: [StreamProfile(name: "Travel", id: "333333333333")]) + let groups = HostArrangement.groups( hosts: hosts, catalog: unused, online: [], sort: .name, grouping: .profile) - XCTAssertEqual(none.map(\.title), ["No Profile"], "every host is unbound in this catalog") + XCTAssertEqual(groups.map(\.title), ["No Profile"]) + // The pins point at profiles this catalog doesn't have, so they produce no cards either. + XCTAssertEqual(labels(groups[0]), ["attic", "Basement", "Couch", "Desk"]) } /// A dangling binding resolves as no profile (§4.4), so it lands in the unbound band rather @@ -316,8 +334,9 @@ final class SharedFoundationTests: XCTestCase { let up = hosts.filter { ["Desk", "Couch"].contains($0.name) }.map(\.id) let some = groups(online: Set(up)) XCTAssertEqual(some.map(\.title), ["Online", "Offline"]) - XCTAssertEqual(some[0].hosts.map(\.name), ["Couch", "Desk"]) - XCTAssertEqual(some[1].hosts.map(\.name), ["attic", "Basement"]) + // A pinned card follows its host's status — same record, same reachability. + XCTAssertEqual(labels(some[0]), ["Couch", "Couch·Game", "Couch·Work", "Desk"]) + XCTAssertEqual(labels(some[1]), ["attic", "Basement"]) // All offline: no empty "Online" band. XCTAssertEqual(groups(online: []).map(\.title), ["Offline"])