diff --git a/src/app/admin/api-access/components/AdminCreateKeyForm.tsx b/src/app/admin/api-access/components/AdminCreateKeyForm.tsx index 738611813..b1bc288df 100644 --- a/src/app/admin/api-access/components/AdminCreateKeyForm.tsx +++ b/src/app/admin/api-access/components/AdminCreateKeyForm.tsx @@ -259,6 +259,7 @@ export function AdminCreateKeyForm(props: Props) {
form.setValue('isActive', checked)} /> -
diff --git a/src/app/admin/releases/components/CreateReleaseCard.tsx b/src/app/admin/releases/components/CreateReleaseCard.tsx index c09e0e21b..89a0f5f1c 100644 --- a/src/app/admin/releases/components/CreateReleaseCard.tsx +++ b/src/app/admin/releases/components/CreateReleaseCard.tsx @@ -327,10 +327,12 @@ export default function CreateReleaseCard() {
- +
@@ -137,7 +147,7 @@ function ProfileInformation(props: Props) { /> ) : (

- {props.userQuery.data?.bio || 'No bio added yet'} + {profileData?.bio || 'No bio added yet'}

)} diff --git a/src/app/users/[id]/UserProfilePage.tsx b/src/app/users/[id]/UserProfilePage.tsx index 49733729b..731319dba 100644 --- a/src/app/users/[id]/UserProfilePage.tsx +++ b/src/app/users/[id]/UserProfilePage.tsx @@ -241,7 +241,7 @@ function UserDetailsPage() { activityTabs.push({ id: 'votes', label: 'Votes', - count: userQuery.data?.votes.pagination.total ?? 0, + count: userQuery.data?._count?.votes ?? 0, }) } if (showBookmarksTab) { @@ -737,6 +737,7 @@ function UserDetailsPage() { {activeTab === 'bookmarks' && bookmarkCounts && ( diff --git a/src/app/users/[id]/components/UserBookmarksSection.tsx b/src/app/users/[id]/components/UserBookmarksSection.tsx index 9eebad94b..4c52ffd76 100644 --- a/src/app/users/[id]/components/UserBookmarksSection.tsx +++ b/src/app/users/[id]/components/UserBookmarksSection.tsx @@ -4,6 +4,7 @@ import BookmarkBrowser from '@/app/profile/components/bookmarks/BookmarkBrowser' interface Props { userId: string + ownProfile?: boolean handheldCount: number pcCount: number } diff --git a/src/components/CookieConsent.test.tsx b/src/components/CookieConsent.test.tsx index a4cc124de..fd7747a40 100644 --- a/src/components/CookieConsent.test.tsx +++ b/src/components/CookieConsent.test.tsx @@ -2,6 +2,7 @@ import { act, cleanup, fireEvent, render, renderHook, screen } from '@testing-li import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import storageKeys from '@/data/storageKeys' import { useLocalStorage } from '@/hooks/useLocalStorage' +import toast from '@/lib/toast' import { BooleanPreferenceSchema } from '@/schemas/preferences' import CookieConsent from './CookieConsent' @@ -17,6 +18,7 @@ Object.defineProperty(window, 'localStorage', { }) beforeEach(() => { + vi.clearAllMocks() storage.clear() vi.useFakeTimers() }) @@ -49,6 +51,7 @@ describe('cookie banner storage updates', () => { act(() => vi.advanceTimersByTime(2000)) fireEvent.click(screen.getByRole('button', { name: 'Necessary Only' })) expect(screen.getByRole('alert')).toHaveTextContent('Please try again') + expect(toast.warning).not.toHaveBeenCalled() expect(screen.getByRole('button', { name: 'Necessary Only' })).toBeInTheDocument() writer.mockRestore() fireEvent.click(screen.getByRole('button', { name: 'Necessary Only' })) diff --git a/src/components/CookieConsent.tsx b/src/components/CookieConsent.tsx index 477e2a6f2..0d0345378 100644 --- a/src/components/CookieConsent.tsx +++ b/src/components/CookieConsent.tsx @@ -113,11 +113,11 @@ function CookieConsent() { setDraftPreferences(newPreferences) setSaveError(null) try { - setSavedPreferences(newPreferences) - setConsentDate(new Date().toISOString()) - setAnalyticsEnabled(newPreferences.analytics) - setPerformanceEnabled(newPreferences.performance) - setHasConsented(true) + setSavedPreferences(newPreferences, { notifyOnError: false }) + setConsentDate(new Date().toISOString(), { notifyOnError: false }) + setAnalyticsEnabled(newPreferences.analytics, { notifyOnError: false }) + setPerformanceEnabled(newPreferences.performance, { notifyOnError: false }) + setHasConsented(true, { notifyOnError: false }) setIsVisible(false) } catch { setSaveError('Your cookie preferences could not be saved. Please try again.') diff --git a/src/components/ui/form/MarkdownToolbarButton.tsx b/src/components/ui/form/MarkdownToolbarButton.tsx index cbcf68f35..b3673dc43 100644 --- a/src/components/ui/form/MarkdownToolbarButton.tsx +++ b/src/components/ui/form/MarkdownToolbarButton.tsx @@ -16,6 +16,7 @@ export function MarkdownToolbarButton(props: Props) { type="button" onClick={props.onClick} title={props.title} + aria-label={props.title} className={cn( 'p-2 rounded-md transition-colors duration-150', 'text-gray-600 dark:text-gray-400', diff --git a/src/components/ui/form/SelectInput.test.tsx b/src/components/ui/form/SelectInput.test.tsx new file mode 100644 index 000000000..71c4ca25e --- /dev/null +++ b/src/components/ui/form/SelectInput.test.tsx @@ -0,0 +1,24 @@ +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { expect, it, vi } from 'vitest' +import { SelectInput } from './SelectInput' + +it('associates each generated label with its own select', async () => { + const user = userEvent.setup() + render( + <> + + + , + ) + const system = screen.getByRole('combobox', { name: 'System' }) + const emulator = screen.getByRole('combobox', { name: 'Emulator' }) + expect(system.id).not.toBe(emulator.id) + await user.click(screen.getByText('System', { selector: 'label' })) + expect(system).toHaveFocus() +}) + +it('names a select whose visible label is hidden', () => { + render() + expect(screen.getByRole('combobox', { name: 'Performance' })).toBeInTheDocument() +}) diff --git a/src/components/ui/form/SelectInput.tsx b/src/components/ui/form/SelectInput.tsx index 109b3ba1a..6eb20122d 100644 --- a/src/components/ui/form/SelectInput.tsx +++ b/src/components/ui/form/SelectInput.tsx @@ -1,4 +1,4 @@ -import { type ChangeEventHandler, type ReactNode } from 'react' +import { type ChangeEventHandler, type ReactNode, useId } from 'react' import { Input } from '@/components/ui' interface Option { @@ -18,15 +18,19 @@ interface Props { } export function SelectInput(props: Props) { + const generatedId = useId() + const id = props.id ?? generatedId + return (
{!props.hideLabel && ( -
@@ -128,7 +128,7 @@ export default function BookmarkBrowser(props: Props) { step={1} value={browser.memoryDrafts.memoryMax} onChange={(event) => browser.changeMemory('memoryMax', event.target.value)} - aria-invalid={browser.memoryErrors.memoryMax || undefined} + aria-invalid={Boolean(browser.memoryErrors.memoryMax) || undefined} aria-describedby={ browser.memoryErrors.memoryMax ? `${maxMemoryId}-error` : undefined } @@ -138,7 +138,7 @@ export default function BookmarkBrowser(props: Props) { id={`${maxMemoryId}-error`} className="mt-1 text-sm text-red-600 dark:text-red-400" > - Enter a positive whole number. + {browser.memoryErrors.memoryMax}

)} diff --git a/src/features/bookmarks/client/components/BookmarkButton.test.tsx b/src/features/bookmarks/client/components/BookmarkButton.test.tsx index 409308296..05e40cfa1 100644 --- a/src/features/bookmarks/client/components/BookmarkButton.test.tsx +++ b/src/features/bookmarks/client/components/BookmarkButton.test.tsx @@ -86,9 +86,18 @@ it('keeps the unavailable status explanation keyboard-accessible without permitt render() const button = screen.getByRole('button', { name: 'Bookmark status unavailable' }) expect(button).toHaveAttribute('aria-disabled', 'true') + expect(button).not.toHaveAttribute('aria-pressed') act(() => button.focus()) expect(button).toHaveFocus() expect(await screen.findByRole('tooltip')).toHaveTextContent('Bookmark status unavailable') fireEvent.click(button) expect(state.handleToggle).not.toHaveBeenCalled() }) + +it('does not announce a bookmark state before status resolution', () => { + state.isStatusPending = true + render() + const button = screen.getByRole('button') + expect(button).toBeDisabled() + expect(button).not.toHaveAttribute('aria-pressed') +}) diff --git a/src/features/bookmarks/client/components/BookmarkButton.tsx b/src/features/bookmarks/client/components/BookmarkButton.tsx index 0856a6560..9326d331d 100644 --- a/src/features/bookmarks/client/components/BookmarkButton.tsx +++ b/src/features/bookmarks/client/components/BookmarkButton.tsx @@ -37,7 +37,7 @@ function BookmarkButton(props: Props) { disabled={disabled && !isStatusError} aria-disabled={disabled} whileTap={disabled ? undefined : { scale: 0.85 }} - aria-pressed={isBookmarked} + aria-pressed={isStatusPending || isStatusError ? undefined : isBookmarked} aria-label={actionLabel} className={cn( 'relative flex items-center justify-center w-9 h-9 rounded-full transition-colors duration-200 outline-none', diff --git a/src/features/bookmarks/client/components/BookmarkGrid.test.tsx b/src/features/bookmarks/client/components/BookmarkGrid.test.tsx index 18f2d3e87..c1480fa08 100644 --- a/src/features/bookmarks/client/components/BookmarkGrid.test.tsx +++ b/src/features/bookmarks/client/components/BookmarkGrid.test.tsx @@ -1,6 +1,8 @@ -import { render, screen, waitFor } from '@testing-library/react' +import { keepPreviousData, QueryClient, QueryClientProvider, useQuery } from '@tanstack/react-query' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { useState } from 'react' import { beforeEach, expect, it, vi } from 'vitest' -import { type RouterOutput } from '@/types/trpc' +import { type RouterInput, type RouterOutput } from '@/types/trpc' import { ApprovalStatus } from '@orm' import BookmarkGrid from './BookmarkGrid' import { BookmarkFiltersSchema } from '../../shared/bookmark-filters.schemas' @@ -108,9 +110,9 @@ it('waits for the current page result before correcting pagination', async () => await waitFor(() => expect(onPageChange).toHaveBeenCalledWith(1)) }) -it('renders PC bookmarks from the PC endpoint and report shape', () => { +function pcBookmarkResponse() { const id = '00000000-0000-4000-a000-000000000002' - const data = { + return { ...pcResponse, pagination: { ...response.pagination, page: 1, pages: 1, offset: 0 }, items: [ @@ -137,6 +139,11 @@ it('renders PC bookmarks from the PC endpoint and report shape', () => { }, ], } satisfies RouterOutput['bookmarks']['getPcListingBookmarks'] +} + +it('renders PC bookmarks from the PC endpoint and report shape', () => { + const id = '00000000-0000-4000-a000-000000000002' + const data = pcBookmarkResponse() mocks.pc.mockReturnValue({ ...queryResult(pcResponse), data }) render( { expect.objectContaining({ enabled: false }), ) }) + +it.each([2, 3])( + 'keeps current navigation after a real page-query error from page %s', + async (requestedPage) => { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false, staleTime: Infinity } }, + }) + const data = pcBookmarkResponse() + const fetchPage = vi.fn(async (page: number | undefined) => { + if (page === 2) throw new Error('Page unavailable') + if (page === 3) + return { + ...data, + items: [], + pagination: { ...data.pagination, page: 3, total: 2, pages: 2 }, + } + return { ...data, pagination: { ...data.pagination, total: 3, pages: 3 } } + }) + mocks.pc.mockImplementation(function usePcQuery( + input: RouterInput['bookmarks']['getPcListingBookmarks'], + ) { + return useQuery({ + queryKey: ['pc-bookmarks', input], + queryFn: () => fetchPage(input.page), + placeholderData: keepPreviousData, + }) + }) + function Grid() { + const [page, setPage] = useState(1) + return ( + + ) + } + const view = render( + + + , + ) + try { + expect(await screen.findByRole('link', { name: /PC bookmark game/ })).toBeInTheDocument() + fireEvent.click(screen.getAllByRole('button', { name: `Go to page ${requestedPage}` })[0]) + expect(await screen.findByText('Something went wrong')).toBeInTheDocument() + expect(screen.queryByRole('link', { name: /PC bookmark game/ })).not.toBeInTheDocument() + if (requestedPage === 3) + expect(screen.queryByRole('button', { name: 'Go to page 3' })).not.toBeInTheDocument() + fireEvent.click(screen.getAllByRole('button', { name: 'Go to previous page' })[0]) + expect(await screen.findByRole('link', { name: /PC bookmark game/ })).toBeInTheDocument() + expect(fetchPage).toHaveBeenCalledWith(2) + } finally { + view.unmount() + client.clear() + } + }, +) + +it.each(['owner', 'variant', 'search', 'filters', 'limit', 'hidden'] as const)( + 'does not reuse pagination metadata after %s changes', + (change) => { + const data = pcBookmarkResponse() + data.pagination.pages = 3 + data.pagination.total = 3 + mocks.pc.mockReturnValue({ ...queryResult(pcResponse), data }) + const props = { + variant: 'pc' as const, + userId: '00000000-0000-4000-a000-000000000001', + page: 1, + limit: 1, + search: '', + filters: BookmarkFiltersSchema.parse({}), + hasFilters: false, + onPageChange: vi.fn(), + emptyContent: null, + } + const view = render() + fireEvent.click(screen.getAllByRole('button', { name: 'Go to page 2' })[0]) + const error = { + data: undefined, + isPending: false, + isError: true, + isFetching: false, + isPlaceholderData: false, + error: new Error('Page unavailable'), + refetch: vi.fn(), + } + mocks.pc.mockReturnValue(error) + mocks.handheld.mockReturnValue(error) + view.rerender() + expect(screen.getByRole('navigation', { name: 'Pagination navigation' })).toBeInTheDocument() + const changes = { + owner: { userId: '00000000-0000-4000-a000-000000000002' }, + variant: { variant: 'handheld' as const }, + search: { search: 'Another game' }, + filters: { filters: BookmarkFiltersSchema.parse({ memoryMin: 16 }) }, + limit: { limit: 20 }, + hidden: {}, + } + if (change === 'hidden') mocks.pc.mockReturnValue({ ...error, data: { visibility: 'hidden' } }) + view.rerender() + expect( + screen.queryByRole('navigation', { name: 'Pagination navigation' }), + ).not.toBeInTheDocument() + }, +) diff --git a/src/features/bookmarks/client/components/BookmarkGrid.tsx b/src/features/bookmarks/client/components/BookmarkGrid.tsx index 6418919a6..97ef0d3a6 100644 --- a/src/features/bookmarks/client/components/BookmarkGrid.tsx +++ b/src/features/bookmarks/client/components/BookmarkGrid.tsx @@ -1,10 +1,11 @@ 'use client' import { keepPreviousData } from '@tanstack/react-query' -import { useEffect, type ReactNode } from 'react' +import { useEffect, useState, type ReactNode } from 'react' import { type z } from 'zod' import { ErrorFallback, LoadingSpinner, Pagination } from '@/components/ui' import { api } from '@/lib/api' +import { type RouterOutput } from '@/types/trpc' import BookmarkCard from './BookmarkCard' import SearchEmptyState from './SearchEmptyState' import { type BookmarkFiltersSchema } from '../../shared/bookmark-filters.schemas' @@ -22,6 +23,11 @@ interface Props { emptyContent: ReactNode } +type BookmarkPagination = Extract< + RouterOutput['bookmarks']['getListingBookmarks'], + { visibility: 'visible' } +>['pagination'] + function BookmarkGrid(props: Props) { const listingQuery = api.bookmarks.getListingBookmarks.useQuery( { @@ -46,6 +52,42 @@ function BookmarkGrid(props: Props) { ) const query = props.variant === 'handheld' ? listingQuery : pcQuery + const paginationScope = JSON.stringify([ + props.userId, + props.variant, + props.limit, + props.search, + props.filters, + ]) + const [lastPagination, setLastPagination] = useState<{ + scope: string + pagination: BookmarkPagination + } | null>(null) + if ( + query.data?.visibility === 'visible' && + !query.isPlaceholderData && + (lastPagination?.scope !== paginationScope || + lastPagination.pagination !== query.data.pagination) + ) { + setLastPagination({ scope: paginationScope, pagination: query.data.pagination }) + } else if (query.data?.visibility === 'hidden' && lastPagination !== null) { + setLastPagination(null) + } + const pagination = + query.data?.visibility === 'visible' + ? query.data.pagination + : query.data?.visibility !== 'hidden' && lastPagination?.scope === paginationScope + ? lastPagination.pagination + : null + const paginationControls = pagination && pagination.pages > 1 && ( + + ) const ids = props.variant === 'handheld' @@ -75,12 +117,15 @@ function BookmarkGrid(props: Props) { if (query.isError) { return ( - { - void query.refetch() - }} - /> + <> + { + void query.refetch() + }} + /> + {paginationControls} + ) } @@ -132,15 +177,7 @@ function BookmarkGrid(props: Props) { return ( <>
{renderItems()}
- {query.data.pagination.pages > 1 && ( - - )} + {paginationControls} ) } diff --git a/src/features/bookmarks/client/hooks/useBookmarkBrowser.ts b/src/features/bookmarks/client/hooks/useBookmarkBrowser.ts index 8ef9d228e..e9cc4c5c0 100644 --- a/src/features/bookmarks/client/hooks/useBookmarkBrowser.ts +++ b/src/features/bookmarks/client/hooks/useBookmarkBrowser.ts @@ -1,16 +1,26 @@ 'use client' import { useState } from 'react' -import { type z } from 'zod' +import { z } from 'zod' import { UI_CONSTANTS } from '@/data/constants' import useDebouncedValue from '@/hooks/useDebouncedValue' -import { BookmarkFiltersSchema } from '../../shared/bookmark-filters.schemas' +import { + BookmarkFiltersSchema, + refineBookmarkMemoryRange, +} from '../../shared/bookmark-filters.schemas' const initialFilters = BookmarkFiltersSchema.parse({}) const initialMemoryDrafts = { memoryMin: '', memoryMax: '' } +const memoryDraftSchema = BookmarkFiltersSchema.pick({ + memoryMin: true, + memoryMax: true, +}).superRefine(refineBookmarkMemoryRange) -function parseMemoryDraft(field: keyof typeof initialMemoryDrafts, value: string) { - return BookmarkFiltersSchema.shape[field].safeParse(value.trim() ? Number(value) : null) +function parseMemoryDrafts(drafts: typeof initialMemoryDrafts) { + return memoryDraftSchema.safeParse({ + memoryMin: drafts.memoryMin.trim() ? Number(drafts.memoryMin) : null, + memoryMax: drafts.memoryMax.trim() ? Number(drafts.memoryMax) : null, + }) } export function useBookmarkBrowser() { @@ -56,16 +66,36 @@ export function useBookmarkBrowser() { } function changeMemory(field: keyof typeof initialMemoryDrafts, value: string) { - setMemoryDrafts((current) => ({ ...current, [field]: value })) - const parsed = parseMemoryDraft(field, value) - if (!parsed.success) return - setFilters((current) => ({ ...current, [field]: parsed.data })) + const nextDrafts = { ...memoryDrafts, [field]: value } + setMemoryDrafts(nextDrafts) + const fieldValue = BookmarkFiltersSchema.shape[field].safeParse( + value.trim() ? Number(value) : null, + ) + if (!fieldValue.success) return + const otherField = field === 'memoryMin' ? 'memoryMax' : 'memoryMin' + const otherDraft = nextDrafts[otherField].trim() + const otherValue = BookmarkFiltersSchema.shape[otherField].safeParse( + otherDraft ? Number(otherDraft) : null, + ) + const applied = otherValue.success + ? parseMemoryDrafts(nextDrafts) + : memoryDraftSchema.safeParse({ ...filters, [field]: fieldValue.data }) + if (!applied.success) return + setFilters((current) => ({ ...current, ...applied.data })) setPage(1) } + const memoryValidation = parseMemoryDrafts(memoryDrafts) + function memoryError(field: keyof typeof initialMemoryDrafts) { + if (memoryValidation.success) return undefined + const issue = memoryValidation.error.issues.find((issue) => issue.path[0] === field) + if (!issue) return undefined + return issue.code === z.ZodIssueCode.custom ? issue.message : 'Enter a positive whole number.' + } + const memoryErrors = { - memoryMin: !parseMemoryDraft('memoryMin', memoryDrafts.memoryMin).success, - memoryMax: !parseMemoryDraft('memoryMax', memoryDrafts.memoryMax).success, + memoryMin: memoryError('memoryMin'), + memoryMax: memoryError('memoryMax'), } const hasFilters = Boolean( diff --git a/src/features/bookmarks/server/bookmark.repository.test.ts b/src/features/bookmarks/server/bookmark.repository.test.ts index a30fce435..decfa26ff 100644 --- a/src/features/bookmarks/server/bookmark.repository.test.ts +++ b/src/features/bookmarks/server/bookmark.repository.test.ts @@ -27,6 +27,24 @@ beforeEach(() => { mocks.pcListingBookmark.count.mockResolvedValue(13) }) +it('counts only approved reports with approved games for the visible profile tabs', async () => { + await new BookmarkRepository(prisma).counts(userId, owner) + expect(mocks.listingBookmark.count).toHaveBeenCalledWith({ + where: { userId, listing: { status: 'APPROVED', game: { status: 'APPROVED' } } }, + }) + expect(mocks.pcListingBookmark.count).toHaveBeenCalledWith({ + where: { userId, pcListing: { status: 'APPROVED', game: { status: 'APPROVED' } } }, + }) +}) + +it('hides bookmark counts before querying either saved-report total', async () => { + await expect( + new BookmarkRepository(prisma).counts(userId, { requestingUserId: 'another-user' }), + ).resolves.toEqual({ visibility: 'hidden' }) + expect(mocks.listingBookmark.count).not.toHaveBeenCalled() + expect(mocks.pcListingBookmark.count).not.toHaveBeenCalled() +}) + describe.each(['handheld', 'pc'] as const)('%s bookmark browsing', (variant) => { function list(input: Parameters[0], context = owner) { const repository = new BookmarkRepository(prisma) @@ -61,7 +79,11 @@ describe.each(['handheld', 'pc'] as const)('%s bookmark browsing', (variant) => userId, [relation]: { status: 'APPROVED', - game: { title: { contains: 'Mario', mode: 'insensitive' }, systemId: { in: [entityId] } }, + game: { + status: 'APPROVED', + title: { contains: 'Mario', mode: 'insensitive' }, + systemId: { in: [entityId] }, + }, emulatorId: { in: [entityId] }, performanceId: { in: [1] }, ...(variant === 'handheld' @@ -85,6 +107,15 @@ describe.each(['handheld', 'pc'] as const)('%s bookmark browsing', (variant) => expect(result).toMatchObject({ visibility: 'visible', pagination: { total: 13, pages: 2 } }) }) + it('requires approved games even without title or system filters', async () => { + await list({ userId }) + const where = expect.objectContaining({ + [relation]: expect.objectContaining({ game: { status: 'APPROVED' } }), + }) + expect(model().findMany).toHaveBeenCalledWith(expect.objectContaining({ where })) + expect(model().count).toHaveBeenCalledWith({ where }) + }) + it.each([ ['savedAt', { createdAt: 'desc' }], ['performance', { [relation]: { performance: { rank: 'desc' } } }], diff --git a/src/features/bookmarks/server/bookmark.repository.ts b/src/features/bookmarks/server/bookmark.repository.ts index 9fccf91aa..be551a395 100644 --- a/src/features/bookmarks/server/bookmark.repository.ts +++ b/src/features/bookmarks/server/bookmark.repository.ts @@ -213,10 +213,19 @@ export class BookmarkRepository extends BaseRepository { const [listingBookmarks, pcListingBookmarks] = await Promise.all([ this.prisma.listingBookmark.count({ - where: { userId, listing: { status: ApprovalStatus.APPROVED } }, + where: { + userId, + listing: { status: ApprovalStatus.APPROVED, game: { status: ApprovalStatus.APPROVED } }, + }, }), this.prisma.pcListingBookmark.count({ - where: { userId, pcListing: { status: ApprovalStatus.APPROVED } }, + where: { + userId, + pcListing: { + status: ApprovalStatus.APPROVED, + game: { status: ApprovalStatus.APPROVED }, + }, + }, }), ]) diff --git a/src/features/bookmarks/server/bookmarks.router.test.ts b/src/features/bookmarks/server/bookmarks.router.test.ts index 8c959e43c..ca633e553 100644 --- a/src/features/bookmarks/server/bookmarks.router.test.ts +++ b/src/features/bookmarks/server/bookmarks.router.test.ts @@ -97,6 +97,18 @@ describe.each(['handheld', 'pc'] as const)('%s bookmark transport', (variant) => expect(mocks.pcListingBookmark.findMany).not.toHaveBeenCalled() }) + it('rejects inverted memory bounds before any database access', async () => { + const query = + variant === 'handheld' ? caller().getListingBookmarks : caller().getPcListingBookmarks + await expect(query({ userId, memoryMin: 32, memoryMax: 16 })).rejects.toMatchObject({ + code: 'BAD_REQUEST', + cause: { issues: expect.arrayContaining([expect.objectContaining({ path: ['memoryMax'] })]) }, + }) + expect(mocks.user.findUnique).not.toHaveBeenCalled() + expect(mocks.listingBookmark.findMany).not.toHaveBeenCalled() + expect(mocks.pcListingBookmark.findMany).not.toHaveBeenCalled() + }) + it('uses the session owner for writes even when input includes another user', async () => { if (variant === 'handheld') { const input = { listingId: reportId, userId: reportId } diff --git a/src/features/bookmarks/server/persistence/bookmark-queries.ts b/src/features/bookmarks/server/persistence/bookmark-queries.ts index f733d08e9..f27fea1a0 100644 --- a/src/features/bookmarks/server/persistence/bookmark-queries.ts +++ b/src/features/bookmarks/server/persistence/bookmark-queries.ts @@ -7,6 +7,7 @@ import { ApprovalStatus, Prisma } from '@orm' function reportFilters(input: z.output) { const game: Prisma.GameWhereInput = { + status: ApprovalStatus.APPROVED, ...(input.search ? { title: { contains: input.search, mode: Prisma.QueryMode.insensitive } } : {}), @@ -14,7 +15,7 @@ function reportFilters(input: z.output) { } return { status: ApprovalStatus.APPROVED, - game: input.search || input.systemIds.length ? game : undefined, + game, emulatorId: input.emulatorIds.length ? { in: input.emulatorIds } : undefined, performanceId: input.performanceIds.length ? { in: input.performanceIds } : undefined, } diff --git a/src/features/bookmarks/shared/bookmark-filters.schemas.test.ts b/src/features/bookmarks/shared/bookmark-filters.schemas.test.ts index ef856ce2d..fd5cebaf4 100644 --- a/src/features/bookmarks/shared/bookmark-filters.schemas.test.ts +++ b/src/features/bookmarks/shared/bookmark-filters.schemas.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'vitest' -import { GetListingBookmarksSchema } from '@/features/bookmarks/shared/bookmark.schemas' +import { + GetListingBookmarksSchema, + GetPcListingBookmarksSchema, +} from '@/features/bookmarks/shared/bookmark.schemas' describe('bookmark filter input', () => { const userId = '00000000-0000-4000-a000-000000000001' @@ -20,4 +23,23 @@ describe('bookmark filter input', () => { ])('rejects invalid filters: %j', (filters) => { expect(GetListingBookmarksSchema.safeParse({ userId, ...filters }).success).toBe(false) }) + it.each([GetListingBookmarksSchema, GetPcListingBookmarksSchema])( + 'rejects inverted memory ranges at the list boundary', + (schema) => { + const result = schema.safeParse({ userId, memoryMin: 32, memoryMax: 16 }) + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error.issues).toEqual( + expect.arrayContaining([expect.objectContaining({ path: ['memoryMax'] })]), + ) + } + for (const range of [ + { memoryMin: 16, memoryMax: 16 }, + { memoryMin: null, memoryMax: 16 }, + { memoryMin: 32, memoryMax: null }, + ]) { + expect(schema.safeParse({ userId, ...range }).success).toBe(true) + } + }, + ) }) diff --git a/src/features/bookmarks/shared/bookmark-filters.schemas.ts b/src/features/bookmarks/shared/bookmark-filters.schemas.ts index 4765dda3c..65c8372ec 100644 --- a/src/features/bookmarks/shared/bookmark-filters.schemas.ts +++ b/src/features/bookmarks/shared/bookmark-filters.schemas.ts @@ -16,3 +16,16 @@ export const BookmarkFiltersSchema = z.object({ sortField: BookmarkSortFieldSchema.default('savedAt'), sortDirection: SortDirectionSchema.default('desc'), }) + +export function refineBookmarkMemoryRange( + input: Pick, 'memoryMin' | 'memoryMax'>, + context: z.RefinementCtx, +) { + if (input.memoryMin !== null && input.memoryMax !== null && input.memoryMin > input.memoryMax) { + context.addIssue({ + code: z.ZodIssueCode.custom, + path: ['memoryMax'], + message: 'Maximum memory must be at least minimum memory.', + }) + } +} diff --git a/src/features/bookmarks/shared/bookmark.schemas.ts b/src/features/bookmarks/shared/bookmark.schemas.ts index 6903a76e2..435252040 100644 --- a/src/features/bookmarks/shared/bookmark.schemas.ts +++ b/src/features/bookmarks/shared/bookmark.schemas.ts @@ -1,6 +1,9 @@ import { z } from 'zod' import { PAGINATION } from '@/data/constants' -import { BookmarkFiltersSchema } from '@/features/bookmarks/shared/bookmark-filters.schemas' +import { + BookmarkFiltersSchema, + refineBookmarkMemoryRange, +} from '@/features/bookmarks/shared/bookmark-filters.schemas' import { refinePaginationOffset } from '@/schemas/pagination' const listingIdSchema = z.object({ @@ -19,6 +22,7 @@ const bookmarkListSchema = z search: z.string().max(100).optional(), }) .merge(BookmarkFiltersSchema) + .superRefine(refineBookmarkMemoryRange) .superRefine(refinePaginationOffset) export const BookmarkListingSchema = listingIdSchema diff --git a/src/features/custom-fields/driver/client/components/DriverVersionField.test.tsx b/src/features/custom-fields/driver/client/components/DriverVersionField.test.tsx index e0a5f9d57..5388db64e 100644 --- a/src/features/custom-fields/driver/client/components/DriverVersionField.test.tsx +++ b/src/features/custom-fields/driver/client/components/DriverVersionField.test.tsx @@ -8,8 +8,10 @@ import DriverVersionField from './DriverVersionField' const mocks = vi.hoisted(() => ({ submit: vi.fn() })) let releases: DriverRelease[] | null = [] let loading = false +let rateLimited = false +let errorMessage: string | null = null vi.mock('../hooks/useDriverVersions', () => ({ - useDriverVersions: () => ({ data: releases, loading, rateLimited: false, errorMessage: null }), + useDriverVersions: () => ({ data: releases, loading, rateLimited, errorMessage }), })) const knownRelease: DriverRelease = { @@ -25,6 +27,7 @@ const knownRelease: DriverRelease = { interface Props { initialDriver?: string required?: boolean + placeholder?: string } function Form(props: Props) { @@ -40,6 +43,7 @@ function Form(props: Props) { name: 'dynamic_driver_version', label: 'Graphics Driver', isRequired: props.required ?? true, + placeholder: props.placeholder, }} rules={props.required === false ? {} : { required: 'Graphics Driver is required' }} errorMessage={form.formState.errors.driver?.message} @@ -55,6 +59,23 @@ beforeEach(() => { vi.clearAllMocks() releases = [] loading = false + rateLimited = false + errorMessage = null +}) + +it('keeps configured placeholder text in custom entry mode', async () => { + const user = userEvent.setup() + render(
) + await user.click(screen.getByRole('button', { name: 'Enter a custom driver' })) + expect(screen.getByPlaceholderText('Driver name and version')).toHaveFocus() +}) + +it('shows actionable custom-driver guidance when the server reports a rate limit', () => { + rateLimited = true + errorMessage = 'GitHub API rate limit exceeded' + render() + expect(screen.getByText(/Select a default option or enter a custom driver/)).toBeInTheDocument() + expect(screen.queryByText(errorMessage, { exact: true })).not.toBeInTheDocument() }) it('submits a custom graphics-driver name instead of a fixed dropdown option', async () => { diff --git a/src/features/custom-fields/driver/client/components/DriverVersionField.tsx b/src/features/custom-fields/driver/client/components/DriverVersionField.tsx index 7efc1cc8d..91062b280 100644 --- a/src/features/custom-fields/driver/client/components/DriverVersionField.tsx +++ b/src/features/custom-fields/driver/client/components/DriverVersionField.tsx @@ -25,11 +25,9 @@ export default function DriverVersionField( props: Props, ) { const driverVersions = useDriverVersions() - const helperMessage = - driverVersions.errorMessage ?? - (driverVersions.rateLimited - ? 'GitHub rate limit exceeded. Select a default option or enter a custom driver.' - : null) + const helperMessage = driverVersions.rateLimited + ? 'GitHub rate limit exceeded. Select a default option or enter a custom driver.' + : driverVersions.errorMessage return (
diff --git a/src/features/custom-fields/driver/client/components/DriverVersionInput.tsx b/src/features/custom-fields/driver/client/components/DriverVersionInput.tsx index c8c87e28e..8df1c451e 100644 --- a/src/features/custom-fields/driver/client/components/DriverVersionInput.tsx +++ b/src/features/custom-fields/driver/client/components/DriverVersionInput.tsx @@ -54,7 +54,7 @@ export function DriverVersionInput(props: Props) { }} onBlur={input.onBlur} leftIcon={input.leftIcon} - placeholder="Enter your graphics driver name and version" + placeholder={input.placeholder} className={input.className} aria-required={input.required} aria-describedby={input.errorId} diff --git a/src/features/emulators/custom-field-config/server/config.mapper.ts b/src/features/emulators/custom-field-config/server/config.mapper.ts index 5187b7d03..3cd1dfb3f 100644 --- a/src/features/emulators/custom-field-config/server/config.mapper.ts +++ b/src/features/emulators/custom-field-config/server/config.mapper.ts @@ -1,4 +1,5 @@ import { createHash } from 'node:crypto' +import { AppError } from '@/lib/errors' import { CustomFieldType } from '@orm' import { type ConfigEmulatorRecord } from './persistence/config.prisma' import { @@ -6,6 +7,7 @@ import { supportsPlaceholder, } from '../../shared/custom-field-attributes.schemas' import { + ConfigCategorySchema, ConfigFieldSchema, CustomFieldConfigSchema, type CustomFieldConfigSnapshot, @@ -20,7 +22,7 @@ export function toConfigField(field: ConfigEmulatorRecord['customFieldDefinition defaultValue = defaultValue === 'true' } if (field.type === CustomFieldType.RANGE) defaultValue = normalizeRangeDefault(defaultValue) - return ConfigFieldSchema.parse({ + const result = ConfigFieldSchema.safeParse({ id: field.id, name: field.name, label: field.label, @@ -37,21 +39,33 @@ export function toConfigField(field: ConfigEmulatorRecord['customFieldDefinition displayOrder: field.displayOrder, categoryOrder: field.categoryOrder, }) + if (!result.success) + return AppError.badRequest( + `Field "${field.name}" has invalid settings. Correct it in the field editor before editing its JSON configuration.`, + ) + return result.data } export function toConfigSnapshot(emulator: ConfigEmulatorRecord): CustomFieldConfigSnapshot { - const configuration = CustomFieldConfigSchema.parse({ + const result = CustomFieldConfigSchema.safeParse({ version: 1, - categories: emulator.customFieldCategories.map((category) => ({ - id: category.id, - name: category.name, - displayOrder: category.displayOrder, - })), + categories: emulator.customFieldCategories.map((category) => { + const result = ConfigCategorySchema.safeParse(category) + if (!result.success) + return AppError.badRequest( + `Category "${category.name}" has invalid settings. Correct it in the category editor before editing its JSON configuration.`, + ) + return result.data + }), fields: emulator.customFieldDefinitions.map(toConfigField), }) + if (!result.success) + return AppError.badRequest( + `Emulator "${emulator.id}" has invalid field or category settings. Correct them before editing its JSON configuration.`, + ) return { - configuration, - revision: createHash('sha256').update(JSON.stringify(configuration)).digest('hex'), + configuration: result.data, + revision: createHash('sha256').update(JSON.stringify(result.data)).digest('hex'), } } diff --git a/src/features/emulators/custom-field-config/server/config.repository.test.ts b/src/features/emulators/custom-field-config/server/config.repository.test.ts index e0101f4f5..fa8d8bc12 100644 --- a/src/features/emulators/custom-field-config/server/config.repository.test.ts +++ b/src/features/emulators/custom-field-config/server/config.repository.test.ts @@ -29,6 +29,18 @@ describe('configuration persistence', () => { }) }) + it('preserves an unexpected database failure as the configuration error cause', async () => { + const cause = new Error('Connection closed during commit') + database.$transaction.mockRejectedValue(cause) + const repository = new CustomFieldConfigRepository(prisma) + + await expect(repository.transaction(async () => undefined)).rejects.toMatchObject({ + code: 'INTERNAL_SERVER_ERROR', + message: 'Database error during apply custom field configuration', + cause, + }) + }) + it('preserves field identity and false values, and clears nullable settings explicitly', async () => { const repository = new CustomFieldConfigRepository(prisma) const field = ConfigFieldSchema.parse({ diff --git a/src/features/emulators/custom-field-config/server/config.repository.ts b/src/features/emulators/custom-field-config/server/config.repository.ts index d5e60fcd8..dfb2e5002 100644 --- a/src/features/emulators/custom-field-config/server/config.repository.ts +++ b/src/features/emulators/custom-field-config/server/config.repository.ts @@ -45,7 +45,10 @@ export class CustomFieldConfigRepository extends PrismaRepository { if (isPrismaError(error, PRISMA_ERROR_CODES.FOREIGN_KEY_CONSTRAINT_VIOLATION)) { return ResourceError.emulator.notFound() } - return AppError.databaseError('apply custom field configuration') + return AppError.databaseError( + 'apply custom field configuration', + error instanceof Error ? error : undefined, + ) } } diff --git a/src/features/emulators/custom-field-config/server/config.router.test.ts b/src/features/emulators/custom-field-config/server/config.router.test.ts index 64d6a3150..9fec47a15 100644 --- a/src/features/emulators/custom-field-config/server/config.router.test.ts +++ b/src/features/emulators/custom-field-config/server/config.router.test.ts @@ -3,6 +3,7 @@ import { prisma } from '@/server/db' import { PERMISSIONS } from '@/utils/permission-system' import { CustomFieldType, Role } from '@orm' import { toConfigSnapshot } from './config.mapper' +import { CustomFieldConfigRepository } from './config.repository' import { CustomFieldConfigService } from './config.service' import { configRecord, TARGET_ID } from '../tests/fixtures' @@ -38,6 +39,63 @@ describe('configuration transport', () => { await expect(caller().get({ emulatorId: TARGET_ID })).resolves.toEqual(snapshot) }) + it.each(['get', 'save'] as const)( + 'names invalid stored fields in a BAD_REQUEST from %s', + async (operation) => { + const record = configRecord() + const snapshot = toConfigSnapshot(record) + const field = record.customFieldDefinitions[0] + if (!field) throw new Error('Missing custom field fixture') + field.defaultValue = 'removed-option' + vi.spyOn(CustomFieldConfigRepository.prototype, 'getEmulator').mockResolvedValue(record) + const saveField = vi.spyOn(CustomFieldConfigRepository.prototype, 'saveField') + const saveCategory = vi.spyOn(CustomFieldConfigRepository.prototype, 'saveCategory') + const request = + operation === 'get' + ? caller().get({ emulatorId: TARGET_ID }) + : caller().save({ + emulatorId: TARGET_ID, + expectedRevision: snapshot.revision, + configuration: snapshot.configuration, + }) + + await expect(request).rejects.toMatchObject({ + code: 'BAD_REQUEST', + message: expect.stringContaining(field.name), + }) + expect(saveField).not.toHaveBeenCalled() + expect(saveCategory).not.toHaveBeenCalled() + }, + ) + + it('names invalid stored categories instead of returning an internal error', async () => { + const record = configRecord() + const category = record.customFieldCategories[0] + if (!category) throw new Error('Missing custom field category fixture') + category.name = 'x'.repeat(101) + vi.spyOn(CustomFieldConfigRepository.prototype, 'getEmulator').mockResolvedValue(record) + + await expect(caller().get({ emulatorId: TARGET_ID })).rejects.toMatchObject({ + code: 'BAD_REQUEST', + message: expect.stringContaining(category.name), + }) + }) + + it('identifies the emulator when stored configuration violates cross-field rules', async () => { + const record = configRecord() + record.customFieldCategories.push({ + id: '00000000-0000-4000-a000-000000000099', + name: 'Graphics', + displayOrder: 2, + }) + vi.spyOn(CustomFieldConfigRepository.prototype, 'getEmulator').mockResolvedValue(record) + + await expect(caller().get({ emulatorId: TARGET_ID })).rejects.toMatchObject({ + code: 'BAD_REQUEST', + message: expect.stringContaining(TARGET_ID), + }) + }) + it('requires the management permission', async () => { const service = vi.spyOn(CustomFieldConfigService.prototype, 'get') await expect(caller([]).get({ emulatorId: TARGET_ID })).rejects.toMatchObject({ diff --git a/src/features/emulators/custom-field-config/server/config.service.test.ts b/src/features/emulators/custom-field-config/server/config.service.test.ts index bf0f4fb24..317740479 100644 --- a/src/features/emulators/custom-field-config/server/config.service.test.ts +++ b/src/features/emulators/custom-field-config/server/config.service.test.ts @@ -69,6 +69,35 @@ describe('custom field configuration service', () => { expect(result.configuration.fields[0]?.id).toBe(FIELD_ID) }) + it('exports and saves signed field and category orders without changing their identity', async () => { + const s = setup() + const category = s.record.customFieldCategories[0] + const field = s.record.customFieldDefinitions[0] + if (!category || !field) throw new Error('Missing custom field fixture') + category.displayOrder = -2_147_483_648 + field.displayOrder = -2 + field.categoryOrder = -1 + const snapshot = await s.service.get(actor, TARGET_ID) + + await s.service.save(actor, { + emulatorId: TARGET_ID, + configuration: snapshot.configuration, + expectedRevision: snapshot.revision, + }) + + expect(s.saveCategory).toHaveBeenCalledWith( + TARGET_ID, + expect.objectContaining({ id: CATEGORY_ID, displayOrder: -2_147_483_648 }), + CATEGORY_ID, + ) + expect(s.saveField).toHaveBeenCalledWith( + TARGET_ID, + expect.objectContaining({ id: FIELD_ID, displayOrder: -2, categoryOrder: -1 }), + CATEGORY_ID, + FIELD_ID, + ) + }) + it('maps pasted categories to destination IDs and removes omitted unused fields', async () => { const s = setup() s.input.configuration.categories = [ @@ -298,8 +327,8 @@ describe('custom field configuration service', () => { field.rangeMax = 4 field.defaultValue = value await expect(s.service.get(actor, TARGET_ID)).rejects.toMatchObject({ - name: 'ZodError', - issues: expect.arrayContaining([expect.objectContaining({ path: ['defaultValue'] })]), + code: 'BAD_REQUEST', + message: expect.stringContaining(field.name), }) expect(s.saveField).not.toHaveBeenCalled() }, diff --git a/src/features/emulators/custom-field-config/shared/config.schemas.test.ts b/src/features/emulators/custom-field-config/shared/config.schemas.test.ts index ca3aecb67..b2517514f 100644 --- a/src/features/emulators/custom-field-config/shared/config.schemas.test.ts +++ b/src/features/emulators/custom-field-config/shared/config.schemas.test.ts @@ -49,6 +49,32 @@ describe('custom field configuration JSON', () => { expect(result.fields[1]?.defaultValue).toBe(0) }) + it.each([-2_147_483_648, -1, 0, 2_147_483_647])( + 'accepts persisted signed ordering %i for fields and categories', + (order) => { + const result = CustomFieldConfigSchema.parse({ + version: 1, + categories: [{ name: 'Graphics', displayOrder: order }], + fields: [{ ...field, category: 'Graphics', displayOrder: order, categoryOrder: order }], + }) + expect(result.categories[0]?.displayOrder).toBe(order) + expect(result.fields[0]?.displayOrder).toBe(order) + expect(result.fields[0]?.categoryOrder).toBe(order) + }, + ) + + it.each([-2_147_483_649, 2_147_483_648, 0.5])('rejects unpersistable ordering %j', (order) => { + expect(ConfigFieldSchema.safeParse({ ...field, displayOrder: order }).success).toBe(false) + expect(ConfigFieldSchema.safeParse({ ...field, categoryOrder: order }).success).toBe(false) + expect( + CustomFieldConfigSchema.safeParse({ + version: 1, + categories: [{ name: 'Graphics', displayOrder: order }], + fields: [], + }).success, + ).toBe(false) + }) + it.each([ { ...field, id: 'not-a-uuid' }, { ...field, categoryId: 'foreign-category' }, diff --git a/src/features/emulators/custom-field-config/shared/config.schemas.ts b/src/features/emulators/custom-field-config/shared/config.schemas.ts index 51695f902..218e71c32 100644 --- a/src/features/emulators/custom-field-config/shared/config.schemas.ts +++ b/src/features/emulators/custom-field-config/shared/config.schemas.ts @@ -1,13 +1,21 @@ import { z } from 'zod' +import { SignedInt32Schema } from '@/schemas/common' import { CreateCustomFieldDefinitionSchema } from '@/schemas/customFieldDefinition' import { CustomFieldAttributesSchema, validateCustomFieldAttributes, } from '../../shared/custom-field-attributes.schemas' -const orderSchema = z.number().int().min(0).max(2_147_483_647) const categoryNameSchema = z.string().trim().min(1).max(100) +export const ConfigCategorySchema = z + .object({ + id: z.string().uuid().optional(), + name: categoryNameSchema, + displayOrder: SignedInt32Schema.default(0), + }) + .strict() + export const ConfigFieldSchema = CreateCustomFieldDefinitionSchema.omit({ emulatorId: true, categoryId: true, @@ -18,8 +26,8 @@ export const ConfigFieldSchema = CreateCustomFieldDefinitionSchema.omit({ name: CreateCustomFieldDefinitionSchema.shape.name.max(100), label: z.string().trim().min(1).max(200), category: categoryNameSchema.nullable().default(null), - displayOrder: orderSchema.default(0), - categoryOrder: orderSchema.default(0), + displayOrder: SignedInt32Schema.default(0), + categoryOrder: SignedInt32Schema.default(0), }) .strict() .superRefine(validateCustomFieldAttributes) @@ -27,17 +35,7 @@ export const ConfigFieldSchema = CreateCustomFieldDefinitionSchema.omit({ export const CustomFieldConfigSchema = z .object({ version: z.literal(1), - categories: z - .array( - z - .object({ - id: z.string().uuid().optional(), - name: categoryNameSchema, - displayOrder: orderSchema.default(0), - }) - .strict(), - ) - .max(100), + categories: z.array(ConfigCategorySchema).max(100), fields: z.array(ConfigFieldSchema).max(500), }) .strict() diff --git a/src/features/emulators/custom-field-templates/server/template.service.test.ts b/src/features/emulators/custom-field-templates/server/template.service.test.ts index c4476422e..04c88d28c 100644 --- a/src/features/emulators/custom-field-templates/server/template.service.test.ts +++ b/src/features/emulators/custom-field-templates/server/template.service.test.ts @@ -219,6 +219,61 @@ describe('template attribute persistence', () => { ) }) + it.each([ + { categoryOrder: 0, fieldOrder: 2_147_483_647, distinctCategories: false }, + { categoryOrder: 0, fieldOrder: 2_147_483_646, distinctCategories: false }, + { categoryOrder: 2_147_483_647, fieldOrder: 0, distinctCategories: false }, + { categoryOrder: 2_147_483_646, fieldOrder: 0, distinctCategories: true }, + ])('rejects ordering overflow before any writes %#', async (testCase) => { + transaction.emulator.findUnique.mockResolvedValue({ + id: EMULATOR_ID, + customFieldCategories: [ + { id: CATEGORY_ID, name: 'Existing category', displayOrder: testCase.categoryOrder }, + ], + customFieldDefinitions: [{ name: 'existing_field', displayOrder: testCase.fieldOrder }], + }) + if (testCase.distinctCategories) { + transaction.customFieldTemplate.findMany.mockResolvedValue([ + template([rangeField, { ...textField, categoryName: 'Driver settings' }]), + ]) + } + + await expect(service().apply(actor, applyInput)).rejects.toMatchObject({ code: 'BAD_REQUEST' }) + expect(transaction.customFieldCategory.create).not.toHaveBeenCalled() + expect(transaction.customFieldDefinition.create).not.toHaveBeenCalled() + }) + + it('uses the final field-order slot while reusing a category already at the maximum', async () => { + transaction.customFieldTemplate.findMany.mockResolvedValue([template([textField])]) + transaction.emulator.findUnique.mockResolvedValue({ + id: EMULATOR_ID, + customFieldCategories: [{ id: CATEGORY_ID, name: 'Graphics', displayOrder: 2_147_483_647 }], + customFieldDefinitions: [{ name: 'existing_field', displayOrder: 2_147_483_646 }], + }) + + await expect(service().apply(actor, applyInput)).resolves.toMatchObject({ createdFields: 1 }) + expect(transaction.customFieldCategory.create).not.toHaveBeenCalled() + expect(transaction.customFieldDefinition.create).toHaveBeenCalledWith( + expect.objectContaining({ data: expect.objectContaining({ displayOrder: 2_147_483_647 }) }), + ) + }) + + it('uses the final category-order slot once for fields sharing a new category', async () => { + transaction.emulator.findUnique.mockResolvedValue({ + id: EMULATOR_ID, + customFieldCategories: [ + { id: CATEGORY_ID, name: 'Existing category', displayOrder: 2_147_483_646 }, + ], + customFieldDefinitions: [], + }) + + await expect(service().apply(actor, applyInput)).resolves.toMatchObject({ createdFields: 2 }) + expect(transaction.customFieldCategory.create).toHaveBeenCalledOnce() + expect(transaction.customFieldCategory.create).toHaveBeenCalledWith( + expect.objectContaining({ data: expect.objectContaining({ displayOrder: 2_147_483_647 }) }), + ) + }) + it('preserves the explanatory no-op error when every field already exists', async () => { transaction.emulator.findUnique.mockResolvedValue({ id: EMULATOR_ID, diff --git a/src/features/emulators/custom-field-templates/server/template.service.ts b/src/features/emulators/custom-field-templates/server/template.service.ts index 39940b8d1..1c76eb713 100644 --- a/src/features/emulators/custom-field-templates/server/template.service.ts +++ b/src/features/emulators/custom-field-templates/server/template.service.ts @@ -1,6 +1,6 @@ import { type z } from 'zod' import { AppError, ResourceError } from '@/lib/errors' -import { createMutationSuccess } from '@/schemas/common' +import { createMutationSuccess, SignedInt32Schema } from '@/schemas/common' import { type Actor } from '@/server/auth/actor' import { parseStoredTemplateField } from './template.mapper' import { assertCanReadTemplates, assertCanWriteTemplates } from './template.policy' @@ -96,6 +96,22 @@ export class CustomFieldTemplateService { (max, field) => Math.max(max, field.displayOrder), -1, ) + 1 + const newCategoryCount = new Set( + fields.flatMap((field) => + field.categoryName && !categories.has(field.categoryName) ? [field.categoryName] : [], + ), + ).size + if (!SignedInt32Schema.safeParse(displayOrder + fields.length - 1).success) + return AppError.badRequest( + 'Custom field ordering has no remaining space. Reorder existing fields before applying templates.', + ) + if ( + newCategoryCount > 0 && + !SignedInt32Schema.safeParse(categoryOrder + newCategoryCount - 1).success + ) + return AppError.badRequest( + 'Category ordering has no remaining space. Reorder existing categories before applying templates.', + ) for (const field of fields) { if (field.categoryName && !categories.has(field.categoryName)) { const category = await repository.createCategory( diff --git a/src/features/reports/approval/server/report-admin-edit.service.test.ts b/src/features/reports/approval/server/report-admin-edit.service.test.ts index bf2bef326..042eccbe9 100644 --- a/src/features/reports/approval/server/report-admin-edit.service.test.ts +++ b/src/features/reports/approval/server/report-admin-edit.service.test.ts @@ -1,29 +1,37 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { invalidateListingSeo, invalidatePcListingSeo } from '@/server/cache/invalidation' +import { invalidateListingsSeo, invalidatePcListingsSeo } from '@/server/cache/invalidation' import { prisma } from '@/server/db' +import { NOTIFICATION_EVENTS, notificationEventEmitter } from '@/server/notifications/eventEmitter' import { trackTrustScoreChange } from '@/server/utils/trust-score-analytics' import { PERMISSIONS } from '@/utils/permission-system' import { ApprovalStatus, PcOs, Role, type UserBan } from '@orm' import { ApprovalTrustRepository } from './approval-trust.repository' import { ReportAdminEditService } from './report-admin-edit.service' import { ReportApprovalRepository } from './report-approval.repository' +import type * as NotificationEvents from '@/server/notifications/eventEmitter' vi.mock('@/server/db', () => ({ prisma: {} })) vi.mock('@/server/utils/trust-score-analytics', () => ({ trackTrustScoreChange: vi.fn() })) vi.mock('@/server/cache/invalidation', () => ({ invalidateListingSeo: vi.fn(), invalidatePcListingSeo: vi.fn(), + invalidateListingsSeo: vi.fn(), + invalidatePcListingsSeo: vi.fn(), +})) +vi.mock('@/server/notifications/eventEmitter', async (importOriginal) => ({ + ...(await importOriginal()), + notificationEventEmitter: { emitNotificationEvent: vi.fn() }, })) vi.mock('@/server/utils/cache/instances', () => ({ listingStatsCache: { delete: vi.fn() }, invalidateCatalogCompatibilityCacheForDevice: vi.fn(), })) -function setup() { +function setup(status: ApprovalStatus = ApprovalStatus.REJECTED) { const userBans: Pick[] = [] const common = { id: 'report-1', - status: ApprovalStatus.REJECTED, + status, authorId: 'author-1', gameId: 'old-game', emulatorId: 'emulator-1', @@ -132,14 +140,16 @@ describe('admin report edits', () => { try { await vi.waitFor(() => expect(run.append).toHaveBeenCalledOnce()) expect(trackTrustScoreChange).not.toHaveBeenCalled() - expect(invalidateListingSeo).not.toHaveBeenCalled() - expect(invalidatePcListingSeo).not.toHaveBeenCalled() + expect(invalidateListingsSeo).not.toHaveBeenCalled() + expect(invalidatePcListingsSeo).not.toHaveBeenCalled() + expect(notificationEventEmitter.emitNotificationEvent).not.toHaveBeenCalled() } finally { commit.resolve() } await edit expect(run.transaction).toHaveBeenCalledOnce() expect(trackTrustScoreChange).toHaveBeenCalledOnce() + expect(notificationEventEmitter.emitNotificationEvent).toHaveBeenCalledOnce() expect(reportType === 'handheld' ? run.updateHandheld : run.updatePc).toHaveBeenCalledOnce() }, ) @@ -154,9 +164,9 @@ describe('admin report edits', () => { expect.objectContaining({ weight: 6 }), ]), ) - expect(invalidateListingSeo).toHaveBeenCalledWith( + expect(invalidateListingsSeo).toHaveBeenCalledWith([ expect.objectContaining({ gameId: 'new-game', deviceId: 'new-device' }), - ) + ]) expect(trackTrustScoreChange).toHaveBeenCalledWith( expect.objectContaining({ delta: 14, oldScore: 92, newScore: 106 }), ) @@ -175,9 +185,9 @@ describe('admin report edits', () => { expect.objectContaining({ weight: 6 }), ]), ) - expect(invalidatePcListingSeo).toHaveBeenCalledWith( + expect(invalidatePcListingsSeo).toHaveBeenCalledWith([ expect.objectContaining({ gameId: 'new-game', cpuId: 'new-cpu' }), - ) + ]) }) it('does not change trust when the report status is unchanged', async () => { @@ -197,9 +207,72 @@ describe('admin report edits', () => { ).rejects.toThrow('Invalid custom field') expect(run.append).not.toHaveBeenCalled() expect(trackTrustScoreChange).not.toHaveBeenCalled() - expect(invalidateListingSeo).not.toHaveBeenCalled() + expect(invalidateListingsSeo).not.toHaveBeenCalled() + expect(notificationEventEmitter.emitNotificationEvent).not.toHaveBeenCalled() }) + it.each(['handheld', 'pc'] as const)( + 'deduplicates public %s revalidation while retaining old and new targets', + async (reportType) => { + const run = setup(ApprovalStatus.APPROVED) + if (reportType === 'handheld') { + await run.service.updateHandheld({ ...run.input, deviceId: 'new-device' }, run.actor) + expect(invalidateListingsSeo).toHaveBeenCalledExactlyOnceWith([ + expect.objectContaining({ gameId: 'old-game', deviceId: 'old-device' }), + expect.objectContaining({ gameId: 'new-game', deviceId: 'new-device' }), + ]) + } else { + await run.service.updatePc( + { ...run.input, cpuId: 'new-cpu', os: PcOs.WINDOWS, osVersion: '11', memorySize: 16 }, + run.actor, + ) + expect(invalidatePcListingsSeo).toHaveBeenCalledExactlyOnceWith([ + expect.objectContaining({ gameId: 'old-game', cpuId: 'old-cpu' }), + expect.objectContaining({ gameId: 'new-game', cpuId: 'new-cpu' }), + ]) + } + expect(notificationEventEmitter.emitNotificationEvent).not.toHaveBeenCalled() + }, + ) + + it.each(['handheld', 'pc'] as const)( + 'notifies a committed %s rejection but does not notify a reset to Pending', + async (reportType) => { + const run = setup(ApprovalStatus.APPROVED) + const statuses = [ApprovalStatus.REJECTED, ApprovalStatus.PENDING] + for (const status of statuses) { + if (reportType === 'handheld') { + await run.service.updateHandheld( + { ...run.input, status, deviceId: 'new-device' }, + run.actor, + ) + } else { + await run.service.updatePc( + { + ...run.input, + status, + cpuId: 'new-cpu', + os: PcOs.WINDOWS, + osVersion: '11', + memorySize: 16, + }, + run.actor, + ) + } + } + expect(notificationEventEmitter.emitNotificationEvent).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ + eventType: + reportType === 'handheld' + ? NOTIFICATION_EVENTS.LISTING_REJECTED + : NOTIFICATION_EVENTS.PC_LISTING_REJECTED, + entityId: 'report-1', + triggeredBy: run.actor.id, + }), + ) + }, + ) + it('rejects unauthorized handheld edits before writing', async () => { const run = setup() await expect( diff --git a/src/features/reports/approval/server/report-admin-edit.service.ts b/src/features/reports/approval/server/report-admin-edit.service.ts index 23d7c43b6..df3305b76 100644 --- a/src/features/reports/approval/server/report-admin-edit.service.ts +++ b/src/features/reports/approval/server/report-admin-edit.service.ts @@ -2,7 +2,7 @@ import { type z } from 'zod' import { AppError, ResourceError } from '@/lib/errors' import { type UpdateListingAdminSchema } from '@/schemas/listing' import { type UpdatePcListingAdminSchema } from '@/schemas/pcListing' -import { invalidateListingSeo, invalidatePcListingSeo } from '@/server/cache/invalidation' +import { invalidateListingsSeo, invalidatePcListingsSeo } from '@/server/cache/invalidation' import { invalidateCatalogCompatibilityCacheForDevice, listingStatsCache, @@ -12,19 +12,31 @@ import { hasPermission, PERMISSIONS } from '@/utils/permission-system' import { hasRolePermission } from '@/utils/permissions' import { ApprovalStatus, Role } from '@orm' import { ApprovalTrustService } from './approval-trust.service' +import { notifyReportDecision } from './report-approval.effects' import { type ApprovalActor } from './report-approval.policy' import { type ReportApprovalRepository } from './report-approval.repository' type ApprovalRecord = NonNullable>> -async function invalidateEditedReport(report: ApprovalRecord): Promise { - if (report.reportType === 'handheld') { +async function invalidateEditedReport( + previous: ApprovalRecord, + updated: ApprovalRecord, +): Promise { + const reports = [previous, updated] + if (updated.reportType === 'handheld') { listingStatsCache.delete('listing-stats') - invalidateCatalogCompatibilityCacheForDevice(report.deviceId) - if (report.status === ApprovalStatus.APPROVED) await invalidateListingSeo(report) + const handheld = reports.filter((report) => report.reportType === 'handheld') + for (const deviceId of new Set(handheld.map((report) => report.deviceId))) { + invalidateCatalogCompatibilityCacheForDevice(deviceId) + } + const publicReports = handheld.filter((report) => report.status === ApprovalStatus.APPROVED) + if (publicReports.length > 0) await invalidateListingsSeo(publicReports) } else { listingStatsCache.delete('pc-listing-stats') - if (report.status === ApprovalStatus.APPROVED) await invalidatePcListingSeo(report) + const publicReports = reports + .filter((report) => report.reportType === 'pc') + .filter((report) => report.status === ApprovalStatus.APPROVED) + if (publicReports.length > 0) await invalidatePcListingsSeo(publicReports) } } @@ -86,8 +98,15 @@ export class ReportAdminEditService { return { previous, updated, change } }) trackTrustScoreChange(result.change) - await invalidateEditedReport(result.previous) - await invalidateEditedReport(result.updated) + await invalidateEditedReport(result.previous, result.updated) + if (result.previous.status !== result.updated.status) { + notifyReportDecision({ + report: result.updated, + updated: result.updated, + actorId: actor.id, + notes: result.updated.processedNotes, + }) + } return { id: result.updated.id, status: result.updated.status, diff --git a/src/features/reports/approval/server/report-approval.effects.ts b/src/features/reports/approval/server/report-approval.effects.ts index 6fd486180..906aa48bd 100644 --- a/src/features/reports/approval/server/report-approval.effects.ts +++ b/src/features/reports/approval/server/report-approval.effects.ts @@ -32,6 +32,14 @@ export async function completeReportApproval(input: ApprovalEffectsInput): Promi if (report.reportType === 'handheld') await invalidateListingSeo(report) else await invalidatePcListingSeo(report) } + notifyReportDecision(input) +} + +export function notifyReportDecision( + input: Pick, +): void { + const report = input.report + const status = input.updated.status if (status === ApprovalStatus.PENDING) return const approved = status === ApprovalStatus.APPROVED try { diff --git a/src/features/users/server/persistence/user.query.ts b/src/features/users/server/persistence/user.query.ts index 20ad4389d..1036948a9 100644 --- a/src/features/users/server/persistence/user.query.ts +++ b/src/features/users/server/persistence/user.query.ts @@ -47,7 +47,10 @@ export function profileActivityWhere( } const votes: Prisma.VoteWhereInput = { userId: input.userId, - ...(game ? { listing: { game } } : {}), + listing: { + ...(visibility.canViewBannedUsers ? {} : { NOT: { status: ApprovalStatus.REJECTED } }), + ...(game ? { game } : {}), + }, OR: buildSearchFilter(input.votesSearch, [ 'listing.game.title', 'listing.device.modelName', diff --git a/src/features/users/server/persistence/user.selects.ts b/src/features/users/server/persistence/user.selects.ts index d77ed8b7a..8082d17bc 100644 --- a/src/features/users/server/persistence/user.selects.ts +++ b/src/features/users/server/persistence/user.selects.ts @@ -53,14 +53,19 @@ export const USER_LIMITED_PROFILE_SELECT = { }, } satisfies Prisma.UserSelect -export const USER_PUBLIC_PROFILE_SELECT = { - ...USER_LIMITED_PROFILE_SELECT, - bio: true, -} satisfies Prisma.UserSelect +export function publicProfileSelect(votesWhere: Prisma.VoteWhereInput) { + return { + ...USER_LIMITED_PROFILE_SELECT, + bio: true, + _count: { + select: { ...USER_LIMITED_PROFILE_SELECT._count.select, votes: { where: votesWhere } }, + }, + } satisfies Prisma.UserSelect +} -export function moderatorProfileSelect(now: Date) { +export function moderatorProfileSelect(now: Date, votesWhere: Prisma.VoteWhereInput) { return { - ...USER_PUBLIC_PROFILE_SELECT, + ...publicProfileSelect(votesWhere), userBans: { where: { isActive: true, OR: [{ expiresAt: null }, { expiresAt: { gt: now } }] }, select: { @@ -151,7 +156,7 @@ export const USER_UPDATE_SOURCE_SELECT = { clerkId: true, } satisfies Prisma.UserSelect -export const USER_UPDATE_RESULT_SELECT = { id: true } satisfies Prisma.UserSelect +export const USER_UPDATE_RESULT_SELECT = { name: true, bio: true } satisfies Prisma.UserSelect export const USER_ADMIN_LIST_SELECT = { id: true, diff --git a/src/features/users/server/user.repository.ts b/src/features/users/server/user.repository.ts index 2385c93c3..fc3f7a442 100644 --- a/src/features/users/server/user.repository.ts +++ b/src/features/users/server/user.repository.ts @@ -50,16 +50,17 @@ export class UserRepository extends PrismaWriteRepository { }) } - publicProfile(userId: string, canViewBannedUsers: boolean) { - if (canViewBannedUsers) { + publicProfile(input: z.output, visibility: ProfileVisibility) { + const votesWhere = profileActivityWhere({ ...input, votesSearch: undefined }, visibility).votes + if (visibility.canViewBannedUsers) { return this.prisma.user.findUnique({ - where: { id: userId }, - select: selects.moderatorProfileSelect(new Date()), + where: { id: input.userId }, + select: selects.moderatorProfileSelect(new Date(), votesWhere), }) } return this.prisma.user.findUnique({ - where: { id: userId }, - select: selects.USER_PUBLIC_PROFILE_SELECT, + where: { id: input.userId }, + select: selects.publicProfileSelect(votesWhere), }) } diff --git a/src/features/users/server/user.router.test.ts b/src/features/users/server/user.router.test.ts index 743ee2074..c2a7731ea 100644 --- a/src/features/users/server/user.router.test.ts +++ b/src/features/users/server/user.router.test.ts @@ -1,7 +1,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' +import { GetUserByIdSchema } from '@/features/users/shared/user.schemas' import { prisma } from '@/server/db' import { PERMISSIONS } from '@/utils/permission-system' import { ApprovalStatus, Role, PcOs, TailwindColor } from '@orm' +import { evaluateProfileAccess } from './profile-access.service' import { UserRepository } from './user.repository' import { usersRouter } from './user.router' import { UserService } from './user.service' @@ -82,7 +84,13 @@ const report = { } function caller( - options: { userId?: string; role?: Role; permissions?: string[]; anonymous?: boolean } = {}, + options: { + userId?: string + role?: Role + permissions?: string[] + anonymous?: boolean + showNsfw?: boolean + } = {}, ) { return usersRouter.createCaller({ prisma, @@ -96,7 +104,7 @@ function caller( email: null, role: options.role ?? Role.USER, permissions: options.permissions ?? [], - showNsfw: false, + showNsfw: options.showNsfw ?? false, }, }, }) @@ -158,7 +166,11 @@ describe('user profile transport', () => { const now = new Date(time) vi.setSystemTime(now) await repository.profileAccessRecord(targetId) - await repository.publicProfile(targetId, true) + const visibility = evaluateProfileAccess(accessRecord(), targetId, { + currentUserRole: Role.MODERATOR, + }) + if (!visibility.accessible) throw new Error('Expected moderator profile access') + await repository.publicProfile(GetUserByIdSchema.parse({ userId: targetId }), visibility) for (const call of database.user.findUnique.mock.calls.slice(-2)) { expect(call[0]).toMatchObject({ select: { @@ -251,6 +263,48 @@ describe('user profile transport', () => { ) }) + it.each([ + { role: Role.USER, showNsfw: false }, + { role: Role.USER, showNsfw: true }, + { role: Role.MODERATOR, showNsfw: false }, + { role: Role.MODERATOR, showNsfw: true }, + ])( + 'uses the visible cast-vote predicate for the list and search-independent count ($role, NSFW:$showNsfw)', + async (viewer) => { + preparePublicProfile() + database.vote.count.mockResolvedValue(0) + const result = await caller(viewer).getUserById({ userId: targetId, votesSearch: 'Mario' }) + const listing = { + ...(viewer.role === Role.MODERATOR ? {} : { NOT: { status: ApprovalStatus.REJECTED } }), + ...(viewer.showNsfw ? {} : { game: { isErotic: false } }), + } + expect(database.user.findUnique).toHaveBeenCalledWith( + expect.objectContaining({ + select: expect.objectContaining({ + _count: { + select: expect.objectContaining({ + votes: { where: { userId: targetId, listing, OR: undefined } }, + }), + }, + }), + }), + ) + const searchedWhere = expect.objectContaining({ + userId: targetId, + listing, + OR: expect.arrayContaining([ + { listing: { game: { title: { contains: 'Mario', mode: 'insensitive' } } } }, + ]), + }) + expect(database.vote.findMany).toHaveBeenCalledWith( + expect.objectContaining({ where: searchedWhere }), + ) + expect(database.vote.count).toHaveBeenCalledWith({ where: searchedWhere }) + expect(result.votes.pagination.total).toBe(0) + expect(result._count.votes).toBe(1) + }, + ) + it('allows owners to view private profiles and approved or pending reports', async () => { preparePublicProfile({ profilePublic: false }) await caller({ userId: targetId }).getUserById({ userId: targetId }) @@ -494,6 +548,29 @@ describe('user administration and edits', () => { expect(database.user.findUnique).toHaveBeenCalledTimes(1) }) + it('returns only the canonical persisted name and bio after a profile write', async () => { + database.user.findUnique.mockResolvedValue({ + id: userId, + email: 'owner@example.com', + clerkId: null, + }) + database.user.findFirst.mockResolvedValue(null) + database.user.update.mockResolvedValue({ name: 'Saved name', bio: 'Persisted bio' }) + await expect( + caller().update({ name: ' Saved name ', bio: 'Submitted bio' }), + ).resolves.toEqual({ + success: true, + profile: { name: 'Saved name', bio: 'Persisted bio' }, + }) + expect(database.user.update).toHaveBeenCalledWith( + expect.objectContaining({ + where: { id: userId }, + data: expect.objectContaining({ name: 'Saved name' }), + select: { name: true, bio: true }, + }), + ) + }) + it('writes profile changes only to the current user and clears the bio', async () => { database.user.findUnique.mockResolvedValue({ id: userId, @@ -501,9 +578,12 @@ describe('user administration and edits', () => { clerkId: 'user_clerk', }) database.user.findFirst.mockResolvedValue(null) - database.user.update.mockResolvedValue({ id: userId }) + database.user.update.mockResolvedValue({ name: 'New Name', bio: null }) const input = { name: 'New Name', bio: '', userId: targetId } - await expect(caller().update(input)).resolves.toEqual({ success: true }) + await expect(caller().update(input)).resolves.toEqual({ + success: true, + profile: { name: 'New Name', bio: null }, + }) expect(database.user.update).toHaveBeenCalledWith( expect.objectContaining({ where: { id: userId }, @@ -520,10 +600,13 @@ describe('user administration and edits', () => { clerkId: 'user_clerk', }) database.user.findFirst.mockResolvedValue(null) - database.user.update.mockResolvedValue({ id: userId }) + database.user.update.mockResolvedValue({ name: 'New Name', bio: null }) effects.updateClerk.mockRejectedValue(new Error('Clerk unavailable')) const log = vi.spyOn(console, 'error').mockImplementation(() => undefined) - await expect(caller().update({ name: 'New Name' })).resolves.toEqual({ success: true }) + await expect(caller().update({ name: 'New Name' })).resolves.toEqual({ + success: true, + profile: { name: 'New Name', bio: null }, + }) expect(database.user.update).toHaveBeenCalledTimes(1) log.mockRestore() }) diff --git a/src/features/users/server/user.router.ts b/src/features/users/server/user.router.ts index 29cfcb599..0d780418e 100644 --- a/src/features/users/server/user.router.ts +++ b/src/features/users/server/user.router.ts @@ -26,6 +26,7 @@ import { UserSessionOutputSchema, TopContributorsOutputSchema, OwnUserProfileOutputSchema, + UpdateUserOutputSchema, PublicUserProfileOutputSchema, AdminUsersOutputSchema, UserSocialOverviewOutputSchema, @@ -65,7 +66,7 @@ export const usersRouter = createTRPCRouter({ ), update: protectedProcedure .input(UpdateUserSchema) - .output(MutationSuccessSchema) + .output(UpdateUserOutputSchema) .mutation(({ ctx, input }) => createUserService(ctx.prisma).update(createActorFromSession(ctx.session), input), ), diff --git a/src/features/users/server/user.service.ts b/src/features/users/server/user.service.ts index 5c455a1b4..f7b470f88 100644 --- a/src/features/users/server/user.service.ts +++ b/src/features/users/server/user.service.ts @@ -27,7 +27,10 @@ import { hasPermission, PERMISSIONS, roleIncludesRole } from '@/utils/permission import { sanitizeBio } from '@/utils/sanitization' import { Role } from '@orm' import { evaluateProfileAccess, PRIVATE_PROFILE_SETTINGS } from './profile-access.service' -import { type PublicUserProfileOutputSchema } from '../shared/user-output.schemas' +import { + type PublicUserProfileOutputSchema, + type UpdateUserOutputSchema, +} from '../shared/user-output.schemas' export class UserService { constructor(private readonly repository: UserRepository) {} @@ -106,7 +109,7 @@ export class UserService { } } - const user = await this.repository.publicProfile(input.userId, access.canViewBannedUsers) + const user = await this.repository.publicProfile(input, access) if (!user) return ResourceError.user.notFound() const showVotes = access.isOwner || access.isMod || access.privacySettings.showVotingActivity const [ @@ -157,7 +160,10 @@ export class UserService { } } - async update(actor: Actor, input: z.output): Promise { + async update( + actor: Actor, + input: z.output, + ): Promise> { const actorUser = requireUserActor(actor) const user = await this.repository.updateSource(actorUser.userId) if (!user) return ResourceError.user.notFound() @@ -183,7 +189,7 @@ export class UserService { console.error('Failed to sync username to Clerk:', error) } } - await this.repository.updateProfile(actorUser.userId, { + const profile = await this.repository.updateProfile(actorUser.userId, { ...(input.name !== undefined && { name: input.name.trim() || null }), ...(input.email && { email: input.email }), ...(input.profileImage && { profileImage: input.profileImage }), @@ -191,7 +197,7 @@ export class UserService { lastActiveAt: new Date(), }) await invalidateUser(actorUser.userId) - return { success: true } + return { success: true, profile } } getAdminUsers(actor: Actor, input: z.output) { diff --git a/src/features/users/shared/user-output.schemas.ts b/src/features/users/shared/user-output.schemas.ts index 40e615379..3211ecf57 100644 --- a/src/features/users/shared/user-output.schemas.ts +++ b/src/features/users/shared/user-output.schemas.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { MutationSuccessSchema } from '@/schemas/common' import { PaginationResultSchema } from '@/schemas/pagination' import { ApprovalStatus, PcOs, Role, TailwindColor, TrustAction } from '@orm' @@ -78,6 +79,10 @@ export const OwnUserProfileOutputSchema = profileIdentity.extend({ _count: z.object({ listings: z.number() }), }) +export const UpdateUserOutputSchema = MutationSuccessSchema.extend({ + profile: OwnUserProfileOutputSchema.pick({ name: true, bio: true }), +}) + export const UserPrivacySettingsSchema = z.object({ profilePublic: z.boolean(), showVotingActivity: z.boolean(), diff --git a/src/hooks/useUrlSearch.test.ts b/src/hooks/useUrlSearch.test.ts new file mode 100644 index 000000000..574538dcc --- /dev/null +++ b/src/hooks/useUrlSearch.test.ts @@ -0,0 +1,163 @@ +import { act, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useUrlSearch } from './useUrlSearch' + +const navigation = vi.hoisted(() => ({ + pathname: '/games', + params: new URLSearchParams('search=Zelda&systemId=switch&page=3'), + router: { replace: vi.fn() }, +})) + +vi.mock('next/navigation', () => ({ + usePathname: () => navigation.pathname, + useSearchParams: () => navigation.params, + useRouter: () => navigation.router, +})) + +describe('URL search updates', () => { + beforeEach(() => { + vi.useFakeTimers() + navigation.pathname = '/games' + navigation.params = new URLSearchParams('search=Zelda&systemId=switch&page=3') + navigation.router.replace.mockReset() + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('keeps newer keystrokes and their remaining debounce when an earlier search commits', () => { + const view = renderHook(() => useUrlSearch()) + act(() => view.result.current[1]('Dark')) + act(() => vi.advanceTimersByTime(300)) + expect(navigation.router.replace).toHaveBeenLastCalledWith( + '/games?search=Dark&systemId=switch', + { scroll: false }, + ) + + act(() => view.result.current[1]('Dark Souls')) + act(() => vi.advanceTimersByTime(100)) + navigation.params = new URLSearchParams('search=Dark&systemId=switch') + view.rerender() + expect(view.result.current[0]).toBe('Dark Souls') + expect(view.result.current[2]).toBe('Dark') + + act(() => vi.advanceTimersByTime(199)) + expect(navigation.router.replace).toHaveBeenCalledTimes(1) + act(() => vi.advanceTimersByTime(1)) + expect(navigation.router.replace).toHaveBeenLastCalledWith( + '/games?search=Dark+Souls&systemId=switch', + { scroll: false }, + ) + }) + + it.each([0, 300])('keeps pagination for a whitespace-only search change (%ims)', (delay) => { + const view = renderHook(() => useUrlSearch(delay)) + act(() => view.result.current[1](' Zelda ')) + act(() => vi.advanceTimersByTime(delay)) + + expect(view.result.current[0]).toBe(' Zelda ') + expect(navigation.router.replace).not.toHaveBeenCalled() + }) + + it('does not navigate for whitespace added to an empty search', () => { + navigation.params = new URLSearchParams('systemId=switch&page=3') + const view = renderHook(() => useUrlSearch()) + act(() => view.result.current[1](' ')) + act(() => vi.advanceTimersByTime(300)) + + expect(navigation.router.replace).not.toHaveBeenCalled() + }) + + it('still submits a return to the current URL search while another value is in flight', () => { + const view = renderHook(() => useUrlSearch()) + act(() => view.result.current[1]('Dark')) + act(() => vi.advanceTimersByTime(300)) + act(() => view.result.current[1]('Zelda')) + act(() => vi.advanceTimersByTime(300)) + + expect(navigation.router.replace).toHaveBeenCalledTimes(2) + expect(navigation.router.replace).toHaveBeenLastCalledWith( + '/games?search=Zelda&systemId=switch', + { scroll: false }, + ) + }) + + it('cancels a newer draft when an unrelated search is navigated to', () => { + const view = renderHook(() => useUrlSearch()) + act(() => view.result.current[1]('Dark')) + act(() => vi.advanceTimersByTime(300)) + act(() => view.result.current[1]('Dark Souls')) + navigation.params = new URLSearchParams('search=Mario&systemId=switch') + view.rerender() + act(() => vi.advanceTimersByTime(300)) + + expect(view.result.current[0]).toBe('Mario') + expect(navigation.router.replace).toHaveBeenCalledTimes(1) + }) + + it('cancels a draft when the pathname changes even if its search is unchanged', () => { + const view = renderHook(() => useUrlSearch()) + act(() => view.result.current[1]('Dark')) + navigation.pathname = '/listings' + view.rerender() + act(() => vi.advanceTimersByTime(300)) + + expect(view.result.current[0]).toBe('Zelda') + expect(navigation.router.replace).not.toHaveBeenCalled() + }) + + it('does not repeat an acknowledged search when its newer draft differs only in whitespace', () => { + const view = renderHook(() => useUrlSearch()) + act(() => view.result.current[1]('Dark')) + act(() => vi.advanceTimersByTime(300)) + act(() => view.result.current[1](' Dark ')) + navigation.params = new URLSearchParams('search=Dark&systemId=switch') + view.rerender() + act(() => vi.advanceTimersByTime(300)) + + expect(view.result.current[0]).toBe(' Dark ') + expect(navigation.router.replace).toHaveBeenCalledTimes(1) + }) + + it('preserves further typing when the latest superseding navigation commits', () => { + const view = renderHook(() => useUrlSearch()) + act(() => view.result.current[1]('Dark')) + act(() => vi.advanceTimersByTime(300)) + act(() => view.result.current[1]('Dark Souls')) + act(() => vi.advanceTimersByTime(300)) + act(() => view.result.current[1]('Dark Souls II')) + act(() => vi.advanceTimersByTime(100)) + navigation.params = new URLSearchParams('search=Dark+Souls&systemId=switch') + view.rerender() + + expect(view.result.current[0]).toBe('Dark Souls II') + act(() => vi.advanceTimersByTime(200)) + expect(navigation.router.replace).toHaveBeenCalledTimes(3) + expect(navigation.router.replace).toHaveBeenLastCalledWith( + '/games?search=Dark+Souls+II&systemId=switch', + { scroll: false }, + ) + }) + + it('uses the latest unrelated URL filters when the debounce completes', () => { + const view = renderHook(() => useUrlSearch()) + act(() => view.result.current[1]('Dark')) + navigation.params = new URLSearchParams('search=Zelda&systemId=ps2&page=4') + view.rerender() + act(() => vi.advanceTimersByTime(300)) + + expect(navigation.router.replace).toHaveBeenLastCalledWith('/games?search=Dark&systemId=ps2', { + scroll: false, + }) + }) + + it('cancels a pending search on unmount', () => { + const view = renderHook(() => useUrlSearch()) + act(() => view.result.current[1]('Dark')) + view.unmount() + act(() => vi.advanceTimersByTime(300)) + + expect(navigation.router.replace).not.toHaveBeenCalled() + }) +}) diff --git a/src/hooks/useUrlSearch.ts b/src/hooks/useUrlSearch.ts index 10cf6f148..c4b42c415 100644 --- a/src/hooks/useUrlSearch.ts +++ b/src/hooks/useUrlSearch.ts @@ -9,14 +9,21 @@ export function useUrlSearch(debounceMs = 300): [string, (value: string) => void const serializedParams = searchParams.toString() const latestParams = useRef(serializedParams) const pendingUpdate = useRef | null>(null) - const [draft, setDraft] = useState({ pathname, search, value: search }) + const [pendingSearch, setPendingSearch] = useState(null) + const [draft, setDraft] = useState({ pathname, search, value: search, navigationVersion: 0 }) if (draft.pathname !== pathname || draft.search !== search) { + const ownNavigation = draft.pathname === pathname && pendingSearch === search setDraft({ pathname, search, - value: draft.pathname === pathname && draft.value.trim() === search ? draft.value : search, + value: + ownNavigation || (draft.pathname === pathname && draft.value.trim() === search) + ? draft.value + : search, + navigationVersion: draft.navigationVersion + (ownNavigation ? 0 : 1), }) + setPendingSearch(null) } useEffect(() => { @@ -30,29 +37,35 @@ export function useUrlSearch(debounceMs = 300): [string, (value: string) => void pendingUpdate.current = null } } - }, [pathname, search]) + }, [draft.navigationVersion]) const setSearch = useCallback( (value: string) => { - setDraft({ pathname, search, value }) + setDraft((current) => ({ ...current, pathname, search, value })) if (pendingUpdate.current !== null) clearTimeout(pendingUpdate.current) function updateUrl() { pendingUpdate.current = null const params = new URLSearchParams(latestParams.current) const normalized = value.trim() + if ( + normalized === (params.get('search') ?? '') && + (pendingSearch === null || pendingSearch === normalized) + ) + return if (normalized) params.set('search', normalized) else params.delete('search') params.delete('page') const query = params.toString() + setPendingSearch(normalized) router.replace(query ? `${pathname}?${query}` : pathname, { scroll: false }) } if (debounceMs > 0) pendingUpdate.current = setTimeout(updateUrl, debounceMs) else updateUrl() }, - [debounceMs, pathname, router, search], + [debounceMs, pathname, pendingSearch, router, search], ) return [draft.value, setSearch, search] diff --git a/src/schemas/game.ts b/src/schemas/game.ts index d8d4c0080..57192eb45 100644 --- a/src/schemas/game.ts +++ b/src/schemas/game.ts @@ -63,7 +63,7 @@ export const GetGamesSchema = z export const GetGameByIdSchema = z.object({ id: z.string().uuid() }) export const CheckExistingByTgdbIdsSchema = z.object({ - tgdbGameIds: z.array(SignedInt32Schema), + tgdbGameIds: z.array(SignedInt32Schema).max(PAGINATION.MAX_LIMIT), }) export const CheckExistingByNamesAndSystemsSchema = z.object({ diff --git a/src/server/api/routers/stored-integer-inputs.test.ts b/src/server/api/routers/stored-integer-inputs.test.ts index ed3231022..7a05cb648 100644 --- a/src/server/api/routers/stored-integer-inputs.test.ts +++ b/src/server/api/routers/stored-integer-inputs.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { type z } from 'zod' +import { PAGINATION } from '@/data/constants' import { bookmarksRouter } from '@/features/bookmarks/server/bookmarks.router' import { BookmarkFiltersSchema } from '@/features/bookmarks/shared/bookmark-filters.schemas' import { CreateCustomFieldCategorySchema } from '@/schemas/customFieldCategory' @@ -225,6 +226,22 @@ describe.each(operations)('$name', (operation) => { }) describe('preserved stored integer contracts', () => { + it('rejects an oversized TGDB lookup batch before database access', async () => { + await expect( + games.checkExistingByTgdbIds({ + tgdbGameIds: Array.from({ length: PAGINATION.MAX_LIMIT + 1 }, (_, index) => index), + }), + ).rejects.toMatchObject({ code: 'BAD_REQUEST' }) + expectNoDatabaseCalls() + }) + + it.each([0, PAGINATION.MAX_LIMIT])('accepts a TGDB lookup batch of %s IDs', async (size) => { + await games.checkExistingByTgdbIds({ + tgdbGameIds: Array.from({ length: size }, (_, index) => index), + }) + expect(database.game.findMany).toHaveBeenCalledOnce() + }) + it.each([-2_147_483_648, -1, 0, 2_147_483_647])( 'passes signed ID %s to Prisma unchanged', async (value) => { diff --git a/src/server/api/routers/trust-score-transactions.test.ts b/src/server/api/routers/trust-score-transactions.test.ts new file mode 100644 index 000000000..97da71ca8 --- /dev/null +++ b/src/server/api/routers/trust-score-transactions.test.ts @@ -0,0 +1,258 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { prisma } from '@/server/db' +import { logAudit } from '@/server/services/audit.service' +import { nullifyUserVotes, restoreUserVotes } from '@/server/services/vote-nullification.service' +import { trackTrustScoreChange, type TrustScoreChange } from '@/server/utils/trust-score-analytics' +import { PERMISSIONS } from '@/utils/permission-system' +import { Role, TrustAction } from '@orm' +import { type Prisma } from '@orm/client' +import { userBansRouter } from './userBans' +import { voteInvestigationRouter } from './voteInvestigation' + +vi.unmock('@orm') +vi.unmock('@orm/client') +vi.unmock('@/server/api/trpc') +vi.unmock('@/server/api/root') +const root = vi.hoisted(() => ({ + $transaction: vi.fn(), + user: { findUnique: vi.fn() }, + userBan: { findUnique: vi.fn() }, +})) +const models = vi.hoisted(() => ({ + user: { findUnique: vi.fn() }, + userBan: { findUnique: vi.fn(), findFirst: vi.fn(), create: vi.fn(), update: vi.fn() }, +})) +vi.mock('@/server/db', () => ({ prisma: root })) +vi.mock('@/server/services/audit.service', () => ({ logAudit: vi.fn(), buildDiff: vi.fn() })) +vi.mock('@/server/services/vote-nullification.service', () => ({ + nullifyUserVotes: vi.fn(), + restoreUserVotes: vi.fn(), +})) +vi.mock('@/server/utils/trust-score-analytics', () => ({ trackTrustScoreChange: vi.fn() })) + +const actorId = '00000000-0000-4000-a000-000000000001' +const userId = '00000000-0000-4000-a000-000000000002' +const banId = '00000000-0000-4000-a000-000000000003' +const headers = new Headers({ 'x-request-id': 'moderation-test' }) +const ban = { + id: banId, + userId, + user: { id: userId }, + reason: 'Spam', + notes: null, + expiresAt: null, + isActive: true, + votesNullified: true, +} +const lifted = { ...ban, isActive: false } +const transaction: Prisma.TransactionClient = { + ...prisma, + user: { ...prisma.user, ...models.user }, + userBan: { ...prisma.userBan, ...models.userBan }, +} +const change: TrustScoreChange = { + userId, + action: TrustAction.ADMIN_ADJUSTMENT_NEGATIVE, + oldScore: 10, + newScore: 5, + delta: -5, +} +const discardedChange: TrustScoreChange = { ...change, oldScore: 20, newScore: 15 } +const nullification = { + handheldVotesNullified: 1, + pcVotesNullified: 2, + commentVotesNullified: 3, + pcCommentVotesNullified: 4, + listingsRecalculated: 5, + commentsRecalculated: 6, + trustAdjustments: 7, +} satisfies Awaited> +const restoration = { + handheldVotesRestored: 1, + pcVotesRestored: 2, + commentVotesRestored: 3, + pcCommentVotesRestored: 4, + listingsRecalculated: 5, + commentsRecalculated: 6, + trustAdjustments: 7, +} satisfies Awaited> + +function context(authenticated = true) { + return { + prisma, + headers, + session: authenticated + ? { + user: { + id: actorId, + role: Role.ADMIN, + permissions: [PERMISSIONS.MANAGE_USER_BANS], + name: null, + email: null, + showNsfw: false, + }, + } + : null, + } +} + +const operations = [ + { + name: 'nullify votes', + run: (authenticated = true) => + voteInvestigationRouter + .createCaller(context(authenticated)) + .nullifyVotes({ userId, reason: 'Moderation', includeCommentVotes: false }), + service: nullifyUserVotes, + params: { + userId, + adminUserId: actorId, + reason: 'Moderation', + includeCommentVotes: false, + headers, + }, + result: nullification, + audited: false, + }, + { + name: 'restore votes', + run: (authenticated = true) => + voteInvestigationRouter + .createCaller(context(authenticated)) + .restoreVotes({ userId, reason: 'Moderation' }), + service: restoreUserVotes, + params: { userId, adminUserId: actorId, reason: 'Moderation', headers }, + result: restoration, + audited: false, + }, + { + name: 'ban and nullify', + run: (authenticated = true) => + userBansRouter + .createCaller(context(authenticated)) + .create({ userId, reason: 'Spam', nullifyVotes: true }), + service: nullifyUserVotes, + params: { + userId, + adminUserId: actorId, + reason: 'Ban: Spam', + includeCommentVotes: true, + headers, + }, + result: ban, + audited: true, + }, + { + name: 'lift ban and restore', + run: (authenticated = true) => + userBansRouter.createCaller(context(authenticated)).lift({ id: banId, notes: 'Reviewed' }), + service: restoreUserVotes, + params: { userId, adminUserId: actorId, reason: 'Ban lifted: Spam', headers }, + result: lifted, + audited: true, + }, +] + +beforeEach(() => { + vi.resetAllMocks() + root.$transaction.mockImplementation( + async (run: (tx: Prisma.TransactionClient) => Promise) => run(transaction), + ) + root.user.findUnique.mockResolvedValue({ role: Role.USER }) + root.userBan.findUnique.mockResolvedValue(ban) + models.user.findUnique.mockImplementation(async (input: { where: { id: string } }) => ({ + role: input.where.id === actorId ? Role.ADMIN : Role.USER, + })) + models.userBan.findUnique.mockResolvedValue(ban) + models.userBan.findFirst.mockResolvedValue(null) + models.userBan.create.mockResolvedValue(ban) + models.userBan.update.mockImplementation(async (input: { data: { isActive?: boolean } }) => + input.data.isActive === false ? lifted : ban, + ) + vi.mocked(nullifyUserVotes).mockImplementation(async (_tx, input) => { + input.trustChanges.push(change) + return nullification + }) + vi.mocked(restoreUserVotes).mockImplementation(async (_tx, input) => { + input.trustChanges.push(change) + return restoration + }) +}) + +describe.each(operations)('$name trust transaction', (operation) => { + it('keeps the actor, result and completion effects behind the real transaction commit', async () => { + const completed = Promise.withResolvers() + const commit = Promise.withResolvers() + root.$transaction.mockImplementationOnce( + async (run: (tx: Prisma.TransactionClient) => Promise) => { + const result = await run(transaction) + completed.resolve() + await commit.promise + return result + }, + ) + const request = operation.run() + try { + await completed.promise + expect(trackTrustScoreChange).not.toHaveBeenCalled() + expect(logAudit).not.toHaveBeenCalled() + expect(operation.service).toHaveBeenCalledWith( + transaction, + expect.objectContaining(operation.params), + ) + expect(root.$transaction).toHaveBeenCalledWith(expect.any(Function), { + isolationLevel: 'Serializable', + maxWait: 120_000, + timeout: 120_000, + }) + commit.resolve() + expect(await request).toEqual(operation.result) + expect(trackTrustScoreChange).toHaveBeenCalledExactlyOnceWith(change) + if (operation.audited) + expect(logAudit).toHaveBeenCalledWith(prisma, expect.objectContaining({ actorId, headers })) + else expect(logAudit).not.toHaveBeenCalled() + } finally { + commit.resolve() + await request + } + }) + + it('discards all completion effects when commit fails', async () => { + root.$transaction.mockImplementationOnce( + async (run: (tx: Prisma.TransactionClient) => Promise) => { + await run(transaction) + throw new Error('Commit rejected') + }, + ) + await expect(operation.run()).rejects.toThrow('Commit rejected') + expect(trackTrustScoreChange).not.toHaveBeenCalled() + expect(logAudit).not.toHaveBeenCalled() + }) + + it('discards receipts from a retry and publishes only the committed attempt', async () => { + root.$transaction.mockImplementationOnce( + async (run: (tx: Prisma.TransactionClient) => Promise) => { + await run(transaction) + throw Object.assign(new Error('Write conflict'), { code: 'P2034' }) + }, + ) + vi.mocked(nullifyUserVotes).mockImplementationOnce(async (_tx, input) => { + input.trustChanges.push(discardedChange) + return nullification + }) + vi.mocked(restoreUserVotes).mockImplementationOnce(async (_tx, input) => { + input.trustChanges.push(discardedChange) + return restoration + }) + expect(await operation.run()).toEqual(operation.result) + expect(root.$transaction).toHaveBeenCalledTimes(2) + expect(trackTrustScoreChange).toHaveBeenCalledExactlyOnceWith(change) + }) + + it('rejects an unauthenticated actor before any transaction', async () => { + await expect(operation.run(false)).rejects.toMatchObject({ code: 'UNAUTHORIZED' }) + expect(root.$transaction).not.toHaveBeenCalled() + expect(nullifyUserVotes).not.toHaveBeenCalled() + expect(restoreUserVotes).not.toHaveBeenCalled() + }) +}) diff --git a/src/server/api/routers/trust.test.ts b/src/server/api/routers/trust.test.ts index 7c6ceb6e6..06cc3a2e8 100644 --- a/src/server/api/routers/trust.test.ts +++ b/src/server/api/routers/trust.test.ts @@ -145,7 +145,7 @@ describe('trust router permission checks', () => { expect(mockPrismaTrustActionLogFindMany).toHaveBeenCalledWith( expect.objectContaining({ where, - orderBy: { user: { trustScore: 'desc' } }, + orderBy: [{ user: { trustScore: 'desc' } }, { id: 'asc' }], skip: 10, take: 10, }), diff --git a/src/server/api/routers/userBans.ts b/src/server/api/routers/userBans.ts index 5dab70fbe..8649fca1c 100644 --- a/src/server/api/routers/userBans.ts +++ b/src/server/api/routers/userBans.ts @@ -18,12 +18,11 @@ import { } from '@/server/api/trpc' import { UserBansRepository } from '@/server/repositories/user-bans.repository' import { logAudit, buildDiff } from '@/server/services/audit.service' +import { withTrustScoreTransaction } from '@/server/services/trust-score-transaction.service' import { nullifyUserVotes, restoreUserVotes } from '@/server/services/vote-nullification.service' -import { withRetryTransaction } from '@/server/utils/transactions' -import { trackTrustScoreChange, type TrustScoreChange } from '@/server/utils/trust-score-analytics' import { PERMISSIONS } from '@/utils/permission-system' import { hasRolePermission } from '@/utils/permissions' -import { AuditAction, AuditEntityType, Prisma, Role } from '@orm/client' +import { AuditAction, AuditEntityType, Role } from '@orm/client' export const userBansRouter = createTRPCRouter({ stats: permissionProcedure(PERMISSIONS.VIEW_STATISTICS).query(async ({ ctx }) => { @@ -83,10 +82,9 @@ export const userBansRouter = createTRPCRouter({ return AppError.badRequest('Expiration date must be in the future') } - const { created, nullificationResult, trustChanges } = await withRetryTransaction( + const { created, nullificationResult } = await withTrustScoreTransaction( ctx.prisma, - async (tx) => { - const trustChanges: TrustScoreChange[] = [] + async (tx, trustChanges) => { const transactionRepository = new UserBansRepository(tx) const { isBanned } = await transactionRepository.checkBanStatus(userId) if (isBanned) return ResourceError.userBan.alreadyBanned() @@ -115,12 +113,10 @@ export const userBansRouter = createTRPCRouter({ }) } - return { created: ban, nullificationResult: nullResult, trustChanges } + return { created: ban, nullificationResult: nullResult } }, - { timeout: 120_000, isolationLevel: Prisma.TransactionIsolationLevel.Serializable }, ) - for (const change of trustChanges) trackTrustScoreChange(change) void logAudit(ctx.prisma, { actorId: ctx.session.user.id, action: AuditAction.BAN, @@ -227,10 +223,9 @@ export const userBansRouter = createTRPCRouter({ isActive: ban.isActive, } - const { lifted, restorationResult, trustChanges } = await withRetryTransaction( + const { lifted, restorationResult } = await withTrustScoreTransaction( ctx.prisma, - async (tx) => { - const trustChanges: TrustScoreChange[] = [] + async (tx, trustChanges) => { const liftedBan = await new UserBansRepository(tx).lift(id, unbannedById, notes) let restoreResult: Awaited> | null = null @@ -244,12 +239,10 @@ export const userBansRouter = createTRPCRouter({ }) } - return { lifted: liftedBan, restorationResult: restoreResult, trustChanges } + return { lifted: liftedBan, restorationResult: restoreResult } }, - { timeout: 120_000, isolationLevel: Prisma.TransactionIsolationLevel.Serializable }, ) - for (const change of trustChanges) trackTrustScoreChange(change) const next = { reason: lifted.reason, notes: lifted.notes ?? null, diff --git a/src/server/api/routers/voteInvestigation.ts b/src/server/api/routers/voteInvestigation.ts index ff96c6cba..4af210ff5 100644 --- a/src/server/api/routers/voteInvestigation.ts +++ b/src/server/api/routers/voteInvestigation.ts @@ -6,16 +6,14 @@ import { RestoreUserVotesSchema, } from '@/schemas/voteInvestigation' import { createTRPCRouter, permissionProcedure } from '@/server/api/trpc' +import { withTrustScoreTransaction } from '@/server/services/trust-score-transaction.service' import { getUserVotes, getVotesOnAuthorListings, analyzeVotePatterns, } from '@/server/services/vote-investigation.service' import { nullifyUserVotes, restoreUserVotes } from '@/server/services/vote-nullification.service' -import { withRetryTransaction } from '@/server/utils/transactions' -import { trackTrustScoreChange, type TrustScoreChange } from '@/server/utils/trust-score-analytics' import { PERMISSIONS } from '@/utils/permission-system' -import { Prisma } from '@orm/client' const manageBansProcedure = permissionProcedure(PERMISSIONS.MANAGE_USER_BANS) @@ -39,45 +37,29 @@ export const voteInvestigationRouter = createTRPCRouter({ nullifyVotes: manageBansProcedure .input(NullifyUserVotesSchema) .mutation(async ({ ctx, input }) => { - const committed = await withRetryTransaction( - ctx.prisma, - async (tx) => { - const changes: TrustScoreChange[] = [] - const result = await nullifyUserVotes(tx, { - trustChanges: changes, - userId: input.userId, - adminUserId: ctx.session.user.id, - reason: input.reason, - includeCommentVotes: input.includeCommentVotes, - headers: ctx.headers, - }) - return { result, changes } - }, - { timeout: 120_000, isolationLevel: Prisma.TransactionIsolationLevel.Serializable }, + return withTrustScoreTransaction(ctx.prisma, (tx, trustChanges) => + nullifyUserVotes(tx, { + trustChanges, + userId: input.userId, + adminUserId: ctx.session.user.id, + reason: input.reason, + includeCommentVotes: input.includeCommentVotes, + headers: ctx.headers, + }), ) - for (const change of committed.changes) trackTrustScoreChange(change) - return committed.result }), restoreVotes: manageBansProcedure .input(RestoreUserVotesSchema) .mutation(async ({ ctx, input }) => { - const committed = await withRetryTransaction( - ctx.prisma, - async (tx) => { - const changes: TrustScoreChange[] = [] - const result = await restoreUserVotes(tx, { - trustChanges: changes, - userId: input.userId, - adminUserId: ctx.session.user.id, - reason: input.reason, - headers: ctx.headers, - }) - return { result, changes } - }, - { timeout: 120_000, isolationLevel: Prisma.TransactionIsolationLevel.Serializable }, + return withTrustScoreTransaction(ctx.prisma, (tx, trustChanges) => + restoreUserVotes(tx, { + trustChanges, + userId: input.userId, + adminUserId: ctx.session.user.id, + reason: input.reason, + headers: ctx.headers, + }), ) - for (const change of committed.changes) trackTrustScoreChange(change) - return committed.result }), }) diff --git a/src/server/notifications/templates.test.ts b/src/server/notifications/templates.test.ts new file mode 100644 index 000000000..da1c498d4 --- /dev/null +++ b/src/server/notifications/templates.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from 'vitest' +import { NotificationType } from '@orm' +import { notificationTemplateEngine } from './templates' + +describe.each([NotificationType.LISTING_COMMENT, NotificationType.COMMENT_ON_LISTING])( + '%s report links', + (type) => { + it.each([ + { context: { listingId: 'handheld-1' }, path: '/listings/handheld-1' }, + { context: { pcListingId: 'pc-1' }, path: '/pc-listings/pc-1' }, + ])('links to the report and its comment at $path', ({ context, path }) => { + expect(notificationTemplateEngine.generateTemplate(type, context).actionUrl).toBe(path) + expect( + notificationTemplateEngine.generateTemplate(type, { ...context, commentId: 'comment-1' }) + .actionUrl, + ).toBe(`${path}#comment-comment-1`) + }) + }, +) diff --git a/src/server/notifications/templates.ts b/src/server/notifications/templates.ts index 14ab46a20..8daf0b048 100644 --- a/src/server/notifications/templates.ts +++ b/src/server/notifications/templates.ts @@ -58,21 +58,25 @@ class NotificationTemplateEngine { private initializeTemplates(): void { // Engagement notifications - this.templates.set(NotificationType.LISTING_COMMENT, (context) => ({ - title: 'New comment on your listing', - message: `${context.userName || 'Someone'} commented on your listing${context.listingTitle ? ` "${context.listingTitle}"` : ''}${context.commentText ? `: "${sanitizeText(context.commentText)}"` : ''}`, - actionUrl: - context.listingId && context.commentId - ? `/listings/${context.listingId}#comment-${context.commentId}` - : context.listingId - ? `/listings/${context.listingId}` - : undefined, - metadata: { - listingId: context.listingId, - commentId: context.commentId, - commentText: context.commentText, - }, - })) + this.templates.set(NotificationType.LISTING_COMMENT, (context) => { + const reportUrl = context.listingId + ? `/listings/${context.listingId}` + : context.pcListingId + ? `/pc-listings/${context.pcListingId}` + : undefined + return { + title: 'New comment on your compatibility report', + message: `${context.userName || 'Someone'} commented on your compatibility report${context.listingTitle ? ` "${context.listingTitle}"` : ''}${context.commentText ? `: "${sanitizeText(context.commentText)}"` : ''}`, + actionUrl: + reportUrl && context.commentId ? `${reportUrl}#comment-${context.commentId}` : reportUrl, + metadata: { + listingId: context.listingId, + pcListingId: context.pcListingId, + commentId: context.commentId, + commentText: context.commentText, + }, + } + }) this.templates.set(NotificationType.LISTING_VOTE_UP, (context) => ({ title: 'Your listing received an upvote', diff --git a/src/server/repositories/trust-admin.repository.test.ts b/src/server/repositories/trust-admin.repository.test.ts new file mode 100644 index 000000000..89f2ab79c --- /dev/null +++ b/src/server/repositories/trust-admin.repository.test.ts @@ -0,0 +1,39 @@ +import { beforeEach, expect, it, vi } from 'vitest' +import { GetTrustLogsSchema } from '@/schemas/trust' +import { prisma } from '@/server/db' +import { TrustAdminRepository } from './trust-admin.repository' + +const mocks = vi.hoisted(() => ({ trustActionLog: { findMany: vi.fn(), count: vi.fn() } })) +vi.mock('@/server/db', () => ({ prisma: mocks })) + +beforeEach(() => { + vi.resetAllMocks() + mocks.trustActionLog.findMany.mockResolvedValue([]) + mocks.trustActionLog.count.mockResolvedValue(21) +}) + +it.each([ + { sortField: 'createdAt', order: { createdAt: 'asc' } }, + { sortField: 'action', order: { action: 'asc' } }, + { sortField: 'weight', order: { weight: 'asc' } }, + { sortField: 'user.trustScore', order: { user: { trustScore: 'asc' } } }, +] as const)('adds a unique tiebreaker to $sortField pagination', async (input) => { + await new TrustAdminRepository(prisma).listLogs( + GetTrustLogsSchema.parse({ + page: 2, + limit: 10, + sortField: input.sortField, + sortDirection: 'asc', + }), + ) + expect(mocks.trustActionLog.findMany).toHaveBeenCalledWith( + expect.objectContaining({ orderBy: [input.order, { id: 'asc' }], skip: 10, take: 10 }), + ) +}) + +it('keeps the default latest-first ordering stable', async () => { + await new TrustAdminRepository(prisma).listLogs(GetTrustLogsSchema.parse({})) + expect(mocks.trustActionLog.findMany).toHaveBeenCalledWith( + expect.objectContaining({ orderBy: [{ createdAt: 'desc' }, { id: 'asc' }] }), + ) +}) diff --git a/src/server/repositories/trust-admin.repository.ts b/src/server/repositories/trust-admin.repository.ts index 03d010f6e..8f858590d 100644 --- a/src/server/repositories/trust-admin.repository.ts +++ b/src/server/repositories/trust-admin.repository.ts @@ -46,7 +46,7 @@ export class TrustAdminRepository extends PrismaRepository { const [logs, total] = await Promise.all([ this.prisma.trustActionLog.findMany({ where, - orderBy, + orderBy: [orderBy, { id: Prisma.SortOrder.asc }], skip: (input.page - 1) * input.limit, take: input.limit, select: TRUST_LOG_SELECT, diff --git a/src/server/services/trust-score-transaction.service.ts b/src/server/services/trust-score-transaction.service.ts new file mode 100644 index 000000000..9c17d538b --- /dev/null +++ b/src/server/services/trust-score-transaction.service.ts @@ -0,0 +1,20 @@ +import { withRetryTransaction } from '@/server/utils/transactions' +import { trackTrustScoreChange, type TrustScoreChange } from '@/server/utils/trust-score-analytics' +import { Prisma, type PrismaClient } from '@orm/client' + +export async function withTrustScoreTransaction( + prisma: PrismaClient, + operation: (tx: Prisma.TransactionClient, trustChanges: TrustScoreChange[]) => Promise, +): Promise { + const committed = await withRetryTransaction( + prisma, + async (tx) => { + const trustChanges: TrustScoreChange[] = [] + const result = await operation(tx, trustChanges) + return { result, trustChanges } + }, + { timeout: 120_000, isolationLevel: Prisma.TransactionIsolationLevel.Serializable }, + ) + for (const change of committed.trustChanges) trackTrustScoreChange(change) + return committed.result +}