Skip to content

feat: Gift card UI improvements and token expiration handling#739

Merged
bfoss765 merged 39 commits intofeature/piggycardsfrom
feature/PC-show-gift-card
Nov 26, 2025
Merged

feat: Gift card UI improvements and token expiration handling#739
bfoss765 merged 39 commits intofeature/piggycardsfrom
feature/PC-show-gift-card

Conversation

@bfoss765
Copy link
Copy Markdown
Contributor

@bfoss765 bfoss765 commented Nov 22, 2025

Summary

This PR implements several improvements to the gift card purchase flow, focusing on UI consistency, barcode handling, and PiggyCards token management.

Changes

1. Gift Card Details UI Updates

  • Fixed spacing between labels to match Figma designs (20px consistent spacing)
  • Added proper bottom padding to containers for improved visual balance
  • Applied to both barcode and claim link variants

2. Payment ID Capture Bug Fix

  • Issue: Payment ID could be lost if payment failed before assignment
  • Fix: Capture giftCardId = response.paymentId before async payment call
  • Impact: Prevents data loss during payment failures

3. PiggyCards Barcode Implementation

  • Implemented Android parity for barcode handling
  • URL Parameter Extraction: Primary method - extract barcode values from URL query parameters (faster, more reliable)
  • Vision Framework Scanning: Fallback - download and scan barcode images to detect format
  • Multi-Format Support: CODE_128, QR_CODE, PDF_417, EAN_13, etc.
  • Benefits: Faster performance, simulator compatible, avoids Neural Engine requirements

Files Added:

  • BarcodeScanner.swift - Core barcode utility with URL extraction and Vision scanning

Files Modified:

  • GiftCardDetailsViewModel.swift - Integrated barcode scanner for PiggyCards flow

4. Discount Formatting Improvements

  • Smart Precision: 1 decimal place for discounts < 1% (e.g., "0.5%"), whole numbers for >= 1% (e.g., "4%")
  • Locations Updated:
    • Merchant details screen (POIDetailsView.swift)
    • Purchase screen (DashSpendPayViewModel.swift)
  • Fixed Issue: Abercrombie & Fitch showing "0%" instead of "0.5%"

5. PiggyCards Token Expiration Handling

  • Critical Difference: Token validation only triggers when user clicks Buy button (NOT on screen load like CTX)
  • Flow: Check token → Refresh if needed → Show payment screen OR show session expired dialog
  • Logout: Automatically logs user out of PiggyCards if token refresh fails
  • Build Safety: All PiggyCards code wrapped in #if PIGGYCARDS_ENABLED

Files Modified:

  • POIDetailsViewController.swift - Added tryRefreshPiggyCardsToken() method and Buy button validation

6. Documentation Updates

  • Added comprehensive documentation for token expiration timing differences
  • Documented barcode URL parameter extraction pattern
  • Added smart discount formatting guidelines
  • Included Git branch tracking configuration for Xcode

Testing Notes

Token Expiration Testing

  1. Sign in to PiggyCards account
  2. Manually expire token (or wait for expiration)
  3. Navigate to merchant details screen (should NOT trigger token check)
  4. Click Buy button (should trigger token validation)
  5. Verify session expired dialog appears
  6. Verify user is logged out

Barcode Testing

  1. Purchase gift card from PiggyCards merchant
  2. Wait for order fulfillment
  3. Verify barcode appears with correct format
  4. Check logs for "Using extracted value from URL" message (optimal path)

Discount Display Testing

  • Verify Abercrombie & Fitch shows "0.5%" (not "0%")
  • Verify larger discounts show as whole numbers (e.g., "10%")

Key Implementation Details

Token Validation Timing

Provider Timing Method Impact
CTX Screen load tryRefreshCtxToken() Proactive validation
PiggyCards Buy button click tryRefreshPiggyCardsToken() On-demand validation

Barcode Processing Priority

  1. URL Parameter Extraction (fastest) - Check ?text=, ?data=, ?code=, ?barcode= parameters
  2. Image Download & Scan (fallback) - Use Vision framework to detect format

Related Issues

  • Fixes discount display for merchants with < 1% savings
  • Implements Android barcode parity for PiggyCards
  • Prevents payment ID loss during failed transactions
  • Adds proper token lifecycle management for PiggyCards

Checklist

  • UI matches Figma designs
  • Barcode implementation matches Android
  • Token expiration handled correctly
  • Discount formatting works for all ranges
  • Code wrapped in conditional compilation flags
  • Documentation updated with lessons learned
  • No CTX behavior modified

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • PiggyCards integrated as an additional gift‑card provider with end‑to‑end ordering, token handling, caching, and UI flows
    • Barcode scanning and claim‑link handling for gift cards
    • New gift‑card ordering models and order status handling
  • Bug Fixes

    • Improved loading states, error handling, polling behavior, and transaction metadata updates
    • Safer authentication gating for payment flows
  • Documentation

    • Expanded operational and troubleshooting docs for translations, integrations, and payment flows
  • Chores

    • Build config and database schema/migration updates; .gitignore entries adjusted

✏️ Tip: You can customize this high-level summary in your review settings.

bfoss765 and others added 30 commits October 7, 2025 14:49
…nd paymentId

Changed GiftCardResponse to be more resilient to API changes:
- Required fields: id, status, paymentId (critical for transaction flow)
- Optional fields: All other fields including paymentCryptoNetwork, percentDiscount, rate

This prevents JSON decoding failures when CTX API doesn't return all expected fields,
which was causing issues in the purchase gift card flow.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Version Update:
- Updated MARKETING_VERSION from 8.4.2 to 8.4.3 across all build configurations

CTX JSON Parsing Fix:
- Made GiftCardResponse more resilient to API changes
- Required fields: id, status, paymentId (critical for transaction flow)
- Optional fields: All other fields including paymentCryptoNetwork, percentDiscount, rate
- Prevents JSON decoding failures when CTX API doesn't return all expected fields

Documentation Updates:
- Added git commit/push policy to CLAUDE.md
- Added detailed git workflow guidelines to AI-DEVELOPMENT-GUIDE.md
- Ensures AI assistants wait for explicit permission before committing/pushing

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…hants

- Fetch real-time merchant availability from CTX API getMerchant endpoint
- Display "Temporarily unavailable" and disable buy button when merchant.enabled is false
- Make getMerchant endpoint public (no authentication required)
- Fix login error handling by adding complete error cases to CTXSpendAPI.request()
- Clean up error handling in CTXSpendRepository and CTXSpendTokenService
- Add "Temporarily unavailable" localization string

Fixes issue where users couldn't see which merchants were disabled before attempting purchase.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Update all remaining MARKETING_VERSION entries to 8.4.3 (8 targets)
- Make providerStatusText provider-aware to fix "Temporarily unavailable" labels
- Remove 30+ debug print statements from gift card purchase flow
- Update CLAUDE.md with version management best practices

Changes ensure consistent versioning across all targets and improve
provider-specific status handling in POIDetailsView.

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix CLAUDE.md verification command with correct grep logic for MARKETING_VERSION checks
- Replace print statements with DSLogger.log in CustomIconMetadataProvider for proper logging
- Fix locale-sensitive number formatting using NumberFormatter with en_US_POSIX locale

Changes ensure proper version verification, consistent logging patterns, and
locale-invariant decimal formatting to prevent rounding issues.

Co-Authored-By: Claude <noreply@anthropic.com>
- Updated clang-format build scripts to exclude DWUpholdMainnetConstants.m
- This file was getting unwanted whitespace changes on every build
- Updated CLAUDE.md documentation to explain the root cause
- The file will no longer be modified by the automatic formatting scripts

Co-Authored-By: Claude <noreply@anthropic.com>
…innet

- Removed cached kBaseURL constant that was preventing network switching
- baseURL property now computes URL dynamically based on current network
- This fixes the issue where switching from testnet to mainnet (or vice versa) required app restart
- Updated CLAUDE.md with network switching documentation and best practices

The issue occurred because the API endpoint was cached at initialization time.
Now it correctly evaluates the network on each request, ensuring:
- Testnet uses: http://staging.spend.ctx.com/
- Mainnet uses: https://spend.ctx.com/

