Skip to content

CU-868ex18rd Fix for splash screen#65

Merged
ucswift merged 3 commits intomasterfrom
develop
Aug 26, 2025
Merged

CU-868ex18rd Fix for splash screen#65
ucswift merged 3 commits intomasterfrom
develop

Conversation

@ucswift
Copy link
Member

@ucswift ucswift commented Aug 25, 2025

Summary by CodeRabbit

  • New Features

    • Auth hydration now runs on app start to restore user state.
  • Improvements

    • Splash screen hides only after auth is settled, with a short debounce and a longer max display to reduce flicker.
    • Onboarding and signin redirects clarified for first-time and returning users.
    • More detailed logging around splash and auth flows for observability.
  • Bug Fixes

    • Logout reliably clears persisted credentials and logs failures.
  • Tests

    • Added tests for splash behavior, auth hydration, onboarding redirects, and logout.

@coderabbitai
Copy link

coderabbitai bot commented Aug 25, 2025

Walkthrough

Auth hydration call added to root layout; splash-screen initialization moved to top-level and hide logic rewritten with status gating (includes 'onboarding') and a 200ms debounce; hydrateAuth exported; AuthState type removed from lib types and declared/exported in the auth store; onboarding's setIsOnboarding call disabled; tests for splash behavior and logout added.

Changes

Cohort / File(s) Summary of changes
App layout & splash (app-level)
src/app/(app)/_layout.tsx, src/app/_layout.tsx
Added hydrateAuth() call in root layout; moved SplashScreen.preventAutoHideAsync() call to top-level init; increased splash setOptions duration to 2000ms; rewrote splash hide effect to trigger only when status ∈ ['signedIn','signedOut','onboarding'] with a 200ms debounce and cleanup; added logging and adjusted onboarding/login redirect logs.
Auth public helper
src/lib/auth/index.tsx
Added export const hydrateAuth = () => useAuthStore.getState().hydrate(); to expose store hydration.
Auth types (lib)
src/lib/auth/types.tsx
Removed exported AuthState interface; preserved AuthStatus and ProfileModel.
Auth store API & hydration
src/stores/auth/store.tsx
Introduced and exported AuthState interface in the store (tokens, refreshTokenExpiresOn, status, profile, userId, flags, and methods including hydrate, isAuthenticated, setIsOnboarding); expanded hydrate to decode/apply persisted auth, set signedIn state or reset to signedOut/isFirstTime on missing/error; logout now removes persisted authResponse and logs on failure; added hydration and logout logging.
Onboarding component & tests
src/app/onboarding.tsx, src/app/__tests__/onboarding.test.tsx
Disabled on-mount setIsOnboarding() side-effect in onboarding component and updated test to assert it is not called.
Tests: splash & auth logout
src/app/(app)/__tests__/_layout.test.tsx, src/stores/auth/__tests__/store-logout.test.ts
Added splash hide unit tests (debounce, gating, cleanup) using fake timers and mocks; added auth-store logout tests covering normal and storage-failure paths with mocked storage/logging/APIs.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Root as App Root (_layout)
  participant Theme as Theme / Nav container
  participant AuthHelper as hydrateAuth()
  participant Store as Auth Store
  participant Splash as SplashScreen

  Root->>Theme: wait for theme/nav container ready
  Theme-->>Root: ready
  Root->>AuthHelper: hydrateAuth()
  AuthHelper->>Store: getState().hydrate()

  rect rgba(220,235,255,0.45)
    Store->>Store: read persisted authResponse
    alt authResponse present
      Store->>Store: set tokens, profile, status = 'signedIn'
    else no authResponse or error
      Store->>Store: clear tokens, set status = 'signedOut', isFirstTime = true
    end
  end

  Root->>Splash: splash hide effect triggered (status guarded)
  Note right of Root: hide only when status in ['signedIn','signedOut','onboarding']\n200ms debounce with cleanup
  Splash-->>Root: hideAsync()
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

