M9 S3 (backend): organize routes adopt toolkit + shared label reconciliation
DRY across the "organize/recall" backend surface:
- serialize.py (new): serialize_label(label) — the base {id,name,color}
shape. labels.py builds on it (adds count); sync deltas will (S4).
- labeling.py (new): resolve_owned_label_ids() + reconcile_manual_labels()
— the "set a note's MANUAL (picker) labels, leave the via_tag rows alone"
logic was duplicated line-for-line between notes.set_note_labels and
sync._apply_note_manual_labels. Now one home; both adopt it (removes the
redundant `chosen`==owned recompute in notes). Behavior-preserving.
- labels.py: json_error/not_found/parse_uuid, colors.normalize_color, and
serialize_label; dropped local LABEL_COLORS + _normalize_label_color
(NOTE_COLORS is the single palette) and `import uuid` (rule 22).
- saved_filters.py: json_error/not_found/parse_uuid for its 2 uuid parses
+ error shapes.
- graph.py: no change — no error/uuid/palette-normalize duplication to fold.
Test: DB-free test_serialize_label_shape guards the base shape.
sync.py's reconciliation swap is behavior-identical; operator-verified on
deploy (no Postgres CI lane).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FRgehjoz7Yv8LkUfADxACm
This commit is contained in:
@@ -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))
|
||||||
+16
-23
@@ -1,26 +1,20 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import uuid
|
|
||||||
|
|
||||||
from quart import Blueprint, g, jsonify, request
|
from quart import Blueprint, g, jsonify, request
|
||||||
from sqlalchemy import func, select
|
from sqlalchemy import func, select
|
||||||
|
|
||||||
from .auth import login_required
|
from .auth import login_required
|
||||||
|
from .colors import normalize_color
|
||||||
from .db import session_scope
|
from .db import session_scope
|
||||||
from .models.label import Label, NoteLabel
|
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")
|
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:
|
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:
|
if count is not None:
|
||||||
data["count"] = count
|
data["count"] = count
|
||||||
return data
|
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:
|
async def _get_owned_label(db, label_id: str) -> Label | None:
|
||||||
try:
|
lid = parse_uuid(label_id)
|
||||||
lid = uuid.UUID(label_id)
|
if lid is None:
|
||||||
except (ValueError, TypeError):
|
|
||||||
return None
|
return None
|
||||||
return await db.scalar(select(Label).where(Label.id == lid, Label.owner_id == g.user_id))
|
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 {}
|
data = await request.get_json(silent=True) or {}
|
||||||
name = (data.get("name") or "").strip()
|
name = (data.get("name") or "").strip()
|
||||||
if not name:
|
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:
|
async with session_scope() as db:
|
||||||
# Idempotent: creating an existing label just returns it.
|
# Idempotent: creating an existing label just returns it.
|
||||||
existing = await db.scalar(select(Label).where(Label.owner_id == g.user_id, Label.name == name))
|
existing = await db.scalar(select(Label).where(Label.owner_id == g.user_id, Label.name == name))
|
||||||
if existing is not None:
|
if existing is not None:
|
||||||
return jsonify(_serialize_label(existing)), 200
|
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)
|
db.add(label)
|
||||||
await db.commit()
|
await db.commit()
|
||||||
await db.refresh(label)
|
await db.refresh(label)
|
||||||
@@ -85,23 +78,23 @@ async def update_label(label_id: str):
|
|||||||
has_name = "name" in data
|
has_name = "name" in data
|
||||||
has_color = "color" in data
|
has_color = "color" in data
|
||||||
if not has_name and not has_color:
|
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
|
name = (data.get("name") or "").strip() if has_name else None
|
||||||
if has_name and not name:
|
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:
|
async with session_scope() as db:
|
||||||
label = await _get_owned_label(db, label_id)
|
label = await _get_owned_label(db, label_id)
|
||||||
if label is None:
|
if label is None:
|
||||||
return jsonify({"error": "not found"}), 404
|
return not_found()
|
||||||
if has_name:
|
if has_name:
|
||||||
clash = await db.scalar(
|
clash = await db.scalar(
|
||||||
select(Label).where(Label.owner_id == g.user_id, Label.name == name, Label.id != label.id)
|
select(Label).where(Label.owner_id == g.user_id, Label.name == name, Label.id != label.id)
|
||||||
)
|
)
|
||||||
if clash is not None:
|
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
|
label.name = name
|
||||||
if has_color:
|
if has_color:
|
||||||
label.color = _normalize_label_color(data.get("color"))
|
label.color = normalize_color(data.get("color"))
|
||||||
await db.commit()
|
await db.commit()
|
||||||
await db.refresh(label)
|
await db.refresh(label)
|
||||||
return jsonify(_serialize_label(label))
|
return jsonify(_serialize_label(label))
|
||||||
@@ -113,7 +106,7 @@ async def delete_label(label_id: str):
|
|||||||
async with session_scope() as db:
|
async with session_scope() as db:
|
||||||
label = await _get_owned_label(db, label_id)
|
label = await _get_owned_label(db, label_id)
|
||||||
if label is None:
|
if label is None:
|
||||||
return jsonify({"error": "not found"}), 404
|
return not_found()
|
||||||
await db.delete(label) # note_labels rows cascade
|
await db.delete(label) # note_labels rows cascade
|
||||||
await db.commit()
|
await db.commit()
|
||||||
return jsonify({"ok": True})
|
return jsonify({"ok": True})
|
||||||
@@ -133,9 +126,9 @@ async def merge_label(label_id: str):
|
|||||||
source = await _get_owned_label(db, label_id)
|
source = await _get_owned_label(db, label_id)
|
||||||
target = await _get_owned_label(db, str(into)) if into is not None else None
|
target = await _get_owned_label(db, str(into)) if into is not None else None
|
||||||
if source is None or target is None:
|
if source is None or target is None:
|
||||||
return jsonify({"error": "not found"}), 404
|
return not_found()
|
||||||
if source.id == target.id:
|
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
|
# 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.
|
# (composite PK), so the source attachment there is just dropped as a dup.
|
||||||
target_notes = set(
|
target_notes = set(
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ from .colors import NOTE_COLORS, normalize_color
|
|||||||
from .common import coerce_bool, iso, parse_dt
|
from .common import coerce_bool, iso, parse_dt
|
||||||
from .config import Config
|
from .config import Config
|
||||||
from .db import session_scope
|
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 .responses import json_error, not_found, parse_uuid
|
||||||
from .settings import get_setting
|
from .settings import get_setting
|
||||||
from .unfurl import UnfurlError, unfurl
|
from .unfurl import UnfurlError, unfurl
|
||||||
@@ -1254,26 +1255,10 @@ async def set_note_labels(note_id: str):
|
|||||||
note = await _get_owned(db, note_id)
|
note = await _get_owned(db, note_id)
|
||||||
if note is None:
|
if note is None:
|
||||||
return not_found()
|
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
|
# The picker manages MANUAL labels only; tag-sourced (via_tag=True) rows are
|
||||||
# governed by the body #tags and must survive a picker save.
|
# governed by the body #tags and survive a picker save (see labeling module).
|
||||||
chosen = {lid for lid in label_ids if lid in owned}
|
owned = await resolve_owned_label_ids(db, label_ids, g.user_id)
|
||||||
existing = (await db.scalars(select(NoteLabel).where(NoteLabel.note_id == note.id))).all()
|
await reconcile_manual_labels(db, note, owned)
|
||||||
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))
|
|
||||||
await db.commit()
|
await db.commit()
|
||||||
return jsonify(await _serialize_note(db, note))
|
return jsonify(await _serialize_note(db, note))
|
||||||
|
|
||||||
|
|||||||
@@ -3,7 +3,6 @@ in one click. Owner-scoped CRUD; `params` mirrors the GET /api/notes facet query
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import json
|
import json
|
||||||
import uuid
|
|
||||||
|
|
||||||
from quart import Blueprint, g, jsonify, request
|
from quart import Blueprint, g, jsonify, request
|
||||||
from sqlalchemy import func, select
|
from sqlalchemy import func, select
|
||||||
@@ -11,6 +10,7 @@ from sqlalchemy import func, select
|
|||||||
from .auth import login_required
|
from .auth import login_required
|
||||||
from .db import session_scope
|
from .db import session_scope
|
||||||
from .models.saved_filter import SavedFilter
|
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")
|
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 {}
|
data = await request.get_json(silent=True) or {}
|
||||||
name = (data.get("name") or "").strip()
|
name = (data.get("name") or "").strip()
|
||||||
if not name:
|
if not name:
|
||||||
return jsonify({"error": "name is required"}), 400
|
return json_error("name is required", 400)
|
||||||
params = clean_params(data.get("params"))
|
params = clean_params(data.get("params"))
|
||||||
async with session_scope() as db:
|
async with session_scope() as db:
|
||||||
max_pos = await db.scalar(
|
max_pos = await db.scalar(
|
||||||
@@ -82,21 +82,20 @@ async def create_saved():
|
|||||||
@bp.patch("/<filter_id>")
|
@bp.patch("/<filter_id>")
|
||||||
@login_required
|
@login_required
|
||||||
async def rename_saved(filter_id: str):
|
async def rename_saved(filter_id: str):
|
||||||
try:
|
fid = parse_uuid(filter_id)
|
||||||
fid = uuid.UUID(filter_id)
|
if fid is None:
|
||||||
except (ValueError, TypeError):
|
return not_found()
|
||||||
return jsonify({"error": "not found"}), 404
|
|
||||||
data = await request.get_json(silent=True) or {}
|
data = await request.get_json(silent=True) or {}
|
||||||
async with session_scope() as db:
|
async with session_scope() as db:
|
||||||
sf = await db.scalar(
|
sf = await db.scalar(
|
||||||
select(SavedFilter).where(SavedFilter.id == fid, SavedFilter.owner_id == g.user_id)
|
select(SavedFilter).where(SavedFilter.id == fid, SavedFilter.owner_id == g.user_id)
|
||||||
)
|
)
|
||||||
if sf is None:
|
if sf is None:
|
||||||
return jsonify({"error": "not found"}), 404
|
return not_found()
|
||||||
if "name" in data:
|
if "name" in data:
|
||||||
name = (data.get("name") or "").strip()
|
name = (data.get("name") or "").strip()
|
||||||
if not name:
|
if not name:
|
||||||
return jsonify({"error": "name is required"}), 400
|
return json_error("name is required", 400)
|
||||||
sf.name = name[:NAME_CAP]
|
sf.name = name[:NAME_CAP]
|
||||||
if "params" in data:
|
if "params" in data:
|
||||||
sf.params = json.dumps(clean_params(data.get("params")))
|
sf.params = json.dumps(clean_params(data.get("params")))
|
||||||
@@ -108,16 +107,15 @@ async def rename_saved(filter_id: str):
|
|||||||
@bp.delete("/<filter_id>")
|
@bp.delete("/<filter_id>")
|
||||||
@login_required
|
@login_required
|
||||||
async def delete_saved(filter_id: str):
|
async def delete_saved(filter_id: str):
|
||||||
try:
|
fid = parse_uuid(filter_id)
|
||||||
fid = uuid.UUID(filter_id)
|
if fid is None:
|
||||||
except (ValueError, TypeError):
|
return not_found()
|
||||||
return jsonify({"error": "not found"}), 404
|
|
||||||
async with session_scope() as db:
|
async with session_scope() as db:
|
||||||
sf = await db.scalar(
|
sf = await db.scalar(
|
||||||
select(SavedFilter).where(SavedFilter.id == fid, SavedFilter.owner_id == g.user_id)
|
select(SavedFilter).where(SavedFilter.id == fid, SavedFilter.owner_id == g.user_id)
|
||||||
)
|
)
|
||||||
if sf is None:
|
if sf is None:
|
||||||
return jsonify({"error": "not found"}), 404
|
return not_found()
|
||||||
await db.delete(sf)
|
await db.delete(sf)
|
||||||
await db.commit()
|
await db.commit()
|
||||||
return jsonify({"ok": True})
|
return jsonify({"ok": True})
|
||||||
|
|||||||
@@ -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}
|
||||||
+4
-14
@@ -21,6 +21,7 @@ from sqlalchemy import func, select
|
|||||||
from .auth import login_required
|
from .auth import login_required
|
||||||
from .config import Config
|
from .config import Config
|
||||||
from .db import session_scope
|
from .db import session_scope
|
||||||
|
from .labeling import reconcile_manual_labels, resolve_owned_label_ids
|
||||||
from .models.label import Label, NoteLabel
|
from .models.label import Label, NoteLabel
|
||||||
from .models.note import Note
|
from .models.note import Note
|
||||||
from .models.note_attachment import NoteAttachment
|
from .models.note_attachment import NoteAttachment
|
||||||
@@ -212,20 +213,9 @@ async def _apply_note_manual_labels(db, note: Note, ch: dict) -> None:
|
|||||||
try:
|
try:
|
||||||
wanted.add(uuid.UUID(str(r)))
|
wanted.add(uuid.UUID(str(r)))
|
||||||
except (ValueError, TypeError):
|
except (ValueError, TypeError):
|
||||||
continue
|
continue # sync is lenient: skip a malformed id rather than reject the push
|
||||||
owned: set = set()
|
owned = await resolve_owned_label_ids(db, wanted, g.user_id)
|
||||||
if wanted:
|
await reconcile_manual_labels(db, note, owned)
|
||||||
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))
|
|
||||||
|
|
||||||
|
|
||||||
async def _purge_note(db, note: Note, edited_at: datetime | None) -> None:
|
async def _purge_note(db, note: Note, edited_at: datetime | None) -> None:
|
||||||
|
|||||||
@@ -1,6 +1,10 @@
|
|||||||
|
import uuid
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from thoughtsync.app import create_app
|
from thoughtsync.app import create_app
|
||||||
|
from thoughtsync.models.label import Label
|
||||||
|
from thoughtsync.serialize import serialize_label
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
@@ -8,6 +12,16 @@ def app():
|
|||||||
return create_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):
|
async def test_labels_requires_auth(app):
|
||||||
client = app.test_client()
|
client = app.test_client()
|
||||||
resp = await client.get("/api/labels")
|
resp = await client.get("/api/labels")
|
||||||
|
|||||||
Reference in New Issue
Block a user