Skip to content

Customer Center - #509

Open
DreamingInBinary wants to merge 64 commits into
developfrom
customer-management-portal
Open

Customer Center#509
DreamingInBinary wants to merge 64 commits into
developfrom
customer-management-portal

Conversation

@DreamingInBinary

@DreamingInBinary DreamingInBinary commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Changes in this pull request

Adds the Customer Center: a native, self-service subscription-management screen inside the SDK. One call presents it:

Superwall.shared.presentCustomerCenter()

It shows the customer's subscriptions and purchases and lets them restore purchases, open Apple's manage-subscriptions sheet, request a refund, change plans, contact support, answer an exit survey, and browse purchase history. There's a SwiftUI view (CustomerCenterView), a UIKit view controller (CustomerCenterViewController), an Objective-C surface, a delegate, five SwiftUI callback modifiers, five new analytics events, and strings for all 41 locales.

Everything is configured in code via SuperwallOptions.customerCenter. The configuration model is Codable and deliberately shaped so a future dashboard/backend can serve the same JSON without changing the public API — resolution order is per-call argument → options → .default.

Zero-config gives a working screen: with an active App Store subscription you get the subscription card, Restore, Change plan, Request a refund, Cancel subscription (with a cancellation survey), See all purchases, and Account details. The only row that needs configuration is Contact support, which is hidden unless a support email is set.

Requires iOS 15+. The SDK's deployment target is unchanged at iOS 13 — the Customer Center symbols are @available(iOS 15.0, *), because every StoreKit API it drives is iOS 15+ anyway.

Reviewing this

108 files is a lot, but five files are the whole feature — the rest is SwiftUI, tests, and localization:

  1. CustomerCenter/Models/CustomerCenterConfiguration.swift — the entire public surface. Start here.
  2. Superwall+CustomerCenter.swift — the entry point (~100 lines).
  3. CustomerCenter/ViewModel/CustomerCenterViewModel.swift — state, flows, event emission.
  4. CustomerCenter/Logic/CustomerCenterPathResolver.swift — which actions appear when. This is the product logic.
  5. CustomerCenter/Logic/PurchasePresentationBuilder.swift — badges, status lines, renewal dedupe.

For 4 and 5, the table-driven tests read like a spec and are the fastest way in. Alternatively the commits are in dependency order, tests first, one concept each: git log --reverse --patch <base>..HEAD -- Sources/SuperwallKit/CustomerCenter.

Changes outside CustomerCenter/ are all small, necessary hooks: SuperwallOptions (+2 lines), LogScope (+1 case), DeviceHelper (+1 internal accessor), DependencyContainer (lazy @MainActor manager), the three analytics files (5 new event cases, purely additive), SuperwallKit.md, and one defaulted parameter on a shared test fixture. TransactionManager gains a presentsFailureAlert flag defaulting to true, so paywall restore behaviour is bit-identical.

Deliberately out of scope

Promotional / win-back retention offers (they need server-side signature generation), remote dashboard configuration, support tickets, virtual currencies, and the Android / Flutter / React Native bridges.

Decisions worth a second opinion

  • Version stayed at 4.16.4. develop was already ahead of master (4.16.3), so per CLAUDE.md the CHANGELOG entries went into the existing staged section rather than bumping again. New public API arguably warrants 4.17.0 — reviewer's call; it's a three-file change.
  • "Customer Center" is also RevenueCat's product name, chosen for discoverability and migration parity. Verified there are no symbol clashes: our Objective-C classes are SWK-prefixed against their RC-prefixed ones, so no duplicate class registration. One real collision was found and fixed — both SDKs put presentCustomerCenter on SwiftUI's View with everything after isPresented defaulted, and Swift silently resolved the bare call to ours (its solver penalises each defaulted argument it fills; ours fills 2, theirs 13), which would have hijacked an existing RevenueCat customer's screen with no error. Our modifier is now presentSuperwallCustomerCenter. Verified by building a target that imports SuperwallKit, RevenueCat and RevenueCatUI together and demangling the linked symbols.
  • Refund stays available on expired subscriptions. Apple permits it, but it's a product call.
  • "Cancel subscription", not "Manage subscription". In the default configuration that row carries the cancellation survey and opens Apple's cancel sheet, so the old label overstated it. Each locale uses its subscription-termination verb (German kündigen, French résilier, Japanese 解約) rather than the dialog-dismiss word.
  • The 41 locale translations are first-draft with no native-speaker review. Worth routing through localization before release.

Known gaps

  • The internal navigation flag was replaced with a visibility count, which closed the embedded-mode dismissal hole. Row ordering is nondeterministic when two products tie on both active-ness and expiry date (contents are deterministic). Support/Appearance/ColorPair override isEqual without hash, matching existing convention in CustomerInfo and friends.
  • dismiss(completion:)'s UIKit-driven completion path and the SDK's alert-suppression have no automated coverage: the hostless test target cannot complete modal presentations or present a UIAlertController, so such assertions would pass whether or not the code works. Both were verified manually instead.

Testing

1006 tests across 110 suites, all passing. Every task was reviewed by someone other than its author, with a cross-cutting review over the whole branch.

A full manual pass was also run on device across 20 scenarios — purchase, cancel, refund, expiry, billing retry, empty state, restore, delegate callbacks, code-driven configuration — and it found two real bugs that static review did not:

  • Apple's manage-subscriptions sheet never appeared after the cancellation survey. ManageSubscriptionsSheet branched on groupId, which turned non-nil in the same update that flipped isPresented true; SwiftUI tore down the modifier that was about to present. Now branches only on #available.
  • Restoring with no purchases showed two stacked alerts — the SDK's paywall-worded failure alert on top of the Customer Center's own.

Also fixed from that pass: disclosure chevrons were removed from action rows (a chevron promises a push, and none of those rows push), and the update banner now animates out instead of blinking.

Checklist

  • All unit tests pass. (1006 tests / 110 suites)
  • All UI tests pass. — N/A, this repo has no UI test target.
  • Demo project builds and runs on iOS. (Basic and Advanced; manually exercised on iPhone 17 simulator)
  • Demo project builds and runs on Mac Catalyst. (framework builds for Catalyst; the Customer Center is #if os(iOS) and available on Catalyst 15+)
  • Demo project builds and runs on visionOS. — not verified; CI doesn't build visionOS.
  • I added/updated tests or detailed why my change isn't tested. (See "Known gaps" for the two paths the hostless test target cannot cover.)
  • I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
  • I have run swiftlint in the main directory and fixed any issues. (10 violations, all pre-existing on develop; zero added.)
  • I have updated the SDK documentation as well as the online docs. — DocC article added (Documentation.docc/CustomerCenter.md) and linked from SuperwallKit.md. The online docs page still needs writing.
  • I have reviewed the contributing guide

cc @yusuftor @jakemor @anglinb

Known limitation: web prices assume USD

/v1/products stamps currency: "usd" unconditionally (products-public.ts); the real per-product currency lives in Product.metadata.__superwall_price_currency and isn't loaded on that path. So a non-USD web subscription renders as dollars here. The V2 products API already resolves it correctly via ProductMapper.normalizeCurrency, so the fix is server-side. Shipping as-is on the assumption app-to-web is US-only today.

DreamingInBinary and others added 29 commits August 20, 2026 13:24
Adds SuperwallEvent.customerCenterOpen/Close/Action/SurveyResponse/RefundRequest
with ObjC mirrors and InternalSuperwallEvent trackable structs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the Customer Center's 74 string keys (screens, paths, survey,
purchase status, badges, stores, sections, restore, refund, update
warning, duplicate subscriptions, and support) to all 41 Localizable.strings
bundles, plus the bundle-backed CustomerCenterStrings.bundled(locale:).

Also folds in two items deferred from Task 6's review: a dedicated
customer_center_expired key so an inactive subscription with no
expiration date shows "Expired" instead of "Refunded", and a
regression test for nil-expiration sort ordering.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e and restore views

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tCustomerCenter

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… actor

DependencyContainer.init constructed CustomerCenterManager via
MainActor.assumeIsolated at the end of init, but init itself isn't
@mainactor. ~20 test suites (and any host app calling Superwall.configure
off-main) construct DependencyContainer off the main thread, crashing with
EXC_BREAKPOINT. Fixed by deferring construction to the customerCenterManager
accessor itself, now marked @mainactor and built lazily on first access; all
production call sites (Superwall.presentCustomerCenter/dismissCustomerCenter/
the Objective-C variant) are already @mainactor, so this needs no
assumeIsolated.

Also logs a loud warning from CustomerCenterManager.makeViewModel(configuration:)
when Superwall hasn't been configured yet, since CustomerCenterView/
CustomerCenterViewController route through it and would otherwise silently
render a dead screen with no purchase data.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a Customer Center button to the Basic and Advanced example apps,
a CustomerCenter.md DocC article, and CHANGELOG entries under the
already-staged 4.16.4 release (develop is ahead of master, so no
version bump is needed).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…doff, embedded dismiss, receipt refresh, ObjC parity

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The screen shows when the customer has no purchases on record at all — no
subscriptions (active or expired), no one-time purchases, no active
entitlements. An expired subscriber routes to the management screen, so
"no active" described a case that never reaches here. The new name matches
the hasAnyPurchases predicate that actually gates it.

Renames the public noActiveScreen property, the internal screen state case,
NoPurchasesScreenView, the customerCenterOpen event's screen value, the
accessibility identifier, and the localization keys across all 41 locales
(keys only — the displayed copy is unchanged).
The final review pass rewrote "Created by Claude" to "Created by Jordan
Morgan" across the whole repo when it should have been scoped to the files
this feature adds. That touched 24 pre-existing files (TestMode,
V2ProductsResponse, TestStoreUser, EntitlementProcessor and several test
files) that have nothing to do with the Customer Center. Restores them to
their state on develop; the header fix stands only on Customer Center files.
…CustomerCenter

RevenueCatUI puts a presentCustomerCenter modifier on View with every
parameter after isPresented defaulted, and so did we. Verified empirically by
building a target that imports SuperwallKit, RevenueCat and RevenueCatUI: with
the shared name, a bare .presentCustomerCenter(isPresented:) call compiled
without error and silently resolved to SuperwallKit's — Swift's solver
penalises each defaulted argument it fills, and ours fills 2 against
RevenueCat's 13. An existing RevenueCat customer adding SuperwallKit would have
had their Customer Center silently swapped for ours, with no diagnostic.

Renaming the modifier makes each resolve to its own module. Confirmed by
demangling the linked symbols: presentCustomerCenter -> RevenueCatUI,
presentSuperwallCustomerCenter -> SuperwallKit.

Objective-C was already safe (RC* vs SWK* prefixes, so no duplicate class
registration at load, which @available could not have prevented). The four
shared Swift type names (CustomerCenterView, CustomerCenterViewController,
CustomerCenterNavigationOptions, CustomerCenterAction) stay as they are —
module qualification resolves those, and it is idiomatic Swift.

Superwall.shared.presentCustomerCenter() is unchanged; it is on our own type
and cannot collide.
In the default configuration, the .manageSubscription path carries the
cancellation survey and leads to Apple's manage-subscriptions sheet, so
its job is cancelling, not general management. "Manage subscription"
overstated what the row does.

The key customer_center_path_manage_subscription is unchanged since it
tracks the PathType.manageSubscription case, not the displayed text —
only the string values change, across englishStrings and all 41
Localizable.strings locales.

Each locale uses its subscription-termination verb (e.g. German
"kündigen", French "résilier", Japanese "解約", Dutch "opzeggen",
Italian "disdire", Croatian "otkazati", Danish/Norwegian "si/sei opp")
rather than reusing customer_center_cancel's dialog-dismiss word, except
where a language genuinely shares one verb for both senses (e.g.
Spanish, Portuguese, Polish, Czech, Vietnamese, Thai, Korean, Chinese),
confirmed against each file's existing register.
The ManageSubscriptionsSheet modifier chose its branch on `groupId`, which is
derived from viewModel.sheet and therefore turns non-nil in the same update
that flips isPresented to true. SwiftUI treats the two branches as different
view identities, so that update tore down the modifier that was about to
present and built a different one — Apple's sheet never appeared. Reported
from a device run: answering the cancellation survey dismissed the survey and
returned to the Customer Center with nothing else shown.

Branch on #available only, which is constant for the process, and pass the
group id through as a value. The sheet is never presented while groupId is
nil, so the empty-string fallback is unreachable in practice.

Not coverable by the existing tests: the view model already asserts the state
transition (sheet == .manageSubscriptions after the survey dismissal), and it
still passes — the failure was entirely in the SwiftUI presentation layer,
which the hostless test target cannot exercise.
A chevron promises a push onto the navigation stack. None of the action rows
push: restore runs in place, cancel/change plan/refund/custom URL present
sheets, and contact support leaves the app. The rows that genuinely push —
"See all purchases" and the purchase detail rows — are NavigationLinks and
draw their own chevron, so those are unaffected.

The rows still read as tappable from the accent-coloured label, matching how
action rows look elsewhere in iOS. The in-row progress indicator is kept.
…stomer Center's

Restoring from the Customer Center with no purchases showed two stacked
alerts: the SDK's paywall-worded restore-failure alert ("No Subscription
Found") on top of the Customer Center's own result alert ("No past
purchases", which is localized and offers Contact support).

tryToRestore gains a presentsFailureAlert flag, defaulting to true so the
public restorePurchases() and all paywall restores are unchanged. The
Customer Center passes false and keeps presenting its own outcome.

No automated coverage: the SDK presents that alert on the top-most view
controller via the key window, which the hostless test target has no way to
provide, so an assertion that no alert appears passes whether or not the fix
works. Verified against the reported device repro instead.
Tapping Continue flipped the flag outside a transaction, so the banner's
section vanished from the list in a single frame. Wrap the change in
withAnimation at the view layer, so removing the section from the list is part
of the same transaction. Reduce Motion gets withAnimation(nil), which applies
the change without animating.

Also adds a round-trip test for the appearance accent: a UIColor passed to
ColorPair is stored as hex and has to parse back into a Color for the theme to
tint anything. Nothing covered that path before.
…e root view

The root view's `.onDisappear` fired `dismiss()` directly, gated by an
`isNavigatingWithinCustomerCenter` flag set/cleared by pushed screens'
onAppear/onDisappear. In embedded mode (`usesExistingNavigation`) the host
owns the navigation stack, so if it tears its stack down while a pushed
screen (purchase detail / purchase history) is on top — popping to root,
resetting a NavigationPath, or a long-press-Back past the Customer Center —
the root view never reappears and the flag never clears. `didDismiss` and
`customerCenterClose` then never fire at all. The flag was also inaccurate
two pushes deep: history → purchase detail cleared it while still inside.

Replaced the boolean with a visibility count on the view model:
`surfaceDidAppear()`/`surfaceDidDisappear()` increment/decrement a counter,
attached to every surface that can be on screen (root, purchase detail
screen, purchase history, purchase detail rows — not sheets, since those
present over a root that stays alive). When the count reaches zero it
debounces briefly (default 0.3s, cancellable) before calling `dismiss()`,
because a push/pop transition can briefly have both or neither surface on
screen — one runloop turn isn't enough to tell "navigating within the
Customer Center" from "actually gone". `dismiss()` keeps its `didDismiss`
latch, so double-firing stays impossible regardless of how many surfaces
disappear.

Sheet mode and the UIKit CustomerCenterViewController are unaffected: the
root view still appears/disappears exactly once for those, so `didDismiss`
still fires exactly once.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Review flagged 0.3s as uncomfortably close to a UINavigationController
push/pop (~0.35s). During a pop the outgoing screen's onDisappear can land
before the root's onAppear, dipping the visible-surface count to zero
mid-transition; if the debounce elapses in that window, didDismiss fires while
the user is still inside the Customer Center. 0.6s clears it with margin.

The interval only delays how soon didDismiss reaches the host, and nothing is
gated on it. Tests inject a short interval, so they are unaffected.
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown

PR author is not in the allowed authors list.

…er test's veto

Puts back two things a blanket `git add` swept into 058dacf from the working tree: the Advanced
example's cancellation survey on the manage path, and its `testModeBehavior = .never`. Both are
part of this PR's demo of the feature; neither had anything to do with that commit's subject.

Also answers two review threads on the controller tests.

`pushedCoverDoesNotFireLateDismissal` called `viewDidDisappear` after the run loop had settled, so
the veto always ran last — the one ordering production doesn't have to guarantee — and nothing
established that SwiftUI's `onDisappear` had armed anything in this harness. The debounce is now
armed explicitly before the veto, so the test fails if the veto stops cancelling. Production
ordering is sound for a different reason, noted there: `viewDidDisappear` calls `super` first,
which is what forwards the disappearance into SwiftUI.

And the bar test's doc comment still opened with the behaviour that change removed, contradicting
the two sentences under it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pullfrog

pullfrog Bot commented Aug 28, 2026

Copy link
Copy Markdown

Pullfrog billing is temporarily unavailable.

