Skip to content

fix(predict): fix claim button not showing and not working for resolved positions#27050

Closed
vinnyhoward wants to merge 6 commits into
mainfrom
fix-tmcu-perps-claim-not-working-homepage
Closed

fix(predict): fix claim button not showing and not working for resolved positions#27050
vinnyhoward wants to merge 6 commits into
mainfrom
fix-tmcu-perps-claim-not-working-homepage

Conversation

@vinnyhoward
Copy link
Copy Markdown
Contributor

@vinnyhoward vinnyhoward commented Mar 5, 2026

Description

Fixed several bugs with the Predict claim button on the homepage that prevented users from seeing and successfully claiming winnings from resolved markets.

  1. Claim failed with "No claimable positions found" — PredictController.claimWithConfirmation used a direct key lookup (state.claimablePositions[signer.address]) to find stored positions. The address key was stored using checksummed format but read back with different casing, causing the lookup to silently return undefined. Fixed with case-insensitive address matching, consistent with the existing pattern in confirmClaim. The duplicated address-lookup logic in both claimWithConfirmation and confirmClaim was extracted into a private getClaimablePositionsByAddress helper.

  2. Claim button didn't disappear after successful claim on homepageclaimWithConfirmation resolves on transaction submission (not on-chain confirmation), so refreshPositions() fired while the API still returned the position as claimable. Fixed with an optimistic hasClaimed flag that immediately hides the button on submission, and _clearPositionsCache() called from usePredictToastRegistrations when the claim transaction is confirmed, so the next visit fetches fresh data.

  3. Simplified positions fetch in PredictionsSection — The hook was called twice: once to fetch active positions and again with claimable: true to fetch claimable positions, resulting in two separate API calls to Polymarket's /positions endpoint. The second call used a redeemable=true filter that returns positions at unpredictable times relative to on-chain settlement, which is why pull-to-refresh missed winning positions while navigating into a position detail (which used an unfiltered React Query call) and back would reveal them. Now usePredictPositionsForHomepage is called once with no filter, fetching all positions, and the result is split client-side using position.claimable.

Changelog

CHANGELOG entry:null

Related issues

Fixes:

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

~

Before

Simulator.Screen.Recording.-.iPhone.16e.-.2026-03-05.at.16.27.41.mov

After

Simulator.Screen.Recording.-.iPhone.16e.-.2026-03-06.at.14.24.40.mov

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Touches the claim flow and homepage positions caching/rendering; mistakes could hide claimable winnings or prevent claims from being triggered, but changes are localized and covered by added tests.

Overview
Fixes Predict claiming and homepage display issues caused by address-casing mismatches and stale/fragmented position fetching.

PredictController now resolves claimablePositions via a shared case-insensitive lookup (findAddressKey/getClaimablePositionsByAddress) for both claimWithConfirmation and confirmClaim, and selectors are updated to read claimable positions case-insensitively.

Homepage Predictions now fetches all positions in one call (usePredictPositionsForHomepage drops the claimable filter and unifies its cache key), splits claimable vs active client-side, and hides the claim button optimistically after submission (usePredictClaim returns {succeeded, wasCancelled} plus hasClaimed/isClaiming state). On tx confirmation, usePredictToastRegistrations clears the homepage positions cache to force a fresh fetch next visit/refresh. Tests were updated/added to cover these scenarios.

Written by Cursor Bugbot for commit 9d40ff2. This will update automatically on new commits. Configure here.

@vinnyhoward vinnyhoward requested a review from a team as a code owner March 5, 2026 10:59
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 5, 2026

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-mobile-ux Mobile UX team label Mar 5, 2026
@github-actions github-actions Bot added the size-S label Mar 5, 2026
@vinnyhoward vinnyhoward changed the title Title: fix(predict): fix claim button not showing and not working for resolved positions fix(predict): fix claim button not showing and not working for resolved positions Mar 5, 2026
// Get claimable positions from state
const claimablePositions = this.state.claimablePositions[signer.address];
// Get claimable positions from state (case-insensitive address match)
const normalizedSignerAddress = signer.address.toLowerCase();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we create a utils function for this logic? This function can be shared for the controller and the selector.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed 1e36186

@github-actions github-actions Bot added size-M and removed size-S labels Mar 6, 2026
Comment thread app/components/UI/Predict/selectors/predictController/index.ts
@github-actions github-actions Bot added size-L and removed size-M labels Mar 6, 2026
@vinnyhoward vinnyhoward added the skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. label Mar 6, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2026

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokePredictions, SmokeWalletPlatform, SmokeConfirmations
  • Selected Performance tags: @PerformancePredict
  • Risk Level: medium
  • AI Confidence: 85%
click to see 🤖 AI reasoning details

