diff --git a/docs/commons-files-desk.md b/docs/commons-files-desk.md index 361624bd..511b0dbc 100644 --- a/docs/commons-files-desk.md +++ b/docs/commons-files-desk.md @@ -124,7 +124,7 @@ Status 2026-11-13: cut 1 merged to dev as bb615570 (PR #147); cuts 2-5 follow. C 2. **Pagination** of albums and files in `useAlbums` (done, PR #148). 3. **Folders by title path** and the album list grouped by it. 4. **Versions**: read `org.gamecult.publish.replaces`, show the latest of a chain with the earlier versions folded under it (done, PR #149). -5. **Add files to an existing album**, with per-file progress, cancel and the refusal messages above (the upload flow). +5. **Add files to an existing album**, with per-file progress, cancel and the refusal messages above (the upload flow). Done: adding to an open album and the three refusal messages (PR #150); cancel of a send in flight is not, and needs an abort path through `uploadContent`. PR #143 (Option B cut 1: contract types, host client, fake host) is **held**; under A it is not needed and would be closed. Its Soul pass found, and a revived B would have to close, an unvalidated `host` that could send the bearer to the wrong origin, hostile-looking entry names beyond separators, loose numeric checks and missing CSP and `Content-Disposition` headers in the fake. diff --git a/docs/file-zone.md b/docs/file-zone.md index 7990485d..0a895226 100644 --- a/docs/file-zone.md +++ b/docs/file-zone.md @@ -120,4 +120,4 @@ Client, `src/app/features/albums`: Flag: `fileZoneShare.enabled` in `config.json`, default `false`. With it off nothing mounts. -Not built: a Mind's own path into the zone (outbox, Persona decision, mouth post) belongs to gamecult-ops / Huginn / the mouth, not this client; sharing into an existing album ("add files"); upload cancel; pagination beyond 100 albums/files. +Not built: a Mind's own path into the zone (outbox, Persona decision, mouth post) belongs to gamecult-ops / Huginn / the mouth, not this client; cancel of a send in flight (a pick can be cancelled before it goes). Built since: pagination to 20 pages of 100 per endpoint (PR #148), adding files to an open album with named refusals (`M_TOO_LARGE`, `M_FORBIDDEN`, `M_LIMIT_EXCEEDED`). diff --git a/src/app/features/albums/AlbumView.tsx b/src/app/features/albums/AlbumView.tsx index 944cc761..fc76744a 100644 --- a/src/app/features/albums/AlbumView.tsx +++ b/src/app/features/albums/AlbumView.tsx @@ -14,7 +14,7 @@ import { markAsRead } from '../../utils/notifications'; import { getBlobSafeMimeType } from '../../utils/mimeTypes'; import { timeDayMonYear } from '../../utils/time'; import { getMemberDisplayName } from '../../utils/room'; -import { Album, AlbumFile } from './buildAlbums'; +import { Album, AlbumFile, lastEventId } from './buildAlbums'; import { folderOf, listFolder, pathLabel } from './albumFolders'; import { FileReadBy } from './FileReadBy'; import { ShareSet, useShareEnabled } from './ShareSet'; @@ -142,6 +142,13 @@ export function AlbumView({ room, onTimeline }: AlbumViewProps) { {authorOf(open)} · {countLabel(open.count)} · {timeDayMonYear(open.ts, dateFormat)} {open.count === 0 && This album has no files.} + {canShare && ( + + )}
{open.files.map((f) => ( diff --git a/src/app/features/albums/ShareSet.folder.test.tsx b/src/app/features/albums/ShareSet.folder.test.tsx index 7f23fd63..a19f6e16 100644 --- a/src/app/features/albums/ShareSet.folder.test.tsx +++ b/src/app/features/albums/ShareSet.folder.test.tsx @@ -27,8 +27,8 @@ vi.mock('./shareController', () => ({ })); vi.mock('./shareSet', async (importOriginal) => ({ ...(await importOriginal()), - sendShareSet: async (_port: unknown, title: string) => { - captured.titles.push(title); + sendShareSet: async (_port: unknown, into: string | { albumId: string }) => { + captured.titles.push(typeof into === 'string' ? into : `into:${into.albumId}`); return { albumId: '$a', sent: 0, failed: [], failedAt: [] }; }, })); @@ -42,12 +42,12 @@ vi.mock('react', async (importOriginal) => { // eslint-disable-next-line import/first import { ShareSet } from './ShareSet'; -const mount = (folder?: string[]) => { +const mount = (folder?: string[], into?: { albumId: string; lastId: string }) => { const room = { roomId: '!r:x', hasEncryptionStateEvent: () => false } as never; renderToStaticMarkup( ({ event_id: '$e' }) } as never}> - + ); @@ -82,4 +82,9 @@ describe('ShareSet: the folder reaches the album title', () => { mount(['a']); expect(await send('b/c')).toBe('a/b/c'); }); + + it('an existing album takes the files instead of a title, whatever the folder', async () => { + mount(['a'], { albumId: '$alb', lastId: '$last' }); + expect(await send('ignored')).toBe('into:$alb'); + }); }); diff --git a/src/app/features/albums/ShareSet.test.tsx b/src/app/features/albums/ShareSet.test.tsx index bfa9a619..6ae47613 100644 --- a/src/app/features/albums/ShareSet.test.tsx +++ b/src/app/features/albums/ShareSet.test.tsx @@ -14,7 +14,7 @@ const text = (html: string) => const noop = () => undefined; const file = (name: string) => ({ name, type: 'audio/ogg' } as File); -const view = (over: Partial> = {}) => +const view = (over: Partial> = {}, adding = false) => renderToStaticMarkup( > = {}) => onTitle={noop} onSend={noop} onCancel={noop} + adding={adding} /> ); @@ -76,6 +77,18 @@ describe('the share panel, drawn', () => { }); }); +describe('adding to an existing album', () => { + it('the idle row says Add files', () => { + expect(text(view({}, true))).toBe('Add files ›'); + }); + + it('picked: no title field, the names, Send and Cancel', () => { + const html = view({ files: [file('a.ogg')] }, true); + expect(html).not.toContain('aria-label="Album title"'); + expect(text(html)).toBe('a.ogg Send 1 file Cancel'); + }); +}); + describe('the flag', () => { it('off, missing or empty offers no share', () => { expect(shareEnabled({}, false)).toBe(false); diff --git a/src/app/features/albums/ShareSet.tsx b/src/app/features/albums/ShareSet.tsx index 1d76753a..a8e65b4f 100644 --- a/src/app/features/albums/ShareSet.tsx +++ b/src/app/features/albums/ShareSet.tsx @@ -10,7 +10,7 @@ import { getMsgContent } from '../room/msgContent'; import { FILE_HASH_KEY, hashForSend } from './fileReceipt'; import { ShareController, ShareState, createShareController } from './shareController'; import { titleIn } from './albumFolders'; -import { DEFAULT_TITLE, sendShareSet } from './shareSet'; +import { DEFAULT_TITLE, ExistingAlbum, sendShareSet } from './shareSet'; import * as css from './AlbumView.css'; /** Off unless `fileZoneShare.enabled` is true. An encrypted zone is never offered the share. */ @@ -29,6 +29,8 @@ type ShareViewProps = { onTitle: (title: string) => void; onSend: () => void; onCancel: () => void; + /** Adding to an existing album: the files join it, so there is no title to ask for. */ + adding?: boolean; inputRef?: React.Ref; }; @@ -43,6 +45,7 @@ export function ShareSetView({ onTitle, onSend, onCancel, + adding, inputRef, }: ShareViewProps) { const { files, title, phase, epoch } = state; @@ -58,7 +61,7 @@ export function ShareSetView({ /> {files.length === 0 && phase.kind !== 'sending' && ( )} {phase.kind === 'done' && ( @@ -68,14 +71,16 @@ export function ShareSetView({ )} {files.length > 0 && ( <> - onTitle(e.target.value)} - /> + {!adding && ( + onTitle(e.target.value)} + /> + )} {files.map((f) => f.name).join(' · ')} @@ -101,15 +106,21 @@ export function ShareSetView({ ); } -/** `folder` is the path the share lands in: its albums are titled under it (docs/commons-files-desk.md). */ -export function ShareSet({ room, folder = [] }: { room: Room; folder?: string[] }) { +type ShareSetProps = { + room: Room; + /** The path a new album lands in: its title is prefixed by it (docs/commons-files-desk.md). */ + folder?: string[]; + /** Add the files to this album instead of making one. */ + into?: ExistingAlbum; +}; +export function ShareSet({ room, folder = [], into }: ShareSetProps) { const mx = useMatrixClient(); const queryClient = useQueryClient(); const input = useRef(null); // Built once per mount (AlbumView keys the panel by room): the pick lives in the controller, so a // rerender with a new client or Room object must not build another. Live values go through `live`. - const live = useRef({ mx, room, queryClient, folder }); - live.current = { mx, room, queryClient, folder }; + const live = useRef({ mx, room, queryClient, folder, into }); + live.current = { mx, room, queryClient, folder, into }; const [controller]: [ShareController, unknown] = useState(() => createShareController({ roomId: room.roomId, @@ -142,7 +153,7 @@ export function ShareSet({ room, folder = [] }: { room: Room; folder?: string[] }, onFile, }, - titleIn(live.current.folder, title, DEFAULT_TITLE), + live.current.into ?? titleIn(live.current.folder, title, DEFAULT_TITLE), files.map((file) => ({ name: file.name, mimeType: file.type, file })) ), }) @@ -157,6 +168,7 @@ export function ShareSet({ room, folder = [] }: { room: Room; folder?: string[] onTitle={controller.setTitle} onSend={controller.send} onCancel={controller.cancel} + adding={!!into} /> ); } diff --git a/src/app/features/albums/buildAlbums.test.ts b/src/app/features/albums/buildAlbums.test.ts index 3053a493..4bf72d0e 100644 --- a/src/app/features/albums/buildAlbums.test.ts +++ b/src/app/features/albums/buildAlbums.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { AlbumInput, ZoneEvent, buildAlbums } from './buildAlbums'; +import { AlbumInput, ZoneEvent, buildAlbums, lastEventId } from './buildAlbums'; const root = (id: string, ts: number, over: Partial = {}): ZoneEvent => ({ event_id: id, @@ -239,6 +239,17 @@ describe('versions: cover and sender', () => { }); }); +describe('lastEventId', () => { + it('is the newest event of the thread, a folded version included; the root when empty', () => { + const v = (id: string, ts: number, replaces?: string) => + reply(id, ts, 'm.file', id, replaces ? { 'org.gamecult.publish': { replaces } } : {}); + expect(lastEventId(one(root('$r', 1), [v('$a', 10), v('$b', 20, '$a'), v('$c', 15)]))).toBe( + '$b' + ); + expect(lastEventId(one(root('$r', 1), []))).toBe('$r'); + }); +}); + describe('Mind receipts', () => { const HASH = 'a'.repeat(64); const file = (id: string, sha?: string) => diff --git a/src/app/features/albums/buildAlbums.ts b/src/app/features/albums/buildAlbums.ts index 636ede38..0fe2d113 100644 --- a/src/app/features/albums/buildAlbums.ts +++ b/src/app/features/albums/buildAlbums.ts @@ -134,3 +134,10 @@ export const buildAlbums = (inputs: AlbumInput[], server = ''): Album[] => { }) .sort((a, b) => b.ts - a.ts); }; + +/** The newest event in an album's thread (shown or folded), else its root: where a new file threads after. */ +export const lastEventId = (album: Album): string => + album.files + .flatMap((f) => [f, ...f.earlier]) + .reduce((a, f) => (!a || f.ts >= a.ts ? f : a), undefined)?.id ?? + album.id; diff --git a/src/app/features/albums/shareSet.test.ts b/src/app/features/albums/shareSet.test.ts index f43e5a35..d1db0b2e 100644 --- a/src/app/features/albums/shareSet.test.ts +++ b/src/app/features/albums/shareSet.test.ts @@ -5,6 +5,7 @@ import { DEFAULT_TITLE, ShareFile, SharePort, + refusalOf, remainingAfter, resultNote, sendShareSet, @@ -56,7 +57,7 @@ describe('sendShareSet', () => { it('reads back as one album of the files in pick order', async () => { const r = room(); const res = await sendShareSet(r.port, 'Take 4', clips); - expect(res).toEqual({ albumId: '$e1', sent: 3, failed: [], failedAt: [] }); + expect(res).toEqual({ albumId: '$e1', sent: 3, failed: [], failedAt: [], reasons: [] }); const album = r.album(); expect(album.title).toBe('Take 4'); expect(album.files.map((f) => [f.name, f.kind])).toEqual([ @@ -158,3 +159,58 @@ describe('after a send', () => { expect(resultNote(res)).toBe('Shared 3 files.'); }); }); + +describe('adding to an existing album', () => { + it('sends no root; each file threads under the album, the first after `lastId`', async () => { + const r = room(); + const sent: Record[] = []; + const port = { + ...r.port, + send: async (c: Record) => { + sent.push(c); + return `$n${sent.length}`; + }, + }; + const res = await sendShareSet(port, { albumId: '$root', lastId: '$last' }, clips.slice(0, 2)); + expect(res.albumId).toBe('$root'); + expect(res.sent).toBe(2); + expect(sent.map((c) => c.msgtype)).toEqual(['m.audio', 'm.audio']); + expect(sent.map((c) => c['m.relates_to'])).toEqual([ + threadRelation('$root', '$last'), + threadRelation('$root', '$n1'), + ]); + }); +}); + +describe('refusals', () => { + const refuse = (errcode: string): SharePort => ({ + send: async () => '$e', + contentOf: async () => { + throw Object.assign(new Error('raw server text'), { errcode }); + }, + }); + + it('names the three upload refusals and nothing else', () => { + expect(refusalOf({ errcode: 'M_TOO_LARGE' })).toBe('too large for this server'); + expect(refusalOf({ errcode: 'M_FORBIDDEN' })).toBe('not allowed to upload here'); + expect(refusalOf({ errcode: 'M_LIMIT_EXCEEDED' })).toBe( + 'uploading too fast, try again shortly' + ); + expect(refusalOf({ errcode: 'M_UNKNOWN' })).toBeUndefined(); + expect(refusalOf({ errcode: 'constructor' })).toBeUndefined(); + expect(refusalOf(new Error('x'))).toBeUndefined(); + expect(refusalOf(null)).toBeUndefined(); + }); + + it('the note carries the reason beside each refused file', async () => { + const res = await sendShareSet(refuse('M_TOO_LARGE'), 't', clips.slice(0, 2)); + expect(resultNote(res)).toBe( + 'Nothing was shared. Not sent: a.ogg (too large for this server), b.ogg (too large for this server)' + ); + }); + + it('an unnamed failure lists the bare name', async () => { + const res = await sendShareSet(refuse('M_UNKNOWN'), 't', clips.slice(0, 1)); + expect(resultNote(res)).toBe('Nothing was shared. Not sent: a.ogg'); + }); +}); diff --git a/src/app/features/albums/shareSet.ts b/src/app/features/albums/shareSet.ts index 3e572595..c3652f62 100644 --- a/src/app/features/albums/shareSet.ts +++ b/src/app/features/albums/shareSet.ts @@ -22,6 +22,23 @@ export type ShareResult = { failed: string[]; /** Positions in the files array of those that failed; names can repeat, positions cannot. */ failedAt: number[]; + /** Why each failed file failed, parallel to `failed`, when the homeserver's refusal is one we name. */ + reasons?: (string | undefined)[]; +}; + +/** An album that already exists: new files join its thread after `lastId`, with no new root. */ +export type ExistingAlbum = { albumId: string; lastId: string }; + +const REFUSALS: Record = { + M_TOO_LARGE: 'too large for this server', + M_FORBIDDEN: 'not allowed to upload here', + M_LIMIT_EXCEEDED: 'uploading too fast, try again shortly', +}; + +/** The sentence for a homeserver refusal we know, from the error's Matrix errcode. */ +export const refusalOf = (e: unknown): string | undefined => { + const code = (e as { errcode?: unknown } | null)?.errcode; + return typeof code === 'string' && Object.hasOwn(REFUSALS, code) ? REFUSALS[code] : undefined; }; /** The reply relation Matrix clients expect for a thread message: root, with the last event as fallback reply. */ @@ -44,12 +61,15 @@ export const titleOrDefault = (title: string): string => title.trim() || DEFAULT */ export const sendShareSet = async ( port: SharePort, - title: string, + into: string | ExistingAlbum, files: F[] ): Promise => { - const albumId = await port.send({ msgtype: 'm.text', body: titleOrDefault(title) }); - const result: ShareResult = { albumId, sent: 0, failed: [], failedAt: [] }; - let lastId = albumId; + const albumId = + typeof into === 'string' + ? await port.send({ msgtype: 'm.text', body: titleOrDefault(into) }) + : into.albumId; + const result: ShareResult = { albumId, sent: 0, failed: [], failedAt: [], reasons: [] }; + let lastId = typeof into === 'string' ? albumId : into.lastId; for (let i = 0; i < files.length; i += 1) { try { // eslint-disable-next-line no-await-in-loop @@ -58,8 +78,9 @@ export const sendShareSet = async ( lastId = await port.send({ ...content, 'm.relates_to': threadRelation(albumId, lastId) }); result.sent += 1; port.onFile?.(i, 'sent'); - } catch { + } catch (e) { result.failed.push(files[i].name); + result.reasons?.push(refusalOf(e)); result.failedAt.push(i); port.onFile?.(i, 'failed'); } @@ -69,15 +90,20 @@ export const sendShareSet = async ( /** * What stays picked after a send: only the files that did not go, so a retry is one tap. A retry - * makes a second album (adding to an existing one is not built), and a share where nothing went + * of a new album makes a second album, and a share where nothing went * leaves its root as a lone message the album list does not show (it lists threads with replies). */ export const remainingAfter = (files: T[], result: ShareResult): T[] => files.filter((_, i) => result.failedAt.includes(i)); +const failedList = (result: ShareResult): string => + result.failed + .map((name, i) => (result.reasons?.[i] ? `${name} (${result.reasons[i]})` : name)) + .join(', '); + export const resultNote = (result: ShareResult): string => { if (result.failed.length === 0) return `Shared ${result.sent} ${result.sent === 1 ? 'file' : 'files'}.`; - if (result.sent === 0) return `Nothing was shared. Not sent: ${result.failed.join(', ')}`; - return `Shared ${result.sent}. Not sent: ${result.failed.join(', ')}`; + if (result.sent === 0) return `Nothing was shared. Not sent: ${failedList(result)}`; + return `Shared ${result.sent}. Not sent: ${failedList(result)}`; };