model-credential service temporarily unavailable — retry shortly

Usually transient; the next dispatch should succeed. If it persists, check status.pullfrog.com or your console.

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

…s something

`Appearance` published five colour slots and only `accent` ever reached a pixel. The other four
were parsed from the host's hex strings, round-tripped through `Codable`, compared and hashed, and
applied nowhere — so a host could set a background or text colour, get no error, and see no
change. Shipping them in 4.17.0 would have meant honouring them later or removing them later, and
removing a property from a public, `Codable`, Objective-C-exposed type is a breaking change.
They'll come back when they're wired up.

Also takes two test-only edits back out of the Advanced example: `testModeBehavior = .never`, and
the cancellation survey on the manage path. Neither was meant to ship — they were left over from
exercising the feature by hand, and I put them back in the previous commit while undoing an
unrelated blanket-add. The example's only change from this PR is now the Customer Center button
and its delegate; `SuperwallAdvancedApp.swift` is untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pullfrog

pullfrog Bot commented Aug 28, 2026

Copy link
Copy Markdown

Pullfrog billing is temporarily unavailable.

model-credential service temporarily unavailable — retry shortly

Usually transient; the next dispatch should succeed. If it persists, check status.pullfrog.com or your console.

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

…tates

The design review needs to see what a customer with more than one
subscription gets, and what changes when one of them didn't come from
Apple. Those are exactly the states that are painful to produce for real
— a live Stripe subscription alongside a live App Store one, two App
Store subscriptions in different groups — so the harness fabricates them
instead.

Adds eight states (23–30): web only, web with no management URL
configured, App Store and web together, that mixed customer drilled into
each of the two subscriptions, the two-group customer drilled in, a
comped entitlement, and a family-shared subscription.

Family sharing isn't in `CustomerInfo` — it's a StoreKit lookup — so
`makeViewModel` now takes a `familyShared` set and feeds it through the
transaction lookup mock. The web fixture gets a priced catalogue entry so
its card renders the way production does: price yes, display name no.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No blocking issues. The Appearance narrowing is clean and settles the last review's scope question outright; the new snapshot scenarios have two fixtures that don't produce the state their caption describes. Suggestions inline.

