refactor(todo): 移除 due_at 字段,改用 order 字段管理象限内顺序
This commit is contained in:
@@ -0,0 +1,108 @@
|
||||
"""replace todo due_at with order field
|
||||
|
||||
Revision ID: 202603200001
|
||||
Revises: 20260319_0004
|
||||
Create Date: 2026-03-20 12:00:00
|
||||
"""
|
||||
|
||||
from typing import Sequence, Union
|
||||
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
revision: str = "202603200001"
|
||||
down_revision: Union[str, Sequence[str], None] = "20260319_0004"
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
bind = op.get_bind()
|
||||
inspector = sa.inspect(bind)
|
||||
columns = {column["name"] for column in inspector.get_columns("todos")}
|
||||
has_order = "order" in columns
|
||||
has_due_at = "due_at" in columns
|
||||
|
||||
if not has_order:
|
||||
op.add_column("todos", sa.Column("order", sa.Integer(), nullable=True))
|
||||
|
||||
if has_due_at:
|
||||
op.execute(
|
||||
"""
|
||||
WITH ranked AS (
|
||||
SELECT
|
||||
id,
|
||||
ROW_NUMBER() OVER (
|
||||
PARTITION BY owner_id, priority
|
||||
ORDER BY due_at NULLS LAST, created_at ASC, id ASC
|
||||
) - 1 AS seq
|
||||
FROM todos
|
||||
WHERE deleted_at IS NULL
|
||||
)
|
||||
UPDATE todos t
|
||||
SET "order" = ranked.seq
|
||||
FROM ranked
|
||||
WHERE t.id = ranked.id
|
||||
"""
|
||||
)
|
||||
else:
|
||||
op.execute(
|
||||
"""
|
||||
WITH ranked AS (
|
||||
SELECT
|
||||
id,
|
||||
ROW_NUMBER() OVER (
|
||||
PARTITION BY owner_id, priority
|
||||
ORDER BY created_at ASC, id ASC
|
||||
) - 1 AS seq
|
||||
FROM todos
|
||||
WHERE deleted_at IS NULL
|
||||
)
|
||||
UPDATE todos t
|
||||
SET "order" = ranked.seq
|
||||
FROM ranked
|
||||
WHERE t.id = ranked.id AND t."order" IS NULL
|
||||
"""
|
||||
)
|
||||
|
||||
op.execute('UPDATE todos SET "order" = 0 WHERE "order" IS NULL')
|
||||
op.alter_column("todos", "order", nullable=False, server_default=sa.text("0"))
|
||||
|
||||
op.execute("DROP INDEX IF EXISTS ix_todos_owner_status_due")
|
||||
op.execute("DROP INDEX IF EXISTS ix_todos_pending_due")
|
||||
|
||||
op.execute(
|
||||
'CREATE INDEX IF NOT EXISTS ix_todos_owner_status_order ON todos (owner_id, status, priority, "order")'
|
||||
)
|
||||
op.execute(
|
||||
"CREATE INDEX IF NOT EXISTS ix_todos_pending_order ON todos (owner_id, priority, \"order\") WHERE status = 'pending'"
|
||||
)
|
||||
|
||||
if has_due_at:
|
||||
op.drop_column("todos", "due_at")
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
bind = op.get_bind()
|
||||
inspector = sa.inspect(bind)
|
||||
columns = {column["name"] for column in inspector.get_columns("todos")}
|
||||
has_order = "order" in columns
|
||||
has_due_at = "due_at" in columns
|
||||
|
||||
if not has_due_at:
|
||||
op.add_column(
|
||||
"todos", sa.Column("due_at", sa.DateTime(timezone=True), nullable=True)
|
||||
)
|
||||
|
||||
op.execute("DROP INDEX IF EXISTS ix_todos_pending_order")
|
||||
op.execute("DROP INDEX IF EXISTS ix_todos_owner_status_order")
|
||||
|
||||
op.execute(
|
||||
"CREATE INDEX IF NOT EXISTS ix_todos_owner_status_due ON todos (owner_id, status, due_at)"
|
||||
)
|
||||
op.execute(
|
||||
"CREATE INDEX IF NOT EXISTS ix_todos_pending_due ON todos (owner_id, due_at) WHERE status = 'pending'"
|
||||
)
|
||||
|
||||
if has_order:
|
||||
op.drop_column("todos", "order")
|
||||
@@ -26,7 +26,7 @@ class TodoPriority(int, Enum):
|
||||
|
||||
class Todo(TimestampMixin, SoftDeleteMixin, Base):
|
||||
__tablename__: str = "todos"
|
||||
__table_args__ = {"extend_existing": True}
|
||||
__table_args__: dict[str, bool] = {"extend_existing": True}
|
||||
|
||||
id: Mapped[uuid.UUID] = mapped_column(
|
||||
UUID(as_uuid=True), primary_key=True, default=uuid.uuid4
|
||||
@@ -43,9 +43,10 @@ class Todo(TimestampMixin, SoftDeleteMixin, Base):
|
||||
String(1000),
|
||||
nullable=True,
|
||||
)
|
||||
due_at: Mapped[datetime | None] = mapped_column(
|
||||
DateTime(timezone=True),
|
||||
nullable=True,
|
||||
order: Mapped[int] = mapped_column(
|
||||
Integer,
|
||||
nullable=False,
|
||||
default=0,
|
||||
)
|
||||
priority: Mapped[int] = mapped_column(
|
||||
Integer,
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
from .contracts import TodoOrder
|
||||
|
||||
__all__ = ["TodoOrder"]
|
||||
@@ -0,0 +1,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Annotated
|
||||
|
||||
from pydantic import Field
|
||||
|
||||
TodoOrder = Annotated[int, Field(ge=0)]
|
||||
@@ -26,8 +26,8 @@ class TodoRepository(Protocol):
|
||||
owner_id: UUID,
|
||||
title: str,
|
||||
description: str | None = None,
|
||||
due_at: datetime | None = None,
|
||||
priority: int = TodoPriority.IMPORTANT_URGENT,
|
||||
order: int = 0,
|
||||
created_by: UUID | None = None,
|
||||
) -> Todo:
|
||||
"""Create a new todo."""
|
||||
@@ -42,8 +42,8 @@ class TodoRepository(Protocol):
|
||||
todo: Todo,
|
||||
title: str | None = None,
|
||||
description: str | None = None,
|
||||
due_at: datetime | None = None,
|
||||
priority: int | None = None,
|
||||
order: int | None = None,
|
||||
status: TodoStatus | None = None,
|
||||
completed_at: datetime | None = None,
|
||||
) -> Todo:
|
||||
@@ -79,6 +79,8 @@ class SQLAlchemyTodoRepository(BaseRepository[Todo]):
|
||||
- No HTTP exceptions - returns None or raises SQLAlchemyError
|
||||
"""
|
||||
|
||||
_session: AsyncSession
|
||||
|
||||
def __init__(self, session: AsyncSession) -> None:
|
||||
super().__init__(session, Todo)
|
||||
self._session = session
|
||||
@@ -88,8 +90,8 @@ class SQLAlchemyTodoRepository(BaseRepository[Todo]):
|
||||
owner_id: UUID,
|
||||
title: str,
|
||||
description: str | None = None,
|
||||
due_at: datetime | None = None,
|
||||
priority: int = TodoPriority.IMPORTANT_URGENT,
|
||||
order: int = 0,
|
||||
created_by: UUID | None = None,
|
||||
) -> Todo:
|
||||
try:
|
||||
@@ -97,8 +99,8 @@ class SQLAlchemyTodoRepository(BaseRepository[Todo]):
|
||||
owner_id=owner_id,
|
||||
title=title,
|
||||
description=description,
|
||||
due_at=due_at,
|
||||
priority=priority,
|
||||
order=order,
|
||||
status=TodoStatus.PENDING,
|
||||
created_by=created_by,
|
||||
)
|
||||
@@ -128,8 +130,8 @@ class SQLAlchemyTodoRepository(BaseRepository[Todo]):
|
||||
todo: Todo,
|
||||
title: str | None = None,
|
||||
description: str | None = None,
|
||||
due_at: datetime | None = None,
|
||||
priority: int | None = None,
|
||||
order: int | None = None,
|
||||
status: TodoStatus | None = None,
|
||||
completed_at: datetime | None = None,
|
||||
) -> Todo:
|
||||
@@ -138,10 +140,10 @@ class SQLAlchemyTodoRepository(BaseRepository[Todo]):
|
||||
todo.title = title
|
||||
if description is not None:
|
||||
todo.description = description
|
||||
if due_at is not None:
|
||||
todo.due_at = due_at
|
||||
if priority is not None:
|
||||
todo.priority = priority
|
||||
if order is not None:
|
||||
todo.order = order
|
||||
if status is not None:
|
||||
todo.status = status
|
||||
if completed_at is not None:
|
||||
@@ -167,7 +169,7 @@ class SQLAlchemyTodoRepository(BaseRepository[Todo]):
|
||||
select(Todo)
|
||||
.where(Todo.owner_id == owner_id)
|
||||
.where(Todo.deleted_at.is_(None))
|
||||
.order_by(Todo.priority.asc(), Todo.due_at.asc().nullslast())
|
||||
.order_by(Todo.priority.asc(), Todo.order.asc(), Todo.created_at.asc())
|
||||
)
|
||||
|
||||
if status is not None:
|
||||
|
||||
@@ -6,7 +6,13 @@ from uuid import UUID
|
||||
from fastapi import APIRouter, Depends, Query, status
|
||||
|
||||
from v1.todo.dependencies import get_todo_service
|
||||
from v1.todo.schemas import TodoComplete, TodoCreate, TodoResponse, TodoUpdate
|
||||
from v1.todo.schemas import (
|
||||
TodoComplete,
|
||||
TodoCreate,
|
||||
TodoReorderRequest,
|
||||
TodoResponse,
|
||||
TodoUpdate,
|
||||
)
|
||||
from v1.todo.service import TodoService
|
||||
|
||||
|
||||
@@ -42,6 +48,17 @@ async def get_todo(
|
||||
return await service.get_by_id(todo_id)
|
||||
|
||||
|
||||
@router.patch(
|
||||
"/reorder",
|
||||
status_code=status.HTTP_204_NO_CONTENT,
|
||||
)
|
||||
async def reorder_todos(
|
||||
payload: TodoReorderRequest,
|
||||
service: Annotated[TodoService, Depends(get_todo_service)],
|
||||
) -> None:
|
||||
await service.reorder(payload)
|
||||
|
||||
|
||||
@router.patch("/{todo_id}", response_model=TodoResponse)
|
||||
async def update_todo(
|
||||
todo_id: UUID,
|
||||
|
||||
@@ -5,14 +5,16 @@ from uuid import UUID
|
||||
|
||||
from pydantic import BaseModel, ConfigDict, Field
|
||||
|
||||
from schemas.todo import TodoOrder
|
||||
|
||||
|
||||
class TodoCreate(BaseModel):
|
||||
model_config: ClassVar[ConfigDict] = ConfigDict(extra="forbid")
|
||||
|
||||
title: str = Field(..., min_length=1, max_length=255)
|
||||
description: str | None = Field(None, max_length=1000)
|
||||
due_at: datetime | None = None
|
||||
priority: int = Field(1, ge=1, le=4)
|
||||
order: TodoOrder | None = None
|
||||
schedule_item_ids: list[UUID] = Field(default_factory=list)
|
||||
|
||||
|
||||
@@ -21,8 +23,8 @@ class TodoUpdate(BaseModel):
|
||||
|
||||
title: str | None = Field(None, min_length=1, max_length=255)
|
||||
description: str | None = Field(None, max_length=1000)
|
||||
due_at: datetime | None = None
|
||||
priority: int | None = Field(None, ge=1, le=4)
|
||||
order: TodoOrder | None = None
|
||||
status: Literal["pending", "done", "canceled"] | None = None
|
||||
schedule_item_ids: list[UUID] | None = None
|
||||
|
||||
@@ -43,8 +45,8 @@ class TodoResponse(BaseModel):
|
||||
owner_id: UUID
|
||||
title: str
|
||||
description: str | None
|
||||
due_at: datetime | None
|
||||
priority: int
|
||||
order: TodoOrder
|
||||
status: str
|
||||
completed_at: datetime | None
|
||||
created_at: datetime
|
||||
@@ -52,6 +54,20 @@ class TodoResponse(BaseModel):
|
||||
schedule_items: list[ScheduleItemBasic] = Field(default_factory=list)
|
||||
|
||||
|
||||
class TodoReorderItem(BaseModel):
|
||||
model_config: ClassVar[ConfigDict] = ConfigDict(extra="forbid")
|
||||
|
||||
id: UUID
|
||||
priority: int = Field(..., ge=1, le=4)
|
||||
order: TodoOrder
|
||||
|
||||
|
||||
class TodoReorderRequest(BaseModel):
|
||||
model_config: ClassVar[ConfigDict] = ConfigDict(extra="forbid")
|
||||
|
||||
items: list[TodoReorderItem] = Field(..., min_length=1)
|
||||
|
||||
|
||||
class TodoComplete(BaseModel):
|
||||
model_config: ClassVar[ConfigDict] = ConfigDict(extra="forbid")
|
||||
|
||||
|
||||
@@ -13,7 +13,13 @@ from core.logging import get_logger
|
||||
from models.todos import Todo, TodoStatus
|
||||
from v1.schedule_items.repository import SQLAlchemyScheduleItemRepository
|
||||
from v1.todo.repository import TodoRepository
|
||||
from v1.todo.schemas import ScheduleItemBasic, TodoCreate, TodoResponse, TodoUpdate
|
||||
from v1.todo.schemas import (
|
||||
ScheduleItemBasic,
|
||||
TodoCreate,
|
||||
TodoReorderRequest,
|
||||
TodoResponse,
|
||||
TodoUpdate,
|
||||
)
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
@@ -52,12 +58,20 @@ class TodoService(BaseService):
|
||||
user_id = self.require_user_id()
|
||||
|
||||
try:
|
||||
order_value = request.order
|
||||
if order_value is None:
|
||||
todos_in_priority = await self._repository.list_by_owner(
|
||||
owner_id=user_id,
|
||||
priority=request.priority,
|
||||
)
|
||||
order_value = len(todos_in_priority)
|
||||
|
||||
todo = await self._repository.create(
|
||||
owner_id=user_id,
|
||||
title=request.title,
|
||||
description=request.description,
|
||||
due_at=request.due_at,
|
||||
priority=request.priority,
|
||||
order=order_value,
|
||||
created_by=user_id,
|
||||
)
|
||||
|
||||
@@ -144,8 +158,8 @@ class TodoService(BaseService):
|
||||
todo,
|
||||
title=request.title,
|
||||
description=request.description,
|
||||
due_at=request.due_at,
|
||||
priority=request.priority,
|
||||
order=request.order,
|
||||
status=status_enum,
|
||||
completed_at=completed_at,
|
||||
)
|
||||
@@ -253,6 +267,53 @@ class TodoService(BaseService):
|
||||
},
|
||||
)
|
||||
|
||||
async def reorder(self, request: TodoReorderRequest) -> None:
|
||||
user_id = self.require_user_id()
|
||||
|
||||
seen_ids: set[UUID] = set()
|
||||
original_priorities: set[int] = set()
|
||||
target_priorities: set[int] = set()
|
||||
|
||||
try:
|
||||
for item in request.items:
|
||||
if item.id in seen_ids:
|
||||
raise HTTPException(status_code=400, detail="Duplicate todo id")
|
||||
seen_ids.add(item.id)
|
||||
|
||||
todo = await self._repository.get_by_id(item.id)
|
||||
if todo is None:
|
||||
raise HTTPException(status_code=404, detail="Todo not found")
|
||||
if todo.owner_id != user_id:
|
||||
raise HTTPException(
|
||||
status_code=403, detail="Not authorized to reorder this todo"
|
||||
)
|
||||
|
||||
original_priorities.add(todo.priority)
|
||||
target_priorities.add(item.priority)
|
||||
|
||||
await self._repository.update(
|
||||
todo,
|
||||
priority=item.priority,
|
||||
order=item.order,
|
||||
)
|
||||
|
||||
affected_priorities = original_priorities.union(target_priorities)
|
||||
for priority in affected_priorities:
|
||||
todos = await self._repository.list_by_owner(
|
||||
owner_id=user_id,
|
||||
status=TodoStatus.PENDING,
|
||||
priority=priority,
|
||||
)
|
||||
todos.sort(key=lambda current: (current.order, current.created_at))
|
||||
for index, todo in enumerate(todos):
|
||||
if todo.order != index:
|
||||
await self._repository.update(todo, order=index)
|
||||
|
||||
await self._session.commit()
|
||||
except SQLAlchemyError:
|
||||
await self._session.rollback()
|
||||
raise HTTPException(status_code=503, detail="Todo service unavailable")
|
||||
|
||||
async def list_todos(
|
||||
self,
|
||||
status: str | None = None,
|
||||
@@ -287,7 +348,7 @@ class TodoService(BaseService):
|
||||
)
|
||||
|
||||
schedule_item_ids = await self._repository.get_schedule_items(todo.id)
|
||||
schedule_items = []
|
||||
schedule_items: list[ScheduleItemBasic] = []
|
||||
for item_id in schedule_item_ids:
|
||||
item = await self._schedule_item_repository.get_by_id(item_id)
|
||||
if item:
|
||||
@@ -305,8 +366,8 @@ class TodoService(BaseService):
|
||||
owner_id=todo.owner_id,
|
||||
title=todo.title,
|
||||
description=todo.description,
|
||||
due_at=todo.due_at,
|
||||
priority=todo.priority,
|
||||
order=todo.order,
|
||||
status=status_value,
|
||||
completed_at=todo.completed_at,
|
||||
created_at=todo.created_at,
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
from pydantic import ValidationError
|
||||
|
||||
from v1.todo.schemas import TodoCreate, TodoReorderRequest, TodoResponse, TodoUpdate
|
||||
|
||||
|
||||
def test_todo_create_accepts_zero_based_order() -> None:
|
||||
request = TodoCreate.model_validate({"title": "Test", "priority": 1, "order": 0})
|
||||
|
||||
assert request.order == 0
|
||||
|
||||
|
||||
def test_todo_create_rejects_due_at_field() -> None:
|
||||
try:
|
||||
TodoCreate.model_validate(
|
||||
{
|
||||
"title": "Test",
|
||||
"priority": 1,
|
||||
"order": 0,
|
||||
"due_at": "2026-01-01T00:00:00Z",
|
||||
}
|
||||
)
|
||||
except ValidationError:
|
||||
return
|
||||
|
||||
raise AssertionError("TodoCreate should reject due_at")
|
||||
|
||||
|
||||
def test_todo_update_rejects_negative_order() -> None:
|
||||
try:
|
||||
TodoUpdate.model_validate({"order": -1})
|
||||
except ValidationError:
|
||||
return
|
||||
|
||||
raise AssertionError("TodoUpdate should reject negative order")
|
||||
|
||||
|
||||
def test_todo_response_contains_order_without_due_at() -> None:
|
||||
payload = {
|
||||
"id": "9d72aa0f-e2d5-4de3-9b2d-4609f53376c0",
|
||||
"owner_id": "2f5b38fb-cb6f-44ef-8824-e7978f644bc2",
|
||||
"title": "Test",
|
||||
"description": None,
|
||||
"priority": 1,
|
||||
"order": 0,
|
||||
"status": "pending",
|
||||
"completed_at": None,
|
||||
"created_at": "2026-03-20T00:00:00Z",
|
||||
"updated_at": "2026-03-20T00:00:00Z",
|
||||
"schedule_items": [],
|
||||
}
|
||||
|
||||
response = TodoResponse.model_validate(payload)
|
||||
dumped = response.model_dump()
|
||||
|
||||
assert dumped["order"] == 0
|
||||
assert "due_at" not in dumped
|
||||
|
||||
|
||||
def test_todo_reorder_request_requires_non_empty_items() -> None:
|
||||
try:
|
||||
TodoReorderRequest(items=[])
|
||||
except ValidationError:
|
||||
return
|
||||
|
||||
raise AssertionError("TodoReorderRequest should reject empty items")
|
||||
Reference in New Issue
Block a user