E2E Test Selection:
This PR makes focused changes to the Predict (Polymarket) functionality:

  1. PredictController changes (CRITICAL file): Added case-insensitive address matching for claimable positions lookup. This fixes a bug where checksummed vs lowercase addresses could cause position lookup failures. The claimWithConfirmation and confirmClaim methods now use a new getClaimablePositionsByAddress helper.

  2. New utility function: Added findAddressKey in accounts.ts for case-insensitive address key lookups in maps.

  3. Selector updates: selectPredictClaimablePositionsByAddress now uses case-insensitive matching.

  4. usePredictClaim hook: Added return type with wasCancelled and succeeded status flags for better claim flow handling.

  5. usePredictPositionsForHomepage hook: Simplified to fetch all positions in a single API call (removed claimable parameter), with client-side filtering via p.claimable.

  6. PredictionsSection component: Refactored to use single positions fetch, added isClaiming/hasClaimed state, improved claim button visibility logic.

  7. Toast registrations: Added cache clearing after successful deposit.

Tag Selection Rationale:

  • SmokePredictions: Primary tag - directly tests prediction market functionality including opening positions, cashing out, and claiming winnings. The claim flow changes are core to this tag.
  • SmokeWalletPlatform: Required per tag description - Predictions is a section inside the Trending tab. Changes to PredictionsSection.tsx affect the homepage display.
  • SmokeConfirmations: Required per SmokePredictions description - claiming positions involves on-chain transactions that go through the confirmation flow.

The changes are well-scoped to the Predict feature with comprehensive unit tests added. Risk is medium because while the changes affect a controller (critical file pattern), they are isolated to the Predict domain and don't cascade to other controllers.

Performance Test Selection:
The changes affect the Predict feature's data fetching and rendering logic. Specifically: (1) usePredictPositionsForHomepage now fetches all positions in a single API call instead of separate calls for active and claimable positions - this could impact loading performance. (2) PredictionsSection now does client-side filtering of positions which could affect render times. (3) Added state management (isClaiming, hasClaimed) that affects UI updates. The @PerformancePredict tag covers prediction market list loading, deposit flows, and balance display - all areas touched by these changes.

View GitHub Actions results

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

jest.clearAllMocks();
mockClaim.mockResolvedValue(undefined);
mockRefreshClaimable.mockResolvedValue(undefined);
mockClaim.mockResolvedValue({ wasCancelled: false });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Test mock omits succeeded field, hiding untested code path

Medium Severity

The mockClaim mock resolves with { wasCancelled: false } but omits the succeeded field that handleClaim destructures. This means succeeded is always undefined in tests, so setHasClaimed(true) is never called, and the PR's key new optimistic-hide behavior for the claim button is entirely untested. The mock needs to include succeeded: true to exercise the production code path.

Additional Locations (1)

Fix in Cursor Fix in Web

const hasError =
!isLoadingPositions &&
!isLoadingMarkets &&
!isLoading &&
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Section analytics incorrect when only claimable positions exist

Low Severity

The rendering condition was expanded to show the positions branch when totalClaimable > 0 (even without active positions), but hasPositions, isEmpty, itemCount, and willRender still only consider active (non-claimable) positions. When only claimable positions exist, the section renders the claim button but isEmpty is true, willRender is false, and itemCount is 0, causing incorrect analytics tracking via useHomeViewedEvent.

Additional Locations (1)

Fix in Cursor Fix in Web

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2026

⚠️ E2E Fixture Validation — Structural changes detected

Category Count
New keys 68
Missing keys 11
Type mismatches 0
Value mismatches 7 (informational)

The committed fixture schema is out of date. To update, comment:

@metamaskbot update-mobile-fixture

View full details | Download diff report

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

@caieu caieu left a comment

Choose a reason for hiding this comment

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

Left a comment.

});
await claimWinnings({});
const result = await claimWinnings({});
return {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We weren’t using the result before. Since we’re using it now, instead of returning flags, let’s return the result object itself so whoever consumes it can use it however they need.

Comment on lines +1570 to +1577
private getClaimablePositionsByAddress(
address: string,
): { positions: PredictPosition[]; matchedKey: string } | undefined {
const matchedKey = findAddressKey(this.state.claimablePositions, address);
if (!matchedKey) return undefined;
return { positions: this.state.claimablePositions[matchedKey], matchedKey };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should not need this change. Also, we now use React Query for our positions queries and you should be reusing our hooks (or at least our queries) rather than creating custom fetch logic.

Let's have a sync on this if needed. We're happy to help fix the issues across the Predict section in the new homepage.

@vinnyhoward
Copy link
Copy Markdown
Contributor Author

Issue has been fixed in main and now closing this PR

@vinnyhoward vinnyhoward closed this Mar 9, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size-L skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. team-mobile-ux Mobile UX team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants