Conversation
WalkthroughAdds a timeTab ('all' | 'weekly') prop across leaderboard components, surfaces finalPxPointsAwardEligible from the data hook into types and UI, and centralizes PX display via calculateFinalPxPoints which conditionally applies a +200 bonus. Tests updated to pass timeTab where needed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Leaderboard Page (index)
participant Hook as useLeaderboardData
participant Boards as Leaderboards
participant Tab as LeaderboardTab
participant Summary as PxPointsSummary
participant Card as OverviewPointsCard
participant Utils as calculateFinalPxPoints
User->>Page: select timeTab ('all' | 'weekly')
Page->>Hook: fetch leaderboard data
Hook-->>Page: LeaderboardTableData (includes finalPxPointsAwardEligible)
Page->>Boards: render(data, timeTab)
Page->>Summary: render(data, timeTab)
Boards->>Tab: render(data, timeTab)
Summary->>Card: render(data, timeTab)
Tab->>Utils: calculateFinalPxPoints(basePoints, eligible, timeTab, activeTab)
Card->>Utils: calculateFinalPxPoints(basePoints, eligible, timeTab, 'migration')
note right of Utils: If activeTab==='trading' && timeTab==='all' ⇒ no bonus\nElse if timeTab==='all' && eligible ⇒ +200\nOtherwise ⇒ basePoints
sequenceDiagram
autonumber
participant Hook as useLeaderboardData
participant API as External API
participant UI as Leaderboard UI
Hook->>API: fetch leaderboard endpoints
API-->>Hook: results (include finalPxPointsAwardEligible)
Hook-->>UI: emit LeaderboardTableData with finalPxPointsAwardEligible
UI->>UI: compute displayed points via calculateFinalPxPoints(...)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying x with
|
| Latest commit: |
7a54906
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ef4fafd1.x-e62.pages.dev |
| Branch Preview URL: | https://chore-additional-px-points.x-e62.pages.dev |
github-actions
bot
temporarily deployed
to
Preview (chore/additional-px-points)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx (1)
162-171: Fix potential crash when addresses is undefined (unsafe access to length)Using
result.addresses?.[result.addresses.length - 1]still dereferencesresult.addresses.lengthwhenaddressesis undefined, leading to a runtime error. Guardlengthtoo, or use a safer pattern.- key={`${result.addresses?.[result.addresses.length - 1]}-${index}`} + key={`${result.addresses?.[result.addresses?.length - 1] ?? 'unknown'}-${index}`} ... - walletAddress={ - result.addresses?.[result.addresses.length - 1] || '' - } + walletAddress={ + result.addresses?.[result.addresses?.length - 1] || '' + }Optional: To avoid repetition and further guardrails, compute once per item (e.g.,
const lastAddress = result.addresses?.[result.addresses?.length - 1];) or considerresult.addresses?.at?.(-1)if your TS/lib target supports Array.prototype.at.
🧹 Nitpick comments (16)
src/types/api.ts (1)
482-483: Add JSDoc clarifyingfinalPxPointsAwardEligibledefault behaviorVerification confirms that all uses of
calculateFinalPxPointsguard with
finalPxPointsAwardEligible === true, soundefinedis already treated asfalse. To make this intent explicit, consider adding JSDoc above both API declarations:• Locations:
– src/types/api.ts:482–483
– src/types/api.ts:874–875transactionHistory?: PointsTransactionHistoryItem; + /** Eligible for the +200 PX points award when timeTab === 'all'; undefined defaults to false. */ finalPxPointsAwardEligible?: boolean;newDropTime?: number; + /** Eligible for the +200 PX points award when timeTab === 'all'; undefined defaults to false. */ finalPxPointsAwardEligible?: boolean;This is a non-blocking nitpick to improve code readability.
src/apps/leaderboard/components/PxPointsSummary/tests/PxPointsSummary.test.tsx (1)
121-129: LGTM: timeTab added to all renders; add “weekly” and bonus eligibility coverageUpdates look consistent with the new API. To harden behavior, add tests that:
- Assert no +200 bonus when timeTab="weekly".
- Assert +200 bonus applies only when finalPxPointsAwardEligible is true and timeTab="all".
Example additional test (place at the end of the suite):
it('applies +200 bonus only for all-time when eligible', () => { const eligibleAllTime: LeaderboardTableData[] = [{ totalPoints: 1000, totalAmountUsd: 0, addresses: [mockWallet], totalGas: 0, finalPxPointsAwardEligible: true }]; const ineligibleAllTime: LeaderboardTableData[] = [{ totalPoints: 1000, totalAmountUsd: 0, addresses: [mockWallet], totalGas: 0, finalPxPointsAwardEligible: false }]; // All-time, eligible -> expect 1200 displayed somewhere in the summary/overview render( <PxPointsSummary allTimeTradingData={eligibleAllTime} allTimeMigrationData={[]} mergedAllTimeData={eligibleAllTime} mergedWeeklyTimeData={[]} isUserInMigrationData={false} timeTab="all" /> ); expect(screen.getByText(/1,200|1200/)).toBeInTheDocument(); // Weekly, eligible -> expect no bonus (1000) render( <PxPointsSummary allTimeTradingData={[]} allTimeMigrationData={[]} mergedAllTimeData={[]} mergedWeeklyTimeData={eligibleAllTime} isUserInMigrationData={false} timeTab="weekly" /> ); expect(screen.getByText(/1,000|1000/)).toBeInTheDocument(); // All-time, ineligible -> expect no bonus (1000) render( <PxPointsSummary allTimeTradingData={ineligibleAllTime} allTimeMigrationData={[]} mergedAllTimeData={ineligibleAllTime} mergedWeeklyTimeData={[]} isUserInMigrationData={false} timeTab="all" /> ); expect(screen.getByText(/1,000|1000/)).toBeInTheDocument(); });Also applies to: 135-144, 156-164, 178-186, 195-203
src/apps/leaderboard/components/PointsCards/tests/OverviewPointsCard.test.tsx (1)
29-33: LGTM: timeTab wiring is correct; add coverage for weekly and eligibility scenariosThese changes align with the new prop. Recommend adding cases to assert:
- Weekly view shows base points (no +200).
- All-time applies +200 only when entry.finalPxPointsAwardEligible is true.
Example additional test:
it('respects timeTab for final PX points in Overview card', () => { const eligible = { ...myAllTimeMergedMock, finalPxPointsAwardEligible: true, totalPoints: 1000 }; const { rerender } = render( <OverviewPointsCard myAllTimeMerged={{ entry: eligible, index: 0 }} myWeeklyMerged={{ entry: { ...myWeeklyMergedMock, totalPoints: 1000, finalPxPointsAwardEligible: true }, index: 0 }} timeTab="all" /> ); expect(screen.getByText(/1,200|1200/)).toBeInTheDocument(); rerender( <OverviewPointsCard myAllTimeMerged={{ entry: eligible, index: 0 }} myWeeklyMerged={{ entry: { ...myWeeklyMergedMock, totalPoints: 1000, finalPxPointsAwardEligible: true }, index: 0 }} timeTab="weekly" /> ); expect(screen.getByText(/1,000|1000/)).toBeInTheDocument(); });Also applies to: 40-44, 51-55, 72-76
src/apps/leaderboard/components/LeaderboardTab/test/LeaderboardTab.test.tsx (1)
42-45: LGTM – tests updated; add assertions for bonus logicBoth the
calculateFinalPxPointshelper and the flooring/formatting (viaMath.floor+formatAmountDisplay) are already wired up inLeaderboardTab.tsx. To lock in the bonus‐point behavior, please enhance the snapshot tests insrc/apps/leaderboard/components/LeaderboardTab/test/LeaderboardTab.test.tsx:
- Add a data row with
finalPxPointsAwardEligible=trueand assert:
- When
timeTab="all", the rendered points include the +200 bonus.- When
timeTab="weekly", the bonus is omitted.- For the “My rank” section, add similar assertions to confirm the user’s displayed points follow the same rule.
src/apps/leaderboard/index.tsx (1)
156-157: Propagating timeTab is consistent; consider centralizing final-points calc across componentsPassing timeTab through App → Summary/Leaderboards → Tab is clean. To avoid drift, centralize calculateFinalPxPoints in a single util (used by PxPointsSummary, OverviewPointsCard, LeaderboardTab).
Additionally, based on our prior learning about weekly migration logic (getLastWeekMigrationData checking currentWeek for completedSwapWeek), please ensure recent timeTab changes did not regress that behavior.
If helpful, create a shared util:
// src/apps/leaderboard/utils/calculateFinalPxPoints.ts export type TimeTab = 'all' | 'weekly'; export const calculateFinalPxPoints = ( basePoints: number | undefined, eligible: boolean | undefined, timeTab: TimeTab ) => { if (!basePoints) return 0; const bonus = timeTab === 'all' && eligible ? 200 : 0; return Math.floor(basePoints + bonus); };Then import and use it in:
- components/PxPointsSummary/PxPointsSummary.tsx
- components/PointsCards/OverviewPointsCard.tsx
- components/LeaderboardTab/LeaderboardTab.tsx
Also, the totals memo looks good and accounts for undefined data safely. No action needed. Also applies to: 213-213, 226-226, 241-241, 256-256, 269-269, 282-282 </blockquote></details> <details> <summary>src/apps/leaderboard/components/Leaderboards/tests/Leaderboards.test.tsx (2)</summary><blockquote> `24-26`: **Consistent propagation of timeTab in tests — consider adding a weekly-case assertion** Passing `timeTab="all"` across loading/error/empty/data states is consistent. Please add at least one test with `timeTab="weekly"` to assert that LeaderboardTab receives it and ensure the weekly code path remains guarded against regressions. I can add a parametric test that renders with both "all" and "weekly" and asserts the forwarded prop. Want me to push that? Also applies to: 40-41, 49-50, 71-79, 91-92, 100-106, 114-119 --- `76-79`: **Stabilize mock assertion to avoid brittle second-arg matching** Relying on the second argument `{}` of the mocked component call can be brittle across React/Vitest internals. Prefer matching props with `objectContaining` and make the second arg permissive. Apply: ```diff - expect(LeaderboardTab).toHaveBeenCalledWith( - { data: mockData, timeTab: 'all' }, - {} - ); + expect(LeaderboardTab).toHaveBeenCalledWith( + expect.objectContaining({ data: mockData, timeTab: 'all' }), + expect.anything() + );src/apps/leaderboard/components/Leaderboards/Leaderboards.tsx (1)
16-17: Avoid repeating literal union across files; introduce a shared TimeTab type
'all' | 'weekly'is now repeated in several components. Create a sharedTimeTabtype (e.g.,src/apps/leaderboard/types.ts) and reuse it to prevent drift.Example:
// src/apps/leaderboard/types.ts export type TimeTab = 'all' | 'weekly';Then in this file:
import type { TimeTab } from '../../types'; // ... interface LeaderboardsProps { // ... timeTab: TimeTab; }src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx (2)
16-29: Duplicate calculateFinalPxPoints — extract to a shared utilityThis helper is also implemented in PxPointsSummary.tsx. Duplicate logic risks divergence. Extract to a single utility and import in both places.
Apply (outside this file):
// src/apps/leaderboard/utils/points.ts export const PX_BONUS_ALL = 200 as const; export type TimeTab = 'all' | 'weekly'; export function calculateFinalPxPoints( basePoints: number, finalPxPointsAwardEligible: boolean | undefined, timeTab: TimeTab = 'all', ): number { if (timeTab === 'all' && finalPxPointsAwardEligible === true) { return basePoints + PX_BONUS_ALL; } return basePoints; }Then update this file:
-/** - * Calculate final PX points by adding bonus points if eligible - */ -const calculateFinalPxPoints = ( - basePoints: number, - finalPxPointsAwardEligible?: boolean, - timeTab: 'all' | 'weekly' = 'all' -): number => { - if (timeTab === 'all' && finalPxPointsAwardEligible === true) { - return basePoints + 200; - } - return basePoints; -}; +import { calculateFinalPxPoints } from '../../utils/points';
33-34: Prop API change is coherent; consider reusing a shared TimeTab typePropagating
timeTabhere matches the rest of the tree. Consider importing a sharedTimeTabtype to avoid repeating the union.Also applies to: 38-39
src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx (3)
18-31: Duplicate calculateFinalPxPoints — consolidate in a utilitySame helper as in OverviewPointsCard. Please DRY this into a shared
utils/points.tsand import it here.Apply:
-/** - * Calculate final PX points by adding bonus points if eligible - */ -const calculateFinalPxPoints = ( - basePoints: number, - finalPxPointsAwardEligible?: boolean, - timeTab: 'all' | 'weekly' = 'all' -): number => { - if (timeTab === 'all' && finalPxPointsAwardEligible === true) { - return basePoints + 200; - } - return basePoints; -}; +import { calculateFinalPxPoints } from '../../utils/points';
38-39: Use a shared TimeTab type to reduce repetition
'all' | 'weekly'is repeated here and in other components. Centralize as aTimeTabtype and import it.Also applies to: 47-48
129-134: Guard against double-counting the +200 bonus on the Migration cardcalculateFinalPxPoints is used in multiple leaderboard contexts:
- PxPointsSummary (Migration card: lines 129–134; Trading card: 146–150)
- OverviewPointsCard (all-time: 58–60; weekly: 113–115)
- LeaderboardTab (my rank: 132–134; table rows: 193–195)
Today, only Trading entries set
finalPxPointsAwardEligible, so only the Trading card (and overview/leaderboard views) show the +200 bonus. If Migration entries begin setting this flag, both Migration and Trading cards would display +200, while overview/leaderboard still only reflect it once—leading to inconsistent UI.Two safe options:
- Apply the bonus only in the Trading card and overview/leaderboard; never in Migration.
- Or compute the bonus once at the merged level and pass a
bonusAppliedflag to each card.Minimal change to shield the Migration card:
--- a/src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx +++ b/src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx @@ -129,7 +129,7 @@ icon={MigrationIcon} background="bg-percentage_red/[.05]" - points={calculateFinalPxPoints( - myAllTimeMigration.entry?.totalPoints || 0, - myAllTimeMigration.entry?.finalPxPointsAwardEligible, - timeTab - )} + points={myAllTimeMigration.entry?.totalPoints || 0} rank={ /* ... */ }Please confirm all usages of
calculateFinalPxPointsinsrc/apps/leaderboardand ensure only the intended cards receive the +200 bonus:rg -nP 'calculateFinalPxPoints\s*\(' src/apps/leaderboardsrc/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx (3)
17-29: Extract magic number and avoid silent default for timeTabThe logic looks correct. Two small refinements:
- Replace the hard-coded 200 with a named constant for readability and future configurability.
- Remove the default 'all' for timeTab to avoid silently applying the bonus if a caller forgets to pass the prop (call sites already pass it).
-/** - * Calculate final PX points by adding bonus points if eligible - */ -const calculateFinalPxPoints = ( - basePoints: number, - finalPxPointsAwardEligible?: boolean, - timeTab: 'all' | 'weekly' = 'all' -): number => { - if (timeTab === 'all' && finalPxPointsAwardEligible === true) { - return basePoints + 200; - } - return basePoints; -}; +/** + * Calculate final PX points by adding bonus points if eligible + */ +const FINAL_PX_BONUS = 200 as const; + +const calculateFinalPxPoints = ( + basePoints: number, + finalPxPointsAwardEligible?: boolean, + timeTab: 'all' | 'weekly' +): number => { + if (timeTab === 'all' && finalPxPointsAwardEligible === true) { + return basePoints + FINAL_PX_BONUS; + } + return basePoints; +};Optional follow-up: If this calculation is (or will be) used in multiple components, consider moving it (and the constant) to a shared utils/constants module to keep it single-sourced.
131-137: Unnest the PX points calculation for readabilityThe nested Math.floor + calculateFinalPxPoints + format chain is correct but hard to scan. Consider pulling the computed PX into a const before rendering to improve readability.
192-198: Apply the same readability tweak in the list rendererSame suggestion as for “My rank”: compute the PX points once per item (e.g., const px = Math.floor(calculateFinalPxPoints(...))) and pass to formatAmountDisplay.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx(3 hunks)src/apps/leaderboard/components/LeaderboardTab/test/LeaderboardTab.test.tsx(4 hunks)src/apps/leaderboard/components/Leaderboards/Leaderboards.tsx(3 hunks)src/apps/leaderboard/components/Leaderboards/tests/Leaderboards.test.tsx(5 hunks)src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx(3 hunks)src/apps/leaderboard/components/PointsCards/tests/OverviewPointsCard.test.tsx(4 hunks)src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx(5 hunks)src/apps/leaderboard/components/PxPointsSummary/tests/PxPointsSummary.test.tsx(5 hunks)src/apps/leaderboard/hooks/useLeaderboardData.tsx(2 hunks)src/apps/leaderboard/index.tsx(7 hunks)src/types/api.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-17T09:20:44.533Z
Learnt from: RanaBug
PR: pillarwallet/x#334
File: src/apps/leaderboard/utils/index.tsx:91-94
Timestamp: 2025-06-17T09:20:44.533Z
Learning: In src/apps/leaderboard/utils/index.tsx, the getLastWeekMigrationData function intentionally uses currentWeek (not lastWeek) for the completedSwapWeek lookup. This is correct business logic - when retrieving last week's migration data, the function should check swap completion against the current week while using lastWeek for points and USD calculations.
Applied to files:
src/apps/leaderboard/hooks/useLeaderboardData.tsxsrc/apps/leaderboard/index.tsxsrc/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx
🧬 Code Graph Analysis (3)
src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx (1)
src/types/api.ts (1)
LeaderboardTableData(865-875)
src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx (1)
src/types/api.ts (1)
LeaderboardTableData(865-875)
src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx (1)
src/types/api.ts (1)
LeaderboardTableData(865-875)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (4)
src/apps/leaderboard/hooks/useLeaderboardData.tsx (1)
120-122: Weekly path propagation looks goodWeekly trading entries now include
finalPxPointsAwardEligible; that aligns with consumers using timeTab-aware point calculations.src/apps/leaderboard/components/Leaderboards/Leaderboards.tsx (1)
16-17: Prop wiring for timeTab looks correctNew
timeTabprop is plumbed through and forwarded toLeaderboardTabwithout altering existing state logic.Also applies to: 26-27, 51-51
src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx (1)
57-64: Correct ordering: floor after applying the bonusYou’re flooring the computed value after applying the potential +200, which is the right order.
Also applies to: 112-118
src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx (1)
33-34: All invocations include the timeTab propI verified every real JSX usage of in both Leaderboards.tsx and the tests—all render/create calls include
timeTab. The only unmatched instance is the<LeaderboardTab />string in a test’sdescribe, which doesn’t affect functionality. ✅
github-actions
bot
temporarily deployed
to
Preview (chore/additional-px-points)
| /** | ||
| * Calculate final PX points by adding bonus points if eligible | ||
| */ | ||
| const calculateFinalPxPoints = ( |
There was a problem hiding this comment.
Maybe this can be in the utils? Just because I can see it is being used twice in this branch at different places
| : formatAmountDisplay( | ||
| Math.floor(myWeeklyMerged.entry?.totalPoints || 0) | ||
| Math.floor( | ||
| calculateFinalPxPoints( |
There was a problem hiding this comment.
This would not apply since it is weekly? My understanding is that we want the the 200 points added for "all" not for "weekly" right?
There was a problem hiding this comment.
This has been corrected 🙏
| /** | ||
| * Calculate final PX points by adding bonus points if eligible | ||
| */ | ||
| const calculateFinalPxPoints = ( |
There was a problem hiding this comment.
See other comment above on this function
github-actions
bot
temporarily deployed
to
Preview (chore/additional-px-points)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/apps/leaderboard/utils/index.tsx (1)
241-258: Avoid magic number + centralize non-negative clamp in the helper.
- Replace literal 200 with a named constant for maintainability.
- Per PR objective (“non-negative PX points”), return non-negative values from the helper (safer than relying on every caller).
Apply this diff:
+const FINAL_PX_BONUS = 200 as const; export const calculateFinalPxPoints = ( basePoints: number, finalPxPointsAwardEligible?: boolean, timeTab: 'all' | 'weekly' = 'all', activeTab?: 'trading' | 'migration' ): number => { // If activeTab is 'trading' and timeTab is 'all', don't add bonus points if (activeTab === 'trading' && timeTab === 'all') { - return basePoints; + return Math.max(0, basePoints); } // Add bonus points only if timeTab is 'all' and eligible if (timeTab === 'all' && finalPxPointsAwardEligible === true) { - return basePoints + 200; + return Math.max(0, basePoints + FINAL_PX_BONUS); } - return basePoints; + return Math.max(0, basePoints); };src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx (1)
182-189: Clamp displayed PX points to non-negative to match PR objective.Currently, negative totals (if ever present) would render as negative. The PR notes “non-negative PX points.” Clamp at the display layer for safety.
Apply this diff:
- Math.floor( - calculateFinalPxPoints( - result.totalPoints, - result.finalPxPointsAwardEligible, - timeTab, - 'migration' - ) - ) || 0 + Math.max( + 0, + Math.floor( + calculateFinalPxPoints( + result.totalPoints ?? 0, + result.finalPxPointsAwardEligible, + timeTab, + 'migration' + ) + ) + )src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx (1)
118-123: Optionally clamp to non-negative before passing to PointsCard.If PointsCard doesn’t clamp internally, negative values (edge cases) could leak to UI.
Apply this diff:
- points={calculateFinalPxPoints( - myAllTimeMigration.entry?.totalPoints || 0, - myAllTimeMigration.entry?.finalPxPointsAwardEligible, - timeTab, - 'migration' - )} + points={Math.max( + 0, + calculateFinalPxPoints( + myAllTimeMigration.entry?.totalPoints || 0, + myAllTimeMigration.entry?.finalPxPointsAwardEligible, + timeTab, + 'migration' + ) + )}src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx (2)
44-51: Clamp all-time PX display to non-negative.Aligns with “non-negative PX points” and avoids negative rendering in edge cases.
Apply this diff:
- Math.floor( - calculateFinalPxPoints( - myAllTimeMerged.entry?.totalPoints || 0, - myAllTimeMerged.entry?.finalPxPointsAwardEligible, - timeTab, - 'migration' - ) - ) + Math.max( + 0, + Math.floor( + calculateFinalPxPoints( + myAllTimeMerged.entry?.totalPoints || 0, + myAllTimeMerged.entry?.finalPxPointsAwardEligible, + timeTab, + 'migration' + ) + ) + )
100-102: Clamp weekly PX display to non-negative as well.Weekly totals shouldn’t display negative values either.
Apply this diff:
- Math.floor( - myWeeklyMerged.entry?.totalPoints || 0 - ) + Math.max( + 0, + Math.floor(myWeeklyMerged.entry?.totalPoints || 0) + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx(3 hunks)src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx(4 hunks)src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx(5 hunks)src/apps/leaderboard/utils/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-17T09:20:44.533Z
Learnt from: RanaBug
PR: pillarwallet/x#334
File: src/apps/leaderboard/utils/index.tsx:91-94
Timestamp: 2025-06-17T09:20:44.533Z
Learning: In src/apps/leaderboard/utils/index.tsx, the getLastWeekMigrationData function intentionally uses currentWeek (not lastWeek) for the completedSwapWeek lookup. This is correct business logic - when retrieving last week's migration data, the function should check swap completion against the current week while using lastWeek for points and USD calculations.
Applied to files:
src/apps/leaderboard/utils/index.tsxsrc/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsxsrc/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsxsrc/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx
🧬 Code Graph Analysis (3)
src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx (1)
src/apps/leaderboard/utils/index.tsx (1)
calculateFinalPxPoints(241-258)
src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx (2)
src/types/api.ts (1)
LeaderboardTableData(865-875)src/apps/leaderboard/utils/index.tsx (1)
calculateFinalPxPoints(241-258)
src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx (1)
src/apps/leaderboard/utils/index.tsx (1)
calculateFinalPxPoints(241-258)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (4)
src/apps/leaderboard/utils/index.tsx (2)
241-258: Utility looks good and aligns with “All-time only” bonus requirement.Clear API and defaulting of timeTab to 'all' read well. Weekly path remains unchanged, which matches the PR objective.
247-255: Confirm business rule: exclude bonus for Trading “All-time”?The guard
activeTab === 'trading' && timeTab === 'all'prevents adding the 200-point bonus for Trading even in “All-time”. If the intent is “bonus applies only to Migration/merged in All-time” this is correct. If the intent is “bonus applies to eligible users in All-time regardless of source,” this would need to be removed.Would you like me to update the helper and call sites if the bonus should also apply to Trading all-time totals?
src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx (1)
118-123: Good use of centralized calculation for Migration all-time points.The four-arg call matches the helper’s contract and keeps Weekly unchanged.
src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx (1)
6-6: Importing calculateFinalPxPoints here is the right call.Keeps PX computation consistent across the UI.
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores