-
Notifications
You must be signed in to change notification settings - Fork 187
feat: improve mixpanel flush strategy #960
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
Changes from all commits
900e655
6775a91
9d70a49
95ee4db
7aa3a7e
af94161
007321b
f6cb0ef
8fd2de0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,11 @@ jest.mock( | |||||||||||||||||||||||||||||||||||||
| { virtual: true }, | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| jest.mock('@env', () => ({ | ||||||||||||||||||||||||||||||||||||||
| ENABLE_DEBUG_LOGS: 'false', | ||||||||||||||||||||||||||||||||||||||
| MIXPANEL_NFC_PROJECT_TOKEN: 'test-token', | ||||||||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| global.FileReader = class { | ||||||||||||||||||||||||||||||||||||||
| constructor() { | ||||||||||||||||||||||||||||||||||||||
| this.onload = null; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -202,13 +207,30 @@ jest.mock('react-native-nfc-manager', () => ({ | |||||||||||||||||||||||||||||||||||||
| // Mock react-native-passport-reader | ||||||||||||||||||||||||||||||||||||||
| jest.mock('react-native-passport-reader', () => ({ | ||||||||||||||||||||||||||||||||||||||
| default: { | ||||||||||||||||||||||||||||||||||||||
| initialize: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| configure: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| scanPassport: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| readPassport: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| cancelPassportRead: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| trackEvent: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| flush: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| reset: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const { NativeModules } = require('react-native'); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| NativeModules.PassportReader = { | ||||||||||||||||||||||||||||||||||||||
| configure: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| scanPassport: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| trackEvent: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| flush: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+220
to
+228
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Keep NativeModules mock consistent and complete
Apply this diff: const { NativeModules } = require('react-native');
NativeModules.PassportReader = {
configure: jest.fn(),
- scanPassport: jest.fn(),
+ scanPassport: jest.fn(),
+ // Alias for parity with d.ts and default export
+ scan: jest.fn(),
trackEvent: jest.fn(),
flush: jest.fn(),
};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| jest.mock('@react-native-community/netinfo', () => ({ | ||||||||||||||||||||||||||||||||||||||
| addEventListener: jest.fn(() => jest.fn()), | ||||||||||||||||||||||||||||||||||||||
| fetch: jest.fn(() => Promise.resolve({ isConnected: true })), | ||||||||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+229
to
+232
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicate NetInfo mock overrides earlier one (breaks This second Apply this diff to remove the duplicate: -jest.mock('@react-native-community/netinfo', () => ({
- addEventListener: jest.fn(() => jest.fn()),
- fetch: jest.fn(() => Promise.resolve({ isConnected: true })),
-}));Then, update the first NetInfo mock (Lines 147–160) to return an unsubscribe from // In the first mock block (existing):
addEventListener: jest.fn(() => jest.fn()), // returns unsubscribe
fetch: jest.fn().mockResolvedValue({ isConnected: true, isInternetReachable: true }),If you want, I can push a combined single mock that covers 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Mock @stablelib packages | ||||||||||||||||||||||||||||||||||||||
| jest.mock('@stablelib/cbor', () => ({ | ||||||||||||||||||||||||||||||||||||||
| encode: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| // SPDX-FileCopyrightText: 2025 Social Connect Labs, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
| // NOTE: Converts to Apache-2.0 on 2029-06-11 per LICENSE. | ||
|
|
||
| // Web mock for react-native-passport-reader | ||
| export const reset = async () => { | ||
| // No-op for web | ||
| return Promise.resolve(); | ||
| }; | ||
|
|
||
| export const scan = async () => { | ||
| throw new Error('NFC scanning is not supported on web'); | ||
| }; | ||
transphorm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -45,6 +45,7 @@ import { ExpandableBottomLayout } from '@/layouts/ExpandableBottomLayout'; | |||||||||||||||||||||||||||||||||||||||||||
| import { useFeedback } from '@/providers/feedbackProvider'; | ||||||||||||||||||||||||||||||||||||||||||||
| import { storePassportData } from '@/providers/passportDataProvider'; | ||||||||||||||||||||||||||||||||||||||||||||
| import useUserStore from '@/stores/userStore'; | ||||||||||||||||||||||||||||||||||||||||||||
| import { flushAllAnalytics, trackNfcEvent } from '@/utils/analytics'; | ||||||||||||||||||||||||||||||||||||||||||||
| import { black, slate100, slate400, slate500, white } from '@/utils/colors'; | ||||||||||||||||||||||||||||||||||||||||||||
| import { sendFeedbackEmail } from '@/utils/email'; | ||||||||||||||||||||||||||||||||||||||||||||
| import { dinot } from '@/utils/fonts'; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -79,6 +80,7 @@ type PassportNFCScanRoute = RouteProp< | |||||||||||||||||||||||||||||||||||||||||||
| const PassportNFCScanScreen: React.FC = () => { | ||||||||||||||||||||||||||||||||||||||||||||
| const selfClient = useSelfClient(); | ||||||||||||||||||||||||||||||||||||||||||||
| const { trackEvent } = selfClient; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| const navigation = useNavigation(); | ||||||||||||||||||||||||||||||||||||||||||||
| const route = useRoute<PassportNFCScanRoute>(); | ||||||||||||||||||||||||||||||||||||||||||||
| const { showModal } = useFeedback(); | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -137,6 +139,7 @@ const PassportNFCScanScreen: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| const openErrorModal = useCallback( | ||||||||||||||||||||||||||||||||||||||||||||
| (message: string) => { | ||||||||||||||||||||||||||||||||||||||||||||
| flushAllAnalytics(); | ||||||||||||||||||||||||||||||||||||||||||||
| showModal({ | ||||||||||||||||||||||||||||||||||||||||||||
| titleText: 'NFC Scan Error', | ||||||||||||||||||||||||||||||||||||||||||||
| bodyText: message, | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -206,6 +209,9 @@ const PassportNFCScanScreen: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||
| trackEvent(PassportEvents.NFC_SCAN_FAILED, { | ||||||||||||||||||||||||||||||||||||||||||||
| error: 'timeout', | ||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||
| trackNfcEvent(PassportEvents.NFC_SCAN_FAILED, { | ||||||||||||||||||||||||||||||||||||||||||||
| error: 'timeout', | ||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||
| openErrorModal('Scan timed out. Please try again.'); | ||||||||||||||||||||||||||||||||||||||||||||
| setIsNfcSheetOpen(false); | ||||||||||||||||||||||||||||||||||||||||||||
| }, 30000); | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -249,10 +255,14 @@ const PassportNFCScanScreen: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||
| passportData = parseScanResponse(scanResponse); | ||||||||||||||||||||||||||||||||||||||||||||
| } catch (e: unknown) { | ||||||||||||||||||||||||||||||||||||||||||||
| console.error('Parsing NFC Response Unsuccessful'); | ||||||||||||||||||||||||||||||||||||||||||||
| const errMsg = sanitizeErrorMessage( | ||||||||||||||||||||||||||||||||||||||||||||
| e instanceof Error ? e.message : String(e), | ||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||
| trackEvent(PassportEvents.NFC_RESPONSE_PARSE_FAILED, { | ||||||||||||||||||||||||||||||||||||||||||||
| error: sanitizeErrorMessage( | ||||||||||||||||||||||||||||||||||||||||||||
| e instanceof Error ? e.message : String(e), | ||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||
| error: errMsg, | ||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||
| trackNfcEvent(PassportEvents.NFC_RESPONSE_PARSE_FAILED, { | ||||||||||||||||||||||||||||||||||||||||||||
| error: errMsg, | ||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -317,10 +327,14 @@ const PassportNFCScanScreen: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| console.error('Passport Parsed Failed:', e); | ||||||||||||||||||||||||||||||||||||||||||||
| const errMsg = sanitizeErrorMessage( | ||||||||||||||||||||||||||||||||||||||||||||
| e instanceof Error ? e.message : String(e), | ||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||
| trackEvent(PassportEvents.PASSPORT_PARSE_FAILED, { | ||||||||||||||||||||||||||||||||||||||||||||
| error: sanitizeErrorMessage( | ||||||||||||||||||||||||||||||||||||||||||||
| e instanceof Error ? e.message : String(e), | ||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||
| error: errMsg, | ||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||
| trackNfcEvent(PassportEvents.PASSPORT_PARSE_FAILED, { | ||||||||||||||||||||||||||||||||||||||||||||
| error: errMsg, | ||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -335,8 +349,13 @@ const PassportNFCScanScreen: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||
| ).toFixed(2); | ||||||||||||||||||||||||||||||||||||||||||||
| console.error('NFC Scan Unsuccessful:', e); | ||||||||||||||||||||||||||||||||||||||||||||
| const message = e instanceof Error ? e.message : String(e); | ||||||||||||||||||||||||||||||||||||||||||||
| const sanitized = sanitizeErrorMessage(message); | ||||||||||||||||||||||||||||||||||||||||||||
| trackEvent(PassportEvents.NFC_SCAN_FAILED, { | ||||||||||||||||||||||||||||||||||||||||||||
| error: sanitizeErrorMessage(message), | ||||||||||||||||||||||||||||||||||||||||||||
| error: sanitized, | ||||||||||||||||||||||||||||||||||||||||||||
| duration_seconds: parseFloat(scanDurationSeconds), | ||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||
| trackNfcEvent(PassportEvents.NFC_SCAN_FAILED, { | ||||||||||||||||||||||||||||||||||||||||||||
| error: sanitized, | ||||||||||||||||||||||||||||||||||||||||||||
| duration_seconds: parseFloat(scanDurationSeconds), | ||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+352
to
360
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use the sanitized message for the error modal to prevent accidental PII exposure You're sanitizing the message for analytics but still passing the raw message to the user-facing modal. Surface the sanitized version there too to avoid exposing MRZ or other sensitive values. Apply this change near the existing code: - const message = e instanceof Error ? e.message : String(e);
- const sanitized = sanitizeErrorMessage(message);
+ const message = e instanceof Error ? e.message : String(e);
+ const sanitized = sanitizeErrorMessage(message);
...
- openErrorModal(message);
+ openErrorModal(sanitized);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||
| openErrorModal(message); | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -350,6 +369,7 @@ const PassportNFCScanScreen: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||
| setIsNfcSheetOpen(false); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } else if (isNfcSupported) { | ||||||||||||||||||||||||||||||||||||||||||||
| flushAllAnalytics(); | ||||||||||||||||||||||||||||||||||||||||||||
| if (Platform.OS === 'ios') { | ||||||||||||||||||||||||||||||||||||||||||||
| Linking.openURL('App-Prefs:root=General&path=About'); | ||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -376,6 +396,7 @@ const PassportNFCScanScreen: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| const onCancelPress = async () => { | ||||||||||||||||||||||||||||||||||||||||||||
| flushAllAnalytics(); | ||||||||||||||||||||||||||||||||||||||||||||
| const hasValidDocument = await hasAnyValidRegisteredDocument(selfClient); | ||||||||||||||||||||||||||||||||||||||||||||
| if (hasValidDocument) { | ||||||||||||||||||||||||||||||||||||||||||||
| navigateToHome(); | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
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.
💡 Verification agent
🧩 Analysis chain
Mock parity with types: add
scanalias to match d.ts and potential callsitesThe default export for
react-native-passport-readeronly exposesscanPassport, while the d.ts (and likely some callsites) referencescan. This will cause test failures or undefined calls in environments that import the default module instead ofNativeModules.Apply this diff to add a
scanalias:jest.mock('react-native-passport-reader', () => ({ default: { configure: jest.fn(), - scanPassport: jest.fn(), + scanPassport: jest.fn(), + // Alias to align with TS types/callsites + scan: jest.fn(), readPassport: jest.fn(), cancelPassportRead: jest.fn(), trackEvent: jest.fn(), flush: jest.fn(), reset: jest.fn(), }, }));Run to spot any lingering
scanPassport(vsscan(mismatches:🏁 Script executed:
Length of output: 228
🏁 Script executed:
Length of output: 1200
Add
scanalias to the PassportReader mock to cover Android callsitesYour Android scanner helper calls
PassportReader.scan(…)(app/src/utils/nfcScanner.ts line 128), but your Jest setup only mocksscanPassport. Without ascanmock, tests hittingscanAndroidwill fail withundefined is not a function.Apply this diff in app/jest.setup.js (around lines 203–213):
jest.mock('react-native-passport-reader', () => ({ default: { configure: jest.fn(), scanPassport: jest.fn(), + // Alias to align with TS types/callsites (e.g. scanAndroid) + scan: jest.fn(), readPassport: jest.fn(), cancelPassportRead: jest.fn(), trackEvent: jest.fn(), flush: jest.fn(), reset: jest.fn(), }, }));— without this, any
PassportReader.scan(...)calls will be unmocked and break your Android tests.📝 Committable suggestion
🤖 Prompt for AI Agents