fix(core): drill-down lists descendants under untracked intermediates
listEntries({ parentId: X }) now applies the top-level rule relative to X: this user's
live entries on descendants of X (up to three levels) with no live-entry work strictly
between them and X, so it agrees with childSummary. Restoring an engagement's re-parenting
onto a soft-deleted entry throws ValidationError("entryId", "target entry is deleted").
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
00fc0180b0
commit
c6865b2522
+3
-1
@@ -34,7 +34,9 @@ Several behaviours are not obvious from the spec and would otherwise have to be
|
||||
- **Hierarchy visibility.** `childSummary` counts the user's live entries up to three levels below the
|
||||
entry's work (child, grandchild, great-grandchild). The default library view (`parentId: null`) lists
|
||||
entries with no ancestor, up to three levels, on which the user has a live entry. An episode whose show
|
||||
is untracked therefore stays visible, and is rolled up once the show gets an entry.
|
||||
is untracked therefore stays visible, and is rolled up once the show gets an entry. Drilling in
|
||||
(`parentId: X`) applies the same rule relative to X: descendants up to three levels with no live-entry
|
||||
work strictly between them and X. `childSummary` still counts every descendant entry.
|
||||
- **Concurrent creates.** `upsertEntry` inserts with `ON CONFLICT (user_id, work_id) DO NOTHING`, then
|
||||
patches through the update path, so a concurrent creator never aborts the caller's transaction.
|
||||
|
||||
|
||||
@@ -264,11 +264,12 @@ export async function restoreEngagementFromHistory(
|
||||
// Undo a merge's re-parenting; the old entry must be this user's.
|
||||
const owner = await k
|
||||
.selectFrom("entries")
|
||||
.select("id")
|
||||
.select(["id", "deleted_at"])
|
||||
.where("id", "=", before.entryId)
|
||||
.where("user_id", "=", userId)
|
||||
.executeTakeFirst();
|
||||
if (!owner) throw new NotFoundError("entry");
|
||||
if (owner.deleted_at !== null) throw new ValidationError("entryId", "target entry is deleted");
|
||||
extra.entry_id = before.entryId;
|
||||
}
|
||||
await write(k, userId, eg, mediaType, patch as Partial<EngagementPatch>, "user", extra);
|
||||
|
||||
@@ -730,6 +730,21 @@ const HAS_ENTRY_ANCESTOR = sql<boolean>`exists (select 1 from works a1
|
||||
and (pe.work_id = a1.id or pe.work_id = a2.id or pe.work_id = a3.id)
|
||||
where a1.id = w.parent_id)`;
|
||||
|
||||
/**
|
||||
* True when the entry's work is a descendant of `parentId` (up to three levels) with no work strictly
|
||||
* between them carrying a live entry of this user: the top-level rule, relative to `parentId`.
|
||||
*/
|
||||
function underParent(parentId: string) {
|
||||
const liveEntryOn = (ids: ReturnType<typeof sql>) =>
|
||||
sql`exists (select 1 from entries pe where pe.user_id = e.user_id and pe.deleted_at is null and pe.work_id in (${ids}))`;
|
||||
return sql<boolean>`(w.parent_id = ${parentId}
|
||||
or exists (select 1 from works p1 where p1.id = w.parent_id and p1.parent_id = ${parentId}
|
||||
and not ${liveEntryOn(sql`p1.id`)})
|
||||
or exists (select 1 from works p1 inner join works p2 on p2.id = p1.parent_id
|
||||
where p1.id = w.parent_id and p2.parent_id = ${parentId}
|
||||
and not ${liveEntryOn(sql`p1.id, p2.id`)}))`;
|
||||
}
|
||||
|
||||
function viewQuery(k: Executor, userId: string) {
|
||||
return k
|
||||
.selectFrom("entries as e")
|
||||
@@ -801,7 +816,8 @@ export interface EntryFilter {
|
||||
q?: string;
|
||||
/**
|
||||
* Default `null`: the top of the user's library, i.e. entries with no ancestor work (up to three
|
||||
* levels) carrying a live entry of this user. A work id lists that work's direct children.
|
||||
* levels) carrying a live entry of this user. A work id applies the same rule relative to that work:
|
||||
* its descendants (up to three levels) with no live-entry work strictly between them and it.
|
||||
*/
|
||||
parentId?: string | null;
|
||||
sort?: "updated" | "title" | "rating";
|
||||
@@ -829,7 +845,7 @@ export async function listEntries(
|
||||
|
||||
let q = viewQuery(db.k, userId);
|
||||
const parentId = filter.parentId === undefined ? null : filter.parentId;
|
||||
q = parentId === null ? q.where(sql<boolean>`not ${HAS_ENTRY_ANCESTOR}`) : q.where("w.parent_id", "=", parentId);
|
||||
q = q.where(parentId === null ? sql<boolean>`not ${HAS_ENTRY_ANCESTOR}` : underParent(parentId));
|
||||
if (filter.mediaType) q = q.where(sql<string>`coalesce(e.media_type_override, w.media_type)`, "=", filter.mediaType);
|
||||
if (filter.status) q = q.where("e.status", "=", filter.status);
|
||||
if (filter.tag) {
|
||||
|
||||
@@ -7,7 +7,7 @@ import {
|
||||
softDeleteEngagement,
|
||||
updateEngagement,
|
||||
} from "../src/engagements.ts";
|
||||
import { softDeleteEntry, upsertEntry } from "../src/entries.ts";
|
||||
import { moveEntryToWork, softDeleteEntry, upsertEntry } from "../src/entries.ts";
|
||||
import { NotFoundError, ValidationError } from "../src/errors.ts";
|
||||
import { getHistory, restoreVersion } from "../src/history.ts";
|
||||
import { makeUser, makeWork } from "./user-layer-fixtures.ts";
|
||||
@@ -122,6 +122,19 @@ forEachDialect("engagements", (getDb) => {
|
||||
await expect(softDeleteEngagement(db, userId, eg.id, "user")).rejects.toThrow(NotFoundError);
|
||||
});
|
||||
|
||||
test("restoring a re-parenting onto a soft-deleted entry is rejected", async () => {
|
||||
const { db, userId, entry } = await setup();
|
||||
const other = await upsertEntry(db, userId, await makeWork(db, { title: "Other" }), {}, "user");
|
||||
const eg = await addEngagement(db, userId, entry.id, {}, "user");
|
||||
await moveEntryToWork(db, userId, entry.id, other.workId, "user");
|
||||
const [h] = await getHistory(db, userId, "engagement", eg.id);
|
||||
expect(h!.before).toEqual({ entryId: entry.id });
|
||||
// the merged-away entry is soft-deleted, so the engagement cannot go back to it
|
||||
const err = await restoreVersion(db, userId, h!.id).catch((e: unknown) => e);
|
||||
expect(err).toBeInstanceOf(ValidationError);
|
||||
expect(err).toMatchObject({ field: "entryId", reason: "target entry is deleted" });
|
||||
});
|
||||
|
||||
test("another user's engagements are not found", async () => {
|
||||
const { db, userId, entry } = await setup();
|
||||
const bob = await makeUser(db, "bob");
|
||||
|
||||
@@ -140,6 +140,18 @@ forEachDialect("entries", (getDb) => {
|
||||
const top2 = await listEntries(db, userId, {});
|
||||
expect(top2.items.map((i) => i.id).sort()).toEqual([showEntry.id, lone.id, show2Entry.id].sort());
|
||||
expect(top2.items.find((i) => i.id === show2Entry.id)!.childSummary).toEqual({ total: 2, completed: 1 });
|
||||
|
||||
// drilling into the show lists the episodes, since the season between them has no entry
|
||||
const drill = await listEntries(db, userId, { parentId: show2 });
|
||||
expect(drill.items.map((i) => i.work.mediaType)).toEqual(["episode", "episode"]);
|
||||
expect(await listEntries(db, userId, { parentId: show2, q: "nothing matches" })).toEqual({ items: [], nextCursor: null });
|
||||
|
||||
// once the season has an entry, the show lists the season and the season lists the episodes
|
||||
const seasonEntry = await upsertEntry(db, userId, season, { status: "in_progress" }, "user");
|
||||
expect((await listEntries(db, userId, { parentId: show2 })).items.map((i) => i.id)).toEqual([seasonEntry.id]);
|
||||
const seasonDrill = await listEntries(db, userId, { parentId: season });
|
||||
expect(seasonDrill.items.map((i) => i.work.mediaType)).toEqual(["episode", "episode"]);
|
||||
expect(seasonDrill.items.find((i) => i.id === seasonEntry.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
test("orphaned child entries are top level until an ancestor has an entry", async () => {
|
||||
|
||||
Reference in New Issue
Block a user