[lists] fix a few lists

packages/domain/src/index.ts

@@ -8,6 +8,7 @@ export * from './room-instance-db'
 export * from './presence-db'
 export * from './gifts-db'
 export * from './inventory-invention-db'
+export * from './lists-db'
 export * from './outfits-db'
 export * from './progression-db'
 export * from './relationships-db'

packages/domain/src/lists-db.ts

@@ -0,0 +1,220 @@
+/**
+ * Player-owned curated lists on the shared `recflare` D1 database — the playlists a player
+ * builds themselves, of which "Saved for Later" is the one the client creates on its own.
+ *
+ * Columns rather than a JSON blob (the `club_announcement` pattern rather than the
+ * `room`/`club` one): nothing here is a client-shaped document that has to survive
+ * round-tripping, it is five scalars and a set of ids, and the ids need their own table to
+ * be queried and de-duplicated at all.
+ *
+ * A list is GENERIC. `list_type` says what the `item_id`s ARE — the `ListEntityType` the
+ * algorithmic lists echo, so `1` is Rooms and an `item_id` is a room id — and nothing here
+ * interprets them: the client resolves each id against the service that type names. That is
+ * also why `item_id` is TEXT rather than an integer. A list of rooms carries room ids, but a
+ * list of discovery sections carries section KEYS, and one column has to hold both.
+ *
+ * The `lists` worker owns this schema/migration (`apps/lists/migrations/0001_curated_list.sql`,
+ * applied under its own `migrations_table` so it doesn't clash with the other workers'
+ * migrations that share the database). `CURATED_LIST_SCHEMA_DDL` mirrors that migration so
+ * tests can build the tables directly.
+ */
+
+/** Schema DDL (mirror of apps/lists/migrations/0001_curated_list.sql). */
+export const CURATED_LIST_SCHEMA_DDL: string[] = [
+	// `list_id` is an ordinary autoincrement integer. The reference's own ids run to 18
+	// digits (`624765592684307326`) and the static captures still carry theirs verbatim, but
+	// nothing requires a list this server MINTS to look like that — and a small id stays well
+	// inside what a JS number holds exactly, so it can't be rounded on its way through D1 or
+	// JSON. AUTOINCREMENT rather than a bare rowid alias: a list id is handed to the client,
+	// so a deleted list's id must not be handed out again to a different list.
+	`CREATE TABLE IF NOT EXISTS list (
+		list_id INTEGER PRIMARY KEY AUTOINCREMENT,
+		creator_account_id INTEGER NOT NULL,
+		list_type INTEGER NOT NULL,
+		list_name TEXT NOT NULL,
+		list_name_lower TEXT GENERATED ALWAYS AS (lower(list_name)) VIRTUAL,
+		list_description TEXT,
+		image_name TEXT NOT NULL DEFAULT '',
+		accessibility INTEGER NOT NULL DEFAULT 1,
+		created_at TEXT NOT NULL
+	)`,
+	// The lookup the client actually makes: `?creatorAccountId=&type=&name=`, all three at
+	// once. UNIQUE because that triple is a list's identity — the client asks for
+	// `__SavedForLater_Rooms` by name expecting the one it has been appending to, so a
+	// player must never end up with two. Folded, since the casing that reaches us is the
+	// client's.
+	`CREATE UNIQUE INDEX IF NOT EXISTS idx_list_owner_type_name
+		ON list (creator_account_id, list_type, list_name_lower)`,
+	`CREATE INDEX IF NOT EXISTS idx_list_creator ON list (creator_account_id)`,
+	// A list's contents — one row per item, insertion order preserved by the surrogate key,
+	// which is the order the `ItemIds` array is served in.
+	//
+	// UNIQUE on the pair: saving the same room twice is a no-op, not a carousel showing it
+	// twice. The section's own `supportsDedupe` is about dedupe ACROSS rows and doesn't help
+	// here.
+	`CREATE TABLE IF NOT EXISTS list_item (
+		list_item_id INTEGER PRIMARY KEY AUTOINCREMENT,
+		list_id INTEGER NOT NULL,
+		item_id TEXT NOT NULL
+	)`,
+	`CREATE UNIQUE INDEX IF NOT EXISTS idx_list_item_pair ON list_item (list_id, item_id)`,
+	`CREATE INDEX IF NOT EXISTS idx_list_item_list ON list_item (list_id)`,
+]
+
+/**
+ * One curated list as the client parses it, whether it came out of D1 or out of a static
+ * capture. `Description` may be null but `ImageName` must be a STRING — the client reads it
+ * straight into a string field — and `ItemIds` are strings even where they stand for
+ * numeric ids, which is what the working captures carry.
+ *
+ * `ListId` is a string HERE ONLY and never reaches the client as one: the `lists` worker's
+ * `serializeCuratedList` puts the digits back on the wire unquoted, because the client's
+ * field is a number and a quoted id fails its parser. It stays a string even though a
+ * STORED id is a small integer, because a CAPTURED one is 18 digits — parsing that would
+ * round it (…307326 → …307300) — and both kinds flow through this one shape.
+ */
+export interface CuratedList {
+	ListId: string
+	CreatorAccountId: number
+	Name: string
+	Description: string | null
+	ImageName: string
+	Type: number
+	ItemIds: string[]
+	Accessibility?: number
+	CreatedAt: string
+}
+
+interface ListRow {
+	list_id: number
+	creator_account_id: number
+	list_type: number
+	list_name: string
+	list_description: string | null
+	image_name: string
+	accessibility: number
+	created_at: string
+}
+
+/**
+ * The image a list carries when nothing set one. Every captured list uses it, and the field
+ * cannot be empty or null without the client rendering a blank tile for the row.
+ */
+export const DEFAULT_LIST_IMAGE = 'DefaultRoomImage.jpg'
+
+/** A stored row plus its items, as the client-facing list. */
+function toCuratedList(row: ListRow, itemIds: string[]): CuratedList {
+	return {
+		ListId: String(row.list_id),
+		CreatorAccountId: row.creator_account_id,
+		Name: row.list_name,
+		Description: row.list_description,
+		ImageName: row.image_name,
+		Type: row.list_type,
+		ItemIds: itemIds,
+		Accessibility: row.accessibility,
+		CreatedAt: row.created_at,
+	}
+}
+
+/** A list's item ids, in the order they were added — the order the row displays them. */
+async function getListItems(db: D1Database, listId: number): Promise<string[]> {
+	const { results } = await db
+		.prepare('SELECT item_id FROM list_item WHERE list_id = ?1 ORDER BY list_item_id')
+		.bind(listId)
+		.all<{ item_id: string }>()
+	return results.map((r) => r.item_id)
+}
+
+/** What identifies a player's list, and what a missing one is created with. */
+export interface PlayerListKey {
+	creatorAccountId: number
+	/** The `ListEntityType` — what the item ids ARE. 1 (Rooms) is what the client sends. */
+	type: number
+	name: string
+	/** Applied only when the list is CREATED; see {@link addPlayerListItem}. */
+	accessibility: number
+}
+
+/**
+ * Add an item to a player's list, creating the list if they don't have one yet — the
+ * `…/items/:itemId/createlistifneeded` call the client makes when someone saves a room for
+ * later. Answers the list as it now stands, which is what the caller re-renders the row from.
+ *
+ * Idempotent in both halves. The list insert is `OR IGNORE` against the
+ * (creator, type, name) unique index and the id is re-read rather than assumed, so two adds
+ * racing to create the same list end up with ONE list and the loser adopts the winner's id
+ * instead of silently writing its item into a list nobody will look up. The item insert is
+ * `OR IGNORE` against (list_id, item_id), so saving the same room twice leaves the row
+ * showing it once, in the position it was first saved into.
+ *
+ * `accessibility` is honoured only on creation. The client sends it on every add, but this
+ * call is "add an item", not "change who can see the list" — applying it each time would let
+ * one stray add flip a list the player had deliberately made public, or the reverse.
+ */
+export async function addPlayerListItem(
+	db: D1Database,
+	key: PlayerListKey,
+	itemId: string
+): Promise<CuratedList> {
+	await db
+		.prepare(
+			`INSERT OR IGNORE INTO list (creator_account_id, list_type, list_name,
+			                             list_description, image_name, accessibility, created_at)
+			 VALUES (?1, ?2, ?3, NULL, ?4, ?5, ?6)`
+		)
+		.bind(
+			key.creatorAccountId,
+			key.type,
+			key.name,
+			DEFAULT_LIST_IMAGE,
+			key.accessibility,
+			new Date().toISOString()
+		)
+		.run()
+
+	// Read the id back rather than taking the insert's: `OR IGNORE` assigns nothing when the
+	// player already had the list, and says nothing about whether the row is ours.
+	const row = await db
+		.prepare(
+			`SELECT list_id FROM list
+			 WHERE creator_account_id = ?1 AND list_type = ?2 AND list_name_lower = lower(?3)`
+		)
+		.bind(key.creatorAccountId, key.type, key.name)
+		.first<{ list_id: number }>()
+	const listId = row!.list_id
+
+	await db
+		.prepare('INSERT OR IGNORE INTO list_item (list_id, item_id) VALUES (?1, ?2)')
+		.bind(listId, itemId)
+		.run()
+
+	return (await getPlayerList(db, key.creatorAccountId, key.type, key.name))!
+}
+
+/**
+ * A player's own list, looked up the way the client asks for one:
+ * `?creatorAccountId=&type=&name=`. Undefined when that player has no such list — the
+ * caller decides what an absent list answers, since a static capture may cover the name.
+ *
+ * The name is matched case-insensitively; a list with no items is still a list and comes
+ * back with an empty `ItemIds`, which is NOT the same answer as undefined.
+ */
+export async function getPlayerList(
+	db: D1Database,
+	creatorAccountId: number,
+	type: number,
+	name: string
+): Promise<CuratedList | undefined> {
+	const row = await db
+		.prepare(
+			`SELECT list_id, creator_account_id, list_type, list_name, list_description,
+			        image_name, accessibility, created_at
+			 FROM list
+			 WHERE creator_account_id = ?1 AND list_type = ?2 AND list_name_lower = lower(?3)`
+		)
+		.bind(creatorAccountId, type, name)
+		.first<ListRow>()
+	if (row === null) return undefined
+	return toCuratedList(row, await getListItems(db, row.list_id))
+}

packages/domain/src/rooms-db.ts

@@ -891,21 +891,30 @@ interface RoomRow {
 const ROOM_COLUMNS = 'data, visits'
 
 /**
- * Two keys on the client's room DTO that nothing here stores, defaulted on every read so
- * the key is PRESENT rather than absent — the seed blobs and every room written since
- * predate them, so they can't come from the data:
+ * Keys on the client's room DTO that nothing here stores, defaulted on every read so the
+ * key is PRESENT rather than absent — the seed blobs and every room written since predate
+ * them, so they can't come from the data:
  *
  * - `BoostCount` — how many boosts the room is carrying. No boost feature exists here, so
  *   it is 0 for every room.
  * - `CurrentSnapshotId` — the room's published snapshot. Nothing takes snapshots, so it is
  *   null, which is also what the reference serves for a room that has none.
+ * - `FriendlyName` — the display name, which the reference lets a creator set apart from
+ *   the unique `Name`. Nothing sets one here, so it falls back to `Name`; it must never be
+ *   null, because the client labels a room from it and renders nothing for a room without
+ *   one.
+ * - `CCU` — concurrent users. No live-population counter exists here, so it is null, which
+ *   is what the reference serves when it has no number rather than 0 (a 0 reads as "nobody
+ *   is in here" in the browse feeds).
  *
- * Defaulted rather than assigned, so a stored value wins if either is ever really written
+ * Defaulted rather than assigned, so a stored value wins if any is ever really written
  * (a blob keeps whatever `serializeRoom` last put in it).
  */
 function attachRoomDtoDefaults(room: Room): void {
 	room.BoostCount ??= 0
 	room.CurrentSnapshotId ??= null
+	room.FriendlyName ??= room.Name
+	room.CCU ??= null
 }
 
 /**
@@ -1782,33 +1791,39 @@ export async function countRoomsByCreator(db: D1Database, accountId: number): Pr
 }
 
 /**
- * Rooms an account CONTRIBUTES to: the ones whose `Roles` name them (Host, Moderator or
- * CoOwner), minus the ones they created themselves.
+ * Every room an account works on: the ones it CREATED plus the ones whose `Roles` name it
+ * (Host, Moderator or CoOwner). Every role tier counts, unlike {@link canManageRoom}'s
+ * owner-or-co-owner gate: this is "you have a job in this room", not "you may administer
+ * it".
  *
- * The creator is excluded deliberately. A room's `Roles` carries its creator too, so
- * without that filter this list would repeat everything `getRoomsByCreator` already
- * serves — and the client shows "rooms you own" and "rooms you contribute to" as two
- * separate lists. Every role tier counts here, unlike {@link canManageRoom}'s
- * owner-or-co-owner gate: this is "somebody gave you a job in their room", not "you may
- * administer it".
+ * The creator half used to be excluded — a room's `Roles` carries its creator too, and the
+ * client shows "rooms you own" and "rooms you contribute to" as separate lists, so the
+ * exclusion kept this from repeating `createdby/me`. It also made the list EMPTY for every
+ * account that had only ever built its own rooms, which is most of them, so the screen
+ * behind it showed nothing at all. Repeating `createdby/me` is the better failure, and
+ * overlap is what a client that renders one list wants anyway.
+ *
+ * The dorm stays out, on `ownedby/me`'s reasoning: it is auto-provisioned rather than a
+ * room the player made. A room matching BOTH halves appears once — the roles half is an
+ * EXISTS, not a join.
  *
  * Roles live inside the room blob rather than in a table of their own, so the match is a
  * `json_each` over `$.Roles`. A room with no `Roles` key (or a null one) simply yields no
- * rows there rather than erroring, so it drops out of the list.
+ * rows there rather than erroring, so it only reaches the list if the account created it.
  */
 export async function getContributedRooms(db: D1Database, accountId: number): Promise<Room[]> {
 	const { results } = await db
 		.prepare(
 			`SELECT ${ROOM_COLUMNS} FROM room
-			 WHERE creator_account_id IS NOT ?1
-			   AND EXISTS (
+			 WHERE creator_account_id = ?1
+			    OR EXISTS (
 			     SELECT 1 FROM json_each(room.data, '$.Roles') AS role
 			     WHERE json_extract(role.value, '$.AccountId') = ?1
 			   )`
 		)
 		.bind(accountId)
 		.all<RoomRow>()
-	return hydrateRooms(db, parseAll(results))
+	return (await hydrateRooms(db, parseAll(results))).filter((r) => r.IsDorm !== true)
 }
 
 /**
@@ -2006,6 +2021,8 @@ function roomHasAnyTag(room: Room, tags: Set<string>): boolean {
  * `#tag` terms match the room's Tags; plain terms match the room name
  * (substring). All terms must match. Returns a paginated `{ Results, TotalResults }`.
  * The dataset is small, so this filters in memory rather than in SQL.
+ *
+ * `#community` is the one tag term that isn't a tag lookup — see {@link COMMUNITY_TAG}.
  */
 export async function searchRooms(
 	db: D1Database,
@@ -2021,9 +2038,15 @@ export async function searchRooms(
 	// EVERY tag asked for, and each term expands to its aliases (`#recroomoriginal` accepts
 	// `rro`). Only the rooms that survive have their blobs read, which is what `room_tag` is
 	// for: a tag search no longer parses every room in the database to ask.
-	const tagSets = terms
-		.filter((t) => t.startsWith('#'))
-		.map((t) => t.slice(1))
+	//
+	// `#community` is held out of that query: no room CARRIES the tag (the browse chip posts
+	// it to the hot feed as a pseudo-tag, and the search box sends the same term), so asking
+	// `room_tag` for it matches nothing and the whole search comes back empty. It filters on
+	// who MADE the room instead, below.
+	const tagTerms = terms.filter((t) => t.startsWith('#')).map((t) => t.slice(1))
+	const communityOnly = tagTerms.includes(COMMUNITY_TAG)
+	const tagSets = tagTerms
+		.filter((tag) => tag !== COMMUNITY_TAG)
 		.map((tag) => [tag, ...(TAG_ALIASES[tag] ?? [])])
 	const { sql, binds } = roomsByTagsQuery(tagSets)
 	const { results } = await db
@@ -2032,6 +2055,11 @@ export async function searchRooms(
 		.all<RoomRow>()
 	let rooms = parseAll(results).filter((r) => r.IsDorm !== true && r.Accessibility === 1)
 
+	// The same test the hot feed's `community` chip applies: every room a player made, which
+	// is every room the Coach account doesn't own. It narrows the other terms rather than
+	// replacing them, so `#community horror` is still a name search within player-made rooms.
+	if (communityOnly) rooms = rooms.filter(isPlayerMade)
+
 	// The plain terms still match in memory: they are substring matches on the name, which
 	// no index helps with.
 	for (const term of terms) {
@@ -2135,6 +2163,9 @@ const NEW_TAG = 'new'
  * isn't the Coach account — the system account that owns the seeded Rec Room rooms.
  * Unlike {@link NEW_TAG} it only filters: the page keeps the feed's normal
  * live-population ordering.
+ *
+ * The chip reaches {@link searchRooms} too, as the tag term `#community` — the search box
+ * carries the same word — so both feeds have to know it names no tag.
  */
 const COMMUNITY_TAG = 'community'