Skip to content

Conversation

@franky47
Copy link

@franky47 franky47 commented Oct 23, 2025

Description

  • Map all filters to a single descriptor object
  • Expand it to describe the URL sent to React Query
  • Derive a loader & URL serializer for the API endpoint

Motivation and Context

Post-Next.js conf chat with Dominik.

How to Test

Screenshots (if applicable)

Video Demo (if applicable)

Types of Changes

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that alters existing functionality)
  • 🎨 UI/UX Improvements
  • ⚡ Performance Enhancement
  • 📖 Documentation (updates to README, docs, or comments)

Summary by CodeRabbit

  • New Features

    • Added upload progress tracking and status messaging for media uploads.
  • Improvements

    • Refactored media filtering and sorting system for improved consistency and URL-based parameter handling.
    • Simplified media controls interface for streamlined user interactions.

- Map all filters to a single descriptor object
- Expand it to describe the URL sent to React Query
- Derive a loader & URL serializer for the API endpoint
@vercel
Copy link

vercel bot commented Oct 23, 2025

@franky47 is attempting to deploy a commit to the Marble Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

Walkthrough

This PR refactors media feature state management by centralizing search parameter parsing into a new search-params.ts module with dedicated utilities. It introduces useMediaPageFilters hook to replace local filter state, implements getMediaApiUrl helper for consistent API URL construction, adds splitMediaSort utility for sort parsing, and updates the media API route and components to use these new abstractions.

Changes

Cohort / File(s) Summary
Search Parameters Centralization
apps/cms/src/lib/search-params.ts
New module introducing useMediaPageFilters, loadMediaApiFilters, and getMediaApiUrl utilities for type-safe parsing, serialization, and server-side loading of media query parameters (type, sort, cursor, limit).
Constants Refactoring
apps/cms/src/lib/constants.ts
Added MEDIA_SORT_BY and SORT_DIRECTIONS constants; refactored MEDIA_SORTS to be computed via flatMap for consistency.
Media Utilities Enhancement
apps/cms/src/utils/media.ts
Enhanced isMediaSort to a type guard; added new splitMediaSort utility to parse sort values into field and direction components.
Validation Cleanup
apps/cms/src/lib/validations/upload.ts
Removed GetSchema export; media API validation now handled via new loadMediaApiFilters helper.
Component Refactoring
apps/cms/src/components/media/media-controls.tsx
Replaced external props (type, setType, sort, setSort) with internal state management via useMediaPageFilters hook.
Media Page Client Update
apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsx
Replaced manual URLSearchParams construction with getMediaApiUrl; removed local filter state in favor of useMediaPageFilters; added upload progress tracking (isUploading, statusMessage).
API Route Refactoring
apps/cms/src/app/api/media/route.ts
Updated request parsing to use loadMediaApiFilters with strict limit validation; introduced splitMediaSort for consistent sort parameter handling.

Sequence Diagram

sequenceDiagram
    participant PageClient as Media Page Client
    participant Hook as useMediaPageFilters
    participant SearchParams as Search Params
    participant API as Media API Route
    participant DB as Database

    PageClient->>Hook: useMediaPageFilters()
    Hook->>SearchParams: Parse type, sort from URL
    SearchParams-->>Hook: Return { type, sort }
    Hook-->>PageClient: Return state & setters
    
    PageClient->>SearchParams: getMediaApiUrl({ type, sort, limit, cursor })
    SearchParams->>SearchParams: Serialize to query string
    SearchParams-->>PageClient: Return API URL

    PageClient->>API: GET /api/media?type=...&sort=...&limit=...
    API->>SearchParams: loadMediaApiFilters()
    SearchParams->>API: Parse & validate params
    
    rect rgb(200, 220, 255)
        Note over API: Validate limit (1..100)
        API->>API: splitMediaSort(sort)
        API->>API: Extract field & direction
    end
    
    API->>DB: Query with filters & orderBy
    DB-->>API: Return media items
    API-->>PageClient: Return { items, nextCursor }
    PageClient-->>PageClient: Update UI with results
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • New module (search-params.ts): Requires validation of type-safe parser setup, defaults handling, and integration with nuqs library; ensure sort parsing logic correctly handles all field/direction combinations
  • API route changes: Verify limit validation bounds (1..100) and error handling; ensure loadMediaApiFilters properly sanitizes all inputs before database queries
  • Component refactoring: Confirm that hook-based state management correctly replaces prop-based control flow in MediaControls; check for side effects or missed state updates
  • Interconnected changes: Trace flow from page client → hook → API URL generation → route handling to ensure consistency in parameter naming and validation

Possibly related PRs

Suggested reviewers

  • taqh
  • prateekbisht23

Poem

🐰 Hops of joy through search-param trees,
Type-safe sorts drift in the breeze!
Hooks now manage the filter dance,
URLs centralized—what a glance!
From chaos to clarity we spring,
A refactored media wing!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ❓ Inconclusive The PR description provides high-level bullet points explaining what changed (mapping filters to descriptors, expanding to describe URLs, deriving loader and serializer), but falls short in critical areas. The motivation is vague, referencing only "Post-Next.js conf chat with Dominik" without explaining the actual problem this refactor solves or what benefit it brings. Testing instructions are completely absent despite being a significant refactor affecting query parameter handling, and the "Types of Changes" checklist is included but no options are marked. While the description is not entirely off-topic, the vague motivation and missing testing guidance create ambiguity about whether the changes have been properly validated.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "refactor: use nuqs for end-to-end type-safe search params" is clear and specific, accurately reflecting the main change across the codebase. It identifies the key tool being introduced (nuqs) and the problem being addressed (type-safe search parameter handling). The title is concise and would enable teammates scanning git history to quickly understand that this refactor centralizes search parameter management with type safety, which aligns with the substantial changes observed across multiple files including the new search-params.ts module and updates to media controls, constants, and utilities.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@mezotv mezotv marked this pull request as ready for review October 25, 2025 20:56
Copy link
Contributor

@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: 0

Caution

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

⚠️ Outside diff range comments (2)
apps/cms/src/app/api/media/route.ts (1)

127-128: Remove unnecessary await on parsed.data.

safeParse returns a sync object; awaiting it is superfluous.

-  const { mediaId, mediaIds } = await parsed.data;
+  const { mediaId, mediaIds } = parsed.data;
apps/cms/src/utils/media.ts (1)

50-54: Fix the type predicate to accept unknown and check against the constant to prevent rejecting valid "audio" and "document" values.

