src/server/api/routers/listings/admin.test.ts (2)

21-43: 💤 Low value

Consider extracting the inline mock implementation to a shared test helper.

The inline implementation of createExistingAuthorBansMap duplicates business logic within the mock. If the real implementation changes, this could drift out of sync. Consider either importing the real helper or creating a shared test utility that both test files can use.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/api/routers/listings/admin.test.ts` around lines 21 - 43, The
inline mock function createExistingAuthorBansMap duplicates production logic and
should be extracted: move the implementation into a shared test helper (e.g.,
tests/helpers or tests/utils) or import the real helper if appropriate, then
replace the inline definition in admin.test.ts with an import of
createExistingAuthorBansMap and update any other tests to reuse that single
exported function so logic remains centralized and in-sync with production code.

---

131-217: ⚡ Quick win

Add test coverage for the non-risky filter path.

The test suite only covers riskFilter: 'risky'. Consider adding test cases for:

• riskFilter: 'all' to verify the standard pending listings path
• Edge cases like no risky listings found, or pagination edge cases

This would ensure both code paths are exercised and prevent regressions.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/api/routers/listings/admin.test.ts` around lines 131 - 217, Add
tests covering the non-risky and no-risky paths: in
src/server/api/routers/listings/admin.test.ts add a test calling
caller.getPending({ riskFilter: 'all', page: 1, limit: 20 }) that stubs
mockGetPendingListings to return expected rows and asserts
mockGetPendingListings was called (and mockGetPendingListingRiskCandidates was
not), verifies returned listings and pagination; also add a test for riskFilter:
'risky' when mockGetPendingListingRiskCandidates returns an empty array to
assert mockGetPendingListingsByIds is not called and result.listings.length and
pagination.total are 0. Ensure you reference and use the existing helpers/mocks
(createCaller, mockGetPendingListingRiskCandidates, mockGetPendingListingsByIds,
mockGetPendingListings) and mirror the style/assertions used in the existing
'filters risk-only listings' test.

src/server/api/routers/pcListings.test.ts (2)

81-103: ⚡ Quick win

Extract the inline mock implementation to a shared test utility.

This duplicates the same inline implementation pattern from admin.test.ts. Both test files would benefit from a shared test helper for createExistingAuthorBansMap to avoid duplication and ensure consistency.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/api/routers/pcListings.test.ts` around lines 81 - 103, Extract the
inline createExistingAuthorBansMap implementation into a shared test utility
function and have both tests import and use it instead of duplicating the logic;
specifically, create and export a helper named createExistingAuthorBansMap that
accepts the same listings shape and returns a Map<string, {reason:string}[]>,
then replace the inline implementations in pcListings.test.ts and admin.test.ts
with imports of that helper so both tests call the single shared function and
retain identical behavior.

---

477-561: ⚡ Quick win

Add test coverage for the non-risky filter path.

Similar to admin.test.ts, this test suite only covers riskFilter: 'risky'. Add test cases for:

• riskFilter: 'all' or undefined to verify the standard pending listings retrieval
• Edge cases such as empty results or different user roles

This ensures both code paths are tested and prevents regressions.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/api/routers/pcListings.test.ts` around lines 477 - 561, The suite
only tests the 'risky' riskFilter path; add tests that exercise the
non-risky/default path by calling caller.pending with riskFilter: 'all' and with
riskFilter omitted/undefined, verifying that mockRepositoryGetPendingListings is
used (not getPendingListingRiskCandidates/getPendingListingsByIds), that
returned pcListings match the mocked pending listings and pagination.total
reflects the count, and add an edge-case test for empty results and a test using
a non-moderator role (e.g., Role.USER) to validate permission-based branches;
reference mocks and helpers like createCaller, caller.pending,
mockRepositoryGetPendingListings, mockRepositoryGetPendingListingRiskCandidates,
mockRepositoryGetPendingListingsByIds, mockComputeAuthorRiskProfiles and
mockComputeSubmissionRiskProfiles to seed expected responses and assertions.

src/components/ui/ReviewRiskIndicator.test.tsx (1)

33-68: ⚡ Quick win

Add test coverage for the investigate interaction path.

The suite does not currently verify the clickable onInvestigate behavior (mouse + keyboard), which is core logic in the component.

🧪 Suggested test addition

-import { render, screen } from '@testing-library/react'
-import { describe, expect, it } from 'vitest'
+import { fireEvent, render, screen } from '@testing-library/react'
+import { describe, expect, it, vi } from 'vitest'
@@
   it('renders for submission risk without author risk', () => {
@@
   })
+
+  it('invokes investigate callback for clickable indicator', () => {
+    const onInvestigate = vi.fn()
+    render(
+      <ReviewRiskIndicator
+        authorRiskProfile={authorRiskProfile}
+        submissionRiskProfile={submissionRiskProfile}
+        onInvestigate={onInvestigate}
+      />,
+    )
+
+    const button = screen.getByRole('button')
+    fireEvent.click(button)
+    fireEvent.keyDown(button, { key: 'Enter' })
+    fireEvent.keyDown(button, { key: ' ' })
+
+    expect(onInvestigate).toHaveBeenCalledWith('author-1')
+    expect(onInvestigate).toHaveBeenCalledTimes(3)
+  })
 })

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/ui/ReviewRiskIndicator.test.tsx` around lines 33 - 68, Add
tests for the investigate interaction by rendering ReviewRiskIndicator with a
jest.fn() onInvestigate prop and asserting it is called on mouse click and
keyboard activation; locate the interactable element via
screen.getByRole('status') (or the same role used in the component), use
userEvent.click(status) to simulate mouse activation and
userEvent.keyboard('{Enter}') and userEvent.keyboard(' ') after status.focus()
to simulate Enter/Space keyboard activation, and assert onInvestigate was
invoked for each interaction; ensure the test covers both the case with
author/submission risk and the case with only submission risk so the click
target exists in both scenarios.

src/server/repositories/pc-listings.repository.ts (1)

890-894: 💤 Low value

Consider extracting active ban criteria to a helper method.

The inline active ban filtering (lines 891-893) duplicates the same logic used in forList include at lines 86-87. While correct, extracting this to a getActiveBanWhere() helper method (similar to listings.repository.ts line 572-577) would improve maintainability and ensure consistency across query builders.

♻️ Suggested refactor

Add a private helper method:

+  private getActiveBanWhere(): Prisma.UserBanWhereInput {
+    return {
+      isActive: true,
+      OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }],
+    }
+  }
+
   private getPendingListingRiskCandidateSelect() {
     return {
       id: true,
       authorId: true,
       author: {
         select: {
           userBans: {
-            where: {
-              isActive: true,
-              OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }],
-            },
+            where: this.getActiveBanWhere(),
             select: { reason: true },
           },
         },
       },

Then reuse in the forList include at line 85:

       author: {
         include: {
           userBans: {
-            where: {
-              isActive: true,
-              OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }],
-            },
+            where: this.getActiveBanWhere(),
             select: { id: true, reason: true, bannedAt: true, expiresAt: true },
           },
         },
       },

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/repositories/pc-listings.repository.ts` around lines 890 - 894,
Extract the duplicated active-ban predicate into a private helper (e.g., private
getActiveBanWhere()) that returns the object { isActive: true, OR: [{ expiresAt:
null }, { expiresAt: { gt: new Date() } }] }, add it to the PCListingsRepository
class, and replace the inline userBans.where in the forList include and the
other occurrence (the block using userBans: { where: { ... } }) to call
getActiveBanWhere() so both queries reuse the same logic for consistency.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/lib/cors.ts (1)

30-34: 💤 Low value

Optional: use a Set to dedupe instead of repeated includes scans.

addMissingOrigins does an O(n) includes per candidate, and getAllowedOrigins calls it 3–4 times. The list is small so this isn't a real performance issue, but a Set-based accumulator would be both cleaner and avoid the in-place mutation pattern.

♻️ Sketch

-function addMissingOrigins(origins: string[], additionalOrigins: string[]) {
-  for (const origin of additionalOrigins) {
-    if (!origins.includes(origin)) origins.push(origin)
-  }
-}
+function addMissingOrigins(origins: Set<string>, additionalOrigins: readonly string[]) {
+  for (const origin of additionalOrigins) origins.add(origin)
+}

Then build origins as a Set<string> in getAllowedOrigins and return [...origins] at the end.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/cors.ts` around lines 30 - 34, The current addMissingOrigins uses
repeated origins.includes scans and mutates an array; change the logic to use a
Set for deduplication: in getAllowedOrigins create a Set<string> (e.g., allowed
= new Set()), add the base origins and then add additional origins with Set.add
(or refactor addMissingOrigins to accept and add into a Set rather than an
array), and finally return Array.from(allowed) (or [...allowed]); update
references to addMissingOrigins/getAllowedOrigins accordingly to remove O(n)
includes and in-place array mutation.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/server/utils/query-builders.test.ts (1)

146-148: ⚡ Quick win

Add explicit null coverage for buildNsfwFilter.

Current tests validate omitted args but not null. Add a case for buildNsfwFilter(null) to lock expected behavior and prevent regressions.

Suggested test addition

   it('should return filter by default', () => {
     expect(buildNsfwFilter()).toEqual({ isErotic: false })
   })
+
+  it('should return filter when showNsfw is null', () => {
+    expect(buildNsfwFilter(null)).toEqual({ isErotic: false })
+  })

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/utils/query-builders.test.ts` around lines 146 - 148, Add an
explicit unit test that calls buildNsfwFilter(null) and asserts it returns the
same default object as when no argument is provided
(expect(buildNsfwFilter(null)).toEqual({ isErotic: false })), so the behavior
for null input is locked and regressions are prevented; place this alongside the
existing 'should return filter by default' test in the query-builders tests and
reference the buildNsfwFilter function in the new test.

tests/helpers/data-factory.ts (1)

595-609: 💤 Low value

Consider moving the click outside the retry loop.

The current implementation clicks the button inside the toPass callback (line 602), which may result in multiple clicks if the dialog takes longer than 2 seconds to appear. While this likely works, it could be more efficient to click once and only retry the visibility assertion:

async function openUserDetailsDialog(page: Page, userRow: Locator): Promise<Locator> {
  const viewDetailsButton = userRow.getByRole('button', { name: 'View User Details' })
  await expect(viewDetailsButton).toBeVisible()
  
  const dialog = page.locator('[role="dialog"]')
  
  // Click once outside the retry loop
  if (!(await dialog.isVisible())) {
    await viewDetailsButton.click()
  }
  
  // Then retry only the visibility check
  await expect(dialog).toBeVisible({ timeout: 10000 })
  
  return dialog
}

However, the current implementation with the conditional click inside toPass does provide more resilience if the button needs to be clicked multiple times, so this is optional.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/helpers/data-factory.ts` around lines 595 - 609, The click on
viewDetailsButton is currently inside the retry callback in
openUserDetailsDialog which can cause multiple clicks; instead, perform a single
click before entering the retry/assertion and then only retry the visibility
check: check viewDetailsButton visibility, compute dialog =
page.locator('[role="dialog"]'), if dialog isn't visible await
viewDetailsButton.click(), then replace the toPass block with a single await
expect(dialog).toBeVisible({ timeout: 10000 }) and return dialog.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/utils/custom-field-validation.ts (1)

57-71: 💤 Low value

Consider adding a default case for defensive coding.

The switch on field.type lacks a default case. If CustomFieldType is extended or runtime data has an unexpected type, the function returns undefined, causing line 94 to treat valid values as invalid. While TypeScript's exhaustiveness checking mitigates compile-time risk, a defensive default prevents silent failures.

🛡️ Suggested defensive default

     case CustomFieldType.TEXT:
     case CustomFieldType.TEXTAREA:
       return typeof value === 'string'
+    default:
+      return true
   }
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/custom-field-validation.ts` around lines 57 - 71, The switch in
isCustomFieldValueValid can fall through and return undefined for unknown
CustomFieldType values; add a defensive default branch in the switch on
field.type (inside isCustomFieldValueValid) that returns false (or logs/throws
if you prefer strict behavior) so the function always returns a boolean for any
runtime value of CustomFieldType and avoids silent failures when new types are
added or invalid data is passed.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes