From 9a790ce294c556b4d044e4821464888be4f1449d Mon Sep 17 00:00:00 2001 From: FragginWagon Date: Thu, 10 Sep 2026 07:56:17 -0400 Subject: [PATCH] fix: allow sellers to remove public contact methods --- .../repositories/listing-repository.js | 4 +-- .../repositories/product-repository.js | 4 +-- .../tests/http/admin-settings-routes.test.js | 31 +++++++++++++++---- .../integration/catalog-repositories.test.js | 13 ++++++++ .../integration/listing-repository.test.js | 13 ++++++++ .../organisms/CollectibleDetail.stories.js | 9 ++++++ .../src/pages/AdminSettingsPage.stories.js | 28 +++++++++++++++-- apps/web/src/pages/AdminSettingsPage.vue | 13 ++++---- docs/API.md | 7 +++++ docs/DATA-MODEL.md | 6 ++++ docs/IMPLEMENTATION-STATUS.md | 14 +++++++-- packages/validation/settings.js | 2 +- 12 files changed, 122 insertions(+), 22 deletions(-) diff --git a/apps/api/src/modules/listings/repositories/listing-repository.js b/apps/api/src/modules/listings/repositories/listing-repository.js index 9f939e2..0e52f8c 100644 --- a/apps/api/src/modules/listings/repositories/listing-repository.js +++ b/apps/api/src/modules/listings/repositories/listing-repository.js @@ -274,8 +274,8 @@ export function createListingRepository(db) { marketValueUpdatedAt: override?.createdAt ?? valuation?.calculatedAt ?? null, sellerDisplayName: result.sellerDisplayName, sellerDescription: result.sellerSettings?.general?.publicDescription || null, - sellerContactEmail: result.sellerSettings?.general?.publicContactEmail ?? null, - sellerContactPhone: result.sellerSettings?.general?.publicContactPhone ?? null, + sellerContactEmail: result.sellerSettings?.general?.publicContactEmail || null, + sellerContactPhone: result.sellerSettings?.general?.publicContactPhone || null, }; }, diff --git a/apps/api/src/modules/products/repositories/product-repository.js b/apps/api/src/modules/products/repositories/product-repository.js index 1dbcf8e..5373edf 100644 --- a/apps/api/src/modules/products/repositories/product-repository.js +++ b/apps/api/src/modules/products/repositories/product-repository.js @@ -48,8 +48,8 @@ export function createProductRepository(db) { ...result.product, sellerDisplayName: result.sellerDisplayName, sellerDescription: result.sellerSettings?.general?.publicDescription || null, - sellerContactEmail: result.sellerSettings?.general?.publicContactEmail ?? null, - sellerContactPhone: result.sellerSettings?.general?.publicContactPhone ?? null, + sellerContactEmail: result.sellerSettings?.general?.publicContactEmail || null, + sellerContactPhone: result.sellerSettings?.general?.publicContactPhone || null, }; }, diff --git a/apps/api/tests/http/admin-settings-routes.test.js b/apps/api/tests/http/admin-settings-routes.test.js index f33556b..d116662 100644 --- a/apps/api/tests/http/admin-settings-routes.test.js +++ b/apps/api/tests/http/admin-settings-routes.test.js @@ -1,6 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; +import { createSellerSettingsService } from '../../src/modules/settings/services/seller-settings-service.js'; import { buildApp } from '../../src/app/build-app.js'; function createTestApp({ authorizeUnsafeRequest } = {}) { @@ -8,13 +9,14 @@ function createTestApp({ authorizeUnsafeRequest } = {}) { return buildApp({ authenticateAdmin: async () => ({ sellerId: 'seller-1' }), authorizeUnsafeRequest, - sellerSettingsService: { - async get() { return settings; }, - async update({ settings: nextSettings }) { - settings = { ...settings, ...nextSettings }; - return settings; + sellerSettingsService: createSellerSettingsService({ sellerSettingsRepository: { + async findBySellerId(sellerId) { assert.equal(sellerId, 'seller-1'); return { settings }; }, + async upsert(sellerId, nextSettings) { + assert.equal(sellerId, 'seller-1'); + settings = nextSettings; + return { settings }; }, - }, + } }), }); } @@ -58,3 +60,20 @@ test('protects settings updates with CSRF validation', async (testContext) => { assert.equal(response.statusCode, 403); assert.equal(response.json().error.code, 'FORBIDDEN'); }); + + +test('clears public contacts explicitly while omitted fields retain their values', async (testContext) => { + const app = createTestApp(); + testContext.after(() => app.close()); + const patch = (general) => app.inject({ method: 'PATCH', url: '/api/v1/admin/settings', payload: { general } }); + assert.equal((await patch({ publicContactEmail: ' owner@example.test ', publicContactPhone: '555-0100', publicDescription: 'Collector.' })).statusCode, 200); + const clearedEmail = await patch({ publicContactEmail: ' ' }); + assert.equal(clearedEmail.statusCode, 200); + assert.deepEqual(clearedEmail.json().data.general, { publicContactEmail: '', publicContactPhone: '555-0100', publicDescription: 'Collector.' }); + assert.equal((await patch({ publicContactPhone: '' })).statusCode, 200); + const read = await app.inject({ method: 'GET', url: '/api/v1/admin/settings' }); + assert.deepEqual(read.json().data.general, { publicContactEmail: '', publicContactPhone: '', publicDescription: 'Collector.' }); + for (const general of [{ publicContactEmail: 'invalid' }, { publicContactEmail: null }, { publicContactPhone: 'x'.repeat(51) }]) { + assert.equal((await patch(general)).statusCode, 400); + } +}); diff --git a/apps/api/tests/integration/catalog-repositories.test.js b/apps/api/tests/integration/catalog-repositories.test.js index 1f019e1..eccf6b4 100644 --- a/apps/api/tests/integration/catalog-repositories.test.js +++ b/apps/api/tests/integration/catalog-repositories.test.js @@ -1,3 +1,5 @@ +import { createSellerSettingsService } from '../../src/modules/settings/services/seller-settings-service.js'; +import { createSellerSettingsRepository } from '../../src/modules/settings/repositories/seller-settings-repository.js'; import assert from 'node:assert/strict'; import test, { after, beforeEach } from 'node:test'; @@ -194,6 +196,17 @@ test('finds only products with active inventory and a public listing', async () assert.equal((await productRepository.findPublicBySlug(product.slug)).sellerDescription, null); await database.db.insert(sellerSettings).values({ sellerId: seller.id, settings: { general: { publicDescription: 'Collector.' }, security: { sessionIdleMinutes: 60 } } }); assert.equal((await productRepository.findPublicBySlug(product.slug)).sellerDescription, 'Collector.'); + const settingsService = createSellerSettingsService({ sellerSettingsRepository: createSellerSettingsRepository(database.db) }); + await settingsService.update({ sellerId: seller.id, settings: { general: { publicContactEmail: 'owner@example.test', publicContactPhone: '555-0100' } } }); + assert.equal((await productRepository.findPublicBySlug(product.slug)).sellerContactEmail, 'owner@example.test'); + assert.equal((await productRepository.findPublicBySlug(product.slug)).sellerContactPhone, '555-0100'); + await settingsService.update({ sellerId: seller.id, settings: { general: { publicContactEmail: '' } } }); + assert.equal((await productRepository.findPublicBySlug(product.slug)).sellerContactEmail, null); + assert.equal((await productRepository.findPublicBySlug(product.slug)).sellerContactPhone, '555-0100'); + await settingsService.update({ sellerId: seller.id, settings: { general: { publicContactPhone: ' ' } } }); + assert.equal((await productRepository.findPublicBySlug(product.slug)).sellerContactPhone, null); + assert.equal((await productRepository.findPublicBySlug(product.slug)).sellerDescription, 'Collector.'); + await database.db.update(sellerSettings).set({ settings: { general: { publicDescription: '' } } }); assert.equal((await productRepository.findPublicBySlug(product.slug)).sellerDescription, null); diff --git a/apps/api/tests/integration/listing-repository.test.js b/apps/api/tests/integration/listing-repository.test.js index dd19e0e..ad49b8b 100644 --- a/apps/api/tests/integration/listing-repository.test.js +++ b/apps/api/tests/integration/listing-repository.test.js @@ -1,3 +1,5 @@ +import { createSellerSettingsService } from '../../src/modules/settings/services/seller-settings-service.js'; +import { createSellerSettingsRepository } from '../../src/modules/settings/repositories/seller-settings-repository.js'; import assert from 'node:assert/strict'; import test, { after, beforeEach } from 'node:test'; @@ -152,6 +154,17 @@ test('searches public listings with pagination and hides sold items by default', assert.equal((await listingRepository.findPublicById(visible.items[0].id)).sellerDescription, null); await database.db.insert(sellerSettings).values({ sellerId: product.sellerId, settings: { general: { publicDescription: 'Collector.' }, security: { sessionIdleMinutes: 60 } } }); assert.equal((await listingRepository.findPublicById(visible.items[0].id)).sellerDescription, 'Collector.'); + const settingsService = createSellerSettingsService({ sellerSettingsRepository: createSellerSettingsRepository(database.db) }); + await settingsService.update({ sellerId: product.sellerId, settings: { general: { publicContactEmail: 'owner@example.test', publicContactPhone: '555-0100' } } }); + assert.equal((await listingRepository.findPublicById(visible.items[0].id)).sellerContactEmail, 'owner@example.test'); + assert.equal((await listingRepository.findPublicById(visible.items[0].id)).sellerContactPhone, '555-0100'); + await settingsService.update({ sellerId: product.sellerId, settings: { general: { publicContactEmail: '' } } }); + assert.equal((await listingRepository.findPublicById(visible.items[0].id)).sellerContactEmail, null); + assert.equal((await listingRepository.findPublicById(visible.items[0].id)).sellerContactPhone, '555-0100'); + await settingsService.update({ sellerId: product.sellerId, settings: { general: { publicContactPhone: ' ' } } }); + assert.equal((await listingRepository.findPublicById(visible.items[0].id)).sellerContactPhone, null); + assert.equal((await listingRepository.findPublicById(visible.items[0].id)).sellerDescription, 'Collector.'); + await database.db.update(sellerSettings).set({ settings: { general: { publicDescription: '' } } }); assert.equal((await listingRepository.findPublicById(visible.items[0].id)).sellerDescription, null); diff --git a/apps/web/src/components/organisms/CollectibleDetail.stories.js b/apps/web/src/components/organisms/CollectibleDetail.stories.js index 08031a8..7700b57 100644 --- a/apps/web/src/components/organisms/CollectibleDetail.stories.js +++ b/apps/web/src/components/organisms/CollectibleDetail.stories.js @@ -108,3 +108,12 @@ export const EnlargedSellerContact = { return () => { document.documentElement.style.fontSize = original; }; }, }; + +export const ClearedSellerContacts = { + args: { ...ListingDetail.args, sellerContactEmail: null, sellerContactPhone: null }, + play: async ({ canvasElement }) => { + const seller = within(within(canvasElement).getByRole('region', { name: 'Sold by' })); + await expect(seller.getByText('Pika Collectibles')).toBeVisible(); + await expect(seller.queryAllByRole('link')).toHaveLength(0); + }, +}; diff --git a/apps/web/src/pages/AdminSettingsPage.stories.js b/apps/web/src/pages/AdminSettingsPage.stories.js index 1a35d11..d9e4848 100644 --- a/apps/web/src/pages/AdminSettingsPage.stories.js +++ b/apps/web/src/pages/AdminSettingsPage.stories.js @@ -5,10 +5,13 @@ export default { title: 'Pages/AdminSettingsPage', component: AdminSettingsPage, beforeEach: () => { const originalFetch = globalThis.fetch; - let stored = { general: { publicDescription: 'Independent collector.', publicContactEmail: 'collector@example.test' } }; + let stored = { general: { publicDescription: 'Independent collector.', publicContactEmail: 'collector@example.test', publicContactPhone: '555-0100' } }; globalThis.fetch = async (url, options) => { if (!String(url).endsWith('/admin/settings')) return originalFetch(url, options); - if (options?.method === 'PATCH') stored = JSON.parse(options.body); + if (options?.method === 'PATCH') { + const update = JSON.parse(options.body); + stored = { ...stored, ...update, general: { ...stored.general, ...update.general } }; + } return new Response(JSON.stringify({ data: stored }), { headers: { 'content-type': 'application/json' } }); }; return () => { globalThis.fetch = originalFetch; }; @@ -32,3 +35,24 @@ export const EditAndClearDescription = { await expect(field).toHaveValue(''); }, }; + +export const ClearPublicContacts = { + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const email = await canvas.findByLabelText('Public contact email (optional)'); + const phone = canvas.getByLabelText('Public contact phone (optional)'); + await expect(email).toHaveValue('collector@example.test'); + await expect(phone).toHaveValue('555-0100'); + await userEvent.clear(email); + await userEvent.click(canvas.getByRole('button', { name: 'Save settings' })); + await waitFor(() => expect(canvas.getByRole('status')).toHaveTextContent('Settings saved.')); + await expect(email).toHaveValue(''); + await expect(phone).toHaveValue('555-0100'); + await userEvent.clear(phone); + await userEvent.click(canvas.getByRole('button', { name: 'Save settings' })); + await waitFor(() => expect(canvas.getByRole('status')).toHaveTextContent('Settings saved.')); + await expect(email).toHaveValue(''); + await expect(phone).toHaveValue(''); + await expect(canvas.getByLabelText('Public seller description (optional)')).toHaveValue('Independent collector.'); + }, +}; diff --git a/apps/web/src/pages/AdminSettingsPage.vue b/apps/web/src/pages/AdminSettingsPage.vue index 1e99945..fc72838 100644 --- a/apps/web/src/pages/AdminSettingsPage.vue +++ b/apps/web/src/pages/AdminSettingsPage.vue @@ -37,8 +37,8 @@ async function save() { ...settings, general: { ...settings.general, - publicContactEmail: settings.general.publicContactEmail || undefined, - publicContactPhone: settings.general.publicContactPhone || undefined, + publicContactEmail: settings.general.publicContactEmail ?? '', + publicContactPhone: settings.general.publicContactPhone ?? '', }, }; Object.assign(settings, await updateAdminSettings(payload, csrfToken())); @@ -66,10 +66,11 @@ onMounted(load);

Shown on public listing and product pages. Up to 2,000 characters. Clear to remove.