-
Notifications
You must be signed in to change notification settings - Fork 180
feat: 2.9 #1325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
feat: 2.9 #1325
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces a comprehensive points and referral ecosystem, adding new dashboard screens, referral sharing flows, earn-points orchestration, push notification integration, and account backup incentives. Version bumps applied across Android, iOS, and package manifests; new DINOT-Bold font added; navigation routes expanded; and supporting utilities for API interactions, event tracking, and deep-link referral handling implemented. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant HomeScreen
participant EarnPointsFlow as useEarnPointsFlow Hook
participant VerificationModals as Modal System
participant ReferralFlow as Referral Registration
participant PointsAPI as Points Backend
participant GratificationScreen
User->>HomeScreen: Taps "Earn Points" Button
HomeScreen->>EarnPointsFlow: onEarnPointsPress(skipReferralFlow=false)
alt No Identity Document
EarnPointsFlow->>VerificationModals: Show Identity Verification Modal
User->>VerificationModals: Confirm
VerificationModals->>HomeScreen: Navigate to DocumentOnboarding
else No Points Disclosure
EarnPointsFlow->>VerificationModals: Show Points Disclosure Modal
User->>VerificationModals: Confirm
VerificationModals->>HomeScreen: Navigate to Prove Screen
else Referrer Present & Confirmed
EarnPointsFlow->>ReferralFlow: Register Referral
rect rgb(200, 220, 255)
ReferralFlow->>PointsAPI: POST /referrals/refer
PointsAPI-->>ReferralFlow: Success
end
ReferralFlow->>EarnPointsFlow: Clear Referrer State
EarnPointsFlow->>GratificationScreen: Navigate with points amount
User->>GratificationScreen: Views Points Earned
else No Referrer
EarnPointsFlow->>HomeScreen: Navigate to Points Dashboard
end
sequenceDiagram
actor User
participant Deeplink as Deep Link Handler
participant UserStore as User Store
participant ReferralConfirm as useReferralConfirmation
participant ReferralModal as Modal System
User->>Deeplink: Click Referral Deep Link (referrer=0x123...)
Deeplink->>UserStore: setDeepLinkReferrer("0x123...")
Deeplink->>UserStore: Home Screen Navigation
UserStore->>ReferralConfirm: hasReferrer = true
ReferralConfirm->>ReferralModal: Show Referral Confirmation Modal
alt User Confirms
User->>ReferralModal: Confirm Button
ReferralModal->>UserStore: markReferrerAsRegistered("0x123...")
ReferralModal->>ReferralConfirm: isReferralConfirmed = true
ReferralConfirm->>User: Trigger onConfirmed Callback
else User Dismisses
User->>ReferralModal: Dismiss
ReferralModal->>UserStore: clearDeepLinkReferrer()
ReferralModal->>ReferralConfirm: isReferralConfirmed = false
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Areas requiring focused attention:
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 |
…registerInputs.ts
- Introduced ReferralScreen for sharing referral links via messages, WhatsApp, and clipboard. - Updated navigation to include ReferralScreen in the home stack. - Added new icons for sharing and messaging. - Enhanced points utility functions to check user identity document registration and points disclosure status. - Minor adjustments to existing components for better integration with the new referral feature.
- Added `@react-native-masked-view/masked-view` and `react-native-linear-gradient` to dependencies for improved UI components. - Refactored `PointHistoryList` to accept `ListHeaderComponent` and `onRefreshRef` props for better integration with parent components. - Enhanced `Points` component to manage notification and backup point events, including user feedback through modals. - Updated navigation to use `PointsNavBar` for a more cohesive user experience. - Introduced new utility functions for managing incoming points and point event records.
…into app/two-point-nine
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/components/NavBar/HomeNavBar.tsx (1)
28-28: Remove console.log statements that log potentially sensitive data.Lines 28, 32, 48, 56, and 68 contain console.log statements that log clipboard content, API responses, and selfApp objects. According to coding guidelines, sensitive data including credentials, tokens, and user identifiers should never be logged.
Apply this diff to remove the logging statements:
const handleConsumeToken = async () => { const content = await Clipboard.getString(); - console.log('Consume token content:', content); const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; if (!content || !uuidRegex.test(content)) { - console.log('Invalid clipboard content'); props.navigation.navigate('DeferredLinkingInfo'); return; } if (uuidRegex.test(content)) { try { const response = await fetch( `https://api.self.xyz/consume-deferred-linking-token?token=${content}`, ); const result = await response.json(); if (result.status !== 'success') { throw new Error( `Failed to consume token: ${result.message || 'Unknown error'}`, ); } const selfApp: SelfApp = JSON.parse(result.data.self_app); - console.log('Consume token selfApp:', selfApp); selfClient.getSelfAppState().setSelfApp(selfApp); selfClient.getSelfAppState().startAppListener(selfApp.sessionId); try { Clipboard.setString(''); } catch {} props.navigation.navigate('Prove'); } catch (error) { - console.error('Error consuming token:', error); if ( error instanceof Error && error.message.includes('Token not found or expired') ) { try { Clipboard.setString(''); } catch {} props.navigation.navigate('DeferredLinkingInfo'); } } } else { - console.log('Clipboard content is not a UUID'); } };As per coding guidelines.
Also applies to: 32-32, 48-48, 56-56, 68-68
packages/mobile-sdk-alpha/src/index.ts (1)
30-45: Re-export removed exports or update all consumer code to fix breaking changes.The verification confirms the review concern is valid. The following removed exports are actively used throughout the codebase and must be re-exported from the public API:
ProvingStateType: Used inapp/src/screens/app/LoadingScreen.tsxand test filesgenerateMockDocument: Used inpackages/mobile-sdk-demo/src/screens/GenerateMock.tsx,app/src/screens/dev/CreateMockScreen.tsxextractNameFromDocument: Used inpackages/mobile-sdk-demo/src/screens/RegisterDocument.tsx,packages/mobile-sdk-demo/src/screens/DocumentsList.tsxsignatureAlgorithmToStrictSignatureAlgorithm: Used inpackages/mobile-sdk-demo/src/components/AlgorithmCountryFields.tsx,app/src/screens/dev/CreateMockScreen.tsxEither restore these exports to
packages/mobile-sdk-alpha/src/index.tsor refactor all consumer code to import directly from internal modules. Removing these without updating consumers will break the build.
🧹 Nitpick comments (15)
packages/mobile-sdk-alpha/src/animations/loading/youWin.json (1)
1-1: Animation asset is well-structured and follows project conventions.The Lottie animation JSON is valid and follows the established naming and directory patterns alongside existing loading animations (fail.json, misc.json, etc.). The 3-second celebration sequence with keyframed transforms and opacity changes looks appropriate for a "you win" reward animation.
Minor consideration: Per your project learnings, consider documenting this new animation asset in
packages/mobile-sdk-alpha/README.mdto help developers discover and understand when/how to use it in the loading orchestration flow.packages/mobile-sdk-alpha/src/animations/loading/fail.json (1)
1-1: LGTM! Animation asset minification for bundle optimization.The JSON formatting change reduces bundle size. Functionally equivalent to the previous structure.
For debugging purposes, consider keeping source maps or pretty versions in development builds if animation troubleshooting becomes necessary.
packages/mobile-sdk-alpha/src/hooks/useSafeBottomPadding.ts (1)
29-30: Consider making the height threshold configurable.The 900px threshold is hardcoded and may not work optimally across all device types. Consider making this configurable or using device-specific heuristics.
-export const useSafeBottomPadding = (basePadding: number = 20): number => { +export const useSafeBottomPadding = ( + basePadding: number = 20, + heightThreshold: number = 900 +): number => { const { height: windowHeight } = Dimensions.get('window'); return useMemo(() => { - const isSmallScreen = windowHeight < 900; + const isSmallScreen = windowHeight < heightThreshold; const fallbackPadding = isSmallScreen ? 50 : 0; return basePadding + fallbackPadding; - }, [windowHeight, basePadding]); + }, [windowHeight, basePadding, heightThreshold]); };app/src/components/NavBar/HomeNavBar.tsx (2)
39-39: Extract hardcoded API URL to configuration.The API endpoint
https://api.self.xyz/consume-deferred-linking-tokenis hardcoded. This makes it difficult to test against different environments and violates the DRY principle.Consider moving this to a configuration file or environment variable:
// In a config file export const API_ENDPOINTS = { CONSUME_DEFERRED_LINKING_TOKEN: `${process.env.API_BASE_URL}/consume-deferred-linking-token`, }; // Usage const response = await fetch( `${API_ENDPOINTS.CONSUME_DEFERRED_LINKING_TOKEN}?token=${content}`, );
26-70: Consider adding timeout and error handling improvements to the fetch call.The fetch request on Line 38-40 lacks a timeout, which could cause the UI to hang if the network is slow or the server is unresponsive.
Consider adding a timeout and improving error handling:
const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), 10000); // 10s timeout try { const response = await fetch( `https://api.self.xyz/consume-deferred-linking-token?token=${content}`, { signal: controller.signal } ); clearTimeout(timeoutId); if (!response.ok) { throw new Error(`HTTP ${response.status}: ${response.statusText}`); } const result = await response.json(); // ... rest of logic } catch (error) { clearTimeout(timeoutId); if (error.name === 'AbortError') { // Handle timeout props.navigation.navigate('DeferredLinkingInfo'); return; } // ... existing error handling }app/src/components/NavBar/PointsNavBar.tsx (1)
52-57: Optional: Consider using flex layout instead of fixed-width spacer.The right spacer uses a fixed width to balance the close button. While this works, a more flexible approach would be to use flex layout properties.
Alternative approach:
<NavBar.Container backgroundColor={slate50} barStyle={'dark'} flexDirection="row" justifyContent="space-between" alignItems="center" paddingTop={Math.max(insets.top, 15) + extraYPadding} paddingBottom={10} paddingHorizontal={20} > <View style={{ width: closeButtonWidth }}> {/* Close button */} </View> <View flex={1} alignItems="center" justifyContent="center"> {/* Title */} </View> <View style={{ width: closeButtonWidth }} /> </NavBar.Container>This makes the layout intent clearer, though your current implementation is perfectly functional.
app/src/stores/pointEventStore.ts (3)
40-42: Replace console.error with proper error logging.The store uses
console.errorfor error logging in Lines 40, 61, 73, and 82. According to coding guidelines, errors should be logged through a proper logging adapter that can be configured for different environments and potentially mask sensitive data.Consider using a logger service:
import { logger } from '@/utils/logger'; // In error handlers: logger.error('Error loading point events', { error });This allows for:
- Consistent error logging across the app
- Environment-specific log levels
- Potential error tracking service integration
- Sanitization of sensitive data in logs
Also applies to: 61-62, 73-74, 82-83
47-48: Consider more robust ID generation to prevent collisions.Line 48 generates IDs using
${type}-${Date.now()}. IfaddEventis called multiple times in rapid succession (e.g., < 1ms apart), this could create duplicate IDs.Consider using a UUID library for guaranteed uniqueness:
import { v4 as uuidv4 } from 'uuid'; const newEvent: PointEvent = { id: uuidv4(), // ... rest of properties };Note: The
uuidpackage is already in the dependencies (Line 146 of package.json).
29-43: Consider adding error propagation for better error handling.Currently, all errors are caught and logged, but never propagated to the caller. This makes it difficult for UI components to show error messages or retry failed operations.
Consider either:
- Re-throwing errors after logging them
- Returning error states from the functions
- Adding error state to the store
Example approach:
interface PointEventState { events: PointEvent[]; isLoading: boolean; error: string | null; // ... existing methods } addEvent: async (title, type, points) => { try { set({ error: null }); // ... existing logic } catch (error) { const message = error instanceof Error ? error.message : 'Failed to add event'; logger.error('Error adding point event', { error }); set({ error: message }); throw error; // Re-throw if caller wants to handle it } }Also applies to: 45-63, 65-75, 77-85
app/src/utils/notifications/notificationService.ts (1)
92-103: Consider adding parameter validation.The function silently returns
falsefor any error and doesn't validate thetopicparameter. While this defensive approach prevents crashes, consider:
- Validating that
topicis a non-empty string- Logging errors in development mode to aid debugging
export async function isTopicSubscribed(topic: string): Promise<boolean> { try { + if (!topic || typeof topic !== 'string') { + return false; + } const readiness = await isNotificationSystemReady(); if (!readiness.ready) { return false; } const subscribedTopics = useSettingStore.getState().subscribedTopics; return subscribedTopics.includes(topic); - } catch { + } catch (error) { + if (!isTestEnv) { + console.error('Error checking topic subscription:', error); + } return false; } }app/src/hooks/useRegisterReferral.ts (1)
14-43: Consider simplifying the return pattern.The
registerReferralfunction both returns a result object ({ success, error }) and sets component state (setError,setIsLoading). This creates two sources of truth and might confuse consumers about which to use.Consider one of these approaches:
Option 1: Return only, let consumers manage their own state
const registerReferral = useCallback(async (referrer: string) => { setIsLoading(true); setError(null); try { if (!ethers.isAddress(referrer)) { const errorMessage = 'Invalid referrer address. Must be a valid hex address.'; setError(errorMessage); throw new Error(errorMessage); } const result = await recordReferralPointEvent(referrer); if (!result.success) { const errorMessage = result.error || 'Failed to register referral'; setError(errorMessage); throw new Error(errorMessage); } } finally { setIsLoading(false); } }, []);Option 2: State only (current consumers already check
errorstate based on tests)// Remove return statements, consumers use stateThe current pattern works but could be clearer about the intended usage.
app/src/components/referral/ReferralHeader.tsx (1)
26-26: Consider responsive sizing for header height.The fixed height of 430 might not scale well across different device sizes, particularly smaller devices where this would consume a significant portion of the screen.
Consider using responsive sizing:
<View height="40%" minHeight={300} maxHeight={430} position="relative" overflow="hidden">Or if the 430 height is from design specs, consider extracting it as a named constant for clarity.
app/src/utils/referralShare.ts (1)
14-28: Consider adding user feedback for share failures.The
shareViaNativefunction silently fails when an error occurs (only logs to console). UnlikeshareViaSMSandshareViaWhatsApp, it doesn't show an alert to the user. Consider whether users should be notified when sharing fails.If user feedback is desired, apply this diff:
export const shareViaNative = async ( message: string, url: string, title: string, ): Promise<void> => { try { await Share.share({ message, title, url, }); } catch (error) { console.error('Error sharing:', error); + Alert.alert('Error', 'Failed to share. Please try again.'); } };app/src/hooks/useTestReferralFlow.ts (1)
24-44: Consider preventing concurrent test flow triggers.The
triggerReferralFlowfunction directly resets state without checking if a referral flow is already in progress. If called multiple times rapidly (e.g., double-tap in the UI), this could cause race conditions.Consider adding a guard:
+ const isFlowInProgress = useRef(false); + const triggerReferralFlow = useCallback(() => { if (IS_DEV_MODE) { + if (isFlowInProgress.current) { + console.log('[DEV MODE] Referral flow already in progress, skipping'); + return; + } + isFlowInProgress.current = true; + const testReferrer = TEST_REFERRER; const store = useUserStore.getState(); // Always reset state for full flow testing console.log('[DEV MODE] Resetting test state for full flow:'); console.log(' - Clearing all registered referrers'); // Clear the "already registered" flag for all referrers useUserStore.setState({ registeredReferrers: new Set<string>() }); console.log(' - Referrer will be treated as first-time registration'); console.log( '[DEV MODE] Simulating referral flow with referrer:', testReferrer, ); store.setDeepLinkReferrer(testReferrer); + + // Reset flag after a delay + setTimeout(() => { + isFlowInProgress.current = false; + }, 1000); // Trigger the referral confirmation modal // The useReferralConfirmation hook will handle showing the modal } }, []);app/src/hooks/useReferralRegistration.ts (1)
60-80: Consider the implications of fire-and-forget registration.The
register()function is called withoutawaiton line 80, making it a fire-and-forget operation. WhileisRegisteringReferralprevents concurrent calls, this pattern means:
- The effect completes before registration finishes
- If the component unmounts during registration, there's no cleanup
- Errors during registration are logged but don't affect the component state
If this is intentional for the referral flow, consider adding a comment explaining the design choice. Otherwise, consider whether the component should track the registration lifecycle more explicitly.
Add a clarifying comment:
if (IS_DEV_MODE) { console.log('[useReferralRegistration] Registering referrer:', referrer); } // Register the referral + // Note: Fire-and-forget pattern - registration continues even if component unmounts + // isRegisteringReferral from useRegisterReferral prevents concurrent registrations const register = async () => {
| <Button | ||
| backgroundColor={black} | ||
| paddingHorizontal={20} | ||
| paddingVertical={14} | ||
| borderRadius={5} | ||
| height={52} | ||
| > | ||
| <Text | ||
| fontFamily="DIN OT" | ||
| fontSize={16} | ||
| color={white} | ||
| textAlign="center" | ||
| > | ||
| Explore apps | ||
| </Text> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing onPress handler leaves CTA inert. The bottom “Explore apps” CTA renders visually, but without an onPress it never fires, so users can’t reach the Explore flow. Please wire the button to the intended navigation/action before release.
🤖 Prompt for AI Agents
In app/src/components/NavBar/Points.tsx around lines 503–517 the “Explore apps”
Button is missing an onPress handler so the CTA is inert; add an onPress prop
that triggers the intended navigation/action (for example call
navigation.navigate('Explore') or props.onExplore) and wire the component to the
appropriate navigation context (use useNavigation from @react-navigation/native
or accept a handler via props if this component doesn't have navigation access).
Ensure the handler is defined nearby, passed into the Button as onPress={...},
and add a descriptive accessibilityLabel for the button.
| <Text | ||
| fontFamily="SF Pro" | ||
| fontSize={24} | ||
| lineHeight={29} | ||
| color={black} | ||
| > | ||
| <ArrowLeft width={24} height={24} /> | ||
| </Text> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Remove unnecessary Text wrapper around SVG.
The ArrowLeft SVG component is wrapped in a Text component with font-related props that have no effect on the SVG rendering. This is semantically incorrect and adds unnecessary markup.
Apply this diff to remove the Text wrapper:
<View
backgroundColor={white}
width={46}
height={46}
borderRadius={60}
alignItems="center"
justifyContent="center"
>
- <Text
- fontFamily="SF Pro"
- fontSize={24}
- lineHeight={29}
- color={black}
- >
- <ArrowLeft width={24} height={24} />
- </Text>
+ <ArrowLeft width={24} height={24} />
</View>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Text | |
| fontFamily="SF Pro" | |
| fontSize={24} | |
| lineHeight={29} | |
| color={black} | |
| > | |
| <ArrowLeft width={24} height={24} /> | |
| </Text> | |
| <ArrowLeft width={24} height={24} /> |
🤖 Prompt for AI Agents
In app/src/components/referral/ReferralHeader.tsx around lines 47 to 54, the
ArrowLeft SVG is unnecessarily wrapped in a Text component with font props;
remove the Text wrapper and render <ArrowLeft width={24} height={24} /> directly
in its place, preserving any layout or spacing by using the surrounding
container props (or a neutral wrapper like View/Box if needed) so no
font-related props remain applied to the SVG.
| // User has completed both checks | ||
| if (!skipReferralFlow && hasReferrer && isReferralConfirmed === true) { | ||
| if (IS_DEV_MODE) { | ||
| console.log( | ||
| '[DEV MODE] 3. Both checks passed, proceeding with referral flow...', | ||
| ); | ||
| } | ||
| await handleReferralFlow(); | ||
| } else { | ||
| // Just go to points upon pressing "Earn Points" button | ||
| if (!hasReferrer) { | ||
| navigation.navigate('Points'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent the Earn Points button from becoming a no-op
Line 229-242 currently require isReferralConfirmed === true before navigating anywhere when a referrer is present. If the user has a referrer but has not yet confirmed it (false/undefined), pressing “Earn Points” does nothing—no referral flow and no fallback navigation—effectively trapping the user. Recommend falling back to the Points screen when the referral flow is skipped or not ready yet, while still returning early on successful referral registration.
- if (!skipReferralFlow && hasReferrer && isReferralConfirmed === true) {
- if (IS_DEV_MODE) {
- console.log(
- '[DEV MODE] 3. Both checks passed, proceeding with referral flow...',
- );
- }
- await handleReferralFlow();
- } else {
- // Just go to points upon pressing "Earn Points" button
- if (!hasReferrer) {
- navigation.navigate('Points');
- }
- }
+ if (!skipReferralFlow && hasReferrer) {
+ if (isReferralConfirmed === true) {
+ if (IS_DEV_MODE) {
+ console.log(
+ '[DEV MODE] 3. Both checks passed, proceeding with referral flow...',
+ );
+ }
+ await handleReferralFlow();
+ return;
+ }
+ }
+
+ navigation.navigate('Points');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // User has completed both checks | |
| if (!skipReferralFlow && hasReferrer && isReferralConfirmed === true) { | |
| if (IS_DEV_MODE) { | |
| console.log( | |
| '[DEV MODE] 3. Both checks passed, proceeding with referral flow...', | |
| ); | |
| } | |
| await handleReferralFlow(); | |
| } else { | |
| // Just go to points upon pressing "Earn Points" button | |
| if (!hasReferrer) { | |
| navigation.navigate('Points'); | |
| } | |
| } | |
| // User has completed both checks | |
| if (!skipReferralFlow && hasReferrer) { | |
| if (isReferralConfirmed === true) { | |
| if (IS_DEV_MODE) { | |
| console.log( | |
| '[DEV MODE] 3. Both checks passed, proceeding with referral flow...', | |
| ); | |
| } | |
| await handleReferralFlow(); | |
| return; | |
| } | |
| } | |
| navigation.navigate('Points'); |
🤖 Prompt for AI Agents
In app/src/hooks/useEarnPointsFlow.ts around lines 229-242 the current branch
only calls handleReferralFlow when hasReferrer is true AND isReferralConfirmed
=== true, which leaves users with a referrer but unconfirmed referral stuck with
no navigation; change the logic so that: if skipReferralFlow is false and
hasReferrer and isReferralConfirmed === true then await handleReferralFlow();
otherwise (including hasReferrer but isReferralConfirmed false/undefined, or
skipReferralFlow true) fall back to navigation.navigate('Points'); ensure the
early-return/flow behavior remains the same (i.e., still await successful
referral registration when confirmed).
app/src/hooks/useReferralMessage.ts
Outdated
| // const baseDomain = 'https://referral.self.xyz'; | ||
| const baseDomain = 'https://redirectselfxyzwebpages.vercel.app'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CRITICAL: Production URL commented out, using development/staging URL.
Lines 18-19 show the production URL (https://referral.self.xyz) is commented out, and a Vercel deployment URL is being used instead. This could lead to:
- Broken referral links in production
- Users sharing links that point to staging/dev environment
- Potential data leakage if staging environment is less secure
This should be controlled via environment configuration:
const baseDomain = process.env.REFERRAL_BASE_URL || 'https://referral.self.xyz';Or at minimum, ensure the correct URL is uncommented before merging to production.
🤖 Prompt for AI Agents
In app/src/hooks/useReferralMessage.ts around lines 18-19, the production
referral domain is commented out and a Vercel deployment URL is hardcoded, which
can cause broken/incorrect referral links; replace the hardcoded value with a
configurable environment lookup that uses process.env.REFERRAL_BASE_URL and
falls back to the production URL 'https://referral.self.xyz' if not set, and
remove the commented-out production string so the code always uses the
env-driven value in production builds.
| setTimeout(() => { | ||
| if (navigationRef.isReady()) { | ||
| navigationRef.navigate('AccountVerifiedSuccess'); | ||
| navigationRef.navigate({ name: 'AccountVerifiedSuccess', params: undefined }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent navigation patterns and formatting issue.
These three navigation calls use object-based route descriptors, while other calls in the same file (lines 142, 187, 231, 235, 263, 266, 270, etc.) still use string-based navigation. This inconsistency makes the codebase harder to maintain and increases the risk of type-safety issues.
Additionally, there's a Prettier formatting issue on line 149 that's blocking the CI pipeline.
Consider standardizing all navigation calls to use the navigateIfReady helper or consistently use one pattern:
addListener(SdkEvents.PROVING_ACCOUNT_VERIFIED_SUCCESS, () => {
setTimeout(() => {
if (navigationRef.isReady()) {
- navigationRef.navigate({ name: 'AccountVerifiedSuccess', params: undefined });
+ navigationRef.navigate({
+ name: 'AccountVerifiedSuccess',
+ params: undefined,
+ });
}
}, 1000);
});Or better yet, use the existing navigateIfReady helper for type safety:
addListener(SdkEvents.PROVING_ACCOUNT_VERIFIED_SUCCESS, () => {
setTimeout(() => {
- if (navigationRef.isReady()) {
- navigationRef.navigate({ name: 'AccountVerifiedSuccess', params: undefined });
- }
+ navigateIfReady('AccountVerifiedSuccess');
}, 1000);
});Apply similar changes to lines 160 and 162.
Also applies to: 160-160, 162-162
🧰 Tools
🪛 GitHub Check: build-deps
[warning] 149-149:
Replace ·name:·'AccountVerifiedSuccess',·params:·undefined with ⏎············name:·'AccountVerifiedSuccess',⏎············params:·undefined,⏎·········
🪛 GitHub Check: workspace-lint
[warning] 149-149:
Replace ·name:·'AccountVerifiedSuccess',·params:·undefined with ⏎············name:·'AccountVerifiedSuccess',⏎············params:·undefined,⏎·········
🤖 Prompt for AI Agents
In app/src/providers/selfClientProvider.tsx around lines 149, 160 and 162, there
are inconsistent navigation calls using object-based descriptors (and a Prettier
formatting issue on line 149); replace those object-based navigation calls with
the project’s navigateIfReady helper (or otherwise standardize to the
string-based pattern used elsewhere) so all navigations in this file use the
same helper and types, and fix the formatting on line 149 to satisfy Prettier;
apply the same change to lines 160 and 162 and run the formatter to ensure CI
passes.
| const handleClearBackupEventsPress = () => { | ||
| Alert.alert( | ||
| 'Clear Backup Events', | ||
| 'Are you sure you want to clear all backup point events from local storage?\n\nThis will remove backup events from your point history.', | ||
| [ | ||
| { | ||
| text: 'Cancel', | ||
| style: 'cancel', | ||
| }, | ||
| { | ||
| text: 'Clear', | ||
| style: 'destructive', | ||
| onPress: async () => { | ||
| const events = usePointEventStore.getState().events; | ||
| const backupEvents = events.filter( | ||
| event => event.type === 'backup', | ||
| ); | ||
| for (const event of backupEvents) { | ||
| await usePointEventStore.getState().removeEvent(event.id); | ||
| } | ||
| Alert.alert('Success', 'Backup events cleared successfully.', [ | ||
| { text: 'OK' }, | ||
| ]); | ||
| }, | ||
| }, | ||
| ], | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider error handling and batch operations for event removal.
The current implementation removes backup events sequentially without handling potential failures:
- If
removeEvent()fails for any event, the user won't be notified which events failed to remove - Sequential async operations could be slower than batch processing
- The success alert shows even if some removals failed
Consider this improved implementation:
const handleClearBackupEventsPress = () => {
Alert.alert(
'Clear Backup Events',
'Are you sure you want to clear all backup point events from local storage?\n\nThis will remove backup events from your point history.',
[
{
text: 'Cancel',
style: 'cancel',
},
{
text: 'Clear',
style: 'destructive',
onPress: async () => {
- const events = usePointEventStore.getState().events;
- const backupEvents = events.filter(
- event => event.type === 'backup',
- );
- for (const event of backupEvents) {
- await usePointEventStore.getState().removeEvent(event.id);
- }
- Alert.alert('Success', 'Backup events cleared successfully.', [
- { text: 'OK' },
- ]);
+ try {
+ const events = usePointEventStore.getState().events;
+ const backupEvents = events.filter(
+ event => event.type === 'backup',
+ );
+
+ const results = await Promise.allSettled(
+ backupEvents.map(event =>
+ usePointEventStore.getState().removeEvent(event.id)
+ )
+ );
+
+ const failures = results.filter(r => r.status === 'rejected');
+ if (failures.length > 0) {
+ Alert.alert(
+ 'Partial Success',
+ `Cleared ${results.length - failures.length} of ${results.length} backup events.`,
+ [{ text: 'OK' }]
+ );
+ } else {
+ Alert.alert('Success', 'Backup events cleared successfully.', [
+ { text: 'OK' },
+ ]);
+ }
+ } catch (error) {
+ console.error('Error clearing backup events:', error);
+ Alert.alert('Error', 'Failed to clear backup events.', [
+ { text: 'OK' },
+ ]);
+ }
},
},
],
);
};🤖 Prompt for AI Agents
In app/src/screens/dev/DevSettingsScreen.tsx around lines 511 to 538, the
clear-backup-events handler removes events sequentially and always shows a
success alert; change it to perform removals in parallel with Promise.allSettled
(or a batch API if available), collect results, and surface failures to the
user. Specifically, get the backupEvents array, map to calls to
removeEvent(event.id), await Promise.allSettled, build lists of succeeded and
failed ids (or errors), and then show an Alert that reports how many succeeded
and which (or how many) failed and include error details for failures; ensure
individual remove errors are caught/recorded so the overall UI does not
incorrectly report success when some removals failed.
app/src/screens/home/HomeScreen.tsx
Outdated
| Record<string, { data: IDDocument; metadata: DocumentMetadata }> | ||
| >({}); | ||
| const [loading, setLoading] = useState(true); | ||
| const [selfPoints] = useState(312); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace hard-coded Self Points total
useState(312) freezes the displayed balance at 312 for every user, so the UI never reflects the real points count or updates after earning more. This breaks the core feature we’re shipping and misleads users. Please source the balance from whichever store/hook is tracking points (e.g., points API snapshot or event store) and keep it reactive instead of hard-coding it.
🤖 Prompt for AI Agents
In app/src/screens/home/HomeScreen.tsx around line 55, the component currently
sets selfPoints using a hard-coded useState(312) which freezes the displayed
balance; replace this with a reactive source: import and use the app's points
store/hook or selector (e.g., usePoints(), useSelector(state =>
state.points.total) or the points API snapshot) to initialize and subscribe to
the current balance, wire it into state (or derive it directly without local
state), and ensure updates are handled (useEffect to subscribe/unsubscribe or
rely on the hook/selector) so the UI reflects real-time point changes instead of
the static 312 value.
| registerReferral: mockRegisterReferral, | ||
| isLoading: false, | ||
| error: null, | ||
| }); | ||
|
|
||
| const mockStore = { | ||
| isReferrerRegistered: mockIsReferrerRegistered, | ||
| markReferrerAsRegistered: mockMarkReferrerAsRegistered, | ||
| }; | ||
|
|
||
| mockUseUserStore.getState = jest.fn(() => mockStore as any); | ||
|
|
||
| mockRegisterReferral.mockResolvedValue({ success: true }); | ||
| mockIsReferrerRegistered.mockReturnValue(false); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix mocked store wiring before calling selectors
Right now the mocked useUserStore never returns the store state, so every selector call inside the hook receives undefined. As soon as the hook hits isReferrerRegistered(referrer) the test suite will throw a TypeError, so none of these cases actually run. Please have the mock forward the selector to the mock store (and keep getState in sync) before invoking the hook.
- const mockStore = {
- isReferrerRegistered: mockIsReferrerRegistered,
- markReferrerAsRegistered: mockMarkReferrerAsRegistered,
- };
-
- mockUseUserStore.getState = jest.fn(() => mockStore as any);
+ const mockStore = {
+ deepLinkReferrer: null,
+ isReferrerRegistered: mockIsReferrerRegistered,
+ markReferrerAsRegistered: mockMarkReferrerAsRegistered,
+ };
+
+ mockUseUserStore.mockImplementation(selector =>
+ selector(mockStore as any),
+ );
+ mockUseUserStore.getState = jest.fn(() => mockStore as any);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| registerReferral: mockRegisterReferral, | |
| isLoading: false, | |
| error: null, | |
| }); | |
| const mockStore = { | |
| isReferrerRegistered: mockIsReferrerRegistered, | |
| markReferrerAsRegistered: mockMarkReferrerAsRegistered, | |
| }; | |
| mockUseUserStore.getState = jest.fn(() => mockStore as any); | |
| mockRegisterReferral.mockResolvedValue({ success: true }); | |
| mockIsReferrerRegistered.mockReturnValue(false); | |
| }); | |
| registerReferral: mockRegisterReferral, | |
| isLoading: false, | |
| error: null, | |
| }); | |
| const mockStore = { | |
| deepLinkReferrer: null, | |
| isReferrerRegistered: mockIsReferrerRegistered, | |
| markReferrerAsRegistered: mockMarkReferrerAsRegistered, | |
| }; | |
| mockUseUserStore.mockImplementation(selector => | |
| selector(mockStore as any), | |
| ); | |
| mockUseUserStore.getState = jest.fn(() => mockStore as any); | |
| mockRegisterReferral.mockResolvedValue({ success: true }); | |
| mockIsReferrerRegistered.mockReturnValue(false); | |
| }); |
🤖 Prompt for AI Agents
In app/tests/src/hooks/useReferralRegistration.test.ts around lines 45-59, the
mocked useUserStore currently doesn't forward selectors to the fake store so
selectors receive undefined; update the mock before invoking the hook by making
getState return the mockStore and configuring the mock useUserStore to call the
provided selector with mockStore (i.e., forward the selector to mockStore) so
both direct getState calls and selector-based calls inside the hook operate on
the same mockStore state.
| // Skip parsing for disclosure if passport is already parsed | ||
| // Re-parsing would overwrite the alternative CSCA used during registration and is unnecessary | ||
| // skip also the register circuit as the passport already got parsed in during the dsc step | ||
| console.log('circuitType', circuitType); | ||
| if (circuitType !== 'dsc') { | ||
| console.log('skipping id document parsing'); | ||
| actor.send({ type: 'FETCH_DATA' }); | ||
| selfClient.trackEvent(ProofEvents.FETCH_DATA_STARTED); | ||
| } else { | ||
| // Skip parsing for disclosure if passport is already parsed | ||
| // Re-parsing would overwrite the alternative CSCA used during registration and is unnecessary | ||
| const isParsed = passportData.passportMetadata !== undefined; | ||
| if (circuitType === 'disclose' && isParsed) { | ||
| actor.send({ type: 'FETCH_DATA' }); | ||
| selfClient.trackEvent(ProofEvents.FETCH_DATA_STARTED); | ||
| } else { | ||
| actor.send({ type: 'PARSE_ID_DOCUMENT' }); | ||
| selfClient.trackEvent(ProofEvents.PARSE_ID_DOCUMENT_STARTED); | ||
| } | ||
| actor.send({ type: 'PARSE_ID_DOCUMENT' }); | ||
| selfClient.trackEvent(ProofEvents.PARSE_ID_DOCUMENT_STARTED); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debug console.log statements.
The console.log statements on lines 904 and 906 appear to be debugging artifacts that should be removed before merging to production.
Apply this diff to remove the console.log statements:
// Skip parsing for disclosure if passport is already parsed
// Re-parsing would overwrite the alternative CSCA used during registration and is unnecessary
// skip also the register circuit as the passport already got parsed in during the dsc step
- console.log('circuitType', circuitType);
if (circuitType !== 'dsc') {
- console.log('skipping id document parsing');
actor.send({ type: 'FETCH_DATA' });
selfClient.trackEvent(ProofEvents.FETCH_DATA_STARTED);
} else {If these logs are needed for debugging, consider using the existing logProofEvent infrastructure instead:
selfClient.logProofEvent('info', 'Skipping ID document parsing', context, {
circuitType,
reason: 'Already parsed during DSC step',
});🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/proving/provingMachine.ts around lines 901 to
912, remove the two debug console.log statements currently printing circuitType
and 'skipping id document parsing' (lines ~904 and ~906). If structured logging
is desired instead of removal, replace those console.log calls with the existing
logging utility (e.g., selfClient.logProofEvent with level 'info' and include
circuitType and reason), keeping the actor.send and selfClient.trackEvent calls
unchanged.
…1361) * fix whitespace * move effects for fetching points and incoming points to hooks, add items to deps array so that they refresh when we expect points to change. * cleanup
* wire cloudbackup with points * wire cloudbackup with points * Remove redundant setSelfPoints()
* add signature and simplify POST api requests * better gitleaks ignore * update toml * have gitguardian ignore gitleaks * add buffer lib * update api with 202 success code
* Setup and add turnkey utils * update CloudBackupScreen * fix: turnkey * add yarn.lock * lint * add podfile.lock * fix deeplink * fix tests: mock turnkey * yarn nice * update .gitleaksignore * patch react-native-svg * fix patch-package * fix lineHeight * points.tsx: fix lineHeight * fix: recover with turnkey disabled state * fix turnkey flow * fix: address generation * minify animations (#1368) * fix pipelines * fix false positives * fix gitguardian --------- Co-authored-by: Justin Hernandez <[email protected]>
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Version Updates