I hopped through layout, logs in tow,
Held the splash till status said so,
Two thousand ms and a gentle 200ms nudge,
Hydrated tokens, cleared the judge,
A rabbit's patch — tidy, swift, and bright. 🐇✨

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 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.

📥 Commits

Reviewing files that changed from the base of the PR and between c18f810 and fd79010.

📒 Files selected for processing (1)
  • src/stores/auth/store.tsx (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/stores/auth/store.tsx
⏰ 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). (1)
  • GitHub Check: test
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch develop

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/stores/auth/store.tsx (1)

123-125: Fix token refresh scheduling calculation

The current code subtracts Date.now() from the TTL, which makes the computed delay negative and triggers an immediate refresh loop. Update the calculation to use the relative TTL directly and subtract the buffer, clamping to a sensible minimum.

• File: src/stores/auth/store.tsx
– Lines 123–124: replace the existing expiresIn calculation and setTimeout call.
– Consider storing the timeout ID and clearing it in logout() to avoid dangling timers.

- const expiresIn = response.expires_in * 1000 - Date.now() - 60000; // Refresh 1 minute before expiry
- setTimeout(() => get().refreshAccessToken(), expiresIn);
+ // Schedule the next refresh 1 minute before expiry, with a minimum delay of 5s
+ const refreshInMs = Math.max(response.expires_in * 1000 - 60_000, 5_000);
+ setTimeout(() => get().refreshAccessToken(), refreshInMs);

Tagging this as a critical fix to prevent immediate refresh loops.

🧹 Nitpick comments (5)
src/lib/auth/index.tsx (1)

22-23: New public hydrator looks good; add explicit return type for clarity

Thin wrapper is fine and keeps consumers decoupled from the store. Consider making the return type explicit to avoid accidental API drift.

-export const hydrateAuth = () => useAuthStore.getState().hydrate();
+export const hydrateAuth = (): void => useAuthStore.getState().hydrate()
src/app/_layout.tsx (3)

26-26: Remove unused import (useAuth)

useAuth is not used anymore after switching to hydrateAuth(). Clean up the import to satisfy linters and reduce bundle size.

-import { hydrateAuth, useAuth } from '@/lib/auth';
+import { hydrateAuth } from '@/lib/auth';

75-76: Defensive wrap around startup hydration

Calling hydrateAuth() at module load is fine, but a small try/catch keeps startup resilient and ensures we log early failures (e.g., if storage throws). This keeps the fix scoped without moving it into React lifecycle.

-//useAuth().hydrate();
-hydrateAuth();
+try {
+  hydrateAuth();
+} catch (error) {
+  logger.error({ message: 'hydrateAuth failed at startup', context: { error } });
+}

If you prefer, I can move this call into a one-time useEffect inside RootLayout to guarantee it runs after providers initialize; let me know.


81-81: Ensure the 2000 ms splash duration isn’t bypassed by the downstream hideAsync call

The root layout sets duration: 2000 ms in SplashScreen options, but the nested (app)/_layout.tsx immediately calls SplashScreen.hideAsync() on status changes, which can cut the splash display short. To guarantee the full 2 s display, condition the hide call on your app’s “ready” state (e.g. assets loaded or initial UI rendered) rather than unconditionally on status updates.

Key locations to address:

  • src/app/_layout.tsx (line 81):
    • Sets duration: 2000,
  • src/app/(app)/_layout.tsx (line 67):
    • Unconditionally calls await SplashScreen.hideAsync();

Consider wrapping the hideAsync() invocation in a readiness check (for example, if (isAppReady) { await SplashScreen.hideAsync(); }) so the splash isn’t hidden before your minimum display time has elapsed.

src/stores/auth/store.tsx (1)

13-28: AuthState surface expanded — watch for duplicate sources of truth

isFirstTime, isAuthenticated, and setIsOnboarding are now part of the store. However, the UI still uses useIsFirstTime() from storage (see (app)/_layout.tsx). Two sources can drift.

  • Option A: remove isFirstTime from the store and keep the dedicated storage hook.
  • Option B: migrate UI to consume isFirstTime from this store and deprecate the separate storage key.

