[SELF-1148, SELF-1149, SELF-1165, SELF-1168] two point nine polish rd3 #1378
Conversation
transphorm
changed the title
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAnalytics tracking integrated into Points UI components with event logging on focus and action. Dev-mode logging removed across hooks and utilities. Self-referral validation added to referral registration flows. Font families and colors migrated to design tokens. Enum naming corrected (REFERAL → REFERRAL) with new REFRESH_HISTORY event added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RecordEvents as recordReferralPointEvent
participant Validation as Self-Referral Check
participant API as registerReferralPoints
participant Store as pointEventStore
participant Poll as pollEventProcessingStatus
User->>RecordEvents: recordReferralPointEvent(referee, referrer, ...)
RecordEvents->>Validation: Compare referee vs referrer<br/>(case-insensitive, trimmed)
alt Self-Referral Detected
Validation-->>RecordEvents: Match found
RecordEvents-->>User: Return failure<br/>(explicit error msg)
else Valid Referral
Validation-->>RecordEvents: No match
RecordEvents->>API: registerReferralPoints(...)<br/>(lowercase addresses)
API-->>RecordEvents: Response with job_id
RecordEvents->>Poll: pollEventProcessingStatus(job_id)
Poll->>Store: Save processed event
Store-->>Poll: Event stored
Poll-->>RecordEvents: Success
RecordEvents-->>User: Return success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
transphorm
changed the title
![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
🧹 Nitpick comments (1)
app/tests/src/utils/points/registerEvents.test.ts (1)
227-248: Address normalization verified.The test confirms that mixed-case addresses are properly lowercased before API submission.
Consider adding a test case that explicitly verifies whitespace trimming in API calls:
it('should trim and lowercase addresses with whitespace', async () => { mockMakeApiRequest.mockResolvedValue({ success: true, status: 200, data: { job_id: 'job-123' }, }); await registerReferralPoints({ referee: ' 0xAbCdEf1234567890aBcDeF1234567890AbCdEf12 ', referrer: ' 0xAbCdEf0987654321aBcDeF0987654321AbCdEf09 ', }); expect(mockMakeApiRequest).toHaveBeenCalledWith('/referrals/refer', { referee: '0xabcdef1234567890abcdef1234567890abcdef12', referrer: '0xabcdef0987654321abcdef0987654321abcdef09', }); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
app/src/components/NavBar/Points.tsx(9 hunks)app/src/components/PointHistoryList.tsx(3 hunks)app/src/hooks/useEarnPointsFlow.ts(0 hunks)app/src/hooks/useReferralRegistration.ts(0 hunks)app/src/hooks/useTestReferralFlow.ts(0 hunks)app/src/screens/account/settings/CloudBackupScreen.tsx(5 hunks)app/src/screens/app/LaunchScreen.tsx(1 hunks)app/src/screens/app/ReferralScreen.tsx(1 hunks)app/src/utils/deeplinks.ts(0 hunks)app/src/utils/points/eventPolling.ts(1 hunks)app/src/utils/points/jobStatus.ts(1 hunks)app/src/utils/points/recordEvents.ts(1 hunks)app/src/utils/points/registerEvents.ts(2 hunks)app/tests/src/utils/points/recordEvents.test.ts(1 hunks)app/tests/src/utils/points/registerEvents.test.ts(1 hunks)packages/mobile-sdk-alpha/src/constants/analytics.ts(1 hunks)
💤 Files with no reviewable changes (4)
- app/src/utils/deeplinks.ts
- app/src/hooks/useEarnPointsFlow.ts
- app/src/hooks/useTestReferralFlow.ts
- app/src/hooks/useReferralRegistration.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
app/src/screens/app/LaunchScreen.tsxapp/src/utils/points/jobStatus.tsapp/src/utils/points/eventPolling.tsapp/src/utils/points/recordEvents.tsapp/src/components/PointHistoryList.tsxapp/src/utils/points/registerEvents.tsapp/src/screens/account/settings/CloudBackupScreen.tsxapp/tests/src/utils/points/registerEvents.test.tsapp/src/screens/app/ReferralScreen.tsxapp/src/components/NavBar/Points.tsxapp/tests/src/utils/points/recordEvents.test.tspackages/mobile-sdk-alpha/src/constants/analytics.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/screens/app/LaunchScreen.tsxapp/src/utils/points/jobStatus.tsapp/src/utils/points/eventPolling.tsapp/src/utils/points/recordEvents.tsapp/src/components/PointHistoryList.tsxapp/src/utils/points/registerEvents.tsapp/src/screens/account/settings/CloudBackupScreen.tsxapp/tests/src/utils/points/registerEvents.test.tsapp/src/screens/app/ReferralScreen.tsxapp/src/components/NavBar/Points.tsxapp/tests/src/utils/points/recordEvents.test.ts
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/screens/app/LaunchScreen.tsxapp/src/utils/points/jobStatus.tsapp/src/utils/points/eventPolling.tsapp/src/utils/points/recordEvents.tsapp/src/components/PointHistoryList.tsxapp/src/utils/points/registerEvents.tsapp/src/screens/account/settings/CloudBackupScreen.tsxapp/src/screens/app/ReferralScreen.tsxapp/src/components/NavBar/Points.tsx
**/*.{test,spec}.{ts,js,tsx,jsx}
⚙️ CodeRabbit configuration file
**/*.{test,spec}.{ts,js,tsx,jsx}: Review test files for:
- Test coverage completeness
- Test case quality and edge cases
- Mock usage appropriateness
- Test readability and maintainability
Files:
app/tests/src/utils/points/registerEvents.test.tsapp/tests/src/utils/points/recordEvents.test.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/src/constants/analytics.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/constants/analytics.ts
🧠 Learnings (18)
📚 Learning: 2025-08-25T14:25:57.586Z
Learnt from: aaronmgdr
Repo: selfxyz/self PR: 951
File: app/src/providers/authProvider.web.tsx:17-18
Timestamp: 2025-08-25T14:25:57.586Z
Learning: The selfxyz/mobile-sdk-alpha/constants/analytics import path is properly configured with SDK exports, Metro aliases, and TypeScript resolution. Import changes from @/consts/analytics to this path are part of valid analytics migration, not TypeScript resolution issues.
Applied to files:
app/src/utils/points/jobStatus.tsapp/src/utils/points/eventPolling.tsapp/src/screens/app/ReferralScreen.tsxapp/src/components/NavBar/Points.tsxpackages/mobile-sdk-alpha/src/constants/analytics.ts
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
Repo: selfxyz/self PR: 936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
app/src/components/PointHistoryList.tsx
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
Repo: selfxyz/self PR: 936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: The main App.tsx file is located at app/App.tsx (not in app/src), and it properly wraps the entire app with SelfClientProvider at the top of the provider hierarchy, enabling useSelfClient() hook usage throughout all navigation screens.
Applied to files:
app/src/components/PointHistoryList.tsx
📚 Learning: 2025-08-23T02:02:02.556Z
Learnt from: transphorm
Repo: selfxyz/self PR: 942
File: app/vite.config.ts:170-0
Timestamp: 2025-08-23T02:02:02.556Z
Learning: In the selfxyz/self React Native app, devTools from '@/navigation/devTools' are intentionally shipped to production builds for testing purposes, not excluded as is typical in most applications.
Applied to files:
app/src/components/PointHistoryList.tsxapp/src/components/NavBar/Points.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Applied to files:
app/src/components/PointHistoryList.tsxapp/tests/src/utils/points/registerEvents.test.ts
📚 Learning: 2025-08-26T14:49:15.210Z
Learnt from: shazarre
Repo: selfxyz/self PR: 936
File: app/src/screens/passport/PassportNFCScanScreen.web.tsx:8-11
Timestamp: 2025-08-26T14:49:15.210Z
Learning: The main App.tsx file is located at app/App.tsx (at the app root), not at app/src/App.tsx, and contains the SelfClientProvider wrapping the entire application.
Applied to files:
app/src/components/PointHistoryList.tsx
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/
Applied to files:
app/src/components/PointHistoryList.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Write integration tests that exercise the real validation logic (not mocks)
Applied to files:
app/tests/src/utils/points/registerEvents.test.tsapp/tests/src/utils/points/recordEvents.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Applied to files:
app/tests/src/utils/points/registerEvents.test.tsapp/tests/src/utils/points/recordEvents.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Never use real user PII in tests; use only synthetic, anonymized, or approved test vectors
Applied to files:
app/tests/src/utils/points/registerEvents.test.tsapp/tests/src/utils/points/recordEvents.test.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to tests/**/*.test.{js,ts,tsx,jsx} : Test error boundaries and recovery mechanisms.
Applied to files:
app/tests/src/utils/points/registerEvents.test.tsapp/tests/src/utils/points/recordEvents.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)
Applied to files:
app/tests/src/utils/points/registerEvents.test.tsapp/tests/src/utils/points/recordEvents.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
app/tests/src/utils/points/registerEvents.test.tsapp/tests/src/utils/points/recordEvents.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data
Applied to files:
app/tests/src/utils/points/registerEvents.test.tsapp/tests/src/utils/points/recordEvents.test.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to app/jest.setup.js : Provide comprehensive Jest setup in app/jest.setup.js with required mocks
Applied to files:
app/tests/src/utils/points/registerEvents.test.tsapp/tests/src/utils/points/recordEvents.test.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to src/navigation/**/*.{js,ts,tsx,jsx} : Platform-specific initial routes: web uses 'Home', mobile uses 'Splash'.
Applied to files:
app/src/components/NavBar/Points.tsx
📚 Learning: 2025-08-26T12:48:14.120Z
Learnt from: aaronmgdr
Repo: selfxyz/self PR: 951
File: app/src/screens/passport/PassportNFCScanScreen.web.tsx:8-9
Timestamp: 2025-08-26T12:48:14.120Z
Learning: SecondaryButton forwards the trackEvent prop to AbstractButton via prop spreading (...props). AbstractButton handles analytics tracking in its handlePress method using the legacy analytics() system rather than the new Self SDK approach, ensuring button analytics work consistently across the app.
Applied to files:
app/src/components/NavBar/Points.tsx
📚 Learning: 2025-08-26T12:48:14.120Z
Learnt from: aaronmgdr
Repo: selfxyz/self PR: 951
File: app/src/screens/passport/PassportNFCScanScreen.web.tsx:8-9
Timestamp: 2025-08-26T12:48:14.120Z
Learning: SecondaryButton in the app forwards the trackEvent prop to AbstractButton, which handles analytics integration. SecondaryButton does not need separate trackEvent prop handling as it delegates to AbstractButton.
Applied to files:
app/src/components/NavBar/Points.tsx
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: type-check
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
🔇 Additional comments (24)
app/src/utils/points/eventPolling.ts (1)
5-5: LGTM! Import path migration to absolute alias.The change from relative to absolute import path is consistent with project-wide refactoring. Based on learnings, the
@/alias is properly configured in this codebase.app/src/utils/points/jobStatus.ts (1)
1-5: LGTM! License header and import path updates.The addition of the SPDX license header ensures proper copyright attribution, and the import path migration to the absolute
@/alias is consistent with the project's refactoring effort.app/src/screens/app/LaunchScreen.tsx (1)
190-190: LGTM - Font token migration.The migration from hard-coded
'DIN OT'to thedinottoken is consistent with the broader design token standardization across the codebase.app/src/screens/account/settings/CloudBackupScreen.tsx (1)
26-27: LGTM - Comprehensive design token migration.The replacement of hard-coded colors and font families with centralized design tokens improves maintainability and consistency across the app.
Also applies to: 378-430
packages/mobile-sdk-alpha/src/constants/analytics.ts (2)
142-144: LGTM - Spelling correction in enum keys.The typo fix from
REFERALtoREFERRALimproves code clarity. Since the string values remain unchanged, analytics data continuity is preserved.
152-152: LGTM - New analytics event added.The
REFRESH_HISTORYevent enables tracking of pull-to-refresh interactions in the Points history list.app/src/components/PointHistoryList.tsx (2)
14-15: LGTM - Analytics integration added.The addition of analytics tracking for the refresh action follows established patterns and properly uses the
useSelfClienthook.Also applies to: 69-69
275-281: LGTM - Refresh tracking implemented correctly.The analytics event is tracked before the refresh operation, and
selfClientis correctly included in the dependency array.app/src/components/NavBar/Points.tsx (3)
451-457: LGTM - Explore apps navigation added.The navigation to an external URL is properly tracked with analytics. The URL
https://apps.self.xyzappears to be a controlled first-party domain.
494-610: LGTM - Font token migration.Consistent replacement of hard-coded
'DIN OT'with thedinottoken across all text styles.
71-85: Analytics system mixing is intentional and correct.The code uses two complementary analytics systems by design, not oversight:
trackScreenView: Tracks screen/page visibility (when Points NavBar gains focus viauseFocusEffect). Also used innavigation/index.tsxfor route navigation tracking.selfClient.trackEvent: Tracks user interactions and business events (button clicks, backup success/failure, notifications). This wrapsanalytics().trackEvent()internally perselfClientProvider.tsx.Both coexist throughout the codebase—trackScreenView logs when screens are viewed, while selfClient.trackEvent logs what users do within them. This is standard analytics architecture.
app/src/screens/app/ReferralScreen.tsx (1)
44-54: LGTM - Analytics keys updated.All referral event tracking calls updated to use the corrected enum keys (
REFERRALinstead ofREFERAL), maintaining consistency with the analytics constants.app/src/utils/points/registerEvents.ts (3)
125-132: LGTM - Self-referral validation with proper normalization.The validation correctly applies
.trim()to both addresses and uses case-insensitive comparison. The 400 status and clear error message are appropriate. This provides defense-in-depth alongside the validation inrecordEvents.ts.
138-139: LGTM - Address normalization for API.Converting both addresses to lowercase before the API call ensures consistency in the backend.
159-166: LGTM - Improved error handling.The catch block properly handles network errors and provides a user-friendly message, preventing unhandled promise rejections.
app/tests/src/utils/points/registerEvents.test.ts (4)
25-96: Excellent self-referral validation coverage.The test suite thoroughly validates self-referral prevention across exact matches, case-insensitive comparisons, mixed casing, and whitespace handling. The verification that
mockMakeApiRequestis not called prevents unnecessary API requests.
98-136: LGTM - success path well covered.Both 200 and 202 status codes are tested with proper jobId extraction verification.
138-193: Strong error handling coverage.Tests cover explicit error messages, generic fallbacks, and the edge case of missing
job_idin otherwise successful responses.
195-225: Robust error handling for network failures.Tests correctly handle both
Errorobjects and unexpected error types, ensuring users receive consistent, friendly error messages.app/tests/src/utils/points/recordEvents.test.ts (5)
58-129: Comprehensive self-referral validation.The test suite thoroughly covers self-referral detection across exact matches, case variations, whitespace handling, and validates user-friendly error messages. The explicit comment about
trim()functionality at line 92 adds helpful context.
131-155: LGTM - successful flow properly tested.Tests verify both the result and the correct interaction with mocked dependencies (
registerReferralPointsandgetPointsAddress).
198-240: Excellent error handling and logging coverage.Tests properly verify error scenarios from both
getPointsAddressandregisterReferralPoints, map them to user-friendly messages, and confirm console logging behavior with appropriate spy cleanup.
242-265: Strong edge case coverage.Tests handle boundary conditions including empty strings and address prefix variations, ensuring robust self-referral detection.
172-183: Test expectations and implementation are consistent—no changes needed.The test correctly reflects the implementation behavior. When
jobIdis missing, the condition at line 140 ofrecordEvents.tsfails (since all three checks must pass:success, status, andjobId), so the function returns{ success: false, error: response.error }. Since the mock response has no error property,errorisundefined—exactly what the test expects.The UX concern is addressed upstream:
useRegisterReferral.tsline 32 provides a fallback message:result.error || 'Failed to register referral'. Users will see "Failed to register referral" if no specific error is provided, so they're not left without feedback.
Summary by CodeRabbit
New Features
Bug Fixes
Style
Tests
Chores