The current implementation has two critical issues:

  1. Logic bug: The hardcoded array ["all", "image", "video"] omits "audio" and "document", which are part of MediaFilterType. This causes the function to incorrectly reject valid values.

  2. Type predicate flaw: The signature accepts MediaFilterType (already narrowed) and returns value is MediaFilterType (no narrowing occurs—it's tautological). Type predicates should accept a broader type like unknown and narrow to the specific type.

+import { MEDIA_FILTER_TYPES } from "@/lib/constants";
 import type { MEDIA_SORT_BY, SORT_DIRECTIONS } from "@/lib/constants";
 import type { MediaFilterType, MediaSort, MediaType } from "@/types/media";

 export function isMediaFilterType(
-  value: MediaFilterType
+  value: unknown
 ): value is MediaFilterType {
-  return ["all", "image", "video"].includes(value as string);
+  return (MEDIA_FILTER_TYPES as readonly string[]).includes(value as string);
 }
🧹 Nitpick comments (7)
.vscode/settings.json (1)

2-2: Workspace settings look good; minor optional tweak.

Config aligns Biome LSP and code actions; LGTM. Optionally, consider using ${workspaceFolder}/node_modules/.bin/biome for clarity on multi-root setups.

Also applies to: 14-17

apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsx (3)

41-50: Pass React Query’s abort signal to fetch to avoid wasted work on cancels.

Use the signal from queryFn context.

-  queryFn: async ({ pageParam }: { pageParam?: string }) => {
+  queryFn: async ({ pageParam, signal }: { pageParam?: string; signal?: AbortSignal }) => {
     try {
       const url = getMediaApiUrl("/api/media", {
         sort,
         type: normalizedType,
         cursor: pageParam,
       });
-      const res = await fetch(url);
+      const res = await fetch(url, { signal });

And in prefetch:

-  queryFn: async ({ pageParam }: { pageParam?: string }) => {
+  queryFn: async ({ pageParam, signal }: { pageParam?: string; signal?: AbortSignal }) => {
     const url = getMediaApiUrl("/api/media", {
       sort: sortOption,
       type: normalizedType,
       cursor: pageParam,
     });
-    const res = await fetch(url);
+    const res = await fetch(url, { signal });

Also applies to: 94-102


145-177: Don’t await inside upload loop; add bounded concurrency and remove console.

Sequential uploads are slow and violate the guideline. Use a small pool (e.g., 3–5). Replace console.error with your logger or surface via toasts.

-  for (const file of Array.from(files)) {
-    try {
-      await uploadFile({ file, type: "media" });
-      uploaded++;
-    } catch (error) {
-      console.error(`Failed to upload ${file.name}:`, error);
-      errors.push({ file: file.name, error: error instanceof Error ? error.message : "Unknown error" });
-      failed++;
-    }
-    const message = `Uploading ${uploaded} of ${total} files...`;
-    toast.loading(message, { id: toastId });
-    setStatusMessage(message);
-  }
+  const queue = Array.from(files);
+  const concurrency = 4;
+  let idx = 0;
+  const runNext = async () => {
+    const i = idx++;
+    const file = queue[i];
+    if (!file) return;
+    try {
+      await uploadFile({ file, type: "media" });
+      uploaded++;
+    } catch (error) {
+      errors.push({ file: file.name, error: error instanceof Error ? error.message : "Unknown error" });
+      failed++;
+    } finally {
+      const message = `Uploading ${uploaded} of ${total} files...`;
+      toast.loading(message, { id: toastId });
+      setStatusMessage(message);
+      await runNext();
+    }
+  };
+  await Promise.allSettled(Array.from({ length: concurrency }, runNext));

Also replace any remaining console.* with your logging utility.


79-115: Prefetch breadth may be heavy (MEDIA_FILTER_TYPES × MEDIA_SORTS).

20+ concurrent prefetches can spike load. Consider gating by idle time, splitting across ticks, or limiting to nearest neighbors of current type/sort.

apps/cms/src/app/api/media/route.ts (1)

161-200: Avoid await in loops; replace console with structured logs.

  • Delete from R2 and send webhooks with bounded concurrency (Promise.allSettled + pool) to reduce latency.
  • Replace console.error with your logger/instrumentation to meet guidelines.
-  for (const media of existingMedia) {
-    if (media.url) {
-      try {
-        // ...
-        await r2.send(new DeleteObjectCommand({ Bucket: R2_BUCKET_NAME, Key: key }));
-        successfullyDeletedFromR2.push({ id: media.id, media });
-      } catch (error) {
-        console.error(/* ... */);
-        failedIds.push(media.id);
-      }
-    } else {
-      console.error(/* ... */);
-      successfullyDeletedFromR2.push({ id: media.id, media });
-    }
-  }
+  const concurrency = 5;
+  let idx = 0;
+  const runDelete = async () => {
+    const i = idx++;
+    const media = existingMedia[i];
+    if (!media) return;
+    try {
+      if (media.url) {
+        // ... compute key as before
+        await r2.send(new DeleteObjectCommand({ Bucket: R2_BUCKET_NAME, Key: key }));
+      }
+      successfullyDeletedFromR2.push({ id: media.id, media });
+    } catch (error) {
+      // logger.error(/* ... */);
+      failedIds.push(media.id);
+    } finally {
+      await runDelete();
+    }
+  };
+  await Promise.allSettled(Array.from({ length: concurrency }, runDelete));

And for webhooks:

-  for (const { media } of successfullyDeletedFromR2) {
-    const webhooks = getWebhooks(sessionData.session, "media_deleted");
-    for (const webhook of await webhooks) {
-      const webhookClient = new WebhookClient({ secret: webhook.secret });
-      await webhookClient.send({ /* ... */ });
-    }
-    deletedIds.push(media.id);
-  }
+  await Promise.allSettled(
+    successfullyDeletedFromR2.map(async ({ media }) => {
+      const webhooks = await getWebhooks(sessionData.session, "media_deleted");
+      await Promise.allSettled(
+        webhooks.map((webhook) =>
+          new WebhookClient({ secret: webhook.secret }).send({ /* ... */ }),
+        ),
+      );
+      deletedIds.push(media.id);
+    }),
+  );

If latency matters, consider scheduling webhook sends after responding (e.g., background task runner).

Also applies to: 209-225, 188-191, 195-199

apps/cms/src/lib/constants.ts (1)

193-198: Apply template-literal type and readonly assertion to preserve MediaSort union.

The current code may allow TypeScript to widen MEDIA_SORTS from a computed union to a broader string[] type. Defining an explicit MediaSort template-literal type and asserting the result as readonly MediaSort[] ensures the literal union is preserved at compile-time and prevents silent type degradation where sortOption becomes string instead of the specific literal union.

The existing MediaSort definition in apps/cms/src/types/media.ts (line 24) will continue to work after this change since (typeof MEDIA_SORTS)[number] will correctly resolve to the union type.

export const MEDIA_SORT_BY = ["createdAt", "name"] as const;
export const SORT_DIRECTIONS = ["asc", "desc"] as const;

+export type MediaSort =
+  `${(typeof MEDIA_SORT_BY)[number]}_${(typeof SORT_DIRECTIONS)[number]}`;
+
-export const MEDIA_SORTS = MEDIA_SORT_BY.flatMap((field) =>
-  SORT_DIRECTIONS.map((direction) => `${field}_${direction}` as const)
-);
+export const MEDIA_SORTS = MEDIA_SORT_BY.flatMap((field) =>
+  SORT_DIRECTIONS.map((direction) => `${field}_${direction}` as const),
+) as readonly MediaSort[];
apps/cms/src/lib/search-params.ts (1)

20-35: LGTM: Sort parser correctly validates field_direction format.

The custom parser properly splits, validates, and recombines sort parameters, returning null for invalid combinations. The logic handles edge cases (e.g., missing underscore) correctly.

Optional: Consider explicit serialize function.

Line 33 uses String as the serialize function. While valid, an explicit arrow function may be more idiomatic:

-    serialize: String,
+    serialize: (value) => value,
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9c5cf0b and f48035d.

📒 Files selected for processing (8)
  • .vscode/settings.json (2 hunks)
  • apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsx (3 hunks)
  • apps/cms/src/app/api/media/route.ts (2 hunks)
  • apps/cms/src/components/media/media-controls.tsx (3 hunks)
  • apps/cms/src/lib/constants.ts (1 hunks)
  • apps/cms/src/lib/search-params.ts (1 hunks)
  • apps/cms/src/lib/validations/upload.ts (0 hunks)
  • apps/cms/src/utils/media.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • apps/cms/src/lib/validations/upload.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{ts,tsx,js,jsx}: Don't use consecutive spaces in regular expression literals
Don't use the arguments object
Don't use the comma operator
Avoid functions exceeding allowed Cognitive Complexity
Don't use unnecessary boolean casts
Don't use unnecessary callbacks with flatMap
Prefer for...of over Array.forEach
Don't create classes that only have static members
Don't use this and super in static contexts
Don't use unnecessary catch clauses
Don't use unnecessary constructors
Don't use unnecessary continue statements
Don't export empty modules
Don't use unnecessary escape sequences in regex literals
Don't use unnecessary labels
Don't use unnecessary nested block statements
Don't rename imports/exports/destructured assignments to the same name
Don't use unnecessary string or template literal concatenation
Don't use String.raw in template literals when no escapes are present
Don't use useless case clauses in switch
Avoid ternaries when simpler alternatives exist
Don't use useless this aliasing
Don't initialize variables to undefined
Don't use the void operator
Prefer arrow functions over function expressions
Use Date.now() for epoch milliseconds
Prefer .flatMap() over map().flat() when possible
Use literal property access instead of computed when possible
Don't use parseInt/Number.parseInt when numeric literals suffice (bin/oct/hex)
Use optional chaining instead of chained logical expressions
Prefer regex literals over RegExp constructor when possible
Don't use non-base-10 or underscore separators in number member names
Remove redundant terms from logical expressions
Prefer while loops when initializer/update not needed in for
Don't reassign const variables
Don't use constant expressions in conditions
Don't use Math.min/Math.max to clamp when result is constant
Don't return a value from a constructor
Don't use empty character classes in regex literals
Don't use empty destructuring patterns
Don't call global object properties as functions
Don't declare functions/vars a...

Files:

  • apps/cms/src/app/api/media/route.ts
  • apps/cms/src/lib/constants.ts
  • apps/cms/src/components/media/media-controls.tsx
  • apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsx
  • apps/cms/src/utils/media.ts
  • apps/cms/src/lib/search-params.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations when initialized with literals
Don't use TypeScript namespaces
Don't use non-null assertions (postfix !)
Don't use parameter properties in class constructors
Don't use user-defined types (type predicates abuse)
Prefer as const over literal types/type annotations
Use array type consistently (either T[] or Array)
Initialize each enum member value explicitly
Use export type for types
Use import type for types
All enum members must be literal values
Don't use const enum
Don't declare empty interfaces
Don't let variables evolve into any via reassignments
Don't use the any type
Don't misuse non-null assertion operator
No implicit any in variable declarations
Don't merge interfaces and classes unsafely
Place overload signatures adjacent
Use namespace keyword instead of module to declare TS namespaces
Use consistent accessibility modifiers on class members
Use function types instead of object types with call signatures
Don't use void type outside of generic or return types

Files:

  • apps/cms/src/app/api/media/route.ts
  • apps/cms/src/lib/constants.ts
  • apps/cms/src/components/media/media-controls.tsx
  • apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsx
  • apps/cms/src/utils/media.ts
  • apps/cms/src/lib/search-params.ts
**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{jsx,tsx}: Don't use the accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, or properties to elements that don't support them
Don't use distracting elements like or
Only use the scope prop on elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Ensure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex
Don't include "image", "picture", or "photo" in img alt text
Don't use an explicit role identical to the element's implicit role
Static elements with click handlers must have a valid role
Always include a <title> element for SVGs
Provide meaningful alt text for elements requiring it
Ensure anchors have content accessible to screen readers
Assign tabIndex to non-interactive elements when using aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Ensure ARIA properties are valid for the element's supported roles
Always include type attribute for button elements
Elements with interactive roles/handlers must be focusable
Headings must have content accessible to screen readers (not aria-hidden)
Always include lang attribute on the html element
Always include title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp/onKeyDown/onKeyPress
Pair onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Prefer semantic elements over role attributes in JSX
Ensure all anchors are valid and navigable
Ensure all aria-* properties are valid
Use valid, non-abstract ARIA roles
Use valid values for the autocomplete attribute on inputs
Use correct ISO codes for the lang attribute
Don't use unnecessary fragments
Don't pass children as props
Don't use the return value of React.render
Ensure all ...

Files:

  • apps/cms/src/components/media/media-controls.tsx
  • apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

Don't destructure props inside JSX components in Solid projects

Files:

  • apps/cms/src/components/media/media-controls.tsx
  • apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsx
🧬 Code graph analysis (5)
apps/cms/src/app/api/media/route.ts (2)
apps/cms/src/lib/search-params.ts (1)
  • loadMediaApiFilters (56-56)
apps/cms/src/utils/media.ts (1)
  • splitMediaSort (42-48)
apps/cms/src/components/media/media-controls.tsx (3)
apps/cms/src/lib/search-params.ts (1)
  • useMediaPageFilters (45-46)
apps/cms/src/types/media.ts (1)
  • MediaFilterType (11-11)
apps/cms/src/utils/media.ts (1)
  • isMediaFilterType (50-54)
apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsx (2)
apps/cms/src/hooks/use-workspace-id.ts (1)
  • useWorkspaceId (7-10)
apps/cms/src/lib/search-params.ts (2)
  • useMediaPageFilters (45-46)
  • getMediaApiUrl (58-60)
apps/cms/src/utils/media.ts (2)
apps/cms/src/types/media.ts (1)
  • MediaSort (24-24)
apps/cms/src/lib/constants.ts (2)
  • MEDIA_SORT_BY (193-193)
  • SORT_DIRECTIONS (194-194)
apps/cms/src/lib/search-params.ts (1)
apps/cms/src/lib/constants.ts (5)
  • SORT_DIRECTIONS (194-194)
  • MEDIA_SORT_BY (193-193)
  • MEDIA_FILTER_TYPES (202-202)
  • MEDIA_TYPES (200-200)
  • MEDIA_LIMIT (204-204)
🔇 Additional comments (15)
apps/cms/src/utils/media.ts (2)

42-48: splitMediaSort looks good.

Typed tuple cast matches constants; LGTM.


35-40: Use MEDIA_SORTS constant for sort validation to prevent drift.

The refactoring suggestion is valid. MEDIA_SORTS is generated by combining MEDIA_SORT_BY (["createdAt", "name"]) with SORT_DIRECTIONS (["asc", "desc"]), producing the exact values currently hardcoded. Using the constant instead ensures that if sort options change, the validation updates automatically without requiring manual edits to the utility function.

The suggested type casting to readonly string[] is necessary to satisfy TypeScript's type guard requirements while calling .includes().

apps/cms/src/app/api/media/route.ts (1)

28-35: GET filters + limit validation: LGTM.

Strict loader + zod bounds check reads clean. The orderBy with id tie‑breaker is correct.

apps/cms/src/app/(main)/[workspace]/(dashboard)/media/page-client.tsx (1)

36-40: This review comment is incorrect and its suggested refactor would break the code.

The current implementation is sound:

  • useWorkspaceId() returns string | null
  • MediaQueryKey type requires the specific tuple structure ["media", string, { type?, sort }]
  • The suggested ternary alternative [["media", "pending"], ...] does not satisfy the MediaQueryKey type contract
  • Passing that malformed key to useMediaActions(mediaQueryKey) at line 126 would cause a type error

The actual pattern here is correct: the code uses runtime guards (enabled: !!workspaceId at line 50 and if (!workspaceId) return in useMediaActions) to ensure the key is only used when workspaceId is truthy. The ! assertions are justified and appropriately documented with lint ignores.

Likely an incorrect or invalid review comment.

apps/cms/src/components/media/media-controls.tsx (4)

16-17: LGTM: Clean import of the new hook.

The imports correctly bring in the new useMediaPageFilters hook from the centralized search-params module, aligning with the refactor's goal of type-safe URL state management.


38-38: LGTM: Hook correctly replaces prop-based state.

The migration from component props to URL-backed state via useMediaPageFilters is implemented correctly, centralizing filter state management in the URL.


42-48: LGTM: Type filter correctly updates URL state.

The handler properly validates the input with isMediaFilterType before updating search params, ensuring type safety at runtime.


59-65: LGTM: Sort filter correctly updates URL state.

The handler properly validates the input with isMediaSort before updating search params, mirroring the type filter pattern.

apps/cms/src/lib/search-params.ts (7)

1-18: LGTM: Clean imports from nuqs library.

The imports correctly bring in nuqs v2.6.0 utilities for type-safe search parameter handling, along with the necessary constants.


37-37: LGTM: Sort parser with sensible default.

The sortParser correctly applies parseAsSort to the allowed media sort fields with a reasonable default of "createdAt_desc".


39-43: LGTM: Page-level search params properly defined.

The mediaPageSearchParams correctly configures page-level filters with MEDIA_FILTER_TYPES (including "all") and sensible defaults.


45-46: LGTM: Clean hook export for page-level filters.

The useMediaPageFilters hook properly wraps useQueryStates with the page-level parameter schema, providing type-safe access to URL state.


48-54: LGTM: API-level search params correctly configured.

The mediaApiSearchParams appropriately uses MEDIA_TYPES (without "all") for API calls, adds pagination support via cursor and limit, and reuses the sortParser. The intentional difference between page and API params is appropriate.


56-56: LGTM: Server-side loader correctly configured.

The loadMediaApiFilters export properly uses createLoader for server-side search parameter parsing.


58-60: LGTM: URL serializer correctly configured.

The getMediaApiUrl export properly uses createSerializer with clearOnDefault: false, ensuring default values are explicitly included in API URLs.

Copy link
Contributor

@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: 0

🧹 Nitpick comments (1)
apps/cms/src/lib/constants.ts (1)

196-198: Add top-level as const assertion for consistency.

While individual elements are correctly typed with as const on line 197, the MEDIA_SORTS array itself remains mutable. For consistency with MEDIA_SORT_BY and SORT_DIRECTIONS (which are readonly), and to match the pattern used elsewhere in this file (lines 102, 109, 115), add a top-level as const assertion:

 export const MEDIA_SORTS = MEDIA_SORT_BY.flatMap((field) =>
   SORT_DIRECTIONS.map((direction) => `${field}_${direction}` as const)
-);
+) as const;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f48035d and 5838155.

📒 Files selected for processing (1)
  • apps/cms/src/lib/constants.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{ts,tsx,js,jsx}: Don't use consecutive spaces in regular expression literals
Don't use the arguments object
Don't use the comma operator
Avoid functions exceeding allowed Cognitive Complexity
Don't use unnecessary boolean casts
Don't use unnecessary callbacks with flatMap
Prefer for...of over Array.forEach
Don't create classes that only have static members
Don't use this and super in static contexts
Don't use unnecessary catch clauses
Don't use unnecessary constructors
Don't use unnecessary continue statements
Don't export empty modules
Don't use unnecessary escape sequences in regex literals
Don't use unnecessary labels
Don't use unnecessary nested block statements
Don't rename imports/exports/destructured assignments to the same name
Don't use unnecessary string or template literal concatenation
Don't use String.raw in template literals when no escapes are present
Don't use useless case clauses in switch
Avoid ternaries when simpler alternatives exist
Don't use useless this aliasing
Don't initialize variables to undefined
Don't use the void operator
Prefer arrow functions over function expressions
Use Date.now() for epoch milliseconds
Prefer .flatMap() over map().flat() when possible
Use literal property access instead of computed when possible
Don't use parseInt/Number.parseInt when numeric literals suffice (bin/oct/hex)
Use optional chaining instead of chained logical expressions
Prefer regex literals over RegExp constructor when possible
Don't use non-base-10 or underscore separators in number member names
Remove redundant terms from logical expressions
Prefer while loops when initializer/update not needed in for
Don't reassign const variables
Don't use constant expressions in conditions
Don't use Math.min/Math.max to clamp when result is constant
Don't return a value from a constructor
Don't use empty character classes in regex literals
Don't use empty destructuring patterns
Don't call global object properties as functions
Don't declare functions/vars a...

Files:

  • apps/cms/src/lib/constants.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations when initialized with literals
Don't use TypeScript namespaces
Don't use non-null assertions (postfix !)
Don't use parameter properties in class constructors
Don't use user-defined types (type predicates abuse)
Prefer as const over literal types/type annotations
Use array type consistently (either T[] or Array)
Initialize each enum member value explicitly
Use export type for types
Use import type for types
All enum members must be literal values
Don't use const enum
Don't declare empty interfaces
Don't let variables evolve into any via reassignments
Don't use the any type
Don't misuse non-null assertion operator
No implicit any in variable declarations
Don't merge interfaces and classes unsafely
Place overload signatures adjacent
Use namespace keyword instead of module to declare TS namespaces
Use consistent accessibility modifiers on class members
Use function types instead of object types with call signatures
Don't use void type outside of generic or return types

Files:

  • apps/cms/src/lib/constants.ts
⏰ 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: Cursor Bugbot

@CLAassistant
Copy link

CLAassistant commented Nov 9, 2025

CLA assistant check
All committers have signed the CLA.

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.

3 participants