Pick one to avoid divergence.

I can audit call sites and generate a migration patch if you choose Option B.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 984247f and 8fe3d06.

📒 Files selected for processing (5)
  • src/app/(app)/_layout.tsx (1 hunks)
  • src/app/_layout.tsx (2 hunks)
  • src/lib/auth/index.tsx (1 hunks)
  • src/lib/auth/types.tsx (0 hunks)
  • src/stores/auth/store.tsx (2 hunks)
💤 Files with no reviewable changes (1)
  • src/lib/auth/types.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
Use react-native-mmkv for local storage
Use axios for API requests

**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests

Files:

  • src/lib/auth/index.tsx
  • src/app/(app)/_layout.tsx
  • src/app/_layout.tsx
  • src/stores/auth/store.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursorrules)

**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&

**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...

Files:

  • src/lib/auth/index.tsx
  • src/app/(app)/_layout.tsx
  • src/app/_layout.tsx
  • src/stores/auth/store.tsx
src/**

📄 CodeRabbit inference engine (.cursorrules)

src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)

Files:

  • src/lib/auth/index.tsx
  • src/app/(app)/_layout.tsx
  • src/app/_layout.tsx
  • src/stores/auth/store.tsx
🧬 Code graph analysis (2)
src/app/_layout.tsx (1)
src/lib/auth/index.tsx (1)
  • hydrateAuth (23-23)
src/stores/auth/store.tsx (1)
src/lib/auth/types.tsx (3)
  • AuthStatus (39-39)
  • ProfileModel (25-37)
  • LoginCredentials (6-9)
🔇 Additional comments (1)
src/stores/auth/store.tsx (1)

8-10: Type-only imports: good use

Using import type for AuthResponse, AuthStatus, and LoginCredentials avoids bundling types and keeps the module lean.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)
src/app/_layout.tsx (1)

12-14: Fix SplashScreen import: setOptions isn’t on expo-splash-screen; use expo-router’s SplashScreen

You’re calling SplashScreen.setOptions, which is provided by expo-router’s SplashScreen wrapper, not expo-splash-screen. With the current import, setOptions will be undefined at runtime and should also fail type-checking. Switch to importing SplashScreen from expo-router and drop the expo-splash-screen import. While here, explicitly void the promise from preventAutoHideAsync to silence no-floating-promises/hot-reload noise.

Apply this diff:

- import * as SplashScreen from 'expo-splash-screen';
- import { Stack, useNavigationContainerRef } from 'expo-router';
+ import { SplashScreen, Stack, useNavigationContainerRef } from 'expo-router';

 // Prevent the splash screen from auto-hiding before asset loading is complete.
-SplashScreen.preventAutoHideAsync();
+void SplashScreen.preventAutoHideAsync();

 // Set the animation options. This is optional.
 SplashScreen.setOptions({
   duration: 2000,
   fade: true,
 });

Also applies to: 35-37, 79-83

src/stores/auth/store.tsx (4)

92-111: Logout leaves userId and refreshTokenExpiresOn set; also clear any pending refresh timers.

Not clearing these fields can leak PII and allow post-logout token refresh callbacks to run. Explicitly null them and cancel pending timeouts.

-import { removeItem, setItem, zustandStorage } from '../../lib/storage';
+import { removeItem, setItem, zustandStorage } from '../../lib/storage';

+// Track scheduled token refresh so we can cancel on logout
+let tokenRefreshTimer: ReturnType<typeof setTimeout> | null = null;

       logout: async () => {
         // Clear persisted authResponse to prevent re-hydration of signed-in session
         try {
           await removeItem('authResponse');
         } catch (error) {
           logger.warn({
             message: 'Failed to remove authResponse from storage during logout',
             context: { error },
           });
         }
 
+        // Cancel any scheduled token refresh
+        if (tokenRefreshTimer) {
+          clearTimeout(tokenRefreshTimer);
+          tokenRefreshTimer = null;
+        }
+
         set({
           accessToken: null,
           refreshToken: null,
+          refreshTokenExpiresOn: null,
           status: 'signedOut',
           error: null,
           profile: null,
           isFirstTime: true,
+          userId: null,
         });
       },

113-135: Incorrect token refresh scheduling (negative delay).

expires_in is already a duration in seconds. Subtracting Date.now() produces a large negative value; the timeout will fire immediately. Compute a simple offset and clamp to a sane minimum; also keep only one active timer.

-          const expiresIn = response.expires_in * 1000 - Date.now() - 60000; // Refresh 1 minute before expiry
-          setTimeout(() => get().refreshAccessToken(), expiresIn);
+          const refreshInMs = Math.max(response.expires_in * 1000 - 60_000, 30_000);
+          if (tokenRefreshTimer) {
+            clearTimeout(tokenRefreshTimer);
+          }
+          tokenRefreshTimer = setTimeout(() => {
+            // Best-effort chain; ignore returned promise
+            void get().refreshAccessToken();
+          }, refreshInMs);

135-139: Await logout in the refresh failure path.

Ensures state is fully reset before any caller continues.

-          // If refresh fails, log out the user
-          get().logout();
+          // If refresh fails, log out the user
+          await get().logout();

146-176: Signed-out hydration should also clear userId (and refreshTokenExpiresOn).

Prevent stale identifiers after a failed/missing hydrate.

           } else {
             logger.info({
               message: 'Hydrating auth: signedOut',
             });
             set({
               accessToken: null,
               refreshToken: null,
+              refreshTokenExpiresOn: null,
               status: 'signedOut',
               error: null,
               profile: null,
               isFirstTime: true,
+              userId: null,
             });
           }
src/app/(app)/_layout.tsx (1)

291-299: Avoid double NovuProvider instances (duplicated sockets).

The page-level NovuProvider wraps content, and CreateNotificationButton also wraps its button with another provider, which can create two sockets and duplicate listeners.

Flatten to a single provider by removing the inner one:

   return (
     <>
       {userId && config && rights?.DepartmentCode ? (
         <NovuProvider subscriberId={`${rights?.DepartmentCode}_User_${userId}`} applicationIdentifier={config.NovuApplicationId} backendUrl={config.NovuBackendApiUrl} socketUrl={config.NovuSocketUrl}>
           {/* NotificationInbox at the root level */}
           <NotificationInbox isOpen={isNotificationsOpen} onClose={() => setIsNotificationsOpen(false)} />
           {content}
         </NovuProvider>
       ) : (
         content
       )}
     </>
   );

