mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-10-08 02:22:12 -03:00
feat(sidebar): manage folders that live under several model roots
The sidebar's folder tree merges every model root into one relative-path
namespace, but folder operations turned a node into a path by prefixing
default_*_root. A folder living under another root failed to delete with
"Folder no longer exists" (recipes under the primary lora root while
default_lora_root is the extra one), and where the same relative folder
exists in both roots the operation silently hit the other copy — 14 of the
16 top-level folders in the reporting library are shared, so guessing a root
was never safe.
Backend:
* ModelMoveService.resolve_folder() and GET /api/lm/{prefix}/resolve-folder
answer which directories a library-relative folder maps to
(folder_path/root/is_symlink), in scanner root order, skipping directories
no root holds and refusing absolute or climbing paths.
* delete_folder/rename_folder tag a vanished directory with code "missing"
so the sidebar can tell "this node is stale, refresh" from a failed
operation.
Frontend:
* _resolveFolderCandidates() is the single place that turns a node into
absolute paths: default root first, the old root-prefix fallback only
while a single root is configured, and an explicit unresolved error for a
multi-root library — nothing is guessed silently any more.
* One copy keeps the single-target modal, which now names the resolved
absolute path. Several copies render one checkbox row per copy, each
dry-run against the delete guard ("no models" / "contains N model
file(s)..." / a deletion is still pending / no longer exists / symbolic
link): a blocked copy is unticked, disabled and explained, the button
reads "Delete N folders", every ticked copy is deleted and guarded on its
own, and a partial failure is reported without discarding the successes.
* Rows are built once per open and only their status text is updated, so
ticking a box no longer rebuilds the list, steals focus or resizes the
modal mid-click; the action row keeps a fixed button width.
* Rename offers a root picker in its inline row, create inherits the
parent's root when the parent resolves to exactly one directory, and the
undo restores every copy a delete removed.
i18n: 26 new keys (sidebar.deleteFolderModal.*, .deleteFolderResult.*,
.renameFolderResult.*, .folderRoot.*, .folderResult.*) translated in all 9
locales, with the en wording normalized to the established "model root" noun
(it had said "library root") and the new terminology recorded in
docs/i18n-translation-guidelines.md.
Verified: pytest 3686 passed, vitest 1473 passed (86 in the folder-management
suite), pytest tests/i18n 20 passed, sync_translation_keys.py --dry-run
clean. A sandboxed standalone instance with two roots confirmed that deleting
one copy leaves the node in place, that the twin's model card survives the
purge, and that deleting both copies and undoing restores both directories.
This commit is contained in:
@@ -50,6 +50,17 @@ function createApiClient(overrides = {}) {
|
||||
fetchUnifiedFolderTree: vi.fn().mockResolvedValue({ tree: { full: {}, empty: {} } }),
|
||||
fetchModelFolders: vi.fn().mockResolvedValue({ folders: ['', 'full'] }),
|
||||
fetchModelRoots: vi.fn().mockResolvedValue({ roots: ['/models/loras'] }),
|
||||
// Mirrors the backend resolver: a single-root library answers with the one
|
||||
// directory the relative folder maps to.
|
||||
resolveFolder: vi.fn((folder) => Promise.resolve({
|
||||
success: true,
|
||||
folder,
|
||||
candidates: [{
|
||||
folder_path: `/models/loras/${folder}`,
|
||||
root: '/models/loras',
|
||||
is_symlink: false,
|
||||
}],
|
||||
})),
|
||||
createFolder: vi.fn().mockResolvedValue({ success: true, folder: 'new-folder', created: true }),
|
||||
deleteFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
@@ -451,6 +462,33 @@ describe('SidebarManager folder creation', () => {
|
||||
expect(manager.refresh).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('creates inside the parent root even when it is not the default root', async () => {
|
||||
const apiClient = createApiClient({
|
||||
fetchModelRoots: vi.fn().mockResolvedValue({
|
||||
roots: ['/models/loras', '/models/extra'],
|
||||
}),
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'recipes',
|
||||
candidates: [{
|
||||
folder_path: '/models/loras/recipes',
|
||||
root: '/models/loras',
|
||||
is_symlink: false,
|
||||
}],
|
||||
}),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
state.global.settings = { default_lora_root: '/models/extra' };
|
||||
|
||||
const success = await manager._createFolder('recipes/presets', 'recipes');
|
||||
|
||||
// A new folder belongs next to the node it was created from; only a
|
||||
// root-level creation falls back to the configured default root.
|
||||
expect(success).toBe(true);
|
||||
expect(apiClient.createFolder).toHaveBeenCalledWith('/models/loras/recipes/presets');
|
||||
});
|
||||
|
||||
it('re-enables empty folders when creating while the preference is off', async () => {
|
||||
const apiClient = createApiClient();
|
||||
const manager = createManager(apiClient);
|
||||
@@ -613,6 +651,7 @@ describe('SidebarManager folder deletion', () => {
|
||||
<h2 data-role="title"></h2>
|
||||
<p class="delete-message" data-role="message"></p>
|
||||
<div class="delete-model-info" data-role="info"></div>
|
||||
<div class="delete-folder-roots" data-role="roots"></div>
|
||||
<div class="modal-actions">
|
||||
<button class="cancel-btn" data-action="cancel-delete-folder">Cancel</button>
|
||||
<button class="delete-btn" data-action="confirm-delete-folder">Delete folder</button>
|
||||
@@ -624,6 +663,14 @@ describe('SidebarManager folder deletion', () => {
|
||||
return document.querySelector('#deleteFolderModal [data-action="confirm-delete-folder"]');
|
||||
}
|
||||
|
||||
function rootChoices() {
|
||||
return [...document.querySelectorAll('#deleteFolderModal input[name="deleteFolderRoot"]')];
|
||||
}
|
||||
|
||||
function keptNote() {
|
||||
return document.querySelector('#deleteFolderModal .folder-root-kept-note');
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
localStorage.clear();
|
||||
document.body.innerHTML = MODAL_HTML;
|
||||
@@ -642,10 +689,14 @@ describe('SidebarManager folder deletion', () => {
|
||||
expect(modal.dataset.state).toBe('confirm');
|
||||
expect(confirmBtn().style.display).toBe('');
|
||||
expect(confirmBtn().disabled).toBe(false);
|
||||
expect(manager._pendingDeleteFolderPath).toBe('empty');
|
||||
// The pending target is the resolved directory, never the bare tree path:
|
||||
// the tree path is relative to a root this node may not even live under.
|
||||
expect(manager._pendingDeleteFolderRelative).toBe('empty');
|
||||
expect(manager._pendingDeleteFolderPath).toBe('/models/loras/empty');
|
||||
expect(modalManager.showModal).toHaveBeenCalledWith('deleteFolderModal');
|
||||
// The prediction is confirmed against the real guard before the user can
|
||||
// act on it.
|
||||
expect(apiClient.resolveFolder).toHaveBeenCalledWith('empty');
|
||||
expect(apiClient.deleteFolder).toHaveBeenCalledWith(
|
||||
'/models/loras/empty', { dryRun: true }
|
||||
);
|
||||
@@ -746,7 +797,7 @@ describe('SidebarManager folder deletion', () => {
|
||||
it('keeps the confirm button disabled until the check settles', async () => {
|
||||
let release;
|
||||
const apiClient = createApiClient({
|
||||
fetchModelRoots: vi.fn(() => new Promise((resolve) => { release = resolve; })),
|
||||
resolveFolder: vi.fn(() => new Promise((resolve) => { release = resolve; })),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.nonEmptyFolders = new Set(['', 'full']);
|
||||
@@ -754,7 +805,15 @@ describe('SidebarManager folder deletion', () => {
|
||||
const pending = manager.showDeleteFolderModal('empty');
|
||||
expect(confirmBtn().disabled).toBe(true);
|
||||
|
||||
release({ roots: ['/models/loras'] });
|
||||
release({
|
||||
success: true,
|
||||
folder: 'empty',
|
||||
candidates: [{
|
||||
folder_path: '/models/loras/empty',
|
||||
root: '/models/loras',
|
||||
is_symlink: false,
|
||||
}],
|
||||
});
|
||||
await pending;
|
||||
|
||||
expect(confirmBtn().disabled).toBe(false);
|
||||
@@ -920,13 +979,386 @@ describe('SidebarManager folder deletion', () => {
|
||||
it('routes the modal buttons to cancel and confirm', () => {
|
||||
const manager = createManager(createApiClient());
|
||||
manager._deleteFolder = vi.fn().mockResolvedValue(true);
|
||||
manager._pendingDeleteFolderPath = 'empty';
|
||||
manager._pendingDeleteFolderRelative = 'empty';
|
||||
manager._pendingDeleteFolderPath = '/models/loras/empty';
|
||||
manager._wireDeleteFolderModal();
|
||||
|
||||
confirmBtn().dispatchEvent(new MouseEvent('click', { bubbles: true }));
|
||||
|
||||
expect(modalManager.closeModal).toHaveBeenCalledWith('deleteFolderModal');
|
||||
expect(manager._deleteFolder).toHaveBeenCalledWith('empty');
|
||||
expect(manager._deleteFolder).toHaveBeenCalledWith('empty', '/models/loras/empty');
|
||||
});
|
||||
|
||||
it('deletes the directory the node actually lives in, not the default root one', async () => {
|
||||
// The reported bug: the node exists under the primary root while
|
||||
// default_lora_root points at the extra root, so prefixing the default root
|
||||
// produced a path that did not exist and the delete always failed.
|
||||
const apiClient = createApiClient({
|
||||
fetchModelRoots: vi.fn().mockResolvedValue({
|
||||
roots: ['/models/loras', '/models/extra'],
|
||||
}),
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'recipes',
|
||||
candidates: [{
|
||||
folder_path: '/models/loras/recipes',
|
||||
root: '/models/loras',
|
||||
is_symlink: false,
|
||||
}],
|
||||
}),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
state.global.settings = { default_lora_root: '/models/extra' };
|
||||
|
||||
await manager.showDeleteFolderModal('recipes');
|
||||
|
||||
const modal = document.getElementById('deleteFolderModal');
|
||||
expect(modal.dataset.state).toBe('confirm');
|
||||
expect(apiClient.deleteFolder).toHaveBeenCalledWith(
|
||||
'/models/loras/recipes', { dryRun: true }
|
||||
);
|
||||
// The absolute path is shown, so the modal names the directory it targets.
|
||||
expect(modal.querySelector('[data-role="info"]').textContent)
|
||||
.toContain('/models/loras/recipes');
|
||||
|
||||
await manager.handleDeleteFolderConfirm();
|
||||
|
||||
expect(apiClient.deleteFolder).toHaveBeenLastCalledWith('/models/loras/recipes');
|
||||
});
|
||||
|
||||
it('lists every root holding the folder as a checked row, and deletes only the ticked ones', async () => {
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'Pony',
|
||||
candidates: [
|
||||
{ folder_path: '/models/extra/Pony', root: '/models/extra', is_symlink: false },
|
||||
{ folder_path: '/models/loras/Pony', root: '/models/loras', is_symlink: false },
|
||||
],
|
||||
}),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
state.global.settings = { default_lora_root: '/models/extra' };
|
||||
|
||||
await manager.showDeleteFolderModal('Pony');
|
||||
|
||||
const modal = document.getElementById('deleteFolderModal');
|
||||
const choices = rootChoices();
|
||||
expect(choices.map((input) => input.value)).toEqual([
|
||||
'/models/extra/Pony',
|
||||
'/models/loras/Pony',
|
||||
]);
|
||||
// Both copies are selected up front: the node stands for all of them, and
|
||||
// deleting it one root at a time was the complaint.
|
||||
expect(choices.map((input) => input.checked)).toEqual([true, true]);
|
||||
expect(choices.every((input) => input.type === 'checkbox')).toBe(true);
|
||||
expect(choices.every((input) => input.disabled === false)).toBe(true);
|
||||
expect(confirmBtn().textContent).toContain('2');
|
||||
// Each copy carries its own verdict instead of one shared claim.
|
||||
expect(modal.querySelectorAll('.folder-root-status')).toHaveLength(2);
|
||||
expect([...modal.querySelectorAll('.folder-root-status')].map((el) => el.textContent))
|
||||
.toEqual(['no models', 'no models']);
|
||||
// The absolute paths live on the rows, so the header does not single one out.
|
||||
expect(modal.querySelector('[data-role="info"]').textContent).not.toContain('/models/');
|
||||
expect(apiClient.deleteFolder).toHaveBeenCalledWith('/models/extra/Pony', { dryRun: true });
|
||||
expect(apiClient.deleteFolder).toHaveBeenCalledWith('/models/loras/Pony', { dryRun: true });
|
||||
|
||||
// Unticking one copy keeps the other, and does not rebuild the list.
|
||||
const firstRow = choices[0];
|
||||
const secondRow = choices[1];
|
||||
expect(keptNote().textContent).toBe('');
|
||||
choices[1].checked = false;
|
||||
choices[1].dispatchEvent(new Event('change', { bubbles: true }));
|
||||
|
||||
expect(rootChoices()[0]).toBe(firstRow);
|
||||
expect(rootChoices()[1]).toBe(secondRow);
|
||||
expect(rootChoices().map((input) => input.checked)).toEqual([true, false]);
|
||||
expect(confirmBtn().textContent).not.toContain('2');
|
||||
// The node stays in the sidebar while another root holds the folder, so the
|
||||
// modal says so before the user commits.
|
||||
expect(keptNote().textContent).toBe('Unchecked copies keep this folder in the sidebar.');
|
||||
|
||||
// Unticking everything just disables the button: the user emptied the
|
||||
// selection, the folders are not "blocked".
|
||||
choices[0].checked = false;
|
||||
choices[0].dispatchEvent(new Event('change', { bubbles: true }));
|
||||
|
||||
expect(confirmBtn().disabled).toBe(true);
|
||||
expect(confirmBtn().style.display).toBe('');
|
||||
expect(modal.querySelector('[data-role="title"]').textContent).toBe('Delete folder?');
|
||||
|
||||
choices[0].checked = true;
|
||||
choices[0].dispatchEvent(new Event('change', { bubbles: true }));
|
||||
expect(confirmBtn().disabled).toBe(false);
|
||||
// Re-ticking the copy clears the warning again...
|
||||
choices[1].checked = true;
|
||||
choices[1].dispatchEvent(new Event('change', { bubbles: true }));
|
||||
expect(keptNote().textContent).toBe('');
|
||||
// ... and the copy is left alone for the rest of the test.
|
||||
choices[1].checked = false;
|
||||
choices[1].dispatchEvent(new Event('change', { bubbles: true }));
|
||||
|
||||
await manager.handleDeleteFolderConfirm();
|
||||
|
||||
// Real deletes: only the ticked copy, and both dry runs are distinct calls.
|
||||
expect(apiClient.deleteFolder).toHaveBeenLastCalledWith('/models/extra/Pony');
|
||||
expect(apiClient.deleteFolder).not.toHaveBeenCalledWith('/models/loras/Pony');
|
||||
expect(manager.refresh).toHaveBeenCalledTimes(1);
|
||||
// A single removed copy keeps the singular wording (no "from 1 roots"), but
|
||||
// the toast stays a plain success: the sidebar node's fate is disk truth.
|
||||
expect(showActionToast).toHaveBeenCalledWith(
|
||||
'sidebar.deleteFolderResult.success',
|
||||
{ name: 'Pony' },
|
||||
'success',
|
||||
expect.any(Object)
|
||||
);
|
||||
});
|
||||
|
||||
it('restores only the deleted copy when a kept copy survives the undo', async () => {
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'test',
|
||||
candidates: [
|
||||
{ folder_path: '/models/loras/test', root: '/models/loras', is_symlink: false },
|
||||
{ folder_path: '/models/extra/test', root: '/models/extra', is_symlink: false },
|
||||
],
|
||||
}),
|
||||
deleteFolder: vi.fn().mockResolvedValue({ success: true, folder: 'test', restorable: true }),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
await manager.showDeleteFolderModal('test');
|
||||
const choices = rootChoices();
|
||||
choices[1].checked = false;
|
||||
choices[1].dispatchEvent(new Event('change', { bubbles: true }));
|
||||
|
||||
await manager.handleDeleteFolderConfirm();
|
||||
|
||||
const undo = showActionToast.mock.calls[0][3].onAction;
|
||||
await undo();
|
||||
|
||||
expect(apiClient.createFolder).toHaveBeenCalledWith('/models/loras/test');
|
||||
expect(apiClient.createFolder).not.toHaveBeenCalledWith('/models/extra/test');
|
||||
});
|
||||
|
||||
it('unchecks and explains a copy that still holds models', async () => {
|
||||
const conflict = Object.assign(new Error('still contains models'), {
|
||||
code: 'not_empty',
|
||||
manifest: { model_count: 2, excluded_model_count: 0 },
|
||||
});
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'Pony',
|
||||
candidates: [
|
||||
{ folder_path: '/models/extra/Pony', root: '/models/extra', is_symlink: false },
|
||||
{ folder_path: '/models/loras/Pony', root: '/models/loras', is_symlink: false },
|
||||
],
|
||||
}),
|
||||
deleteFolder: vi.fn((path, options) => (
|
||||
path === '/models/loras/Pony' && options?.dryRun
|
||||
? Promise.reject(conflict)
|
||||
: Promise.resolve({ success: true, folder: 'Pony', restorable: true })
|
||||
)),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
await manager.showDeleteFolderModal('Pony');
|
||||
|
||||
const choices = rootChoices();
|
||||
expect(choices.map((input) => input.checked)).toEqual([true, false]);
|
||||
expect(choices[1].disabled).toBe(true);
|
||||
const statuses = [...document.querySelectorAll('#deleteFolderModal .folder-root-status')];
|
||||
expect(statuses[0].textContent).toBe('no models');
|
||||
expect(statuses[1].textContent).toContain('2 model file(s)');
|
||||
expect(statuses[1].classList.contains('blocked')).toBe(true);
|
||||
expect(confirmBtn().disabled).toBe(false);
|
||||
|
||||
await manager.handleDeleteFolderConfirm();
|
||||
|
||||
// The clean copy is deleted; the blocked one is never asked for a real delete.
|
||||
expect(apiClient.deleteFolder).toHaveBeenLastCalledWith('/models/extra/Pony');
|
||||
expect(apiClient.deleteFolder).not.toHaveBeenCalledWith('/models/loras/Pony');
|
||||
});
|
||||
|
||||
it('blocks the whole modal when no copy can be deleted', async () => {
|
||||
const conflict = Object.assign(new Error('still contains models'), {
|
||||
code: 'not_empty',
|
||||
manifest: { model_count: 1, excluded_model_count: 0 },
|
||||
});
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'Pony',
|
||||
candidates: [
|
||||
{ folder_path: '/models/extra/Pony', root: '/models/extra', is_symlink: false },
|
||||
{ folder_path: '/models/loras/Pony', root: '/models/loras', is_symlink: false },
|
||||
],
|
||||
}),
|
||||
deleteFolder: vi.fn().mockRejectedValue(conflict),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
|
||||
await manager.showDeleteFolderModal('Pony');
|
||||
|
||||
const modal = document.getElementById('deleteFolderModal');
|
||||
expect(confirmBtn().style.display).toBe('none');
|
||||
expect(modal.querySelector('[data-role="title"]').textContent).toBe('Folder is not empty');
|
||||
expect(rootChoices().every((input) => input.disabled)).toBe(true);
|
||||
});
|
||||
|
||||
it('deletes every ticked copy in one action and restores them all on undo', async () => {
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'test',
|
||||
candidates: [
|
||||
{ folder_path: '/models/loras/test', root: '/models/loras', is_symlink: false },
|
||||
{ folder_path: '/models/extra/test', root: '/models/extra', is_symlink: false },
|
||||
],
|
||||
}),
|
||||
deleteFolder: vi.fn().mockResolvedValue({ success: true, folder: 'test', restorable: true }),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
await manager.showDeleteFolderModal('test');
|
||||
await manager.handleDeleteFolderConfirm();
|
||||
|
||||
expect(apiClient.deleteFolder).toHaveBeenCalledWith('/models/loras/test');
|
||||
expect(apiClient.deleteFolder).toHaveBeenCalledWith('/models/extra/test');
|
||||
expect(showActionToast).toHaveBeenCalledWith(
|
||||
'sidebar.deleteFolderResult.successMulti',
|
||||
{ name: 'test', count: 2 },
|
||||
'success',
|
||||
expect.any(Object)
|
||||
);
|
||||
|
||||
const undo = showActionToast.mock.calls[0][3].onAction;
|
||||
await undo();
|
||||
|
||||
expect(apiClient.createFolder).toHaveBeenCalledWith('/models/loras/test');
|
||||
expect(apiClient.createFolder).toHaveBeenCalledWith('/models/extra/test');
|
||||
});
|
||||
|
||||
it('reports the copies it managed to delete when one of them fails', async () => {
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'test',
|
||||
candidates: [
|
||||
{ folder_path: '/models/loras/test', root: '/models/loras', is_symlink: false },
|
||||
{ folder_path: '/models/extra/test', root: '/models/extra', is_symlink: false },
|
||||
],
|
||||
}),
|
||||
deleteFolder: vi.fn((path, options) => {
|
||||
if (options?.dryRun) return Promise.resolve({ success: true });
|
||||
if (path === '/models/extra/test') {
|
||||
return Promise.reject(Object.assign(new Error('gone'), { code: 'missing' }));
|
||||
}
|
||||
return Promise.resolve({ success: true, folder: 'test', restorable: true });
|
||||
}),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
await manager.showDeleteFolderModal('test');
|
||||
const success = await manager.handleDeleteFolderConfirm();
|
||||
|
||||
expect(success).toBe(true);
|
||||
expect(showToast).toHaveBeenCalledWith(
|
||||
'sidebar.deleteFolderResult.partial',
|
||||
{ count: 1, total: 2, failed: 1 },
|
||||
'warning'
|
||||
);
|
||||
expect(manager.refresh).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('reports a stale tree node as missing instead of fabricating a path', async () => {
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true, folder: 'removed', candidates: [],
|
||||
}),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.nonEmptyFolders = new Set(['', 'full']);
|
||||
|
||||
await manager.showDeleteFolderModal('removed');
|
||||
|
||||
const modal = document.getElementById('deleteFolderModal');
|
||||
expect(modal.dataset.state).toBe('missing');
|
||||
expect(confirmBtn().style.display).toBe('none');
|
||||
expect(manager._pendingDeleteFolderPath).toBeNull();
|
||||
expect(apiClient.deleteFolder).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('refuses a node whose every copy is a symbolic link', async () => {
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'linked',
|
||||
candidates: [
|
||||
{ folder_path: '/models/loras/linked', root: '/models/loras', is_symlink: true },
|
||||
],
|
||||
}),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
|
||||
await manager.showDeleteFolderModal('linked');
|
||||
|
||||
expect(document.getElementById('deleteFolderModal').dataset.state).toBe('symlink');
|
||||
expect(confirmBtn().style.display).toBe('none');
|
||||
expect(apiClient.deleteFolder).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not guess a root when the resolver fails on a multi-root library', async () => {
|
||||
const apiClient = createApiClient({
|
||||
fetchModelRoots: vi.fn().mockResolvedValue({
|
||||
roots: ['/models/loras', '/models/extra'],
|
||||
}),
|
||||
resolveFolder: vi.fn().mockRejectedValue(new Error('boom')),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
const success = await manager._deleteFolder('Pony');
|
||||
|
||||
expect(success).toBe(false);
|
||||
expect(apiClient.deleteFolder).not.toHaveBeenCalled();
|
||||
expect(showToast).toHaveBeenCalledWith('sidebar.folderResult.unresolved', {}, 'error');
|
||||
});
|
||||
|
||||
it('keeps the single-root fallback for clients without the resolver', async () => {
|
||||
const apiClient = createApiClient({ resolveFolder: undefined });
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
const success = await manager._deleteFolder('empty');
|
||||
|
||||
expect(success).toBe(true);
|
||||
expect(apiClient.deleteFolder).toHaveBeenCalledWith('/models/loras/empty');
|
||||
});
|
||||
|
||||
it('reports a folder that vanished between the check and the confirm', async () => {
|
||||
const apiClient = createApiClient({
|
||||
deleteFolder: vi.fn().mockRejectedValue(
|
||||
Object.assign(new Error('Folder no longer exists'), { code: 'missing' })
|
||||
),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
const success = await manager._deleteFolder('empty');
|
||||
|
||||
expect(success).toBe(false);
|
||||
expect(showToast).toHaveBeenCalledWith(
|
||||
'sidebar.deleteFolderResult.missing', {}, 'warning'
|
||||
);
|
||||
});
|
||||
|
||||
it('routes the context-menu action to the delete modal', () => {
|
||||
@@ -969,12 +1401,12 @@ describe('SidebarManager folder rename', () => {
|
||||
return document.querySelector('#sidebarRenameFolderInput .sidebar-rename-folder-input');
|
||||
}
|
||||
|
||||
it('turns the node into a prefilled inline row in tree mode', () => {
|
||||
it('turns the node into a prefilled inline row in tree mode', async () => {
|
||||
const manager = createManager(createApiClient());
|
||||
manager.treeData = { characters: { anime: {} } };
|
||||
manager.renderTree();
|
||||
|
||||
manager.showRenameFolderInput('characters/anime');
|
||||
await manager.showRenameFolderInput('characters/anime');
|
||||
|
||||
const row = document.getElementById('sidebarRenameFolderInput');
|
||||
expect(row).not.toBeNull();
|
||||
@@ -986,12 +1418,12 @@ describe('SidebarManager folder rename', () => {
|
||||
expect(manager._renameFolderPath).toBe('characters/anime');
|
||||
});
|
||||
|
||||
it('inserts the row in place in list mode', () => {
|
||||
it('inserts the row in place in list mode', async () => {
|
||||
const manager = createManager(createApiClient(), { displayMode: 'list' });
|
||||
manager.foldersList = ['characters', 'characters/anime'];
|
||||
manager.renderFolderList();
|
||||
|
||||
manager.showRenameFolderInput('characters/anime');
|
||||
await manager.showRenameFolderInput('characters/anime');
|
||||
|
||||
const row = document.getElementById('sidebarRenameFolderInput');
|
||||
expect(row.querySelector('.sidebar-node-content')).not.toBeNull();
|
||||
@@ -999,12 +1431,12 @@ describe('SidebarManager folder rename', () => {
|
||||
expect(row.nextElementSibling).toBe(item);
|
||||
});
|
||||
|
||||
it('restores the node when the edit is canceled', () => {
|
||||
it('restores the node when the edit is canceled', async () => {
|
||||
const manager = createManager(createApiClient());
|
||||
manager.treeData = { characters: { anime: {} } };
|
||||
manager.renderTree();
|
||||
|
||||
manager.showRenameFolderInput('characters/anime');
|
||||
await manager.showRenameFolderInput('characters/anime');
|
||||
manager.handleRenameFolderCancel();
|
||||
|
||||
expect(document.getElementById('sidebarRenameFolderInput')).toBeNull();
|
||||
@@ -1054,7 +1486,7 @@ describe('SidebarManager folder rename', () => {
|
||||
manager.treeData = { characters: { anime: {} } };
|
||||
manager.renderTree();
|
||||
|
||||
manager.showRenameFolderInput('characters/anime');
|
||||
await manager.showRenameFolderInput('characters/anime');
|
||||
renameInput().value = 'anime';
|
||||
await manager.handleRenameFolderSubmit();
|
||||
|
||||
@@ -1068,7 +1500,7 @@ describe('SidebarManager folder rename', () => {
|
||||
manager.treeData = { characters: { anime: {} } };
|
||||
manager.renderTree();
|
||||
|
||||
manager.showRenameFolderInput('characters/anime');
|
||||
await manager.showRenameFolderInput('characters/anime');
|
||||
renameInput().value = 'bad/name';
|
||||
await manager.handleRenameFolderSubmit();
|
||||
|
||||
@@ -1102,6 +1534,97 @@ describe('SidebarManager folder rename', () => {
|
||||
expect(manager.showRenameFolderInput).toHaveBeenCalledWith('characters/anime');
|
||||
});
|
||||
|
||||
it('renames the directory the node actually lives in', async () => {
|
||||
// Same trap as the delete flow: the node is under the primary root while
|
||||
// the configured default root is another one.
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'recipes',
|
||||
candidates: [{
|
||||
folder_path: '/models/loras/recipes',
|
||||
root: '/models/loras',
|
||||
is_symlink: false,
|
||||
}],
|
||||
}),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
state.global.settings = { default_lora_root: '/models/extra' };
|
||||
|
||||
const success = await manager._renameFolder('recipes', 'presets');
|
||||
|
||||
expect(success).toBe(true);
|
||||
expect(apiClient.renameFolder).toHaveBeenCalledWith('/models/loras/recipes', 'presets');
|
||||
});
|
||||
|
||||
it('lets the user pick the copy to rename when several roots hold the folder', async () => {
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'Pony',
|
||||
candidates: [
|
||||
{ folder_path: '/models/extra/Pony', root: '/models/extra', is_symlink: false },
|
||||
{ folder_path: '/models/loras/Pony', root: '/models/loras', is_symlink: false },
|
||||
],
|
||||
}),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
state.global.settings = { default_lora_root: '/models/extra' };
|
||||
manager.treeData = { Pony: {} };
|
||||
manager.renderTree();
|
||||
|
||||
await manager.showRenameFolderInput('Pony');
|
||||
|
||||
const picker = document.querySelector('#sidebarRenameFolderInput .sidebar-folder-root-select');
|
||||
expect([...picker.options].map((option) => option.value)).toEqual([
|
||||
'/models/extra/Pony',
|
||||
'/models/loras/Pony',
|
||||
]);
|
||||
// The default root's copy is preselected, the other one stays reachable.
|
||||
expect(picker.value).toBe('/models/extra/Pony');
|
||||
|
||||
picker.value = '/models/loras/Pony';
|
||||
renameInput().value = 'PonyV6';
|
||||
await manager.handleRenameFolderSubmit();
|
||||
|
||||
expect(apiClient.renameFolder).toHaveBeenCalledWith('/models/loras/Pony', 'PonyV6');
|
||||
});
|
||||
|
||||
it('reports a stale node instead of opening the rename row', async () => {
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true, folder: 'gone', candidates: [],
|
||||
}),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.treeData = { gone: {} };
|
||||
manager.renderTree();
|
||||
|
||||
await manager.showRenameFolderInput('gone');
|
||||
|
||||
expect(document.getElementById('sidebarRenameFolderInput')).toBeNull();
|
||||
expect(showToast).toHaveBeenCalledWith('sidebar.renameFolderResult.missing', {}, 'warning');
|
||||
});
|
||||
|
||||
it('does not guess a root when the resolver fails on a multi-root library', async () => {
|
||||
const apiClient = createApiClient({
|
||||
fetchModelRoots: vi.fn().mockResolvedValue({
|
||||
roots: ['/models/loras', '/models/extra'],
|
||||
}),
|
||||
resolveFolder: vi.fn().mockRejectedValue(new Error('boom')),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
manager.refresh = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
const success = await manager._renameFolder('Pony', 'PonyV6');
|
||||
|
||||
expect(success).toBe(false);
|
||||
expect(apiClient.renameFolder).not.toHaveBeenCalled();
|
||||
expect(showToast).toHaveBeenCalledWith('sidebar.folderResult.unresolved', {}, 'error');
|
||||
});
|
||||
|
||||
it('hides the rename entry when folder management is unsupported', () => {
|
||||
document.body.insertAdjacentHTML('beforeend', `
|
||||
<div id="sidebarFolderContextMenu" class="context-menu">
|
||||
|
||||
@@ -901,3 +901,54 @@ def test_download_model_returns_skipped_success(mock_service, download_manager_s
|
||||
await client.close()
|
||||
|
||||
asyncio.run(scenario())
|
||||
|
||||
|
||||
def test_resolve_folder_answers_with_the_root_that_holds_the_directory(
|
||||
mock_service, mock_scanner, tmp_path: Path
|
||||
):
|
||||
"""The route the sidebar needs to stop guessing a root for a tree node.
|
||||
|
||||
The unified folder tree merges every model root into one relative-path
|
||||
namespace, so "recipes" below must resolve to the root that actually has it
|
||||
even though another root is configured as the library default.
|
||||
"""
|
||||
|
||||
primary = tmp_path / "primary"
|
||||
extra = tmp_path / "extra"
|
||||
(primary / "recipes").mkdir(parents=True)
|
||||
extra.mkdir()
|
||||
mock_scanner.get_model_roots = lambda: [str(primary), str(extra)]
|
||||
|
||||
async def scenario():
|
||||
client = await create_test_client(mock_service)
|
||||
try:
|
||||
response = await client.get(
|
||||
"/api/lm/test-models/resolve-folder", params={"folder": "recipes"}
|
||||
)
|
||||
payload = await response.json()
|
||||
|
||||
assert response.status == 200
|
||||
assert payload["success"] is True
|
||||
assert payload["folder"] == "recipes"
|
||||
assert payload["candidates"] == [
|
||||
{
|
||||
"folder_path": str(primary / "recipes"),
|
||||
"root": str(primary),
|
||||
"is_symlink": False,
|
||||
}
|
||||
]
|
||||
|
||||
stale = await client.get(
|
||||
"/api/lm/test-models/resolve-folder", params={"folder": "gone"}
|
||||
)
|
||||
assert (await stale.json())["candidates"] == []
|
||||
|
||||
refused = await client.get(
|
||||
"/api/lm/test-models/resolve-folder",
|
||||
params={"folder": str(primary / "recipes")},
|
||||
)
|
||||
assert refused.status == 400
|
||||
finally:
|
||||
await client.close()
|
||||
|
||||
asyncio.run(scenario())
|
||||
|
||||
@@ -12,6 +12,7 @@ class FakeMoveService:
|
||||
self.received_path = None
|
||||
self.received_dry_run = None
|
||||
self.received_new_name = None
|
||||
self.received_folder = None
|
||||
|
||||
async def create_folder(self, folder_path):
|
||||
self.received_path = folder_path
|
||||
@@ -27,6 +28,10 @@ class FakeMoveService:
|
||||
self.received_new_name = new_name
|
||||
return self._result
|
||||
|
||||
def resolve_folder(self, folder):
|
||||
self.received_folder = folder
|
||||
return self._result
|
||||
|
||||
|
||||
class FakeRequest:
|
||||
def __init__(self, payload):
|
||||
@@ -36,6 +41,13 @@ class FakeRequest:
|
||||
return self._payload
|
||||
|
||||
|
||||
class FakeQueryRequest:
|
||||
"""Request stand-in exposing only the query string."""
|
||||
|
||||
def __init__(self, query):
|
||||
self.query = query
|
||||
|
||||
|
||||
def _make_handler(result):
|
||||
service = FakeMoveService(result)
|
||||
handler = ModelMoveHandler(
|
||||
@@ -141,6 +153,43 @@ async def test_delete_folder_forwards_dry_run():
|
||||
assert json.loads(response.text)["dry_run"] is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolve_folder_returns_the_matching_roots():
|
||||
handler, service = _make_handler(
|
||||
{
|
||||
"success": True,
|
||||
"folder": "recipes",
|
||||
"candidates": [
|
||||
{
|
||||
"folder_path": "/library/recipes",
|
||||
"root": "/library",
|
||||
"is_symlink": False,
|
||||
}
|
||||
],
|
||||
}
|
||||
)
|
||||
|
||||
response = await handler.resolve_folder(FakeQueryRequest({"folder": "recipes"}))
|
||||
|
||||
assert response.status == 200
|
||||
payload = json.loads(response.text)
|
||||
assert payload["success"] is True
|
||||
assert payload["candidates"][0]["folder_path"] == "/library/recipes"
|
||||
assert service.received_folder == "recipes"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolve_folder_maps_a_refused_path_to_400():
|
||||
handler, _service = _make_handler(
|
||||
{"success": False, "error": "Folder path is required"}
|
||||
)
|
||||
|
||||
response = await handler.resolve_folder(FakeQueryRequest({}))
|
||||
|
||||
assert response.status == 400
|
||||
assert json.loads(response.text)["success"] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_folder_missing_path():
|
||||
handler, service = _make_handler({"success": True})
|
||||
@@ -206,6 +255,20 @@ async def test_delete_folder_containment_failure_maps_to_400():
|
||||
assert "outside configured library" in payload["error"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_folder_reports_a_vanished_directory_as_missing():
|
||||
handler, _service = _make_handler(
|
||||
{"success": False, "code": "missing", "error": "Folder no longer exists"}
|
||||
)
|
||||
|
||||
response = await handler.delete_folder(
|
||||
FakeRequest({"folder_path": "/library/gone"})
|
||||
)
|
||||
|
||||
assert response.status == 400
|
||||
assert json.loads(response.text)["code"] == "missing"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_folder_invalid_json_body():
|
||||
class BadJsonRequest:
|
||||
|
||||
@@ -15,6 +15,7 @@ class FakeScanner:
|
||||
self._roots = [str(root) for root in roots]
|
||||
self.known_folders: List[str] = []
|
||||
self.removed_folders: List[str] = []
|
||||
self.removed_folder_paths: List[str | None] = []
|
||||
self.renamed_folders: List[tuple] = []
|
||||
self._excluded = list(excluded or [])
|
||||
|
||||
@@ -27,8 +28,9 @@ class FakeScanner:
|
||||
async def add_known_folder(self, folder: str) -> None:
|
||||
self.known_folders.append(folder)
|
||||
|
||||
async def remove_known_folder(self, folder: str) -> None:
|
||||
async def remove_known_folder(self, folder: str, absolute_path: str | None = None) -> None:
|
||||
self.removed_folders.append(folder)
|
||||
self.removed_folder_paths.append(absolute_path)
|
||||
|
||||
async def rename_known_folder(self, previous: str, current: str, **kwargs) -> None:
|
||||
self.renamed_folders.append((previous, current, kwargs))
|
||||
@@ -107,6 +109,114 @@ async def test_create_folder_requires_path(tmp_path: Path):
|
||||
assert result["success"] is False
|
||||
|
||||
|
||||
def test_resolve_folder_finds_the_directory_under_the_root_that_holds_it(tmp_path: Path):
|
||||
# The reported bug: the tree node lives under the primary root while the
|
||||
# configured default root is the extra one. Resolution must answer with the
|
||||
# directory that exists, not with a path fabricated from the default root.
|
||||
primary = tmp_path / "primary"
|
||||
extra = tmp_path / "extra"
|
||||
primary.mkdir()
|
||||
extra.mkdir()
|
||||
(primary / "recipes").mkdir()
|
||||
|
||||
service = ModelMoveService(FakeScanner([primary, extra]), "lora")
|
||||
|
||||
result = service.resolve_folder("recipes")
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["folder"] == "recipes"
|
||||
assert [Path(candidate["folder_path"]) for candidate in result["candidates"]] == [
|
||||
primary / "recipes"
|
||||
]
|
||||
assert result["candidates"][0]["root"] == primary.as_posix()
|
||||
assert result["candidates"][0]["is_symlink"] is False
|
||||
|
||||
|
||||
def test_resolve_folder_returns_a_candidate_per_root_holding_the_folder(tmp_path: Path):
|
||||
primary = tmp_path / "primary"
|
||||
extra = tmp_path / "extra"
|
||||
(primary / "characters" / "anime").mkdir(parents=True)
|
||||
(extra / "characters" / "anime").mkdir(parents=True)
|
||||
|
||||
service = ModelMoveService(FakeScanner([primary, extra]), "lora")
|
||||
|
||||
result = service.resolve_folder("characters/anime")
|
||||
|
||||
# Order follows the scanner's root order so the caller can prefer the
|
||||
# default root without losing the rest.
|
||||
assert [Path(candidate["folder_path"]) for candidate in result["candidates"]] == [
|
||||
primary / "characters" / "anime",
|
||||
extra / "characters" / "anime",
|
||||
]
|
||||
assert [candidate["root"] for candidate in result["candidates"]] == [
|
||||
primary.as_posix(),
|
||||
extra.as_posix(),
|
||||
]
|
||||
|
||||
|
||||
def test_resolve_folder_reports_no_candidate_for_a_stale_tree_node(tmp_path: Path):
|
||||
root = tmp_path / "library"
|
||||
root.mkdir()
|
||||
|
||||
service = ModelMoveService(FakeScanner([root]), "lora")
|
||||
|
||||
result = service.resolve_folder("gone")
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["candidates"] == []
|
||||
|
||||
|
||||
def test_resolve_folder_flags_symlinked_candidates(tmp_path: Path):
|
||||
root = tmp_path / "library"
|
||||
root.mkdir()
|
||||
real = tmp_path / "real"
|
||||
real.mkdir()
|
||||
try:
|
||||
(root / "linked").symlink_to(real, target_is_directory=True)
|
||||
except (OSError, NotImplementedError): # pragma: no cover - platform guard
|
||||
pytest.skip("symlinks are not supported on this platform")
|
||||
|
||||
service = ModelMoveService(FakeScanner([root]), "lora")
|
||||
|
||||
result = service.resolve_folder("linked")
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["candidates"][0]["is_symlink"] is True
|
||||
|
||||
|
||||
def test_resolve_folder_normalizes_the_relative_path(tmp_path: Path):
|
||||
root = tmp_path / "library"
|
||||
(root / "characters" / "anime").mkdir(parents=True)
|
||||
|
||||
service = ModelMoveService(FakeScanner([root]), "lora")
|
||||
|
||||
result = service.resolve_folder(" characters\\anime/ ")
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["folder"] == "characters/anime"
|
||||
assert len(result["candidates"]) == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"folder",
|
||||
["", " ", "/library/recipes", "C:/library/recipes", "..", "../outside", "a/../../b"],
|
||||
)
|
||||
def test_resolve_folder_rejects_paths_outside_the_relative_namespace(
|
||||
tmp_path: Path, folder: str
|
||||
):
|
||||
root = tmp_path / "library"
|
||||
root.mkdir()
|
||||
outside = tmp_path / "outside"
|
||||
outside.mkdir()
|
||||
|
||||
service = ModelMoveService(FakeScanner([root]), "lora")
|
||||
|
||||
result = service.resolve_folder(folder)
|
||||
|
||||
assert result["success"] is False
|
||||
assert "candidates" not in result
|
||||
|
||||
|
||||
def _make_nested(root: Path) -> Path:
|
||||
target = root / "characters" / "anime"
|
||||
target.mkdir(parents=True)
|
||||
@@ -127,6 +237,9 @@ async def test_delete_folder_removes_empty_directory_and_forgets_it(tmp_path: Pa
|
||||
assert result["restorable"] is True
|
||||
assert not target.exists()
|
||||
assert scanner.removed_folders == ["characters/anime"]
|
||||
# The scanner purges by the removed directory: the same relative folder may
|
||||
# exist under another root, whose model cards must survive.
|
||||
assert scanner.removed_folder_paths == [str(target)]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
Reference in New Issue
Block a user