diff --git a/packages/account-ui/src/pages/drive.jsx b/packages/account-ui/src/pages/drive.jsx index 6e753dd..17f55e1 100644 --- a/packages/account-ui/src/pages/drive.jsx +++ b/packages/account-ui/src/pages/drive.jsx @@ -36,11 +36,6 @@ function downloadHref(file, space) { return `/account/api/drive/file?${query.toString()}`; } -/** The last segment of a space AT-URI, which is the name its owner gave it. */ -function spaceLabel(uri) { - return uri.split('/').pop() || uri; -} - /** The path back to the top level, one link per folder along the way. */ function Breadcrumb({ ancestry, space, onNavigate }) { if (!ancestry?.length) return null; @@ -272,7 +267,7 @@ export function DrivePage({ data: account, folder, space, onNavigate }) { size="sm" onClick={() => onNavigate(folderHref('', entry.uri))} > - {spaceLabel(entry.uri)} + {entry.name} ))} diff --git a/packages/core/src/handlers/account.js b/packages/core/src/handlers/account.js index cb39714..24ded16 100644 --- a/packages/core/src/handlers/account.js +++ b/packages/core/src/handlers/account.js @@ -144,6 +144,32 @@ function parseByteRange(header, size) { return { start, end: Math.min(end, size - 1) }; } +/** + * Name each space for a picker. A space AT-URI ends in its record key, which + * is a TID or the literal `self` and names nothing a person recognises; the + * type NSID is the part that does. The key joins the name only where two + * spaces share a type, since there the type alone would name both. + * @param {Array<{uri: string, spaceType: string}>} rows + * @returns {Array<{uri: string, type: string, key: string, name: string}>} + */ +function nameSpaces(rows) { + /** @type {Map} */ + const byType = new Map(); + for (const row of rows) { + byType.set(row.spaceType, (byType.get(row.spaceType) ?? 0) + 1); + } + return rows.map((row) => { + const key = row.uri.split('/').pop() ?? ''; + const short = row.spaceType.split('.').pop() || row.spaceType; + return { + uri: row.uri, + type: row.spaceType, + key, + name: (byType.get(row.spaceType) ?? 0) > 1 ? `${short} (${key})` : short, + }; + }); +} + /** * An attachment header naming the download. The quoted form carries ASCII * alone, so a name with any other character rides the RFC 5987 field beside @@ -2371,12 +2397,10 @@ export function createAccountHandlers(ctx) { ); if (!listing) throw new Error('That folder name is not valid.'); // The spaces this drive can also live in, so the page can offer them. - const spaces = spaceBrowser - ? (await spaceBrowser.listSpaces()) - .filter((space) => !space.deletedAt) - .map((space) => ({ uri: space.uri, name: space.uri })) + const rows = spaceBrowser + ? (await spaceBrowser.listSpaces()).filter((space) => !space.deletedAt) : []; - return { enabled: true, ...listing, spaces }; + return { enabled: true, ...listing, spaces: nameSpaces(rows) }; }); } diff --git a/packages/drive/test/account-api.test.js b/packages/drive/test/account-api.test.js index f472f74..f59c038 100644 --- a/packages/drive/test/account-api.test.js +++ b/packages/drive/test/account-api.test.js @@ -52,7 +52,7 @@ async function fileRecord(file) { } /** - * @param {{drive?: boolean, readOnly?: boolean, upload?: boolean}} [opts] + * @param {{drive?: boolean, readOnly?: boolean, upload?: boolean, spaces?: Array<{uri: string, spaceType: string}>}} [opts] */ async function makePds(opts = {}) { /** @type {Map} */ @@ -176,6 +176,13 @@ async function makePds(opts = {}) { readOnly: opts.readOnly, driveBrowser, driveWriter, + spaceBrowser: opts.spaces + ? /** @type {any} */ ({ + listSpaces: async () => + opts.spaces?.map((space) => ({ ...space, deletedAt: null })), + countRecords: async () => [], + }) + : undefined, }); // Commit and reap run their full course in integration; here the API // contract is under test, not the repo machinery. @@ -739,3 +746,69 @@ describe('the change feed as a client would walk it', () => { expect(again.body.deleted).toEqual([]); }); }); + +describe('naming the spaces a drive can live in', () => { + const uriFor = (/** @type {string} */ type, /** @type {string} */ key) => + `at://${DID}/space/${type}/${key}`; + + it('names a space by its type, not by its record key', async () => { + // A key is a TID or the literal `self`, which names nothing a person + // recognises. The type NSID is the part that does. + const { pds } = await makePds({ + spaces: [ + { + uri: uriFor('sh.tangled.repo', '3msw4lgrmisi4'), + spaceType: 'sh.tangled.repo', + }, + { + uri: uriFor('com.example.notes', 'self'), + spaceType: 'com.example.notes', + }, + ], + }); + const cookie = await signInCookie(pds); + const { body } = await api(pds, '/account/api/drive/files', { cookie }); + expect(body.spaces.map((/** @type {any} */ s) => s.name)).toEqual([ + 'repo', + 'notes', + ]); + }); + + it('joins the key only where two spaces share a type', async () => { + const { pds } = await makePds({ + spaces: [ + { uri: uriFor('sh.tangled.repo', 'aaa'), spaceType: 'sh.tangled.repo' }, + { uri: uriFor('sh.tangled.repo', 'bbb'), spaceType: 'sh.tangled.repo' }, + { + uri: uriFor('com.example.notes', 'self'), + spaceType: 'com.example.notes', + }, + ], + }); + const cookie = await signInCookie(pds); + const { body } = await api(pds, '/account/api/drive/files', { cookie }); + expect(body.spaces.map((/** @type {any} */ s) => s.name)).toEqual([ + 'repo (aaa)', + 'repo (bbb)', + 'notes', + ]); + }); + + it('carries the type and key beside the name', async () => { + const { pds } = await makePds({ + spaces: [ + { + uri: uriFor('sh.tangled.repo', 'self'), + spaceType: 'sh.tangled.repo', + }, + ], + }); + const cookie = await signInCookie(pds); + const { body } = await api(pds, '/account/api/drive/files', { cookie }); + expect(body.spaces[0]).toMatchObject({ + type: 'sh.tangled.repo', + key: 'self', + name: 'repo', + }); + }); +});