Reviewed changes — the three commits since the prior pullfrog review at 058dacf: 576f842, 00c2354 and 290e626.

  • Cut Appearance down to the one colour that reaches a pixelbackground, text, buttonText and buttonBackground are gone from the public class (property, init, isEqual, hash) and from CustomerCenterTheme, with a doc comment recording that removing them from a public Codable ObjC type after release would have been breaking. Nothing in Sources/, Tests/ or Examples/ read them, Appearance() still compiles, and the CHANGELOG and DocC intro now say "accent colour". This answers the previous review's scope question rather than deferring it.
  • Made pushedCoverDoesNotFireLateDismissal able to fail — the debounce is now armed explicitly with surfaceDidDisappear() instead of relying on SwiftUI's onDisappear firing in a hostless harness, so the test no longer passes with the veto deleted.
  • Removed the two stale sentences from the bar test's doc comment — the pair describing the nav-bar takeover that 1b1d7b3 deleted are gone, leaving only the accurate ones. Thread resolved.
  • Added seven design-review scenarios (23-30) — web-only, web with no management URL, mixed App Store + web, two mixed detail screens, two subscription groups, a comped entitlement and a family-shared purchase, plus a web_pro_monthly catalogue entry and a familyShared parameter on the fixture wired through StoreKitTransactionLookupMock.
  • Flip-flopped the Advanced example and landed where it started576f842 restored testModeBehavior = .never and the cancellation survey, 00c2354 took both back out, so Examples/Advanced/** is byte-identical to 058dacf.

I checked the three fixture behaviours the new scenarios depend on and they hold: 25's duplicate banner does fire (an active .appStore store alongside .stripe is exactly the condition at CustomerCenterViewModel.swift:142-145), 30's family-sharing does gate all three of manage/refund/change-plan (familyShared: ["monthly_pro"] matches subscription()'s default productId), and CustomerCenterConfiguration.default is a computed var, so the per-scenario mutations can't leak between snapshots.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread Sources/SuperwallKit/CustomerCenter/Views/CustomerCenterView.swift Outdated
… they claim

Three of the snapshot scenarios rendered something other than their
caption, which is worse than not having them: a designer reviews the PNG,
not the fixture.

- 23 and 24 were byte-identical. `EnvironmentMock` takes the web
  management URL directly and defaults it to nil; only `LiveEnvironment`
  reads it out of the configuration, so 23 rendered the unconfigured
  screen. It now sets both. The list is genuinely the same either way —
  the row stays put, because removing it would strand a paying customer —
  so 24 now captures what the tap actually produces, which is where the
  two diverge.
- 29 used `Entitlement(id:)`, a convenience init that hardcodes
  `store: .appStore`. That labelled the card App Store and sent the
  resolver down the App Store arm rather than the `.entitlementOnly` one
  written for comped grants. Passing `store: nil` renders the real thing.
- Every screen with account details showed "December 31, 1969", because
  the mock's install date defaults to the epoch. Harmless in a unit test;
  in a design review it reads as a bug and costs the room a conversation.

Also documents how to actually run this suite. The header claimed
`CUSTOMER_CENTER_SNAPSHOT_DIR=... xcodebuild test` works; it doesn't, and
neither does xcodebuild's `TEST_RUNNER_` prefix — neither reaches the test
process in the simulator, and the suite skips in silence. The variable now
sits on the scheme, off by default, so it survives xcodegen.

Two comments corrected alongside: one asserted an `onDisappear` ordering
Apple documents as view-type-dependent, the other argued against a
`theme.background` that 00c2354 deleted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No blocking issues — all four findings from the last review were applied verbatim, and the two fixture corrections do change what gets rendered. One suggestion inline. Not approving only because a dozen older threads are still open.

Reviewed changes — the single commit since the prior pullfrog review at 290e626: f01911b, which corrects the design-review fixtures and moves the snapshot switch onto the scheme. The only Sources/ edit is a comment, so there is no shipped behaviour change in this delta.

  • Gave scenario 23 a management URL the resolver can actually seeEnvironmentMock(webManagementURL:) is now passed alongside support.webManagementURL, so 23 resolves .webManage rather than silently rendering 24's unconfigured screen. Verified: PathResolutionContext.webManagementURL is sourced only from dependencies.environment.webManagementURL (CustomerCenterViewModel.swift:173), and only LiveEnvironment reads the configuration slot (CustomerCenterDependencies.swift:236).
  • Split scenario 24 off from 23 by capturing what the tap produces — since both destinations are isWebManagement, PathsListView titles them identically and the two lists really are the same screen; 24 now captures the customer_center_web_manage_unavailable copy instead.
  • Made scenario 29 a genuine comped grantEntitlement(id: "pro", store: nil) reaches the internal memberwise init (Entitlement.swift:175-189) instead of the public convenience one that hardcodes store: .appStore, so the fixture takes the .entitlementOnly arm the caption describes.
  • Stopped every screenshot showing a 1969 install datemakeViewModel replaces EnvironmentMock's epoch default with a plausible date before building dependencies, so Account details reads sensibly across all 30 scenarios.
  • Moved the snapshot switch onto the schemeCUSTOMER_CENTER_SNAPSHOT_DIR is declared in project.yml with isEnabled: false and regenerated into all three scheme actions, and the file header now documents the Xcode-scheme route plus the deliberate closing Issue.record. Off by default, so scripts/test.sh is unaffected.
  • Repointed the loading cover's comment at the configuration slot and recorded the viewDidDisappearonDisappear ordering as an assumption — both are the previous review's suggested wordings applied as-is.

I checked the three things this could plausibly have broken and they hold: Entitlement(id:store:) resolves to the internal init under @testable import with every other parameter defaulted, so there is no ambiguity with the public init(id:); EnvironmentMock is a struct with a var originalDownloadDate: Date?, so the local var environment = environment copy-and-mutate compiles and the Date? == Date comparison promotes correctly; and no changed line exceeds the lint length limit or carries trailing whitespace.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread Tests/SuperwallKitTests/CustomerCenter/Views/DesignReviewSnapshots.swift Outdated
…ne's sheet

`isManagePresented`'s setter cleared `sheet` whatever it held, while its
sibling nine lines below checked identity first. SwiftUI writes `false`
into a boolean sheet binding whenever its getter goes false — which
happens to every surface the moment something is pushed over it, not only
when that surface's own sheet closes. So a covered screen could tear down
the sheet the visible one had just opened and run `sheetDidDismiss()`,
receipt refresh included, against a sheet that was never showing.

Both setters now go through `CustomerCenterSheetOwnership.dismissalClears`,
alongside the depth rule that was already there.

The modifier and the two bindings are internal rather than private so the
test can drive them. Testing the rule alone would have passed against the
unguarded setter — which is exactly how this gate shipped inert once
before — so the regression test writes `false` into the manage binding
while the refund sheet is up and asserts the refund sheet survives.
Verified it fails against the old setter before keeping it.

Also captures design-review scenario 24 as sheet content rather than
through `captureDetail`, which wrapped it in a `NavigationView` and drew a
title bar the real page sheet does not have — hiding the point of the
screenshot, which is that the sheet is one unstyled sentence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues of substance — the manage-setter asymmetry from the last review is closed, and the new binding-level test fails against the old setter. Two nitpicks inline. Not approving only because older threads remain open.

Reviewed changes — the single commit since the prior pullfrog review at f01911b: edb9e48, which stops a covered surface tearing down the visible surface's sheet. Three files, one behavioural line.

  • Gated the manage sheet's dismissal write on the sheet actually upisManagePresented's setter went from an unconditional viewModel.sheet = nil + sheetDidDismiss() to the new CustomerCenterSheetOwnership.dismissalClears(_:_:), so a stale false write can no longer clear someone else's sheet or run a receipt refresh against a sheet that was never showing. refundBinding's setter is byte-equivalent to its old case .refund check, only rerouted through the shared rule.
  • Made the rule and the modifier testableCustomerCenterSheetsModifier and its two boolean bindings went private → internal, with a comment recording why (the gate has been wrong twice, and both times in the binding rather than in the rule).
  • Added three tests, one of which reaches the real binding — a 4-row table over dismissalClears, the nil case, and staleDismissalLeavesTheOpenSheetAlone, which constructs the modifier at surfaceDepth: 0 and writes false into isManagePresented while sheet == .refund. That last one fails against the pre-edb9e48 setter, which is what the previous review asked for.
  • Gave scenario 24 the container production actually uses — a new captureSheet helper renders the sheet copy with no NavigationView, so the PNG no longer paints a title bar the real page sheet doesn't have.

I checked the thing this guard could plausibly have left open and it holds up: dismissalClears matches on the sheet's case, not on which surface presented it, so in principle a covered surface could still clear a same-case sheet another surface has up. Tracing every production writer of pushDepth (CustomerCenterPushNavigator.push, reachable only from CustomerCenterDrillDown's button, and onRemovedFromParent), changing the depth requires touching a screen that Apple's StoreKit sheet is covering — and no SDK code pops the stack programmatically. CustomerCenterPathResolver also needs a non-nil purchase for both the manage and refund destinations while ManagementScreenView.swift:49 passes nil from the root whenever there is more than one purchase, so in the only shape where two depths coexist the root can't resolve those paths at all. The same argument covers itemSheet's still-ungated setter. The one residual is a host popping its own navigation stack in .pushed mode while a StoreKit sheet is live, which is outside anything the SDK drives.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread Sources/SuperwallKit/CustomerCenter/Views/CustomerCenterSheets.swift Outdated
…nding

The comment above `isTopmost` still read "only the getters are gated",
three lines above two guarded setters. The distinction it is drawing is
depth versus identity, so it now says that: getters gated on depth,
setters on the sheet's identity, and the depth-gating hazard it documents
spelled out as the reason for the split.

The regression test pinned the negative case for the manage binding and
the positive for refund, but never the positive for manage — the one that
runs `sheetDidDismiss()`. It now clears a manage sheet from a surface that
is no longer topmost, which is the "the owner may always clear" invariant
the earlier depth-gated setter broke.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues. Both nitpicks from the last review were applied essentially verbatim, and the added assertion fails against the setter it guards. Not approving only because thirteen older threads remain open.

Reviewed changes — the single commit since the prior pullfrog review at edb9e48: a9ad11d, which answers both of that review's inline nitpicks. Two files, +13/−4, and no shipped behavioural surface.

  • Said which gate applies to which half of the bindingisTopmost's doc comment (CustomerCenterSheets.swift:67-71) went from "Only the getters are gated" to the depth-vs-identity split, which is the distinction the code actually draws. Comment only: isTopmost, both boolean bindings and itemSheet are byte-identical to edb9e48.
  • Pinned the manage binding's positive casestaleDismissalLeavesTheOpenSheetAlone now raises pushDepth to 1, puts a manage sheet up, writes false into isManagePresented from the non-topmost surface, and asserts sheet == nil. That is the "the owner may always clear" invariant the earlier depth-gated setter broke, and the assertion fails if depth gating is reintroduced on the setter — so it is real coverage rather than a restatement of the two rule-level tests above it.

I checked the one thing worth checking on a test that fires an unstructured task. The setter's Task { await viewModel.sheetDidDismiss() } outlives the test body, but dependencies.customerInfo is CustomerInfoProviderMock, lastPresentedSheet was never set so it takes the cached fetchCustomerInfo() arm rather than a receipt refresh, and the view model is test-local — nothing escapes into the rest of the .serialized suite. The assertion itself deliberately stops at sheet == nil; asserting the task ran would be near-tautological, since the nil write always precedes it on that path.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

DreamingInBinary and others added 3 commits September 9, 2026 12:58
**A comped grant is a nil store, not a missing transaction.** The manage
row was withheld from any `.entitlementOnly` purchase, on the assumption
that no transaction means nothing to manage. But a web purchase arrives as
a bare entitlement whenever the backend sends no matching transaction —
and the only production constructors of `SubscriptionTransaction` and
`NonSubscriptionTransaction` hardcode `store: .appStore`, so a paying
Stripe or Paddle subscriber may only ever be `.entitlementOnly`. That took
the row back off exactly the customers it was added for. It now keys on
`entitlement.store == nil`, which is what "comped" actually means.

The test that was meant to cover this couldn't fail: `Entitlement(id:)`
hardcodes `store: .appStore`, so the purchase never reached the web branch
and `manage == nil` came from the App Store branch instead — green with
the rule deleted. It now passes `store: nil`, asserts the store it is
testing, and has a sibling pinning the other half: a web entitlement with
no transaction keeps its row.

**A dismissed container is a dismissed Customer Center.** The pushed
drill-down checked `isBeingDismissed`/`isMovingFromParent` on itself only,
while the root controller walks the whole parent chain and documents why.
So when a host presented a navigation controller holding a pushed Customer
Center and dismissed it from a drill-down, UIKit marked the container,
the drill-down read that as a cover, vetoed the debounce — and the root
underneath, being covered, never got a `viewDidDisappear` to correct it.
No dismissal was delivered at all. Both now share one check.

**The update lookup's region.** `Locale.current.regionCode` is deprecated
at iOS 16 and was read unguarded, while `DeviceHelper` guards the same
property. It is also the device's region setting rather than the App Store
storefront that decides which listing exists; the storefront would be
better but reports three-letter codes this endpoint won't take, so the
doc comment now says that instead of claiming otherwise, and the
no-listing log names the region as the likely cause.

Also covers the lookup itself. `defaults`, `session` and `now` existed on
the initializer purely as test seams and nothing used them, so the 24h
cache, the region query item and the non-2xx branch all shipped
unexercised. A `URLProtocol` stub and a throwaway defaults suite reach all
three without touching the network.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…comments overclaiming

**The veto assumed an ordering it doesn't control.** Being covered
cancelled a pending dismissal, which only works if SwiftUI has already
delivered `onDisappear` by the time `viewDidDisappear` runs. Apple
documents that moment as depending on the view type and ties it to no
UIKit callback — arriving a runloop turn later, the veto found nothing to
cancel, the debounce armed unopposed, and the premature `didDismiss`
fired and latched, silencing the real teardown. It now suppresses until
the next appearance, which covers both orderings. Genuine teardowns are
unaffected: the controllers deliver those through `dismiss()` directly.

The existing test armed the debounce before the veto — the easy ordering,
which passes either way. It keeps that case and gains the one production
can't guarantee, verified to fail against the cancel-only veto.

**The catalogue gap-fill had no test that reached it.** Both pricing tests
built `ProductDisplayInfo` by hand, so deleting the whole gap-fill left
them green: what they pinned was that `APIStoreProduct` yields a usable
price, not that the provider ever asks for one. The rule is now a static
`fillingGaps(in:requested:from:)` — `products(for:)` itself reaches
`Superwall.shared` and the container's network, so the rule is as close as
a test can get — and five cases cover it, including the one that matters
for money: an App Store product StoreKit failed to return is never filled
from the catalogue, whose price is the dashboard's storefront rather than
the customer's.

**Comments.** The locale note named `SuperwallOptions.localeIdentifier` in
two places; `environment.locale` is `DeviceHelper.preferredLocaleIdentifier`,
the device's preferred language, and the option only applies when there are
no preferred languages at all. The `load()` ordering comment implied open
always precedes close, which holds for the App Store lookup but not for a
customer who closes while `apply` is still awaiting StoreKit.

**CHANGELOG.** Four entries described fixes to the Customer Center under
the release that introduces it, so they read as fixes to something nobody
has shipped against — and one of them repeated the locale claim above.
Dropped; the two genuine fixes to shipped behaviour stay.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…change

`apply(customerInfo:refetchProducts:)` runs on load, on every restore and
after every sheet dismissal, and all four call sites refetch — so a
web-store customer paid for the whole Superwall catalogue several times
inside a single visit, for a product list that changes on the dashboard's
timescale rather than theirs. The 5s bound added earlier caps how long
each of those can hurt; it doesn't stop them happening.

A short-lived cache in front of the fetch: long enough to cover one visit,
short enough that a price edit shows up the next time anyone opens the
screen. Failures aren't cached — the cards render without prices and the
next `apply` should try again rather than serve the failure for five
minutes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The new AppStoreVersionLookup tests share URLProtocol statics across a suite that has no .serialized trait, so they can stomp each other's stubbed response and race on an unsynchronised Array. One line to fix, and it's the only new finding.

Reviewed changes — the single commit since the prior pullfrog review at a9ad11d: be729b8, which answers three long-open threads. Two are now retired, two are partial and stay open.

  • Walked the whole parent chain when a pushed drill-down disappears — the chain walk was lifted out of CustomerCenterViewController.isLeavingHierarchy into an internal UIViewController.isLeavingHierarchyOrAContainerIs, and CustomerCenterPushedHostingController.viewDidDisappear now uses it instead of testing self alone. A host dismissing a container that holds a pushed Customer Center while the user is on a drill-down no longer vetoes the debounce, so the dismissal is actually delivered. The root controller's behaviour is byte-for-byte unchanged.
  • Narrowed the comped-grant exclusion from the purchase kind to a nil entitlement store.entitlementOnly alone was taking the management row away from web purchases that arrive as a bare entitlement, which is the shape the backend produces whenever it sends no matching transaction. Keying on entitlement.store == nil keeps the row for paying Stripe/Paddle customers and drops it only for grants with nothing behind them.
  • Guarded the deprecated region read and named the region in the failure logregionCode's default moved to a new deviceRegionCode that makes the same #available(iOS 16, *) split as DeviceHelper.regionCode, the doc comment stopped calling the value a storefront, and the "no App Store listing" warning now says which region it asked about.
  • Exercised AppStoreVersionLookup end to end — a URLProtocol stub, per-test UserDefaults(suiteName:) and an injected clock now cover the bundleId/country query items, the empty-region omission, non-2xx responses and both sides of the 24-hour cache.
  • Pinned both halves of the web-entitlement rulemakeEntitlementOnlyViewModel(store:) was extracted, the comped case now asserts it actually reaches the web branch, and a new parameterised test over [.stripe, .paddle] fails if the rule is keyed on the kind again.

Three things I checked that came back clean. The chain walk has no demonstrated false positive: a nav controller at the window root terminates the walk, a UITabBarController tab switch sets neither flag because children stay in viewControllers, and presentation is orthogonal to parent — only a custom container that "covers" by literally calling removeFromParent() could trip it, and nothing here does that. The veto side is still pinned by the pre-existing coveringADrillDownDoesNotDismiss (CustomerCenterSheetOwnershipTests.swift:207-233), which fails if the walk is replaced with a constant true. And deviceRegionCode's claim that it matches DeviceHelper.regionCode holds line for line.

ℹ️ Nitpicks

  • Tests/SuperwallKitTests/CustomerCenter/Logic/AppStoreUpdateCheckTests.swift:218-221makeDefaults() never calls removePersistentDomain(forName:), so each run leaves eight UUID-named preference plists behind in the test host's container.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread Tests/SuperwallKitTests/CustomerCenter/Logic/AppStoreUpdateCheckTests.swift Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues in these two commits — they close five open threads and each fix comes with a test that fails against the old code. One access-level nitpick below. Not approving because the .serialized finding from the previous review and five older threads are still open.

Reviewed changes — the two commits since the review at be729b8: 7841bc13 (the veto's ordering race) and c473247 (the catalogue cache).

  • Made the cover veto independent of when SwiftUI delivers onDisappear — a new isDismissalSuppressed flag replaces the one-shot cancelPendingDismissal() at both veto sites. surfaceDidDisappear now refuses to arm the debounce while it's set, and surfaceDidAppear clears it, so a covered .pushed Customer Center stays quiet whether SwiftUI reports the disappearance before or after viewDidDisappear returns. pushedCoverSurvivesALateDisappearance pins the ordering the old code lost, and also asserts the genuine teardown still lands afterwards.
  • Corrected three comments that overclaimed — the two copies of the date-locale note now name DeviceHelper.preferredLocaleIdentifier instead of SuperwallOptions.localeIdentifier, and load()'s "close before open" claim was narrowed to the lookup, since closing while apply's StoreKit round trip is up can still emit close first.
  • Dropped the four Customer Center entries from the CHANGELOG's ### Fixes — only the two that predate this branch remain under ## 4.17.0, so nothing reads as a fix to a feature nobody has shipped against.
  • Cached the Superwall catalogue for five minutes — a new CatalogueCache actor wraps the /v1/products call, so a web-store customer no longer pays for it again on every restore and every sheet dismissal. A throwing fetch is deliberately not cached, and all three cases are tested against an injected clock.
  • Extracted the gap-fill into a testable staticLiveProductsProvider.fillingGaps(in:requested:from:) is now pure, and five new cases pin that StoreKit always wins, that an App Store product is never priced from the dashboard catalogue, and that the catalogue's name reaches the card. Each fails if its rule is removed.

Two things I traced and found clean. The dismissal state machine holds under the new flag across the four shapes that matter — drill-down popped normally, whole stack popped at once (the root's didMove(toParent: nil) reaches dismiss() directly, bypassing suppression, which the new test's second assertion pins), host push then pop, and a page sheet over the modal style, which produces no viewDidDisappear at all and so never sets the flag. And the catalogue's guard !missing.isEmpty early return is still before the fetch, so an App-Store-only customer still issues no request.

ℹ️ Nitpicks

  • Sources/SuperwallKit/CustomerCenter/ViewModel/CustomerCenterDependencies.swift:184 and :233withCatalogueTimeout lost its private and CatalogueCache.isFresh never had one, but each has exactly one caller in the same file (:136 and :225). On an actor, a non-private isFresh also invites an await from outside that nothing needs.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

…o partial fixes

**`.url` now requires a title.** It was the one path type with no sensible
default: every other type has a fixed meaning and a localized label, while
a URL could be anything. The fallback was `url.host`, which keeps the part
of a URL that is identical across an app's own links and throws away the
part that says what the page is — so FAQ, terms and privacy all rendered
as three identical rows reading "example.com". There is no better
derivation, because the developer is the only one who knows what to call
it, so the case now carries the title and the compiler asks for it. The
ObjC factory's `title` goes non-optional for the same reason.

Breaking, and deliberately taken now: the Customer Center hasn't shipped,
so this is free today and wouldn't be after 4.17.0.

`PathsListView.title(for:strings:)` is static so the rule can be tested —
a title that reads the same for two different paths is only visible by
comparing two of them, which a rendered view can't easily be asked about.
The regression test is exactly that: three URLs on one host, three
distinct rows.

**The update lookup's cache is keyed on region.** It held one answer keyed
on time alone, so changing device region served the previous store's
version for the rest of the day — precisely when it is most likely wrong.

**The web management row now gates on state.** The App Store branch checks
that a subscription is live; the web branch checked only the store, so a
one-off Stripe charge with no subscription behind it and a subscription
that had already lapsed or been revoked were each offered a "Manage
subscription" row.

Also serializes `AppStoreUpdateCheckTests`, whose `URLProtocol` stub holds
static state that eight tests clobbered under parallel execution — the
failure currently red on CI.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues in this delta — the URL title change is complete and the two partial fixes both land. One minor suggestion inline. Not approving only because two prior threads are still legitimately open (AppStoreVersionLookup's country= source, and the catalogue fetch on the first-paint path).

Reviewed changes — the single commit since the prior pullfrog review at c473247: 92a2c20, which gives a URL path its own row name and closes two findings that were previously only half-answered.

  • Made a URL row name itselfPathType.url gained a required title associated value, and PathsListView.title reads it instead of url.host, so FAQ, terms and privacy on one host no longer render as three identical rows. Every Swift, Objective-C, DocC, example and test call site was updated; nothing constructs .url without a title.
  • Made the title rule directly testablePathsListView.title(for:strings:) is now static, with a new PathTitleTests suite covering the same-host case, the built-in localized fallbacks, explicit-title precedence, and the manage row's App-Store-versus-web wording.
  • Keyed the App Store version cache on the region it was fetched for — a third UserDefaults key compared in cachedVersion(), plus lookupDoesNotServeAnotherRegionsAnswer, which fetches in GB then JP at the same instant and fails without the key.
  • Gated the web management row on the purchase's kind and state — the branch now returns nil for a .nonSubscription purchase and for a revoked or inactive subscription, pinned by nothingToManageMeansNoRow and oneOffWebPurchaseHasNoRow. I checked the states this could have over-narrowed: grace period, billing retry and cancelled-but-live all carry isActive == true in this model (badge(for:) reaches .billingIssue only when isActive is true), and build() pre-filters entitlements on isActive, so no .entitlementOnly presentation can arrive inactive. No paying customer loses their route to the billing portal.
  • Serialised AppStoreUpdateCheckTests — the suite's shared StubURLProtocol statics are no longer read and written by concurrently-scheduled cases.

ℹ️ title is now the one key a dashboard-served path payload can't omit

PathType is Codable and the stated intent is that a future dashboard serves this same JSON. Support has a hand-written init(from:) precisely so a payload written before a key existed still decodes; url's title is a required associated value with no equivalent tolerance, and Screen.paths decodes as a whole array, so one path missing title would throw the entire configuration away rather than degrade. Nothing decodes this from the wire today, so this is a decision to record rather than a bug to fix.

Technical details
# Decide whether a dashboard-served `url` path may omit `title`

## Affected sites
- `Sources/SuperwallKit/CustomerCenter/Models/CustomerCenterConfiguration.swift:190``case url(URL, title: String, openMethod: OpenMethod)`. The SE-0295 synthesised decoder
  requires the `title` key and throws `keyNotFound` without it.
- `Sources/SuperwallKit/CustomerCenter/Models/CustomerCenterConfiguration.swift:118``public var paths: [Path]` decodes with the synthesised array decoder, so a single
  undecodable path fails the whole `Screen`, and therefore the whole
  `CustomerCenterConfiguration`.
- `Sources/SuperwallKit/CustomerCenter/Models/CustomerCenterConfiguration.swift` (`Support`'s
  hand-written `init(from:)`) — the in-file precedent, whose doc comment states the contract:
  "Everything the dashboard will eventually serve has to survive being decoded from JSON
  written before the key existed."

## Required outcome
- A recorded answer to whether the eventual backend contract makes `title` mandatory for a
  `url` path, or whether the client must tolerate its absence. Either is fine; the risk is
  only that the asymmetry with `Support` is unintentional.

## Open questions for the human
- If the dashboard will always send `title`, is that enforced server-side, or would a
  hand-rolled `PathType` decoder that falls back to the URL host on a missing `title` be
  cheaper insurance than a configuration that fails closed?

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread Sources/SuperwallKit/CustomerCenter/Models/CustomerCenterConfiguration+ObjC.swift Outdated
…ens its own screen

Three pieces of design-review feedback.

**A purchase with no display name is not shown.** A card is titled with
its product's name, and a card without one reads as a raw identifier —
`test:price_1TuEiI4PyZVB2o4B7dU1tBMh:no-trial`, wrapping to two lines —
which is worse than no card. `ProductDisplayInfo` now records whether it
found a name (StoreKit's `displayName`, or the catalogue's `name`), and
the builder drops subscriptions and one-off purchases whose product
resolved without one.

Two deliberate edges. A product that didn't resolve at all keeps the
identifier fallback: that is a lookup failure, not a naming decision, and
turning every StoreKit hiccup into a vanished subscription would be the
wrong trade. And entitlement-only rows are outside the rule — they have
no product to be named by, and the entitlement's own identifier is what
they show.

Today this hides every web purchase, because `/v1/products` carries no
`name` yet. The SDK already reads the field; they appear the moment the
backend sends it.

**Purchase history is gone, and every subscription row opens its detail
screen.** The single-subscription layout used to inline that
subscription's actions on the root; now one subscription and several are
laid out the same way, and the root keeps only the actions that apply to
the account. `showsPurchaseHistory`, `PurchaseHistoryView`,
`PurchaseDetailRows`, `historySections()` and their five strings across
41 localizations are removed; the inline cap on one-off purchases goes
with them, since there is no longer a second screen to reach the rest.

**Why the Stripe card showed an identifier** was a finding rather than a
fix: the title chain is StoreKit `displayName` → catalogue `name` →
product id, the catalogue sends no name, and the product's entitlement —
reachable via `entitlementsByProductId` — is never consulted for a
title. With the rule above, that path now yields no card instead of a
bad one.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Making every subscription a drill-down moved the per-purchase actions off the root screen, but entitlement-only purchases never get a drill-down — so the web management row the last few commits built is now unreachable for them. Three earlier threads also remain open (AppStoreVersionLookup.swift, CustomerCenterDependencies.swift, CustomerCenterConfiguration+ObjC.swift), untouched by this commit.

Reviewed changes — the delta since the prior pullfrog review at 92a2c20, a single commit (d0be8f8) across 10 files.

  • Hid purchases whose product resolved without a display name. New ProductDisplayInfo.hasDisplayName (set from whether SK1/SK2/catalogue supplied a name) and PurchasePresentationBuilder.isNameable, applied via compactMap in both subscriptionPresentations and nonSubscriptionPresentations. A product that didn't resolve at all keeps its identifier fallback; entitlement-only rows are exempt.
  • Deleted the purchase-history screen. PurchaseHistoryView.swift, PurchaseDetailRows, CustomerCenterViewModel.historySections(), ManagementScreenView.visibleOthers/inlineOthersLimit, the public CustomerCenterConfiguration.showsPurchaseHistory (property, init parameter, isEqual, hash), ManagementScreenViewTests, and five string keys across all 41 locales. Grep-verified: no dangling references.
  • Made every subscription a drill-down row. ManagementScreenView dropped the isSingle single-purchase layout; the root action list now always resolves with purchase: nil, and per-subscription actions live on PurchaseDetailScreenView.
  • Reworked the web-pricing tests. cardShowsPrice split into unnamedWebProductIsHidden (no name, no card) and a named variant asserting card.title == "Pro"; three new PurchasePresentationBuilderTests cases pin the hide rule per card. All are falsifiable against a deleted filter.
  • Updated the CHANGELOG, DocC article and design-review fixtures to describe the new shape, including a new "no display name isn't shown" limitation.

⚠️ Two of this release's CHANGELOG bullets now contradict each other

Bullet 5 still promises that the web management row "stays visible when no management URL is configured, explaining that the link is in the customer's emailed receipt, instead of disappearing and leaving them with no action at all". Bullet 4, added by this commit, says a web purchase with no catalogue display name "isn't shown at all" — and the catalogue doesn't return names yet. Taken together, on today's backend a paying web subscriber whose purchase arrives with a transaction sees an empty management screen (state is still .management, because hasAnyPurchases reads raw CustomerInfo rather than the filtered presentations), and the one whose purchase arrives as a bare entitlement sees an inert card with no action. Neither can reach .webManage or .webManageUnavailable.

Technical details
# The web management row is unreachable on today's backend

## Affected sites
- `CHANGELOG.md:12` — bullet 4 ("isn't shown at all") and `CHANGELOG.md:13` — bullet 5 ("it stays visible … instead of disappearing and leaving them with no action at all") describe mutually exclusive behaviour in the same release section.
- `Sources/SuperwallKit/Documentation.docc/CustomerCenter.md:232-234` — the limitation is documented accurately, but it is documented as a temporary catalogue gap rather than as "web subscription management is not shipping in this release".
- `Sources/SuperwallKit/CustomerCenter/ViewModel/CustomerCenterViewModel.swift:150``state = hasAnyPurchases(customerInfo) ? .management : .noPurchases` is computed from raw `CustomerInfo`, not from the filtered `purchases`, so a customer whose only purchase was hidden lands on the management screen with nothing on it rather than on the no-purchases screen.

## Required outcome
- The release's user-facing notes describe one behaviour, not two. Either bullet 5's promise is qualified, or the hide rule exempts purchases that have a reachable management action.
- A customer whose every purchase was hidden is not shown an empty "Manage subscriptions" screen.

## Open questions for the human
- Is the catalogue `name` field landing before this SDK release? If yes, this is a sequencing note and the bullets just need reconciling. If no, shipping bullet 5 as written describes a row no customer can see.
- Should `hasAnyPurchases` be computed from the filtered presentations so the no-purchases screen (which at least offers Restore and Contact support with an explanation) is shown instead of a blank management screen?

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread Sources/SuperwallKit/CustomerCenter/ViewModel/CustomerCenterDependencies.swift Outdated
Comment thread Sources/SuperwallKit/CustomerCenter/Views/PathsListView.swift Outdated
Comment thread Sources/SuperwallKit/CustomerCenter/Views/CustomerCenterStrings+English.swift Outdated
DreamingInBinary and others added 2 commits September 11, 2026 12:17
…its card

d0be8f8 read "if no name, hide the entitlement" as hiding the purchase. It
meant the label. A paying customer's subscription is never withheld from
the screen that exists to manage it; what's withheld is a title made from
a raw identifier.

So `title` is now optional on both `ProductDisplayInfo` and
`PurchasePresentation`, and the fallback chain is: the product's display
name (StoreKit's `displayName`, or the catalogue's `name`), else the
entitlement the purchase unlocks — reachable through
`entitlementsByProductId`, which is what the Stripe card was missing —
else nothing. The product identifier never appears. A product can grant
several entitlements; the lowest identifier is taken so the title is
stable between renders.

The card lays its badge beside the text column rather than on a row of
its own, so a titleless card doesn't open with an empty line. The detail
screen's bar shows no title in the same case.

The hiding rule, its `hasDisplayName` flag and the tests that pinned it
are gone; the tests now pin the fallback chain and that the card survives
every step of it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…reen back, and close three review threads

Making every subscription a drill-down (d0be8f8) split rows on
`subscription != nil`, and an entitlement-only purchase has no
`SubscriptionTransaction` — so it fell into the plain-card section, the
root stopped passing a purchase to the resolver, and the "Manage
subscription" row for a Stripe customer who arrived as a bare entitlement
became unreachable. That is the exact customer be729b8 fixed the resolver
for. Rows now split on `opensDetail`, which is true for subscriptions and
entitlement-only purchases and false for one-offs, which have no action of
their own.

Three review threads closed alongside:

- `CatalogueCache` is an actor, and actors are reentrant across `await`:
  two `apply` calls overlapping on a cold cache both passed the freshness
  check and both fetched. An in-flight task is now handed to whoever
  arrives during it; the test races two callers and counts one fetch.
- The Objective-C `url` factory put the title in the enum only, and `type`
  is `@nonobjc` — so an ObjC caller could set a title and never read it
  back. It now sets `Path.title` too.
- Seven string keys were orphaned when `PurchaseDetailRows` went, across
  English and 41 locales. Removed. A comment in `PathsListView` still named
  the deleted "See all purchases" row.

The DocC limitation stops claiming every App Store product has a display
name: StoreKit reports an empty one until the product is localized.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

opensDetail decides whether to push a detail screen from the purchase's kind, but the screen it pushes contains nothing except the paths that resolve for that purchase — and under the shipped zero-config default, several realistic shapes resolve none. Details inline on PurchasePresentation.swift:61.

Reviewed changes — the two commits since the prior pullfrog review at d0be8f8: d6dfcdb (a nameless purchase loses its title, not its card) and 95baa1c (entitlement-only purchases get their detail screen back, plus three review threads closed).

  • Reverted the hide-when-unnamed ruleProductDisplayInfo.title and PurchasePresentation.title are both String? now, hasDisplayName and PurchasePresentationBuilder.isNameable are deleted, and subscriptionPresentations/nonSubscriptionPresentations went back from compactMap to map. Nothing is filtered out of the screen any more.
  • Added an entitlement fallback for the card heading — new PurchasePresentationBuilder.entitlementTitle(_:) takes the lowest entitlement id from CustomerInfo.entitlementsByProductId, so the chain is product display name → entitlement id → nothing, and the product identifier never appears. PurchaseCardView moved the badge beside the text column so a titleless card has no empty first line.
  • Gave entitlement-only purchases a drill-down — new PurchasePresentation.opensDetail, false only for .nonSubscription, replaces ManagementScreenView's subscription != nil filter. This is the fix for the prior review's ManagementScreenView.swift:48 finding.
  • Coalesced overlapping catalogue fetchesCatalogueCache gained an inFlight: Task<…>? handoff so two callers arriving on a cold cache share one request, with a test for the overlap.
  • Populated Path.title from the Objective-C url factory, so an ObjC caller can read a URL row's label back through the @objcMembers property rather than only through the @nonobjc enum.
  • Pruned seven orphaned localization keys (purchase_date, expiration_date, transaction_id, product_id, store, sandbox, offer) from English and all 41 locale files, along with the stale no-chevron comment and the DocC limitation note.

Things I checked that need no attention: key parity is exact after the prune (65 English keys, identical sets in all 41 .lproj files); hasDisplayName/isNameable have zero remaining references; only PurchaseCardView and PurchaseDetailScreenView's navigationTitle read purchase.title, and both handle nil; .refund and .changePlan both require a SubscriptionTransaction, so opensDetail == false for one-off purchases is correct; the catalogue fetch is parameterless, so sharing the first caller's closure can't serve one caller another's request; and CatalogueCache has no suspension point between the freshness check and inFlight = task, so two callers genuinely cannot both fetch and inFlight is never left stale or poisoned by a throwing fetch.

ℹ️ An internal entitlement identifier is now a customer-facing card heading

The fallback chain replaces one raw identifier with another. Entitlement ids are developer-chosen strings — pro, premium, tier_1 — that a customer has never seen, and they render verbatim: unlocalized, uncapitalized, exactly as typed into the dashboard. The PR's own test asserts the card reads "pro". Before d6dfcdb this only affected entitlement-only rows, which have no product to be named by; now any purchase whose product the catalogue hasn't named can be headed this way, which today is every web purchase.

This is a product call rather than a defect — entitlementPresentation has always used entitlement.id, and the CHANGELOG and DocC both describe the new behaviour — so it's raised only because the commit widened who sees it.

Technical details
# Decide whether an entitlement id is a customer-facing string

## Affected sites
- `Sources/SuperwallKit/CustomerCenter/Logic/PurchasePresentationBuilder.swift:63-65``entitlementTitle(_:)` returns `entitlements.map(\.id).sorted().first` untransformed.
- `Sources/SuperwallKit/CustomerCenter/Logic/PurchasePresentationBuilder.swift:125,179` — the two
  call sites that put it on a card.
- `Sources/SuperwallKit/CustomerCenter/Logic/PurchasePresentationBuilder.swift:206``entitlementPresentation` already did this for entitlement-only rows.
- `Tests/SuperwallKitTests/CustomerCenter/Logic/PurchasePresentationBuilderTests.swift``unnamedProductFallsBackToEntitlement` pins `title == "pro"`, lowercase.

## Required outcome
- A recorded decision on whether a dashboard-side entitlement identifier is acceptable as a card
  heading, or whether a nameless purchase should show no title at all until the catalogue names it.
- If it stays, a note in `Documentation.docc/CustomerCenter.md` telling integrators their
  entitlement ids can reach customers, so they name them accordingly.

## Open questions for the human
- `CustomerInfo.entitlementsByProductId` (`CustomerInfo.swift:25`) includes inactive entitlements,
  so a lapsed subscription with an unnamed product can be headed by an entitlement the customer no
  longer holds. Intended, or should the fallback filter on `isActive`?

ℹ️ Nitpicks

  • Sources/SuperwallKit/CustomerCenter/ViewModel/CustomerCenterDependencies.swift:237Task { try await fetch() } is unstructured, so it no longer inherits the caller's cancellation the way the plain try await fetch() it replaced did. A Customer Center dismissed mid-fetch keeps the request alive until withCatalogueTimeout's 5s fires instead of unwinding at once. Bounded, so not a hang, but it's a behaviour change the comment above doesn't mention.
  • Tests/SuperwallKitTests/CustomerCenter/Logic/CatalogueCacheTests.swift:72overlappingCallersShareOneFetch depends on the first async let child reaching the actor before the second. Nothing guarantees that dispatch order, and in the reverse interleaving the second closure (no sleep) completes and caches before the first arrives, so the assertion holds against the un-coalesced implementation too. Sequencing the two explicitly — let the first signal from inside its closure that it is in flight, then start the second — would make the test discriminate in every interleaving.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Every subscription row opens its detail screen — that is the rule, and it
holds whether or not there is anything left to do. But the screen's only
content below the card was the resolved actions, and under the shipped
default configuration several realistic purchases resolve to none: a
revoked App Store subscription, a lapsed web one, anything from the Play
Store or another store, a comped grant with no management page. Each
opened onto an "Actions" heading over an empty list.

The view model now answers `hasActions(for:)` — the same resolver call the
list makes — and the detail screen shows the actions when there are any
and a single line saying there is nothing to manage when there aren't.
The row still opens; what changed is what it opens onto.

Tests drive that decision from `.default` rather than a helper that
pre-sets a support email, which is what had made the resolver tests read
as non-empty for these shapes. The new string is added to English and all
41 locales in English, to keep key parity; it needs translating.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The new "nothing to manage" line is reachable for a subscription the customer is currently being billed for: a live Play Store (or .other/.custom) subscription drills into a screen whose card reads "Active — Renews on …" and whose body says there is nothing to manage. Details inline on ManagementScreenView.swift:86.

Reviewed changes — the single commit since the prior pullfrog review at 95baa1c: 207663b, which answers that review's opensDetail finding by giving the detail screen something to say instead of narrowing which rows push.

  • Gave the purchase detail screen an empty statePurchaseDetailScreenView now renders the "Actions" section only when something resolves, and otherwise a secondary-styled explanation carrying accessibilityIdentifier("customer_center.detail.nothing_to_manage"). The purchase card above it is unchanged, so the screen is never blank.
  • Added CustomerCenterViewModel.hasActions(for:)!paths(for: purchase, isScreenLevel: false).isEmpty, the same call with the same arguments that PathsListView makes one line below, so the gate and the list cannot disagree. paths is pure and allocation-cheap, so evaluating it twice per body pass is free.
  • Added customer_center_detail_nothing_to_manage — to englishStrings and to all 41 .lproj bundles, keeping CustomerCenterStringsTests.allLocalesComplete green.
  • Added PurchaseDetailActionsTests — six @MainActor cases driven off the shipped .default configuration: four shapes that resolve to nothing (revoked App Store, lapsed web, .playStore/.other, comped grant with no management page) and two that don't (active App Store, active web with no management URL, which still gets the receipt-link row). Each #requires the row exists first, so a shape that stopped reaching a card would fail rather than silently pass.

ℹ️ Nitpicks

  • Tests/SuperwallKitTests/CustomerCenter/Views/DesignReviewSnapshots.swift:465,630,655 — the design-review fixtures render PurchaseDetailScreenView for the multi-subscription, mixed-store and two-group shapes, but none for a purchase with no resolvable actions. This commit adds a new customer-visible screen state that the PNG set a designer looks at won't contain.
  • Sources/SuperwallKit/Resources/Localizations/*/Localizable.strings — the new key is appended below each file's /* Customer Center – support */ comment block, though it belongs to the detail screen rather than support. Harmless at runtime; it just makes the next key that genuinely is a support string land in the wrong place.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread Sources/SuperwallKit/CustomerCenter/Views/ManagementScreenView.swift Outdated
