diff --git a/src/thoughtsync/labeling.py b/src/thoughtsync/labeling.py new file mode 100644 index 0000000..9b07f3f --- /dev/null +++ b/src/thoughtsync/labeling.py @@ -0,0 +1,36 @@ +"""Shared note↔label membership reconciliation. The "set a note's MANUAL (picker) +labels, leave the tag-sourced ones alone" logic was duplicated line-for-line between +the labels-picker API (notes.set_note_labels) and sync push (sync._apply_note_manual_labels). +Single home so both stay in lockstep. via_tag=True rows track the body #tags and are +governed by _reconcile_tags — this function never touches them.""" +from __future__ import annotations + +from sqlalchemy import select + +from .models.label import Label, NoteLabel +from .models.note import Note + + +async def resolve_owned_label_ids(db, label_ids, owner_id) -> set: + """Of `label_ids` (an iterable of UUIDs), the subset actually owned by `owner_id`. + Callers parse/validate the raw ids first; this just enforces ownership.""" + ids = list(label_ids) + if not ids: + return set() + return set( + (await db.scalars(select(Label.id).where(Label.owner_id == owner_id, Label.id.in_(ids)))).all() + ) + + +async def reconcile_manual_labels(db, note: Note, owned_label_ids: set) -> None: + """Make the note's MANUAL (via_tag=False) memberships exactly `owned_label_ids`: + drop manual rows no longer wanted, add missing ones. Tag-sourced rows survive + untouched. Caller commits.""" + existing = (await db.scalars(select(NoteLabel).where(NoteLabel.note_id == note.id))).all() + attached = {r.label_id for r in existing} + for r in existing: + if not r.via_tag and r.label_id not in owned_label_ids: + await db.delete(r) + for lid in owned_label_ids: + if lid not in attached: + db.add(NoteLabel(note_id=note.id, label_id=lid, via_tag=False)) diff --git a/src/thoughtsync/labels.py b/src/thoughtsync/labels.py index 5a7e6d8..5a2ec1d 100644 --- a/src/thoughtsync/labels.py +++ b/src/thoughtsync/labels.py @@ -1,26 +1,20 @@ from __future__ import annotations -import uuid - from quart import Blueprint, g, jsonify, request from sqlalchemy import func, select from .auth import login_required +from .colors import normalize_color from .db import session_scope from .models.label import Label, NoteLabel +from .responses import json_error, not_found, parse_uuid +from .serialize import serialize_label bp = Blueprint("labels", __name__, url_prefix="/api/labels") -LABEL_COLORS = {"default", "red", "orange", "yellow", "green", "teal", "blue", "purple", "pink", "gray"} - - -def _normalize_label_color(color: object) -> str: - return color if color in LABEL_COLORS else "default" - - def _serialize_label(label: Label, count: int | None = None) -> dict: - data = {"id": str(label.id), "name": label.name, "color": label.color} + data = serialize_label(label) if count is not None: data["count"] = count return data @@ -34,9 +28,8 @@ async def _label_note_count(db, label_id) -> int: async def _get_owned_label(db, label_id: str) -> Label | None: - try: - lid = uuid.UUID(label_id) - except (ValueError, TypeError): + lid = parse_uuid(label_id) + if lid is None: return None return await db.scalar(select(Label).where(Label.id == lid, Label.owner_id == g.user_id)) @@ -65,13 +58,13 @@ async def create_label(): data = await request.get_json(silent=True) or {} name = (data.get("name") or "").strip() if not name: - return jsonify({"error": "label name is required"}), 400 + return json_error("label name is required", 400) async with session_scope() as db: # Idempotent: creating an existing label just returns it. existing = await db.scalar(select(Label).where(Label.owner_id == g.user_id, Label.name == name)) if existing is not None: return jsonify(_serialize_label(existing)), 200 - label = Label(owner_id=g.user_id, name=name, color=_normalize_label_color(data.get("color"))) + label = Label(owner_id=g.user_id, name=name, color=normalize_color(data.get("color"))) db.add(label) await db.commit() await db.refresh(label) @@ -85,23 +78,23 @@ async def update_label(label_id: str): has_name = "name" in data has_color = "color" in data if not has_name and not has_color: - return jsonify({"error": "nothing to update"}), 400 + return json_error("nothing to update", 400) name = (data.get("name") or "").strip() if has_name else None if has_name and not name: - return jsonify({"error": "label name is required"}), 400 + return json_error("label name is required", 400) async with session_scope() as db: label = await _get_owned_label(db, label_id) if label is None: - return jsonify({"error": "not found"}), 404 + return not_found() if has_name: clash = await db.scalar( select(Label).where(Label.owner_id == g.user_id, Label.name == name, Label.id != label.id) ) if clash is not None: - return jsonify({"error": "a label with that name already exists"}), 409 + return json_error("a label with that name already exists", 409) label.name = name if has_color: - label.color = _normalize_label_color(data.get("color")) + label.color = normalize_color(data.get("color")) await db.commit() await db.refresh(label) return jsonify(_serialize_label(label)) @@ -113,7 +106,7 @@ async def delete_label(label_id: str): async with session_scope() as db: label = await _get_owned_label(db, label_id) if label is None: - return jsonify({"error": "not found"}), 404 + return not_found() await db.delete(label) # note_labels rows cascade await db.commit() return jsonify({"ok": True}) @@ -133,9 +126,9 @@ async def merge_label(label_id: str): source = await _get_owned_label(db, label_id) target = await _get_owned_label(db, str(into)) if into is not None else None if source is None or target is None: - return jsonify({"error": "not found"}), 404 + return not_found() if source.id == target.id: - return jsonify({"error": "cannot merge a label into itself"}), 400 + return json_error("cannot merge a label into itself", 400) # Notes already carrying the target: a note can't hold the same label twice # (composite PK), so the source attachment there is just dropped as a dup. target_notes = set( diff --git a/src/thoughtsync/notes.py b/src/thoughtsync/notes.py index d8db5af..4f1709c 100644 --- a/src/thoughtsync/notes.py +++ b/src/thoughtsync/notes.py @@ -20,6 +20,7 @@ from .colors import NOTE_COLORS, normalize_color from .common import coerce_bool, iso, parse_dt from .config import Config from .db import session_scope +from .labeling import reconcile_manual_labels, resolve_owned_label_ids from .responses import json_error, not_found, parse_uuid from .settings import get_setting from .unfurl import UnfurlError, unfurl @@ -1254,26 +1255,10 @@ async def set_note_labels(note_id: str): note = await _get_owned(db, note_id) if note is None: return not_found() - owned: set = set() - if label_ids: - owned = set( - ( - await db.scalars( - select(Label.id).where(Label.owner_id == g.user_id, Label.id.in_(label_ids)) - ) - ).all() - ) # The picker manages MANUAL labels only; tag-sourced (via_tag=True) rows are - # governed by the body #tags and must survive a picker save. - chosen = {lid for lid in label_ids if lid in owned} - existing = (await db.scalars(select(NoteLabel).where(NoteLabel.note_id == note.id))).all() - attached_ids = {r.label_id for r in existing} - for r in existing: - if not r.via_tag and r.label_id not in chosen: - await db.delete(r) - for lid in chosen: - if lid not in attached_ids: - db.add(NoteLabel(note_id=note.id, label_id=lid, via_tag=False)) + # governed by the body #tags and survive a picker save (see labeling module). + owned = await resolve_owned_label_ids(db, label_ids, g.user_id) + await reconcile_manual_labels(db, note, owned) await db.commit() return jsonify(await _serialize_note(db, note)) diff --git a/src/thoughtsync/saved_filters.py b/src/thoughtsync/saved_filters.py index c9ef2a8..35af67d 100644 --- a/src/thoughtsync/saved_filters.py +++ b/src/thoughtsync/saved_filters.py @@ -3,7 +3,6 @@ in one click. Owner-scoped CRUD; `params` mirrors the GET /api/notes facet query from __future__ import annotations import json -import uuid from quart import Blueprint, g, jsonify, request from sqlalchemy import func, select @@ -11,6 +10,7 @@ from sqlalchemy import func, select from .auth import login_required from .db import session_scope from .models.saved_filter import SavedFilter +from .responses import json_error, not_found, parse_uuid bp = Blueprint("saved_filters", __name__, url_prefix="/api/saved-filters") @@ -64,7 +64,7 @@ async def create_saved(): data = await request.get_json(silent=True) or {} name = (data.get("name") or "").strip() if not name: - return jsonify({"error": "name is required"}), 400 + return json_error("name is required", 400) params = clean_params(data.get("params")) async with session_scope() as db: max_pos = await db.scalar( @@ -82,21 +82,20 @@ async def create_saved(): @bp.patch("/") @login_required async def rename_saved(filter_id: str): - try: - fid = uuid.UUID(filter_id) - except (ValueError, TypeError): - return jsonify({"error": "not found"}), 404 + fid = parse_uuid(filter_id) + if fid is None: + return not_found() data = await request.get_json(silent=True) or {} async with session_scope() as db: sf = await db.scalar( select(SavedFilter).where(SavedFilter.id == fid, SavedFilter.owner_id == g.user_id) ) if sf is None: - return jsonify({"error": "not found"}), 404 + return not_found() if "name" in data: name = (data.get("name") or "").strip() if not name: - return jsonify({"error": "name is required"}), 400 + return json_error("name is required", 400) sf.name = name[:NAME_CAP] if "params" in data: sf.params = json.dumps(clean_params(data.get("params"))) @@ -108,16 +107,15 @@ async def rename_saved(filter_id: str): @bp.delete("/") @login_required async def delete_saved(filter_id: str): - try: - fid = uuid.UUID(filter_id) - except (ValueError, TypeError): - return jsonify({"error": "not found"}), 404 + fid = parse_uuid(filter_id) + if fid is None: + return not_found() async with session_scope() as db: sf = await db.scalar( select(SavedFilter).where(SavedFilter.id == fid, SavedFilter.owner_id == g.user_id) ) if sf is None: - return jsonify({"error": "not found"}), 404 + return not_found() await db.delete(sf) await db.commit() return jsonify({"ok": True}) diff --git a/src/thoughtsync/serialize.py b/src/thoughtsync/serialize.py new file mode 100644 index 0000000..645dae5 --- /dev/null +++ b/src/thoughtsync/serialize.py @@ -0,0 +1,14 @@ +"""Shared JSON serializers — the single home for turning ORM rows into the dict +shapes the API returns, so a field name or format is defined once. Each entity has a +BASE serializer with the fields common to every context; callers layer on the +extras their surface needs (usage count in the labels API, sync_revision/purged_at +in sync deltas). Grows as sections adopt it (labels here; note/user/device later).""" +from __future__ import annotations + +from .models.label import Label + + +def serialize_label(label: Label) -> dict: + """Base label shape {id, name, color}. The labels API adds `count`; sync deltas + (S4) add `sync_revision`/`purged_at`/`created_at` on top of this.""" + return {"id": str(label.id), "name": label.name, "color": label.color} diff --git a/src/thoughtsync/sync.py b/src/thoughtsync/sync.py index 76c2e59..a105d2d 100644 --- a/src/thoughtsync/sync.py +++ b/src/thoughtsync/sync.py @@ -21,6 +21,7 @@ from sqlalchemy import func, select from .auth import login_required from .config import Config from .db import session_scope +from .labeling import reconcile_manual_labels, resolve_owned_label_ids from .models.label import Label, NoteLabel from .models.note import Note from .models.note_attachment import NoteAttachment @@ -212,20 +213,9 @@ async def _apply_note_manual_labels(db, note: Note, ch: dict) -> None: try: wanted.add(uuid.UUID(str(r))) except (ValueError, TypeError): - continue - owned: set = set() - if wanted: - owned = set( - (await db.scalars(select(Label.id).where(Label.owner_id == g.user_id, Label.id.in_(wanted)))).all() - ) - existing = (await db.scalars(select(NoteLabel).where(NoteLabel.note_id == note.id))).all() - attached = {r.label_id for r in existing} - for r in existing: - if not r.via_tag and r.label_id not in owned: - await db.delete(r) - for lid in owned: - if lid not in attached: - db.add(NoteLabel(note_id=note.id, label_id=lid, via_tag=False)) + continue # sync is lenient: skip a malformed id rather than reject the push + owned = await resolve_owned_label_ids(db, wanted, g.user_id) + await reconcile_manual_labels(db, note, owned) async def _purge_note(db, note: Note, edited_at: datetime | None) -> None: diff --git a/tests/test_labels.py b/tests/test_labels.py index fa52856..85d28f2 100644 --- a/tests/test_labels.py +++ b/tests/test_labels.py @@ -1,6 +1,10 @@ +import uuid + import pytest from thoughtsync.app import create_app +from thoughtsync.models.label import Label +from thoughtsync.serialize import serialize_label @pytest.fixture @@ -8,6 +12,16 @@ def app(): return create_app() +def test_serialize_label_shape(): + # The base {id, name, color} shape the labels API and (S4) sync deltas both build on. + lid = uuid.uuid4() + assert serialize_label(Label(id=lid, name="ideas", color="teal")) == { + "id": str(lid), + "name": "ideas", + "color": "teal", + } + + async def test_labels_requires_auth(app): client = app.test_client() resp = await client.get("/api/labels")