feat(admin-web): ship operator dashboard for phase 5#15
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe admin landing page is replaced with an async operator dashboard that resolves operator session (env token or bootstrap sign-in), concurrently loads verification and dispute queue states, and adds server actions to review verifications and advance dispute transitions, plus presenter helpers and tests. Changes
Sequence DiagramsequenceDiagram
participant Browser as Browser/Client
participant Page as AdminHomePage
participant SessionMgr as resolveOperatorSession
participant QueueMgr as Queue Loaders
participant API as Platform API
Browser->>Page: Request dashboard
Page->>SessionMgr: resolveOperatorSession()
SessionMgr->>API: GET /api/v1/auth/session or POST /api/v1/auth/sign-in
API-->>SessionMgr: session payload / token
SessionMgr-->>Page: resolved session (ok:true) or error
Page->>QueueMgr: load verification & dispute queues (concurrent)
QueueMgr->>API: Fetch queue endpoints
API-->>QueueMgr: queue items / errors
QueueMgr-->>Page: queue states
Page->>Browser: Render sections and forms
Browser->>Page: Submit review / dispute form (server action)
Page->>API: POST decision / transition (server action)
API-->>Page: confirmation
Page->>Browser: Revalidate / refresh dashboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/admin-web/src/app/page.js (2)
19-26: Validatedecisionbefore loading queue state to avoid unnecessary API call.The decision validation at Line 24 happens after
loadQueueStateis called. Moving validation earlier would skip the queue fetch when the decision is already invalid.♻️ Reorder validation
const verificationId = String(formData.get('verificationId') ?? ''); const decision = String(formData.get('decision') ?? 'approved'); const reviewNote = String(formData.get('reviewNote') ?? '').trim(); + if (decision !== 'approved' && decision !== 'rejected') { + return; + } + const currentState = await loadQueueState(sessionResult.sessionToken); - if (currentState.status !== 'loaded' || (decision !== 'approved' && decision !== 'rejected')) { + if (currentState.status !== 'loaded') { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-web/src/app/page.js` around lines 19 - 26, Validate the decision variable before calling loadQueueState to avoid an unnecessary API call: check that decision is either 'approved' or 'rejected' (using the existing decision variable) right after it's derived, and return early if invalid; only after that validation call loadQueueState(sessionResult.sessionToken) and then use currentState.status as before. Reference decision and loadQueueState to locate where to reorder the logic.
135-135: Hardcoded'de-AT'locale may not suit all operators.The date formatting uses a fixed Austrian German locale. Consider deriving the locale from operator preferences, browser settings, or a configurable default.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-web/src/app/page.js` at line 135, The date formatting in the page component currently forces the 'de-AT' locale for the Submitted timestamp (the expression using verification.submittedAt and toLocaleString('de-AT')); change it to use a configurable locale (e.g., derived from an operator preference, a prop/context value, or navigator.language as a fallback) and format via Intl.DateTimeFormat or toLocaleString with that locale variable so the displayed date respects operator/browser settings; update the rendering path where verification.submittedAt is used to consume the new locale source (prop/context/config) instead of the hardcoded 'de-AT'.apps/admin-web/src/features/dashboard/dashboard-presenter.test.ts (1)
61-67: Consider adding coverage for error states within loaded queues.The
loadedtests only cover the happy path wherereviewAction.statusisidle. The presenter has a branch forstate.reviewAction.status === 'error'(Lines 35-36 in dashboard-presenter.ts) that surfaceserrorMessagein the detail field. Similarly for dispute'squeueAction.status === 'error'.📋 Example additional test case
it('describes verification loaded state with review error', () => { const state = { status: 'loaded' as const, verifications: [verification], reviewAction: { status: 'error' as const, verificationId: 'v-1', errorMessage: 'Review failed' }, }; expect(describeVerificationQueue(state)).toEqual({ badge: '1 pending', headline: 'Provider verification queue is live.', detail: 'Review failed', }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-web/src/features/dashboard/dashboard-presenter.test.ts` around lines 61 - 67, Add tests covering the loaded-but-failing branches so the presenter surfaces error messages: create a loaded state for describeVerificationQueue where reviewAction.status === 'error' with a verificationId and errorMessage and assert the returned detail equals that errorMessage; similarly create a loaded state for describeDisputeQueue where queueAction.status === 'error' with disputeId and errorMessage and assert the detail contains the errorMessage. Use the existing helper createLoadedQueueState or construct the state object directly and reference describeVerificationQueue, describeDisputeQueue, reviewAction, queueAction, and errorMessage when building assertions.apps/admin-web/src/features/operator-session/operator-session.test.ts (1)
5-20: Environment restoration looks correct but may be fragile in parallel test execution.Capturing
process.envvalues at module load time works for sequential test runs. If Vitest runs test files in parallel (default behavior), mutations toprocess.envcould leak across files since they share the same process.Consider using
vi.stubEnv()for safer environment variable mocking if parallel isolation becomes an issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-web/src/features/operator-session/operator-session.test.ts` around lines 5 - 20, The test captures environment variables at module load (previousSessionToken, previousBootstrapEmail) and restores them in afterEach, which is fragile under Vitest's parallel test execution; update operator-session.test.ts to use Vitest's vi.stubEnv (or capture/restore inside beforeEach/afterEach per test) instead of module-level snapshots so env changes are isolated—replace the module-level previousSessionToken/previousBootstrapEmail usage with vi.stubEnv(...) when setting test env or move the snapshot logic into a beforeEach that saves process.env values and an afterEach that restores them, referencing the existing afterEach block and the previousSessionToken/previousBootstrapEmail identifiers.apps/admin-web/src/features/operator-session/operator-session.ts (1)
92-98: Fallback values for missing email/userId may mask API contract violations.When
payload.session.emailorpayload.session.userIdis undefined, the code falls back to hardcoded values ('operator@quickwerk.local','operator-missing'). Given that the type guard already validates these fields exist as strings (Lines 44-46), this scenario shouldn't occur. If it does, it indicates a logic bug.Consider logging a warning or removing the nullish coalescing since the type guard guarantees these fields are present.
📋 Remove unnecessary fallbacks
return { ok: true, sessionToken, - operatorEmail: payload.session.email ?? 'operator@quickwerk.local', - operatorUserId: payload.session.userId ?? 'operator-missing', + operatorEmail: payload.session!.email!, + operatorUserId: payload.session!.userId!, source: 'env-token', };The non-null assertions are safe here because Lines 85-90 already verify
payload.session?.role !== 'operator'would return an error, meaning we only reach this point whensessionis defined with valid fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-web/src/features/operator-session/operator-session.ts` around lines 92 - 98, The return object uses fallback literals for payload.session.email and payload.session.userId which can mask bugs despite the earlier type guard; remove the nullish coalescing and return the values directly (use payload.session!.email and payload.session!.userId or the non-null asserted properties) in the function that constructs the session response, and optionally add a processLogger.warn or throw an invariant if payload.session is unexpectedly missing to fail fast. Ensure you update the return object keys (sessionToken, operatorEmail, operatorUserId, source) to use the asserted session fields (payload.session!.email, payload.session!.userId) instead of the hardcoded defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/admin-web/src/app/page.js`:
- Around line 11-37: The server action reviewVerificationAction currently
returns early on failures (when resolveOperatorSession() yields ok=false, when
loadQueueState() status isn't 'loaded', or when decision is invalid) which
silently discards errors; modify reviewVerificationAction to surface errors by
returning a structured result (eg. { ok: false, error: 'message' }) or by
throwing descriptive Errors instead of returning undefined—include contextual
messages for each failure point (session failure from resolveOperatorSession,
queue load failure from loadQueueState, invalid decision) and keep successful
path returning { ok: true } (or consistent success shape) so the
caller/component can use useFormState or similar to display feedback; ensure
calls to submitReviewDecision and revalidatePath remain in the success branch.
---
Nitpick comments:
In `@apps/admin-web/src/app/page.js`:
- Around line 19-26: Validate the decision variable before calling
loadQueueState to avoid an unnecessary API call: check that decision is either
'approved' or 'rejected' (using the existing decision variable) right after it's
derived, and return early if invalid; only after that validation call
loadQueueState(sessionResult.sessionToken) and then use currentState.status as
before. Reference decision and loadQueueState to locate where to reorder the
logic.
- Line 135: The date formatting in the page component currently forces the
'de-AT' locale for the Submitted timestamp (the expression using
verification.submittedAt and toLocaleString('de-AT')); change it to use a
configurable locale (e.g., derived from an operator preference, a prop/context
value, or navigator.language as a fallback) and format via Intl.DateTimeFormat
or toLocaleString with that locale variable so the displayed date respects
operator/browser settings; update the rendering path where
verification.submittedAt is used to consume the new locale source
(prop/context/config) instead of the hardcoded 'de-AT'.
In `@apps/admin-web/src/features/dashboard/dashboard-presenter.test.ts`:
- Around line 61-67: Add tests covering the loaded-but-failing branches so the
presenter surfaces error messages: create a loaded state for
describeVerificationQueue where reviewAction.status === 'error' with a
verificationId and errorMessage and assert the returned detail equals that
errorMessage; similarly create a loaded state for describeDisputeQueue where
queueAction.status === 'error' with disputeId and errorMessage and assert the
detail contains the errorMessage. Use the existing helper createLoadedQueueState
or construct the state object directly and reference describeVerificationQueue,
describeDisputeQueue, reviewAction, queueAction, and errorMessage when building
assertions.
In `@apps/admin-web/src/features/operator-session/operator-session.test.ts`:
- Around line 5-20: The test captures environment variables at module load
(previousSessionToken, previousBootstrapEmail) and restores them in afterEach,
which is fragile under Vitest's parallel test execution; update
operator-session.test.ts to use Vitest's vi.stubEnv (or capture/restore inside
beforeEach/afterEach per test) instead of module-level snapshots so env changes
are isolated—replace the module-level
previousSessionToken/previousBootstrapEmail usage with vi.stubEnv(...) when
setting test env or move the snapshot logic into a beforeEach that saves
process.env values and an afterEach that restores them, referencing the existing
afterEach block and the previousSessionToken/previousBootstrapEmail identifiers.
In `@apps/admin-web/src/features/operator-session/operator-session.ts`:
- Around line 92-98: The return object uses fallback literals for
payload.session.email and payload.session.userId which can mask bugs despite the
earlier type guard; remove the nullish coalescing and return the values directly
(use payload.session!.email and payload.session!.userId or the non-null asserted
properties) in the function that constructs the session response, and optionally
add a processLogger.warn or throw an invariant if payload.session is
unexpectedly missing to fail fast. Ensure you update the return object keys
(sessionToken, operatorEmail, operatorUserId, source) to use the asserted
session fields (payload.session!.email, payload.session!.userId) instead of the
hardcoded defaults.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 72970213-92dd-4e81-b455-177f2a05de04
📒 Files selected for processing (6)
apps/admin-web/src/app/page.jsapps/admin-web/src/features/dashboard/dashboard-presenter.test.tsapps/admin-web/src/features/dashboard/dashboard-presenter.tsapps/admin-web/src/features/operator-session/operator-session.test.tsapps/admin-web/src/features/operator-session/operator-session.tsapps/admin-web/src/features/provider-review/verification-queue-actions.test.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apps/admin-webValidation
Traceability
0b63525Summary by CodeRabbit
New Features
Tests