From 3de365a0c92e3e7cf4801f1fbec6547e386c909f Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Mon, 24 Aug 2026 23:43:26 +0000 Subject: [PATCH] fix(stats): count reading attempts in completion analytics Use completed reading attempts as the canonical source for timelines, goals, and completion latency. Preserve user and library scoping while excluding deleted, skimmed, and undated attempts. Fixes #1132 --- .../user-statistics.repository.test.ts | 42 ++++++++- .../user-statistics.repository.ts | 77 +++++++--------- server/test/reading-attempts.e2e-spec.ts | 89 +++++++++++++++++++ 3 files changed, 160 insertions(+), 48 deletions(-) diff --git a/server/src/modules/user-statistics/user-statistics.repository.test.ts b/server/src/modules/user-statistics/user-statistics.repository.test.ts index f5e8d260b..80beb6e95 100644 --- a/server/src/modules/user-statistics/user-statistics.repository.test.ts +++ b/server/src/modules/user-statistics/user-statistics.repository.test.ts @@ -90,9 +90,7 @@ describe('UserStatisticsRepository', () => { [], [{ hour: 9, format: 'EPUB', source: 'koreader', readingSeconds: 500, eventsCount: 3 }], [{ dayOfWeek: 2, source: 'manual', format: 'EPUB', readingSeconds: 900, eventsCount: 4 }], - [], [{ year: 2026, month: 4, count: 2 }], - [], [{ year: 2026, month: 4, count: 2 }], ]); const repo = new UserStatisticsRepository(db as never); @@ -111,6 +109,44 @@ describe('UserStatisticsRepository', () => { await expect(repo.getMonthlyCompletions(5, false, [2], 365)).resolves.toEqual([{ year: 2026, month: 4, count: 2 }]); }); + it('counts completed reading attempts when their sessions finish below 99 percent', async () => { + const calls: Array<{ text: string; params: unknown[] }> = []; + const fakeClient = { + query: vi.fn().mockImplementation((cfg: { text: string }, params: unknown[]) => { + calls.push({ text: cfg.text, params }); + const count = cfg.text.includes('"reading_attempts"') ? 3 : 1; + return Promise.resolve({ rows: [[2026, 8, count]] }); + }), + }; + const db = drizzle({ client: fakeClient as never, schema }); + const repo = new UserStatisticsRepository(db as never); + + await expect(repo.getCompletionTimeline(5, true, undefined, 365)).resolves.toEqual([{ year: 2026, month: 8, count: 3 }]); + + expect(calls).toHaveLength(1); + expect(calls[0]!.text).toContain('"reading_attempts"'); + expect(calls[0]!.text).not.toContain('"reading_sessions"."end_progress"'); + }); + + it('uses completed reading attempt dates for completion latency', async () => { + const calls: Array<{ text: string; params: unknown[] }> = []; + const fakeClient = { + query: vi.fn().mockImplementation((cfg: { text: string }, params: unknown[]) => { + calls.push({ text: cfg.text, params }); + return Promise.resolve({ rows: [[2]] }); + }), + }; + const db = drizzle({ client: fakeClient as never, schema }); + const repo = new UserStatisticsRepository(db as never); + + await expect(repo.getCompletionLatencyDays(5, true, undefined, 365)).resolves.toEqual([2]); + + expect(calls).toHaveLength(1); + expect(calls[0]!.text).toContain('"reading_attempts"'); + expect(calls[0]!.text).toContain('"reading_attempts"."deleted_at" is null'); + expect(calls[0]!.params).toContain('completed'); + }); + it('returns peak reading hour buckets in the provided timezone', async () => { const db = makeDb([[], [{ hour: 23, format: 'EPUB', source: 'kobo', readingSeconds: 1800, eventsCount: 1 }]]); const repo = new UserStatisticsRepository(db as never); @@ -349,8 +385,6 @@ describe('UserStatisticsRepository', () => { it('normalizes completion latency, reading pace, survival, race, and archetype points', async () => { const db = makeDb([ - [], - [], [{ days: '3.5' }, { days: -1 }, { days: 'not-a-number' }, { days: 7 }], [{ durationSeconds: 300, progressDelta: 1.25, source: 'kobo', format: 'PDF' }], [], diff --git a/server/src/modules/user-statistics/user-statistics.repository.ts b/server/src/modules/user-statistics/user-statistics.repository.ts index 5fd2b1032..c915df4cc 100644 --- a/server/src/modules/user-statistics/user-statistics.repository.ts +++ b/server/src/modules/user-statistics/user-statistics.repository.ts @@ -1,5 +1,5 @@ import { Inject, Injectable } from '@nestjs/common'; -import { and, eq, gt, gte, inArray, isNotNull, lt, ne, sql } from 'drizzle-orm'; +import { and, eq, gt, gte, inArray, isNotNull, isNull, lt, ne, sql } from 'drizzle-orm'; import type { NodePgDatabase } from 'drizzle-orm/node-postgres'; import type { @@ -29,6 +29,7 @@ import { bookMetadata, books, genres, + readingAttempts, readingProgress, readingSessions, userBookStatus, @@ -468,21 +469,9 @@ export class UserStatisticsRepository { ): Promise { const accessible = await this.getAccessibleLibraryIds(userId, isSuperuser); const libraryFilter = this.libraryFilter(this.intersectLibraryIds(accessible, filterLibraryIds)); - const since = this.sinceDateForDays(days); - - const firstCompletion = this.db - .select({ - bookId: readingSessions.bookId, - firstCompletedAt: sql`min(${readingSessions.endedAt})`.as('first_completed_at'), - }) - .from(readingSessions) - .innerJoin(books, eq(books.id, readingSessions.bookId)) - .where(and(eq(readingSessions.userId, userId), gte(readingSessions.endProgress, 99), libraryFilter)) - .groupBy(readingSessions.bookId) - .as('first_completion'); - - const yearExpr = sql`extract(year from ${firstCompletion.firstCompletedAt})::int`; - const monthExpr = sql`extract(month from ${firstCompletion.firstCompletedAt})::int`; + const since = this.formatDayKey(this.sinceDateForDays(days)); + const yearExpr = sql`extract(year from ${readingAttempts.endedOn})::int`; + const monthExpr = sql`extract(month from ${readingAttempts.endedOn})::int`; return this.db .select({ @@ -490,8 +479,18 @@ export class UserStatisticsRepository { month: monthExpr, count: sql`count(*)::int`, }) - .from(firstCompletion) - .where(sql`${firstCompletion.firstCompletedAt} >= ${since}`) + .from(readingAttempts) + .innerJoin(books, eq(books.id, readingAttempts.bookId)) + .where( + and( + eq(readingAttempts.userId, userId), + eq(readingAttempts.outcome, 'completed'), + isNotNull(readingAttempts.endedOn), + isNull(readingAttempts.deletedAt), + gte(readingAttempts.endedOn, since), + libraryFilter, + ), + ) .groupBy(yearExpr, monthExpr) .orderBy(yearExpr, monthExpr); } @@ -546,36 +545,26 @@ export class UserStatisticsRepository { async getCompletionLatencyDays(userId: number, isSuperuser: boolean, filterLibraryIds?: number[], days = 1825): Promise { const accessible = await this.getAccessibleLibraryIds(userId, isSuperuser); const libraryFilter = this.libraryFilter(this.intersectLibraryIds(accessible, filterLibraryIds)); - const since = this.sinceDateForDays(days); - - const completedInWindow = this.db - .select({ - bookId: readingSessions.bookId, - completedAt: sql`min(${readingSessions.endedAt})`.as('completed_at'), - }) - .from(readingSessions) - .innerJoin(books, eq(books.id, readingSessions.bookId)) - .where(and(eq(readingSessions.userId, userId), gte(readingSessions.startedAt, since), gte(readingSessions.endProgress, 99), libraryFilter)) - .groupBy(readingSessions.bookId) - .as('completed_in_window'); - - const startedAndCompleted = this.db - .select({ - completedAt: completedInWindow.completedAt, - startedAt: sql`min(${readingSessions.startedAt})`.as('started_at'), - }) - .from(readingSessions) - .innerJoin(completedInWindow, eq(completedInWindow.bookId, readingSessions.bookId)) - .where(eq(readingSessions.userId, userId)) - .groupBy(completedInWindow.bookId, completedInWindow.completedAt) - .as('started_and_completed'); + const since = this.formatDayKey(this.sinceDateForDays(days)); const rows = await this.db .select({ - days: sql`extract(epoch from (${startedAndCompleted.completedAt} - ${startedAndCompleted.startedAt})) / 86400`, + days: sql`${readingAttempts.endedOn} - ${readingAttempts.startedOn}`, }) - .from(startedAndCompleted) - .where(and(sql`${startedAndCompleted.startedAt} is not null`, sql`${startedAndCompleted.completedAt} >= ${startedAndCompleted.startedAt}`)); + .from(readingAttempts) + .innerJoin(books, eq(books.id, readingAttempts.bookId)) + .where( + and( + eq(readingAttempts.userId, userId), + eq(readingAttempts.outcome, 'completed'), + isNotNull(readingAttempts.startedOn), + isNotNull(readingAttempts.endedOn), + isNull(readingAttempts.deletedAt), + gte(readingAttempts.endedOn, since), + sql`${readingAttempts.endedOn} >= ${readingAttempts.startedOn}`, + libraryFilter, + ), + ); return rows .map((row) => (typeof row.days === 'number' ? row.days : Number.parseFloat(String(row.days)))) diff --git a/server/test/reading-attempts.e2e-spec.ts b/server/test/reading-attempts.e2e-spec.ts index 18dde137b..33192c62a 100644 --- a/server/test/reading-attempts.e2e-spec.ts +++ b/server/test/reading-attempts.e2e-spec.ts @@ -707,4 +707,93 @@ describe('Reading attempts main-flow simulation (docker e2e)', { timeout: TIMEOU expect(response.json()).toMatchObject({ errorCode: 'READING_DATES_STATUS_CONFLICT' }); expect((await listAttempts(book.bookId)).total).toBe(0); }); + + it('31. counts completed attempts in completion analytics regardless of session end progress', async () => { + const username = `attempt-stats-${randomUUID()}`; + const password = 'AttemptStats123!'; + const [user] = await ctx.db + .insert(schema.users) + .values({ + username, + name: 'Attempt Stats User', + passwordHash: await hash(password, 4), + isDefaultPassword: false, + provisioningMethod: 'local', + }) + .returning({ id: schema.users.id }); + await ctx.db.insert(schema.userLibraryAccess).values({ userId: user.id, libraryId, accessLevel: 'viewer' }); + + const attempts = ctx.app.get(ReadingAttemptService); + const endProgressValues = [99.7, 94.7, 86.8]; + for (const [index, endProgress] of endProgressValues.entries()) { + const book = await createBook(); + const endedAt = new Date(); + endedAt.setUTCDate(endedAt.getUTCDate() - (index + 1) * 3); + const startedAt = new Date(endedAt); + startedAt.setUTCDate(startedAt.getUTCDate() - 2); + const startedOn = startedAt.toISOString().slice(0, 10); + const endedOn = endedAt.toISOString().slice(0, 10); + const attempt = await attempts.createHistorical(user.id, book.bookId, { startedOn, endedOn, outcome: 'completed' }); + + const sessionStartedAt = new Date(`${startedOn}T10:00:00.000Z`); + const sessionEndedAt = new Date(sessionStartedAt.getTime() + 60 * 60 * 1000); + await ctx.db.insert(schema.readingSessions).values({ + userId: user.id, + bookFileId: book.fileId, + bookId: book.bookId, + attemptId: attempt.id, + sessionId: `attempt-stats-${randomUUID()}`, + source: 'koreader', + startedAt: sessionStartedAt, + endedAt: sessionEndedAt, + durationSeconds: 3600, + progressDelta: 10, + endProgress, + }); + } + + const excludedBook = await createBook(); + const excluded = await attempts.createHistorical(user.id, excludedBook.bookId, { + startedOn: '2026-01-01', + endedOn: '2026-01-02', + outcome: 'completed', + }); + await attempts.delete(user.id, excludedBook.bookId, excluded.id); + await attempts.createHistorical(user.id, excludedBook.bookId, { + startedOn: '2026-01-03', + endedOn: '2026-01-04', + outcome: 'skimmed', + }); + await attempts.createHistorical(user.id, excludedBook.bookId, { startedOn: null, endedOn: null, outcome: 'completed' }); + + const login = await ctx.app.inject({ method: 'POST', url: '/api/v1/auth/login', payload: { username, password } }); + expect(login.statusCode).toBe(200); + const token = (login.json() as { accessToken: string }).accessToken; + + const timelineResponse = await ctx.app.inject({ + method: 'GET', + url: '/api/v1/user-statistics/completion-timeline?days=365', + headers: auth(token), + }); + expect(timelineResponse.statusCode).toBe(200); + const timeline = timelineResponse.json() as Array<{ count: number }>; + expect(timeline.reduce((sum, point) => sum + point.count, 0)).toBe(3); + + const goalResponse = await ctx.app.inject({ + method: 'GET', + url: '/api/v1/user-statistics/goal-trajectory?days=365&goalBooks=12', + headers: auth(token), + }); + expect(goalResponse.statusCode).toBe(200); + const goal = goalResponse.json() as { points: Array<{ actualCumulative: number }> }; + expect(goal.points.at(-1)?.actualCumulative).toBe(3); + + const latencyResponse = await ctx.app.inject({ + method: 'GET', + url: '/api/v1/user-statistics/completion-latency?days=365', + headers: auth(token), + }); + expect(latencyResponse.statusCode).toBe(200); + expect(latencyResponse.json()).toMatchObject({ totalCompletions: 3, medianDays: 2 }); + }); });