Copy a master packing item to other people from the Master Lists tab — independent deep-equal copies the rules engine materialises onto trips.
MasterPackingItem copies → rules-engine materialisation onto matching trips.ItemConditions, not the raw blob) — deep-equal and independent for free via value semantics.globals vs tripsLocal) decouple the two failure regimes.Person.masterPackingItems via normalizedName; a test proves the unsaved-insert race (AC 3.5) is covered.PersonAvatar added to the picker row, eligibility computed once per body, a testable copyOutcome seam with branch tests, a Person.displayName helper, and the rules-engine triggers note updated.Ready to push
All 21 acceptance criteria are implemented and traced to symbols; four parallel review agents (reuse, quality, efficiency, spec/docs) found no blockers and no spec divergences. The findings raised were one reuse-major (picker row missing PersonAvatar), an efficiency/quality recompute, one spec-major test gap (the performCopy save-failure / all-skipped / deferred branches), and minor cleanups — all fixed in this review. One caveat before merge: verification ran against a flaky simulator test-host; the new suites were confirmed green on a single booted simulator, but a clean-environment make test is advisable.
8a06714 [doc]: Add copy-master-packing-items spec 8f97aed [feat]: Add copy-master-packing persistence helpers cd4c0de [feat]: harden copy-master-packing-item test suite 0aa5ab9 [doc]: changelog for persistence phase 85fce99 [feat]: copy master packing items to other people (UI & wiring) 38533d9 [refactor]: Clarify copy-sheet identity and toast duration cd3d3d2 [doc]: changelog for UI phase 78631ef [refactor]: apply pre-push review fixes Each person in the Master Lists tab has their own packing items, and every item carries the rules that decide when it appears on a trip. Re-typing “underwear” or “shirts” for each family member is tedious. This adds a “Copy to people…” action: pick the others, and each gets an independent copy — same name, same rules — which the app then adds to every matching trip automatically.
One action replaces re-entering the same item per person, and the copy behaves exactly like one authored by hand.
MasterPersistence.copyPacking(source:toPersonIDs:in:) — one MasterPackingItem per eligible target (de-duped ids, skips same-name owners), inserted into globals without saving. Returns value-only CopyResult. Pure helpers normalizedName and copyToastMessage.CopyPackingItemSheet — the picker; a @MainActor static eligibleTargets(source:people:) seam computes eligibility from Person.masterPackingItems via the same normalizedName the skip uses. Multi-select, ineligible rows disabled, empty-state, confirm gated on selection ∩ eligible.MasterPackingList — per-row swipe + context affordance (gated on source eligibility), one SheetTarget enum behind a single .sheet(item:), the toast host, and performCopy.performCopy mirrors MasterPackingEditor.runEngineAndDismiss: copyPacking → save (rollback + toast on throw) → runForAllNonPastTrips() (best-effort) → confirmation toast. Branch selection is extracted into a pure copyOutcome(createdCount:save:runEngine:) so save-failure / all-skipped / deferred paths are unit-testable.
Person.masterPackingItems through normalizedName; a test proves SwiftData surfaces an unsaved in-context same-name insert through that relationship, so the AC 3.5 re-check holds against a picker→confirm race.LocalWriteHook.mapping(for:) returns nil for Person/Master*, so master creation bypasses the hook and saves to globals; only the engine’s trip-zone writes go through the hook. Copied masters sync via the globals CloudKit mirror; copied trip items reach participants via the hook-backed apply path — both reused.runForAllNonPastTrips swallows per-trip apply failures (rollback + continue) and throws only at the top level, so the deferred toast covers a top-level recompute failure; per-trip failures self-heal on the next trigger (Decision 8 — shared runner unchanged).No schema change. Additive: one view, one persistence helper family, a per-row affordance + toast host. SheetTarget collapses the list’s sheets into one modifier with a "copy-\(id)" discriminator preventing .copy/.edit identity collision.
copyOutcome seam, not a forced-SwiftData-failure integration test.MasterPersistence.swift
Why it matters. Core of the feature — creates the independent copies, owns the same-name skip and de-dupe, and is the sole filter authority at confirm time.
What to look at. MasterPersistence.swift — copyPacking(source:toPersonIDs:in:), normalizedName, copyToastMessage, CopyResult
MasterPackingList.swift
Why it matters. Orchestrates save→engine→toast across two containers; the extracted seam makes the save-failure, all-skipped, and deferred branches unit-testable without forcing a real SwiftData save failure.
What to look at. MasterPackingList.swift — performCopy, copyOutcome(createdCount:save:runEngine:), CopyOutcome enum
CopyPackingItemSheet.swift
Why it matters. Picker correctness depends on display eligibility and the persistence skip never diverging; the avatar restores the app-wide person-identity affordance.
What to look at. CopyPackingItemSheet.swift — eligibleTargets(source:people:), single-pass body, PersonAvatar row
MasterPackingList.swift
Why it matters. Avoids the SwiftUI multiple-.sheet conflict when adding the copy presentation alongside create/edit.
What to look at. MasterPackingList.swift — SheetTarget { create, edit(id), copy(id) }
Conditions live only on per-person MasterPackingItem; TripPackingItem has none. A master-level copy is the only place “requirements stay the same” is literally true and reusable across trips. (Decision 1)
A target already owning a same-name item is skipped (marked ineligible in the picker, re-checked at confirm), never overwritten — protects deliberately customised conditions. Case-insensitive, trimmed. (Decision 2)
The toast hosts on MasterPackingList (which survives the picker dismissing), and an all-skipped confirm creates nothing, skips save + engine, and reports success. (Decision 5)
runForAllNonPastTrips swallows per-trip failures and throws only top-level; the deferred toast covers a top-level failure, per-trip failures self-heal on the next trigger. The shared runner was deliberately not modified for this feature. (Decision 8)
The copy confirmation uses duration: 5 so it isn’t lost while the picker sheet dismisses; commented at the call site. A deliberate, commented deviation.
| Severity | Area | Finding | Resolution |
|---|---|---|---|
| major | CopyPackingItemSheet row (reuse) | Picker row omitted PersonAvatar that every other person-row surface uses — a reuse miss and a person-colour design-pillar inconsistency. | Added PersonAvatar(name:colorKey:size:.standard) leading the row, mirroring TripEditorPeoplePicker.personRow. |
| major | performCopy orchestration (spec/test) | performCopy save-failure / all-skipped / deferred-recompute branches (AC 3.6/3.7/4.2) had no direct test — only the underlying helpers and one happy-path UI test. | Extracted pure copyOutcome(createdCount:save:runEngine:) seam and added CopyOutcomeTests covering all four branches incl. effect-not-called assertions. |
| minor | CopyPackingItemSheet (efficiency/quality) | eligibleTargets recomputed 2+N times per body eval (eligibleIDs / hasEligibleTarget / canConfirm / per row). | Compute eligibility once in body and thread eligibleIDs into the rows/empty-state/confirm checks; static helper kept for tests. |
| minor | Person model (reuse) | person.name.isEmpty ? "Unnamed" : name literal duplicated; no shared helper. | Added Person.displayName extension and routed the new copy-feature call site through it (pre-existing sites left out of scope). |
| minor | docs/agent-notes/rules-engine.md | The recompute-triggers table omitted the new MasterPackingList.performCopy caller of runForAllNonPastTrips(). | Added a one-line trigger-table row for the copy flow. |
| minor | MasterPersistence.formatNames (reuse) | formatNames hand-rolls Oxford-comma list joining that Foundation's ListFormatter / formatted(.list(type:.and)) provides. | Skipped: switching couples the hardened exact-string toast tests to locale; the app's localization is project-deferred. Revisit at localization time. |
| minor | performCopy vs MasterPackingEditor (quality) | performCopy duplicates the ~3-line runner construction shared with MasterPackingEditor.runEngineAndDismiss. | Skipped: a true extraction means modifying the untouched MasterPackingEditor (regression risk) for marginal DRY; the mirroring is intentional house pattern. |
| minor | SheetTarget.id (quality) | SheetTarget.id uses a stringly-typed "copy-\(id)" discriminator. | Skipped: justified and commented; matches the existing MasterTasksList/TaskForm convention. |
Click to expand.
diff --git a/Scramble/Scramble/Features/MasterLists/CopyPackingItemSheet.swift b/Scramble/Scramble/Features/MasterLists/CopyPackingItemSheet.swiftnew file mode 100644index 0000000..ae0d1a7--- /dev/null+++ b/Scramble/Scramble/Features/MasterLists/CopyPackingItemSheet.swift@@ -0,0 +1,169 @@+import SwiftData+import SwiftUI++#if canImport(UIKit)+ import UIKit+#endif++/// Target-person picker for copying a single `MasterPackingItem` to other+/// people (Req 1.2). Presentation only: it owns the multi-select state and+/// passes the RAW selected ids to `onCopy` — `MasterPersistence.copyPacking`+/// is the sole authority that drops ineligible / raced-out targets, so the+/// sheet never re-filters on confirm (design "Components and Interfaces").+///+/// Eligibility for display is read off `Person.masterPackingItems` via the+/// SAME `MasterPersistence.normalizedName` comparator the persistence skip+/// uses, so the picker display and the confirm-time skip agree.+@MainActor struct CopyPackingItemSheet: View {+ let source: MasterPackingItem+ /// Raw selected person ids; the parent (`MasterPackingList.performCopy`)+ /// does the work and `copyPacking` re-checks eligibility.+ let onCopy: ([UUID]) -> Void+ let onCancel: () -> Void++ @Query(sort: \Person.name) private var allPeople: [Person]+ @Environment(\.theme) private var theme+ @Environment(\.colorScheme) private var colorScheme++ @State private var selected: Set<UUID> = []++ /// Eligible target people for a copy of `source`: everyone EXCEPT the source+ /// owner (2.1), MINUS anyone who already owns a same-name item (2.3). Uses+ /// `MasterPersistence.normalizedName` — the same comparator `copyPacking`+ /// applies — over each person's `masterPackingItems` relationship, the same+ /// data path the persistence skip reads.+ static func eligibleTargets(source: MasterPackingItem, people: [Person]) -> [Person] {+ let ownerID = source.person?.id+ let sourceKey = MasterPersistence.normalizedName(source.name)+ return people.filter { person in+ guard person.id != ownerID else { return false }+ let alreadyOwns = (person.masterPackingItems ?? []).contains {+ MasterPersistence.normalizedName($0.name) == sourceKey+ }+ return !alreadyOwns+ }+ }++ /// People rendered in the picker: everyone except the owner, sorted by name+ /// (2.1). Ineligible (same-name owner) people stay in the list but disabled.+ private var targetPeople: [Person] {+ allPeople.filter { $0.id != source.person?.id }+ }++ var body: some View {+ // Compute eligibility ONCE per body pass (the O(people×items) scan), then+ // thread `eligibleIDs` into the rows / empty-state / confirm-enabled checks+ // instead of each computed property re-running the scan.+ let eligible = Self.eligibleTargets(source: source, people: allPeople)+ let eligibleIDs = Set(eligible.map(\.id))+ let hasEligibleTarget = !eligibleIDs.isEmpty+ // Confirm enabled only when at least one *eligible* person is selected+ // (2.4). Intersecting against `eligibleIDs` means a stale selection of a+ // since-ineligible person cannot enable the button.+ let canConfirm = !selected.isDisjoint(with: eligibleIDs)++ return NavigationStack {+ Group {+ if hasEligibleTarget {+ peopleList(eligibleIDs: eligibleIDs)+ } else {+ emptyState+ }+ }+ .navigationTitle("Copy “\(source.name)”")+ .navigationBarTitleDisplayMode(.inline)+ .toolbar {+ ToolbarItem(placement: .cancellationAction) {+ Button("Cancel") { onCancel() }+ }+ ToolbarItem(placement: .confirmationAction) {+ Button("Copy") { confirm() }+ .disabled(!canConfirm)+ .accessibilityLabel("Copy item to selected people")+ #if DEBUG+ .accessibilityIdentifier("copyPacking.confirm")+ #endif+ }+ }+ }+ }++ // MARK: - Subviews++ private func peopleList(eligibleIDs: Set<UUID>) -> some View {+ let variant = theme.variant(for: colorScheme)+ return List {+ Section {+ ForEach(targetPeople) { person in+ row(for: person, eligibleIDs: eligibleIDs, variant: variant)+ }+ } footer: {+ Text("Already-owned items are skipped.")+ }+ }+ .listStyle(.insetGrouped)+ }++ @ViewBuilder+ private func row(+ for person: Person,+ eligibleIDs: Set<UUID>,+ variant: ThemeVariant+ ) -> some View {+ let isEligible = eligibleIDs.contains(person.id)+ let isSelected = selected.contains(person.id)++ Button {+ toggle(person.id)+ } label: {+ HStack(spacing: 12) {+ PersonAvatar(name: person.name, colorKey: person.colorKey, size: .standard)+ Text(person.displayName)+ .foregroundStyle(isEligible ? variant.textPrimary : variant.textSecondary)+ Spacer()+ if !isEligible {+ Text("already has it")+ .font(.footnote)+ .foregroundStyle(variant.textSecondary)+ } else if isSelected {+ Image(systemName: "checkmark")+ .foregroundStyle(variant.accent)+ }+ }+ .contentShape(Rectangle())+ }+ .buttonStyle(.plain)+ .disabled(!isEligible)+ .accessibilityValue(isEligible ? "" : "already has this item")+ #if DEBUG+ .accessibilityIdentifier("copyPacking.person.\(person.name)")+ #endif+ }++ private var emptyState: some View {+ ContentUnavailableView(+ "No one else needs this",+ systemImage: "checkmark.circle",+ description: Text("Everyone else already has an item named “\(source.name)”.")+ )+ #if DEBUG+ .accessibilityIdentifier("copyPacking.emptyState")+ #endif+ }++ // MARK: - Interaction++ private func toggle(_ id: UUID) {+ if selected.contains(id) {+ selected.remove(id)+ } else {+ selected.insert(id)+ }+ }++ private func confirm() {+ // Pass the RAW selected ids; copyPacking is the sole authority that drops+ // ineligible / raced-out targets (design "Components and Interfaces").+ onCopy(Array(selected))+ }+}
diff --git a/Scramble/Scramble/Features/MasterLists/MasterPackingList.swift b/Scramble/Scramble/Features/MasterLists/MasterPackingList.swiftindex 12b0fd4..66a4c56 100644--- a/Scramble/Scramble/Features/MasterLists/MasterPackingList.swift+++ b/Scramble/Scramble/Features/MasterLists/MasterPackingList.swift@@ -1,26 +1,45 @@ import SwiftData import SwiftUI +#if canImport(UIKit)+ import UIKit+#endif+ /// AC 2.1 — list every `MasterPackingItem` grouped by `Person`, sorted by /// `Person.name` ascending, people with zero items omitted, per-person count /// in the header. AC 2.7 — when no `Person` exists, show empty state and hide /// the "+ Add item" affordance.+///+/// Copy feature: each eligible row also exposes a "Copy to people…" action via+/// a trailing swipe + long-press context menu (Decision 7 — net-new UI; the+/// whole-row tap still opens the editor). Confirming the picker runs the+/// save → engine → toast sequence in `performCopy`. @MainActor struct MasterPackingList: View { @Query(sort: \MasterPackingItem.name) private var allItems: [MasterPackingItem] @Query(sort: \Person.name) private var allPeople: [Person] @Environment(\.theme) private var theme @Environment(\.colorScheme) private var colorScheme+ @Environment(\.modelContext) private var modelContext+ @Environment(\.tripsLocalContainer) private var tripsLocalContainer+ @Environment(\.localWriteHook) private var hook - @State private var editTarget: EditTarget?+ @State private var sheetTarget: SheetTarget?+ @State private var toastMessage: String? - enum EditTarget: Identifiable, Hashable {+ enum SheetTarget: Identifiable, Hashable { case create case edit(PersistentIdentifier)+ case copy(PersistentIdentifier) var id: AnyHashable { switch self { case .create: AnyHashable("create") case .edit(let id): AnyHashable(id)+ // The "copy-" prefix is load-bearing: it keeps `.copy(x)` and `.edit(x)`+ // from resolving to the same `.sheet(item:)` identity for one item id.+ // Without it, presenting an edit sheet then a copy sheet for the same+ // item would reuse the prior identity and the copy sheet wouldn't show.+ case .copy(let id): AnyHashable("copy-\(id)") } } }@@ -44,15 +63,7 @@ import SwiftUI if !items.isEmpty { Section { ForEach(items) { item in- Button {- editTarget = .edit(item.persistentModelID)- } label: {- Text(item.name.isEmpty ? "Unnamed item" : item.name)- .foregroundStyle(variant.textPrimary)- .frame(maxWidth: .infinity, alignment: .leading)- .contentShape(Rectangle())- }- .buttonStyle(.plain)+ itemRow(item, variant: variant) } } header: { HStack {@@ -67,19 +78,71 @@ import SwiftUI Section { DashedAddButton(title: "Add item", accent: variant.accent) {- editTarget = .create+ sheetTarget = .create } } } .listStyle(.insetGrouped)- .sheet(item: $editTarget) { target in+ .sheet(item: $sheetTarget) { target in sheet(for: target) }+ // 5s (vs the app-wide 3s default) is deliberate so the post-copy+ // confirmation isn't lost while the picker sheet finishes dismissing.+ .transientToast(message: $toastMessage, duration: 5)+ }+ }++ // MARK: - Rows++ @ViewBuilder+ private func itemRow(_ item: MasterPackingItem, variant: ThemeVariant) -> some View {+ let canCopy = isCopyEligible(item)+ Button {+ sheetTarget = .edit(item.persistentModelID)+ } label: {+ Text(item.name.isEmpty ? "Unnamed item" : item.name)+ .foregroundStyle(variant.textPrimary)+ .frame(maxWidth: .infinity, alignment: .leading)+ .contentShape(Rectangle())+ }+ .buttonStyle(.plain)+ .swipeActions(edge: .trailing, allowsFullSwipe: false) {+ if canCopy {+ Button {+ sheetTarget = .copy(item.persistentModelID)+ } label: {+ Label("Copy to people…", systemImage: "person.2")+ }+ .tint(variant.accent)+ } }+ .contextMenu {+ if canCopy {+ Button {+ sheetTarget = .copy(item.persistentModelID)+ } label: {+ Label("Copy to people…", systemImage: "person.2")+ }+ }+ }+ #if DEBUG+ .accessibilityIdentifier("masterPacking.itemRow.\(item.name)")+ #endif+ }++ /// Source eligibility for the copy action (Req 1.3 / Decision 6): at least+ /// two people exist, the source has an owner, and its trimmed name is+ /// non-empty.+ private func isCopyEligible(_ item: MasterPackingItem) -> Bool {+ guard allPeople.count >= 2 else { return false }+ guard item.person != nil else { return false }+ return !item.name.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty } + // MARK: - Sheet+ @ViewBuilder- private func sheet(for target: EditTarget) -> some View {+ private func sheet(for target: SheetTarget) -> some View { switch target { case .create: MasterPackingEditor(mode: .create)@@ -87,6 +150,112 @@ import SwiftUI if let item = allItems.first(where: { $0.persistentModelID == id }) { MasterPackingEditor(mode: .edit(item)) }+ case .copy(let id):+ // An unresolved `.copy` id (source deleted / owner changed between row+ // tap and presentation) renders nothing; dismissing writes nothing.+ if let item = allItems.first(where: { $0.persistentModelID == id }) {+ CopyPackingItemSheet(+ source: item,+ onCopy: { ids in+ sheetTarget = nil+ performCopy(source: item, toPersonIDs: ids)+ },+ onCancel: { sheetTarget = nil }+ )+ } } }++ // MARK: - Copy++ /// Branch selection of the post-`copyPacking` orchestration, extracted as a+ /// pure function so it is unit-testable without SwiftData. It owns ONLY the+ /// control flow; `performCopy` supplies the real save / run-engine closures+ /// and maps the outcome onto the rollback + toast side effects.+ enum CopyOutcome: Equatable {+ case nothingToCopy+ case saveFailed+ case copied(deferred: Bool)+ }++ /// Decides the outcome of a copy:+ /// - `createdCount == 0` → `.nothingToCopy` — calls NEITHER `save` nor+ /// `runEngine`.+ /// - else `try save()`; on throw → `.saveFailed` — does NOT call `runEngine`.+ /// - else `try runEngine()`; on throw → `.copied(deferred: true)`.+ /// - else → `.copied(deferred: false)`.+ static func copyOutcome(+ createdCount: Int,+ save: () throws -> Void,+ runEngine: () throws -> Void+ ) -> CopyOutcome {+ guard createdCount > 0 else { return .nothingToCopy }+ do {+ try save()+ } catch {+ return .saveFailed+ }+ do {+ try runEngine()+ } catch {+ return .copied(deferred: true)+ }+ return .copied(deferred: false)+ }++ /// save → engine → toast sequence (design Flow). `copyPacking` only inserts;+ /// this method owns `save()`, the recompute, and the user-facing toast — the+ /// list stays mounted while the picker dismisses, so the toast renders here.+ /// The branch selection is delegated to the pure `copyOutcome(...)`; this+ /// method maps the outcome onto rollback + the existing toast wording.+ private func performCopy(source: MasterPackingItem, toPersonIDs ids: [UUID]) {+ let result = MasterPersistence.copyPacking(+ source: source,+ toPersonIDs: ids,+ in: modelContext+ )++ // Materialise onto matching non-past trips (4.1). Best-effort: a top-level+ // recompute failure keeps the saved masters and surfaces the deferred-+ // update wording (4.2), mirroring MasterPackingEditor.runEngineAndDismiss.+ let runner = RulesEngineRunner(+ context: tripsLocalContainer.mainContext,+ mastersContext: modelContext,+ hook: hook+ )++ let outcome = Self.copyOutcome(+ createdCount: result.createdCount,+ save: { try modelContext.save() },+ runEngine: { _ = try runner.runForAllNonPastTrips() }+ )++ switch outcome {+ case .saveFailed:+ modelContext.rollback()+ announce("Copy failed — try again.")+ case .copied(deferred: true):+ announce("Copied. Some trips couldn't be updated — they'll sync on next launch.")+ case .nothingToCopy, .copied(deferred: false):+ // All targets skipped at confirm (3.7) reports everyone already had it;+ // a clean copy reports who received it — both via copyToastMessage.+ announce(toastMessage(for: result))+ }+ }++ private func toastMessage(for result: CopyResult) -> String {+ MasterPersistence.copyToastMessage(+ copiedNames: result.copiedNames,+ skippedNames: result.skippedNames+ )+ }++ /// Shows the toast and posts a VoiceOver announcement, matching+ /// `PackingItemRow`'s move-announcement pattern.+ private func announce(_ message: String) {+ toastMessage = message+ #if canImport(UIKit)+ UIAccessibility.post(notification: .announcement, argument: message)+ #endif+ } }
diff --git a/Scramble/Scramble/Features/MasterLists/MasterPersistence.swift b/Scramble/Scramble/Features/MasterLists/MasterPersistence.swiftindex db20707..223a51f 100644--- a/Scramble/Scramble/Features/MasterLists/MasterPersistence.swift+++ b/Scramble/Scramble/Features/MasterLists/MasterPersistence.swift@@ -2,6 +2,19 @@ import Foundation import SwiftData import os +/// Outcome of `MasterPersistence.copyPacking`. Value-only — it carries no+/// `@Model` instances out of the helper, so it crosses isolation boundaries+/// freely and feeds `copyToastMessage` from any context.+nonisolated struct CopyResult: Sendable {+ /// Copies inserted into the context, NOT yet saved.+ var createdCount: Int+ /// Names of the target people that received a copy.+ var copiedNames: [String]+ /// Names of the target people skipped because they already owned a+ /// same-name item.+ var skippedNames: [String]+}+ /// Helpers for applying master drafts to the model context. Mirrors /// `TripPersistence`: the helpers mutate but do NOT call `context.save()` — /// the master editor closure owns the mutate → save → run-engine sequence@@ -69,6 +82,98 @@ import os context.delete(item) } + // MARK: - Copy master packing to people++ /// Trimmed + case-insensitive key for same-name detection (Req 2.3 / 3.5).+ /// `nonisolated` because `MasterPersistence` is a `@MainActor` enum but this+ /// is a pure `String` derivation safe to call from any context (and from+ /// unit tests without `await`).+ nonisolated static func normalizedName(_ raw: String) -> String {+ raw.trimmingCharacters(in: .whitespacesAndNewlines).lowercased()+ }++ /// Builds the 5.1 / 5.2 / 3.7 confirmation strings from the plain name+ /// arrays (not the `@Model`-bearing `CopyResult`) so it stays `nonisolated`+ /// and unit-testable. Three outcomes:+ /// - copied-only: lists the people who received the item;+ /// - copied-with-skips: lists copied people and notes who was skipped;+ /// - all-skipped (empty `copiedNames`): states everyone already had it.+ nonisolated static func copyToastMessage(+ copiedNames: [String],+ skippedNames: [String]+ ) -> String {+ let copied = formatNames(copiedNames)+ let skipped = formatNames(skippedNames)++ if copiedNames.isEmpty {+ return "Everyone already had this item — skipped \(skipped)."+ }+ if skippedNames.isEmpty {+ return "Copied to \(copied)."+ }+ return "Copied to \(copied). Skipped \(skipped) — already had it."+ }++ /// Inserts one copy per target person that does not already own a same-name+ /// item. De-duplicates `toPersonIDs` (it is the sole authority on what gets+ /// created). Does NOT call `save()` — the caller owns the+ /// mutate → save → run-engine sequence, per the `MasterPersistence`+ /// convention. The same-name check covers Req 3.5 (a target that became a+ /// same-name owner since the picker opened).+ @discardableResult+ static func copyPacking(+ source: MasterPackingItem,+ toPersonIDs: [UUID],+ in context: ModelContext+ ) -> CopyResult {+ let trimmedName = source.name.trimmingCharacters(in: .whitespacesAndNewlines)+ let sourceConditions = source.conditions+ let sourceKey = normalizedName(trimmedName)++ var result = CopyResult(createdCount: 0, copiedNames: [], skippedNames: [])+ var seen = Set<UUID>()++ for id in toPersonIDs {+ guard seen.insert(id).inserted else { continue }+ guard let target = resolvePerson(id: id, in: context) else { continue }++ let alreadyOwns = (target.masterPackingItems ?? []).contains {+ normalizedName($0.name) == sourceKey+ }+ if alreadyOwns {+ result.skippedNames.append(target.name)+ continue+ }++ let copy = MasterPackingItem(+ name: trimmedName,+ person: target,+ conditions: sourceConditions+ )+ context.insert(copy)+ result.createdCount += 1+ result.copiedNames.append(target.name)+ }++ return result+ }++ /// Joins names with commas and a trailing "and" for the final element so the+ /// toast reads naturally for one, two, or many people.+ private nonisolated static func formatNames(_ names: [String]) -> String {+ switch names.count {+ case 0:+ return ""+ case 1:+ return names[0]+ case 2:+ return "\(names[0]) and \(names[1])"+ default:+ let head = names.dropLast().joined(separator: ", ")+ return "\(head), and \(names[names.count - 1])"+ }+ }+ // MARK: - Internal private static func resolvePerson(id: UUID?, in context: ModelContext) -> Person? {
diff --git a/Scramble/Scramble/Models/Person.swift b/Scramble/Scramble/Models/Person.swiftindex a2b637b..b2b7c88 100644--- a/Scramble/Scramble/Models/Person.swift+++ b/Scramble/Scramble/Models/Person.swift@@ -29,6 +29,12 @@ final class Person { extension Person { var initial: String { name.firstGraphemeUppercased } + /// Display-safe name: the person's `name`, or `"Unnamed"` when it is empty.+ /// `nonisolated` to match the other pure derivations in this extension (the+ /// file inherits `MainActor` isolation but this is a pure `String`+ /// derivation).+ nonisolated var displayName: String { name.isEmpty ? "Unnamed" : name }+ /// Short-form name derivation per Phase 4 design §"Short-form name /// derivation". Trims whitespace, returns the first space-separated token, /// or the full trimmed name when there is no space, or `"?"` when the name
diff --git a/Scramble/Scramble/Persistence/UITestSeed.swift b/Scramble/Scramble/Persistence/UITestSeed.swiftindex ecc5667..98530af 100644--- a/Scramble/Scramble/Persistence/UITestSeed.swift+++ b/Scramble/Scramble/Persistence/UITestSeed.swift@@ -17,6 +17,10 @@ case twoQualifyingTrips = "two-qualifying-trips" case singleEditableTrip = "single-editable-trip" case masterListsEmptyWithPerson = "master-lists-empty-with-person"+ /// Copy feature: two people ("Alex" owns a "Socks" master packing item,+ /// "Sam" owns nothing) so the per-row "Copy to people…" action is eligible+ /// (>= 2 people) and Sam is an eligible copy target.+ case masterPackingCopyTwoPeople = "master-packing-copy-two-people" case masterTaskAdvancedConditions = "master-task-advanced-conditions" case phase2RulesFixture = "phase2-rules-fixture" case phase2RulesFixtureSunTrip = "phase2-rules-fixture-sun-trip"@@ -152,6 +156,7 @@ let end = calendar.date(byAdding: .day, value: 35, to: day) ?? day context.insert(Trip(name: "Sample Trip", startDate: start, endDate: end)) case .masterListsEmptyWithPerson,+ .masterPackingCopyTwoPeople, .masterTaskAdvancedConditions, .phase2RulesFixture, .phase2RulesFixtureSunTrip,@@ -194,6 +199,16 @@ context.insert( MasterPackingItem(name: "Seeded item", person: alex, conditions: .always) )+ case .masterPackingCopyTwoPeople:+ // Two people so the copy action is eligible (Req 1.3 needs >= 2).+ // Alex owns "Socks"; Sam owns nothing → Sam is an eligible target.+ let alex = Person(name: "Alex", colorKey: "blue")+ let sam = Person(name: "Sam", colorKey: "red")+ context.insert(alex)+ context.insert(sam)+ context.insert(+ MasterPackingItem(name: "Socks", person: alex, conditions: .always)+ ) case .masterTaskAdvancedConditions: // Out-of-domain weather value forces AdvancedConditionView per AC 3.7a. context.insert(Person(name: "Alex", colorKey: "blue"))
diff --git a/Scramble/ScrambleTests/MasterLists/CopyPackingTests.swift b/Scramble/ScrambleTests/MasterLists/CopyPackingTests.swiftnew file mode 100644index 0000000..b338e22--- /dev/null+++ b/Scramble/ScrambleTests/MasterLists/CopyPackingTests.swift@@ -0,0 +1,609 @@+import Foundation+import SwiftData+import Testing++@testable import Scramble++// MARK: - Pure helpers (Task 1)++@Suite("MasterPersistence.normalizedName")+struct NormalizedNameTests {++ @Test("Trims surrounding whitespace and lowercases")+ func trimsAndLowercases() {+ #expect(MasterPersistence.normalizedName(" Socks ") == "socks")+ #expect(MasterPersistence.normalizedName("SOCKS") == "socks")+ #expect(MasterPersistence.normalizedName("Socks") == "socks")+ }++ @Test("' Socks ' and 'socks' normalise equal")+ func trimmedVariantsMatch() {+ #expect(+ MasterPersistence.normalizedName(" Socks ")+ == MasterPersistence.normalizedName("socks")+ )+ }++ @Test("Empty and whitespace-only normalise to empty string")+ func emptyAndWhitespaceMapToEmpty() {+ #expect(MasterPersistence.normalizedName("") == "")+ #expect(MasterPersistence.normalizedName(" ") == "")+ #expect(MasterPersistence.normalizedName("\n\t ") == "")+ }+}++@Suite("MasterPersistence.copyToastMessage")+struct CopyToastMessageTests {++ // The three branches share name fragments ("Alice", "Bob"), so a bare+ // `contains(name)` check would still pass if two branches were transposed.+ // Each assertion below pins a DISTINGUISHING phrase from the source string so+ // swapping the copied-only / copied-with-skips / all-skipped arms fails a test.+ // Source phrasing (MasterPersistence.copyToastMessage):+ // copied-only: "Copied to \(copied)."+ // copied-with-skips: "Copied to \(copied). Skipped \(skipped) — already had it."+ // all-skipped: "Everyone already had this item — skipped \(skipped)."+ private static let skippedClause = "already had it"+ private static let copiedClause = "Copied to"+ private static let allSkippedLead = "Everyone already had this item"++ @Test("Copied-only: copied-clause present, skipped-clause absent")+ func copiedOnly() {+ let message = MasterPersistence.copyToastMessage(+ copiedNames: ["Alice", "Bob"],+ skippedNames: []+ )+ #expect(message.contains("Alice"))+ #expect(message.contains("Bob"))+ // Distinguishes from copied-with-skips: it copies, it does NOT skip anyone.+ #expect(message.contains(Self.copiedClause))+ #expect(message.contains(Self.skippedClause) == false)+ #expect(message.contains("Skipped") == false)+ #expect(message.contains(Self.allSkippedLead) == false)+ }++ @Test("Copied-with-skips: BOTH the copied-clause and the skipped-clause present")+ func copiedWithSkips() {+ let message = MasterPersistence.copyToastMessage(+ copiedNames: ["Alice"],+ skippedNames: ["Bob"]+ )+ #expect(message.contains("Alice"))+ #expect(message.contains("Bob"))+ // Distinguishes from BOTH other branches: copies AND notes a skip.+ #expect(message.contains(Self.copiedClause))+ #expect(message.contains(Self.skippedClause))+ // Not the all-skipped branch, which would lead with "Everyone…".+ #expect(message.contains(Self.allSkippedLead) == false)+ }++ @Test("All-skipped: 'already had it' wording, no 'Copied to' clause")+ func allSkipped() {+ let message = MasterPersistence.copyToastMessage(+ copiedNames: [],+ skippedNames: ["Alice", "Bob"]+ )+ #expect(message.contains("Alice"))+ #expect(message.contains("Bob"))+ // Distinguishes from the two copied branches: nobody received a copy, so the+ // "Copied to" clause must NOT leak through. The all-skipped string leads with+ // "Everyone already had this item — skipped …" and uses the verb "skipped";+ // it does NOT carry the copied-with-skips-only trailing "— already had it"+ // clause, so asserting that clause is ABSENT here guards against transposing+ // these two skip-bearing branches.+ #expect(message.contains(Self.allSkippedLead))+ #expect(message.contains("skipped"))+ #expect(message.contains(Self.skippedClause) == false)+ #expect(message.contains(Self.copiedClause) == false)+ }++ // Exercises the formatNames joiner indirectly through copyToastMessage (the+ // helper is private). Single name → bare name; two → "X and Y"; three+ →+ // oxford "A, B, and C". Asserted against the copied-only branch so the joined+ // string is the whole list of names.+ @Test("formatNames join: 1 name renders the bare name")+ func formatNamesSingle() {+ let message = MasterPersistence.copyToastMessage(+ copiedNames: ["Alice"],+ skippedNames: []+ )+ #expect(message.contains("Copied to Alice."))+ // No join words for a single name.+ #expect(message.contains(" and ") == false)+ #expect(message.contains(",") == false)+ }++ @Test("formatNames join: 2 names use the 'X and Y' join, no comma")+ func formatNamesPair() {+ let message = MasterPersistence.copyToastMessage(+ copiedNames: ["Alice", "Bob"],+ skippedNames: []+ )+ #expect(message.contains("Copied to Alice and Bob."))+ // Two names join with " and " and NO comma (distinguishes from 3+).+ #expect(message.contains(", and ") == false)+ }++ @Test("formatNames join: 3+ names use the oxford 'A, B, and C' join")+ func formatNamesOxford() {+ let message = MasterPersistence.copyToastMessage(+ copiedNames: ["Alice", "Bob", "Carol"],+ skippedNames: []+ )+ #expect(message.contains("Copied to Alice, Bob, and Carol."))+ }+}++// MARK: - copyPacking (Task 3)++@Suite("MasterPersistence.copyPacking", .serialized)+@MainActor+struct CopyPackingHelperTests {++ private static func makeContainer() throws -> ModelContainer {+ let schema = Schema(versionedSchema: SchemaV3.self)+ let config = ModelConfiguration(+ schema: schema,+ isStoredInMemoryOnly: true,+ cloudKitDatabase: .none+ )+ return try ModelContainer(for: schema, configurations: [config])+ }++ @Test("One copy per eligible target, each owned by the right person; trimmed name (3.1/3.2)")+ func createsOneCopyPerEligibleTarget() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ let bob = Person(name: "Bob", colorKey: "orange")+ context.insert(owner)+ context.insert(alice)+ context.insert(bob)+ let source = MasterPackingItem(name: " Socks ", person: owner, conditions: .always)+ context.insert(source)+ try context.save()++ let result = MasterPersistence.copyPacking(+ source: source,+ toPersonIDs: [alice.id, bob.id],+ in: context+ )++ #expect(result.createdCount == 2)+ #expect(Set(result.copiedNames) == ["Alice", "Bob"])+ #expect(result.skippedNames.isEmpty)++ let copies = try context.fetch(FetchDescriptor<MasterPackingItem>())+ .filter { $0.id != source.id }+ #expect(copies.count == 2)+ // Names are trimmed copies of the source (3.2).+ #expect(copies.allSatisfy { $0.name == "Socks" })+ // Each copy is owned by exactly one of the targets (3.1).+ let aliceCopy = try #require(copies.first { $0.person?.id == alice.id })+ let bobCopy = try #require(copies.first { $0.person?.id == bob.id })+ #expect(aliceCopy.name == "Socks")+ #expect(bobCopy.name == "Socks")+ }++ @Test("Skips a target already owning a same-name item; trimmed + case-insensitive (2.3/3.5)")+ func skipsSameNameOwnerCaseInsensitive() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ context.insert(owner)+ context.insert(alice)+ // Alice already owns "socks" — source is " Socks " → normalised equal.+ let aliceExisting = MasterPackingItem(name: "socks", person: alice, conditions: .always)+ context.insert(aliceExisting)+ let source = MasterPackingItem(name: " Socks ", person: owner, conditions: .always)+ context.insert(source)+ try context.save()++ let result = MasterPersistence.copyPacking(+ source: source,+ toPersonIDs: [alice.id],+ in: context+ )++ #expect(result.createdCount == 0)+ #expect(result.copiedNames.isEmpty)+ #expect(result.skippedNames == ["Alice"])+ // No new item created for Alice.+ let aliceItems = (alice.masterPackingItems ?? [])+ #expect(aliceItems.count == 1)+ }++ @Test("All targets already own it → createdCount == 0 and nothing inserted (3.7)")+ func allTargetsSkippedInsertsNothing() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ let bob = Person(name: "Bob", colorKey: "orange")+ context.insert(owner)+ context.insert(alice)+ context.insert(bob)+ context.insert(MasterPackingItem(name: "Socks", person: alice, conditions: .always))+ context.insert(MasterPackingItem(name: "Socks", person: bob, conditions: .always))+ let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+ context.insert(source)+ try context.save()++ let before = try context.fetch(FetchDescriptor<MasterPackingItem>()).count+ let result = MasterPersistence.copyPacking(+ source: source,+ toPersonIDs: [alice.id, bob.id],+ in: context+ )++ #expect(result.createdCount == 0)+ #expect(result.copiedNames.isEmpty)+ #expect(Set(result.skippedNames) == ["Alice", "Bob"])+ let after = try context.fetch(FetchDescriptor<MasterPackingItem>()).count+ #expect(after == before)+ }++ @Test("Duplicate ids in toPersonIDs produce exactly one copy (de-dupe)")+ func deDupesRepeatedIDs() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ context.insert(owner)+ context.insert(alice)+ let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+ context.insert(source)+ try context.save()++ let result = MasterPersistence.copyPacking(+ source: source,+ toPersonIDs: [alice.id, alice.id, alice.id],+ in: context+ )++ #expect(result.createdCount == 1)+ #expect(result.copiedNames == ["Alice"])+ #expect((alice.masterPackingItems ?? []).count == 1)+ }++ @Test("Skips a same-name owner whose item was inserted but NOT saved (3.5 race)")+ func skipsUnsavedSameNameInsert() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ context.insert(owner)+ context.insert(alice)+ let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+ context.insert(source)+ try context.save()++ // The load-bearing race (Req 3.5): a same-name item lands for the target+ // AFTER the picker opened but BEFORE the copy runs — and crucially before+ // any save(). The skip re-check reads `Person.masterPackingItems`, so it+ // must surface this pending in-context insert. We deliberately do NOT call+ // context.save() here.+ let aliceUnsaved = MasterPackingItem(name: "Socks", person: alice, conditions: .always)+ context.insert(aliceUnsaved)+ #expect(context.hasChanges == true)++ let result = MasterPersistence.copyPacking(+ source: source,+ toPersonIDs: [alice.id],+ in: context+ )++ // If this fails — i.e. the unsaved insert is invisible through the+ // relationship and the helper creates a duplicate — that is a real defect,+ // NOT a test to relax.+ #expect(result.createdCount == 0)+ #expect(result.copiedNames.isEmpty)+ #expect(result.skippedNames == ["Alice"])+ // Only the one pending insert exists for Alice; no copy was added.+ #expect((alice.masterPackingItems ?? []).count == 1)+ }++ @Test("Unresolvable person id is neither copied nor skipped (off-roster contract)")+ func unresolvableIDIsNeitherCopiedNorSkipped() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ context.insert(owner)+ let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+ context.insert(source)+ try context.save()++ // A random id resolves to no Person in the store.+ let ghost = UUID()+ let result = MasterPersistence.copyPacking(+ source: source,+ toPersonIDs: [ghost],+ in: context+ )++ // An unresolved id is dropped silently: not a copy, and NOT a skip (a skip+ // means "a real person already had it"). Pin that asymmetry.+ #expect(result.createdCount == 0)+ #expect(result.copiedNames.isEmpty)+ #expect(result.skippedNames.isEmpty)+ }++ @Test("Pure mechanism: does NOT guard an empty/whitespace source name (Decision 6)")+ func doesNotGuardEmptySourceName() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ context.insert(owner)+ context.insert(alice)+ // Source name trims to "" — the UI source-eligibility gate (Decision 6 /+ // Req 1.3) blocks this upstream; the helper itself does NOT, so it copies an+ // empty-named item. This test documents the helper's actual behaviour, not a+ // guard we want it to grow.+ let source = MasterPackingItem(name: " ", person: owner, conditions: .always)+ context.insert(source)+ try context.save()++ let result = MasterPersistence.copyPacking(+ source: source,+ toPersonIDs: [alice.id],+ in: context+ )++ // The helper is a pure mechanism: it creates a copy with the trimmed+ // (empty) name and reports Alice as copied.+ #expect(result.createdCount == 1)+ #expect(result.copiedNames == ["Alice"])+ #expect(result.skippedNames.isEmpty)+ let copy = try #require(+ try context.fetch(FetchDescriptor<MasterPackingItem>())+ .first { $0.person?.id == alice.id }+ )+ #expect(copy.name == "")+ }++ @Test("Conditions fidelity: advanced nested form copied by value; source unchanged (3.3/3.4)")+ func conditionsFidelityAndIndependence() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ context.insert(owner)+ context.insert(alice)+ let nested: ItemConditions = .all([+ .any([+ .match(attribute: .weather, anyOf: ["rain", "snow"]),+ .match(attribute: .weather, anyOf: ["wind"]),+ ]),+ .match(attribute: .purpose, anyOf: ["hiking"]),+ ])+ let source = MasterPackingItem(name: "Layers", person: owner, conditions: nested)+ context.insert(source)+ try context.save()++ MasterPersistence.copyPacking(source: source, toPersonIDs: [alice.id], in: context)++ let copy = try #require(+ try context.fetch(FetchDescriptor<MasterPackingItem>())+ .first { $0.person?.id == alice.id }+ )+ // Decoded value equality, NOT conditionsData byte-equality.+ #expect(copy.conditions == source.conditions)+ #expect(copy.conditions == nested)++ // Mutating the copy's conditions leaves the source untouched (3.4).+ copy.conditions = .always+ #expect(source.conditions == nested)+ #expect(copy.conditions == .always)+ }+}++// MARK: - copyOutcome branch selection (pure orchestration)++/// Drives `MasterPackingList.copyOutcome`, the pure branch-selection extracted+/// from `performCopy`. These tests assert the control flow ONLY — which closures+/// run and the returned `CopyOutcome` — using recording flags, so they need no+/// SwiftData. The toast wording and rollback side effects stay in `performCopy`.+@Suite("MasterPackingList.copyOutcome")+@MainActor+struct CopyOutcomeTests {++ struct CopyError: Error {}++ @Test("createdCount 0 → .nothingToCopy; neither save nor runEngine runs (3.7)")+ func nothingToCopySkipsSaveAndEngine() {+ var didSave = false+ var didRun = false+ let outcome = MasterPackingList.copyOutcome(+ createdCount: 0,+ save: { didSave = true },+ runEngine: { didRun = true }+ )+ #expect(outcome == .nothingToCopy)+ #expect(didSave == false)+ #expect(didRun == false)+ }++ @Test("save throws → .saveFailed; runEngine is NOT invoked (3.6)")+ func saveFailureSkipsEngine() {+ var didRun = false+ let outcome = MasterPackingList.copyOutcome(+ createdCount: 2,+ save: { throw CopyError() },+ runEngine: { didRun = true }+ )+ #expect(outcome == .saveFailed)+ #expect(didRun == false)+ }++ @Test("save ok + runEngine throws → .copied(deferred: true) (4.2)")+ func engineFailureIsDeferred() {+ var didSave = false+ let outcome = MasterPackingList.copyOutcome(+ createdCount: 2,+ save: { didSave = true },+ runEngine: { throw CopyError() }+ )+ #expect(outcome == .copied(deferred: true))+ #expect(didSave == true)+ }++ @Test("save ok + runEngine ok → .copied(deferred: false)")+ func cleanCopyIsNotDeferred() {+ var didSave = false+ var didRun = false+ let outcome = MasterPackingList.copyOutcome(+ createdCount: 2,+ save: { didSave = true },+ runEngine: { didRun = true }+ )+ #expect(outcome == .copied(deferred: false))+ #expect(didSave == true)+ #expect(didRun == true)+ }+}++// MARK: - Engine integration + save atomicity (Task 5)++@Suite("Copy sequence: materialisation + atomicity", .serialized)+@MainActor+struct CopySequenceTests {++ private static func makeContainer() throws -> ModelContainer {+ let schema = Schema(versionedSchema: SchemaV3.self)+ let config = ModelConfiguration(+ schema: schema,+ isStoredInMemoryOnly: true,+ cloudKitDatabase: .none+ )+ return try ModelContainer(for: schema, configurations: [config])+ }++ private static func rainyAttributes() -> TripAttributes {+ var a = TripAttributes()+ a.toggle(.weather, value: "rain")+ return a+ }++ private static func sunnyAttributes() -> TripAttributes {+ var a = TripAttributes()+ a.toggle(.weather, value: "sun")+ return a+ }++ /// Seeds a trip with the given attributes plus a `TripPersonSnapshot` for the+ /// target person, which Phase 5.1's engine requires to materialise a+ /// rule-driven `TripPackingItem` (an orphan otherwise).+ private static func seedTrip(+ name: String,+ attributes: TripAttributes,+ person: Person,+ in context: ModelContext+ ) -> Trip {+ let trip = Trip(name: name, startDate: .now, endDate: .now, attributes: attributes)+ context.insert(trip)+ let snapshot = TripPersonSnapshot(+ personID: person.id,+ name: person.name,+ colourID: person.colorKey,+ initialSource: "name",+ isRosterMember: true,+ trip: trip+ )+ context.insert(snapshot)+ return trip+ }++ @Test("Materialisation: copy → save → engine puts item only on the matching trip (4.1)")+ func materialisesOntoMatchingTripOnly() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ context.insert(owner)+ context.insert(alice)+ // Source matches rainy trips only.+ let source = MasterPackingItem(+ name: "Rain jacket",+ person: owner,+ conditions: .match(attribute: .weather, anyOf: ["rain"])+ )+ context.insert(source)++ let rainy = Self.seedTrip(+ name: "Rainy", attributes: Self.rainyAttributes(), person: alice, in: context)+ let sunny = Self.seedTrip(+ name: "Sunny", attributes: Self.sunnyAttributes(), person: alice, in: context)+ try context.save()++ let result = MasterPersistence.copyPacking(+ source: source, toPersonIDs: [alice.id], in: context)+ #expect(result.createdCount == 1)+ try context.save()++ let hook = LocalWriteHook(notifier: RecordingNotifier())+ let runner = RulesEngineRunner(context: context, mastersContext: context, hook: hook)+ _ = try runner.runForAllNonPastTrips()++ let packs = try context.fetch(FetchDescriptor<TripPackingItem>())+ let aliceOnRainy = packs.filter {+ $0.trip?.id == rainy.id && $0.personSnapshot?.personID == alice.id+ }+ let aliceOnSunny = packs.filter {+ $0.trip?.id == sunny.id && $0.personSnapshot?.personID == alice.id+ }+ #expect(aliceOnRainy.count == 1)+ #expect(aliceOnRainy.first?.name == "Rain jacket")+ #expect(aliceOnSunny.isEmpty)+ }++ @Test("Atomicity: copyPacking inserts but does not save; rollback persists zero copies (3.6)")+ func rollbackLeavesNoCopiesAndNoEngineRun() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ let bob = Person(name: "Bob", colorKey: "orange")+ context.insert(owner)+ context.insert(alice)+ context.insert(bob)+ let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+ context.insert(source)+ try context.save()++ let baseline = try context.fetch(FetchDescriptor<MasterPackingItem>()).count++ let result = MasterPersistence.copyPacking(+ source: source, toPersonIDs: [alice.id, bob.id], in: context)+ #expect(result.createdCount == 2)+ // Inserted but not saved — the helper never calls save().+ #expect(context.hasChanges == true)++ // Simulate the caller's save-failure path: drop all uncommitted inserts.+ context.rollback()+ #expect(context.hasChanges == false)++ let after = try context.fetch(FetchDescriptor<MasterPackingItem>()).count+ #expect(after == baseline)++ // The engine is never invoked on the failure path — assert no trip-level+ // items exist (no recompute ran). (A notifier assertion was removed here: an+ // unwired LocalWriteHook can never record a call, so it was vacuous —+ // copyPacking touches globals, not a trip zone, and never goes through the+ // hook; only the engine run would, and the engine never runs on rollback.)+ let packs = try context.fetch(FetchDescriptor<TripPackingItem>())+ #expect(packs.isEmpty)+ }+}
diff --git a/Scramble/ScrambleTests/MasterLists/CopyPackingItemSheetEligibilityTests.swift b/Scramble/ScrambleTests/MasterLists/CopyPackingItemSheetEligibilityTests.swiftnew file mode 100644index 0000000..db00ae2--- /dev/null+++ b/Scramble/ScrambleTests/MasterLists/CopyPackingItemSheetEligibilityTests.swift@@ -0,0 +1,104 @@+import Foundation+import SwiftData+import Testing++@testable import Scramble++/// Task 6 — drives `CopyPackingItemSheet.eligibleTargets(source:people:)`, the+/// static helper that resolves the eligible target people for the picker. It+/// excludes the source owner (2.1) and any person who already owns a same-name+/// item (2.3), using `MasterPersistence.normalizedName` as the SAME comparator+/// `copyPacking` applies. When every other person is ineligible the set is+/// empty, which drives the picker's 2.5 empty-state.+@Suite("CopyPackingItemSheet.eligibleTargets", .serialized)+@MainActor+struct CopyPackingItemSheetEligibilityTests {++ private static func makeContainer() throws -> ModelContainer {+ let schema = Schema(versionedSchema: SchemaV3.self)+ let config = ModelConfiguration(+ schema: schema,+ isStoredInMemoryOnly: true,+ cloudKitDatabase: .none+ )+ return try ModelContainer(for: schema, configurations: [config])+ }++ @Test("Source owner is excluded from the eligible set (2.1)")+ func ownerExcluded() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ let bob = Person(name: "Bob", colorKey: "orange")+ context.insert(owner)+ context.insert(alice)+ context.insert(bob)+ let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+ context.insert(source)+ try context.save()++ let eligible = CopyPackingItemSheet.eligibleTargets(+ source: source,+ people: [owner, alice, bob]+ )++ let ids = Set(eligible.map(\.id))+ #expect(ids == [alice.id, bob.id])+ #expect(ids.contains(owner.id) == false)+ }++ @Test("A person already owning a same-name item is ineligible; trimmed + case-insensitive (2.3)")+ func sameNameOwnerIneligible() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ let bob = Person(name: "Bob", colorKey: "orange")+ context.insert(owner)+ context.insert(alice)+ context.insert(bob)+ // Alice already owns "socks" — source is " Socks " → normalises equal.+ context.insert(MasterPackingItem(name: "socks", person: alice, conditions: .always))+ let source = MasterPackingItem(name: " Socks ", person: owner, conditions: .always)+ context.insert(source)+ try context.save()++ let eligible = CopyPackingItemSheet.eligibleTargets(+ source: source,+ people: [owner, alice, bob]+ )++ let ids = Set(eligible.map(\.id))+ // Alice is dropped (same-name owner); Bob remains; owner is always excluded.+ #expect(ids == [bob.id])+ #expect(ids.contains(alice.id) == false)+ }++ @Test("When every other person is ineligible the eligible set is empty (drives 2.5)")+ func everyoneIneligibleYieldsEmpty() throws {+ let container = try Self.makeContainer()+ let context = container.mainContext++ let owner = Person(name: "Owner", colorKey: "blue")+ let alice = Person(name: "Alice", colorKey: "cyan")+ let bob = Person(name: "Bob", colorKey: "orange")+ context.insert(owner)+ context.insert(alice)+ context.insert(bob)+ context.insert(MasterPackingItem(name: "Socks", person: alice, conditions: .always))+ context.insert(MasterPackingItem(name: "SOCKS", person: bob, conditions: .always))+ let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+ context.insert(source)+ try context.save()++ let eligible = CopyPackingItemSheet.eligibleTargets(+ source: source,+ people: [owner, alice, bob]+ )++ #expect(eligible.isEmpty)+ }+}
diff --git a/Scramble/ScrambleUITests/CopyMasterPackingItemUITests.swift b/Scramble/ScrambleUITests/CopyMasterPackingItemUITests.swiftnew file mode 100644index 0000000..800a78d--- /dev/null+++ b/Scramble/ScrambleUITests/CopyMasterPackingItemUITests.swift@@ -0,0 +1,82 @@+import XCTest++/// Task 9 — happy-path coverage for the "Copy to people…" flow on the Master+/// Lists packing list. Seeds two people (Alex owns "Socks"; Sam owns nothing),+/// reveals the per-row copy action, selects Sam, confirms, and asserts the+/// sheet dismisses and the confirmation toast names Sam (Req 1.1 / 1.2 / 5.1).+/// Driven entirely through the `accessibilityIdentifier`s added in Tasks 7/8.+final class CopyMasterPackingItemUITests: XCTestCase {++ override func setUpWithError() throws {+ continueAfterFailure = false+ }++ @MainActor+ private func launchedApp(fixture: String) -> XCUIApplication {+ let app = XCUIApplication()+ app.launchArguments = ["-uitest", "1", "-seed-fixture", fixture]+ app.launch()++ let marker = app.descendants(matching: .any)+ .matching(identifier: "modelStore.in-memory")+ .firstMatch+ XCTAssertTrue(marker.waitForExistence(timeout: 5))+ return app+ }++ @MainActor+ private func openMasterLists(_ app: XCUIApplication) {+ let tab = app.tabBars.buttons["Master Lists"]+ XCTAssertTrue(tab.waitForExistence(timeout: 3))+ tab.tap()+ XCTAssertTrue(app.navigationBars["Master Lists"].waitForExistence(timeout: 3))+ }++ @MainActor+ func testCopyToPersonShowsConfirmationToast() {+ let app = launchedApp(fixture: "master-packing-copy-two-people")+ openMasterLists(app)+ // Packing Items is the default segment.++ let row = app.descendants(matching: .any)+ .matching(identifier: "masterPacking.itemRow.Socks")+ .firstMatch+ XCTAssertTrue(row.waitForExistence(timeout: 3), "Seeded 'Socks' row should be visible")++ // Reveal the trailing swipe action and tap "Copy to people…".+ row.swipeLeft()+ let copyAction = app.buttons["Copy to people…"]+ XCTAssertTrue(+ copyAction.waitForExistence(timeout: 3),+ "Trailing swipe should reveal the 'Copy to people…' action"+ )+ copyAction.tap()++ // The picker presents Sam as an eligible target (Alex is the owner).+ let samRow = app.descendants(matching: .any)+ .matching(identifier: "copyPacking.person.Sam")+ .firstMatch+ XCTAssertTrue(samRow.waitForExistence(timeout: 3), "Sam should be an eligible copy target")+ samRow.tap()++ let confirm = app.buttons["copyPacking.confirm"]+ XCTAssertTrue(confirm.waitForExistence(timeout: 3))+ XCTAssertTrue(confirm.isEnabled, "Confirm should enable once Sam is selected")+ confirm.tap()++ // Assert the toast FIRST: it is set the instant `performCopy` runs (right+ // after dismissing the picker) and self-dismisses after a few seconds, so+ // checking it before the slower dismissal poll avoids racing its timeout.+ let toast = app.staticTexts["Copied to Sam."]+ XCTAssertTrue(+ toast.waitForExistence(timeout: 5),+ "A confirmation toast naming Sam should appear on the list"+ )++ // And the picker is gone (its confirm button no longer exists).+ XCTAssertFalse(+ confirm.exists,+ "The copy sheet should dismiss after confirming"+ )+ }+}
diff --git a/docs/agent-notes/rules-engine.md b/docs/agent-notes/rules-engine.mdindex f0620ec..5919ba5 100644--- a/docs/agent-notes/rules-engine.md+++ b/docs/agent-notes/rules-engine.md@@ -42,6 +42,7 @@ The flag is scoped to a single `TripTask` record — deleting on Trip A does not | 5.1 | Trip created | `TripListView.swift` `onSave` closure (create) calls `runner.runForTrip(newTrip)` after `context.save()` | | 5.2 | Trip attributes edited | `TripDetailView.swift` `onSave` closure for the edit sheet, same pattern | | 5.3 | Master item saved/deleted | `MasterTaskEditor.swift` / `MasterPackingEditor.swift` `runEngineAndDismiss()` calls `runner.runForAllNonPastTrips()` |+| 5.3 | Master packing item copied to other people | `MasterPackingList.swift` `performCopy(...)` also calls `runner.runForAllNonPastTrips()` after the copy save (copy-master-packing-items flow) | | 5.4 | Cold launch | `ScrambleApp.init()` — runs after `UITestSeed.applyIfRequested(...)`, before `WindowGroup` mounts. Synchronous on purpose to satisfy the Phase 1 auto-open ordering invariant (Phase 1 AC 5.6). | | 5.7 | Foreground transition | `RootView.swift` `onChange(of: scenePhase)` with a `hasBeenBackgrounded: Bool` latch (see Decision 9) |
This worktree’s simulator test-host has a launch-crash flake that makes xcodebuild’s xcresult aggregate report misleading failures while the action exits 0. The new Copy/eligibility/outcome suites and the UI test were confirmed green on a single booted simulator, but a full clean-environment make test is advisable to rule out interaction with untouched suites.
The copy confirmation toast uses duration: 5 vs the app-wide 3s default — deliberate (so it isn’t lost during picker dismissal) and commented at the call site.