@@
-  return (
-    <NovuProvider subscriberId={`${departmentCode}_User_${userId}`} applicationIdentifier={config.NovuApplicationId} backendUrl={config.NovuBackendApiUrl} socketUrl={config.NovuSocketUrl}>
-      <NotificationButton onPress={() => setIsNotificationsOpen(true)} />
-    </NovuProvider>
-  );
+  return <NotificationButton onPress={() => setIsNotificationsOpen(true)} />;

Also applies to: 341-345

🧹 Nitpick comments (15)
src/app/_layout.tsx (2)

106-107: Hydration call: make intent explicit and confirm error handling

hydrateAuth() is invoked and intentionally not awaited. If hydrate() in the store is async and can reject, this could surface as an unhandled promise rejection. If it’s sync, great—make that intent explicit.

Consider one of:

  • If sync: change call to explicitly ignore return and annotate hydrateAuth to return void.
  • If async: wrap in a fire-and-forget IIFE with catch + log.

Example if async:

-    hydrateAuth();
+    (async () => {
+      try {
+        await hydrateAuth();
+      } catch (error) {
+        logger.error({ message: 'Auth hydration failed', context: { error } });
+      }
+    })();

77-78: Remove commented-out hydrate call

The legacy //useAuth().hydrate(); is stale and can confuse readers. Safe to delete.