Comment thread Sources/SuperwallKit/Resources/Localizations/de.lproj/Localizable.strings Outdated
…o manage

207663b gave the detail screen an empty state keyed on "no actions
resolved". That sentence — "There's nothing to manage for this purchase"
— was reachable for a subscription the customer is actively being billed
for: a live Play Store subscription on an iOS client drills into a screen
whose card reads "Active, renews on …" and whose body says there is
nothing to manage. The resolver allowlists App Store and the web stores
before it ever checks liveness, so no configuration could change that.

The empty state now branches on the purchase, not on emptiness alone. A
live, non-revoked subscription from a store this SDK can't drive is told
where to manage it — by name for the Play Store, generically for `.other`
and `.custom`, whose shared label is "Other" and would have read "manage
this subscription through Other" — and only a purchase with genuinely
nothing left to do gets the original sentence. That is the
`webManageUnavailable` precedent: when there is nowhere to send them, say
where to look.

The decision lives on the view model (`detailEmptyState(for:)`), in an
extension so the main file stays under the length limit, and the tests
now assert the reason rather than mere emptiness — the previous
`otherStoreHasNoActions` case had pinned the bug.

The three strings carry first-draft translations in all 40 non-English
locales rather than the English sentence, matching the other 65 keys.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Both [!IMPORTANT] threads from the last review are closed cleanly — one small copy accuracy point inline.

