feat(dashboard): Task 1E — Mobile-first review dashboard with approve/reject, config panel, pipeline status#666
Conversation
Task 1E deliverables: - Review Queue page (mobile-first card layout, quality badges) - Review Detail page (inline editing, approve/reject with touch-friendly buttons) - Review server actions (approve, reject, updateVideoField with field whitelist) - Config Panel page with grouped form sections (6 fieldsets) - Config Form client component (save to API, success toast) - Config API route (GET/PUT with fail-closed auth, field whitelist validation) - Pipeline Status page (12 status count cards, active workflows, recent completions) - Review API route (POST approve/reject with fail-closed auth, CF Workflow webhook) - Updated sidebar navigation with Review Queue, Pipeline, Config links Co-authored-by: dashboard <dashboard@miriad.systems>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
codercatdev
left a comment
codercatdev
left a comment
There was a problem hiding this comment.
PR #666 Review — Task 1E: Review Dashboard
Verdict: 🔴 REQUEST_CHANGES
🚨 BLOCKER: Server Actions Missing Auth (1 issue, 3 functions affected)
app/(dashboard)/dashboard/review/actions.ts — The three server actions (approveVideo, rejectVideo, updateVideoField) have zero auth checks. Server actions are exposed as POST endpoints by Next.js and can be called by anyone without authentication.
This is the #1 recurring issue on this project (caught 4+ times before). The API routes (/api/dashboard/config and /api/dashboard/review) correctly implement requireAuth() with fail-closed behavior, but the server actions bypass all of that.
Fix: Add requireAuth() at the top of each server action, identical to the pattern used in the API routes.
What's Good ✅
- API route auth is correct — Both
/api/dashboard/configand/api/dashboard/reviewuserequireAuth()that returns 503 when Supabase env vars are missing and 401 when user is not authenticated. Fail-closed. ✅ - Field whitelists — Config PUT has 28 allowed fields via
Set. Server actions haveALLOWED_FIELDSforupdateVideoField. Review API validatesactiontoapprove/rejectonly. ✅ getEngineConfig()integration — Correctly uses the new config API from PR #665, includinginvalidateEngineConfig()after writes. ✅- GROQ queries are mostly bounded — Pipeline page uses
[0..19]and[0..9]. Review detail uses[0]for single doc. ✅ - Mobile-first design — 44px min-height on all interactive buttons, responsive grids (
grid-cols-2 sm:grid-cols-3 md:grid-cols-4),flex-col gap-3 sm:flex-rowfor approve/reject. ✅ - CF Workflow webhook — Fire-and-forget with error logging, doesn't block approval. ✅
force-dynamicon all pages. ✅"use client"only on interactive components (config-form, review-detail-client). ✅- Sidebar navigation — Clean additions with appropriate icons. ✅
- rateCardTiers — Properly adds
_keyand_typefor Sanity array items. ✅
Issues to Fix 🔶
| # | Severity | File | Issue |
|---|---|---|---|
| 1 | 🚨 Blocker | review/actions.ts |
Server actions have NO auth — anyone can approve/reject/edit videos |
| 2 | 🔶 Medium | review/page.tsx |
Unbounded GROQ query (no [0..N] limit) |
| 3 | 🟡 Minor | config/config-form.tsx:371 |
as any type cast on toggle |
| 4 | 🟡 Minor | review/[id]/page.tsx:49 |
video as any bypasses type safety |
| 5 | 🟡 Minor | api/dashboard/config/route.ts:140 |
as any[] on rateCardTiers |
| 6 | 🟡 Minor | api/dashboard/config/route.ts |
notificationEmails, string fields not type-validated |
Architecture Notes
- No middleware.ts exists — Dashboard route protection relies entirely on per-route auth checks. This is acceptable as long as every route/action checks auth, but a middleware would provide defense-in-depth. Consider adding one in a follow-up.
- Dashboard layout has a comment saying "the proxy will have already redirected to login for protected routes" but no proxy/middleware exists. The layout itself doesn't block unauthenticated access — it just renders without the sidebar. This means unauthenticated users can see page content if the page itself doesn't check auth. The server component pages use
dashboardQuerywhich returns empty results without a Sanity token, so data leakage is limited, but the server actions are the real risk.
Summary
The API routes are well-secured. The UI is excellent — mobile-first, good UX, proper component architecture. But the server actions are a critical auth gap. Fix the auth on actions.ts and add a limit to the review queue query, then this is ready to merge.
| const ALLOWED_FIELDS = ["title", "script.hook", "script.cta"] as const; | ||
| type AllowedField = (typeof ALLOWED_FIELDS)[number]; | ||
|
|
||
| export async function approveVideo(id: string) { |
There was a problem hiding this comment.
🚨 BLOCKER: Server actions have NO auth checks
These server actions (approveVideo, rejectVideo, updateVideoField) are callable by anyone — they don't verify the user is authenticated before mutating Sanity documents.
Server actions are exposed as POST endpoints by Next.js. Any unauthenticated request can call them directly. This is the #1 recurring issue on this project.
Required fix: Add auth check at the top of each action:
import { createClient } from "@/lib/supabase/server";
async function requireAuth() {
const hasSupabase =
(process.env.NEXT_PUBLIC_SUPABASE_URL || process.env.SUPABASE_URL) &&
(process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY || process.env.SUPABASE_ANON_KEY);
if (!hasSupabase) throw new Error("Auth not configured");
const supabase = await createClient();
const { data: { user } } = await supabase.auth.getUser();
if (!user) throw new Error("Unauthorized");
return user;
}
export async function approveVideo(id: string) {
await requireAuth(); // ← ADD THIS
// ... rest of function
}Each of the 3 exported actions needs this check. Without it, anyone can approve/reject videos or update fields by calling the server action endpoint directly.
| export default async function ReviewQueuePage() { | ||
| const videos = await dashboardQuery<ReviewVideo[]>( | ||
| `*[_type == "automatedVideo" && status == "pending_review"] | order(_updatedAt desc) { | ||
| _id, title, qualityScore, qualityIssues, status, _updatedAt, |
There was a problem hiding this comment.
🔶 Unbounded GROQ query — no limit clause
This query fetches ALL pending_review videos with no [0..N] slice. If there are hundreds of pending videos, this will be slow and return a large payload.
The pipeline page correctly uses [0..19] and [0..9] limits. This should too:
*[_type == "automatedVideo" && status == "pending_review"] | order(_updatedAt desc) [0..49] {
...
}
50 is a reasonable limit for a review queue page. Add pagination if more are needed.
| > | ||
| <span | ||
| className={`pointer-events-none block h-5 w-5 rounded-full bg-background shadow-lg ring-0 transition-transform ${ | ||
| config[key] ? "translate-x-5" : "translate-x-0" |
There was a problem hiding this comment.
🔶 as any type cast — avoid this
This as any cast defeats TypeScript's type safety. The issue is that !config[key] returns boolean but the type system expects EngineConfig[typeof key].
Fix by being explicit about the boolean type:
onClick={() => update(key, !config[key] as EngineConfig[typeof key])}Or better, use a dedicated toggle handler:
const togglePlatform = (key: keyof Pick<EngineConfig, 'youtubeEnabled' | 'twitterEnabled' | ...>) => {
update(key, !config[key]);
};Minor issue but any should not be in new code per project standards.
| </Link> | ||
| </div> | ||
| <ReviewDetailClient video={video as any} /> | ||
| </div> |
There was a problem hiding this comment.
🔶 as any type cast on video prop
video as any bypasses type checking entirely. The dashboardQuery return type should match VideoData from the client component. Either:
- Add a generic type parameter:
dashboardQuery<VideoData>(...) - Or define a shared type and use it in both places
This is a code quality issue — the VideoData interface in review-detail-client.tsx should be the source of truth.
| if (!hasSupabase) { | ||
| return { | ||
| error: NextResponse.json( | ||
| { error: "Auth not configured" }, |
There was a problem hiding this comment.
🟢 Auth pattern looks correct — fails closed
Good: requireAuth() correctly returns 503 when Supabase env vars are missing, and 401 when user is not authenticated. Both GET and PUT call requireAuth() before any data access.
The "error" in auth && auth.error check pattern works but is slightly fragile. Consider a discriminated union pattern for clarity:
type AuthResult = { ok: true; user: User } | { ok: false; error: NextResponse };Not blocking — the current pattern works correctly.
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
🟡 Config PUT: String fields not validated for type or length
Fields like geminiModel, infographicModel, systemInstruction, brandPrimary, brandBackground, brandText, youtubeUploadVisibility, and contentCategories are whitelisted but not type-checked. A client could send:
geminiModel: 12345(number instead of string)systemInstruction: "<very long string>"(no length limit)brandPrimary: "not-a-hex-color"youtubeUploadVisibility: "invalid-value"
At minimum, validate that string fields are actually strings and have reasonable length limits. The youtubeUploadVisibility should be validated against ["private", "unlisted", "public"].
Not blocking but worth hardening.
|
|
||
| if ("rateCardTiers" in sanitized) { | ||
| if (!Array.isArray(sanitized.rateCardTiers)) { | ||
| return NextResponse.json( |
There was a problem hiding this comment.
🟡 rateCardTiers mapping uses as any[]
Another any cast. This should be typed:
interface RateCardTierInput {
_key?: string;
name?: unknown;
description?: unknown;
price?: unknown;
}
sanitized.rateCardTiers = (sanitized.rateCardTiers as RateCardTierInput[]).map(...)| await fetch(cfWorkersUrl, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ |
There was a problem hiding this comment.
🟢 Good: CF Workflow webhook failure doesn't block approval
Correct pattern — the webhook is fire-and-forget with error logging. The approval succeeds even if the CF Workers URL is unreachable. This is the right approach for a non-critical side effect.
| @@ -0,0 +1,177 @@ | |||
| import { NextResponse } from "next/server"; | |||
| import { createClient } from "@/lib/supabase/server"; | |||
| import { getEngineConfig, invalidateEngineConfig } from "@/lib/config"; | |||
There was a problem hiding this comment.
🟢 Good: Uses getEngineConfig() from PR #665
Correctly imports and uses the new config API from @/lib/config. The invalidateEngineConfig() call after PUT is also correct — ensures the next read gets fresh data.
| ).join(",\n ")} | ||
| }` | ||
| ); | ||
|
|
There was a problem hiding this comment.
🟢 Good: Pipeline queries are properly bounded
All three queries have proper limits:
- Status counts: single aggregation query (efficient)
- Active workflows:
[0..19](max 20) - Recent completions:
[0..9](max 10)
The status count query is clever — builds a single GROQ object with all counts in one round-trip. Well done.
- Add requireAuth() to all 3 server actions (approveVideo, rejectVideo, updateVideoField) - Fail closed: throws if Supabase not configured or user not authenticated - Use actual user email for reviewedBy instead of hardcoded string - Add [0..49] limit to review queue GROQ query Co-authored-by: dashboard <dashboard@miriad.systems>
1 participant
Task 1E: Review Dashboard — Mobile-First Control Center
Spec:
41bSaBJF(Content Engine v2)Task:
i9AiY0S7Branch:
feat/review-dashboard10 files, +1,686 lines
What's Built
Review Queue (
/dashboard/review)pending_reviewvideosReview Detail (
/dashboard/review/[id])Config Panel (
/dashboard/config)Pipeline Status (
/dashboard/pipeline)API Routes
POST /api/dashboard/review— approve/reject with CF Workflow webhookGET/PUT /api/dashboard/config— read/write engineConfigServer Actions
approveVideo(id)— patches status + reviewedAt/reviewedByrejectVideo(id, reason)— patches status + flaggedReasonupdateVideoField(id, field, value)— whitelist: title, script.hook, script.ctaAuth & Security
_keyfor Sanity arraysPatterns
force-dynamicon all pages"use client"only on interactive componentsDepends On