diff --git a/scripts/check_api_conventions.py b/scripts/check_api_conventions.py index ce50eab26..de22cb519 100644 --- a/scripts/check_api_conventions.py +++ b/scripts/check_api_conventions.py @@ -83,7 +83,6 @@ ALLOWLIST: dict[tuple[str, str], int] = { ("solstone/apps/import/routes.py", "bare-array"): 1, ("solstone/apps/link/routes.py", "abort"): 1, ("solstone/apps/observer/routes.py", "bare-array"): 2, - ("solstone/apps/todos/routes.py", "bare-return"): 5, ("solstone/convey/services_scout.py", "abort"): 1, ("solstone/convey/services_scout.py", "inline-error"): 2, } diff --git a/solstone/apps/todos/call.py b/solstone/apps/todos/call.py index 70fc8249d..10f6c75ea 100644 --- a/solstone/apps/todos/call.py +++ b/solstone/apps/todos/call.py @@ -14,6 +14,7 @@ from pathlib import Path import typer from solstone.apps.todos import todo +from solstone.apps.todos.todo import TodoMovePartialError, TodoNotMovableError from solstone.think.facets import log_call_action from solstone.think.journal_io.errors import LockTimeout from solstone.think.utils import get_journal, require_solstone @@ -309,12 +310,17 @@ def move_todo( ) raise typer.Exit(1) + if from_facet == to_facet: + typer.echo("Error: source and destination facet are the same.", err=True) + raise typer.Exit(1) + try: source_checklist = todo.TodoChecklist.load(day, from_facet) if not source_checklist.exists: raise FileNotFoundError() todo.validate_line_number(line_number, len(source_checklist.items)) item = source_checklist.items[line_number - 1] + # Completion is enforced pre-call; move_entry intentionally stays policy-neutral. if item.completed: raise todo.TodoError("Cannot move a completed todo.") if item.cancelled: @@ -333,45 +339,15 @@ def move_todo( raise typer.Exit(1) try: - - def _append_dest( - checklist: todo.TodoChecklist, - ) -> tuple[todo.TodoChecklist, todo.TodoItem]: - new_item = checklist.append_entry( - item.text, - item.nudge, - created_at=item.created_at, - ) - return checklist, new_item - - _, new_item = todo.TodoChecklist.locked_modify(day, to_facet, _append_dest) - except Exception as exc: - typer.echo( - f"Error: Failed to append to destination facet '{to_facet}': {exc}. Source todo is unchanged.", - err=True, + new_item, cancelled = todo.TodoChecklist.move_entry( + day, from_facet, line_number, day, to_facet ) + except (IndexError, TodoNotMovableError) as exc: + typer.echo(f"Error: {exc}", err=True) raise typer.Exit(1) - - try: - - def _cancel_source( - checklist: todo.TodoChecklist, - ) -> tuple[todo.TodoChecklist, todo.TodoItem]: - todo.validate_line_number(line_number, len(checklist.items)) - current_item = checklist.items[line_number - 1] - if current_item.completed: - raise todo.TodoError("Cannot move a completed todo.") - if current_item.cancelled: - raise todo.TodoError("Cannot move an already cancelled todo.") - cancelled_item = checklist.cancel_entry( - line_number, - cancelled_reason="moved_to_facet", - moved_to=to_facet, - ) - return checklist, cancelled_item - - _, item = todo.TodoChecklist.locked_modify(day, from_facet, _cancel_source) - except (FileNotFoundError, IndexError, todo.TodoError, LockTimeout): + except LockTimeout: + _exit_todo_busy() + except TodoMovePartialError: typer.echo( f"Warning: Item was appended to '{to_facet}' but could not cancel source in '{from_facet}'. Cancel it manually with: sol call todos cancel {line_number} --day {day} --facet {from_facet}", err=True, @@ -382,7 +358,7 @@ def move_todo( "moved_from": from_facet, "moved_to": to_facet, "line_number": line_number, - "text": item.text, + "text": cancelled.text, } params_in: dict[str, object] = { "moved_from": from_facet, @@ -396,7 +372,7 @@ def move_todo( log_call_action(facet=from_facet, action="todo_move_out", params=params_out) log_call_action(facet=to_facet, action="todo_move_in", params=params_in) typer.echo( - f"Moved todo {line_number} ('{item.text}') from '{from_facet}' to '{to_facet}'." + f"Moved todo {line_number} ('{cancelled.text}') from '{from_facet}' to '{to_facet}'." ) diff --git a/solstone/apps/todos/routes.py b/solstone/apps/todos/routes.py index 294b2ceae..a0d1c26de 100644 --- a/solstone/apps/todos/routes.py +++ b/solstone/apps/todos/routes.py @@ -22,6 +22,8 @@ from solstone.apps.todos.todo import ( TodoChecklist, TodoEmptyTextError, TodoItem, + TodoMovePartialError, + TodoNotMovableError, format_nudge, get_todos, ) @@ -216,7 +218,11 @@ def todos_page() -> str: @todos_bp.route("/", methods=["GET", "POST"]) def todos_day(day: str): # type: ignore[override] if not DATE_RE.fullmatch(day): - return "", 404 + return error_response( + INVALID_DAY, + status=404, + detail="Day must be in YYYYMMDD format.", + ) if request.method == "POST": action = request.form.get("action") @@ -404,25 +410,9 @@ def todos_day(day: str): # type: ignore[override] # If facet changed, move the todo (cancel source, add to target) if new_facet != facet: - source_item = checklist.get_item(index) - old_text = source_item.text - - def _add_to_new(cl: TodoChecklist) -> TodoItem: - created = cl.append_entry( - text, - nudge=source_item.nudge, - created_at=source_item.created_at, - ) - if source_item.completed: - cl.mark_done(created.index) - return created - - TodoChecklist.locked_modify(day, new_facet, _add_to_new) - - def _cancel_source(cl: TodoChecklist) -> None: - cl.cancel_entry(index) - - TodoChecklist.locked_modify(day, facet, _cancel_source) + _created, cancelled = TodoChecklist.move_entry( + day, facet, index, day, new_facet, text=text + ) log_app_action( app="todos", @@ -430,7 +420,7 @@ def todos_day(day: str): # type: ignore[override] action="todo_edit", params={ "line_number": index, - "old_text": old_text, + "old_text": cancelled.text, "new_text": text, "old_facet": facet, "new_facet": new_facet, @@ -465,6 +455,10 @@ def todos_day(day: str): # type: ignore[override] flash("Cannot update todo to empty text", "error") except IndexError: flash(f"Todo list changed, {CONVEY_RELOAD_HINT}", "error") + except TodoNotMovableError: + flash(f"Todo list changed, {CONVEY_RELOAD_HINT}", "error") + except TodoMovePartialError: + flash(f"Todo list changed, {CONVEY_RELOAD_HINT}", "error") except LockTimeout: flash(f"Todo list changed, {CONVEY_RELOAD_HINT}", "error") @@ -602,7 +596,11 @@ def todos_overflow(day: str, facet: str, section: str): def move_todo(day: str): # type: ignore[override] """Move a todo to a different day by cancelling source and adding to target.""" if not DATE_RE.fullmatch(day): - return "", 404 + return error_response( + INVALID_DAY, + status=404, + detail="Day must be in YYYYMMDD format.", + ) payload = request.get_json(silent=True) or {} target_day = (payload.get("target_day") or "").strip() @@ -631,49 +629,26 @@ def move_todo(day: str): # type: ignore[override] } ) + # L9: replaying an already-moved item returns 409 without appending again. try: - source_checklist = TodoChecklist.load(day, facet) - except RuntimeError as exc: - current_app.logger.debug( - "Failed to load source todo list for %s/%s: %s", facet, day, exc + _created, cancelled = TodoChecklist.move_entry( + day, facet, index, target_day, facet ) - return error_response( - OPERATION_NO_LONGER_AVAILABLE, - status=409, - detail=f"Todo list changed, {CONVEY_RELOAD_HINT}", - ) - - try: - TodoChecklist.load(target_day, facet) - except RuntimeError as exc: + except (IndexError, TodoNotMovableError, TodoMovePartialError, LockTimeout) as exc: current_app.logger.debug( - "Failed to load target todo list for %s/%s: %s", facet, target_day, exc - ) - return error_response( - TODO_OPERATION_FAILED, - detail="Unable to access target day.", + "Failed to move todo %s from %s/%s to %s/%s: %s", + index, + facet, + day, + facet, + target_day, + exc, ) - - try: - source_item = source_checklist.get_item(index) - except IndexError as exc: - current_app.logger.debug("Failed to locate todo %s on %s: %s", index, day, exc) return error_response( OPERATION_NO_LONGER_AVAILABLE, status=409, detail=f"Todo list changed, {CONVEY_RELOAD_HINT}", ) - - def _append_target(cl: TodoChecklist) -> TodoItem: - created = cl.append_entry( - source_item.text, nudge=source_item.nudge, created_at=source_item.created_at - ) - if source_item.completed: - cl.mark_done(created.index) - return created - - try: - _new_item = TodoChecklist.locked_modify(target_day, facet, _append_target) except TodoEmptyTextError as exc: current_app.logger.debug("Failed to append todo to %s: %s", target_day, exc) return error_response( @@ -681,26 +656,6 @@ def move_todo(day: str): # type: ignore[override] status=400, detail="Unable to move todo to the selected day.", ) - except LockTimeout as exc: - current_app.logger.debug("Todo list busy for %s: %s", target_day, exc) - return error_response( - OPERATION_NO_LONGER_AVAILABLE, - status=409, - detail=f"Todo list changed, {CONVEY_RELOAD_HINT}", - ) - - def _cancel_source(cl: TodoChecklist) -> None: - cl.cancel_entry(index) - - try: - TodoChecklist.locked_modify(day, facet, _cancel_source) - except LockTimeout as exc: - current_app.logger.debug("Todo list busy for %s: %s", day, exc) - return error_response( - OPERATION_NO_LONGER_AVAILABLE, - status=409, - detail=f"Todo list changed, {CONVEY_RELOAD_HINT}", - ) log_app_action( app="todos", @@ -710,8 +665,8 @@ def move_todo(day: str): # type: ignore[override] "source_day": day, "target_day": target_day, "line_number": index, - "text": source_item.text, - "completed": source_item.completed, + "text": cancelled.text, + "completed": cancelled.completed, }, day=day, ) @@ -731,7 +686,11 @@ def move_todo(day: str): # type: ignore[override] @todos_bp.route("//generate", methods=["POST"]) def generate_todos(day: str): # type: ignore[override] if not DATE_RE.fullmatch(day): - return "", 404 + return error_response( + INVALID_DAY, + status=404, + detail="Day must be in YYYYMMDD format.", + ) payload = request.get_json(silent=True) or {} facet = (payload.get("facet") or "personal").strip() @@ -791,7 +750,11 @@ Write the generated checklist to facets/{facet}/todos/{day}.jsonl""" @todos_bp.route("//generation-status") def todo_generation_status(day: str): # type: ignore[override] if not DATE_RE.fullmatch(day): - return "", 404 + return error_response( + INVALID_DAY, + status=404, + detail="Day must be in YYYYMMDD format.", + ) facet = request.args.get("facet", "personal") use_id = request.args.get("use_id") @@ -838,7 +801,11 @@ def todo_generation_status(day: str): # type: ignore[override] def generate_weekly_todos(day: str, facet: str): # type: ignore[override] """Spawn todo_weekly agent for a specific facet.""" if not DATE_RE.fullmatch(day): - return "", 404 + return error_response( + INVALID_DAY, + status=404, + detail="Day must be in YYYYMMDD format.", + ) day_date = datetime.strptime(day, "%Y%m%d") diff --git a/solstone/apps/todos/tests/test_call.py b/solstone/apps/todos/tests/test_call.py index 77c30197f..441e13ed3 100644 --- a/solstone/apps/todos/tests/test_call.py +++ b/solstone/apps/todos/tests/test_call.py @@ -511,6 +511,27 @@ class TestTodosMove: assert result.exit_code == 1 assert "does not exist" in result.output + def test_move_same_facet_rejected(self, move_env): + _, src_facet, _dst_facet = move_env([{"text": "Ship feature"}]) + + result = runner.invoke( + call_app, + [ + "todos", + "move", + "1", + "--day", + "20240101", + "--from", + src_facet, + "--to", + src_facet, + ], + ) + + assert result.exit_code == 1 + assert "source and destination facet are the same" in result.output + class TestSolEnvResolution: """Tests for SOL_* env var resolution in todos commands.""" diff --git a/solstone/apps/todos/tests/test_routes.py b/solstone/apps/todos/tests/test_routes.py new file mode 100644 index 000000000..05a8e34f1 --- /dev/null +++ b/solstone/apps/todos/tests/test_routes.py @@ -0,0 +1,198 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright (c) 2026 sol pbc + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from solstone.apps.todos.todo import TodoChecklist +from solstone.convey import create_app +from solstone.think.call import call_app + +runner = CliRunner() +STABLE_KEYS = ( + "text", + "nudge", + "created_at", + "completed", + "cancelled", + "cancelled_reason", + "moved_to", +) + + +@pytest.fixture +def todos_client(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) + + def _create(): + app = create_app(journal=str(tmp_path)) + app.config["TESTING"] = True + client = app.test_client() + with client.session_transaction() as session: + session["logged_in"] = True + session.permanent = True + return client + + return _create + + +def _ensure_facet(root: Path, facet: str) -> None: + facet_dir = root / "facets" / facet + facet_dir.mkdir(parents=True, exist_ok=True) + (facet_dir / "facet.json").write_text( + json.dumps( + { + "title": facet.title(), + "description": f"{facet} facet", + "color": "#6b7280", + } + ) + + "\n", + encoding="utf-8", + ) + + +def _write_todo(root: Path, facet: str, day: str, entry: dict) -> Path: + _ensure_facet(root, facet) + todos_dir = root / "facets" / facet / "todos" + todos_dir.mkdir(parents=True, exist_ok=True) + path = todos_dir / f"{day}.jsonl" + path.write_text(json.dumps(entry, ensure_ascii=False) + "\n", encoding="utf-8") + return path + + +def _read_jsonl(path: Path) -> list[dict]: + return [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines()] + + +def _stable_items(path: Path) -> list[dict]: + return [ + {key: item[key] for key in STABLE_KEYS if key in item} + for item in _read_jsonl(path) + ] + + +def _client_for_journal(journal: Path): + app = create_app(journal=str(journal)) + app.config["TESTING"] = True + client = app.test_client() + with client.session_transaction() as session: + session["logged_in"] = True + session.permanent = True + return client + + +def test_move_same_item_twice_via_route(tmp_path: Path, todos_client) -> None: + day = "20260415" + target_day = "20260416" + facet = "personal" + _write_todo( + tmp_path, + facet, + day, + {"text": "Move once", "created_at": 1704067200000}, + ) + + client = todos_client() + payload = {"target_day": target_day, "facet": facet, "index": 1} + + first = client.post(f"/app/todos/{day}/move", json=payload) + target_path = tmp_path / "facets" / facet / "todos" / f"{target_day}.jsonl" + assert first.status_code == 200 + assert [item["text"] for item in _read_jsonl(target_path)] == ["Move once"] + + second = client.post(f"/app/todos/{day}/move", json=payload) + assert second.status_code == 409 + assert [item["text"] for item in _read_jsonl(target_path)] == ["Move once"] + + +def test_move_route_partial_failure_preserves_source( + tmp_path: Path, todos_client, monkeypatch: pytest.MonkeyPatch +) -> None: + day = "20260417" + target_day = "20260418" + facet = "personal" + source_path = _write_todo( + tmp_path, + facet, + day, + {"text": "Partial route", "created_at": 1704067200000}, + ) + + def fail_cancel( + self: TodoChecklist, + line_number: int, + cancelled_reason: str | None = None, + moved_to: str | None = None, + ): + raise RuntimeError("cancel failed") + + monkeypatch.setattr(TodoChecklist, "cancel_entry", fail_cancel) + + response = todos_client().post( + f"/app/todos/{day}/move", + json={"target_day": target_day, "facet": facet, "index": 1}, + ) + + target_path = tmp_path / "facets" / facet / "todos" / f"{target_day}.jsonl" + assert response.status_code == 409 + assert response.get_json()["reason_code"] == "operation_no_longer_available" + assert _read_jsonl(source_path)[0].get("cancelled") is None + assert _read_jsonl(target_path)[0]["text"] == "Partial route" + + +def test_edit_move_route_cli_byte_parity( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + day = "20260419" + entry = { + "text": "Parity task", + "nudge": "20260419T10:30", + "created_at": 1704067200000, + } + + route_journal = tmp_path / "route" + _write_todo(route_journal, "work", day, entry) + _ensure_facet(route_journal, "personal") + monkeypatch.setenv("SOLSTONE_JOURNAL", str(route_journal)) + route_response = _client_for_journal(route_journal).post( + f"/app/todos/{day}", + data={ + "action": "edit", + "facet": "work", + "index": "1", + "text": "Parity task #personal", + }, + ) + assert route_response.status_code == 302 + + cli_journal = tmp_path / "cli" + _write_todo(cli_journal, "work", day, entry) + _ensure_facet(cli_journal, "personal") + monkeypatch.setenv("SOLSTONE_JOURNAL", str(cli_journal)) + cli_result = runner.invoke( + call_app, + [ + "todos", + "move", + "1", + "--day", + day, + "--from", + "work", + "--to", + "personal", + ], + ) + assert cli_result.exit_code == 0, cli_result.output + + for facet in ("work", "personal"): + relpath = Path("facets") / facet / "todos" / f"{day}.jsonl" + assert _stable_items(route_journal / relpath) == _stable_items( + cli_journal / relpath + ) diff --git a/solstone/apps/todos/tests/test_todo.py b/solstone/apps/todos/tests/test_todo.py index bc7cedf5d..49c30947d 100644 --- a/solstone/apps/todos/tests/test_todo.py +++ b/solstone/apps/todos/tests/test_todo.py @@ -12,7 +12,10 @@ import pytest from solstone.apps.todos.todo import ( TodoChecklist, + TodoEmptyTextError, TodoItem, + TodoMovePartialError, + TodoNotMovableError, find_cross_facet_matches, get_facets_with_todos, get_todos, @@ -37,6 +40,10 @@ def _write_todos(root: Path, facet: str, day: str, items: list[dict]) -> Path: return todo_path +def _read_todos(path: Path) -> list[dict]: + return [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines()] + + def test_get_todos_returns_none_when_missing(monkeypatch, journal_root): monkeypatch.setenv("SOLSTONE_JOURNAL", str(journal_root)) assert get_todos("20240101", "personal") is None @@ -476,6 +483,157 @@ def test_checklist_cancel_entry(monkeypatch, journal_root): assert data["cancelled"] is True +def test_move_entry_cross_facet_same_day(monkeypatch, journal_root): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(journal_root)) + _write_todos( + journal_root, + "work", + "20240105", + [ + { + "text": "Ship feature", + "nudge": "20240105T10:30", + "created_at": 1704067200000, + } + ], + ) + + created, cancelled = TodoChecklist.move_entry( + "20240105", "work", 1, "20240105", "personal" + ) + + assert created.text == "Ship feature" + assert created.nudge == "20240105T10:30" + assert created.created_at == 1704067200000 + assert cancelled.cancelled is True + assert cancelled.cancelled_reason == "moved_to_facet" + assert cancelled.moved_to == "personal" + source = _read_todos(journal_root / "facets" / "work" / "todos" / "20240105.jsonl") + dest = _read_todos( + journal_root / "facets" / "personal" / "todos" / "20240105.jsonl" + ) + assert source[0]["cancelled"] is True + assert source[0]["moved_to"] == "personal" + assert dest[0]["text"] == "Ship feature" + assert dest[0]["created_at"] == 1704067200000 + + +def test_move_entry_cross_day_same_facet_carries_completion(monkeypatch, journal_root): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(journal_root)) + _write_todos( + journal_root, + "personal", + "20240105", + [ + { + "text": "Done thing", + "completed": True, + "created_at": 1704067200000, + } + ], + ) + + created, cancelled = TodoChecklist.move_entry( + "20240105", "personal", 1, "20240106", "personal" + ) + + assert created.completed is True + assert cancelled.completed is True + dest = _read_todos( + journal_root / "facets" / "personal" / "todos" / "20240106.jsonl" + ) + assert dest[0]["completed"] is True + + +def test_move_entry_text_override(monkeypatch, journal_root): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(journal_root)) + _write_todos(journal_root, "work", "20240105", [{"text": "Old text"}]) + + created, cancelled = TodoChecklist.move_entry( + "20240105", "work", 1, "20240105", "personal", text="New text" + ) + + assert cancelled.text == "Old text" + assert created.text == "New text" + + +def test_move_entry_empty_text_override_preserves_source(monkeypatch, journal_root): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(journal_root)) + source_path = _write_todos(journal_root, "work", "20240105", [{"text": "Old text"}]) + + with pytest.raises(TodoEmptyTextError): + TodoChecklist.move_entry("20240105", "work", 1, "20240105", "personal", text="") + + assert "cancelled" not in _read_todos(source_path)[0] + assert not ( + journal_root / "facets" / "personal" / "todos" / "20240105.jsonl" + ).exists() + + +def test_move_entry_rejects_identical_source_and_destination(monkeypatch, journal_root): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(journal_root)) + _write_todos(journal_root, "personal", "20240105", [{"text": "Same place"}]) + + with pytest.raises(ValueError, match="identical"): + TodoChecklist.move_entry("20240105", "personal", 1, "20240105", "personal") + + +def test_move_entry_rejects_out_of_range(monkeypatch, journal_root): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(journal_root)) + _write_todos(journal_root, "work", "20240105", [{"text": "Only one"}]) + + with pytest.raises(IndexError): + TodoChecklist.move_entry("20240105", "work", 2, "20240105", "personal") + + +def test_move_entry_rejects_cancelled_source(monkeypatch, journal_root): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(journal_root)) + _write_todos( + journal_root, + "work", + "20240105", + [{"text": "Already moved", "cancelled": True}], + ) + + with pytest.raises(TodoNotMovableError, match="already cancelled"): + TodoChecklist.move_entry("20240105", "work", 1, "20240105", "personal") + + +def test_move_entry_partial_failure_preserves_source(monkeypatch, journal_root): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(journal_root)) + source_path = _write_todos( + journal_root, + "work", + "20240105", + [{"text": "Keep source", "created_at": 1704067200000}], + ) + + def fail_cancel( + self: TodoChecklist, + line_number: int, + cancelled_reason: str | None = None, + moved_to: str | None = None, + ): + raise RuntimeError("cancel failed") + + monkeypatch.setattr(TodoChecklist, "cancel_entry", fail_cancel) + + with pytest.raises(TodoMovePartialError) as exc_info: + TodoChecklist.move_entry("20240105", "work", 1, "20240105", "personal") + + assert exc_info.value.src_day == "20240105" + assert exc_info.value.src_facet == "work" + assert exc_info.value.dst_day == "20240105" + assert exc_info.value.dst_facet == "personal" + assert exc_info.value.new_index == 1 + source = _read_todos(source_path) + dest = _read_todos( + journal_root / "facets" / "personal" / "todos" / "20240105.jsonl" + ) + assert source[0].get("cancelled") is None + assert dest[0]["text"] == "Keep source" + + def test_checklist_display_includes_cancelled_with_strikethrough( monkeypatch, journal_root ): diff --git a/solstone/apps/todos/todo.py b/solstone/apps/todos/todo.py index 7d3070836..c1c8ef9a4 100644 --- a/solstone/apps/todos/todo.py +++ b/solstone/apps/todos/todo.py @@ -26,6 +26,8 @@ __all__ = [ "TodoItem", "TodoError", "TodoEmptyTextError", + "TodoNotMovableError", + "TodoMovePartialError", "find_cross_facet_matches", "parse_nudge", "format_nudge", @@ -152,6 +154,47 @@ class TodoEmptyTextError(TodoError): super().__init__("todo text cannot be empty") +class TodoNotMovableError(TodoError): + """Raised when a todo cannot be moved from its source.""" + + line_number: int + + def __init__(self, line_number: int) -> None: + self.line_number = line_number + super().__init__(f"todo {line_number} is already cancelled") + + +class TodoMovePartialError(TodoError): + """Raised when a move append succeeded but source finalization failed.""" + + src_day: str + src_facet: str + line_number: int + dst_day: str + dst_facet: str + new_index: int + + def __init__( + self, + src_day: str, + src_facet: str, + line_number: int, + dst_day: str, + dst_facet: str, + new_index: int, + ) -> None: + self.src_day = src_day + self.src_facet = src_facet + self.line_number = line_number + self.dst_day = dst_day + self.dst_facet = dst_facet + self.new_index = new_index + super().__init__( + f"todo {line_number} was appended to {dst_facet}/{dst_day} " + f"as line {new_index}, but the source could not be finalized" + ) + + @dataclass(slots=True) class TodoItem: """Structured representation of a todo entry.""" @@ -340,6 +383,58 @@ class TodoChecklist: checklist = cls.load(day, facet) return modify_fn(checklist) + @classmethod + def move_entry( + cls, + src_day: str, + src_facet: str, + line_number: int, + dst_day: str, + dst_facet: str, + *, + text: str | None = None, + ) -> tuple[TodoItem, TodoItem]: + """Move a todo entry between todo files under ordered source/dest locks.""" + src_path = todo_file_path(src_day, src_facet) + dst_path = todo_file_path(dst_day, dst_facet) + if src_path == dst_path: + raise ValueError("todo move source and destination are identical") + + first, second = sorted((src_path, dst_path), key=str) + with hold_lock(first), hold_lock(second): + source = cls.load(src_day, src_facet) + _index, item = source._get_item(line_number) + if item.cancelled: + raise TodoNotMovableError(line_number) + + effective_text = item.text if text is None else text + dest = cls.load(dst_day, dst_facet) + created = dest.append_entry( + effective_text, + nudge=item.nudge, + created_at=item.created_at, + ) + + try: + if item.completed: + dest.mark_done(created.index) + cancelled = source.cancel_entry( + line_number, + cancelled_reason="moved_to_facet", + moved_to=dst_facet, + ) + except Exception as exc: + raise TodoMovePartialError( + src_day, + src_facet, + line_number, + dst_day, + dst_facet, + created.index, + ) from exc + + return created, cancelled + def save(self) -> None: """Persist the checklist to disk via durable atomic replace.""" lines = [json.dumps(item.to_jsonl(), ensure_ascii=False) for item in self.items]