fix(swift-example-app): load document price on Purchase sheet appear#3947
Conversation
The production document Purchase flow was blocked: the on-chain price never loaded, so "Purchase / Broadcast" stayed permanently disabled. `DocumentWithPriceView` fetched the price only via the TextField's `onChange(of: documentId)`. `PurchaseDocumentView` seeds `documentIdField` as the binding mounts (and disables the field), so `onChange` never fires and the user can't trigger it by editing -> `fetchDocument()` never ran, `documentPrice` stayed nil, and submit was gated off forever. Fix: trigger the fetch from `.onAppear` when the id is already populated (`.disabled(true)` blocks hit-testing, not lifecycle events, so it fires in the Purchase flow). The editable TransitionInputView path starts empty, so this is a no-op there and `onChange` still handles typing. Also restore the relaxed Purchase gating so the action surfaces for a for-sale, tradeable doc whenever the wallet holds a controlled identity other than the owner (buyer != owner) -- matching the file's own doc-comment. Both of these were fixed once on the #3945 branch (588435aff4) but the v3.1-dev merge (96cba16) dropped that commit, so the bug recurred. Verified end-to-end on the iOS simulator (testnet): created a card doc owned by one identity, set price 50000 credits, opened Purchase as a second controlled identity (price loaded + button enabled), broadcast the purchase, and confirmed ZOWNERID flipped to the buyer (rev 2->3, $price cleared). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
QuantumExplorer
requested review from
ZocoLini,
llbartekll and
shumkov
as code owners
|
Warning Review limit reached
More reviews will be available in 50 minutes and 56 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTwo fixes in the SwiftExampleApp: ChangesDocument Pricing and Action Menu Fixes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Review complete (commit ed85f55) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DocumentsView.swift`:
- Around line 278-299: The tradeable variable definition uses the comparison
(docType?.tradeMode ?? 0) > 0, but other marketplace gating logic throughout the
codebase defines tradeable as tradeMode == 1. Change the comparison operator
from > 0 to == 1 in the tradeable variable assignment to align with the existing
contract semantics and prevent setPrice and purchase actions from being exposed
for unintended trade modes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24d6053b-7625-4206-891c-87ab89bef166
📒 Files selected for processing (2)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DocumentWithPriceView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DocumentsView.swift
Align the Purchase/Set Price gating with the DirectPurchase mode (`tradeMode == 1`) used elsewhere for marketplace operations (TransitionInputView), instead of the looser `> 0`, so the actions are not exposed for any future non-DirectPurchase trade mode. Functionally identical today (only modes 0/1 exist). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Narrow SwiftExampleApp fix to load price on Purchase sheet appear and to surface Purchase for two-identities-in-one-wallet flow. One in-scope consistency suggestion: the new shared tradeable predicate uses > 0 while the rest of the marketplace UI uses == 1. One optional hardening suggestion on the onAppear fetch path.
🟡 1 suggestion(s) | 💬 1 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DocumentsView.swift`:
- [SUGGESTION] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DocumentsView.swift:278: Align `tradeable` predicate with the rest of the marketplace UI (`== 1`)
This PR introduces `let tradeable = (docType?.tradeMode ?? 0) > 0` as the shared predicate gating both Set Price and Purchase. The existing marketplace entry points in `TransitionInputView.swift` (lines 273/276 and 409/410) restrict tradeable types to `tradeMode == 1` (commented as "tradeable document types (tradeMode = 1)"). DPP currently treats `0` as not tradeable and `1` as direct purchase; if persisted state ever carries another positive value (protocol skew or a future non-direct-purchase mode), this menu will offer Set Price / Purchase that route to the direct-purchase broadcast path while the rest of the app hides them. Match the existing predicate so the production menu and the broadcast flow stay aligned.
| .onAppear { | ||
| // `onChange(of: documentId)` does NOT fire when the id is | ||
| // established as the binding mounts — e.g. PurchaseDocumentView | ||
| // seeds `documentIdField` in its own `onAppear` and disables this | ||
| // view, so relying on onChange alone the price probe would never | ||
| // run and Purchase would stay gated off forever. Kick the fetch | ||
| // here so a pre-seeded id loads its price without a user edit. | ||
| // `.disabled(true)` only blocks hit-testing, not lifecycle events, | ||
| // so this still fires in the Purchase flow. When the field starts | ||
| // empty (the editable transition flow) this is a no-op and the | ||
| // existing onChange path continues to handle typing. | ||
| if !documentId.isEmpty { | ||
| handleDocumentIdChange(documentId) | ||
| } | ||
| } |
There was a problem hiding this comment.
💬 Nitpick: Guard onAppear fetch to avoid re-probing an already-loaded document
onAppear unconditionally calls handleDocumentIdChange(documentId), which clears documentExists, documentPrice, fetchedDocument, transitionState.canPurchaseDocument, and transitionState.documentPrice before re-arming a 500ms debounce. If the host sheet re-appears (sheet detent change, return from a nested presentation, parent remount), the Purchase button will briefly disable and the price will flash to nil. Guarding with if !documentId.isEmpty && fetchedDocument == nil && !isLoading preserves the pre-seeded fetch fix while avoiding redundant re-probes and transient state stomps.
source: ['claude']
There was a problem hiding this comment.
Declining the fetchedDocument == nil guard: it would regress the submit button. PurchaseDocumentView gates Purchase / Broadcast on the app-wide transitionState.documentPrice, which its own .onAppear clears to nil — the unconditional re-probe here is precisely what re-publishes that value (via fetchDocument's MainActor.run). With the guard, a sheet re-appear would skip the re-probe and leave transitionState.documentPrice stuck at nil → button permanently disabled, the exact bug this PR fixes. The transient flash is the acceptable cost of keeping the gating correct without a larger refactor. The clean way to also kill the flash is to gate submit on local @State instead of the shared transitionState (the approach in the dropped 588435aff4) — left as a follow-up to keep this PR's diff minimal.
🤖 Addressed by Claude Code
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Narrow SwiftExampleApp UI fix. The only delta since the prior reviewed commit (da9e68e) is a clarifying comment above the tradeable predicate explaining that tradeMode == 1 (DirectPurchase) matches TransitionInputView's marketplace gating. Both reviewer agents found no in-scope issues, and verification of the file confirms the change is correct, idempotent, and scoped to UI behavior — no consensus, SDK, or FFI surface is touched.
|
Correction to my automated review at Latest delta (
Prior finding reconciliation
I also removed an incorrect auto-resolved reply on that still-valid |
2 participants
Issue being fixed or feature implemented
The production document Purchase flow in SwiftExampleApp was blocked: the on-chain price never loaded, so the Purchase / Broadcast button stayed permanently disabled.
DocumentWithPriceViewfetched the price only via the TextField'sonChange(of: documentId).PurchaseDocumentViewseedsdocumentIdFieldas the binding mounts (and the field is.disabled(true)), soonChangenever fires and the user can't trigger it by editing →fetchDocument()never ran,transitionState.documentPricestayednil, and submit was gated off forever. Confirmed via Rust logs that zero GetDocuments queries fired while the Purchase sheet was open.This is a merge regression: the same bug (plus the Purchase gating and a submit-button gating fix) was already fixed on the #3945 branch in commit
588435aff4, but the v3.1-dev merge (96cba166ba) dropped that commit, so it recurred.What was done?
DocumentWithPriceView: added an.onAppearthat triggers the fetch when the id is already populated at mount..disabled(true)blocks hit-testing, not lifecycle events, so it fires in the Purchase flow. The editableTransitionInputViewpath starts empty → guarded no-op, andonChangestill handles typing there.DocumentsView: restored the relaxed Purchase gating so the action surfaces for a for-sale, tradeable doc whenever the wallet holds a controlled identity other than the owner (buyer ≠ owner) — matching the file's own surviving doc-comment. Without this, the Purchase sheet is unreachable for the two-identities-in-one-wallet scenario.How Has This Been Tested?
Driven end-to-end on the iOS simulator (QA-iPhone16Pro, testnet), contract
Contract with card(5jpKat9U82PGcfmeYm8QZZWX6q7zDo2C32PZMxHpbiGB):J52PSRr4…)GLXNTysahyLnUKPS1hzji…, rev 1$price:50000envSngUJ…)ZOWNERIDflip (SwiftData)J52PSRr4…→envSngUJ…, rev 2→3,$priceclearedThe displayed price comes only from
fetchDocument's network read (sdk.documentGet), andPurchaseDocumentView.onAppearhad just nil'd the shared price — so its appearance is direct proof the probe now fires on appear.Breaking Changes
None. Example-app UI only; no consensus, SDK, or FFI changes.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
New Features