diff --git a/api/lib/playlist-members.ts b/api/lib/playlist-members.ts index 929c0c9d..595d4c25 100644 --- a/api/lib/playlist-members.ts +++ b/api/lib/playlist-members.ts @@ -1,7 +1,8 @@ /** * Decides which of a playlist's dumps a viewer may see. The playlist service - * filters its listings with it, and the WebSocket service filters each - * client's `playlist_dumps_updated` message the same way. + * filters its listings with it, the WebSocket service filters each client's + * `playlist_dumps_updated` message the same way, and the queries that list + * playlists count their dumps by the same rule. * * @module */ @@ -34,3 +35,28 @@ export function isMemberVisibleTo( return !member.isPrivate || (member.userId === viewerId && viewerId === playlist.userId); } + +/** + * The `dump_count` column of a query on `playlists p`: how many of the + * playlist's dumps the viewer may see, by {@link isMemberVisibleTo}'s rule. + * Its `?` takes the viewer's id, or `""` for a signed-out one, and comes + * before the query's other parameters, the column being in its select list. + */ +export const VISIBLE_DUMP_COUNT = `(SELECT COUNT(*) FROM playlist_dumps pd + INNER JOIN dumps d ON d.id = pd.dump_id + WHERE pd.playlist_id = p.id + AND (d.is_private = 0 OR (d.user_id = p.user_id AND p.user_id = ?)) + ) as dump_count`; + +/** + * Returns how many of `members` the viewer may see in `playlist`, the + * `dumpCount` the playlist has for them. + */ +export function countVisibleTo( + members: PlaylistMember[], + playlist: Pick, + viewerId: string | null | undefined, +): number { + return members.filter((m) => isMemberVisibleTo(m, playlist, viewerId)) + .length; +} diff --git a/api/routes/playlists.ts b/api/routes/playlists.ts index 5de684e8..e120e15b 100644 --- a/api/routes/playlists.ts +++ b/api/routes/playlists.ts @@ -77,12 +77,18 @@ const router: Router = new Router({ prefix: "/api/playlists", }); -router.get("/", (ctx) => { +router.get("/", async (ctx) => { + const requestingUserId = await parseOptionalAuth(ctx); const params = ctx.request.url.searchParams; const { page, limit } = parsePagination(params); const slug = params.get("category"); const categoryId = slug ? getCategoryBySlug("playlist", slug).id : undefined; - const { items, hasMore } = listPublicPlaylists(page, limit, categoryId); + const { items, hasMore } = listPublicPlaylists( + requestingUserId, + page, + limit, + categoryId, + ); const responseBody: APIResponse> = { success: true, data: { items, hasMore }, diff --git a/api/routes/users.ts b/api/routes/users.ts index eee49c0f..c66b2695 100644 --- a/api/routes/users.ts +++ b/api/routes/users.ts @@ -245,10 +245,16 @@ router.get("/:key/follow/users", (ctx) => { ctx.response.body = responseBody; }); -router.get("/:key/follow/playlists", (ctx) => { +router.get("/:key/follow/playlists", async (ctx) => { const user = resolveUserKey(ctx.params.key); + const requestingUserId = await parseOptionalAuth(ctx); const { page, limit } = parsePagination(ctx.request.url.searchParams); - const { items, hasMore } = getFollowedPlaylistsByUser(user.id, page, limit); + const { items, hasMore } = getFollowedPlaylistsByUser( + user.id, + requestingUserId, + page, + limit, + ); const responseBody: APIResponse> = { success: true, data: { items, hasMore }, diff --git a/api/services/follow-service.ts b/api/services/follow-service.ts index 0bed65e1..6c418682 100644 --- a/api/services/follow-service.ts +++ b/api/services/follow-service.ts @@ -23,6 +23,7 @@ import { } from "../model/dump.ts"; import { isFollowRow } from "../model/follow.ts"; import { isPlaylistRow, playlistRowToApi } from "../model/playlist.ts"; +import { VISIBLE_DUMP_COUNT } from "../lib/playlist-members.ts"; import { type Page, pageOf } from "../lib/pagination.ts"; import { attachCategoryIds, categoryJoin } from "./category-service.ts"; import { sendToUser } from "./ws-service.ts"; @@ -232,12 +233,14 @@ export function getFollowedPlaylistsDumpFeed( /** * Returns a page of the public playlists `userId` follows, most recently - * followed first, for their profile and playlists pages. + * followed first, for their profile and playlists pages. Their dumps are + * counted for `requestingUserId`, null being a signed-out viewer. * * @throws {APIException} 500 when a row doesn't parse as a playlist. */ export function getFollowedPlaylistsByUser( userId: string, + requestingUserId: string | null, page: number, limit: number, ): Page { @@ -245,7 +248,7 @@ export function getFollowedPlaylistsByUser( const rawRows = db.prepare( `SELECT p.*, u.username as owner_username, - (SELECT COUNT(*) FROM playlist_dumps pd WHERE pd.playlist_id = p.id) as dump_count + ${VISIBLE_DUMP_COUNT} FROM playlists p LEFT JOIN users u ON u.id = p.user_id INNER JOIN follows f ON f.followed_playlist_id = p.id @@ -253,7 +256,7 @@ export function getFollowedPlaylistsByUser( AND p.is_public = 1 ORDER BY f.created_at DESC LIMIT ? OFFSET ?;`, - ).all(userId, limit + 1, offset); + ).all(requestingUserId ?? "", userId, limit + 1, offset); if (!rawRows.every(isPlaylistRow)) { throw new APIException(500, "Malformed playlist data"); diff --git a/api/services/playlist-service.ts b/api/services/playlist-service.ts index 8f66e73d..b352887b 100644 --- a/api/services/playlist-service.ts +++ b/api/services/playlist-service.ts @@ -44,19 +44,28 @@ import { import { isMemberVisibleTo, type PlaylistMember, + VISIBLE_DUMP_COUNT, } from "../lib/playlist-members.ts"; import { LIKE, likePattern } from "../lib/like.ts"; import { parseRows } from "../model/rows.ts"; /** * The select list and `FROM` clause of a playlist query, adding the owner's - * name and the dump count. Callers append the `WHERE`. The dump count includes - * dumps the viewer can't see. + * name and the count of the dumps the viewer may see. Callers append the + * `WHERE`, and pass the viewer's id first, as {@link viewerParam} gives it. */ const PLAYLIST_SELECT = `p.*, u.username as owner_username, - (SELECT COUNT(*) FROM playlist_dumps pd WHERE pd.playlist_id = p.id) as dump_count + ${VISIBLE_DUMP_COUNT} FROM playlists p JOIN users u ON u.id = p.user_id`; +/** + * Returns what {@link VISIBLE_DUMP_COUNT}'s parameter takes for `viewerId`: a + * signed-out viewer matches no user. + */ +function viewerParam(viewerId: string | null | undefined): string { + return viewerId ?? ""; +} + /** * Converts the rows of a {@link PLAYLIST_SELECT} to playlists, their * categories filled in. @@ -69,13 +78,17 @@ function playlistsOf(rows: unknown[]): Playlist[] { /** * Returns playlist `playlistId`, private or not. A caller that shows it to - * someone must check `isPublic` first, as the OG middleware does. + * someone must check `isPublic` first, as the OG middleware does. Its + * `dumpCount` is `viewerId`'s, or a signed-out viewer's without one. * * @throws {APIException} 404 when there is no such playlist. */ -export function getPlaylistById(playlistId: string): Playlist { +export function getPlaylistById( + playlistId: string, + viewerId: string | null = null, +): Playlist { const row = db.prepare(`SELECT ${PLAYLIST_SELECT} WHERE p.id = ?;`) - .get(playlistId); + .get(viewerParam(viewerId), playlistId); if (!row || !isPlaylistRow(row)) { throw new APIException(404, "Playlist not found"); } @@ -136,7 +149,7 @@ export function createPlaylist( } linkAttachments(req.description, "playlist", id); } - broadcastPlaylistChange(null, playlist); + broadcastPlaylistChange(null, playlist, []); return playlist; } @@ -144,7 +157,8 @@ export function createPlaylist( * Returns a playlist with the dumps `requestingUserId` may see in it, in * order, for `GET /api/playlists/:playlistId`, and for * `GET /api/users/:key/playlists/:slug` once it has found the playlist by its - * path. A null `requestingUserId` is a signed-out viewer. + * path. A null `requestingUserId` is a signed-out viewer. Its `dumpCount` is + * the number of those dumps. * * @throws {APIException} 404 when there is no such playlist, or it is private * and not theirs. @@ -175,7 +189,7 @@ export function getPlaylist( isMemberVisibleTo(d, playlist, requestingUserId) ); - return { ...playlist, dumps: visibleDumps }; + return { ...playlist, dumpCount: visibleDumps.length, dumps: visibleDumps }; } /** @@ -198,10 +212,17 @@ export function searchPlaylists( const rows = requestingUserId ? db.prepare( `SELECT ${PLAYLIST_SELECT} ${catJoin} WHERE (p.is_public = 1 OR p.user_id = ?) AND ${searchClause} ORDER BY p.created_at DESC LIMIT ?;`, - ).all(...catParams, requestingUserId, pattern, pattern, limit) + ).all( + requestingUserId, + ...catParams, + requestingUserId, + pattern, + pattern, + limit, + ) : db.prepare( `SELECT ${PLAYLIST_SELECT} ${catJoin} WHERE p.is_public = 1 AND ${searchClause} ORDER BY p.created_at DESC LIMIT ?;`, - ).all(...catParams, pattern, pattern, limit); + ).all(viewerParam(null), ...catParams, pattern, pattern, limit); return playlistsOf(rows); } @@ -209,9 +230,11 @@ export function searchPlaylists( /** * Returns a page of every public playlist, newest first, for * `GET /api/playlists`, optionally only those in the playlist category - * `categoryId`. The client's `/playlists` page lists them. + * `categoryId`. The client's `/playlists` page lists them. Their dumps are + * counted for `requestingUserId`, null being a signed-out viewer. */ export function listPublicPlaylists( + requestingUserId: string | null, page: number, limit: number, categoryId?: string, @@ -222,21 +245,22 @@ export function listPublicPlaylists( `SELECT ${PLAYLIST_SELECT} ${catJoin} WHERE p.is_public = 1 ORDER BY p.created_at DESC LIMIT ? OFFSET ?;`, - ).all(...catParams, limit + 1, offset); + ).all(viewerParam(requestingUserId), ...catParams, limit + 1, offset); return pageOf(playlistsOf(rows), limit); } /** * Returns the public playlists created from `from` to `to`, the fullest first * and the newer on a tie. The community service gathers them for the - * `/community` page and the digests. + * `/community` page and the digests, which show every reader the same + * figures: only their public dumps are counted. */ export function listNewPublicPlaylists(from: Date, to: Date): Playlist[] { const rows = db.prepare( `SELECT ${PLAYLIST_SELECT} WHERE p.is_public = 1 AND p.created_at >= ? AND p.created_at < ? ORDER BY dump_count DESC, p.created_at DESC;`, - ).all(from.toISOString(), to.toISOString()); + ).all(viewerParam(null), from.toISOString(), to.toISOString()); return playlistsOf(rows); } @@ -262,7 +286,13 @@ export function listPlaylistsByUser( `SELECT ${PLAYLIST_SELECT} ${catJoin} WHERE p.user_id = ?${privacyFilter} ORDER BY p.created_at DESC LIMIT ? OFFSET ?;`, - ).all(...catParams, userId, limit + 1, offset); + ).all( + viewerParam(requestingUserId), + ...catParams, + userId, + limit + 1, + offset, + ); return pageOf(playlistsOf(rows), limit); } @@ -287,7 +317,7 @@ export function updatePlaylist( requestingUserId: string, canModerate: boolean, ): Playlist { - const playlist = getPlaylistById(playlistId); + const playlist = getPlaylistById(playlistId, requestingUserId); if (playlist.userId !== requestingUserId && !canModerate) { throw new APIException(403, "Only its owner or a moderator may do that"); @@ -347,7 +377,7 @@ export function updatePlaylist( } linkAttachments(newDescription, "playlist", playlist.id); } - broadcastPlaylistChange(playlist, updated); + broadcastPlaylistChange(playlist, updated, getCurrentMembers(playlist.id)); // Its followers weren't told of the dumps added while it was private. if (!playlist.isPublic && newIsPublic) { notifyPlaylistFollowersNowPublic({ playlistId: playlist.id }); @@ -374,7 +404,7 @@ export function deletePlaylist( } db.prepare(`DELETE FROM playlists WHERE id = ?;`).run(playlist.id); - broadcastPlaylistChange(playlist, null); + broadcastPlaylistChange(playlist, null, []); } /** @@ -390,7 +420,7 @@ export function getEditablePlaylist( requestingUserId: string, canModerate: boolean, ): Playlist { - const playlist = getPlaylistById(playlistId); + const playlist = getPlaylistById(playlistId, requestingUserId); if (playlist.userId !== requestingUserId && !canModerate) { throw new APIException(403, "Only its owner or a moderator may do that"); } @@ -421,8 +451,8 @@ export function setPlaylistImage( db.prepare( `UPDATE playlists SET image_mime = ?, updated_at = ? WHERE id = ?;`, ).run(imageMime, new Date().toISOString(), playlist.id); - const updated = getPlaylistById(playlist.id); - broadcastPlaylistChange(playlist, updated); + const updated = getPlaylistById(playlist.id, requestingUserId); + broadcastPlaylistChange(playlist, updated, getCurrentMembers(playlist.id)); return updated; } @@ -590,13 +620,13 @@ export function getPlaylistMembershipsForDump( ): PlaylistMembership[] { const rows = db.prepare( `SELECT p.*, u.username as owner_username, - (SELECT COUNT(*) FROM playlist_dumps x WHERE x.playlist_id = p.id) as dump_count, + ${VISIBLE_DUMP_COUNT}, EXISTS (SELECT 1 FROM playlist_dumps pd WHERE pd.playlist_id = p.id AND pd.dump_id = ?) as has_dump FROM playlists p JOIN users u ON u.id = p.user_id WHERE p.user_id = ? ORDER BY p.created_at DESC;`, - ).all(dumpId, userId); + ).all(userId, dumpId, userId); const memberships = rows.map((row) => { if (!isPlaylistRow(row)) { @@ -626,7 +656,7 @@ export function getPlaylistsForDump( INNER JOIN playlist_dumps pd ON pd.playlist_id = p.id WHERE pd.dump_id = ? AND (p.is_public = 1 OR p.user_id = ?) ORDER BY p.created_at DESC LIMIT 20;`, - ).all(dumpId, requestingUserId ?? ""); + ).all(viewerParam(requestingUserId), dumpId, requestingUserId ?? ""); return playlistsOf(rows); } @@ -701,7 +731,7 @@ export function playlistsHolding(dumpId: string): PlaylistHolder[] { /** * Sends each of `holders` its dumps again, after one of them was deleted or - * changed visibility: the count moved, or what each viewer may see of them. + * changed visibility, which changes what each viewer may see of them. */ export function broadcastHoldersDumps(holders: PlaylistHolder[]): void { for (const holder of holders) { diff --git a/api/services/ws-service.ts b/api/services/ws-service.ts index c7bf680b..9b990aee 100644 --- a/api/services/ws-service.ts +++ b/api/services/ws-service.ts @@ -18,6 +18,7 @@ import type { OnlineUser, ServerToClientMessage } from "../../shared/ws.ts"; import type { Playlist } from "../../shared/playlist.ts"; import type { PublicUser, Role } from "../../shared/user.ts"; import { + countVisibleTo, isMemberVisibleTo, type PlaylistMember, } from "../lib/playlist-members.ts"; @@ -179,35 +180,43 @@ export function broadcastPresence(): void { * `before` is null for a new item, `after` for a deleted one. A private item's * changes reach its owner's sockets alone. When it goes private, everyone else * is told it was deleted, and gets nothing of the change; when it goes public, - * they are told it was created, since to them it is new. + * they are told it was created, since to them it is new. The owner's sockets + * and the others' each get a message of their own, built with `toOwner` set + * for the former, for what the two don't see alike. */ function broadcastChange( before: T | null, after: T | null, isPrivate: (item: T) => boolean, messages: { - created: (item: T) => ServerToClientMessage; - updated: (item: T) => ServerToClientMessage; + created: (item: T, toOwner: boolean) => ServerToClientMessage; + updated: (item: T, toOwner: boolean) => ServerToClientMessage; deleted: (item: T) => ServerToClientMessage; }, ): void { const item = after ?? before; if (!item) return; - const toOwner = (client: WsClient) => client.userId === item.userId; - const toOthers = (client: WsClient) => client.userId !== item.userId; + const wasPublic = !!before && !isPrivate(before); + const isPublic = !!after && !isPrivate(after); - if (!after) { - broadcast(messages.deleted(item), isPrivate(item) ? toOwner : undefined); - } else if (!before) { - broadcast(messages.created(after), isPrivate(after) ? toOwner : undefined); - } else if (isPrivate(after)) { - broadcast(messages.updated(after), toOwner); - if (!isPrivate(before)) broadcast(messages.deleted(after), toOthers); - } else if (isPrivate(before)) { - broadcast(messages.updated(after), toOwner); - broadcast(messages.created(after), toOthers); - } else { - broadcast(messages.updated(after)); + broadcast( + !after + ? messages.deleted(item) + : before + ? messages.updated(after, true) + : messages.created(after, true), + (client) => client.userId === item.userId, + ); + const toOthers = (client: WsClient) => client.userId !== item.userId; + if (after && isPublic) { + broadcast( + wasPublic + ? messages.updated(after, false) + : messages.created(after, false), + toOthers, + ); + } else if (wasPublic) { + broadcast(messages.deleted(item), toOthers); } } @@ -265,15 +274,32 @@ export function broadcastCommentLikeUpdate( * Tells the clients that may see a playlist how it changed, as * {@link broadcastChange} says, for the playlist service after every write to * its details. A playlist turned private thus leaves the others' views without - * their learning what it became. + * their learning what it became. `members` are the dumps of `after`: each + * socket gets the playlist with the count of those its user may see, the + * owner's private dumps being theirs alone to count. */ export function broadcastPlaylistChange( before: Playlist | null, after: Playlist | null, + members: PlaylistMember[], ): void { + const counted = (playlist: Playlist, toOwner: boolean): Playlist => ({ + ...playlist, + dumpCount: countVisibleTo( + members, + playlist, + toOwner ? playlist.userId : null, + ), + }); broadcastChange(before, after, (playlist) => !playlist.isPublic, { - created: (playlist) => ({ type: "playlist_created", playlist }), - updated: (playlist) => ({ type: "playlist_updated", playlist }), + created: (playlist, toOwner) => ({ + type: "playlist_created", + playlist: counted(playlist, toOwner), + }), + updated: (playlist, toOwner) => ({ + type: "playlist_updated", + playlist: counted(playlist, toOwner), + }), deleted: (p) => ({ type: "playlist_deleted", playlistId: p.id, @@ -285,7 +311,8 @@ export function broadcastPlaylistChange( /** * Sends `playlist_dumps_updated` to the playlist's audience, each socket with * only the dumps its user may see, as `GET` would list them. The private dumps - * in a public playlist stay hidden from everyone but their posters. + * in a public playlist stay hidden from everyone but their posters, and so + * does their number: a client counts the dumps it is sent. */ export function broadcastPlaylistDumpsUpdated( playlist: Pick, @@ -299,7 +326,6 @@ export function broadcastPlaylistDumpsUpdated( dumpIds: members .filter((m) => isMemberVisibleTo(m, playlist, client.userId)) .map((m) => m.id), - dumpCount: members.length, }); } } diff --git a/api/test/routes/playlists.test.ts b/api/test/routes/playlists.test.ts index 2cf2c8b5..8bf36e26 100644 --- a/api/test/routes/playlists.test.ts +++ b/api/test/routes/playlists.test.ts @@ -146,6 +146,72 @@ test("a public playlist hides its private dumps from everyone but their owner", ); }); +test("a playlist's dumpCount is of the dumps the viewer may see", async () => { + const owner = await createUser(); + const author = await createUser(); + const pl = insertPlaylist({ userId: owner.id, title: "Counted banana" }); + const pub = insertDump({ userId: owner.id }); + const mine = insertDump({ userId: owner.id, isPrivate: true }); + // Someone else's, gone private after it was added: its owner no longer + // sees it there, and neither does its poster. + const theirs = insertDump({ userId: author.id, isPrivate: true }); + for (const [i, d] of [pub, mine, theirs].entries()) { + db.prepare( + `INSERT INTO playlist_dumps (playlist_id, dump_id, position, added_at) + VALUES (?, ?, ?, ?);`, + ).run(pl.id, d.id, i, new Date().toISOString()); + } + await put(`/api/me/follow/playlists/${pl.id}`, { auth: author.auth }); + + const lists = [ + "/api/playlists", + `/api/users/@${owner.username}/playlists`, + `/api/users/@${author.username}/follow/playlists`, + ]; + for ( + const [auth, expected] of [ + [owner.auth, 2], + [author.auth, 1], + [undefined, 1], + ] as const + ) { + for (const url of lists) { + const { items } = (await get>(url, { auth })) + .data; + assertEquals(items.map((p) => p.dumpCount), [expected], url); + } + assertEquals( + (await get<{ playlists: Playlist[] }>("/api/search?q=banana", { auth })) + .data.playlists.map((p) => p.dumpCount), + [expected], + ); + assertEquals( + (await get(`/api/dumps/${pub.id}/playlists`, { auth })).data + .map((p) => p.dumpCount), + [expected], + ); + const page = await get(`/api/playlists/${pl.id}`, { + auth, + }); + assertEquals(page.data.dumpCount, expected); + assertEquals(ids(page).length, expected); + } + assertEquals( + (await get(`/api/me/playlists?contains=${pub.id}`, { + auth: owner.auth, + })).data.map((m) => [m.hasDump, m.playlist.dumpCount]), + [[true, 2]], + ); + // An edit answers with the editor's count. + assertEquals( + (await put(`/api/playlists/${pl.id}`, { + auth: owner.auth, + json: { title: "Counted" }, + })).data.dumpCount, + 2, + ); +}); + test("reorder: must name exactly the current members", async () => { const owner = await createUser(); const { id } = insertPlaylist({ userId: owner.id }); diff --git a/api/test/routes/ws.test.ts b/api/test/routes/ws.test.ts index c48fa4ba..023a9a4d 100644 --- a/api/test/routes/ws.test.ts +++ b/api/test/routes/ws.test.ts @@ -709,8 +709,32 @@ wsTest( for (const c of [a, x, anon]) { const msg = await c.next("playlist_dumps_updated"); assertEquals(msg.dumpIds, [added.id, pub.id]); - // The count is the playlist's, as its dumpCount gives it to everyone. - assertEquals(msg.dumpCount, 4); + } + + // A change to its details carries the count of what each may see, too. + await put(`/api/playlists/${pl.id}`, { + auth: owner.auth, + json: { title: "Renamed" }, + }); + assertEquals((await o.next("playlist_updated")).playlist.dumpCount, 3); + for (const c of [a, x, anon]) { + assertEquals((await c.next("playlist_updated")).playlist.dumpCount, 2); + } + await put(`/api/playlists/${pl.id}`, { + auth: owner.auth, + json: { isPublic: false }, + }); + await put(`/api/playlists/${pl.id}`, { + auth: owner.auth, + json: { isPublic: true }, + }); + assertEquals( + (await o.next("playlist_updated", (m) => m.playlist.isPublic)).playlist + .dumpCount, + 3, + ); + for (const c of [a, x, anon]) { + assertEquals((await c.next("playlist_created")).playlist.dumpCount, 2); } }, ); @@ -817,7 +841,6 @@ wsTest( }); const hidden = await x.next("playlist_dumps_updated"); assertEquals(hidden.dumpIds, [keep.id, gone.id]); - assertEquals(hidden.dumpCount, 3); assertEquals( (await o.next("playlist_dumps_updated")).dumpIds, [keep.id, flip.id, gone.id], @@ -835,7 +858,7 @@ wsTest( await del(`/api/dumps/${gone.id}`, { auth: owner.auth }); const after = await x.next( "playlist_dumps_updated", - (m) => m.dumpCount === 2, + (m) => m.dumpIds.length === 2, ); assertEquals(after.dumpIds, [keep.id, flip.id]); diff --git a/api/test/services/community-service.test.ts b/api/test/services/community-service.test.ts index f5a3dd64..1f740cea 100644 --- a/api/test/services/community-service.test.ts +++ b/api/test/services/community-service.test.ts @@ -43,9 +43,25 @@ test("communityStatsOf: the period's top public dumps, members, public playlists insertPlaylist({ userId: ann.id, title: "Small" }); const full = insertPlaylist({ userId: bob.id, title: "Full" }); insertPlaylist({ userId: bob.id, title: "Hidden", isPublic: false }); + // Bob's private dump isn't counted: every reader gets the same figures. + const secret = insertDump({ + userId: bob.id, + title: "Secret", + isPrivate: true, + }); db.prepare( - `INSERT INTO playlist_dumps (playlist_id, dump_id, position, added_at) VALUES (?, ?, 0, ?), (?, ?, 1, ?);`, - ).run(full.id, hit.id, ago(0), full.id, tie.id, ago(0)); + `INSERT INTO playlist_dumps (playlist_id, dump_id, position, added_at) VALUES (?, ?, 0, ?), (?, ?, 1, ?), (?, ?, 2, ?);`, + ).run( + full.id, + hit.id, + ago(0), + full.id, + tie.id, + ago(0), + full.id, + secret.id, + ago(0), + ); const stats = communityStatsOf(...week()); assertEquals(stats.top.map((d) => d.title), ["Hit", "Newer", "Tie"]); diff --git a/shared/playlist.ts b/shared/playlist.ts index 84c42b27..95ad215f 100644 --- a/shared/playlist.ts +++ b/shared/playlist.ts @@ -40,8 +40,8 @@ export interface Playlist { /** It is undefined when the playlist has no cover. */ imageMime?: string; /** - * It counts the dumps the viewer can't see too. It is undefined when the - * query didn't count them. + * It counts the dumps the viewer may see in it, as its page lists them. It + * is undefined when the query didn't count them. */ dumpCount?: number; /** The owner's current name, which the playlist's path starts with. */ diff --git a/shared/ws.ts b/shared/ws.ts index 8aeddb04..b6152202 100644 --- a/shared/ws.ts +++ b/shared/ws.ts @@ -295,7 +295,8 @@ export interface PlaylistCreatedMessage { /** * Sent by the server when a playlist changes, to every socket if it is public, - * else to its owner's, as when it has just gone private. + * else to its owner's, as when it has just gone private. Its `dumpCount` is + * the socket's user's, as in `playlist_created`. */ export interface PlaylistUpdatedMessage { type: "playlist_updated"; @@ -325,8 +326,6 @@ export interface PlaylistDumpsUpdatedMessage { * private dump is left out. */ dumpIds: string[]; - /** Counts every dump, hidden ones included, as `Playlist.dumpCount` does. */ - dumpCount: number; } /** diff --git a/src/contexts/ws/WSContext.ts b/src/contexts/ws/WSContext.ts index 3e0460b9..ef50b298 100644 --- a/src/contexts/ws/WSContext.ts +++ b/src/contexts/ws/WSContext.ts @@ -50,8 +50,6 @@ export interface PlaylistEvent { * order. */ dumpIds?: string[]; - /** Set when the dumps changed. It counts private dumps too. */ - dumpCount?: number; } /** diff --git a/src/contexts/ws/WSProvider.tsx b/src/contexts/ws/WSProvider.tsx index 5c93265d..9904d37b 100644 --- a/src/contexts/ws/WSProvider.tsx +++ b/src/contexts/ws/WSProvider.tsx @@ -595,12 +595,11 @@ export function WSProvider({ children }: WSProviderProps): React.JSX.Element { } case "playlist_dumps_updated": { - const { playlistId, dumpIds, dumpCount } = msg; + const { playlistId, dumpIds } = msg; events.emit("playlist", { type: "dumps_updated", playlistId, dumpIds, - dumpCount, }); break; } diff --git a/src/hooks/playlist/usePlaylistListSync.ts b/src/hooks/playlist/usePlaylistListSync.ts index e67b2b5e..d5eb2b33 100644 --- a/src/hooks/playlist/usePlaylistListSync.ts +++ b/src/hooks/playlist/usePlaylistListSync.ts @@ -85,10 +85,8 @@ export function usePlaylistListSync( const idx = prev.findIndex((p) => p.id === ev.playlistId); if (idx === -1) return prev; const next = [...prev]; - next[idx] = { - ...next[idx], - dumpCount: ev.dumpCount ?? ev.dumpIds.length, - }; + // The dumps this user may see, which are those the playlist counts. + next[idx] = { ...next[idx], dumpCount: ev.dumpIds.length }; return next; } diff --git a/src/test/contexts/ws/WSProvider.test.tsx b/src/test/contexts/ws/WSProvider.test.tsx index 31425377..00e8c484 100644 --- a/src/test/contexts/ws/WSProvider.test.tsx +++ b/src/test/contexts/ws/WSProvider.test.tsx @@ -481,13 +481,11 @@ describe("dumps, playlists, comments, users", () => { type: "playlist_dumps_updated", playlistId: "p", dumpIds: ["x"], - dumpCount: 2, }); expect(events().playlist.at(-1)).toEqual({ type: "dumps_updated", playlistId: "p", dumpIds: ["x"], - dumpCount: 2, }); socket.receive({ type: "playlist_deleted", playlistId: "p", userId: "u" }); expect(ws().deletedPlaylistIds.has("p")).toBe(true); diff --git a/src/test/hooks/playlist/usePlaylistListSync.test.tsx b/src/test/hooks/playlist/usePlaylistListSync.test.tsx index 0839d3d2..f49f1289 100644 --- a/src/test/hooks/playlist/usePlaylistListSync.test.tsx +++ b/src/test/hooks/playlist/usePlaylistListSync.test.tsx @@ -99,7 +99,7 @@ describe("usePlaylistListSync", () => { expect(ids(items())).toEqual(["c"]); }); - test("dumps_updated sets the count", () => { + test("dumps_updated counts the dumps this viewer may see", () => { const { emit, items } = setup([makePlaylist({ id: "a", dumpCount: 1 })]); emit("playlist", { type: "dumps_updated", @@ -109,17 +109,6 @@ describe("usePlaylistListSync", () => { expect(items()[0].dumpCount).toBe(3); }); - test("dumps_updated counts the members this viewer can't see too", () => { - const { emit, items } = setup([makePlaylist({ id: "a", dumpCount: 3 })]); - emit("playlist", { - type: "dumps_updated", - playlistId: "a", - dumpIds: ["x", "y"], - dumpCount: 4, - }); - expect(items()[0].dumpCount).toBe(4); - }); - test("a renamed owner's playlists take the new name", () => { const { emit, items } = setup([ makePlaylist({ id: "a", userId: "u1", ownerUsername: "old" }), diff --git a/src/test/pages/Index.test.tsx b/src/test/pages/Index.test.tsx index 7af06bce..65939031 100644 --- a/src/test/pages/Index.test.tsx +++ b/src/test/pages/Index.test.tsx @@ -423,7 +423,6 @@ describe("Followed feed", () => { type: "playlist_dumps_updated", playlistId: "mix", dumpIds: ["x"], - dumpCount: 1, }); expect(await screen.findByText("Added to the mix")).toBeInTheDocument(); }); diff --git a/src/test/pages/PlaylistDetail.test.tsx b/src/test/pages/PlaylistDetail.test.tsx index 5013c038..40663451 100644 --- a/src/test/pages/PlaylistDetail.test.tsx +++ b/src/test/pages/PlaylistDetail.test.tsx @@ -452,7 +452,6 @@ describe("PlaylistDetail", () => { type: "playlist_dumps_updated", playlistId: "p1", dumpIds: ["a", "c"], - dumpCount: 3, }); expect(titles()).toEqual(["Alpha", "Charlie"]); socket.receive({ @@ -463,7 +462,6 @@ describe("PlaylistDetail", () => { type: "playlist_dumps_updated", playlistId: "p1", dumpIds: ["a", "b", "c"], - dumpCount: 3, }); expect(await screen.findByText("Bravo")).toBeInTheDocument(); expect(titles()).toEqual(["Alpha", "Bravo", "Charlie"]); @@ -522,7 +520,6 @@ describe("PlaylistDetail", () => { type: "playlist_dumps_updated", playlistId: "p1", dumpIds: ["d", "a", "b", "c"], - dumpCount: 4, }); expect(await screen.findByText("Delta")).toBeInTheDocument(); socket.receive({