From 7697f3d35b5e3f82afdd80b4d2d8d7a9d9c7ffcb Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sat, 9 May 2026 00:06:30 +0200 Subject: [PATCH] feat(ui): handle errors on folder creation or folder/file renaming - protect file_management_service::rename_file with validate_storage_name - remove specific rename modal and use the generic modal class (less duplicate) - handle errors on modal action: do not close the modal on error and display this error - hide "Go to parent folder" contextMenu if section is files and folder is the same as current one --- .../services/file_management_service.rs | 7 + static/css/components/dialogs.css | 4 + static/css/components/modals.css | 10 ++ static/index.html | 1 + static/js/app/main.js | 7 +- static/js/app/ui.js | 23 ---- static/js/core/modal.js | 87 ++++++++++-- static/js/features/files/contextMenus.js | 129 +++--------------- static/js/features/files/fileOperations.js | 66 +++++---- 9 files changed, 160 insertions(+), 174 deletions(-) diff --git a/src/application/services/file_management_service.rs b/src/application/services/file_management_service.rs index 0d959eac..1c4bf638 100644 --- a/src/application/services/file_management_service.rs +++ b/src/application/services/file_management_service.rs @@ -6,6 +6,7 @@ use crate::application::ports::storage_ports::{CopyFolderTreeResult, FileReadPor use crate::application::ports::trash_ports::TrashUseCase; use crate::application::services::trash_service::TrashService; use crate::common::errors::DomainError; +use crate::domain::services::path_service::validate_storage_name; use crate::infrastructure::repositories::pg::file_blob_read_repository::FileBlobReadRepository; use crate::infrastructure::repositories::pg::file_blob_write_repository::FileBlobWriteRepository; use crate::infrastructure::repositories::pg::folder_db_repository::FolderDbRepository; @@ -189,6 +190,12 @@ impl FileManagementUseCase for FileManagementService { } async fn rename_file(&self, file_id: &str, new_name: &str) -> Result { + if let Err(reason) = validate_storage_name(new_name) { + return Err(DomainError::validation_error(format!( + "Invalid file name '{new_name}': {reason}" + ))); + } + info!("Renaming file with ID: {} to \"{}\"", file_id, new_name); let renamed_file = self diff --git a/static/css/components/dialogs.css b/static/css/components/dialogs.css index 284b2724..4aec2cfb 100644 --- a/static/css/components/dialogs.css +++ b/static/css/components/dialogs.css @@ -78,6 +78,10 @@ box-shadow: 0 0 0 3px var(--color-accent-ring); } +.rename-dialog input--error { + border-color: var(--color-error-text); +} + .rename-dialog-buttons { display: flex; justify-content: flex-end; diff --git a/static/css/components/modals.css b/static/css/components/modals.css index 570fad38..a69fdd58 100644 --- a/static/css/components/modals.css +++ b/static/css/components/modals.css @@ -253,6 +253,16 @@ color: var(--color-text-placeholder); } +.modal-input--error { + border-color: var(--color-error-text); +} + +.modal-error { + margin-top: 8px; + font-size: 13px; + color: var(--color-error-text); +} + .modal-footer { display: flex; justify-content: flex-end; diff --git a/static/index.html b/static/index.html index 3d5037de..fdfea9b0 100644 --- a/static/index.html +++ b/static/index.html @@ -258,6 +258,7 @@
+
diff --git a/static/js/app/main.js b/static/js/app/main.js index 4042e07c..d0cb5bc2 100644 --- a/static/js/app/main.js +++ b/static/js/app/main.js @@ -203,10 +203,9 @@ function setupActionsBarDelegation() { break; } case 'new-folder-btn': { - const folderName = await Modal.promptNewFolder(); - if (folderName) { - fileOps.createFolder(folderName); - } + await Modal.promptNewFolder(async (name) => { + await fileOps.createFolder(name); + }); break; } case 'grid-view-btn': diff --git a/static/js/app/ui.js b/static/js/app/ui.js index f8cb49bc..10bf748a 100644 --- a/static/js/app/ui.js +++ b/static/js/app/ui.js @@ -112,29 +112,6 @@ const ui = { i18n.translateElement(fileMenu); } - // Rename dialog — modern - if (!document.getElementById('rename-dialog')) { - const renameDialog = document.createElement('div'); - renameDialog.classList.add('rename-dialog', 'hidden'); - renameDialog.id = 'rename-dialog'; - renameDialog.innerHTML = ` -
-
- - Rename -
-
- -
-
- - -
-
- `; - document.body.appendChild(renameDialog); - } - // Move dialog — modern with navigation if (!document.getElementById('move-file-dialog')) { const moveDialog = document.createElement('div'); diff --git a/static/js/core/modal.js b/static/js/core/modal.js index c29e17cf..02770c31 100644 --- a/static/js/core/modal.js +++ b/static/js/core/modal.js @@ -31,6 +31,12 @@ const Modal = { onConfirm: null, onCancel: null, + /** @private @type {((value: string) => Promise) | null} */ + _action: null, + + /** @private @type {HTMLElement | null} */ + errorEl: null, + // Rename mode: select only name without extension _selectNameOnly: false, @@ -53,6 +59,8 @@ const Modal = { this.closeBtn = document.getElementById('modal-close-btn'); // Event listeners + this.errorEl = document.getElementById('modal-error'); + this.cancelBtn?.addEventListener('click', () => this.close(false)); this.closeBtn?.addEventListener('click', () => this.close(false)); this.confirmBtn?.addEventListener('click', () => this.confirm()); @@ -64,6 +72,9 @@ const Modal = { } }); + // Clear inline error as soon as the user starts typing + this.input?.addEventListener('input', () => this.clearError()); + // Handle Enter and Escape keys this.input?.addEventListener('keydown', (e) => { if (e.key === 'Enter') { @@ -75,6 +86,23 @@ const Modal = { }); }, + /** @param {string} message */ + showError(message) { + this.input?.classList.add('modal-input--error'); + if (this.errorEl) { + this.errorEl.textContent = message; + this.errorEl.classList.remove('hidden'); + } + }, + + clearError() { + this.input?.classList.remove('modal-input--error'); + if (this.errorEl) { + this.errorEl.textContent = ''; + this.errorEl.classList.add('hidden'); + } + }, + /** * Show input modal (replacement for prompt()) * @param {Object} options - Modal configuration @@ -85,11 +113,23 @@ const Modal = { * @param {string} options.icon - Font Awesome icon class (e.g., 'fa-folder-plus') * @param {string} options.confirmText - Confirm button text * @param {string} options.cancelText - Cancel button text + * @param {(value: string) => Promise} [options.action] - Async action called on confirm. + * Throw an Error to keep the modal open and display the error message inline. + * When omitted the modal resolves immediately with the input value (legacy behaviour). * @returns {Promise} - Resolves with input value or null if cancelled */ prompt(options = {}) { return new Promise((resolve) => { - const { title = 'Input', label = '', placeholder = '', value = '', icon = 'fa-keyboard', confirmText = null, cancelText = null } = options; + const { + title = 'Input', + label = '', + placeholder = '', + value = '', + icon = 'fa-keyboard', + confirmText = null, + cancelText = null, + action = null + } = options; // Set modal content - update the icon const iconContainer = document.querySelector('.modal-icon'); @@ -120,6 +160,9 @@ const Modal = { this.cancelBtn.textContent = i18n.t('actions.cancel'); } + this._action = action; + this.clearError(); + // Set callbacks this.onConfirm = () => { const inputValue = this.input.value.trim(); @@ -134,15 +177,17 @@ const Modal = { /** * Show modal for creating new folder + * @param {(value: string) => Promise} [action] * @returns {Promise} */ - promptNewFolder() { + promptNewFolder(action = null) { return this.prompt({ title: i18n.t('dialogs.new_folder_title'), label: i18n.t('dialogs.folder_name'), placeholder: i18n.t('dialogs.folder_placeholder'), icon: 'fa-folder-plus', - confirmText: i18n.t('actions.create') + confirmText: i18n.t('actions.create'), + action }); }, @@ -150,9 +195,10 @@ const Modal = { * Show modal for renaming * @param {string} currentName - Current name of file/folder * @param {boolean} isFolder - Whether it's a folder + * @param {(value: string) => Promise} [action] * @returns {Promise} */ - promptRename(currentName, isFolder = false) { + promptRename(currentName, isFolder = false, action = null) { this._selectNameOnly = !isFolder; return this.prompt({ @@ -161,7 +207,8 @@ const Modal = { placeholder: '', value: currentName, icon: isFolder ? 'fa-folder' : 'fa-file', - confirmText: i18n.t('actions.rename') + confirmText: i18n.t('actions.rename'), + action }); }, @@ -206,6 +253,8 @@ const Modal = { close(confirmed = false) { if (!this.overlay) return; + this.clearError(); + this._action = null; this.overlay.classList.remove('active'); setTimeout(() => { @@ -222,13 +271,31 @@ const Modal = { }, /** - * Confirm the action + * Confirm the action. When an async action is set, the modal stays open + * until it resolves — closing only on success, showing the error inline on failure. */ - confirm() { - if (this.onConfirm) { - this.onConfirm(); + async confirm() { + if (!this._action) { + if (this.onConfirm) this.onConfirm(); + this.close(true); + return; + } + + const inputValue = this.input.value.trim(); + if (!inputValue) return; + + this.clearError(); + this.confirmBtn.disabled = true; + + try { + await this._action(inputValue); + if (this.onConfirm) this.onConfirm(); + this.close(true); + } catch (e) { + this.showError(e.message || 'An error occurred'); + this.confirmBtn.disabled = false; + this.input.focus(); } - this.close(true); } }; diff --git a/static/js/features/files/contextMenus.js b/static/js/features/files/contextMenus.js index bd47d5db..c25145b5 100644 --- a/static/js/features/files/contextMenus.js +++ b/static/js/features/files/contextMenus.js @@ -11,6 +11,7 @@ import { showConfirmDialog, ui } from '../../app/ui.js'; import { getCsrfHeaders } from '../../core/csrf.js'; import { escapeHtml } from '../../core/formatters.js'; import { i18n } from '../../core/i18n.js'; +import { Modal } from '../../core/modal.js'; import { favorites } from '../library/favorites.js'; import { musicView } from '../library/music.js'; import { fileSharing } from '../sharing/fileSharing.js'; @@ -69,7 +70,8 @@ const contextMenus = { const option = document.getElementById('open-parent-folder-option'); if (!option) return; const folderId = app?.contextMenuTargetFile?.folder_id; - option.classList.toggle('hidden', !folderId); + const alreadyViewing = folderId && folderId === app?.currentPath; + option.classList.toggle('hidden', !folderId || alreadyViewing); }, syncAddToPlaylistOption() { @@ -120,11 +122,14 @@ const contextMenus = { ui.closeContextMenu(); }); - document.getElementById('rename-folder-option').addEventListener('click', () => { - if (app.contextMenuTargetFolder) { - this.showRenameDialog(app.contextMenuTargetFolder); - } + document.getElementById('rename-folder-option').addEventListener('click', async () => { + const folder = app.contextMenuTargetFolder; ui.closeContextMenu(); + if (!folder) return; + const newName = await Modal.promptRename(folder.name, true, async (name) => { + await fileOps.renameFolder(folder.id, name); + }); + if (newName) loadFiles(); }); document.getElementById('move-folder-option').addEventListener('click', () => { @@ -239,11 +244,14 @@ const contextMenus = { ui.closeFileContextMenu(); }); - document.getElementById('rename-file-option').addEventListener('click', () => { - if (app.contextMenuTargetFile) { - this.showRenameFileDialog(app.contextMenuTargetFile); - } + document.getElementById('rename-file-option').addEventListener('click', async () => { + const file = app.contextMenuTargetFile; ui.closeFileContextMenu(); + if (!file) return; + const newName = await Modal.promptRename(file.name, false, async (name) => { + await fileOps.renameFile(file.id, name); + }); + if (newName) loadFiles(); }); document.getElementById('move-file-option').addEventListener('click', () => { @@ -281,23 +289,6 @@ const contextMenus = { } }); - // Rename dialog events - const renameCancelBtn = document.getElementById('rename-cancel-btn'); - const renameConfirmBtn = document.getElementById('rename-confirm-btn'); - const renameInput = document.getElementById('rename-input'); - - renameCancelBtn.addEventListener('click', this.closeRenameDialog); - renameConfirmBtn.addEventListener('click', () => contextMenus.renameItem()); - - // Rename on Enter key - renameInput.addEventListener('keyup', (e) => { - if (e.key === 'Enter') { - contextMenus.renameItem(); - } else if (e.key === 'Escape') { - this.closeRenameDialog(); - } - }); - // Move dialog events const moveCancelBtn = document.getElementById('move-cancel-btn'); const moveConfirmBtn = document.getElementById('move-confirm-btn'); @@ -386,55 +377,6 @@ const contextMenus = { }); }, - /** - * Show rename dialog for a folder - * @param {Object} folder - Folder object - */ - showRenameDialog(folder) { - const renameInput = document.getElementById('rename-input'); - const renameDialog = document.getElementById('rename-dialog'); - - app.renameMode = 'folder'; - // Store the folder reference so it survives context menu cleanup - app.renameTarget = folder; - renameInput.value = folder.name; - // Update header text - const headerSpan = renameDialog.querySelector('.rename-dialog-header span'); - if (headerSpan) headerSpan.textContent = i18n.t('dialogs.rename_folder'); - renameDialog?.classList.remove('hidden'); - renameInput.focus(); - renameInput.select(); - }, - - /** - * Show rename dialog for a file - * @param {Object} file - File object - */ - showRenameFileDialog(file) { - const renameInput = document.getElementById('rename-input'); - const renameDialog = document.getElementById('rename-dialog'); - - app.renameMode = 'file'; - // Store the file reference so it survives context menu cleanup - app.renameTarget = file; - renameInput.value = file.name; - // Update header text - const headerSpan = renameDialog.querySelector('.rename-dialog-header span'); - if (headerSpan) headerSpan.textContent = i18n.t('dialogs.rename_file'); - renameDialog?.classList.remove('hidden'); - renameInput.focus(); - renameInput.select(); - }, - - /** - * Close rename dialog - */ - closeRenameDialog() { - document.getElementById('rename-dialog')?.classList.add('hidden'); - app.contextMenuTargetFolder = null; - app.renameTarget = null; - }, - /** * Show move dialog for a file or folder * @param {Object} item - File or folder object @@ -508,43 +450,6 @@ const contextMenus = { app.contextMenuTargetFolder = null; }, - /** - * Rename the selected folder or file - */ - async renameItem() { - const newName = document.getElementById('rename-input').value.trim(); - if (!newName) { - alert(i18n.t('errors.empty_name')); - return; - } - - // Use renameTarget which was saved before the context menu was closed - const target = app.renameTarget; - if (!target) { - console.error('No rename target available'); - return; - } - - if (app.renameMode === 'file') { - const success = await fileOps.renameFile(target.id, newName); - if (success) { - contextMenus.closeRenameDialog(); - loadFiles(); - } - } else if (app.renameMode === 'folder') { - const success = await fileOps.renameFolder(target.id, newName); - if (success) { - contextMenus.closeRenameDialog(); - loadFiles(); - } - } - }, - - // Keep backward compat - renameFolder() { - return contextMenus.renameItem(); - }, - /** * Load folders for the move dialog with navigation support * Shows subfolders of the specified parent folder and allows navigation diff --git a/static/js/features/files/fileOperations.js b/static/js/features/files/fileOperations.js index 7296b3c8..3afcaf69 100644 --- a/static/js/features/files/fileOperations.js +++ b/static/js/features/files/fileOperations.js @@ -685,6 +685,10 @@ const fileOps = { } }, + /** + * Create a new folder + * @param {string} name - Folder name + */ /** * Create a new folder * @param {string} name - Folder name @@ -718,13 +722,20 @@ const fileOps = { ui.showNotification('Folder created', `"${name}" created successfully`); } else { - const errorData = await response.text(); - console.error('Create folder error:', errorData); - ui.showNotification('Error', 'Error creating the folder'); + const errorText = await response.text(); + console.error('Create folder error:', errorText); + let errorMessage = 'Unknown error'; + try { + const errorData = JSON.parse(errorText); + errorMessage = errorData.error || response.statusText; + } catch (_e) { + errorMessage = errorText || response.statusText; + } + throw new Error(errorMessage); } } catch (error) { console.error('Error creating folder:', error); - ui.showNotification('Error', 'Error creating the folder'); + throw error; } }, @@ -978,6 +989,12 @@ const fileOps = { * @param {string} newName - New file name * @returns {Promise} - Success status */ + /** + * Rename a file + * @param {string} fileId - File ID + * @param {string} newName - New file name + * @returns {Promise} - null on success, error message string on failure + */ async renameFile(fileId, newName) { try { console.log(`Renaming file ${fileId} to "${newName}"`); @@ -995,24 +1012,22 @@ const fileOps = { if (response.ok) { ui.showNotification(i18n.t('notifications.file_renamed'), i18n.t('notifications.file_renamed_to', { name: newName })); - return true; } else { const errorText = await response.text(); console.error('Error response:', errorText); - let errorMessage = 'Unknown error'; try { const errorData = JSON.parse(errorText); - errorMessage = errorData.error || response.statusText; - } catch (_e) { - errorMessage = errorText || response.statusText; + throw new Error(errorData.error || response.statusText); + } catch (parseError) { + if (parseError instanceof SyntaxError) { + throw new Error(errorText || response.statusText); + } + throw parseError; } - ui.showNotification('Error', `Error renaming the file: ${errorMessage}`); - return false; } } catch (error) { console.error('Error renaming file:', error); - ui.showNotification('Error', 'Error renaming the file'); - return false; + throw error; } }, @@ -1022,6 +1037,12 @@ const fileOps = { * @param {string} newName - New folder name * @returns {Promise} - Success status */ + /** + * Rename a folder + * @param {string} folderId - Folder ID + * @param {string} newName - New folder name + * @returns {Promise} - null on success, error message string on failure + */ async renameFolder(folderId, newName) { try { console.log(`Renaming folder ${folderId} to "${newName}"`); @@ -1039,28 +1060,23 @@ const fileOps = { if (response.ok) { ui.showNotification('Folder renamed', `Folder renamed to "${newName}"`); - return true; } else { const errorText = await response.text(); console.error('Error response:', errorText); - - let errorMessage = 'Unknown error'; try { // Try to parse as JSON const errorData = JSON.parse(errorText); - errorMessage = errorData.error || response.statusText; - } catch (_e) { - // If not JSON, use text as is - errorMessage = errorText || response.statusText; + throw new Error(errorData.error || response.statusText); + } catch (parseError) { + if (parseError instanceof SyntaxError) { + throw new Error(errorText || response.statusText); + } + throw parseError; } - - ui.showNotification('Error', `Error renaming the folder: ${errorMessage}`); - return false; } } catch (error) { console.error('Error renaming folder:', error); - ui.showNotification('Error', 'Error renaming the folder'); - return false; + throw error; } },