Repository navigation
fix(web): resolve admin redirect and 401 errors, improve test coverag… - #42
Conversation
📝 WalkthroughWalkthroughThe changes refactor the login authentication flow to use an email-based sign-in method via authClient.signIn.email, followed by fetching an enriched session from /auth/refresh-session. The API client signature is updated to support optional tokens and cookie credentials. Test infrastructure and environment variable handling are adjusted accordingly. Changes
Sequence DiagramsequenceDiagram
participant User
participant LoginPage
participant authClient
participant API as /auth/refresh-session
participant AuthContext
User->>LoginPage: Enter email & password
LoginPage->>authClient: authClient.signIn.email(email, password)
authClient-->>LoginPage: {success, user?, session?}
alt Email sign-in succeeds
LoginPage->>API: fetch /auth/refresh-session<br/>(credentials: include)
API-->>LoginPage: {user, session.token}
LoginPage->>LoginPage: Validate enriched data
LoginPage->>AuthContext: setAuthState({accessToken, user})
LoginPage->>User: Navigate to /dashboard or admin
else Email sign-in fails
LoginPage->>User: Display error message
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/modules/identity/pages/LoginPage.spec.tsx (1)
31-33:⚠️ Potential issue | 🟡 MinorMove global
fetchmock tobeforeEachand clean up withvi.unstubAllGlobals().
The module-scopedglobalThis.fetch = vi.fn()at line 32 lacks proper cleanup. Currently,vi.clearAllMocks()only clears call history but does not restore the global—it should be moved tobeforeEachusingvi.stubGlobal, withvi.unstubAllGlobals()inafterEachfor proper isolation.🔧 Suggested changes
-// Setup global fetch mock -globalThis.fetch = vi.fn();beforeEach(() => { vi.clearAllMocks(); + vi.stubGlobal('fetch', vi.fn()); // Stub VITE_API_URL vi.stubEnv('VITE_API_URL', 'http://test-api.com');afterEach(() => { vi.unstubAllEnvs(); + vi.unstubAllGlobals(); Object.defineProperty(globalThis, 'location', { value: originalLocation, writable: true, configurable: true }); });
🤖 Fix all issues with AI agents
In `@apps/web/src/modules/identity/pages/LoginPage.tsx`:
- Around line 87-89: In LoginPage remove the two client-side console.log calls
that print userPermissions and permission checks (the lines referencing
userPermissions and Resources.ADMIN_DASHBOARD / Actions.VIEW); instead either
delete them or replace with a non-client log path (e.g., send to a server-side
audit/log endpoint or use a dev-only logger gated by an explicit environment
flag). Update any tests or references expecting those console outputs. Ensure no
sensitive permission data is emitted to the browser console.
- Around line 74-90: Check enrichedData before setting auth state: verify
enrichedData.user exists and enrichedData.session?.token is present in the
LoginPage flow, and if either is missing fail fast (throw or return an error and
avoid calling setAuthState). Use guards around rawUser before accessing
rawUser.permissions (or default to an empty array) to prevent accessing
properties on undefined, and ensure the code path that calls hasPermission only
runs after these validations; include a clear error/log message and abort the
login continuation when validation fails.
In `@apps/web/src/shared/lib/api.ts`:
- Around line 12-15: The current spread of options.headers into the headers
Record is unsafe because RequestInit.headers can be Headers, string[][], or
Record<string,string>; replace that logic by normalizing headers into a Headers
instance (e.g., new Headers(options.headers)) and then ensure 'Content-Type':
'application/json' is set if not present; update the fetch call to pass this
Headers instance (or its plain object if needed) instead of the spread object.
Locate and change code around the headers variable and the fetch call that uses
options.headers so it constructs and uses the safe Headers object and preserves
all header forms.
In `@apps/web/vite.config.ts`:
- Around line 35-44: Remove the dead Vitest test/coverage configuration block
(the test: { ... } object) from vite.config.ts so Vitest loads only
vitest.config.ts; then fix vitest.config.ts by defining __dirname before it's
used (replace the raw __dirname usage with a computed value using
fileURLToPath(import.meta.url) and path.dirname) — use the same pattern as in
vite.config.ts (import or reference fileURLToPath and path, compute const
__filename = fileURLToPath(import.meta.url); const __dirname =
path.dirname(__filename)) to ensure __dirname is defined for the code that
references it.
| test: { | ||
| globals: true, | ||
| environment: 'jsdom', | ||
| setupFiles: './src/setupTests.ts', | ||
| coverage: { | ||
| provider: 'v8', | ||
| reporter: ['text', 'json', 'html'], | ||
| include: ['src/**/*.{ts,tsx}'], | ||
| exclude: ['src/**/*.spec.{ts,tsx}', 'src/setupTests.ts', 'src/**/index.ts', 'src/shared/components/ui/*.tsx'], | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find package.json and check test:cov script
echo "=== Finding package.json and test:cov script ==="
find . -name package.json -path "*/apps/web/*" | head -1 | while read pkg; do
echo "Found: $pkg"
grep -A2 -B2 'test:cov' "$pkg" || echo "No test:cov found"
done
echo -e "\n=== Checking for config files in apps/web ==="
fd -a '\(vite|vitest\)\.config\.' apps/web/
echo -e "\n=== Reading vite.config.ts ==="
if [ -f apps/web/vite.config.ts ]; then
cat -n apps/web/vite.config.ts
fi
echo -e "\n=== Reading vitest.config.ts ==="
if [ -f apps/web/vitest.config.ts ]; then
cat -n apps/web/vitest.config.ts
fiRepository: pramodnarayana/nexiom
Length of output: 2869
🏁 Script executed:
#!/bin/bash
# Check if __dirname is defined in vitest.config.ts
echo "=== Checking vitest.config.ts for __dirname ==="
head -15 apps/web/vitest.config.ts | grep -E 'import|__dirname|__filename'Repository: pramodnarayana/nexiom
Length of output: 276
Remove unused test config from vite.config.ts; fix missing __dirname in vitest.config.ts.
The test:cov command runs vitest run --coverage, which loads vitest.config.ts exclusively. The test/coverage block in vite.config.ts (lines 35-44) is dead code and should be removed. Additionally, vitest.config.ts uses __dirname on line 10 without defining it—in ES modules, you must define it using fileURLToPath(import.meta.url) and path.dirname(), as shown in vite.config.ts.
🤖 Prompt for AI Agents
In `@apps/web/vite.config.ts` around lines 35 - 44, Remove the dead Vitest
test/coverage configuration block (the test: { ... } object) from vite.config.ts
so Vitest loads only vitest.config.ts; then fix vitest.config.ts by defining
__dirname before it's used (replace the raw __dirname usage with a computed
value using fileURLToPath(import.meta.url) and path.dirname) — use the same
pattern as in vite.config.ts (import or reference fileURLToPath and path,
compute const __filename = fileURLToPath(import.meta.url); const __dirname =
path.dirname(__filename)) to ensure __dirname is defined for the code that
references it.
…uards, fix headers API, configure test env
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/modules/identity/pages/LoginPage.tsx (1)
132-144: 🧹 Nitpick | 🔵 TrivialConsider user feedback for social login errors.
The Google sign-in error handling logs to console and sets a generic error, but
console.erroron Line 141 exposes error details in the browser console. For consistency with the PR objective of removing console.logs, consider removing or replacing with structured error handling.♻️ Suggested improvement
} catch (error) { - console.error('Social login error', error); setError('Failed to initiate Google login'); }
1 participant
…e configuration
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.