feat: feedback verbetering 05

This commit is contained in:
kodi
2026-03-15 15:30:55 +01:00
parent 61d0c8de41
commit ae6a9d8c45
13 changed files with 139 additions and 31 deletions
+5
View File
@@ -86,6 +86,11 @@ Success (202):
}
```
Notes:
- Batch move is supported as one task-based operation via `{ "sources": [...], "destination_base": "..." }`.
- Cross-root batch move is supported for file-only selections.
- Cross-root batch move with any directory in the selection remains unsupported in v1.
## Tasks read endpoints
### `GET /api/tasks`
@@ -95,10 +95,11 @@ class MoveTaskService:
)
root_alias = next(iter(source_aliases))
if root_alias != resolved_destination_base.alias:
has_directory = any(resolved_source.absolute.is_dir() for resolved_source in resolved_sources)
if root_alias != resolved_destination_base.alias and has_directory:
raise AppError(
code="invalid_request",
message="Cross-root batch directory move is not supported in v1",
message="Cross-root batch move with directories is not supported in v1",
status_code=400,
details={"destination_base": destination_base},
)
+21 -1
View File
@@ -1,5 +1,6 @@
from __future__ import annotations
import errno
import os
import shutil
import threading
@@ -355,7 +356,13 @@ class TaskRunner:
completed_items = self._move_directory_item(task_id, item, completed_items, total_items)
else:
file_entry = self._file_entries(item)[0]
completed_items = self._move_single_planned_file(task_id, file_entry, completed_items, total_items)
completed_items = self._move_single_planned_file(
task_id,
file_entry,
completed_items,
total_items,
same_root=bool(item.get("same_root", True)),
)
if self._is_cancel_requested(task_id):
self._finalize_cancelled(task_id, done_items=completed_items, total_items=total_items)
return
@@ -631,6 +638,8 @@ class TaskRunner:
file_entry: dict[str, str],
completed_items: int,
total_items: int,
*,
same_root: bool,
) -> int:
self._repository.update_progress(
task_id=task_id,
@@ -638,7 +647,18 @@ class TaskRunner:
total_items=total_items,
current_item=file_entry["label"],
)
try:
if same_root:
self._filesystem.move_file(source=file_entry["source"], destination=file_entry["destination"])
else:
self._filesystem.copy_file(source=file_entry["source"], destination=file_entry["destination"])
self._filesystem.delete_file(Path(file_entry["source"]))
except OSError as exc:
if same_root and exc.errno == errno.EXDEV:
self._filesystem.copy_file(source=file_entry["source"], destination=file_entry["destination"])
self._filesystem.delete_file(Path(file_entry["source"]))
else:
raise
completed_items += 1
self._repository.update_progress(
task_id=task_id,
Binary file not shown.
@@ -1,6 +1,7 @@
from __future__ import annotations
import asyncio
import errno
import sys
import tempfile
import threading
@@ -52,6 +53,11 @@ class BlockingMoveFilesystemAdapter(FilesystemAdapter):
super().move_file(source, destination)
class CrossDeviceMoveFilesystemAdapter(FilesystemAdapter):
def move_file(self, source: str, destination: str) -> None:
raise OSError(errno.EXDEV, "Invalid cross-device link")
class MoveApiGoldenTest(unittest.TestCase):
def setUp(self) -> None:
self.temp_dir = tempfile.TemporaryDirectory()
@@ -312,6 +318,59 @@ class MoveApiGoldenTest(unittest.TestCase):
self.assertTrue((self.root1 / "b.txt").exists())
self.assertFalse((target / "b.txt").exists())
def test_move_batch_cross_root_files_success(self) -> None:
first = self.root1 / "first.txt"
second = self.root1 / "second.txt"
first.write_text("a", encoding="utf-8")
second.write_text("b", encoding="utf-8")
response = self._request(
"POST",
"/api/files/move",
{
"sources": ["storage1/first.txt", "storage1/second.txt"],
"destination_base": "storage2",
},
)
self.assertEqual(response.status_code, 202)
detail = self._wait_task(response.json()["task_id"])
self.assertEqual(detail["status"], "completed")
self.assertEqual(detail["done_items"], 2)
self.assertEqual(detail["total_items"], 2)
self.assertTrue((self.root2 / "first.txt").exists())
self.assertTrue((self.root2 / "second.txt").exists())
self.assertFalse(first.exists())
self.assertFalse(second.exists())
def test_move_batch_cross_root_files_falls_back_from_exdev(self) -> None:
first = self.root1 / "first.txt"
second = self.root1 / "second.txt"
first.write_text("a", encoding="utf-8")
second.write_text("b", encoding="utf-8")
path_guard = PathGuard({"storage1": str(self.root1), "storage2": str(self.root2)})
self._set_services(path_guard=path_guard, filesystem=CrossDeviceMoveFilesystemAdapter())
response = self._request(
"POST",
"/api/files/move",
{
"sources": ["storage1/first.txt", "storage1/second.txt"],
"destination_base": "storage2",
},
)
self.assertEqual(response.status_code, 202)
detail = self._wait_task(response.json()["task_id"])
self.assertEqual(detail["status"], "completed")
self.assertEqual(detail["done_items"], 2)
self.assertEqual(detail["total_items"], 2)
self.assertTrue((self.root2 / "first.txt").exists())
self.assertTrue((self.root2 / "second.txt").exists())
self.assertFalse(first.exists())
self.assertFalse(second.exists())
def test_move_batch_cross_root_directories_blocked(self) -> None:
first = self.root1 / "first-dir"
second = self.root1 / "second-dir"
@@ -329,6 +388,26 @@ class MoveApiGoldenTest(unittest.TestCase):
self.assertEqual(response.status_code, 400)
self.assertEqual(response.json()["error"]["code"], "invalid_request")
self.assertEqual(response.json()["error"]["message"], "Cross-root batch move with directories is not supported in v1")
def test_move_batch_cross_root_mixed_files_and_directories_blocked(self) -> None:
first = self.root1 / "first.txt"
first.write_text("a", encoding="utf-8")
second = self.root1 / "second-dir"
second.mkdir()
response = self._request(
"POST",
"/api/files/move",
{
"sources": ["storage1/first.txt", "storage1/second-dir"],
"destination_base": "storage2",
},
)
self.assertEqual(response.status_code, 400)
self.assertEqual(response.json()["error"]["code"], "invalid_request")
self.assertEqual(response.json()["error"]["message"], "Cross-root batch move with directories is not supported in v1")
def test_move_batch_mixed_root_selection_blocked(self) -> None:
first = self.root1 / "first-dir"
@@ -368,7 +368,7 @@ class UiSmokeGoldenTest(unittest.TestCase):
pollTimer: null,
lastRenderKey: "",
}};
const ACTIVE_TASK_OPERATIONS = new Set(["copy", "move", "duplicate", "delete"]);
const ACTIVE_OPERATION_OPERATIONS = new Set(["copy", "move", "duplicate"]);
const ACTIVE_TASK_STATUSES = new Set(["queued", "running", "cancelling"]);
{functions}
@@ -388,28 +388,28 @@ class UiSmokeGoldenTest(unittest.TestCase):
];
const activeTasks = activeTasksFromItems(mixedTasks);
assert(activeTasks.length === 5, "Only active task-based file actions should count as active");
assert(activeTasks.length === 4, "Only active user-visible operations should count as active");
assert(activeTasks.every((task) => isActiveTask(task)), "All filtered tasks should be active");
assert(activeTasks.some((task) => task.operation === "delete"), "Delete should count once it uses the shared task flow");
assert(!activeTasks.some((task) => task.operation === "delete"), "Delete should stay out of operation UI until it maps cleanly to one user-visible operation");
assert(activeTasks.some((task) => task.status === "cancelling"), "Cancelling tasks should remain visible while stopping");
assert(activeTaskChipLabel(activeTasks) === "5 active tasks", "Chip label should reflect active task count");
assert(activeTaskChipLabel(activeTasks) === "4 active operations", "Chip label should reflect active operation count");
updateHeaderTaskState(mixedTasks);
assert(!elements["header-task-chip-container"].classList.contains("hidden"), "Chip should be visible with active tasks");
assert(elements["header-task-chip-label"].textContent === "5 active tasks", "Chip label should render active task count");
assert(elements["header-task-chip-label"].textContent === "4 active operations", "Chip label should render active operation count");
assert(shouldPollHeaderTasks(), "Active tasks should enable header polling");
setHeaderTaskPopoverOpen(true);
assert(headerTaskState.popoverOpen, "Popover should open when active tasks exist");
assert(!elements["header-task-popover"].classList.contains("hidden"), "Popover should be visible when open");
assert(elements["header-task-chip-btn"].attributes["aria-expanded"] === "true", "Chip button should expose expanded state");
assert(elements["header-task-popover-list"].children.length === 5, "Popover should render only active file-action tasks");
assert(elements["header-task-popover-list"].children.length === 4, "Popover should render only active operations");
const moveRow = elements["header-task-popover-list"].children[1];
const moveProgress = moveRow.children[3];
const moveCurrent = moveRow.children[4];
assert(moveProgress.textContent === "1/3", "Popover should show done/total progress when available");
assert(moveCurrent.textContent === "b.mkv", "Popover should show compact current item");
const cancellingRow = elements["header-task-popover-list"].children[4];
const cancellingRow = elements["header-task-popover-list"].children[3];
const cancellingProgress = cancellingRow.children[3];
const cancellingCurrent = cancellingRow.children[4];
const cancellingSubtext = cancellingRow.children[5];
@@ -417,7 +417,7 @@ class UiSmokeGoldenTest(unittest.TestCase):
assert(cancellingCurrent.textContent === "nested/final-file.txt", "Cancelling tasks should show current item");
assert(cancellingSubtext.textContent === "Stopping after current item...", "Cancelling tasks should explain stop semantics");
const firstActionButton = elements["header-task-popover-list"].children[0].children[3].children[0];
const cancellingActionButton = elements["header-task-popover-list"].children[4].children[6].children[0];
const cancellingActionButton = elements["header-task-popover-list"].children[3].children[6].children[0];
assert(firstActionButton.textContent === "Stop", "Queued/running tasks should expose a Stop action");
assert(!firstActionButton.disabled, "Queued/running tasks should be cancellable");
assert(cancellingActionButton.textContent === "Stopping...", "Cancelling tasks should show stopping state");
@@ -582,7 +582,7 @@ class UiSmokeGoldenTest(unittest.TestCase):
pollTimer: null,
lastRenderKey: "",
}};
const ACTIVE_TASK_OPERATIONS = new Set(["copy", "move", "duplicate", "delete"]);
const ACTIVE_OPERATION_OPERATIONS = new Set(["copy", "move", "duplicate"]);
const ACTIVE_TASK_STATUSES = new Set(["queued", "running", "cancelling"]);
{functions}
@@ -905,6 +905,7 @@ class UiSmokeGoldenTest(unittest.TestCase):
def test_ui_static_assets_are_present_and_mapped(self) -> None:
mount = self._ui_mount()
static_root = Path(mount.app.directory)
index_html = (static_root / "index.html").read_text(encoding="utf-8")
self.assertTrue((static_root / "app.js").exists())
self.assertTrue((static_root / "base.css").exists())
self.assertTrue((static_root / "theme-default.css").exists())
@@ -964,9 +965,9 @@ class UiSmokeGoldenTest(unittest.TestCase):
self.assertIn('function inferDownloadTaskContext(task)', app_js)
self.assertIn('function formatTaskLine(task)', app_js)
self.assertIn('let headerTaskState = {', app_js)
self.assertIn('const ACTIVE_TASK_OPERATIONS = new Set(["copy", "move", "duplicate", "delete"]);', app_js)
self.assertIn('const ACTIVE_OPERATION_OPERATIONS = new Set(["copy", "move", "duplicate"]);', app_js)
self.assertIn('const ACTIVE_TASK_STATUSES = new Set(["queued", "running", "cancelling"]);', app_js)
self.assertIn("The header chip reflects only user-visible file actions that use the shared task system.", app_js)
self.assertIn("The header chip/popover reflects user-visible file operations, not every task-backed file action.", app_js)
self.assertIn('function headerTaskElements()', app_js)
self.assertIn('function isActiveTask(task)', app_js)
self.assertIn('function activeTasksFromItems(items)', app_js)
@@ -986,12 +987,12 @@ class UiSmokeGoldenTest(unittest.TestCase):
self.assertIn('function renderHeaderTaskChip(items)', app_js)
self.assertIn('function updateHeaderTaskState(taskItems)', app_js)
self.assertIn('function applyTaskSnapshot(taskItems)', app_js)
self.assertIn('return `${count} active task${count === 1 ? "" : "s"}`;', app_js)
self.assertIn('return `${count} active operation${count === 1 ? "" : "s"}`;', app_js)
self.assertIn('return task.operation === "copy" || task.operation === "duplicate";', app_js)
self.assertIn('return `${action} ${task.done_items}/${task.total_items}`;', app_js)
self.assertIn('return `${action} running`;', app_js)
self.assertIn('return "Stopping after current item...";', app_js)
self.assertIn('ACTIVE_TASK_OPERATIONS.has(task.operation)', app_js)
self.assertIn('ACTIVE_OPERATION_OPERATIONS.has(task.operation)', app_js)
self.assertIn('headerTaskState.activeItems = activeTasksFromItems(taskItems);', app_js)
self.assertIn('const open = Boolean(nextOpen) && headerTaskState.activeItems.length > 0;', app_js)
self.assertIn('const headerTasks = headerTaskElements();', app_js)
@@ -1135,6 +1136,8 @@ class UiSmokeGoldenTest(unittest.TestCase):
self.assertIn('destination_base: baseDestination,', app_js)
self.assertIn('setStatus("Copy: operation started");', app_js)
self.assertIn('setStatus("Move: operation started");', app_js)
self.assertIn('Active operations', index_html)
self.assertIn('No active operations right now.', app_js)
self.assertIn('const confirmed = await openConfirmModal({', app_js)
self.assertIn('title: selectedItems.length === 1 ? "Delete item?" : "Delete selected items?"', app_js)
self.assertIn('title: "Discard unsaved changes?"', app_js)
+6 -6
View File
@@ -120,8 +120,8 @@ let headerTaskState = {
pollTimer: null,
lastRenderKey: "",
};
// The header chip reflects only user-visible file actions that use the shared task system.
const ACTIVE_TASK_OPERATIONS = new Set(["copy", "move", "duplicate", "delete"]);
// The header chip/popover reflects user-visible file operations, not every task-backed file action.
const ACTIVE_OPERATION_OPERATIONS = new Set(["copy", "move", "duplicate"]);
const ACTIVE_TASK_STATUSES = new Set(["queued", "running", "cancelling"]);
const VALID_THEME_FAMILIES = [
"default",
@@ -3870,7 +3870,7 @@ function formatTaskLine(task) {
}
function isActiveTask(task) {
return Boolean(task) && ACTIVE_TASK_OPERATIONS.has(task.operation) && ACTIVE_TASK_STATUSES.has(task.status);
return Boolean(task) && ACTIVE_OPERATION_OPERATIONS.has(task.operation) && ACTIVE_TASK_STATUSES.has(task.status);
}
function activeTasksFromItems(items) {
@@ -3878,7 +3878,7 @@ function activeTasksFromItems(items) {
}
function taskIsCancellable(task) {
return Boolean(task) && ACTIVE_TASK_OPERATIONS.has(task.operation) && ["queued", "running"].includes(task.status);
return Boolean(task) && ACTIVE_OPERATION_OPERATIONS.has(task.operation) && ["queued", "running"].includes(task.status);
}
async function cancelTaskRequest(taskId) {
@@ -3925,7 +3925,7 @@ function compactTaskCurrentItem(task) {
function activeTaskChipLabel(items) {
const count = Array.isArray(items) ? items.length : 0;
if (count !== 1) {
return `${count} active task${count === 1 ? "" : "s"}`;
return `${count} active operation${count === 1 ? "" : "s"}`;
}
const task = items[0];
const action = formatTaskOperationLabel(task);
@@ -4025,7 +4025,7 @@ function renderHeaderTaskPopover(items) {
if (!Array.isArray(items) || items.length === 0) {
const empty = document.createElement("div");
empty.className = "header-task-item-empty";
empty.textContent = "No active tasks right now.";
empty.textContent = "No active operations right now.";
elements.popoverList.append(empty);
headerTaskState.lastRenderKey = renderKey;
return;
+5 -5
View File
@@ -100,8 +100,8 @@ body {
position: absolute;
top: calc(100% + 8px);
right: 0;
width: min(360px, calc(100vw - 24px));
padding: 12px;
width: min(540px, calc(100vw - 24px));
padding: 14px;
border: 1px solid var(--color-border);
border-radius: var(--radius-md);
background: var(--color-surface-elevated);
@@ -131,8 +131,8 @@ body {
.header-task-popover-list {
display: flex;
flex-direction: column;
gap: 8px;
max-height: 260px;
gap: 10px;
max-height: 360px;
overflow-y: auto;
}
@@ -140,7 +140,7 @@ body {
border: 1px solid var(--color-border);
border-radius: var(--radius-sm);
background: var(--color-surface);
padding: 8px 9px;
padding: 10px 12px;
}
.header-task-item-title {
+3 -3
View File
@@ -34,11 +34,11 @@
aria-expanded="false"
aria-controls="header-task-popover"
>
<span id="header-task-chip-label">1 active task</span>
<span id="header-task-chip-label">1 active operation</span>
</button>
<div id="header-task-popover" class="header-task-popover hidden" role="dialog" aria-label="Active tasks">
<div id="header-task-popover" class="header-task-popover hidden" role="dialog" aria-label="Active operations">
<div class="header-task-popover-header">
<strong>Active tasks</strong>
<strong>Active operations</strong>
<button id="header-task-logs-btn" type="button" class="header-task-link">View in Logs</button>
</div>
<div id="header-task-popover-list" class="header-task-popover-list"></div>