Reviewed changes — one new commit since the 207663b review, c3424db, which replaces the detail screen's single empty-state sentence with a reason-aware one and translates the three keys involved.

  • Empty state branches on the purchase, not on emptiness — new CustomerCenterViewModel+PurchaseDetail.swift carries hasActions(for:) (moved out of CustomerCenterViewModel.swift) plus DetailEmptyState and detailEmptyState(for:); a live, non-revoked purchase from a store the SDK can't drive is now told where to manage it instead of that there is nothing to manage.
  • Store naming split.playStore fills customer_center_detail_managed_through with its localized label; .other and .custom fall to customer_center_detail_managed_where_bought rather than reading "through Other".
  • PurchaseDetailScreenView renders the reasonManagementScreenView.swift:103-113 maps the three cases onto their strings, with the actions section now on the else branch.
  • Localization completed — the two new keys plus the previously-English customer_center_detail_nothing_to_manage now carry translated values in all 41 bundles.
  • Tests assert the reason, not emptinessPurchaseDetailActionsTests compares against exact DetailEmptyState values across eight shapes, including the live-vs-lapsed Play Store pair that distinguishes the two sentences.

Verified independently: ProductStore has exactly seven cases and isDrivable splits them exhaustively; the only live drivable purchase that resolves to no actions is a comped grant with no management page (CustomerCenterPathResolver.swift:100), for which "nothing to manage" is true, while a live web-store entitlement still gets .webManageUnavailable (resolver:103). Locale checks were mechanical: key sets identical to en in all 41 bundles, exactly one %@ in every _managed_through value, and no English sentence surviving outside en*.

