diff --git a/apps/ios/components/ItemsScreen.tsx b/apps/ios/components/ItemsScreen.tsx index 1004184..4b25c37 100644 --- a/apps/ios/components/ItemsScreen.tsx +++ b/apps/ios/components/ItemsScreen.tsx @@ -1,4 +1,4 @@ -import type { Item, Status } from "@pluck/shared-schemas"; +import { isTask, type Item, type Status } from "@pluck/shared-schemas"; import { useQueryClient } from "@tanstack/react-query"; import * as Haptics from "expo-haptics"; import { router, useFocusEffect, usePathname } from "expo-router"; @@ -93,7 +93,7 @@ export function ItemsScreen({ status }: { status: "inbox" | "later" }) { const more = () => { const options = [ ...(one && one.url ? ["note" as const] : []), - ...(chosen.every((i) => i.kind === "todo" || i.kind === "thing") ? ["done" as const] : []), + ...(chosen.every((i) => isTask(i.kind)) ? ["done" as const] : []), "all" as const, ]; const names = { note: tr("items.addNote"), done: tr("common.done"), all: tr("items.selectAll") }; diff --git a/apps/web/components/ItemBar.tsx b/apps/web/components/ItemBar.tsx index 66da9b1..8b3258f 100644 --- a/apps/web/components/ItemBar.tsx +++ b/apps/web/components/ItemBar.tsx @@ -1,6 +1,6 @@ "use client"; -import type { Item, Status } from "@pluck/shared-schemas"; +import { isTask, type Item, type Status } from "@pluck/shared-schemas"; import { useTranslations } from "next-intl"; import Link from "next/link"; import { useRouter } from "next/navigation"; @@ -43,7 +43,7 @@ export function ItemBar({ item, top, state, before }: { item: Item; top?: ReactN return void (before ?? (async () => {}))().then(() => router.push(backHref(origin))); } if (isTyping(e.target) || window.getSelection()?.toString()) return; - const run = { l: () => move("later"), e: () => move("archived"), d: () => (item.kind === "todo" || item.kind === "thing") && move("done") }[e.key]; + const run = { l: () => move("later"), e: () => move("archived"), d: () => isTask(item.kind) && move("done") }[e.key]; if (run) { e.preventDefault(); void run(); @@ -71,7 +71,7 @@ export function ItemBar({ item, top, state, before }: { item: Item; top?: ReactN - {(item.kind === "todo" || item.kind === "thing") && ( + {isTask(item.kind) && ( diff --git a/apps/web/components/ItemList.tsx b/apps/web/components/ItemList.tsx index f6ea75e..5810222 100644 --- a/apps/web/components/ItemList.tsx +++ b/apps/web/components/ItemList.tsx @@ -1,6 +1,6 @@ "use client"; -import type { Item, ItemPage, List, PluckResponse, Status, Tag } from "@pluck/shared-schemas"; +import { isTask, type Item, type ItemPage, type List, type PluckResponse, type Status, type Tag } from "@pluck/shared-schemas"; import { useTranslations } from "next-intl"; import { useRouter } from "next/navigation"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; @@ -137,7 +137,7 @@ export function ItemList({ view, initial, total: initialTotal, tags: initialTags // Moves slide the rows out, then call the API; a failure puts them back. const move = useCallback( async (ids: string[], status: Status) => { - const moved = real.filter((r) => ids.includes(r.id) && (status !== "done" || r.kind === "todo" || r.kind === "thing")); + const moved = real.filter((r) => ids.includes(r.id) && (status !== "done" || isTask(r.kind))); if (!moved.length) return; const movedIds = moved.map((r) => r.id); setLeaving(new Set(movedIds)); @@ -301,7 +301,7 @@ export function ItemList({ view, initial, total: initialTotal, tags: initialTags const emptyNode = !everPlucked && rows.length === 0 ? empty() : null; const selecting = selected.size > 0; const selectedRows = real.filter((r) => selected.has(r.id)); - const allTodo = selectedRows.length > 0 && selectedRows.every((r) => r.kind === "todo" || r.kind === "thing"); + const allTodo = selectedRows.length > 0 && selectedRows.every((r) => isTask(r.kind)); return ( <> @@ -381,7 +381,7 @@ export function ItemList({ view, initial, total: initialTotal, tags: initialTags move([r.id], "archived")} /> setPicking([r.id])} /> openLists([r.id])} /> - {(r.kind === "todo" || r.kind === "thing") && move([r.id], "done")} />} + {isTask(r.kind) && move([r.id], "done")} />} ) diff --git a/apps/web/lib/library/items.test.ts b/apps/web/lib/library/items.test.ts index 405f701..5e40e1a 100644 --- a/apps/web/lib/library/items.test.ts +++ b/apps/web/lib/library/items.test.ts @@ -109,12 +109,17 @@ test("edits follow the rules for links and notes", async () => { expect((await updateItem(a, link.id, { kind: "video" })).kind).toBe("video"); }); -test("Done is only for todos and things; batch skips what does not fit", async () => { - const { updateItem: update, batch } = await import("./index"); +test("Done is only for todos and things; batch refuses it as an edit does, before any tag moves", async () => { + const { updateItem: update, batch, addTag } = await import("./index"); const { a } = await twoUsers(); const link = (await pluckAny(a, { url: `${FIX}/blog-4.html` })).results[0]!.item; const todo = (await pluckAny(a, { text: "Book the room", kind: "todo" })).results[0]!.item; + const tag = (await addTag(a, "errands")).tag; await expect(update(a, link.id, { status: "done" })).rejects.toMatchObject({ status: 400, message: "Done is for todos and things." }); + await expect(batch(a, { itemIds: [link.id, todo.id], patch: { status: "done", addTagIds: [tag.id] } })).rejects.toMatchObject({ + status: 400, message: "Done is for todos and things.", details: { itemIds: [link.id] }, + }); + expect(await readItem(a, todo.id)).toMatchObject({ status: "inbox", tagIds: [] }); expect((await update(a, todo.id, { status: "done" })).status).toBe("done"); expect(await batch(a, { itemIds: [link.id, todo.id], patch: { status: "archived" } })).toEqual({ matched: 2, updated: 2 }); }); diff --git a/apps/web/lib/library/items.ts b/apps/web/lib/library/items.ts index 35dd9ca..ea215c0 100644 --- a/apps/web/lib/library/items.ts +++ b/apps/web/lib/library/items.ts @@ -1,11 +1,11 @@ import { deletePrefix, keys as blobKeys, putObject, signedUrl } from "@pluck/blob"; import { batchTags, batchUpdate, countItems, createHighlight, createTextItem, deleteHighlight, editItem, findByUrl, getContent, getItem, getItemBlobKeys, - hitRateLimit, listHighlights, listItems, listsForItem, markOpened, moveToBin, pluckLink, purgeBin, purgeFromBin, resetExtraction, restoreFromBin, setBlobKeys, setItemTags, + hitRateLimit, listHighlights, listItems, listsForItem, markOpened, misfit, moveToBin, pluckLink, purgeBin, purgeFromBin, resetExtraction, restoreFromBin, setBlobKeys, setItemTags, updateHighlight, } from "@pluck/db"; import { - extractUrls, isTextKind, LINK_KINDS, normalizeUrl, type BatchBody, type CreateHighlightBody, type CreateItemBody, type CreateItemResult, + extractUrls, isTextKind, normalizeUrl, type BatchBody, type CreateHighlightBody, type CreateItemBody, type CreateItemResult, type Highlight, type Item, type ItemPage, type ListItemsQuery, type ListMembership, type PluckResponse, type Status, type UpdateItemBody, } from "@pluck/shared-schemas"; import { ApiError } from "../errors"; @@ -106,13 +106,9 @@ import { tagIdsFor, type Actor } from "./tags"; // Edits from the person. Text and the text kinds belong to items with no URL; link kinds to items with one. export async function updateItem(userId: string, itemId: string, patch: UpdateItemBody, { by = "user" }: { by?: Actor } = {}): Promise { const current = await readItem(userId, itemId); - const isLink = !isTextKind(current.kind); - if (patch.text !== undefined && isLink) throw new ApiError("bad_request", "Only notes, todos and things have text to edit.", 400); - if (patch.kind && isLink !== (LINK_KINDS as readonly string[]).includes(patch.kind)) - throw new ApiError("bad_request", isLink ? "A link can be Read, Watch, PDF or Image." : "This can be a Note, Todo or Thing.", 400); + const refused = misfit(current.kind, patch); + if (refused) throw new ApiError("bad_request", refused, 400); if (patch.preview !== undefined && current.preview === null) throw new ApiError("bad_request", "Only links open on a Preview.", 400); - if (patch.status === "done" && !["todo", "thing"].includes(patch.kind ?? current.kind)) - throw new ApiError("bad_request", "Done is for todos and things.", 400); const { tagIds, ...rest } = patch; if (tagIds) await setItemTags(userId, itemId, await tagIdsFor(userId, tagIds), by); if (!Object.keys(rest).length) return readItem(userId, itemId); @@ -131,10 +127,12 @@ export async function batch(userId: string, body: BatchBody, { by = "user" }: { const { addTagIds, removeTagIds, ...p } = body.patch; const add = addTagIds ? await tagIdsFor(userId, addTagIds) : []; const remove = removeTagIds ? await tagIdsFor(userId, removeTagIds) : []; - let tagged = 0; - if (add.length || remove.length) tagged = await batchTags(userId, body.itemIds, add, remove, by); - if (!Object.keys(p).length) return { matched: body.itemIds.length, updated: tagged }; - const res = await batchUpdate(userId, body.itemIds, { ...p, dueAt: p.dueAt === undefined ? undefined : p.dueAt === null ? null : new Date(p.dueAt) }); + // The change goes first: one that does not fit every Item is refused, like an edit of one, before any tag moves. + const res = Object.keys(p).length + ? await batchUpdate(userId, body.itemIds, { ...p, dueAt: p.dueAt === undefined ? undefined : p.dueAt === null ? null : new Date(p.dueAt) }) + : { matched: body.itemIds.length, updated: 0, refused: undefined }; + if (res.refused) throw new ApiError("bad_request", res.refused.reason, 400, { itemIds: res.refused.itemIds }); + const tagged = add.length || remove.length ? await batchTags(userId, body.itemIds, add, remove, by) : 0; return { matched: res.matched, updated: Math.max(res.updated, tagged) }; } diff --git a/docs/architecture.md b/docs/architecture.md index 7e50531..cb0f01e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -86,6 +86,7 @@ These are the places where the code is shallower than it needs to be: a rule wri - **Files:** `packages/db/src/items.ts` (pluckLink, createFileItem, createTextItem, the bin, extraction updates), `email-in.ts` (createEmailItem), `account.ts` (importLink), `lists.ts` (around `doneFollowsMark`), `expiry.ts` (`archive`). - **Problem:** a whole new Item document (about 30 fields) is spelled out by hand in five places, and the Re-pluck update is copied between `items.ts` and `email-in.ts`. Status is written in about six places: `statusFields()` handles the archived reason, but ticking a Todo in a List and expiry write it inline. The Touch rule (`lastTouchedAt`) is applied in about twelve places, with an exception for reading progress. A new field or Status rule means editing five or six files. `items.ts` (453 lines) changes for four reasons: writes, extraction, the bin and listing. - **Shape:** one small module (`item-doc.ts`) with `newItem(userId, fields, now)`, `replucked(now)`, `toStatus(status, reason, now)` and `touched(now)`, used by every writer. Then split `items.ts` along its four reasons. Tests cross the module's interface: one table test per Status transition (the changed time, the archived reason, the List tick), and Re-pluck for both a URL and an email. +- **Done so far:** `item-doc.ts` has `newItem`, `replucked`, `statusFields` and `misfit` (which changes fit an Item: text, Kind, Done); every writer above but expiry uses them, and an edit and a batch refuse on the same `misfit`. The UIs ask `isTask` in `shared-schemas`. Left: `touched`, expiry's Status write, and the split of `items.ts`. ### 2. Derived fields have no single owner (M) diff --git a/docs/mcp.md b/docs/mcp.md index 0036930..49cfa08 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -44,7 +44,7 @@ A tool is only listed when the token carries its scope. | `list_items` | Browse by status; `status, kinds[], sort (newest, untouched), limit (max 25), cursor` | | `get_item` | One item with its facts, its kept text (paged at about 20k tokens, follow `textCursor`), note, highlights and lists | | `save_item` | Pluck a URL, every URL in a block of text, or a note, todo or thing; `url or text, kind, title, note, tagIds[], listId, dueAt`. Saving a link twice brings it back to the Inbox. `tagIds` take existing tag ids or names; unknown tags (refused with the nearest existing ones) or an unknown `listId` refuse the call before anything is saved | -| `batch_update` | One change over up to 100 items: status, kind, due date, note, add or remove tags. Unknown tags are refused with the nearest existing ones; tags are never created here | +| `batch_update` | One change over up to 100 items: status, kind, due date, note, add or remove tags. Unknown tags are refused with the nearest existing ones; tags are never created here. A change that does not fit every item (Done is only for Todos and Things; a link Kind only for links) refuses the call, naming those items in `details.itemIds`, and nothing changes | | `list_tags` | Tags with counts, and whether an AI made them; paged with `cursor` | | `create_tag` | A new tag, only after the person agreed. Marked as made by AI | | `list_lists` | Lists with purpose, counts and visibility | diff --git a/packages/db/src/account.ts b/packages/db/src/account.ts index 769c21b..938b290 100644 --- a/packages/db/src/account.ts +++ b/packages/db/src/account.ts @@ -3,6 +3,7 @@ import type { ImportEntry } from "@pluck/extract"; import type { ImportStatus, Item } from "@pluck/shared-schemas"; import { ObjectId } from "mongodb"; import { apiKeys, contents, db, emailAliases, exportsCol, highlights, imports, items, listRows, lists, listShares, oid, reports, tags, users, type ImportDoc } from "./collections.ts"; +import { newItem } from "./item-doc.ts"; import { toItem } from "./items.ts"; import { textForSearch } from "./search.ts"; import { createTag } from "./tags.ts"; @@ -43,13 +44,10 @@ export async function importLink(userId: string, url: string, e: Pick { diff --git a/packages/db/src/index.ts b/packages/db/src/index.ts index f1f5836..2d701b0 100644 --- a/packages/db/src/index.ts +++ b/packages/db/src/index.ts @@ -7,6 +7,7 @@ export { countItems, BIN_DAYS, type PluckLinkInput, type TextItemInput, type ItemEdit, type BatchEdit, type ExtractionResult, } from "./items.ts"; +export { misfit } from "./item-doc.ts"; export { createHighlight, listHighlights, updateHighlight, deleteHighlight, type HighlightInput } from "./highlights.ts"; export { enqueue, claimJob, completeJob, retryJob, failJob, pendingJobCount, type Job } from "./jobs.ts"; export { hitRateLimit } from "./rate-limit.ts"; diff --git a/packages/db/src/item-doc.test.ts b/packages/db/src/item-doc.test.ts new file mode 100644 index 0000000..7da4d6c --- /dev/null +++ b/packages/db/src/item-doc.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, test } from "vitest"; +import { misfit, statusFields } from "./item-doc.ts"; + +const TEXT = "Only notes, todos and things have text to edit."; +const LINK = "A link can be Read, Watch, PDF or Image."; +const NOTE = "This can be a Note, Todo or Thing."; +const DONE = "Done is for todos and things."; + +describe("which changes fit an Item", () => { + test.each([ + ["note", { text: "x" }, null], + ["link", { text: "x" }, TEXT], + ["link", { kind: "video" }, null], + ["pdf", { kind: "note" }, LINK], + ["note", { kind: "todo" }, null], + ["thing", { kind: "image" }, NOTE], + ["todo", { status: "done" }, null], + ["thing", { status: "done" }, null], + ["note", { status: "done" }, DONE], + ["link", { status: "done" }, DONE], + ["note", { kind: "todo", status: "done" }, null], // Done goes by the Kind the Item ends with + ["todo", { kind: "note", status: "done" }, DONE], + ["video", { kind: "todo", status: "done" }, LINK], + ["link", { status: "archived" }, null], + ["note", { status: "later" }, null], + ] as const)("%s with %o", (kind, change, why) => { + expect(misfit(kind, change)).toBe(why); + }); + + test("a status change stamps the time, and only the person's archive has the reason user", () => { + const now = new Date(); + expect(statusFields("archived", now)).toEqual({ status: "archived", statusChangedAt: now, archivedReason: "user" }); + expect(statusFields("done", now)).toEqual({ status: "done", statusChangedAt: now, archivedReason: null }); + }); +}); diff --git a/packages/db/src/item-doc.ts b/packages/db/src/item-doc.ts new file mode 100644 index 0000000..eccb4a2 --- /dev/null +++ b/packages/db/src/item-doc.ts @@ -0,0 +1,37 @@ +import { isTask, isTextKind, type Kind, type Status } from "@pluck/shared-schemas"; +import { ObjectId } from "mongodb"; +import { oid, type ItemDoc } from "./collections.ts"; + +// What an Item is and how it may change. Every writer makes its new Item here and every edit asks here whether it +// fits, so a new field or rule is one change. + +// A whole new Item: in the Inbox, touched now, nothing read yet. `fields` sets what this writer knows. +export function newItem(userId: string, fields: Pick & Partial, now: Date): ItemDoc { + return { + _id: new ObjectId(), userId: oid(userId), url: null, canonicalUrl: null, titleIsDerived: true, siteName: null, author: null, excerpt: null, + coverKey: null, htmlKey: null, status: "inbox", archivedReason: null, dueAt: null, tagIds: [], note: null, readingProgress: 0, + wordCount: null, lang: null, extraction: { state: "pending", attempts: 0, lastError: null }, createdAt: now, updatedAt: now, + lastOpenedAt: null, lastTouchedAt: now, statusChangedAt: now, deletedAt: null, ...fields, + }; +} + +// The fields a status change sets: archived by the person is "user"; leaving the archive clears the reason. +export function statusFields(status: Status, now: Date): Pick { + return { status, statusChangedAt: now, archivedReason: status === "archived" ? "user" : null }; +} + +// Re-pluck: the Item already there goes back to the Inbox, out of the Bin, and is touched. Never a second Item. +export const replucked = (now: Date) => ({ ...statusFields("inbox", now), deletedAt: null, lastTouchedAt: now, updatedAt: now }); + +// What an upsert writes only when it makes the Item: Mongo refuses a field in both $set and $setOnInsert. +export const onInsert = (doc: ItemDoc, set: object): Partial => Object.fromEntries(Object.entries(doc).filter(([k]) => !(k in set))); + +// Why a change does not fit an Item of this Kind, or null when it does; an edit and a batch refuse on the same rule. +// Text and the text Kinds belong to Notes, Todos and Things, the link Kinds to the rest, and Done to the Kind it ends with. +export function misfit(kind: Kind, change: { kind?: Kind; status?: Status; text?: string }): string | null { + const text = isTextKind(kind); + if (change.text !== undefined && !text) return "Only notes, todos and things have text to edit."; + if (change.kind && text !== isTextKind(change.kind)) return text ? "This can be a Note, Todo or Thing." : "A link can be Read, Watch, PDF or Image."; + if (change.status === "done" && !isTask(change.kind ?? kind)) return "Done is for todos and things."; + return null; +} diff --git a/packages/db/src/items.ts b/packages/db/src/items.ts index e322a9b..f6d7dce 100644 --- a/packages/db/src/items.ts +++ b/packages/db/src/items.ts @@ -1,7 +1,8 @@ import { createHash } from "node:crypto"; -import { BIN_DAYS, defaultPreview, deriveExcerpt, deriveTitle, isTextKind, LINK_KINDS, TEXT_KINDS, type Item, type ItemPage, type Facts, type Kind, type ListItemsQuery, type Preview, type Source, type Status, type TextKind } from "@pluck/shared-schemas"; +import { BIN_DAYS, defaultPreview, deriveExcerpt, deriveTitle, isTextKind, Kind, TASK_KINDS, type Item, type ItemPage, type Facts, type ListItemsQuery, type Preview, type Source, type Status, type TextKind } from "@pluck/shared-schemas"; import { ObjectId, type Filter } from "mongodb"; import { contents, highlights, items, LEAN, listRows, oid, type ItemDoc } from "./collections.ts"; +import { misfit, newItem, onInsert, replucked, statusFields } from "./item-doc.ts"; import { markFollowsDone, recountList, recountListsOf } from "./lists.ts"; import { recountTags } from "./tags.ts"; import { bodyForSearch, textForSearch } from "./search.ts"; @@ -57,22 +58,13 @@ export async function pluckLink( const now = new Date(); const uid = oid(userId); const hash = urlHash(input.url); + const set = replucked(now); + const fresh = newItem(userId, { + kind: "link", url: input.url, urlHash: hash, title: input.title?.trim() || input.url, siteName: new URL(input.url).hostname.replace(/^www\./, ""), + note: input.note?.trim() || null, source: input.source, searchText: textForSearch({ url: input.url }), + }, now); const run = () => - col.findOneAndUpdate( - { userId: uid, urlHash: hash }, - { - $set: { status: "inbox", archivedReason: null, deletedAt: null, lastTouchedAt: now, updatedAt: now, statusChangedAt: now }, - $setOnInsert: { - userId: uid, kind: "link", url: input.url, urlHash: hash, canonicalUrl: null, - title: input.title?.trim() || input.url, titleIsDerived: true, siteName: new URL(input.url).hostname.replace(/^www\./, ""), - author: null, excerpt: null, coverKey: null, htmlKey: null, dueAt: null, tagIds: [], note: input.note?.trim() || null, - source: input.source, readingProgress: 0, wordCount: null, lang: null, - extraction: { state: "pending", attempts: 0, lastError: null }, createdAt: now, lastOpenedAt: null, - searchText: textForSearch({ url: input.url }), - }, - }, - { upsert: true, returnDocument: "after", includeResultMetadata: true }, - ); + col.findOneAndUpdate({ userId: uid, urlHash: hash }, { $set: set, $setOnInsert: onInsert(fresh, set) }, { upsert: true, returnDocument: "after", includeResultMetadata: true }); let res; try { res = await run(); @@ -111,17 +103,10 @@ export const recountAround = (userId: string, doc: Pick { - const now = new Date(); - const _id = new ObjectId(); + const doc = newItem(userId, { kind: input.kind, title: input.title, source: input.source, searchText: textForSearch({}) }, new Date()); const col = await items(); - await col.insertOne({ - _id, userId: oid(userId), kind: input.kind, url: null, canonicalUrl: null, title: input.title, titleIsDerived: true, siteName: null, - author: null, excerpt: null, coverKey: null, htmlKey: null, status: "inbox", archivedReason: null, dueAt: null, tagIds: [], note: null, - source: input.source, readingProgress: 0, wordCount: null, lang: null, extraction: { state: "pending", attempts: 0, lastError: null }, - createdAt: now, updatedAt: now, lastOpenedAt: null, lastTouchedAt: now, statusChangedAt: now, deletedAt: null, - searchText: textForSearch({}), - }); - return toItem((await col.findOne({ _id }))!); + await col.insertOne(doc); + return toItem((await col.findOne({ _id: doc._id }))!); } export type TextItemInput = { kind: TextKind; text: string; title?: string | null; dueAt?: Date | null; source: Source; clientKey?: string }; @@ -132,15 +117,11 @@ export async function createTextItem(userId: string, input: TextItemInput): Prom const now = new Date(); const uid = oid(userId); const typed = input.title?.trim(); - const doc: ItemDoc = { - _id: new ObjectId(), userId: uid, kind: input.kind, url: null, canonicalUrl: null, - title: typed || deriveTitle(input.text), titleIsDerived: !typed, siteName: null, author: null, - excerpt: deriveExcerpt(input.text, !typed), coverKey: null, htmlKey: null, status: "inbox", archivedReason: null, - dueAt: input.dueAt ?? null, tagIds: [], note: null, source: input.source, readingProgress: 0, wordCount: null, lang: null, - extraction: { state: "none", attempts: 0, lastError: null }, createdAt: now, updatedAt: now, lastOpenedAt: null, - lastTouchedAt: now, statusChangedAt: now, deletedAt: null, searchBody: bodyForSearch(input.text), + const doc = newItem(userId, { + kind: input.kind, title: typed || deriveTitle(input.text), titleIsDerived: !typed, excerpt: deriveExcerpt(input.text, !typed), + dueAt: input.dueAt ?? null, source: input.source, extraction: { state: "none", attempts: 0, lastError: null }, searchBody: bodyForSearch(input.text), ...(input.clientKey ? { clientKey: input.clientKey } : {}), - }; + }, now); const col = await items(); const sent = async () => (input.clientKey ? await col.findOne({ userId: uid, clientKey: input.clientKey }) : null); const first = await sent(); @@ -163,13 +144,8 @@ export type ItemEdit = { preview?: Preview | null; // the person's choice of screen; null goes back to the guess }; -// The fields a status change sets: archived by the person is "user"; leaving the archive clears the reason. -function statusFields(status: Status, now: Date): Partial { - return { status, statusChangedAt: now, archivedReason: status === "archived" ? "user" : null }; -} - // Edits by the person. Title rules: a typed title sticks; an empty one goes back to the first line. -// The caller has already checked the edit fits the item (text and text kinds only on items with no URL). +// The caller has already checked the edit fits the item (`misfit` in item-doc.ts). export async function editItem(userId: string, itemId: string, edit: ItemEdit): Promise { const uid = oid(userId); const _id = oid(itemId); @@ -221,34 +197,31 @@ export async function editItem(userId: string, itemId: string, edit: ItemEdit): // A Todo or Thing's mark in its Lists follows its Done (see markFollowsDone). async function marksFollow(uid: ObjectId, ids: ObjectId[], status: Status): Promise { if (status === "archived") return; - const tasks = await (await items()).find({ _id: { $in: ids }, userId: uid, kind: { $in: ["todo", "thing"] } }, { projection: { _id: 1 } }).toArray(); + const tasks = await (await items()).find({ _id: { $in: ids }, userId: uid, kind: { $in: [...TASK_KINDS] } }, { projection: { _id: 1 } }).toArray(); await markFollowsDone(uid, tasks.map((t) => t._id), status === "done"); } export type BatchEdit = { status?: Status; kind?: Kind; dueAt?: Date | null; note?: string | null }; -// One change over many of the person's items. Changes that do not fit an item are skipped for that item: -// Done only for todos and things, link kinds only for links, text kinds only for items with no URL. -export async function batchUpdate(userId: string, itemIds: string[], patch: BatchEdit): Promise<{ matched: number; updated: number }> { +export type BatchResult = { matched: number; updated: number; refused?: { reason: string; itemIds: string[] } }; + +// One change over many of the person's items, all or none: if it does not fit one of them (`misfit`), nothing +// changes and `refused` names those items, as an edit of one would be refused. +export async function batchUpdate(userId: string, itemIds: string[], patch: BatchEdit): Promise { const col = await items(); const now = new Date(); const base = { userId: oid(userId), _id: { $in: itemIds.map(oid) }, deletedAt: null }; + const fits = Kind.options.filter((k) => !misfit(k, patch)); + const odd = await col.find({ ...base, kind: { $nin: fits } }, { projection: { _id: 1, kind: 1 } }).toArray(); + const matched = await col.countDocuments(base); + if (odd.length) return { matched, updated: 0, refused: { reason: misfit(odd[0]!.kind, patch)!, itemIds: odd.map((d) => d._id.toHexString()) } }; const set: Partial = { updatedAt: now, lastTouchedAt: now }; if (patch.dueAt !== undefined) set.dueAt = patch.dueAt; if (patch.note !== undefined) set.note = patch.note?.trim() || null; - const filter: Record = { ...base }; - if (patch.status) { - Object.assign(set, statusFields(patch.status, now)); - if (patch.status === "done") filter.kind = { $in: ["todo", "thing"] }; - } - if (patch.kind) { - set.kind = patch.kind; - // A link kind fits links (with or without a URL, like email-in); a text kind fits notes, todos and things. - const group: readonly string[] = (LINK_KINDS as readonly string[]).includes(patch.kind) ? LINK_KINDS : TEXT_KINDS; - filter.kind = { $in: (patch.status === "done" ? group.filter((k) => k === "todo" || k === "thing") : group) as Kind[] }; - } - const matched = await col.countDocuments(base); - const res = await col.updateMany(filter, { $set: set }); + if (patch.status) Object.assign(set, statusFields(patch.status, now)); + if (patch.kind) set.kind = patch.kind; + // The Kind is checked again as it is written, so one changed since the read above is left alone. + const res = await col.updateMany({ ...base, kind: { $in: fits } }, { $set: set }); if (patch.status) { const moved = await col.find({ ...base, status: patch.status }, { projection: { _id: 1 } }).toArray(); await marksFollow(base.userId, moved.map((d) => d._id), patch.status); diff --git a/packages/db/src/lists.ts b/packages/db/src/lists.ts index 2a5f3b8..e90db72 100644 --- a/packages/db/src/lists.ts +++ b/packages/db/src/lists.ts @@ -1,6 +1,7 @@ -import type { Item, List, ListEntry, ListMembership } from "@pluck/shared-schemas"; +import { isTask, TASK_KINDS, type Item, type List, type ListEntry, type ListMembership } from "@pluck/shared-schemas"; import { ObjectId } from "mongodb"; import { items, LEAN, listRows, lists, listShares, oid, users, type ListDoc, type ListItemDoc } from "./collections.ts"; +import { statusFields } from "./item-doc.ts"; import { toItem } from "./items.ts"; import { getMe } from "./users.ts"; import { slugify } from "./tags.ts"; @@ -234,20 +235,18 @@ export async function markEntries(userId: string, listId: string, itemIds: strin return res.matchedCount; } -const isTask = (kind: string) => kind === "todo" || kind === "thing"; - // A Todo or Thing marked in a List is Done; unmarked, a Done one goes back to the Inbox. Its other Lists follow. async function doneFollowsMark(uid: ObjectId, itemIds: ObjectId[], ticked: boolean): Promise { const col = await items(); const tasks = await col - .find({ _id: { $in: itemIds }, userId: uid, kind: { $in: ["todo", "thing"] }, status: ticked ? { $ne: "done" } : "done" }, { projection: { _id: 1 } }) + .find({ _id: { $in: itemIds }, userId: uid, kind: { $in: [...TASK_KINDS] }, status: ticked ? { $ne: "done" } : "done" }, { projection: { _id: 1 } }) .toArray(); if (!tasks.length) return; const ids = tasks.map((t) => t._id); const now = new Date(); await col.updateMany( { _id: { $in: ids }, userId: uid }, - { $set: { status: ticked ? "done" : "inbox", statusChangedAt: now, archivedReason: null, updatedAt: now, lastTouchedAt: now } }, + { $set: { ...statusFields(ticked ? "done" : "inbox", now), updatedAt: now, lastTouchedAt: now } }, ); await markFollowsDone(uid, ids, ticked); } diff --git a/packages/db/src/triage.test.ts b/packages/db/src/triage.test.ts index 93b5fee..45ac6e9 100644 --- a/packages/db/src/triage.test.ts +++ b/packages/db/src/triage.test.ts @@ -44,24 +44,29 @@ describe("status changes", () => { }); describe("batch", () => { - test("applies one change to many, skipping what does not fit, never touching B's items", async () => { + test("applies one change to many, all or none, never touching B's items", async () => { const { a, b } = await twoUsers(); const l1 = (await pluckLink(a, { url: "https://example.com/b1", source: "web" })).item; const l2 = (await pluckLink(a, { url: "https://example.com/b2", source: "web" })).item; const { item: todo } = await createTextItem(a, { kind: "todo", text: "t", source: "web" }); + const { item: note } = await createTextItem(b, { kind: "note", text: "n", source: "web" }); const theirs = (await pluckLink(b, { url: "https://example.com/b3", source: "web" })).item; expect(await batchUpdate(a, [l1.id, l2.id, theirs.id], { status: "later" })).toEqual({ matched: 2, updated: 2 }); expect((await getItem(b, theirs.id))?.status).toBe("inbox"); - // Done only fits the todo. - expect((await batchUpdate(a, [l1.id, todo.id], { status: "done" })).updated).toBe(1); - expect((await getItem(a, l1.id))?.status).toBe("later"); - // A link kind only fits links; a text kind only fits items with no URL. - expect((await batchUpdate(a, [l1.id, todo.id], { kind: "video" })).updated).toBe(1); - expect((await getItem(a, todo.id))?.kind).toBe("todo"); - expect((await batchUpdate(a, [l1.id, todo.id], { kind: "thing" })).updated).toBe(1); - expect((await getItem(a, l1.id))?.kind).toBe("video"); + // Done only fits the todo, so nothing moves and the link is named, as an edit of it alone would be refused. + expect(await batchUpdate(a, [l1.id, todo.id], { status: "done" })).toEqual({ + matched: 2, updated: 0, refused: { reason: "Done is for todos and things.", itemIds: [l1.id] }, + }); + expect((await getItem(a, todo.id))?.status).toBe("inbox"); + // A link kind only fits links; a text kind only fits notes, todos and things. + expect((await batchUpdate(a, [l1.id, todo.id], { kind: "video" })).refused?.itemIds).toEqual([todo.id]); + expect((await batchUpdate(a, [l1.id, l2.id], { kind: "video" })).updated).toBe(2); + expect((await batchUpdate(a, [todo.id], { kind: "note", status: "done" })).refused?.reason).toBe("Done is for todos and things."); + // B's items are neither changed nor named, even ones that would not fit. + expect(await batchUpdate(a, [todo.id, note.id, theirs.id], { status: "done" })).toEqual({ matched: 1, updated: 1 }); + expect((await getItem(b, theirs.id))?.status).toBe("inbox"); }); }); diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index 573d3d3..b605002 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -202,7 +202,7 @@ export function createPluckServer(lib: PluckLibrary, { scopes, version = "1.0.0" tool( "batch_update", { - description: "Apply one change to up to 100 items: status, kind, due date, note, or adding and removing existing tags.", + description: "Apply one change to up to 100 items (status, kind, due date, note, or adding and removing existing tags), all or none: if it does not fit some items, such as Done on a note, nothing changes and the error names them.", inputSchema: z.object({ itemIds: z.array(ObjectIdString).min(1).max(100), patch: z diff --git a/packages/shared-schemas/src/notes.ts b/packages/shared-schemas/src/notes.ts index 68ea48a..79b83bf 100644 --- a/packages/shared-schemas/src/notes.ts +++ b/packages/shared-schemas/src/notes.ts @@ -3,6 +3,9 @@ export const TEXT_KINDS = ["note", "todo", "thing"] as const; export type TextKind = (typeof TEXT_KINDS)[number]; export const isTextKind = (k: string | undefined): k is TextKind => (TEXT_KINDS as readonly string[]).includes(k ?? ""); +// The Kinds that can be Done: a Todo or Thing gets finished, a Note does not. +export const TASK_KINDS = ["todo", "thing"] as const; +export const isTask = (k: string): boolean => (TASK_KINDS as readonly string[]).includes(k); export function deriveTitle(text: string): string { const line = text.split("\n").map((l) => l.trim()).find(Boolean);