From 7c7ca012040fc7e9bba28b2dea231a28ddeb6123 Mon Sep 17 00:00:00 2001 From: Precious Oritsedere Date: Thu, 15 Oct 2026 14:52:10 +0100 Subject: [PATCH 01/12] Compare folder URLs consistently in the move dialog --- app/components/MoveDialog.tsx | 133 ++++++++++++++++++---------------- 1 file changed, 69 insertions(+), 64 deletions(-) diff --git a/app/components/MoveDialog.tsx b/app/components/MoveDialog.tsx index 202e4c7..e65ba17 100644 --- a/app/components/MoveDialog.tsx +++ b/app/components/MoveDialog.tsx @@ -19,6 +19,7 @@ import { getAuthenticatedSession, decodeResourceNameFromUrl, ensureTrailingSlash, + folderUrlsEqual, } from "../lib/helpers"; import { Folder, AlertTriangle } from "lucide-react"; @@ -144,9 +145,11 @@ export default function MoveDialog({ } } catch (error) { console.error("Failed to move file:", error); - toast.add({ title: error instanceof Error + toast.add({ + title: error instanceof Error ? `Failed to move: ${error.message}` - : "Failed to move file", type: "error" }); + : "Failed to move file", type: "error" + }); } finally { setIsMoving(false); } @@ -160,9 +163,11 @@ export default function MoveDialog({ if (!file) return null; - // Filter out the current folder (where the file currently is) + // Filter out the current folder (where the file currently is). + // Compare normalised URLs: currentLocationUrl can arrive without a + // trailing slash from the ?url= param, while listing URLs always have one. const filteredFolders = allFolders.filter( - (folder) => folder.type === "folder" && folder.url !== currentLocationUrl + (folder) => folder.type === "folder" && !folderUrlsEqual(folder.url, currentLocationUrl) ); return ( @@ -174,71 +179,71 @@ export default function MoveDialog({ {file.name} -
- {/* Current Location */} -
- -
- - - {currentLocationUrl - ? availableFolders.find(f => f.url === currentLocationUrl)?.name || - (() => { - try { - const url = new URL(currentLocationUrl); - return url.pathname.split("/").filter(Boolean).pop() || currentLocationUrl; - } catch { - return currentLocationUrl; - } - })() - : "My Storages"} - -
-
- - {/* Available Folders */} -
- - {isLoadingFolders ? ( -
- -

Loading folders...

-
- ) : filteredFolders.length === 0 ? ( -
- No folders available to move to +
+ {/* Current Location */} +
+ +
+ + + {currentLocationUrl + ? availableFolders.find((f) => folderUrlsEqual(f.url, currentLocationUrl))?.name || + (() => { + try { + const url = new URL(currentLocationUrl); + return url.pathname.split("/").filter(Boolean).pop() || currentLocationUrl; + } catch { + return currentLocationUrl; + } + })() + : "My Storages"} +
- ) : ( -
- {filteredFolders.map((folder) => ( -
+ + {/* Available Folders */} +
+ + {isLoadingFolders ? ( +
+ +

Loading folders...

+
+ ) : filteredFolders.length === 0 ? ( +
+ No folders available to move to +
+ ) : ( +
+ {filteredFolders.map((folder) => ( + - ))} + }`} + > + + {folder.name} + + ))} +
+ )} +
+ + {filteredFolders.length > 0 && !selectedFolderUrl && ( +
+ + Select a location to show the folder path
)} -
- - {filteredFolders.length > 0 && !selectedFolderUrl && ( -
- - Select a location to show the folder path -
- )} -
+ - ))} + + + {/* Available Folders */} +
+ + {isLoadingFolders ? ( +
+ +

Loading folders...

+
+ ) : filteredFolders.length === 0 ? ( +
+ No folders available to move to +
+ ) : ( +
+ {filteredFolders.map((folder) => ( + + ))} +
+ )} +
+ + {filteredFolders.length > 0 && !selectedFolderUrl && ( +
+ + Select a location to show the folder path
)} - - - {filteredFolders.length > 0 && !selectedFolderUrl && ( -
- - Select a location to show the folder path -
- )} - - - - - - - + + + + + + + + {conflictDialog} + ); } diff --git a/app/lib/helpers/moveUtils.ts b/app/lib/helpers/moveUtils.ts index 90fcb18..0cafb11 100644 --- a/app/lib/helpers/moveUtils.ts +++ b/app/lib/helpers/moveUtils.ts @@ -1,14 +1,19 @@ // Solid has no server-side move, so moving is copy-then-delete. -import { deleteFile, getFile, UrlString } from "@inrupt/solid-client"; +import { deleteFile, getFile, overwriteFile, UrlString } from "@inrupt/solid-client"; +import { loadContainerListing } from "../cache"; +import { fetchContainerListing } from "./containerListingUtils"; import { decodeResourceNameFromUrl } from "./copyUtils"; import { getHttpStatus } from "./httpErrorUtils"; import { createResourceWithSlug, isIndeterminateWriteError, + isNameConflictError, type SlugWriteResult, } from "./slugUtils"; +const MAX_KEEP_BOTH_ATTEMPTS = 100; + export interface MoveResult { /** Where the resource actually ended up. */ url: string; @@ -22,6 +27,14 @@ export interface MoveResult { sourceRemoved: boolean; } + +export interface MoveConflict { + existingName: string; + targetUrl: string; +} + +export type MoveConflictChoice = "replace" | "keepBoth"; + /** * Thrown when a write may or may not have reached the destination. */ @@ -37,6 +50,25 @@ export class IndeterminateMoveError extends Error { } } +/** + * Look for a resource already using requestedName at the destination, so the + * user can be asked before any bytes are transferred. + */ +export async function findMoveConflict( + destinationFolderUrl: string, + requestedName: string, + fetchFn: typeof fetch, +): Promise { + const listing = await loadContainerListing(destinationFolderUrl, () => + fetchContainerListing(destinationFolderUrl, fetchFn), + ); + + const existing = listing.find((item) => item.name === requestedName); + return existing + ? { existingName: existing.name, targetUrl: existing.url } + : null; +} + /** * Move a file into another container, optionally under a new name. * @@ -51,33 +83,133 @@ export async function moveFileResource( ): Promise { const originalName = file.name ?? decodeResourceNameFromUrl(file.url); const requestedName = newName?.trim() || originalName; + const { body, contentType } = await readSource(file, fetchFn); - const fileBlob = await getFile(file.url as UrlString, { fetch: fetchFn }); - const contentType = - fileBlob.type || file.mimeType || "application/octet-stream"; - - let written: SlugWriteResult; - try { - written = await createResourceWithSlug( + const written = await writeOrFlagIndeterminate(destinationFolderUrl, () => + createResourceWithSlug( destinationFolderUrl, - fileBlob, + body, requestedName, contentType, fetchFn, - ); + ), + ); + + return completeMove(written, requestedName, originalName, file.url, fetchFn); +} + +/** Move a file when the user has decided how to resolve a name clash. */ +export async function moveFileWithConflictChoice( + file: { url: string; name?: string; mimeType?: string }, + destinationFolderUrl: string, + newName: string, + conflict: MoveConflict, + choice: MoveConflictChoice, + fetchFn: typeof fetch, +): Promise { + const originalName = file.name ?? decodeResourceNameFromUrl(file.url); + const requestedName = newName?.trim() || originalName; + const { body, contentType } = await readSource(file, fetchFn); + + const written = + choice === "replace" + ? await writeOrFlagIndeterminate(destinationFolderUrl, async () => { + await overwriteFile(conflict.targetUrl as UrlString, body, { + contentType, + fetch: fetchFn, + }); + // We wrote over a resource we had already located, so the name is exact. + return { + url: conflict.targetUrl, + name: conflict.existingName, + slugRespected: true, + }; + }) + : await createKeepingBoth( + destinationFolderUrl, + body, + requestedName, + contentType, + fetchFn, + ); + + return completeMove(written, requestedName, originalName, file.url, fetchFn); +} + +/** Read the resource being moved, with the content type to write it back under. */ +async function readSource( + file: { url: string; mimeType?: string }, + fetchFn: typeof fetch, +): Promise<{ body: Blob; contentType: string }> { + const body = await getFile(file.url as UrlString, { fetch: fetchFn }); + return { + body, + contentType: body.type || file.mimeType || "application/octet-stream", + }; +} + +/** Create under "name (1)", "name (2)" and so on, since the plain name is taken. */ +async function createKeepingBoth( + destinationFolderUrl: string, + body: Blob, + requestedName: string, + contentType: string, + fetchFn: typeof fetch, +): Promise { + const lastDot = requestedName.lastIndexOf("."); + const base = lastDot > 0 ? requestedName.slice(0, lastDot) : requestedName; + const ext = lastDot > 0 ? requestedName.slice(lastDot) : ""; + + for (let attempt = 1; attempt <= MAX_KEEP_BOTH_ATTEMPTS; attempt += 1) { + try { + return await writeOrFlagIndeterminate(destinationFolderUrl, () => + createResourceWithSlug( + destinationFolderUrl, + body, + `${base} (${attempt})${ext}`, + contentType, + fetchFn, + ), + ); + } catch (error) { + if (!isNameConflictError(error)) { + throw error; + } + } + } + + throw new Error(`Could not find a free name based on "${requestedName}".`); +} + +/** Convert a write we cannot account for into a signal not to touch the source. */ +async function writeOrFlagIndeterminate( + destinationFolderUrl: string, + write: () => Promise, +): Promise { + try { + return await write(); } catch (error) { if (isIndeterminateWriteError(error)) { throw new IndeterminateMoveError(destinationFolderUrl, error); } throw error; } +} +/** Remove the original now the copy is confirmed, and report what happened. */ +async function completeMove( + written: SlugWriteResult, + requestedName: string, + originalName: string, + sourceUrl: string, + fetchFn: typeof fetch, +): Promise { return { url: written.url, name: written.name, requestedName, originalName, - sourceRemoved: await deleteSource(file.url, fetchFn), + sourceRemoved: await deleteSource(sourceUrl, fetchFn), }; } From 223178adbc7a502d64c5a797bc3196ecce37a552 Mon Sep 17 00:00:00 2001 From: Precious Oritsedere Date: Fri, 16 Oct 2026 15:06:12 +0100 Subject: [PATCH 09/12] Report the name the server actually assigned after a move --- app/components/MoveDialog.tsx | 48 ++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/app/components/MoveDialog.tsx b/app/components/MoveDialog.tsx index 893ce67..173a9a3 100644 --- a/app/components/MoveDialog.tsx +++ b/app/components/MoveDialog.tsx @@ -19,9 +19,11 @@ import { moveFileWithConflictChoice, findMoveConflict, getAuthenticatedSession, + getParentContainerUrl, ensureTrailingSlash, folderUrlsEqual, fetchContainerListing, + type MoveResult, } from "../lib/helpers"; import { useNameConflictPrompt } from "./useNameConflictPrompt"; import { loadContainerListing } from "../lib/cache"; @@ -36,6 +38,42 @@ interface MoveDialogProps { onMoved?: (destinationUrl: string) => void; } +/** + * Describes what actually happened, which is not always what was asked for: + * the server may rename the resource, ignore a rename, or refuse to let go of + * the original. + */ +function describeMoveResult(result: MoveResult): { + title: string; + type: "success" | "warning" | "error"; +} { + // Worst case first: the copy landed but the pod now holds two of them. + if (!result.sourceRemoved) { + return { + title: `Copied "${result.name}", but the original could not be removed. Both copies now exist.`, + type: "error", + }; + } + + if (result.name === result.requestedName) { + return { title: `Moved "${result.name}"`, type: "success" }; + } + + // A rename was asked for and the old name stuck, so the move landed but the + // rename did not. + if ( + result.requestedName !== result.originalName && + result.name === result.originalName + ) { + return { + title: `Moved, but the name was not changed. It is still "${result.originalName}".`, + type: "warning", + }; + } + + return { title: `Moved as "${result.name}"`, type: "success" }; +} + export default function MoveDialog({ isOpen, onClose, @@ -152,13 +190,15 @@ export default function MoveDialog({ fetchFn, ); + let result: MoveResult; + if (conflict) { const choice = await askConflictChoice(conflict.existingName); if (choice === "cancel") { return; } - await moveFileWithConflictChoice( + result = await moveFileWithConflictChoice( file, selectedFolderUrl, trimmedName, @@ -167,15 +207,15 @@ export default function MoveDialog({ fetchFn, ); } else { - await moveFileResource(file, selectedFolderUrl, fetchFn, trimmedName); + result = await moveFileResource(file, selectedFolderUrl, fetchFn, trimmedName); } - toast.add({ title: `Moved "${file.name}"`, type: "success" }); + toast.add(describeMoveResult(result)); onClose(); // Notify parent to refresh if (onMoved) { - onMoved(selectedFolderUrl); + onMoved(getParentContainerUrl(result.url)); } } catch (error) { console.error("Failed to move file:", error); From e35f6485356d9517b0e97f2939f5f5196e15196c Mon Sep 17 00:00:00 2001 From: Precious Oritsedere Date: Fri, 16 Oct 2026 15:15:44 +0100 Subject: [PATCH 10/12] Give move failures specific error messages --- app/components/MoveDialog.tsx | 89 ++++++++++++++++++++++++++++++++--- 1 file changed, 82 insertions(+), 7 deletions(-) diff --git a/app/components/MoveDialog.tsx b/app/components/MoveDialog.tsx index 173a9a3..c6118b4 100644 --- a/app/components/MoveDialog.tsx +++ b/app/components/MoveDialog.tsx @@ -20,9 +20,11 @@ import { findMoveConflict, getAuthenticatedSession, getParentContainerUrl, + getHttpStatus, ensureTrailingSlash, folderUrlsEqual, fetchContainerListing, + IndeterminateMoveError, type MoveResult, } from "../lib/helpers"; import { useNameConflictPrompt } from "./useNameConflictPrompt"; @@ -74,6 +76,77 @@ function describeMoveResult(result: MoveResult): { return { title: `Moved as "${result.name}"`, type: "success" }; } +/** Maps a failed move to something the user can act on. */ +function describeMoveError(error: unknown): { + title: string; + type: "warning" | "error"; +} { + // The write may have landed, so the original was deliberately kept. + if (error instanceof IndeterminateMoveError) { + return { + title: + "Could not confirm the move. Check the destination folder before trying again — the original has been left in place.", + type: "warning", + }; + } + + const status = getHttpStatus(error); + + switch (status) { + case 401: + case 403: + return { + title: + "You do not have permission to write to that folder. Nothing was moved.", + type: "error", + }; + case 404: + return { + title: + "The file or the destination folder no longer exists. Nothing was moved.", + type: "error", + }; + case 409: + case 412: + return { + title: + "Something else took that name just now. Try again. Nothing was moved.", + type: "error", + }; + case 413: + return { + title: "The file is too large for that destination. Nothing was moved.", + type: "error", + }; + case 415: + return { + title: + "The server rejected the file's content type. Nothing was moved.", + type: "error", + }; + case 507: + return { + title: "There is not enough space in that storage. Nothing was moved.", + type: "error", + }; + } + + if (status === undefined) { + return { + title: "Could not reach the server. Nothing was moved.", + type: "error", + }; + } + + return { + title: + error instanceof Error + ? `Failed to move: ${error.message}` + : "Failed to move file", + type: "error", + }; +} + export default function MoveDialog({ isOpen, onClose, @@ -177,11 +250,17 @@ export default function MoveDialog({ return; } + let fetchFn: typeof fetch; + try { + ({ fetch: fetchFn } = getAuthenticatedSession()); + } catch { + toast.add({ title: "Not authenticated", type: "error" }); + return; + } + setIsMoving(true); try { - const { fetch: fetchFn } = getAuthenticatedSession(); - // Ask before transferring anything, so a clash does not cost a round trip // of the file's bytes. The server's own 409 is still the backstop. const conflict = await findMoveConflict( @@ -219,11 +298,7 @@ export default function MoveDialog({ } } catch (error) { console.error("Failed to move file:", error); - toast.add({ - title: error instanceof Error - ? `Failed to move: ${error.message}` - : "Failed to move file", type: "error" - }); + toast.add(describeMoveError(error)); } finally { setIsMoving(false); } From c76b824445394fe432e97bf7d513e136971c1683 Mon Sep 17 00:00:00 2001 From: Precious Oritsedere Date: Fri, 16 Oct 2026 15:25:03 +0100 Subject: [PATCH 11/12] Move the move-result messages out of the dialog --- app/components/MoveDialog.tsx | 111 +------------------------------- app/lib/helpers/index.ts | 1 + app/lib/helpers/moveMessages.ts | 108 +++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 109 deletions(-) create mode 100644 app/lib/helpers/moveMessages.ts diff --git a/app/components/MoveDialog.tsx b/app/components/MoveDialog.tsx index c6118b4..fa6ce12 100644 --- a/app/components/MoveDialog.tsx +++ b/app/components/MoveDialog.tsx @@ -20,11 +20,11 @@ import { findMoveConflict, getAuthenticatedSession, getParentContainerUrl, - getHttpStatus, ensureTrailingSlash, folderUrlsEqual, fetchContainerListing, - IndeterminateMoveError, + describeMoveError, + describeMoveResult, type MoveResult, } from "../lib/helpers"; import { useNameConflictPrompt } from "./useNameConflictPrompt"; @@ -40,113 +40,6 @@ interface MoveDialogProps { onMoved?: (destinationUrl: string) => void; } -/** - * Describes what actually happened, which is not always what was asked for: - * the server may rename the resource, ignore a rename, or refuse to let go of - * the original. - */ -function describeMoveResult(result: MoveResult): { - title: string; - type: "success" | "warning" | "error"; -} { - // Worst case first: the copy landed but the pod now holds two of them. - if (!result.sourceRemoved) { - return { - title: `Copied "${result.name}", but the original could not be removed. Both copies now exist.`, - type: "error", - }; - } - - if (result.name === result.requestedName) { - return { title: `Moved "${result.name}"`, type: "success" }; - } - - // A rename was asked for and the old name stuck, so the move landed but the - // rename did not. - if ( - result.requestedName !== result.originalName && - result.name === result.originalName - ) { - return { - title: `Moved, but the name was not changed. It is still "${result.originalName}".`, - type: "warning", - }; - } - - return { title: `Moved as "${result.name}"`, type: "success" }; -} - -/** Maps a failed move to something the user can act on. */ -function describeMoveError(error: unknown): { - title: string; - type: "warning" | "error"; -} { - // The write may have landed, so the original was deliberately kept. - if (error instanceof IndeterminateMoveError) { - return { - title: - "Could not confirm the move. Check the destination folder before trying again — the original has been left in place.", - type: "warning", - }; - } - - const status = getHttpStatus(error); - - switch (status) { - case 401: - case 403: - return { - title: - "You do not have permission to write to that folder. Nothing was moved.", - type: "error", - }; - case 404: - return { - title: - "The file or the destination folder no longer exists. Nothing was moved.", - type: "error", - }; - case 409: - case 412: - return { - title: - "Something else took that name just now. Try again. Nothing was moved.", - type: "error", - }; - case 413: - return { - title: "The file is too large for that destination. Nothing was moved.", - type: "error", - }; - case 415: - return { - title: - "The server rejected the file's content type. Nothing was moved.", - type: "error", - }; - case 507: - return { - title: "There is not enough space in that storage. Nothing was moved.", - type: "error", - }; - } - - if (status === undefined) { - return { - title: "Could not reach the server. Nothing was moved.", - type: "error", - }; - } - - return { - title: - error instanceof Error - ? `Failed to move: ${error.message}` - : "Failed to move file", - type: "error", - }; -} - export default function MoveDialog({ isOpen, onClose, diff --git a/app/lib/helpers/index.ts b/app/lib/helpers/index.ts index dd929a9..f9ea8c7 100644 --- a/app/lib/helpers/index.ts +++ b/app/lib/helpers/index.ts @@ -19,3 +19,4 @@ export * from "./httpErrorUtils"; export * from "./containerListingUtils"; export * from "./slugUtils"; export * from "./moveUtils"; +export * from "./moveMessages"; diff --git a/app/lib/helpers/moveMessages.ts b/app/lib/helpers/moveMessages.ts new file mode 100644 index 0000000..e4fd69b --- /dev/null +++ b/app/lib/helpers/moveMessages.ts @@ -0,0 +1,108 @@ +import { getHttpStatus } from "./httpErrorUtils"; +import { IndeterminateMoveError, type MoveResult } from "./moveUtils"; + +export interface MoveMessage { + title: string; + type: "success" | "warning" | "error"; +} + +/** + * Describes what actually happened, which is not always what was asked for; + * the server may rename the resource, ignore a rename, or refuse to let go of + * the original. + */ +export function describeMoveResult(result: MoveResult): MoveMessage { + // Worst case first: the copy landed but the pod now holds two of them. + if (!result.sourceRemoved) { + return { + title: `Copied "${result.name}", but the original could not be removed. Both copies now exist.`, + type: "error", + }; + } + + if (result.name === result.requestedName) { + return { title: `Moved "${result.name}"`, type: "success" }; + } + + // A rename was asked for and the old name stuck, so the move landed but the + // rename did not. + if ( + result.requestedName !== result.originalName && + result.name === result.originalName + ) { + return { + title: `Moved, but the name was not changed. It is still "${result.originalName}".`, + type: "warning", + }; + } + + return { title: `Moved as "${result.name}"`, type: "success" }; +} + +/** Maps a failed move to something the user can act on. */ +export function describeMoveError(error: unknown): MoveMessage { + // The write may have landed, so the original was deliberately kept. + if (error instanceof IndeterminateMoveError) { + return { + title: + "Could not confirm the move. Check the destination folder before trying again — the original has been left in place.", + type: "warning", + }; + } + + const status = getHttpStatus(error); + + switch (status) { + case 401: + case 403: + return { + title: + "You do not have permission to write to that folder. Nothing was moved.", + type: "error", + }; + case 404: + return { + title: + "The file or the destination folder no longer exists. Nothing was moved.", + type: "error", + }; + case 409: + case 412: + return { + title: + "Something else took that name just now. Try again. Nothing was moved.", + type: "error", + }; + case 413: + return { + title: "The file is too large for that destination. Nothing was moved.", + type: "error", + }; + case 415: + return { + title: + "The server rejected the file's content type. Nothing was moved.", + type: "error", + }; + case 507: + return { + title: "There is not enough space in that storage. Nothing was moved.", + type: "error", + }; + } + + if (status === undefined) { + return { + title: "Could not reach the server. Nothing was moved.", + type: "error", + }; + } + + return { + title: + error instanceof Error + ? `Failed to move: ${error.message}` + : "Failed to move file", + type: "error", + }; +} \ No newline at end of file From 38d27add18f74e72790b6ae3d80b338a315192ea Mon Sep 17 00:00:00 2001 From: Precious Oritsedere Date: Mon, 19 Oct 2026 09:12:39 +0100 Subject: [PATCH 12/12] add eol --- app/lib/helpers/moveMessages.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/lib/helpers/moveMessages.ts b/app/lib/helpers/moveMessages.ts index e4fd69b..a696fcf 100644 --- a/app/lib/helpers/moveMessages.ts +++ b/app/lib/helpers/moveMessages.ts @@ -105,4 +105,4 @@ export function describeMoveError(error: unknown): MoveMessage { : "Failed to move file", type: "error", }; -} \ No newline at end of file +}