ℹ️ Nitpicks

  • ManagementScreenView.swift:88accessibilityIdentifier("customer_center.detail.nothing_to_manage") now labels the .managedElsewhere text too, so the screen's one UI-test hook names the wrong state for two of the three sentences. Nothing reads it yet, so renaming it is free.
  • badge != .revoked is the only term of the new predicate no test exercises: it changes the answer solely for a revoked-and-active non-drivable subscription, and isRevoked/isActive are independently decoded (SubscriptionTransaction.swift:139-140), so the shape is representable. One more argument in the existing parameterized style would pin it.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

…e elsewhere

c3424db routed every live, non-revoked purchase from a store this SDK
can't drive to "manage this subscription through …". A lifetime
entitlement is live and never revoked, so a lifetime Play Store unlock
viewed on iOS read "Lifetime" on the card and "Manage this subscription
through Google Play" beneath it — pointing at a renewal that doesn't
exist.

Nothing renews, so there is nothing to manage anywhere: the lifetime badge
now lands on the original sentence, the same guard the resolver already
applies to change-plan. Test pins the shape by badge before asserting the
outcome.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues. The lifetime-grant thread from the last review is closed cleanly; approval is held only by the two older threads that are still open.

Reviewed changes — the single commit since the prior pullfrog review at c3424db: 80b1535, two files, 17 added / 1 removed.

  • Excluded a lifetime badge from isLivedetailEmptyState(for:) (CustomerCenterViewModel+PurchaseDetail.swift:40) now routes an active, non-revoked lifetime grant from a store the SDK can't drive to .nothingToDo instead of .managedElsewhere, mirroring the purchase?.badge != .lifetime guard the resolver already applies to change-plan.
  • Pinned the shape with lifetimePlayStoreGrant — the new case asserts purchase.badge == .lifetime before asserting the empty state, so it cannot pass on a fixture that never reaches the branch, and it fails against the old two-term predicate.

I traced the blast radius rather than taking the one-line change on faith. PurchaseBadge.lifetime is produced only by entitlementPresentation (PurchasePresentationBuilder.swift:209) — badge(for:) has no lifetime case — so the new term touches entitlement-only rows and nothing else. For a drivable store the !isDrivable guard already returned .nothingToDo, and a live .stripe/.paddle/.superwall entitlement carrying a store resolves .webManageUnavailable (CustomerCenterPathResolver.swift:103) so hasActions short-circuits before the predicate runs. The entire behaviour delta is the .playStore/.other/.custom lifetime case the thread described, and .playStore genuinely resolves no paths (CustomerCenterPathResolver.swift:82), so the row does reach the empty state.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants