From a2407cbeaca28423391595d461696b0366a3d2da Mon Sep 17 00:00:00 2001 From: Producdevity Date: Fri, 13 Nov 2026 23:26:59 +0100 Subject: [PATCH 1/7] fix: preserve custom field ordering and stored configuration errors --- .../server/config.mapper.ts | 32 +++++++--- .../server/config.repository.test.ts | 12 ++++ .../server/config.repository.ts | 5 +- .../server/config.router.test.ts | 58 +++++++++++++++++++ .../server/config.service.test.ts | 33 ++++++++++- .../shared/config.schemas.test.ts | 26 +++++++++ .../shared/config.schemas.ts | 26 ++++----- .../server/template.service.test.ts | 55 ++++++++++++++++++ .../server/template.service.ts | 18 +++++- 9 files changed, 238 insertions(+), 27 deletions(-) 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 5187b7d0..3cd1dfb3 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 e0101f4f..fa8d8bc1 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 d5e60fcd..dfb2e500 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 64d6a315..9fec47a1 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 bf0f4fb2..31774047 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 ca3aecb6..b2517514 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 51695f90..218e71c3 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 c4476422..04c88d28 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 39940b8d..1c76eb71 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( From 269706304403f32160cf804c16238ef3e8cf2976 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Fri, 13 Nov 2026 23:27:07 +0100 Subject: [PATCH 2/7] fix: retain bookmark and profile browsing state --- src/app/devices/DevicesPage.test.tsx | 90 ++++++++++ src/app/devices/DevicesPage.tsx | 11 +- .../components/ProfileInformation.test.tsx | 11 ++ .../profile/components/ProfileInformation.tsx | 33 ++-- src/app/users/[id]/UserProfilePage.tsx | 3 +- .../[id]/components/UserBookmarksSection.tsx | 1 + .../components/BookmarkBrowser.test.tsx | 23 +++ .../client/components/BookmarkBrowser.tsx | 8 +- .../client/components/BookmarkButton.test.tsx | 9 + .../client/components/BookmarkButton.tsx | 2 +- .../client/components/BookmarkGrid.test.tsx | 128 +++++++++++++- .../client/components/BookmarkGrid.tsx | 69 ++++++-- .../client/hooks/useBookmarkBrowser.ts | 37 +++- .../server/bookmark.repository.test.ts | 33 +++- .../bookmarks/server/bookmark.repository.ts | 13 +- .../bookmarks/server/bookmarks.router.test.ts | 12 ++ .../server/persistence/bookmark-queries.ts | 3 +- .../shared/bookmark-filters.schemas.test.ts | 24 ++- .../shared/bookmark-filters.schemas.ts | 13 ++ .../bookmarks/shared/bookmark.schemas.ts | 6 +- src/hooks/useUrlSearch.test.ts | 163 ++++++++++++++++++ src/hooks/useUrlSearch.ts | 23 ++- 22 files changed, 658 insertions(+), 57 deletions(-) create mode 100644 src/app/devices/DevicesPage.test.tsx create mode 100644 src/hooks/useUrlSearch.test.ts diff --git a/src/app/devices/DevicesPage.test.tsx b/src/app/devices/DevicesPage.test.tsx new file mode 100644 index 00000000..ee548e69 --- /dev/null +++ b/src/app/devices/DevicesPage.test.tsx @@ -0,0 +1,90 @@ +import { render, screen } from '@testing-library/react' +import { type ComponentProps } from 'react' +import { beforeEach, expect, it, vi } from 'vitest' +import { type api } from '@/lib/api' +import { type RouterOutput } from '@/types/trpc' +import DevicesPage from './DevicesPage' +import type DeviceViewModal from './components/DeviceViewModal' +import type * as UI from '@/components/ui' + +type Device = RouterOutput['devices']['get']['devices'][number] +const selected: Device = { + id: 'device-1', + modelName: 'Selected device', + brandId: 'brand-1', + socId: null, + brand: { id: 'brand-1', name: 'Brand', createdAt: new Date('2026-11-01') }, + soc: null, + _count: { listings: 1 }, +} +let devices: Device[] = [] +let params = new URLSearchParams() +let byId: { data: RouterOutput['devices']['byId'] | undefined; isError: boolean } +const byIdQuery = vi.hoisted(() => + vi.fn<(...args: Parameters) => typeof byId>(), +) + +vi.mock('next/navigation', () => ({ + useRouter: () => ({ replace: vi.fn() }), + useSearchParams: () => params, +})) +vi.mock('@/hooks', () => ({ useColumnVisibility: () => ({ isColumnVisible: () => false }) })) +vi.mock('@/lib/api', () => ({ + api: { + devices: { + get: { useQuery: () => ({ data: { devices }, isPending: false }) }, + byId: { useQuery: byIdQuery }, + }, + }, +})) +vi.mock('@/components/ui', async (importOriginal) => ({ + ...(await importOriginal()), + ColumnVisibilityControl: () => null, +})) +vi.mock('./components/DeviceViewModal', () => ({ + default: (props: ComponentProps) => + props.isOpen && props.deviceData ?
{props.deviceData.modelName}
: null, +})) + +beforeEach(() => { + devices = [selected] + params = new URLSearchParams('deviceId=device-1') + byId = { data: undefined, isError: false } + byIdQuery.mockReset().mockImplementation(() => byId) +}) + +it('keeps the latest resolved device open while it leaves the current results', () => { + const view = render() + expect(screen.getByRole('dialog')).toHaveTextContent('Selected device') + expect(byIdQuery).toHaveBeenLastCalledWith({ id: 'device-1' }, { enabled: false }) + + devices = [] + view.rerender() + expect(byIdQuery).toHaveBeenLastCalledWith({ id: 'device-1' }, { enabled: true }) + expect(screen.getByRole('dialog')).toHaveTextContent('Selected device') + + byId.data = { ...selected, modelName: 'Fetched device' } + view.rerender() + expect(screen.getByRole('dialog')).toHaveTextContent('Fetched device') + + byId.data = undefined + view.rerender() + expect(screen.getByRole('dialog')).toHaveTextContent('Fetched device') +}) + +it('does not show an old device after changing or removing its URL id, or a failed lookup', () => { + const view = render() + params = new URLSearchParams('deviceId=another-device') + view.rerender() + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + + params = new URLSearchParams('deviceId=device-1') + devices = [] + byId.isError = true + view.rerender() + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + + params = new URLSearchParams() + view.rerender() + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() +}) diff --git a/src/app/devices/DevicesPage.tsx b/src/app/devices/DevicesPage.tsx index c0d0a309..62c9c7f5 100644 --- a/src/app/devices/DevicesPage.tsx +++ b/src/app/devices/DevicesPage.tsx @@ -74,10 +74,19 @@ function DevicesPage() { }, ) - const selectedDevice = + const availableDevice = deviceInCurrentPage ?? (singleDeviceQuery.data?.id === deviceIdFromUrl ? (singleDeviceQuery.data ?? null) : null) + const [lastSelectedDevice, setLastSelectedDevice] = useState(null) + if (availableDevice && availableDevice !== lastSelectedDevice) + setLastSelectedDevice(availableDevice) + const selectedDevice = + availableDevice ?? + (!singleDeviceQuery.isError && lastSelectedDevice?.id === deviceIdFromUrl + ? lastSelectedDevice + : null) + const openModal = (device: DeviceData) => { // Update URL with device ID const params = new URLSearchParams(searchParams.toString()) diff --git a/src/app/profile/components/ProfileInformation.test.tsx b/src/app/profile/components/ProfileInformation.test.tsx index 96c3be43..ca709d40 100644 --- a/src/app/profile/components/ProfileInformation.test.tsx +++ b/src/app/profile/components/ProfileInformation.test.tsx @@ -175,6 +175,17 @@ describe('profile information draft', () => { ) expect(toast.success).not.toHaveBeenCalled() expect(toast.error).not.toHaveBeenCalled() + + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })) + expect(screen.getByText('My saved name')).toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: 'Edit' })) + expect(screen.getByDisplayValue('My saved name')).toBeInTheDocument() + + act(() => { + queryClient.setQueryData(['profile'], { ...profile, name: 'Latest server name' }) + }) + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })) + expect(await screen.findByText('Latest server name', { selector: 'p' })).toBeInTheDocument() } finally { queryClient.clear() errorLog.mockRestore() diff --git a/src/app/profile/components/ProfileInformation.tsx b/src/app/profile/components/ProfileInformation.tsx index 6239eef5..a7a7829e 100644 --- a/src/app/profile/components/ProfileInformation.tsx +++ b/src/app/profile/components/ProfileInformation.tsx @@ -24,19 +24,34 @@ function ProfileInformation(props: Props) { name: '', bio: '', }) + const [savedProfile, setSavedProfile] = useState<{ + data: UserProfileData + previousQueryData: Props['userQuery']['data'] + } | null>(null) + const profileData = + savedProfile && savedProfile.previousQueryData === props.userQuery.data + ? savedProfile.data + : props.userQuery.data const utils = api.useUtils() function handleEditToggle() { setFormData({ - name: props.userQuery.data?.name ?? '', - bio: props.userQuery.data?.bio ?? '', + name: profileData?.name ?? '', + bio: profileData?.bio ?? '', }) setIsEditing(!isEditing) } const updateProfile = api.users.update.useMutation({ - onSuccess: async () => { + onSuccess: async (_result, variables) => { + setSavedProfile({ + data: { + name: variables.name ?? profileData?.name ?? null, + bio: variables.bio ?? profileData?.bio ?? null, + }, + previousQueryData: props.userQuery.data, + }) try { await utils.users.getProfile.invalidate(undefined, undefined, { throwOnError: true }) toast.success('Profile updated successfully!') @@ -57,10 +72,10 @@ function ProfileInformation(props: Props) { } const handleFormReset = () => { - if (props.userQuery.data) { + if (profileData) { setFormData({ - name: props.userQuery.data.name ?? '', - bio: props.userQuery.data.bio ?? '', + name: profileData.name ?? '', + bio: profileData.bio ?? '', }) } setIsEditing(false) @@ -116,9 +131,7 @@ function ProfileInformation(props: Props) { className="w-full" /> ) : ( -

- {props.userQuery.data?.name || 'Not set'} -

+

{profileData?.name || 'Not set'}

)} @@ -137,7 +150,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 49733729..731319db 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 9eebad94..4c52ffd7 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/features/bookmarks/client/components/BookmarkBrowser.test.tsx b/src/features/bookmarks/client/components/BookmarkBrowser.test.tsx index b1bc6ecc..fc973c8b 100644 --- a/src/features/bookmarks/client/components/BookmarkBrowser.test.tsx +++ b/src/features/bookmarks/client/components/BookmarkBrowser.test.tsx @@ -149,3 +149,26 @@ it.each(['Minimum memory (GB)', 'Maximum memory (GB)'])( ) }, ) + +it('retains an inverted memory draft and the last valid query until the range is corrected', () => { + render() + fireEvent.click(screen.getByRole('button', { name: /PC/ })) + const minimum = screen.getByLabelText('Minimum memory (GB)') + const maximum = screen.getByLabelText('Maximum memory (GB)') + fireEvent.change(minimum, { target: { value: '16' } }) + fireEvent.change(maximum, { target: { value: '32' } }) + fireEvent.change(minimum, { target: { value: '64' } }) + expect(minimum).toHaveValue(64) + expect(maximum).toHaveAttribute('aria-invalid', 'true') + expect(maximum).toHaveAccessibleDescription('Maximum memory must be at least minimum memory.') + expect(mocks.pc).toHaveBeenLastCalledWith( + expect.objectContaining({ memoryMin: 16, memoryMax: 32 }), + expect.anything(), + ) + fireEvent.change(maximum, { target: { value: '64' } }) + expect(maximum).not.toHaveAttribute('aria-invalid') + expect(mocks.pc).toHaveBeenLastCalledWith( + expect.objectContaining({ memoryMin: 64, memoryMax: 64 }), + expect.anything(), + ) +}) diff --git a/src/features/bookmarks/client/components/BookmarkBrowser.tsx b/src/features/bookmarks/client/components/BookmarkBrowser.tsx index 2aa6e1dc..ca6ba82a 100644 --- a/src/features/bookmarks/client/components/BookmarkBrowser.tsx +++ b/src/features/bookmarks/client/components/BookmarkBrowser.tsx @@ -103,7 +103,7 @@ export default function BookmarkBrowser(props: Props) { step={1} value={browser.memoryDrafts.memoryMin} onChange={(event) => browser.changeMemory('memoryMin', event.target.value)} - aria-invalid={browser.memoryErrors.memoryMin || undefined} + aria-invalid={Boolean(browser.memoryErrors.memoryMin) || undefined} aria-describedby={ browser.memoryErrors.memoryMin ? `${minMemoryId}-error` : undefined } @@ -113,7 +113,7 @@ export default function BookmarkBrowser(props: Props) { id={`${minMemoryId}-error`} className="mt-1 text-sm text-red-600 dark:text-red-400" > - Enter a positive whole number. + {browser.memoryErrors.memoryMin}

)} @@ -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 40930829..05e40cfa 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 0856a656..9326d331 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 18f2d3e8..c1480fa0 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 6418919a..97ef0d3a 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 8ef9d228..b23dc451 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,25 @@ export function useBookmarkBrowser() { } function changeMemory(field: keyof typeof initialMemoryDrafts, value: string) { - setMemoryDrafts((current) => ({ ...current, [field]: value })) - const parsed = parseMemoryDraft(field, value) + const nextDrafts = { ...memoryDrafts, [field]: value } + setMemoryDrafts(nextDrafts) + const parsed = parseMemoryDrafts(nextDrafts) if (!parsed.success) return - setFilters((current) => ({ ...current, [field]: parsed.data })) + setFilters((current) => ({ ...current, ...parsed.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 a30fce43..decfa26f 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 9fccf91a..be551a39 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 8c959e43..ca633e55 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 f733d08e..f27fea1a 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 ef856ce2..fd5cebaf 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 4765dda3..65c8372e 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 6903a76e..43525204 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/hooks/useUrlSearch.test.ts b/src/hooks/useUrlSearch.test.ts new file mode 100644 index 00000000..574538dc --- /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 10cf6f14..c4b42c41 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] From 41329c67cf67aa13f1270a9b8ae281069c490246 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Fri, 13 Nov 2026 23:27:13 +0100 Subject: [PATCH 3/7] fix: align report decision effects and notification links --- .../server/report-admin-edit.service.test.ts | 93 +++++++++++++++++-- .../server/report-admin-edit.service.ts | 35 +++++-- .../server/report-approval.effects.ts | 8 ++ src/server/notifications/templates.test.ts | 19 ++++ src/server/notifications/templates.ts | 34 ++++--- 5 files changed, 156 insertions(+), 33 deletions(-) create mode 100644 src/server/notifications/templates.test.ts 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 bf2bef32..042eccbe 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 23d7c43b..df3305b7 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 6fd48618..906aa48b 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/server/notifications/templates.test.ts b/src/server/notifications/templates.test.ts new file mode 100644 index 00000000..da1c498d --- /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 14ab46a2..8daf0b04 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', From 4d7af0b7d2ba3a3eda155217f7613ae5666adf09 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Fri, 13 Nov 2026 23:27:20 +0100 Subject: [PATCH 4/7] fix: bound TGDB duplicate lookup batches --- src/schemas/game.ts | 2 +- .../api/routers/stored-integer-inputs.test.ts | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/schemas/game.ts b/src/schemas/game.ts index d8d4c008..57192eb4 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 ed323102..7a05cb64 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) => { From 9c92ca72bfeecea07bf6e21057bf464ac328a6f4 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Fri, 13 Nov 2026 23:27:27 +0100 Subject: [PATCH 5/7] refactor: centralize committed moderation trust effects --- .../routers/trust-score-transactions.test.ts | 257 ++++++++++++++++++ src/server/api/routers/trust.test.ts | 2 +- src/server/api/routers/userBans.ts | 23 +- src/server/api/routers/voteInvestigation.ts | 54 ++-- .../trust-admin.repository.test.ts | 39 +++ .../repositories/trust-admin.repository.ts | 2 +- .../trust-score-transaction.service.ts | 20 ++ 7 files changed, 344 insertions(+), 53 deletions(-) create mode 100644 src/server/api/routers/trust-score-transactions.test.ts create mode 100644 src/server/repositories/trust-admin.repository.test.ts create mode 100644 src/server/services/trust-score-transaction.service.ts 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 00000000..a544b9bf --- /dev/null +++ b/src/server/api/routers/trust-score-transactions.test.ts @@ -0,0 +1,257 @@ +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 })) + } 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 7c6ceb6e..06cc3a2e 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 5dab70fb..8649fca1 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 ff96c6cb..4af210ff 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/repositories/trust-admin.repository.test.ts b/src/server/repositories/trust-admin.repository.test.ts new file mode 100644 index 00000000..89f2ab79 --- /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 03d010f6..8f858590 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 00000000..9c17d538 --- /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 +} From 03beaa78d33f7efcfb4496047701745558897b46 Mon Sep 17 00:00:00 2001 From: Producdevity Date: Fri, 13 Nov 2026 23:27:33 +0100 Subject: [PATCH 6/7] fix: name form controls and preserve preference feedback --- .../components/AdminCreateKeyForm.tsx | 1 + .../admin/badges/components/BadgeModal.tsx | 7 +----- .../releases/components/CreateReleaseCard.tsx | 10 ++++---- .../hooks/useUrlFilterState.page-size.test.ts | 12 ++++++++++ .../shared/hooks/useUrlFilterState.ts | 6 +++-- src/components/CookieConsent.test.tsx | 3 +++ src/components/CookieConsent.tsx | 10 ++++---- .../ui/form/MarkdownToolbarButton.tsx | 1 + src/components/ui/form/SelectInput.test.tsx | 24 +++++++++++++++++++ src/components/ui/form/SelectInput.tsx | 10 +++++--- src/components/ui/form/Toggle.tsx | 6 ++--- .../components/DriverVersionField.test.tsx | 23 +++++++++++++++++- .../client/components/DriverVersionField.tsx | 8 +++---- .../client/components/DriverVersionInput.tsx | 2 +- 14 files changed, 93 insertions(+), 30 deletions(-) create mode 100644 src/components/ui/form/SelectInput.test.tsx diff --git a/src/app/admin/api-access/components/AdminCreateKeyForm.tsx b/src/app/admin/api-access/components/AdminCreateKeyForm.tsx index 73861181..b1bc288d 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 c09e0e21..89a0f5f1 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() {
- +