fix: allow sellers to remove public contact methods
Quality / test (push) Canceled after 0s

This commit is contained in:
2026-09-10 07:56:17 -04:00
parent 73c430b8a5
commit 9a790ce294
12 changed files with 122 additions and 22 deletions
@@ -274,8 +274,8 @@ export function createListingRepository(db) {
marketValueUpdatedAt: override?.createdAt ?? valuation?.calculatedAt ?? null, marketValueUpdatedAt: override?.createdAt ?? valuation?.calculatedAt ?? null,
sellerDisplayName: result.sellerDisplayName, sellerDisplayName: result.sellerDisplayName,
sellerDescription: result.sellerSettings?.general?.publicDescription || null, sellerDescription: result.sellerSettings?.general?.publicDescription || null,
sellerContactEmail: result.sellerSettings?.general?.publicContactEmail ?? null, sellerContactEmail: result.sellerSettings?.general?.publicContactEmail || null,
sellerContactPhone: result.sellerSettings?.general?.publicContactPhone ?? null, sellerContactPhone: result.sellerSettings?.general?.publicContactPhone || null,
}; };
}, },
@@ -48,8 +48,8 @@ export function createProductRepository(db) {
...result.product, ...result.product,
sellerDisplayName: result.sellerDisplayName, sellerDisplayName: result.sellerDisplayName,
sellerDescription: result.sellerSettings?.general?.publicDescription || null, sellerDescription: result.sellerSettings?.general?.publicDescription || null,
sellerContactEmail: result.sellerSettings?.general?.publicContactEmail ?? null, sellerContactEmail: result.sellerSettings?.general?.publicContactEmail || null,
sellerContactPhone: result.sellerSettings?.general?.publicContactPhone ?? null, sellerContactPhone: result.sellerSettings?.general?.publicContactPhone || null,
}; };
}, },
@@ -1,6 +1,7 @@
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import test from 'node:test'; import test from 'node:test';
import { createSellerSettingsService } from '../../src/modules/settings/services/seller-settings-service.js';
import { buildApp } from '../../src/app/build-app.js'; import { buildApp } from '../../src/app/build-app.js';
function createTestApp({ authorizeUnsafeRequest } = {}) { function createTestApp({ authorizeUnsafeRequest } = {}) {
@@ -8,13 +9,14 @@ function createTestApp({ authorizeUnsafeRequest } = {}) {
return buildApp({ return buildApp({
authenticateAdmin: async () => ({ sellerId: 'seller-1' }), authenticateAdmin: async () => ({ sellerId: 'seller-1' }),
authorizeUnsafeRequest, authorizeUnsafeRequest,
sellerSettingsService: { sellerSettingsService: createSellerSettingsService({ sellerSettingsRepository: {
async get() { return settings; }, async findBySellerId(sellerId) { assert.equal(sellerId, 'seller-1'); return { settings }; },
async update({ settings: nextSettings }) { async upsert(sellerId, nextSettings) {
settings = { ...settings, ...nextSettings }; assert.equal(sellerId, 'seller-1');
return settings; 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.statusCode, 403);
assert.equal(response.json().error.code, 'FORBIDDEN'); 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);
}
});
@@ -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 assert from 'node:assert/strict';
import test, { after, beforeEach } from 'node:test'; 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); 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 } } }); 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.'); 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: '' } } }); await database.db.update(sellerSettings).set({ settings: { general: { publicDescription: '' } } });
assert.equal((await productRepository.findPublicBySlug(product.slug)).sellerDescription, null); assert.equal((await productRepository.findPublicBySlug(product.slug)).sellerDescription, null);
@@ -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 assert from 'node:assert/strict';
import test, { after, beforeEach } from 'node:test'; 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); 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 } } }); 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.'); 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: '' } } }); await database.db.update(sellerSettings).set({ settings: { general: { publicDescription: '' } } });
assert.equal((await listingRepository.findPublicById(visible.items[0].id)).sellerDescription, null); assert.equal((await listingRepository.findPublicById(visible.items[0].id)).sellerDescription, null);
@@ -108,3 +108,12 @@ export const EnlargedSellerContact = {
return () => { document.documentElement.style.fontSize = original; }; 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);
},
};
@@ -5,10 +5,13 @@ export default {
title: 'Pages/AdminSettingsPage', component: AdminSettingsPage, title: 'Pages/AdminSettingsPage', component: AdminSettingsPage,
beforeEach: () => { beforeEach: () => {
const originalFetch = globalThis.fetch; 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) => { globalThis.fetch = async (url, options) => {
if (!String(url).endsWith('/admin/settings')) return originalFetch(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 new Response(JSON.stringify({ data: stored }), { headers: { 'content-type': 'application/json' } });
}; };
return () => { globalThis.fetch = originalFetch; }; return () => { globalThis.fetch = originalFetch; };
@@ -32,3 +35,24 @@ export const EditAndClearDescription = {
await expect(field).toHaveValue(''); 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.');
},
};
+7 -6
View File
@@ -37,8 +37,8 @@ async function save() {
...settings, ...settings,
general: { general: {
...settings.general, ...settings.general,
publicContactEmail: settings.general.publicContactEmail || undefined, publicContactEmail: settings.general.publicContactEmail ?? '',
publicContactPhone: settings.general.publicContactPhone || undefined, publicContactPhone: settings.general.publicContactPhone ?? '',
}, },
}; };
Object.assign(settings, await updateAdminSettings(payload, csrfToken())); Object.assign(settings, await updateAdminSettings(payload, csrfToken()));
@@ -66,10 +66,11 @@ onMounted(load);
<label class="fixture-label" for="settings-description">Public seller description (optional)</label> <label class="fixture-label" for="settings-description">Public seller description (optional)</label>
<p id="settings-description-help">Shown on public listing and product pages. Up to 2,000 characters. Clear to remove.</p> <p id="settings-description-help">Shown on public listing and product pages. Up to 2,000 characters. Clear to remove.</p>
<textarea id="settings-description" v-model="settings.general.publicDescription" class="fixture-input" rows="5" maxlength="2000" aria-describedby="settings-description-help" /> <textarea id="settings-description" v-model="settings.general.publicDescription" class="fixture-input" rows="5" maxlength="2000" aria-describedby="settings-description-help" />
<label class="fixture-label" for="settings-email">Public contact email</label> <label class="fixture-label" for="settings-email">Public contact email (optional)</label>
<input id="settings-email" v-model="settings.general.publicContactEmail" class="fixture-input" type="email" /> <input id="settings-email" v-model="settings.general.publicContactEmail" class="fixture-input" type="email" aria-describedby="settings-contact-help" />
<label class="fixture-label" for="settings-phone">Public contact phone</label> <label class="fixture-label" for="settings-phone">Public contact phone (optional)</label>
<input id="settings-phone" v-model="settings.general.publicContactPhone" class="fixture-input" /> <input id="settings-phone" v-model="settings.general.publicContactPhone" class="fixture-input" type="tel" maxlength="50" aria-describedby="settings-contact-help" />
<p id="settings-contact-help">Shown on public listing and product pages. Clear a field and save to remove that contact method.</p>
</fieldset> </fieldset>
<fieldset class="showcase-panel"> <fieldset class="showcase-panel">
<legend>Inventory</legend> <legend>Inventory</legend>
+7
View File
@@ -190,3 +190,10 @@ Public listing and product details include nullable `sellerDescription` from
`PATCH /api/v1/admin/settings` accepts this optional plain-text field (trimmed, `PATCH /api/v1/admin/settings` accepts this optional plain-text field (trimmed,
maximum 2,000 characters); send an empty string to remove it. Omitted fields maximum 2,000 characters); send an empty string to remove it. Omitted fields
retain their current values. Private settings are not exposed in public DTOs. retain their current values. Private settings are not exposed in public DTOs.
Seller contact removal uses the same explicit-empty-string convention:
`general.publicContactEmail` accepts a valid email or `""`, and
`general.publicContactPhone` accepts up to 50 characters or `""`. Both are trimmed;
whitespace-only strings clear the contact. Omitted contact fields retain their
saved values. `null` is not a settings input. Public listing/product detail DTOs
return `null` for empty or absent contacts, so no contact link is rendered.
+6
View File
@@ -208,3 +208,9 @@ Security events are stored separately.
Store money as integer minor units. Negative values are prohibited; zero is valid. USD is canonical for stored/calculated market values unless a documented external representation requires another currency. Store money as integer minor units. Negative values are prohibited; zero is valid. USD is canonical for stored/calculated market values unless a documented external representation requires another currency.
Public seller descriptions use optional `general.publicDescription` in seller settings: trimmed plain text, at most 2,000 characters. An empty string clears the description. Public listing and product detail DTOs expose only this field as `sellerDescription`, alongside the existing public seller name and contact fields; other settings remain private. No database migration is required. Public seller descriptions use optional `general.publicDescription` in seller settings: trimmed plain text, at most 2,000 characters. An empty string clears the description. Public listing and product detail DTOs expose only this field as `sellerDescription`, alongside the existing public seller name and contact fields; other settings remain private. No database migration is required.
Public contact methods also use the existing seller settings JSON. Explicit
empty strings in `general.publicContactEmail` or `general.publicContactPhone`
represent removal; omitted fields retain their values during partial updates.
Public detail projections normalize empty contacts to `null`. No migration is
required, and seller ownership and private settings boundaries are unchanged.
+11 -3
View File
@@ -120,7 +120,7 @@ docker context use orbstack
| HTTP | Product and inventory creation, validation, authentication/recovery, CSRF, rate limiting, and session management | | HTTP | Product and inventory creation, validation, authentication/recovery, CSRF, rate limiting, and session management |
| Integration | PostgreSQL catalog/session/audit/password recovery persistence and Redis-shared login throttling | | Integration | PostgreSQL catalog/session/audit/password recovery persistence and Redis-shared login throttling |
Verified locally: `npm test` passes 166 tests with no failures (23 domain, 79 application, and 64 HTTP); worker tests, web tests, web build, Storybook build, Storybook interaction/accessibility checks, integration tests, and authenticated E2E workflows also pass when their required local services and credentials are available. Verified locally: `npm test` passes 167 tests with no failures (23 domain, 79 application, and 65 HTTP); worker tests, web tests, web build, Storybook build, Storybook interaction/accessibility checks, integration tests, and authenticated E2E workflows also pass when their required local services and credentials are available.
### Public Seller Description ### Public Seller Description
@@ -138,7 +138,7 @@ Verified locally: `npm test` passes 166 tests with no failures (23 domain, 79 ap
- Added keyboard, 320px reflow, 200% text enlargement, and detail-state regression stories. Browser review and its native-zoom/screen-reader limitations are recorded in `docs/ACCESSIBILITY.md`. - Added keyboard, 320px reflow, 200% text enlargement, and detail-state regression stories. Browser review and its native-zoom/screen-reader limitations are recorded in `docs/ACCESSIBILITY.md`.
- Validation: 166 API tests, 10 web tests, production web build, Storybook build, and 32 Storybook suites / 93 tests. This frontend-only slice changes no persistence or API contracts. - Validation: 166 API tests, 10 web tests, production web build, Storybook build, and 32 Storybook suites / 93 tests. This frontend-only slice changes no persistence or API contracts.
### Latest Vertical Slice: Public Navigation Accessibility ### Public Navigation Accessibility
- Catalog cards use router links; catalog and detail pages have descriptive titles and named, focused main landmarks on entry. - Catalog cards use router links; catalog and detail pages have descriptive titles and named, focused main landmarks on entry.
- Async responses and query updates preserve user focus. Filtered catalog URLs survive the explicit return link and browser Back; return destinations are restricted to the catalog. - Async responses and query updates preserve user focus. Filtered catalog URLs survive the explicit return link and browser Back; return destinations are restricted to the catalog.
@@ -146,7 +146,15 @@ Verified locally: `npm test` passes 166 tests with no failures (23 domain, 79 ap
- Validation: 166 API tests, 11 web tests, 3 public Playwright tests, production web build, Storybook build, and 32 Storybook suites / 93 tests. No API or persistence changes. - Validation: 166 API tests, 11 web tests, 3 public Playwright tests, production web build, Storybook build, and 32 Storybook suites / 93 tests. No API or persistence changes.
- Screen-reader speech and native browser zoom remain manual review items; this slice verifies browser focus and title behavior. - Screen-reader speech and native browser zoom remain manual review items; this slice verifies browser focus and title behavior.
Next bounded slice: clearing configured seller contact methods. The Settings form currently converts blank email/phone values to omitted fields, so the server's partial-update merge retains old contacts. Support explicit removal through validation, settings UI/API, and public detail output, with edit/clear regression coverage. ### Latest Vertical Slice: Clear Public Contact Methods
- Settings now sends explicit empty strings when a seller clears email or phone; help text explains removal. Omitted API fields still retain existing values.
- Validation accepts empty/whitespace-only email as removal while rejecting invalid non-empty addresses. Both public detail repositories expose absent/cleared contacts as null.
- HTTP coverage exercises real settings-service merging, validation, persistence reads and unchanged fields; PostgreSQL integration coverage verifies clear-one/clear-both behavior for listing and product detail projections.
- Storybook covers clearing contacts while retaining the description and rendering seller information without contact links.
- Validation: 167 API tests, 23 isolated integration tests, 11 web tests, production web build, Storybook build, and 32 Storybook suites / 95 tests. No migration required.
Next bounded slice: Settings loading/error recovery. A failed settings read currently leaves editable default values visible. Hide the form until settings load successfully, provide a retry action, and preserve entered values across failed saves, with loading/error/retry Storybook coverage.
## Next Milestone ## Next Milestone
+1 -1
View File
@@ -3,7 +3,7 @@ import { z } from 'zod';
export const sellerSettingsSchema = z.object({ export const sellerSettingsSchema = z.object({
general: z.object({ general: z.object({
publicDescription: z.string().trim().max(2000).optional(), publicDescription: z.string().trim().max(2000).optional(),
publicContactEmail: z.string().trim().email().optional(), publicContactEmail: z.string().trim().pipe(z.union([z.literal(''), z.string().email()])).optional(),
publicContactPhone: z.string().trim().max(50).optional(), publicContactPhone: z.string().trim().max(50).optional(),
}).strict().optional(), }).strict().optional(),
inventory: z.object({ inventory: z.object({