-//useAuth().hydrate();
docs/splash-screen-enhancement.md (1)

47-54: Minor copy edits for list readability

A few bullets read as sentence fragments without consistent punctuation. Optional tidy-up below.

- - **Better UX**: Prevents premature splash screen hiding that could expose uninitialized content
- - **Smoother Transitions**: 200ms debounce provides smoother visual transitions
- - **Device Optimization**: Particularly beneficial for lower-end devices that may take longer to render
- - **Memory Safety**: Proper timeout cleanup prevents memory leaks
- - **Consistent Behavior**: Ensures splash screen behavior is predictable across different authentication flows
+ - **Better UX**: Prevents premature splash screen hiding that could expose uninitialized content.
+ - **Smoother transitions**: A 200ms debounce provides smoother visual transitions.
+ - **Device optimization**: Particularly beneficial for lower-end devices that may take longer to render.
+ - **Memory safety**: Proper timeout cleanup prevents memory leaks.
+ - **Consistent behavior**: Ensures splash screen behavior is predictable across different authentication flows.
src/app/onboarding.tsx (1)

65-65: Remove dead/commented code and unused store import

Since setIsOnboarding is no longer used, drop the import and the commented effect to keep the component lean.

-import { useAuthStore } from '@/lib/auth';
@@
-  //const { setIsOnboarding } = useAuthStore();
@@
-  //useEffect(() => {
-  //  setIsOnboarding();
-  //}, [setIsOnboarding]);

Also applies to: 73-76

src/app/(app)/__tests__/_layout.test.tsx (3)

12-18: Remove unused auth-store mock and type alias

The test component takes status as a prop and doesn’t use useAuthStore. These mocks/aliases add noise.

-jest.mock('@/lib/auth', () => ({
-  useAuthStore: jest.fn(),
-}));
-
-const mockUseAuthStore = useAuthStore as jest.MockedFunction<typeof useAuthStore>;

79-91: Add a negative-time assertion to catch premature hides (pre-debounce)

Strengthen the debounce guarantee by asserting no hide before 200ms.

   it('should hide splash with debounce when status is signedIn', () => {
     render(<TestSplashComponent status="signedIn" />);
 
     // Splash should not be hidden immediately
     expect(mockHideAsync).not.toHaveBeenCalled();
 
+    // Should still not hide before the debounce elapses
+    jest.advanceTimersByTime(199);
+    expect(mockHideAsync).not.toHaveBeenCalled();
+
     // Fast-forward timers by 200ms (the debounce duration)
     jest.advanceTimersByTime(200);
 
     // Now splash should be hidden
     expect(mockHideAsync).toHaveBeenCalledTimes(1);
   });

19-27: Test the error path: hideAsync rejection should be swallowed

