mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-08-14 09:43:22 -03:00
refactor(delete): make undo unconditional, remove undo toggle and button delay
- Remove delete_undo_enabled setting (backend default, frontend state, settings modal UI, 10 locales); staged deletes with 30s undo are now the only delete path and stale settings keys are silently ignored - Remove the 1500ms delete-button arm delay (armDeleteButton) from all delete modals; misclicks are recoverable via the undo toast - Delete modal always shows the recoverable warning - Log the first staged file path in staging log lines for easier support
This commit is contained in:
@@ -10,7 +10,6 @@ import pytest
|
||||
|
||||
from py.services.model_lifecycle_service import ModelLifecycleService, _require_path_in_library_roots
|
||||
from py.services.pending_delete_service import PENDING_DELETE_DIR_NAME, _reset_pending_delete_service
|
||||
from py.services.settings_manager import get_settings_manager
|
||||
from py.utils.metadata_manager import MetadataManager
|
||||
from py.utils.models import LoraMetadata
|
||||
|
||||
@@ -953,11 +952,11 @@ def _make_delete_service(scanner: Any) -> ModelLifecycleService:
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_model_stages_file_when_undo_enabled(tmp_path: Path):
|
||||
"""Undo enabled (the default): artifacts are renamed into a
|
||||
``.lm-pending-delete/<batch_id>/`` staging dir under the model root, the
|
||||
response carries the batch_id, the cache entry is removed and the cache
|
||||
is persisted (``_persist_calls`` tracked by ``ScannerForDelete``)."""
|
||||
async def test_delete_model_stages_file(tmp_path: Path):
|
||||
"""Artifacts are renamed into a ``.lm-pending-delete/<batch_id>/`` staging
|
||||
dir under the model root, the response carries the batch_id, the cache
|
||||
entry is removed and the cache is persisted (``_persist_calls`` tracked by
|
||||
``ScannerForDelete``)."""
|
||||
root = tmp_path / "loras"
|
||||
root.mkdir()
|
||||
model_path = root / "model.safetensors"
|
||||
@@ -999,41 +998,6 @@ async def test_delete_model_stages_file_when_undo_enabled(tmp_path: Path):
|
||||
assert scanner._persist_calls == [True]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_model_hard_deletes_when_undo_disabled(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
):
|
||||
"""delete_undo_enabled=false: old os.remove behavior, batch_id is None and
|
||||
no staging directory is ever created."""
|
||||
root = tmp_path / "loras"
|
||||
root.mkdir()
|
||||
model_path = root / "model.safetensors"
|
||||
model_path.write_bytes(b"content")
|
||||
|
||||
settings_manager = get_settings_manager()
|
||||
monkeypatch.setattr(
|
||||
settings_manager,
|
||||
"get",
|
||||
lambda key, default=None: False
|
||||
if key == "delete_undo_enabled"
|
||||
else default,
|
||||
)
|
||||
|
||||
scanner = ScannerForDelete(
|
||||
raw_data=[{"file_path": str(model_path)}],
|
||||
roots=[str(root)],
|
||||
)
|
||||
service = _make_delete_service(scanner)
|
||||
|
||||
result = await service.delete_model(str(model_path))
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["batch_id"] is None
|
||||
assert result["deleted_files"]
|
||||
assert not model_path.exists()
|
||||
assert not (root / PENDING_DELETE_DIR_NAME).exists()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_model_falls_back_when_staging_fails(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
|
||||
@@ -22,7 +22,6 @@ from py.services.pending_delete_service import (
|
||||
_reset_pending_delete_service,
|
||||
)
|
||||
from py.services.persistent_model_cache import PersistentModelCache, DEFAULT_LICENSE_FLAGS
|
||||
from py.services.settings_manager import get_settings_manager
|
||||
from py.utils.civitai_utils import build_license_flags
|
||||
from py.utils.models import BaseModelMetadata
|
||||
|
||||
@@ -1166,33 +1165,6 @@ async def test_bulk_delete_merge_failure_falls_back_to_batch_ids(
|
||||
assert sorted(staged_files) == ["one.txt", "two.txt"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bulk_delete_undo_disabled_hard_deletes(tmp_path: Path):
|
||||
"""delete_undo_enabled=false -> old hard delete, no batch, no staging dirs."""
|
||||
root = tmp_path / "loras"
|
||||
root.mkdir()
|
||||
first = root / "one.txt"
|
||||
first.write_text("one", encoding="utf-8")
|
||||
second = root / "two.txt"
|
||||
second.write_text("two", encoding="utf-8")
|
||||
scanner = _make_bulk_scanner(root, [first, second])
|
||||
|
||||
get_settings_manager().settings["delete_undo_enabled"] = False
|
||||
|
||||
result = await scanner.bulk_delete_models([str(first), str(second)])
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["status"] == "success"
|
||||
assert result["total_deleted"] == 2
|
||||
assert result.get("batch_id") is None
|
||||
assert "batch_ids" not in result
|
||||
|
||||
# Old hard-delete behavior: files removed, zero staging dirs created.
|
||||
assert not first.exists()
|
||||
assert not second.exists()
|
||||
assert not (root / PENDING_DELETE_DIR_NAME).exists()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bulk_delete_cancelled_after_one_staged_batch_present(
|
||||
tmp_path: Path, monkeypatch
|
||||
|
||||
@@ -31,7 +31,6 @@ from py.services.pending_delete_service import (
|
||||
)
|
||||
from py.services.model_hash_index import ModelHashIndex
|
||||
from py.services.model_scanner import ModelScanner
|
||||
from py.services.settings_manager import DEFAULT_SETTINGS, get_settings_manager
|
||||
from py.utils import settings_paths
|
||||
from py.utils.models import LoraMetadata
|
||||
|
||||
@@ -771,32 +770,6 @@ async def test_l2_merge_basename_collision_aborts_without_dropping_files(
|
||||
assert (sub_b / "model.safetensors").read_bytes() == b"model-data"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# (m) delete_undo_enabled=false -> stage returns None, nothing created
|
||||
# ---------------------------------------------------------------------------
|
||||
async def test_m_undo_disabled_returns_none(tmp_path: Path) -> None:
|
||||
root = tmp_path / "loras"
|
||||
root.mkdir()
|
||||
model = root / "model.safetensors"
|
||||
model.write_bytes(b"data")
|
||||
|
||||
get_settings_manager().settings["delete_undo_enabled"] = False
|
||||
|
||||
service = await PendingDeleteService.get_instance()
|
||||
batch_id = await service.stage_model_delete(
|
||||
scanner=ScannerForStage([root]),
|
||||
target_dir=str(root),
|
||||
file_name="model",
|
||||
main_extension=".safetensors",
|
||||
original_file_path=str(model),
|
||||
cached_entry=None,
|
||||
)
|
||||
|
||||
assert batch_id is None
|
||||
assert model.exists()
|
||||
assert not (root / PENDING_DELETE_DIR_NAME).exists()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# (n) simulated OSError during staging -> rollback, no orphaned batch dir
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -839,13 +812,6 @@ async def test_n_staging_oserror_rolls_back(tmp_path: Path, monkeypatch) -> None
|
||||
assert not any(staging.iterdir())
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# (o) DEFAULT_SETTINGS contains delete_undo_enabled=True
|
||||
# ---------------------------------------------------------------------------
|
||||
def test_o_default_settings_contains_undo_enabled() -> None:
|
||||
assert DEFAULT_SETTINGS.get("delete_undo_enabled") is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# (p) SCANNER EXCLUSION
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -31,7 +31,6 @@ from py.services.recipes.persistence_service import (
|
||||
PersistenceResult,
|
||||
RecipePersistenceService,
|
||||
)
|
||||
from py.services.settings_manager import get_settings_manager
|
||||
from py.utils import settings_paths
|
||||
|
||||
|
||||
@@ -223,29 +222,6 @@ async def test_delete_recipe_skips_missing_preview_image(tmp_path: Path) -> None
|
||||
assert scanner.removed == ["r2"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# (3) undo disabled -> no staging, payload batch_id None, existing behavior
|
||||
# ---------------------------------------------------------------------------
|
||||
async def test_delete_recipe_undo_disabled_no_staging(tmp_path: Path) -> None:
|
||||
get_settings_manager().settings["delete_undo_enabled"] = False
|
||||
|
||||
scanner = RecipeScannerStub(tmp_path)
|
||||
json_path, image_path, _recipe_data = _write_recipe(tmp_path, "r3")
|
||||
scanner.register_recipe("r3", json_path)
|
||||
|
||||
result = await _make_service().delete_recipe(
|
||||
recipe_scanner=scanner, recipe_id="r3"
|
||||
)
|
||||
|
||||
assert result.payload["batch_id"] is None
|
||||
# No staging leftovers when undo is disabled.
|
||||
assert not _staging_parent().exists()
|
||||
# Existing hard delete behavior unchanged.
|
||||
assert not json_path.exists()
|
||||
assert not image_path.exists()
|
||||
assert scanner.removed == ["r3"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# (4) bulk_delete with 2 ids -> single batch_id, one batch dir with both
|
||||
# recipes, re-anchored expires_at in the merged manifest
|
||||
|
||||
@@ -1178,8 +1178,3 @@ def test_skip_previously_downloaded_model_versions_coerces_string_input(manager)
|
||||
|
||||
assert manager.get_skip_previously_downloaded_model_versions() is True
|
||||
assert manager.settings["skip_previously_downloaded_model_versions"] is True
|
||||
|
||||
|
||||
def test_delete_undo_enabled_defaults_true(manager):
|
||||
assert settings_manager_module.DEFAULT_SETTINGS.get("delete_undo_enabled") is True
|
||||
assert manager.get("delete_undo_enabled") is True
|
||||
|
||||
Reference in New Issue
Block a user