Co-Authored-By: Claude <noreply@anthropic.com>
When PIGGYCARDS_ENABLED is not defined, the merchant list was incorrectly
showing combined discounts (e.g., 14% for Domino's) instead of CTX-only
discounts (6%). This fix:

- Adds calculateEffectiveDiscount() method to extract CTX-only discounts
- Properly converts basis points to percentages (600 -> 6%)
- Filters out PiggyCards discounts when the feature flag is disabled
- Ensures merchant list shows the same discount as the details view

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add 'X-Client-Id: dcg_ios' header to all CTX API requests for proper
client identification and tracking. This header helps CTX identify
traffic from the iOS Dash Wallet application.

While production doesn't strictly require this header, it's recommended
for analytics and may be required for staging environment access.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix staging URL to use HTTPS instead of HTTP
- Add authentication to getMerchant endpoint for staging compatibility
- Handle dual field names (savingsPercentage/userDiscount) for API compatibility
- Fix provider filtering to exclude PiggyCards data when feature is disabled
- Use provider denomination type instead of merchant table for accurate data
- Add comprehensive documentation for CTX/DashSpend API integration

These changes ensure the app works correctly with both staging and production
CTX APIs despite their structural differences.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add MCP configuration section to CLAUDE.md
- Add MCP-CONFIGURATION-GUIDE.md with detailed setup instructions
- Add verify-mcp.sh script to test MCP server connectivity
- Document Figma Dev Mode MCP server setup for design extraction
- Add new Spend shortcut type that navigates to Where to Spend feature
- Update shortcut bar to show 4 different states based on balance and passphrase verification:
  1. Zero balance + Not verified: Backup, Receive, Buy & Sell, Spend
  2. Zero balance + Verified: Receive, Send, Buy & Sell, Spend
  3. Has balance + Verified: Receive, Send, Scan, Spend
  4. Has balance + Not verified: Backup, Receive, Send, Spend
- Replace PNG icons with SVG for better scaling (Spend and Scan buttons)
- Update Scan button with new QR code design from Figma
- Bump version to 8.4.3
- Document SVG usage and shortcut implementation in CLAUDE.md

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Fixed documentation comment in CTXConstants.swift to correctly show https:// URL for staging
- Removed redundant nil initialization in DashSpendPayViewModel.swift

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Removed 100+ debug logging statements that were added during development:
- Removed all DSLogger.log statements with 🔍 debug markers
- Cleaned up debug output from CTXConstants, CTXSpendRepository, POIDetailsViewModel, and DashSpendPayViewModel
- Retained only essential error logging for production use

This cleanup improves performance and reduces console noise in production builds.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Added "Spend" entry to Localizable.strings so the new Spend shortcut button
can be translated into other languages via Transifex.

The ShortcutAction.swift already uses NSLocalizedString("Spend", ...) so it will
automatically pick up the translations once they're provided.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…rized commits

Updated both CLAUDE.md and AI-DEVELOPMENT-GUIDE.md with much stronger and clearer
language about the critical requirement to NEVER commit or push without explicit
user permission.

Changes include:
- Added prominent visual markers (emojis, bold text) to catch attention
- Declared this the #1 rule and #1 user complaint
- Provided concrete examples of violations vs correct behavior
- Listed specific phrases that grant permission
- Made it absolutely clear there are NO EXCEPTIONS to this rule

This addresses the recurring issue where AI assistants commit changes
without waiting for user permission.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Unchecked the "Run Script - clang-format" build phase in Xcode
- Prevents clang-format from running during builds
- Stops the recurring issue with DWUpholdMainnetConstants.m being modified

This change was made through Xcode's UI to disable the formatting script.
- Also unchecked "Run Script - clang-format" for the dashpay target
- Both dashwallet and dashpay targets now have clang-format disabled
- Ensures the script doesn't run regardless of which scheme is built

This fully prevents DWUpholdMainnetConstants.m from being modified during builds.
feat: implement new shortcut bar design with Spend button
Fixed critical issues with PiggyCards gift card integration:

1. Fixed missing sourceId in MerchantDAO SQL queries
   - Added sourceId column to SELECT statements in 3 locations
   - Properly handle sourceId as String, Int64, or Int
   - Without sourceId, PiggyCards API calls were failing

2. Fixed denomination type handling using API as source of truth
   - API response now correctly overrides database values when signed in
   - CTX and PiggyCards can have different denomination types for same merchant
   - Fixed state clearing when switching between fixed/flexible types

3. Fixed provider data isolation
   - Clear min/max values when switching to fixed denominations
   - Clear denominations array when switching to flexible amounts
   - Prevents cross-contamination between CTX and PiggyCards data

4. Enhanced debugging and error handling
   - Added comprehensive logging with 🎯 emoji markers
   - Log state BEFORE and AFTER API calls
   - Special handling for Domino's to prefer fixed price cards
   - Validate min/max values from API responses

5. Updated CLAUDE.md documentation
   - Added PiggyCards integration section with critical lessons learned
   - Documented sourceId database architecture requirements
   - Added debugging strategies and common pitfalls

Expected behavior after fix:
- AutoZone: Shows keyboard with $10-$200 range for PiggyCards
- Domino's: Shows $25 fixed denomination button for PiggyCards
- Macy's: Shows keyboard with $5-$500 range for PiggyCards

🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
When POIDetailsViewController refreshed merchant info from the CTX API,
it was creating a new merchant object but forgetting to copy the
giftCardProviders array. This caused the sourceId to be lost, preventing
PiggyCards API calls from working correctly.

Changes:
- Added giftCardProviders parameter to updatingMerchant() to preserve
  the providers array populated by MerchantDAO
- Added debug logging to MerchantDAO to trace provider queries

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Added comprehensive documentation for adding test merchants to the explore database, including:
- Complete SQL scripts with FTS trigger handling
- PiggyCards test merchant specifications (merchant ID: 2e393eee-4508-47fe-954d-66209333fc96)
- Verification and cleanup procedures
- Best practices for test data management

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Added critical documentation about handling UTF-16 encoded translation files
downloaded from Transifex. Includes:
- Explanation of the encoding difference between source and translated files
- Examples of correct vs incorrect grep usage
- Complete bash script for searching translations regardless of encoding
- Note that Xcode/iOS handle both encodings transparently

This prevents confusion when command-line tools fail to find translations
that are actually present but in UTF-16 encoding.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Pull latest translations including:
- Initial translations for "Spend" button text in German, Greek, and Spanish
- General translation updates for 20+ languages
- Updated Japanese App Store description

Translation status for "Spend" shortcut:
- 🇩🇪 German: "Ausgegeben"
- 🇬🇷 Greek: "Ξοδέψτε"
- 🇪🇸 Spanish: "Gastar"
- Other languages: pending translation

Note: Some translation files use UTF-16 encoding (standard for iOS localization)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Added tx binary to .gitignore to prevent accidental commit of the
locally installed Transifex CLI tool.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…ider support

Major improvements:
- Added PIN authorization for all PiggyCards transactions
- Implemented provider column in database to distinguish CTX vs PiggyCards
- Fixed transaction icons to properly show gift card purchases

Authentication & Security:
- Modified SendCoinsService to explicitly request PIN before signing
- Added @mainactor authentication wrapper to ensure UI operations on main thread
- Fixed authentication crash by properly handling async operations

Database & Migration:
- Added provider column to gift_cards table via migration
- Created AddProviderToGiftCardsTable migration
- Updated GiftCard model and DAO to handle provider field
- Made database operations defensive to handle missing columns gracefully

PiggyCards Features:
- Implemented dedicated PiggyCards polling (60 seconds max, 1.5s intervals)
- Added order status checking for PiggyCards API
- Support for both barcode and claim link redemption types
- Fixed case sensitivity issues in API field names

Gift Card Improvements:
- Updated GiftCardDetailsViewModel to route to correct provider API
- Enhanced GiftCardMetadataProvider to observe gift card changes in real-time
- Gift cards now immediately show with orange icon in transaction history
- Fixed barcode generation from claim codes

API & Models:
- Fixed all PiggyCards API field mappings to camelCase
- Made API response fields optional to handle variations
- Removed verbose logging to prevent console overflow
- Temporarily bypassed exchange rate for testing

Bug Fixes:
- Resolved crash when opening app due to missing provider column
- Fixed gift card selection algorithm for denomination matching
- Corrected date formatting for PiggyCards API
- Fixed unnecessary await warnings in transaction signing

🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
bfoss765 and others added 7 commits November 20, 2025 13:28
- Add database migration to automatically insert PiggyCards test merchant
- Migration only runs in DEBUG or TESTFLIGHT builds
- Handles FTS triggers properly to avoid SQLite errors
- Checks for existing merchant to avoid duplicates
- Adds both merchant and gift_card_providers records
- Must be removed before production release

Test merchant details:
- Name: Piggy Cards Test Merchant
- ID: 2e393eee-4508-47fe-954d-66209333fc96
- Source ID: 177
- 10% discount
- Fixed denomination type

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove Amazon test merchant from TestFlight builds
- Remove special handling logic for test merchants
- Keep only PiggyCards test merchant without special treatment
- Test merchant now uses standard merchant flow and real API
- Clean up all debug messages and special test logic
- Update version to 8.5.0 across all targets

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Move giftCardId assignment before the async payment call to ensure
the payment ID is stored even if the payment fails or throws an error.
Previously, if payWithDashUrl() failed, the giftCardId would never be
set, preventing proper error tracking and recovery.
- Add BarcodeScanner utility to download and scan barcodes from URLs
- Update PiggyCards to use Vision framework for multi-format barcode detection
- Match Android implementation: scan barcode images instead of hardcoding CODE_128
- Fix discount display to show 1 decimal for values < 1% (e.g., 0.5%)
- Apply smart formatting to both merchant details and purchase screens
- Maintain whole number display for discounts >= 1% (e.g., 4%, 10%)
- Validate PiggyCards token when user clicks Buy button (not on screen load like CTX)
- Show session expired dialog if token refresh fails
- Automatically log user out of PiggyCards on token expiration
- CTX token handling remains unchanged (still checks on screen load)
- All PiggyCards code wrapped in PIGGYCARDS_ENABLED flag for build compatibility

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Document PiggyCards token expiration timing (Buy button vs screen load)
- Add barcode URL parameter extraction pattern for performance
- Document smart discount formatting based on value magnitude
- Add Git branch tracking configuration for Xcode display
- Include comprehensive examples and common pitfalls

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 22, 2025

Walkthrough

Adds PiggyCards integration: database migration adding a provider column, model and API additions for PiggyCards orders, token refresh actor, in-memory PiggyCards cache, barcode scanner utility, UI changes for purchase/details/pay flows, test merchant setup, and assorted config/documentation edits. No public API removals.

Changes

Cohort / File(s) Summary
Configuration & Docs
\.gitignore, CLAUDE.md, DashWallet.xcodeproj/xcshareddata/xcschemes/dashwallet.xcscheme
.gitignore updated (SQLite temp files, tx), extensive CLAUDE.md docs added/updated (MCP, UTF‑16, PiggyCards, barcode, token handling), scheme changed Launch/Archive buildConfiguration to Testflight.
DB Migrations
DashWallet/Sources/Infrastructure/Database/DatabaseConnection.swift, .../Migrations/AddProviderToGiftCardsTable.swift
Added AddProviderToGiftCardsTable migration and included it in migrations list; adds nullable provider column to GiftCards table (version 20251120150000).
Core Models
.../Entites/GiftCard.swift, .../Entites/ExplorePointOfUse.swift
Added optional provider: String? to GiftCard (SQLite expression, constructor/row decoding) and added sourceId: String? to GiftCardProviderInfo with updated initializer.
PiggyCards Models & Constants
.../DashSpend/PiggyCardsConstants.swift, .../Entites/PiggyCardsModels.swift
Introduced production baseURI, stagingBaseURI, token/fee/polling constants; added comprehensive PiggyCards request/response models, brands, giftcard catalog, exchange rate, and domain enums/types.
PiggyCards Services
.../PiggyCardsAPI.swift, .../PiggyCardsEndpoint.swift, .../PiggyCardsRepository.swift, .../PiggyCardsCache.swift, .../PiggyCardsTokenService.swift
New endpoints (brands, giftcards, orders, status, exchange-rate); orderGiftCard replaces legacy purchase flow and returns GiftCardInfo; in-memory PiggyCardsCache with selection logic; actor-based token refresh with proactive expiry checks and deduplication.
Barcode Scanning
DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift
New Vision-based BarcodeScanner with URL parameter extraction, image download + scan, multi-format support, and async error handling.
DAO / Persistence
.../DAO Impl/MerchantDAO.swift, .../DAO/GiftCardsDAO.swift
MerchantDAO queries extended to select/handle sourceId (String/Int variants) and parameterized queries; GiftCardsDAO persists provider through create and update flows.
Test Merchant Setup
.../ExploreDatabaseConnection.swift
Conditional DEBUG/Testflight test merchant insertion (addPiggyCardsTestMerchant) after sync; manages FTS triggers and inserts piggycards provider rows for local testing.
Transaction Flow
.../SendCoinsService.swift
Added @MainActor authenticate gate (biometric/PIN) before signing; replaced not-enough-funds path with descriptive DashSpendError.paymentProcessingError.
Service Name
.../ServiceName.swift
Added piggyCards = "piggycards" enum case.
UI — Gift Card Details
.../DashSpend/GiftCardDetailsView.swift, .../GiftCardDetailsViewModel.swift
Support for claim links, barcode download/scan fallback, long-polling (maxRetries→40, threshold→40), provider-aware fetch (PiggyCards vs CTX), added UI state flags and provider propagation.
UI — Payment / Pay Screen
.../DashSpend/DashSpendPayScreen.swift, .../DashSpendPayViewModel.swift
Loading state for merchant info, provider-specific purchase flow (CTX vs PiggyCards using orderGiftCard), provider-aware denominations, cost messaging with adaptive discount formatting, markGiftCardTransaction now accepts provider.
UI — Merchant Details & Providers
.../POIDetailsViewController.swift, .../POIDetailsView.swift, .../POIDetailsViewModel.swift
Propagated giftCardProviders, added PiggyCards token refresh attempt in buy flow, introduced sortedProviders deterministic ordering and centralized discount formatting helper.
UI — Merchant List
.../MerchantItemCell.swift, .../AllMerchantLocationsDataProvider.swift, .../AllMerchantLocationsViewController.swift
Use max discount from giftCardProviders for display; removed debug prints.
Metadata & Observers
.../Home/Tx Metadata/GiftCardMetadataProvider.swift
Observe gift card DAO updates, update TxRowMetadata per gift card and notify observers on main thread.
Utilities
DashWallet/Sources/Utils/PercentageFormatter.swift
New PercentageFormatter to format percentages/basis points with smart decimal precision.
Localization / Fastlane
fastlane/metadata/ja/description.txt
Minor Japanese text/spacing adjustments.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant UI as GiftCardDetailsView
    participant VM as GiftCardDetailsViewModel
    participant PiggyRepo as PiggyCardsRepository
    participant Scanner as BarcodeScanner

    User->>UI: Open gift card (provider = PiggyCards)
    UI->>VM: loadGiftCard()
    VM->>VM: detect provider == PiggyCards
    VM->>PiggyRepo: getOrderStatus(orderId)
    PiggyRepo-->>VM: order status

    alt status == complete
        VM->>Scanner: downloadAndScan(barcodeLink)
        alt barcode in URL
            Scanner-->>VM: BarcodeResult
        else image scanned
            Scanner-->>VM: BarcodeResult
        else scan failed
            VM->>VM: derive barcode from claimCode
        end
        VM->>UI: update UI with barcode/claimLink
    else status failed/rejected
        VM->>UI: show error
    else pending
        VM->>VM: schedule retry (up to 40)
    end
Loading
sequenceDiagram
    autonumber
    actor User
    participant UI as DashSpendPayScreen
    participant VM as DashSpendPayViewModel
    participant PiggyRepo as PiggyCardsRepository
    participant Tx as SendCoinsService
    participant DAO as GiftCardsDAO

    User->>UI: confirm purchase (amount)
    UI->>VM: purchaseGiftCard()
    VM->>VM: determine provider = PiggyCards
    VM->>PiggyRepo: orderGiftCard(merchantId, amount)
    PiggyRepo-->>VM: GiftCardInfo(orderId, paymentAddress)
    VM->>Tx: authenticate() `@MainActor`
    Tx-->>VM: authenticated
    VM->>Tx: sendCoins(to: paymentAddress)
    Tx-->>VM: tx signed & published
    VM->>DAO: persist GiftCard(provider="PiggyCards", txId/orderId)
    VM->>UI: navigate to GiftCardDetailsView (start polling)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Areas to重点 review:

  • PiggyCardsTokenService (actor, deduplication, expiry boundaries)
  • PiggyCardsRepository.orderGiftCard() end-to-end flow, error handling, polling
  • GiftCardDetailsViewModel long-polling, barcode scan fallback, state transitions
  • DashSpendPayViewModel provider branching and transaction persistence with provider metadata
  • BarcodeScanner network/image/Vision error handling and threading
  • DB migration AddProviderToGiftCardsTable and DAO sourceId type handling

Possibly related PRs

  • feat: UI improvements #732 — Appears to overlap the PiggyCards/DashSpend workstreams and modify the same models, repositories, and UI surfaces.

Poem

🐰 I hopped in code to add a card,

PiggyCards and CTX now play their part,
Barcodes scanned, orders chase the sun,
Tokens refresh so buys can run,
Merchants smile — a rabbit’s clever start.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 32.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main focus of the changeset: UI improvements and token expiration handling for gift cards, which aligns with the substantial changes across gift card views, barcode handling, discount formatting, and PiggyCards token validation.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/PC-show-gift-card

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 13

🧹 Nitpick comments (16)
DashWallet/Sources/Models/Transactions/SendCoinsService.swift (1)

64-85: Auth gate before signing looks correct; address SwiftLint warnings and minor style

The main-thread auth gate via @MainActor and withCheckedContinuation looks sound and ensures UI auth runs on the main actor before signing. To satisfy SwiftLint and reduce noise:

  • Put the @MainActor attribute on its own line (Line 66 warning).
  • Replace the unused usedBiometrics closure parameter with _ (Line 72 warning).

Suggested diff:

-        @MainActor func authenticate() async -> Bool {
+        @MainActor
+        func authenticate() async -> Bool {
             return await withCheckedContinuation { continuation in
                 DSAuthenticationManager.sharedInstance().authenticate(
                     withPrompt: nil,
                     usingBiometricAuthentication: DWGlobalOptions.sharedInstance().biometricAuthEnabled,
                     alertIfLockout: true
-                ) { authenticatedOrSuccess, usedBiometrics, cancelled in
+                ) { authenticatedOrSuccess, _, cancelled in
                     continuation.resume(returning: authenticatedOrSuccess && !cancelled)
                 }
             }
         }

If this pattern will be reused elsewhere, consider extracting authenticate() into a private @MainActor helper on SendCoinsService rather than a local function, but that’s optional.

DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift (1)

162-207: Simplify withCheckedContinuation usage and constrain Vision request symbologies (optional)

The withCheckedContinuation + DispatchQueue.global + resumed flag works, but it’s a bit defensive/complex for a synchronous VNImageRequestHandler.perform(_:) call, and you’re scanning all supported symbologies.

[alternate approach suggestion]

Two optional cleanups:

  1. Limit symbologies for performance and fewer false positives (e.g., to formats you actually support in BarcodeFormat and expect for PiggyCards):
- let request = VNDetectBarcodesRequest { request, error in
+ let request = VNDetectBarcodesRequest { request, error in
    ...
 }
+ request.symbologies = [
+     .code128, .qr, .pdf417, .ean13, .ean8, .upca, .upce
+ ]
  1. Remove the resumed flag and extra GCD hop, relying on perform’s synchronous semantics and a single completion path:
private static func scanBarcode(from cgImage: CGImage) async -> BarcodeResult? {
-    return await withCheckedContinuation { continuation in
-        DispatchQueue.global(qos: .userInitiated).async {
-            var resumed = false  // Prevent double-resumption
-
-            let request = VNDetectBarcodesRequest { request, error in
-                guard !resumed else { return }
-                resumed = true
-                ...
-            }
-            let handler = VNImageRequestHandler(cgImage: cgImage, options: [:])
-            do {
-                try handler.perform([request])
-            } catch {
-                guard !resumed else { return }
-                resumed = true
-                ...
-            }
-        }
-    }
+    return await withCheckedContinuation { continuation in
+        let request = VNDetectBarcodesRequest { request, error in
+            if let error = error {
+                DSLogger.log("🔍 BarcodeScanner: Vision error: \(error.localizedDescription)")
+                continuation.resume(returning: nil)
+                return
+            }
+            guard let observations = request.results as? [VNBarcodeObservation],
+                  let firstBarcode = observations.first,
+                  let payloadString = firstBarcode.payloadStringValue else {
+                DSLogger.log("🔍 BarcodeScanner: No barcode found in image")
+                continuation.resume(returning: nil)
+                return
+            }
+            let format = BarcodeFormat.from(symbology: firstBarcode.symbology)
+            let result = BarcodeResult(value: payloadString, format: format)
+            DSLogger.log("🔍 BarcodeScanner: Successfully scanned barcode - Format: \(format.rawValue)")
+            continuation.resume(returning: result)
+        }
+
+        let handler = VNImageRequestHandler(cgImage: cgImage, options: [:])
+        do {
+            try handler.perform([request])
+        } catch {
+            DSLogger.log("🔍 BarcodeScanner: Failed to perform Vision request: \(error.localizedDescription)")
+            continuation.resume(returning: nil)
+        }
+    }
}

Please verify against the Vision API guarantees for your minimum iOS version; adjust symbologies as needed.

DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift (1)

58-76: Provider field is correctly persisted and propagated, but consider reducing duplication

  • The create(dto:) insert now includes both note and provider, and the cache is updated with the same dto, so provider will be stored and observable correctly.
  • Both updateCardDetails and updateBarcode rebuild GiftCard instances while preserving existingCard.provider (and note where appropriate), which avoids accidentally dropping the provider on partial updates.

Two small follow‑ups you might consider:

  • The manual GiftCard reconstruction is duplicated and touches many fields; a helper (e.g. an initializer or copyWith(...)-style method) would reduce the risk of missing new fields in future.
  • SwiftLint’s vertical_parameter_alignment_on_call warnings at the .update(...) calls can be silenced by aligning the parameters vertically.

Also applies to: 105-135, 137-166

DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsTokenService.swift (1)

88-123: Avoid force unwrap of refreshTask in TokenRefreshActor

The force unwrap at line 122 is unnecessary and can be eliminated. Use a local task variable to keep a non-optional reference:

         // Start new refresh task
-        refreshTask = Task {
+        let task = Task {
             defer {
                 refreshTask = nil
                 isRefreshing = false
             }
 
             isRefreshing = true
             let success = try await performRefresh()
 
             if success {
                 DSLogger.log("PiggyCards: Token refresh completed successfully")
             } else {
                 DSLogger.log("PiggyCards: Token refresh failed")
                 throw DashSpendError.tokenRefreshFailed
             }
 
             return success
         }
-
-        _ = try await refreshTask!.value
+        refreshTask = task
+        _ = try await task.value

Optionally, remove the isRefreshing variable (lines 91, 106, 109) since it's never read and only toggled for tracking.

DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsView.swift (3)

149-249: Claim-link and card number/PIN UI are solid; a couple of small cleanups are possible.

Functionally this section behaves well (claim-link button, copy actions, error display). You could optionally:

  • Drop the redundant !viewModel.uiState.isClaimLink check inside a branch that already excludes claim-link.
  • Consider separating claimLink from cardNumber in the view model for clearer intent.

332-352: Provider footer works; consider centralizing the provider identifier.

Conditionally showing PiggyCards vs CTX branding based on provider == "PiggyCards" is fine, but using a string literal here and elsewhere can drift over time. If this spreads, it may be worth introducing a small provider enum or shared constants for "PiggyCards"/"CTX" and mapping that to the logo.


355-368: Address SwiftLint’s multiple_closures_with_trailing_closure around NavigationLink.

The NavigationLink here passes more than one closure argument while also using a trailing closure for the label, which triggers the SwiftLint warning. You can silence it by using explicit label: { ... } syntax instead of a trailing closure.

DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsCache.swift (1)

82-126: Gift card selection priority is clear; watch for exact Double equality.

The disabled-card filtering and fixed/range/option priority logic mirror the documented behavior. If amount ever comes from calculations (vs a directly selected denomination string), strict Double equality checks (Double(card.denomination) == amount) could occasionally miss near-equal values; if that shows up in practice, consider switching to Decimal or introducing a small tolerance.

DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsEndpoint.swift (1)

25-37: New PiggyCards endpoints are wired consistently; consider pruning the invalid legacy case.

The added cases, auth mapping, paths, and tasks (including brandId/currency query params and createOrder body) align with the repository usage and look correct. Given getGiftCard is now explicitly commented as “Doesn't exist”, it may be safer long‑term to remove or clearly deprecate that case to avoid accidental calls to a non‑existent endpoint.

Based on learnings

Also applies to: 41-50, 56-79, 81-103

CLAUDE.md (1)

1588-1595: Minor markdownlint nits: fenced language and bare URL

To keep markdownlint happy and make the docs a bit clearer, consider:

  • Adding a language to the short fenced block showing ~/Library/.../explore.db (e.g., bash or text).
  • Optionally wrapping any bare URLs in angle brackets or markdown links to avoid MD034 (“bare URL”) warnings.

Purely cosmetic; no functional impact.

DashWallet/Sources/Models/Explore Dash/Model/Entites/PiggyCardsModels.swift (5)

110-123: Clarify or retire the legacy PiggyCardsPurchaseRequest

This struct is explicitly marked “Legacy purchase request (may need to be removed/replaced)”. If it’s no longer used in the new order-based flow, consider removing it (and its callsites) to reduce confusion. If it’s still needed for some endpoint, adding a short comment about which API path still requires it would help future readers.

A quick search for PiggyCardsPurchaseRequest usages in the codebase would confirm whether it can be safely deleted.


137-189: Order response and gift-card status models: good structure, consider optionality

The PiggyCardsOrderResponse, PiggyCardsOrderStatusResponse, PiggyCardsOrderData, and PiggyCardsOrderGiftCard structs line up with the documented shapes (camelCase CodingKeys for payTo, payMessage, orderId, deliveryTime, claimCode, barcodeLink, etc.), and the PiggyCardsOrderStatusGiftCardStatus mapping is straightforward.

Two small robustness considerations:

  • If the API can ever omit data or cards (e.g., on error or before fulfillment), making data and/or cards optional would avoid decode failures on partial responses.
  • cardStatus is a String; if the set of values is stable, a small enum (or reuse of GiftCardStatus) would give stronger typing like you did for PiggyCardsOrderStatus.

If the server contract guarantees data and cards are always present, the current non-optional modeling is fine.

Would recommend confirming a couple of real /order and order-status responses to see if data/cards can be missing in error or intermediate states.


200-236: Gift card models are coherent; think about money precision and raw value semantics

The PiggyCardsGiftcardResponse and PiggyCardsGiftcard types look consistent, and the snake_case CodingKeys (price_type, discount_percentage, min_denomination, brand_id) match the comments.

A few minor points to consider:

  • discountPercentage is documented as a decimal fraction (0.15 = 15%); that’s compatible with the discount formatting logic elsewhere, but it would be helpful to make sure all callsites treat it as a fraction (not “percent already multiplied by 100”).
  • Amount-like fields (denomination, minDenomination, maxDenomination) are Double. If you start seeing rounding artefacts (e.g., 0.009999), switching these to Decimal in the future would be safer for money, though it’s not strictly required for this PR.

Structurally this all looks fine for now.

When you wire this up end-to-end, please sanity-check with a few live PiggyCards products that the decoded values (min/max, denomination, discount) exactly match the dashboard/API.


254-280: Old PiggyCardsMerchantResponse likely obsolete—consider deprecation or removal

The comment above this struct notes that “gift cards are retrieved via order status”, which suggests this earlier merchant-centric model may no longer be used in the new flow. To avoid confusion:

  • If unused, remove the type and its decoding path.
  • If still used for some legacy endpoint, add a brief note (e.g., “Used only by X endpoint”) and possibly mark it as deprecated.

This will keep future PiggyCards work focused on the order/giftcard models above.

A quick search for PiggyCardsMerchantResponse callsites will tell whether it can be safely deleted or should be clearly marked as legacy.


284-329: Domain models and enums are clear; consider stronger typing and trimming redundant raw values

  • GiftCardInfo is a useful aggregation for UI, but status is a plain String even though you define GiftCardStatus and PiggyCardsOrderStatus. If possible, using GiftCardStatus directly (and deriving it from PiggyCardsOrderStatus) would catch invalid states at compile time instead of at runtime.
  • PiggyCardsOrderStatus.giftCardStatus mapping looks correct and matches the raw string values, assuming the backend uses exactly those phrases.
  • For PiggyCardsPriceType, the explicit raw values ("fixed", "range", "option") are redundant since they match the case names; SwiftLint flagged this. You can simplify to:
    enum PiggyCardsPriceType: String {
        case fixed
        case range
        case option
    }
    without changing behavior.

These are all low-impact tidy-ups; the current implementation is functionally fine.

If you decide to switch GiftCardInfo.status to GiftCardStatus, double-check all serialization/bridging points so you don’t rely on the raw string anywhere important.

DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift (1)

252-256: Nit: remove blank line before catch to satisfy SwiftLint

SwiftLint is flagging vertical_whitespace_closing_braces for the empty line between the success print and the catch. You can fix it by tightening that up:

-            print("✅ PiggyCards test merchant added successfully")
-
-        } catch {
+            print("✅ PiggyCards test merchant added successfully")
+        } catch {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6892a21 and 033b1f5.

⛔ Files ignored due to path filters (1)
  • Podfile.lock is excluded by !**/*.lock
📒 Files selected for processing (32)
  • .gitignore (1 hunks)
  • CLAUDE.md (4 hunks)
  • DashWallet.xcodeproj/xcshareddata/xcschemes/dashwallet.xcscheme (2 hunks)
  • DashWallet/Sources/Infrastructure/Database/DatabaseConnection.swift (1 hunks)
  • DashWallet/Sources/Infrastructure/Database/Migrations/AddProviderToGiftCardsTable.swift (1 hunks)
  • DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift (7 hunks)
  • DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift (5 hunks)
  • DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift (3 hunks)
  • DashWallet/Sources/Models/Explore Dash/Model/DashSpend/PiggyCardsConstants.swift (1 hunks)
  • DashWallet/Sources/Models/Explore Dash/Model/Entites/ExplorePointOfUse.swift (1 hunks)
  • DashWallet/Sources/Models/Explore Dash/Model/Entites/GiftCard.swift (4 hunks)
  • DashWallet/Sources/Models/Explore Dash/Model/Entites/PiggyCardsModels.swift (3 hunks)
  • DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift (1 hunks)
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsAPI.swift (2 hunks)
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsCache.swift (1 hunks)
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsEndpoint.swift (4 hunks)
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (1 hunks)
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsTokenService.swift (2 hunks)
  • DashWallet/Sources/Models/Transactions/SendCoinsService.swift (2 hunks)
  • DashWallet/Sources/Models/Tx Metadata/ServiceName.swift (1 hunks)
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift (3 hunks)
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift (4 hunks)
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swift (3 hunks)
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocationsViewController.swift (0 hunks)
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/MerchantItemCell.swift (1 hunks)
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Model/AllMerchantLocationsDataProvider.swift (0 hunks)
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayScreen.swift (2 hunks)
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (6 hunks)
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsView.swift (4 hunks)
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (4 hunks)
  • DashWallet/Sources/UI/Home/Tx Metadata/GiftCardMetadataProvider.swift (3 hunks)
  • fastlane/metadata/ja/description.txt (1 hunks)
💤 Files with no reviewable changes (2)
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Model/AllMerchantLocationsDataProvider.swift
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocationsViewController.swift
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift:21-23
Timestamp: 2025-09-27T15:36:10.803Z
Learning: In PiggyCardsRepository.swift, there's an ACL mismatch (public static let shared in an internal class) and potential thread safety concerns that will be addressed during dedicated PiggyCards development rather than in the current UI improvements PR.
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsEndpoint.swift:25-32
Timestamp: 2025-09-27T23:33:40.458Z
Learning: PiggyCards API getGiftCard endpoint uses an invalid path "gift-cards/\(txid)" that doesn't exist on the server (marked with TODO comment). The correct path should likely be "orders" with query parameters following the CTX API pattern, since PiggyCards uses "orders" for purchases while CTX uses "gift-cards" for both purchase and retrieval operations.
📚 Learning: 2025-09-27T15:36:10.803Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift:21-23
Timestamp: 2025-09-27T15:36:10.803Z
Learning: In PiggyCardsRepository.swift, there's an ACL mismatch (public static let shared in an internal class) and potential thread safety concerns that will be addressed during dedicated PiggyCards development rather than in the current UI improvements PR.

Applied to files:

  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsView.swift
  • DashWallet/Sources/Models/Explore Dash/Model/DashSpend/PiggyCardsConstants.swift
  • DashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsCache.swift
  • DashWallet/Sources/UI/Home/Tx Metadata/GiftCardMetadataProvider.swift
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsAPI.swift
  • DashWallet/Sources/Models/Tx Metadata/ServiceName.swift
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsEndpoint.swift
  • DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsTokenService.swift
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift
  • DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayScreen.swift
  • DashWallet/Sources/Models/Explore Dash/Model/Entites/PiggyCardsModels.swift
📚 Learning: 2025-09-12T21:24:36.691Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 729
File: DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Model/AllMerchantLocationsDataProvider.swift:89-89
Timestamp: 2025-09-12T21:24:36.691Z
Learning: The pointOfUseId computed property on ExplorePointOfUse acts as an abstraction that returns the appropriate identifier based on the category - for merchants it returns merchantId, making direct calls with pointOfUseId equivalent to using merchantId for merchant entries.

Applied to files:

  • DashWallet/Sources/Models/Explore Dash/Model/Entites/ExplorePointOfUse.swift
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift
📚 Learning: 2025-09-05T04:46:12.717Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 728
File: DashWallet.xcodeproj/project.pbxproj:1595-1599
Timestamp: 2025-09-05T04:46:12.717Z
Learning: In iOS projects, the `DashWallet.xcodeproj/project.pbxproj` file is automatically generated and managed by Xcode. Manual changes to this file should not be made, and the changes shown in diffs are typically the result of Xcode updating project configuration, dependencies, or build settings.

Applied to files:

  • DashWallet.xcodeproj/xcshareddata/xcschemes/dashwallet.xcscheme
📚 Learning: 2025-08-25T21:01:26.493Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 723
File: DashWallet/da.lproj/Localizable.strings:2987-2987
Timestamp: 2025-08-25T21:01:26.493Z
Learning: The DashWallet project uses Transifex for translation management rather than direct manual edits to .lproj/Localizable.strings files.

Applied to files:

  • DashWallet.xcodeproj/xcshareddata/xcschemes/dashwallet.xcscheme
  • CLAUDE.md
📚 Learning: 2025-09-05T04:46:21.894Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 728
File: DashWallet.xcodeproj/project.pbxproj:3174-3180
Timestamp: 2025-09-05T04:46:21.894Z
Learning: Xcode project files (.pbxproj) are automatically generated by Xcode and should not be manually edited. Hash changes in Pod library references (like libPods-dashwallet.a) are normal and occur when CocoaPods updates or when Xcode regenerates the project structure.

Applied to files:

  • DashWallet.xcodeproj/xcshareddata/xcschemes/dashwallet.xcscheme
📚 Learning: 2025-09-27T23:33:40.458Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsEndpoint.swift:25-32
Timestamp: 2025-09-27T23:33:40.458Z
Learning: PiggyCards API getGiftCard endpoint uses an invalid path "gift-cards/\(txid)" that doesn't exist on the server (marked with TODO comment). The correct path should likely be "orders" with query parameters following the CTX API pattern, since PiggyCards uses "orders" for purchases while CTX uses "gift-cards" for both purchase and retrieval operations.

Applied to files:

  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsEndpoint.swift
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift
  • DashWallet/Sources/Models/Explore Dash/Model/Entites/PiggyCardsModels.swift
🧬 Code graph analysis (11)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/MerchantItemCell.swift (1)
DashWallet/Sources/Models/Explore Dash/Model/Entites/ExplorePointOfUse.swift (1)
  • toSavingPercentages (103-105)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (7)
DashWallet/Sources/Models/Transactions/SendCoinsService.swift (2)
  • payWithDashUrl (98-114)
  • sendCoins (27-94)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (3)
  • orderGiftCard (186-266)
  • getMerchant (418-421)
  • getGiftCards (340-362)
DashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift (1)
  • updateIcon (126-179)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)
  • getMerchant (204-228)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsCache.swift (1)
  • getGiftCards (48-52)
DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift (1)
  • create (58-79)
DashWallet/Sources/Models/Tx Metadata/DAO/TransactionMetadataDAO.swift (1)
  • create (57-82)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (3)
DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift (2)
  • from (39-66)
  • downloadAndScan (100-127)
DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift (3)
  • get (81-94)
  • updateCardDetails (105-135)
  • updateBarcode (137-166)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (1)
  • getOrderStatus (302-312)
DashWallet/Sources/Models/Explore Dash/Model/Entites/GiftCard.swift (1)
DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift (1)
  • get (81-94)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsCache.swift (1)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (2)
  • getGiftCards (340-362)
  • getExchangeRate (315-328)
DashWallet/Sources/UI/Home/Tx Metadata/GiftCardMetadataProvider.swift (1)
DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift (1)
  • observeAll (97-99)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsAPI.swift (1)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendAPI.swift (4)
  • request (24-45)
  • request (47-68)
  • handleUnauthorizedError (79-90)
  • checkAccessTokenIfNeeded (92-100)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsEndpoint.swift (3)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (5)
  • getBrands (331-337)
  • getGiftCards (340-362)
  • getOrderStatus (302-312)
  • getExchangeRate (315-328)
  • getMerchant (418-421)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsCache.swift (2)
  • getGiftCards (48-52)
  • getExchangeRate (68-72)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsAPI.swift (2)
  • request (22-30)
  • request (32-48)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsTokenService.swift (3)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (3)
  • load (265-284)
  • refreshToken (129-131)
  • save (252-263)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendTokenService.swift (1)
  • refreshAccessToken (41-82)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsAPI.swift (2)
  • request (22-30)
  • request (32-48)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (2)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsCache.swift (5)
  • getGiftCards (48-52)
  • selectGiftCard (84-126)
  • getExchangeRate (68-72)
  • storeExchangeRate (62-66)
  • storeGiftCards (42-46)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsAPI.swift (2)
  • request (22-30)
  • request (32-48)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift (1)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swift (1)
  • logout (115-117)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md

1571-1571: Bare URL used

(MD034, no-bare-urls)


1589-1589: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🪛 SwiftLint (0.57.0)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsView.swift

[Warning] 157-157: Trailing closure syntax should not be used when passing more than one closure argument

(multiple_closures_with_trailing_closure)


[Warning] 205-205: Trailing closure syntax should not be used when passing more than one closure argument

(multiple_closures_with_trailing_closure)


[Warning] 235-235: Trailing closure syntax should not be used when passing more than one closure argument

(multiple_closures_with_trailing_closure)

DashWallet/Sources/Models/Transactions/SendCoinsService.swift

[Warning] 66-66: Attributes should be on their own lines in functions and types, but on the same line as variables and imports

(attributes)


[Warning] 72-72: Unused parameter in a closure should be replaced with _

(unused_closure_parameter)

DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift

[Warning] 36-36: Initializing an optional variable with nil is redundant

(redundant_optional_initialization)


[Warning] 187-187: Prefer != nil over let _ =

(unused_optional_binding)


[Warning] 210-210: Prefer != nil over let _ =

(unused_optional_binding)


[Warning] 268-268: Function should have complexity 10 or less; currently complexity is 11

(cyclomatic_complexity)


[Warning] 284-284: Don't include vertical whitespace (empty line) after opening braces

(vertical_whitespace_opening_braces)

DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift

[Warning] 69-69: Function parameters should be aligned vertically if they're in multiple lines in a method call

(vertical_parameter_alignment_on_call)


[Warning] 70-70: Function parameters should be aligned vertically if they're in multiple lines in a method call

(vertical_parameter_alignment_on_call)

DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsTokenService.swift

[Warning] 122-122: Force unwrapping should be avoided

(force_unwrapping)

DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift

[Warning] 186-186: Function should have complexity 10 or less; currently complexity is 13

(cyclomatic_complexity)


[Warning] 269-269: Function parameters should be aligned vertically if they're in multiple lines in a declaration

(vertical_parameter_alignment)


[Warning] 270-270: Function parameters should be aligned vertically if they're in multiple lines in a declaration

(vertical_parameter_alignment)


[Warning] 271-271: Function parameters should be aligned vertically if they're in multiple lines in a declaration

(vertical_parameter_alignment)


[Warning] 187-187: Don't include vertical whitespace (empty line) after opening braces

(vertical_whitespace_opening_braces)


[Warning] 272-272: Don't include vertical whitespace (empty line) after opening braces

(vertical_whitespace_opening_braces)

DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift

[Error] 160-160: Prefer checking isEmpty over comparing count to zero

(empty_count)


[Warning] 253-253: Don't include vertical whitespace (empty line) before closing braces

(vertical_whitespace_closing_braces)

DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayScreen.swift

[Warning] 291-291: Don't include vertical whitespace (empty line) after opening braces

(vertical_whitespace_opening_braces)

DashWallet/Sources/Models/Explore Dash/Model/Entites/PiggyCardsModels.swift

[Warning] 131-131: String enum values can be omitted when they are equal to the enumcase name

(redundant_string_enum_value)


[Warning] 326-326: String enum values can be omitted when they are equal to the enumcase name

(redundant_string_enum_value)


[Warning] 327-327: String enum values can be omitted when they are equal to the enumcase name

(redundant_string_enum_value)


[Warning] 328-328: String enum values can be omitted when they are equal to the enumcase name

(redundant_string_enum_value)

🔇 Additional comments (27)
fastlane/metadata/ja/description.txt (1)

13-13: Excellent localization choice.

The change from the English loanword "Face ID" to the native Japanese term "顔認証" (facial authentication) is a good improvement for the Japanese audience. It's more appropriate terminology and better reflects the wallet's capabilities beyond just Apple's proprietary biometric method.

DashWallet/Sources/Models/Transactions/SendCoinsService.swift (1)

45-52: Insufficient-funds error now uses DashSpendError.paymentProcessingError with context

The new error includes balance, amount, and fee, which is helpful for debugging and user messaging. Please just confirm that any callers that previously relied on a more specific error type or message pattern still behave correctly (e.g., mapping to UI alerts or analytics).

.gitignore (1)

51-56: SQLite and tx ignore patterns are appropriate

Ignoring SQLite temp files and the Transifex CLI binary is correct and helps keep the repo clean; no issues here.

DashWallet/Sources/Models/Tx Metadata/ServiceName.swift (1)

26-27: piggyCards service name addition looks consistent

Adding case piggyCards = "piggycards" cleanly extends the enum for PiggyCards without affecting existing cases; naming and raw value style match the rest.

DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayScreen.swift (1)

127-140: LGTM: Loading state prevents UI flicker.

The loading state logic is well-structured and mutually exclusive with the fixed-denomination path. This prevents the UI from flickering when merchant info is being fetched.

DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift (1)

287-287: LGTM: Using sortedProviders ensures consistent ordering.

The use of sortedProviders ensures providers are displayed in the correct order (highest discount first, with PiggyCards as tiebreaker).

DashWallet/Sources/Infrastructure/Database/DatabaseConnection.swift (1)

73-78: LGTM: Migration addition is correct.

The new migration is properly added to the end of the migrations list, maintaining the correct sequential order.

DashWallet/Sources/Models/Explore Dash/Model/Entites/ExplorePointOfUse.swift (1)

42-48: LGTM: Optional property addition maintains backward compatibility.

The sourceId property is correctly added with an optional type and default value, ensuring existing code continues to work without modification.

DashWallet/Sources/Models/Explore Dash/Model/DashSpend/PiggyCardsConstants.swift (1)

25-28: LGTM: Configuration constants are well-documented.

The new constants have clear purposes:

  • 1-hour token expiration is standard
  • 1.5% service fee is reasonable
  • 250ms polling delay prevents excessive API calls
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swift (1)

162-197: LGTM: Provider sorting logic is correct and well-structured.

The implementation correctly:

  1. Collects provider info with discount data
  2. Sorts by discount (highest first) with PiggyCards as tiebreaker
  3. Maintains both a dictionary for lookups and an ordered array for UI display
  4. Selects the best provider as default

This ensures users see the most advantageous option first.

DashWallet/Sources/UI/Home/Tx Metadata/GiftCardMetadataProvider.swift (2)

43-52: LGTM: Gift card observation enables real-time metadata updates.

The subscription to giftCardDao.observeAll() ensures metadata is refreshed whenever gift cards change, providing a reactive data flow that keeps the UI in sync.


89-115: LGTM: Metadata update logic is correct and thread-safe.

The implementation properly:

  1. Processes all gift cards and builds metadata
  2. Uses metadataQueue for thread-safe dictionary access
  3. Sends notifications on the main thread for UI updates
DashWallet/Sources/Infrastructure/Database/Migrations/AddProviderToGiftCardsTable.swift (1)

22-27: Migration for provider column looks correct; just verify it’s registered.

Versioning and the nullable provider column match the model. Please double‑check that AddProviderToGiftCardsTable() is included in the migrations list in DatabaseConnection so existing installs get upgraded.

DashWallet/Sources/Models/Explore Dash/Model/Entites/GiftCard.swift (1)

33-59: Provider field wiring between model and DB looks sound.

The new provider property, its SQLite.Expression<String?> definition, and the init(row:) use of try? row.get(GiftCard.provider) provide a safe, backward‑compatible way to read the column, and the extended init keeps existing call sites intact.

Also applies to: 61-73

DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsView.swift (3)

41-86: Header layout and spacing updates look consistent.

The merchant header plus 20pt horizontal/top padding align with the rest of this screen’s spacing system; no functional issues here.


89-131: Barcode vs loading vs claim-link handling is clear and robust.

The 3‑way split between claim‑link mode, barcode image, and the loading/error placeholder (with the long‑polling timeout text) reads well and should cover the main PiggyCards/CTX states without strange overlaps.


324-329: “How to use” block container changes match the rest of the card UI.

Wrapping the instructions in secondaryBackground with 20pt horizontal padding keeps this section visually aligned with the main gift card container.

DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsCache.swift (1)

23-32: Thread-safe in-memory caches look well-structured.

The singleton plus per-cache concurrent queues with barrier writes and sync reads provide a straightforward, thread-safe cache for cards and exchange rates, and clearAllCaches() correctly funnels through the individual clear methods.

Also applies to: 42-78, 130-133

CLAUDE.md (3)

44-48: Figma MCP configuration renaming looks consistent

The updated figma-dev-mode-mcp-server key and @figma/mcp-server-figma-dev-mode package name match the tool identifiers listed below and should avoid past confusion with the older claude-talk-to-figma-mcp naming. No issues from my side here.


76-80: Troubleshooting bullets correctly reflect new MCP server identifiers

The checks for the package name and server name now align with the updated configuration and expected tool IDs, which should make MCP setup failures easier to diagnose. Looks good.


205-241: UTF‑16 translation guidance is accurate and very helpful

The UTF‑16 vs UTF‑8 explanation and the iconv/file examples match how Transifex delivers .strings files and clarify why grep alone misses translations. This is a solid, actionable addition and aligns with the existing Transifex-based localization workflow.

DashWallet/Sources/Models/Explore Dash/Model/Entites/PiggyCardsModels.swift (2)

64-108: Order request models and CodingKeys look consistent with documented API

PiggyCardsOrderRequest, PiggyCardsOrder, PiggyCardsUser, and PiggyCardsUserMetadata all use camelCase property names with CodingKeys that match the comments (e.g., recipientEmail, productId, registeredSince), which should serialize correctly if the API expects camelCase JSON. Access levels (public only on the top-level request) also seem reasonable for now.

If there’s any doubt about the exact JSON field names, please double-check against the latest PiggyCards API docs or a real response to ensure the camelCase expectations (recipientEmail, productId, registeredSince) are correct.


240-250: Exchange rate CodingKeys correctly use snake_case; model shape looks fine

PiggyCardsExchangeRateResult explicitly maps exchangeRate and dateModified to exchange_rate / date_modified, which matches the comment that this endpoint uses snake_case. Using Double for a rate is acceptable here, since it’s not a user-facing currency balance.

It’s still worth validating one sample response to ensure the endpoint really returns exchange_rate and date_modified for all environments.

DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift (4)

41-46: Post‑sync DEBUG/Testflight hook for test merchant looks good

Wiring addTestMerchantAfterSync() into databaseHasBeenUpdatedNotification under #if DEBUG || Testflight is a clean way to keep the PiggyCards test merchant present only in non‑production builds. The existence check later on makes this idempotent.


71-86: Early return on missing dbPath and initial DEBUG/Testflight hook both look correct

The new guard let dbPath = dbPath() else { ... return } cleanly short‑circuits once an in‑memory DB is created, avoiding unnecessary work when the on‑disk DB hasn’t been downloaded yet. The delayed insertTestMerchant() call for DEBUG/Testflight builds is also fine, given the later existence check on merchantId prevents duplicate inserts.


120-129: No behavioral change in raw‑SQL execute helper

Only spacing changed here; the raw‑SQL execute(query: String) still bails out when db is nil and safely returns an empty array on errors. Nothing to adjust.


132-150: Test‑merchant scheduling helpers are reasonable

addTestMerchantAfterSync() and insertTestMerchant() nicely encapsulate the scheduling and DB‑ready check, and are correctly wrapped in #if DEBUG || Testflight so they don’t affect production. This looks good.

Comment thread DashWallet.xcodeproj/xcshareddata/xcschemes/dashwallet.xcscheme
Comment thread fastlane/metadata/ja/description.txt Outdated
Copy link
Copy Markdown
Contributor

@HashEngineering HashEngineering left a comment

Choose a reason for hiding this comment

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

LGTM

@bfoss765 bfoss765 changed the title PiggyCards: Gift card UI improvements and token expiration handling feat: Gift card UI improvements and token expiration handling Nov 24, 2025
Addresses coderabbit review comments:

1. Fixed isLoading state management in DashSpendPayViewModel
   - Reset isLoading flag before early returns in PiggyCards branch
   - Prevents view from getting stuck in loading state when repository or sourceId is missing

2. Fixed Japanese localization formatting
   - Added missing spaces after asterisks in bullet points (lines 13, 15, 17)
   - Ensures consistent formatting across all bullet points in description.txt

3. Improved PiggyCards token expiration handling
   - Changed from refreshAccessToken() to refreshTokenIfNeeded()
   - Only refreshes token when actually expired or expiring within 5 minutes
   - Distinguishes between authentication failures and network errors
   - Only logs out on auth failures (tokenRefreshFailed, unauthorized)
   - Shows connection error for network issues without logging out user
   - More efficient: prevents unnecessary API calls on every Buy button click

These changes improve user experience by:
- Preventing UI from freezing during error conditions
- Not logging out users due to temporary network issues
- Reducing unnecessary token refresh API calls
- Providing appropriate error messages for different failure types

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (1)

239-263: Avoid flooring PiggyCards DASH amount when converting to satoshis

This block still does UInt64(giftCardInfo.amount * 100_000_000), which floors the value and can underpay by 1+ satoshis due to Double rounding (as previously flagged). That can cause PiggyCards orders to be rejected.

Reusing the earlier suggestion, you can switch to a decimal‑aware, round‑up conversion:

-            // PiggyCards returns a simple address and amount, not a BIP70 URL
-            // Convert DASH amount to satoshis (1 DASH = 100,000,000 satoshis)
-            let dashAmountInSatoshis = UInt64(giftCardInfo.amount * 100_000_000)
+            // PiggyCards returns a simple address and amount, not a BIP70 URL
+            // Convert DASH amount to satoshis (1 DASH = 100,000,000 satoshis) using decimal rounding up
+            let dashAmountDecimal = NSDecimalNumber(value: giftCardInfo.amount)
+                .multiplying(by: NSDecimalNumber(value: 100_000_000))
+            let rounding = NSDecimalNumberHandler(
+                roundingMode: .up,
+                scale: 0,
+                raiseOnExactness: false,
+                raiseOnOverflow: true,
+                raiseOnUnderflow: true,
+                raiseOnDivideByZero: true
+            )
+            let satoshiAmount = dashAmountDecimal.rounding(accordingToBehavior: rounding)
+            let dashAmountInSatoshis = satoshiAmount.uint64Value
+            if dashAmountInSatoshis == 0 {
+                throw DashSpendError.invalidAmount
+            }

This guarantees you never underpay the amount PiggyCards expects.

🧹 Nitpick comments (4)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (3)

135-165: Double‑check denomination type casing and fallback comment

The provider‑specific init logic is generally solid (basis‑point → fraction conversion is correct and sourceId is wired up), but two small points:

  • providerIsFixed relies on providerInfo.denominationsType == DenominationType.Fixed.rawValue. If database values are stored lowercase ("fixed", "range", "option" per prior learnings), and DenominationType.Fixed.rawValue is capitalized, this comparison will never be true, so isFixedDenomination may be false until the API refreshes the merchant info. Consider normalizing case on the string side or comparing against a lowercased rawValue.
  • The comment says “Only use merchant-level denominations as fallback if we don't have provider-specific info,” but denominations are currently populated whenever merchant.merchant?.denominations != nil, regardless of whether provider info was found. If you truly want “fallback only,” you might gate this on sourceId == nil or a similar signal.

Also applies to: 169-171


347-351: PiggyCards merchant info flow looks good; remove dead loop and consider minimal cleanup

The revamped updateMerchantInfo correctly:

  • Guards on isUserSignedIn.
  • Sets/clears isLoading on all CTX and PiggyCards paths, including early returns and errors.
  • Normalizes PiggyCards priceType, applies service fee, and derives range vs fixed/option denominations.

Two small cleanups:

  • The for (index, card) in giftCards.enumerated() { } loop (Lines 399–401) is now a no‑op but still iterates all cards; it can be deleted.
  • (Optional) country: "US" is hard‑coded; if/when you add multi‑country support, this will be a natural place to thread through the user’s country or merchant country.

Also applies to: 376-487, 490-500


536-546: Gift card metadata persistence is good; simplify markGiftCardTransaction signature

Persisting giftCardId in note and storing the provider on GiftCard is useful for future provider‑specific behavior.

In markGiftCardTransaction, the extra provider providerName: String parameter is unused; the switch relies on self.provider instead. To avoid confusion:

  • Either remove the providerName parameter entirely, or
  • Change it to provider: GiftCardProvider and switch on that value instead of self.provider.

Example of removing the unused parameter:

-    private func markGiftCardTransaction(txId: Data, provider providerName: String) {
+    private func markGiftCardTransaction(txId: Data) {
@@
-        // Set the service name based on the provider
-        switch provider {
+        // Set the service name based on the provider
+        switch provider {

(And update the call site accordingly.)

Also applies to: 553-568

DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift (1)

319-327: Token refresh UX for CTX and PiggyCards is clear and consistent

  • CTX: on DashSpendError.tokenRefreshFailed, tryRefreshCtxToken now shows a “Your session expired” modal and returns false, which is appropriate for a background refresh triggered from refreshTokenAndMerchantInfo().
  • PiggyCards: tryRefreshPiggyCardsToken cleanly distinguishes:
    • Credential/authorization failures (tokenRefreshFailed / unauthorized): logout + “Your session expired” modal.
    • Transient/network failures: log and show “Connection Error” without logging out.

Only small (optional) layering nit: you’re calling PiggyCardsRepository.shared.logout() directly here; if you later centralize logout behavior (e.g., via a view model or shared repository registry), this would be a good place to route through that abstraction instead.

Also applies to: 329-355

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 033b1f5 and efae52f.

📒 Files selected for processing (3)
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift (3 hunks)
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (6 hunks)
  • fastlane/metadata/ja/description.txt (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift:21-23
Timestamp: 2025-09-27T15:36:10.803Z
Learning: In PiggyCardsRepository.swift, there's an ACL mismatch (public static let shared in an internal class) and potential thread safety concerns that will be addressed during dedicated PiggyCards development rather than in the current UI improvements PR.
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsEndpoint.swift:25-32
Timestamp: 2025-09-27T23:33:40.458Z
Learning: PiggyCards API getGiftCard endpoint uses an invalid path "gift-cards/\(txid)" that doesn't exist on the server (marked with TODO comment). The correct path should likely be "orders" with query parameters following the CTX API pattern, since PiggyCards uses "orders" for purchases while CTX uses "gift-cards" for both purchase and retrieval operations.
📚 Learning: 2025-09-27T15:36:10.803Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift:21-23
Timestamp: 2025-09-27T15:36:10.803Z
Learning: In PiggyCardsRepository.swift, there's an ACL mismatch (public static let shared in an internal class) and potential thread safety concerns that will be addressed during dedicated PiggyCards development rather than in the current UI improvements PR.

Applied to files:

  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
  • fastlane/metadata/ja/description.txt
📚 Learning: 2025-11-24T15:33:27.627Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 739
File: DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift:152-251
Timestamp: 2025-11-24T15:33:27.627Z
Learning: In ExploreDatabaseConnection.swift test merchant setup, savingsPercentage values in both merchant and gift_card_providers tables must be consistent and expressed in basis points (e.g., 1000 = 10%). The values are divided by 10000 in DashSpendPayViewModel, so both tables should use the same basis point value for the same merchant to avoid discount calculation mismatches.

Applied to files:

  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
  • fastlane/metadata/ja/description.txt
📚 Learning: 2025-09-27T23:33:40.458Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsEndpoint.swift:25-32
Timestamp: 2025-09-27T23:33:40.458Z
Learning: PiggyCards API getGiftCard endpoint uses an invalid path "gift-cards/\(txid)" that doesn't exist on the server (marked with TODO comment). The correct path should likely be "orders" with query parameters following the CTX API pattern, since PiggyCards uses "orders" for purchases while CTX uses "gift-cards" for both purchase and retrieval operations.

Applied to files:

  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-11-24T15:33:27.627Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 739
File: DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift:152-251
Timestamp: 2025-11-24T15:33:27.627Z
Learning: In ExploreDatabaseConnection.swift test merchant setup, denominationsType values must use lowercase ('fixed', 'range', 'option') in both merchant and gift_card_providers INSERT statements to match downstream filtering logic. Using uppercase values like 'Fixed' will cause merchants to fail denomination type filtering.

Applied to files:

  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
  • fastlane/metadata/ja/description.txt
📚 Learning: 2025-09-12T21:24:36.691Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 729
File: DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Model/AllMerchantLocationsDataProvider.swift:89-89
Timestamp: 2025-09-12T21:24:36.691Z
Learning: The pointOfUseId computed property on ExplorePointOfUse acts as an abstraction that returns the appropriate identifier based on the category - for merchants it returns merchantId, making direct calls with pointOfUseId equivalent to using merchantId for merchant entries.

Applied to files:

  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift
📚 Learning: 2025-09-27T15:37:44.449Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift:94-96
Timestamp: 2025-09-27T15:37:44.449Z
Learning: User bfoss765 prefers to defer fixing force unwrapping warnings until crashes are observed during testing, rather than proactively addressing them.

Applied to files:

  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-08-25T21:01:26.493Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 723
File: DashWallet/da.lproj/Localizable.strings:2987-2987
Timestamp: 2025-08-25T21:01:26.493Z
Learning: The DashWallet project uses Transifex for translation management rather than direct manual edits to .lproj/Localizable.strings files.

Applied to files:

  • fastlane/metadata/ja/description.txt
🧬 Code graph analysis (1)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift (1)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swift (1)
  • logout (115-117)
🪛 SwiftLint (0.57.0)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift

[Warning] 120-120: Function parameters should be aligned vertically if they're in multiple lines in a method call

(vertical_parameter_alignment_on_call)


[Warning] 426-426: Don't include vertical whitespace (empty line) after opening braces

(vertical_whitespace_opening_braces)

🔇 Additional comments (4)
fastlane/metadata/ja/description.txt (1)

13-13: Formatting consistency issue resolved.

The spacing after asterisks on lines 13, 15, and 17 has been corrected to match the format of other bullet points, resolving the prior review comment. All bullet points now consistently use * formatting. ✓

Also applies to: 15-15, 17-17

DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (1)

103-121: Discount percentage formatting behavior looks correct

The new discountPercent / discountText logic matches the requirements: <1% shows one decimal and ≥1% is rounded to a whole number, and the localized string uses %@%% appropriately. No functional issues here.

DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift (2)

195-214: PiggyCards buy flow now correctly gates on token refresh

The updated buyGiftCardHandler does what you described:

  • For PiggyCards (when enabled), it refreshes the token first and only presents DashSpendPayScreen if tryRefreshPiggyCardsToken() returns true; otherwise it just exits after showing the appropriate dialog.
  • For CTX (or when PiggyCards is compiled out), it falls back to showing the pay screen directly on the main actor.

Behavior looks consistent and avoids entering the PiggyCards purchase flow with an expired session.


399-413: Preserving redeemType and giftCardProviders in updatingMerchant is correct

Forwarding redeemType and giftCardProviders from currentMerchant into updatedMerchant ensures you don’t lose redeem behavior or provider metadata when refreshing denominations from CTX.

This matches the broader PiggyCards integration and keeps POI state consistent across updates.

…ging

- Add PercentageFormatter utility for consistent discount formatting
  with smart decimal precision (1 decimal for <1%, whole numbers otherwise)
- Fix PiggyCards token refresh deadlock by excluding auth endpoints
- Fix GiftCardMetadataProvider race condition with Data object capture
- Add strategic API debugging logs for gift card fetching
- Remove excessive barcode scanner debug logging

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (2)
DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift (1)

38-66: Missing UPC-A symbology mapping.

BarcodeFormat.upca is defined at line 30, but from(symbology:) doesn't include a case for VNBarcodeSymbology.upcA. UPC-A scans will incorrectly return .unknown.

         case .ean8:
             return .ean8
+        case .upcA:
+            return .upca
         case .upce:
             return .upce
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (1)

242-252: DASH amount flooring risk persists.

UInt64(giftCardInfo.amount * 100_000_000) truncates rather than rounds. Due to floating-point representation, 0.00000001 DASH could become 0.999...9 satoshis and floor to 0, causing underpayment. Use Decimal or round up to avoid payment rejections.

-            let dashAmountInSatoshis = UInt64(giftCardInfo.amount * 100_000_000)
+            let dashAmountDecimal = Decimal(giftCardInfo.amount) * Decimal(100_000_000)
+            var rounded = Decimal()
+            NSDecimalRound(&rounded, &dashAmountDecimal, 0, .up)
+            let dashAmountInSatoshis = NSDecimalNumber(decimal: rounded).uint64Value
🧹 Nitpick comments (11)
DashWallet/Sources/UI/Home/Tx Metadata/GiftCardMetadataProvider.swift (2)

92-93: Clarify comment about race condition.

The comment mentions avoiding a race condition with the Data object, but Data is a value type (struct) in Swift. Accessing giftCard.txId creates a copy, so there's no race condition risk from concurrent mutation of the same Data instance. The capture is still good practice for clarity and to ensure the correct value is used within the async block, but the reasoning in the comment could be more precise.

Consider revising the comment to:

-// Capture txId before async block to avoid race condition with Data object
+// Capture txId before async block to ensure stable reference
 let txId = giftCard.txId

89-117: Extract duplicated metadata update logic.

The updateMetadataForGiftCards(_:) method (lines 89-117) and onMetadataUpdated(metadata:) method (lines 119-144) share nearly identical logic:

  • Both format the same localized title string
  • Both check/update/create txRowMetadata with identical properties
  • Both update _availableMetadata and send notifications on the main thread

The only difference is the source of the transaction ID (giftCard.txId vs metadata.txHash). Consider extracting this common logic into a private helper method to improve maintainability and reduce duplication.

Example refactoring:

private func updateMetadata(txId: Data, merchantName: String) {
    let title = String.localizedStringWithFormat(NSLocalizedString("Gift card · %@", comment: "DashSpend"), merchantName)
    
    metadataQueue.async { [weak self] in
        guard let self = self else { return }
        var txRowMetadata = self._availableMetadata[txId]
        
        if txRowMetadata != nil {
            txRowMetadata!.title = title
            txRowMetadata!.secondaryIcon = .custom("image.explore.dash.wts.payment.gift-card")
        } else {
            txRowMetadata = TxRowMetadata(
                title: title,
                secondaryIcon: .custom("image.explore.dash.wts.payment.gift-card")
            )
        }
        
        self._availableMetadata[txId] = txRowMetadata
        
        DispatchQueue.main.async {
            self.metadataUpdated.send(txId)
        }
    }
}

private func updateMetadataForGiftCards(_ giftCards: [GiftCard]) async {
    for giftCard in giftCards {
        updateMetadata(txId: giftCard.txId, merchantName: giftCard.merchantName)
    }
}

Then onMetadataUpdated can also call updateMetadata(txId: metadata.txHash, merchantName: giftCard.merchantName).

DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsAPI.swift (2)

45-47: Redundant catch-all can be removed.

The catch { throw error } clause is unnecessary in Swift—unhandled errors propagate automatically. Consider removing it for cleaner code.

         } catch HTTPClientError.statusCode(let r) where r.statusCode == 400 {
             if target.path.contains("/verify-otp") {
                 throw DashSpendError.invalidCode
             }
             throw HTTPClientError.statusCode(r)
-        } catch {
-            throw error
         }
     }

79-82: Consider using guard let for idiomatic nil check.

Using guard let is more idiomatic Swift and avoids the separate nil check.

         default:
-            let token = PiggyCardsTokenService.shared.accessToken
-            if token == nil {
+            guard PiggyCardsTokenService.shared.accessToken != nil else {
                 throw DashSpendError.unauthorized
             }
         }
DashWallet/Sources/Utils/PercentageFormatter.swift (1)

29-39: includeSign parameter only supports negative sign.

The includeSign parameter hardcodes "-" rather than deriving the sign from the value. If the intent is to display discounts as negative values (e.g., "-10%"), this works. However, the parameter name suggests it would handle both positive and negative signs dynamically.

Consider renaming to includeNegativeSign or asNegative for clarity, or deriving the sign from the actual value if bidirectional support is needed.

-    static func format(percent: Double, includeSign: Bool = false, includePercent: Bool = true) -> String {
-        let sign = includeSign ? "-" : ""
+    static func format(percent: Double, asNegative: Bool = false, includePercent: Bool = true) -> String {
+        let sign = asNegative ? "-" : ""
DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift (1)

96-96: Minor: Remove empty line after opening brace.

Static analysis flagged vertical whitespace after the opening brace.

 class BarcodeScanner {
-
     /// Download barcode image from URL and scan it to extract value and format
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (2)

400-415: Hardcoded merchant-specific logic is a maintenance concern.

The "domino" string check creates a special case that won't scale and may break if merchant names change. Consider:

  1. Adding a comment explaining why Domino's requires different handling
  2. Moving this logic to a configuration or the merchant data model

149-150: Empty else block can be removed.

The else { } block at lines 149-150 serves no purpose and should be removed.

                     break
                 }
             }
-        } else {
         }
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (2)

140-143: Consider more robust URL validation.

The current check number.starts(with: "http") is case-sensitive and will miss "HTTP" or "Http" variants. Consider using URL validation for more reliable claim link detection.

Apply this diff to improve URL detection:

-            // Check if this is a claim link (URL in number field)
-            if let number = card.number, number.starts(with: "http") {
+            // Check if this is a claim link (URL in number field)
+            if let number = card.number, number.lowercased().starts(with: "http"), URL(string: number) != nil {
                 self.uiState.isClaimLink = true
             }

300-330: Consolidate duplicate barcode fallback logic.

Lines 311-320 and 321-330 contain identical fallback logic that generates a barcode from claimCode. The duplication can be simplified by restructuring the conditions.

Apply this diff to consolidate the fallback:

-                        // Download and scan barcode from URL if available (matching Android)
-                        if let barcodeLink = firstCard.barcodeLink, !barcodeLink.isEmpty {
-                            if let result = await BarcodeScanner.downloadAndScan(from: barcodeLink) {
-                                // Clean the barcode value (remove spaces and dashes)
-                                let cleanValue = result.value.replacingOccurrences(of: " ", with: "")
-                                    .replacingOccurrences(of: "-", with: "")
-                                await giftCardsDAO.updateBarcode(
-                                    txId: txId,
-                                    value: cleanValue,
-                                    format: result.format.rawValue
-                                )
-                            } else {
-                                // Fallback: Generate barcode from claimCode
-                                let cleanCode = claimCode.replacingOccurrences(of: " ", with: "")
-                                    .replacingOccurrences(of: "-", with: "")
-                                await giftCardsDAO.updateBarcode(
-                                    txId: txId,
-                                    value: cleanCode,
-                                    format: "CODE_128"
-                                )
-                            }
-                        } else {
-                            // Fallback: Generate barcode from claimCode (legacy behavior)
+                        // Try to download and scan barcode from URL if available
+                        var barcodeResult: BarcodeScanner.BarcodeResult? = nil
+                        if let barcodeLink = firstCard.barcodeLink, !barcodeLink.isEmpty {
+                            barcodeResult = await BarcodeScanner.downloadAndScan(from: barcodeLink)
+                        }
+                        
+                        if let result = barcodeResult {
+                            // Use scanned barcode
+                            let cleanValue = result.value.replacingOccurrences(of: " ", with: "")
+                                .replacingOccurrences(of: "-", with: "")
+                            await giftCardsDAO.updateBarcode(
+                                txId: txId,
+                                value: cleanValue,
+                                format: result.format.rawValue
+                            )
+                        } else {
+                            // Fallback: Generate barcode from claimCode
                             let cleanCode = claimCode.replacingOccurrences(of: " ", with: "")
                                 .replacingOccurrences(of: "-", with: "")
                             await giftCardsDAO.updateBarcode(
                                 txId: txId,
                                 value: cleanCode,
                                 format: "CODE_128"
                             )
                         }
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (1)

193-203: Remove empty debug loops.

Lines 193-194 and 198-203 contain empty for loops that serve no purpose. These appear to be leftover debug scaffolding.

Apply this diff to remove the empty loops:

-        for (index, card) in giftCards.enumerated() {
-        }
-
         // Step 2: Select the appropriate gift card
         guard let selectedCard = PiggyCardsCache.shared.selectGiftCard(from: giftCards, forAmount: fiatAmount) else {
-            for card in giftCards {
-                let normalizedType = card.priceType.lowercased()
-                if normalizedType == "fixed" {
-                } else if normalizedType == "range" {
-                }
-            }
             throw DashSpendError.invalidAmount
         }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between efae52f and c86a194.

📒 Files selected for processing (9)
  • DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift (1 hunks)
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1 hunks)
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsAPI.swift (2 hunks)
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (1 hunks)
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift (4 hunks)
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (6 hunks)
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (4 hunks)
  • DashWallet/Sources/UI/Home/Tx Metadata/GiftCardMetadataProvider.swift (3 hunks)
  • DashWallet/Sources/Utils/PercentageFormatter.swift (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swift
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift:21-23
Timestamp: 2025-09-27T15:36:10.803Z
Learning: In PiggyCardsRepository.swift, there's an ACL mismatch (public static let shared in an internal class) and potential thread safety concerns that will be addressed during dedicated PiggyCards development rather than in the current UI improvements PR.
📚 Learning: 2025-11-24T15:33:27.646Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 739
File: DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift:152-251
Timestamp: 2025-11-24T15:33:27.646Z
Learning: In ExploreDatabaseConnection.swift test merchant setup, savingsPercentage values in both merchant and gift_card_providers tables must be consistent and expressed in basis points (e.g., 1000 = 10%). The values are divided by 10000 in DashSpendPayViewModel, so both tables should use the same basis point value for the same merchant to avoid discount calculation mismatches.

Applied to files:

  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift
  • DashWallet/Sources/Utils/PercentageFormatter.swift
  • DashWallet/Sources/UI/Home/Tx Metadata/GiftCardMetadataProvider.swift
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift
📚 Learning: 2025-09-27T23:33:40.458Z
Learnt from: HashEngineering
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsEndpoint.swift:25-32
Timestamp: 2025-09-27T23:33:40.458Z
Learning: PiggyCards API getGiftCard endpoint uses an invalid path "gift-cards/\(txid)" that doesn't exist on the server (marked with TODO comment). The correct path should likely be "orders" with query parameters following the CTX API pattern, since PiggyCards uses "orders" for purchases while CTX uses "gift-cards" for both purchase and retrieval operations.

Applied to files:

  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift
📚 Learning: 2025-09-27T15:36:10.803Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift:21-23
Timestamp: 2025-09-27T15:36:10.803Z
Learning: In PiggyCardsRepository.swift, there's an ACL mismatch (public static let shared in an internal class) and potential thread safety concerns that will be addressed during dedicated PiggyCards development rather than in the current UI improvements PR.

Applied to files:

  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsAPI.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift
  • DashWallet/Sources/UI/Home/Tx Metadata/GiftCardMetadataProvider.swift
  • DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift
  • DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift
📚 Learning: 2025-11-24T15:33:27.646Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 739
File: DashWallet/Sources/Models/Explore Dash/Infrastructure/Database Connection/ExploreDatabaseConnection.swift:152-251
Timestamp: 2025-11-24T15:33:27.646Z
Learning: In ExploreDatabaseConnection.swift test merchant setup, denominationsType values must use lowercase ('fixed', 'range', 'option') in both merchant and gift_card_providers INSERT statements to match downstream filtering logic. Using uppercase values like 'Fixed' will cause merchants to fail denomination type filtering.

Applied to files:

  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
📚 Learning: 2025-09-27T15:37:44.449Z
Learnt from: bfoss765
Repo: dashpay/dashwallet-ios PR: 732
File: DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift:94-96
Timestamp: 2025-09-27T15:37:44.449Z
Learning: User bfoss765 prefers to defer fixing force unwrapping warnings until crashes are observed during testing, rather than proactively addressing them.

Applied to files:

  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
🧬 Code graph analysis (3)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (11)
DashWallet/Sources/Utils/PercentageFormatter.swift (2)
  • format (29-39)
  • format (47-50)
DashWallet/Sources/Models/Explore Dash/Model/Entites/ExplorePointOfUse.swift (1)
  • toSavingsFraction (107-109)
DashWallet/Sources/Models/Taxes/Services/TaxReportGenerator.swift (1)
  • value (122-164)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (3)
  • orderGiftCard (186-266)
  • getMerchant (420-423)
  • getGiftCards (342-364)
DashWallet/Sources/UI/Home/Tx Metadata/CustomIconMetadataProvider.swift (1)
  • updateIcon (126-179)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)
  • getMerchant (204-228)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsCache.swift (1)
  • getGiftCards (48-52)
DashWallet/Sources/UI/Payments/SendReceivePageController.swift (1)
  • index (123-125)
DashWallet/Sources/UI/Payments/Amount/Model/Send/SendAmountModel.swift (1)
  • checkAmountForErrors (104-118)
DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift (1)
  • create (58-79)
DashWallet/Sources/Models/Tx Metadata/DAO/TransactionMetadataDAO.swift (1)
  • create (57-82)
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (3)
DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift (2)
  • from (39-66)
  • downloadAndScan (100-123)
DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift (3)
  • get (81-94)
  • updateCardDetails (105-135)
  • updateBarcode (137-166)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (1)
  • getOrderStatus (302-314)
DashWallet/Sources/UI/Home/Tx Metadata/GiftCardMetadataProvider.swift (1)
DashWallet/Sources/Models/Explore Dash/Model/DAO/GiftCardsDAO.swift (1)
  • observeAll (97-99)
🪛 SwiftLint (0.57.0)
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift

[Warning] 186-186: Function should have complexity 10 or less; currently complexity is 13

(cyclomatic_complexity)


[Warning] 269-269: Function parameters should be aligned vertically if they're in multiple lines in a declaration

(vertical_parameter_alignment)


[Warning] 270-270: Function parameters should be aligned vertically if they're in multiple lines in a declaration

(vertical_parameter_alignment)


[Warning] 271-271: Function parameters should be aligned vertically if they're in multiple lines in a declaration

(vertical_parameter_alignment)


[Warning] 187-187: Don't include vertical whitespace (empty line) after opening braces

(vertical_whitespace_opening_braces)


[Warning] 272-272: Don't include vertical whitespace (empty line) after opening braces

(vertical_whitespace_opening_braces)

DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift

[Warning] 39-39: Function should have complexity 10 or less; currently complexity is 12

(cyclomatic_complexity)


[Warning] 96-96: Don't include vertical whitespace (empty line) after opening braces

(vertical_whitespace_opening_braces)

🔇 Additional comments (18)
DashWallet/Sources/UI/Home/Tx Metadata/GiftCardMetadataProvider.swift (1)

42-52: LGTM! Clean observation pattern.

The observation of gift card changes is properly implemented with:

  • Subscription stored in cancellableBag to prevent deallocation
  • Updates received on main thread before spawning async task
  • Weak self capture to avoid retain cycles
DashWallet/Sources/Models/Explore Dash/Services/DashSpend/CTX/CTXSpendRepository.swift (1)

232-234: LGTM - Debug logging for API troubleshooting.

The added logging captures the base URL and endpoint info for debugging. The txid is a blockchain transaction identifier (not a gift card code), so logging it is acceptable for troubleshooting purposes.

DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsAPI.swift (1)

62-72: Recursive token refresh deadlock fix is properly implemented.

The handleUnauthorizedError method now correctly distinguishes between auth endpoints and protected endpoints. This prevents the infinite loop where a 401 on .login would trigger a token refresh that calls .login again.

DashWallet/Sources/Utils/PercentageFormatter.swift (1)

1-51: LGTM - Clean utility with appropriate precision handling.

The formatter correctly uses 1 decimal place for sub-1% values and whole numbers for larger percentages, matching the PR objective for discount display (e.g., "0.5%" vs "4%").

DashWallet/Sources/Models/Explore Dash/Services/BarcodeScanner.swift (2)

159-197: Proper continuation guard against double-resumption.

The resumed flag correctly prevents multiple calls to continuation.resume() which would cause a runtime crash. Good defensive coding.


100-123: Sensitive logging appears to have been removed.

The past review flagged logging of full barcode values and URLs. The current code no longer contains these DSLogger.log calls, addressing the privacy concern.

DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift (2)

369-383: Loading state reset before early returns - properly addressed.

The isLoading = false is now set on both early return paths, preventing the UI from being stuck in a loading state.


107-114: Discount formatting correctly uses new PercentageFormatter.

The conversion from savingsFraction (decimal) to percentage and the formatting logic align with the PR objective for smart decimal precision (0.5% vs 4%).

DashWallet/Sources/UI/Explore Dash/Views/DashSpend/GiftCardDetailsViewModel.swift (5)

34-36: LGTM!

The new state fields properly support the PiggyCards integration. All fields have safe default values and clear purposes.


49-50: LGTM!

The increased retry count (40 retries over 60 seconds) is appropriate for the gift card order fulfillment workflow, which may take longer than the previous 3-retry limit.


185-211: LGTM!

The provider-based routing logic correctly defaults to CTX for backward compatibility and includes helpful debug logging. The long-polling threshold detection is well-placed.


213-272: LGTM!

The CTX fetch path maintains the existing logic correctly. Error handling with retry count and stop conditions is appropriate.


177-183: LGTM!

Properly resets all polling-related state including the new hasBeenPollingForLongTime flag.

DashWallet/Sources/Models/Explore Dash/Services/DashSpend/PiggyCards/PiggyCardsRepository.swift (5)

302-314: LGTM!

Proper error handling and logging for order status retrieval.


317-330: LGTM!

Proper caching implementation for exchange rates with appropriate error handling.


333-364: LGTM!

The brand and gift card fetching logic properly caches results for subsequent order creation. Error handling is appropriate.

Note: Line 354-355 has another empty loop similar to the earlier ones, which can be removed as part of the same cleanup.


367-371: LGTM!

The discount calculation correctly converts decimal to percentage and accounts for the service fee. Based on learnings, this aligns with the expected discount handling across the codebase.


374-416: LGTM!

The payment URI parsing logic properly validates the format and extracts address/amount components with appropriate error handling.

@bfoss765 bfoss765 changed the base branch from master to feature/piggycards November 26, 2025 18:37
@bfoss765 bfoss765 merged commit 51ef2f7 into feature/piggycards Nov 26, 2025
4 checks passed
@bfoss765 bfoss765 deleted the feature/PC-show-gift-card branch November 26, 2025 19:45
@coderabbitai coderabbitai Bot mentioned this pull request Dec 17, 2025
5 tasks
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