hideSplash wraps hideAsync in try/catch; add a test to ensure a rejection doesn’t crash the component.

 describe('App Layout Splash Screen Logic', () => {
@@
   afterEach(() => {
     jest.runOnlyPendingTimers();
     jest.useRealTimers();
   });
 
+  it('should swallow errors when SplashScreen.hideAsync rejects', () => {
+    mockHideAsync.mockRejectedValueOnce(new Error('boom'));
+    expect(() => {
+      render(<TestSplashComponent status="signedIn" />);
+      jest.advanceTimersByTime(200);
+    }).not.toThrow();
+  });

Also applies to: 105-117

src/stores/auth/__tests__/store-logout.test.ts (2)

55-66: Resetting store state is appropriate; consider also clearing persisted slice if used elsewhere.

useAuthStore.setState(...) restores a clean baseline per test. If other suites depend on the persisted slice, add a one-time useAuthStore.persist.clearStorage?.() in a global test setup to avoid cross-suite leakage.


85-101: Cover the failure path more thoroughly.

Also assert that we still attempted to remove the key, and that the warning is logged exactly once.

   await useAuthStore.getState().logout();

   // Verify warning was logged
-  expect(mockedLogger.warn).toHaveBeenCalledWith({
+  expect(mockedLogger.warn).toHaveBeenCalledTimes(1);
+  expect(mockedLogger.warn).toHaveBeenCalledWith({
     message: 'Failed to remove authResponse from storage during logout',
     context: { error: mockError },
   });
+
+  // Removal was attempted
+  expect(mockedRemoveItem).toHaveBeenCalledWith('authResponse');
src/stores/auth/store.tsx (1)

41-53: Persist the auth blob reliably and name expiry consistently.

  • If setItem is async (like removeItem), prefer await setItem<AuthResponse>(...) so the persisted state isn’t lost on process kill after login.
  • The field refreshTokenExpiresOn looks like access token expiry derived from expires_in. Either store accessTokenExpiresOn or populate the actual refresh expiry if available (e.g., refresh_expires_in).

I can wire this once you confirm the API fields.

src/app/(app)/_layout.tsx (5)

273-275: Localize user-facing string.

Wrap “Close” with t() to meet i18n guidelines.

-                <Button onPress={() => setIsOpen(false)} className="w-full bg-primary-600">
-                  <ButtonText>Close</ButtonText>
+                <Button onPress={() => setIsOpen(false)} className="w-full bg-primary-600">
+                  <ButtonText>{t('common.close', 'Close')}</ButtonText>
                 </Button>

42-42: Use a selector to reduce unnecessary re-renders from Zustand.

Selecting the whole store (useAuthStore()) will re-render on any store change. Select only status.

-  const { status } = useAuthStore();
+  const status = useAuthStore((s) => s.status);

47-50: Avoid inline handlers; memoize callbacks to reduce renders.

Define stable callbacks once and reuse them.

   // Memoize drawer navigation handler for better performance
   const handleNavigate = useCallback(() => {
     setIsOpen(false);
   }, []);
+  const openDrawer = useCallback(() => setIsOpen(true), []);
+  const closeDrawer = useCallback(() => setIsOpen(false), []);
+  const openNotifications = useCallback(() => setIsNotificationsOpen(true), []);
+  const closeNotifications = useCallback(() => setIsNotificationsOpen(false), []);

@@
-          <Drawer isOpen={isOpen} onClose={() => setIsOpen(false)}>
+          <Drawer isOpen={isOpen} onClose={closeDrawer}>
@@
-                <Button onPress={() => setIsOpen(false)} className="w-full bg-primary-600">
+                <Button onPress={closeDrawer} className="w-full bg-primary-600">
@@
-          <NotificationInbox isOpen={isNotificationsOpen} onClose={() => setIsNotificationsOpen(false)} />
+          <NotificationInbox isOpen={isNotificationsOpen} onClose={closeNotifications} />
@@
-      onPress={() => {
-        setIsOpen(true);
-      }}
+      onPress={openDrawer}

Also applies to: 267-278, 294-295, 314-321


5-6: Prune unused imports to keep bundle lean.

size from lodash and most lucide icons aren’t referenced.

-import { size } from 'lodash';
-import { Contact, Home, ListTree, Mail, Map, Megaphone, Menu, Notebook, Truck, Users } from 'lucide-react-native';
+import { Menu } from 'lucide-react-native';

81-141: Initialization flow: OK, but consider scheduling initial token refresh post-login.

App init gating on signedIn is good. Given the store’s token refresh only reschedules after a refresh event, ensure the first refresh is scheduled after login (in the store) to avoid silent expiry. Happy to draft that once store-side changes land.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 8fe3d06 and c18f810.

📒 Files selected for processing (8)
  • docs/splash-screen-enhancement.md (1 hunks)
  • src/app/(app)/__tests__/_layout.test.tsx (1 hunks)
  • src/app/(app)/_layout.tsx (3 hunks)
  • src/app/__tests__/onboarding.test.tsx (1 hunks)
  • src/app/_layout.tsx (4 hunks)
  • src/app/onboarding.tsx (1 hunks)
  • src/stores/auth/__tests__/store-logout.test.ts (1 hunks)
  • src/stores/auth/store.tsx (4 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
Use react-native-mmkv for local storage
Use axios for API requests

**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests

Files:

  • src/app/__tests__/onboarding.test.tsx
  • src/stores/auth/__tests__/store-logout.test.ts
  • src/app/onboarding.tsx
  • src/app/(app)/__tests__/_layout.test.tsx
  • src/stores/auth/store.tsx
  • src/app/(app)/_layout.tsx
  • src/app/_layout.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursorrules)

**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&

**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...

Files:

  • src/app/__tests__/onboarding.test.tsx
  • src/app/onboarding.tsx
  • src/app/(app)/__tests__/_layout.test.tsx
  • src/stores/auth/store.tsx
  • src/app/(app)/_layout.tsx
  • src/app/_layout.tsx
src/**

📄 CodeRabbit inference engine (.cursorrules)

src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)

Files:

  • src/app/__tests__/onboarding.test.tsx
  • src/stores/auth/__tests__/store-logout.test.ts
  • src/app/onboarding.tsx
  • src/app/(app)/__tests__/_layout.test.tsx
  • src/stores/auth/store.tsx
  • src/app/(app)/_layout.tsx
  • src/app/_layout.tsx
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests

Files:

  • src/app/__tests__/onboarding.test.tsx
  • src/stores/auth/__tests__/store-logout.test.ts
  • src/app/(app)/__tests__/_layout.test.tsx
src/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors

Files:

  • src/app/__tests__/onboarding.test.tsx
  • src/stores/auth/__tests__/store-logout.test.ts
  • src/app/(app)/__tests__/_layout.test.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to src/**/*.test.{ts,tsx} : Create and use Jest tests to validate all generated components

Applied to files:

  • src/app/(app)/__tests__/_layout.test.tsx
🧬 Code graph analysis (4)
src/app/__tests__/onboarding.test.tsx (1)
src/app/onboarding.tsx (1)
  • Onboarding (63-209)
src/stores/auth/__tests__/store-logout.test.ts (1)
src/lib/storage/index.tsx (1)
  • removeItem (27-29)
src/stores/auth/store.tsx (3)
src/lib/auth/types.tsx (3)
  • AuthStatus (39-39)
  • ProfileModel (25-37)
  • LoginCredentials (6-9)
src/lib/storage/index.tsx (1)
  • removeItem (27-29)
src/lib/storage/secure-storage.ts (1)
  • set (118-130)
src/app/_layout.tsx (1)
src/lib/auth/index.tsx (1)
  • hydrateAuth (23-23)
🪛 LanguageTool
docs/splash-screen-enhancement.md

[grammar] ~10-~10: There might be a mistake here.
Context: ...during app initialization. ### Solution Updated the splash screen hiding logic w...

(QB_NEW_EN)


[grammar] ~49-~49: There might be a mistake here.
Context: ... that could expose uninitialized content - Smoother Transitions: 200ms debounce p...

(QB_NEW_EN)


[grammar] ~50-~50: There might be a mistake here.
Context: ...nce provides smoother visual transitions - Device Optimization: Particularly bene...

(QB_NEW_EN)


[grammar] ~51-~51: There might be a mistake here.
Context: ...d devices that may take longer to render - Memory Safety: Proper timeout cleanup ...

(QB_NEW_EN)


[grammar] ~52-~52: There might be a mistake here.
Context: ...er timeout cleanup prevents memory leaks - Consistent Behavior: Ensures splash sc...

(QB_NEW_EN)

⏰ 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). (1)
  • GitHub Check: test
🔇 Additional comments (4)
docs/splash-screen-enhancement.md (1)

13-18: Verify status gating matches the implementation across layouts

Doc says to hide only when status is 'signedIn' or 'signedOut'. Ensure this matches all layouts handling splash (e.g., (app)/_layout.tsx). If any layout also hides on 'onboarding', document it here for consistency.

I can update the doc once you confirm the intended set of “settled” statuses.

src/app/__tests__/onboarding.test.tsx (1)

297-303: Good alignment with product change: assert setIsOnboarding is not called

This test guards against regressions after removing the on-mount onboarding toggle. Nicely scoped and clear.

src/stores/auth/__tests__/store-logout.test.ts (1)

4-14: Good isolation: storage mocked before importing the store.

Mocking @/lib/storage (including zustandStorage) before importing the store prevents persist rehydration from touching real storage. This keeps tests deterministic. Nicely done.

src/app/(app)/_layout.tsx (1)

167-190: Splash hide gating + debounce looks good.

Hiding only when status is settled and adding a 200ms debounce will reduce white flashes. The cleanup guards against stacked timers. Solid improvement.

Comment on lines +69 to +83
it('should clear authResponse from storage and reset auth state', async () => {
await useAuthStore.getState().logout();

// Verify authResponse was removed from storage
expect(mockedRemoveItem).toHaveBeenCalledWith('authResponse');

// Verify auth state was reset
const state = useAuthStore.getState();
expect(state.accessToken).toBeNull();
expect(state.refreshToken).toBeNull();
expect(state.status).toBe('signedOut');
expect(state.error).toBeNull();
expect(state.profile).toBeNull();
expect(state.isFirstTime).toBe(true);
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Expand assertions to catch PII leakage on logout.

The store currently does not null out userId or refreshTokenExpiresOn on logout. Add assertions here to lock that down once fixed.

       const state = useAuthStore.getState();
       expect(state.accessToken).toBeNull();
       expect(state.refreshToken).toBeNull();
       expect(state.status).toBe('signedOut');
       expect(state.error).toBeNull();
       expect(state.profile).toBeNull();
       expect(state.isFirstTime).toBe(true);
+      expect(state.userId).toBeNull();
+      expect(state.refreshTokenExpiresOn).toBeNull();
📝 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.

Suggested change
it('should clear authResponse from storage and reset auth state', async () => {
await useAuthStore.getState().logout();
// Verify authResponse was removed from storage
expect(mockedRemoveItem).toHaveBeenCalledWith('authResponse');
// Verify auth state was reset
const state = useAuthStore.getState();
expect(state.accessToken).toBeNull();
expect(state.refreshToken).toBeNull();
expect(state.status).toBe('signedOut');
expect(state.error).toBeNull();
expect(state.profile).toBeNull();
expect(state.isFirstTime).toBe(true);
});
it('should clear authResponse from storage and reset auth state', async () => {
await useAuthStore.getState().logout();
// Verify authResponse was removed from storage
expect(mockedRemoveItem).toHaveBeenCalledWith('authResponse');
// Verify auth state was reset
const state = useAuthStore.getState();
expect(state.accessToken).toBeNull();
expect(state.refreshToken).toBeNull();
expect(state.status).toBe('signedOut');
expect(state.error).toBeNull();
expect(state.profile).toBeNull();
expect(state.isFirstTime).toBe(true);
expect(state.userId).toBeNull();
expect(state.refreshTokenExpiresOn).toBeNull();
});
🤖 Prompt for AI Agents
In src/stores/auth/__tests__/store-logout.test.ts around lines 69 to 83, the
test verifies many auth fields are cleared on logout but neglects to assert that
sensitive PII fields userId and refreshTokenExpiresOn are nulled; update the
test to assert expect(state.userId).toBeNull() and
expect(state.refreshTokenExpiresOn).toBeNull() after fetching the state, and if
those values are also persisted to storage, add corresponding
expect(mockedRemoveItem).toHaveBeenCalledWith('<key>') checks for their storage
keys so the test fully covers PII removal on logout.

@ucswift
Copy link
Member Author

ucswift commented Aug 26, 2025

Approve

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is approved.

@ucswift ucswift merged commit aaa2458 into master Aug 26, 2025
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant