style(client): refine app shell actions

Move GitHub actions into the sidebar footer and tighten header grouping.
This commit is contained in:
neon
2026-08-15 02:30:01 +00:00
parent c9ebf0ab7f
commit f49541cb20
152 changed files with 4532 additions and 4370 deletions
+28 -8
View File
@@ -43,21 +43,41 @@ describe('loggerConfig', () => {
const config = await loadLoggerConfig('development');
const pinoHttp = config.pinoHttp!;
expect(pinoHttp.customSuccessMessage?.({ method: 'GET', url: '/api/books' } as never, { statusCode: 201 } as never, 7.2)).toBe(
expect(pinoHttp.customSuccessMessage?.({ method: 'GET', url: '/api/books' } as never, { headersSent: true, statusCode: 201 } as never, 7.2)).toBe(
'[HTTP] GET /api/books 201 +7ms',
);
expect(pinoHttp.customErrorMessage?.({ method: 'POST', url: '/api/books' } as never, { statusCode: 500 } as never, new Error('boom'))).toBe(
'[HTTP] POST /api/books 500 - boom',
);
expect(
pinoHttp.customErrorMessage?.(
{ method: 'POST', url: '/api/books' } as never,
{ headersSent: true, statusCode: 500 } as never,
new Error('boom'),
),
).toBe('[HTTP] POST /api/books 500 - boom');
expect(pinoHttp.customLogLevel?.({} as never, { statusCode: 200 } as never)).toBe('debug');
expect(pinoHttp.customLogLevel?.({} as never, { statusCode: 404 } as never)).toBe('warn');
expect(pinoHttp.customLogLevel?.({} as never, { statusCode: 503 } as never)).toBe('error');
expect(pinoHttp.customLogLevel?.({} as never, { statusCode: 200 } as never, new Error('fail'))).toBe('error');
expect(pinoHttp.customLogLevel?.({} as never, { headersSent: true, statusCode: 200 } as never)).toBe('debug');
expect(pinoHttp.customLogLevel?.({} as never, { headersSent: true, statusCode: 404 } as never)).toBe('warn');
expect(pinoHttp.customLogLevel?.({} as never, { headersSent: true, statusCode: 503 } as never)).toBe('error');
expect(pinoHttp.customLogLevel?.({} as never, { headersSent: true, statusCode: 200 } as never, new Error('fail'))).toBe('error');
expect(pinoHttp.customProps).toBeUndefined();
});
it('reports a request whose response never reached the client as aborted, not 200', async () => {
const config = await loadLoggerConfig('development');
const pinoHttp = config.pinoHttp!;
// Node initialises `statusCode` to 200, so an aborted request carries a 200 it never sent.
const neverSent = { headersSent: false, statusCode: 200 } as never;
expect(pinoHttp.customSuccessMessage?.({ method: 'GET', url: '/api/v1/smart-scopes' } as never, neverSent, 15)).toBe(
'[HTTP] GET /api/v1/smart-scopes aborted +15ms',
);
expect(pinoHttp.customErrorMessage?.({ method: 'GET', url: '/api/v1/smart-scopes' } as never, neverSent, new Error('socket hang up'))).toBe(
'[HTTP] GET /api/v1/smart-scopes aborted - socket hang up',
);
expect(pinoHttp.customLogLevel?.({} as never, neverSent)).toBe('warn');
});
it('filters framework context logs while keeping regular logs', async () => {
const config = await loadLoggerConfig('development');
const logMethod = config.pinoHttp?.hooks?.logMethod;
+24 -4
View File
@@ -6,6 +6,23 @@ const logLevel = process.env.LOG_LEVEL ?? 'info';
const FRAMEWORK_CONTEXTS = new Set(['InstanceLoader', 'RouterExplorer', 'RoutesResolver']);
const NO_RESPONSE_STATUS = 'aborted';
/**
* `ServerResponse.statusCode` is 200 from the moment the object is created, so a request whose
* connection died before a single header was written still reports 200 here. That made failed
* requests indistinguishable from successful ones in the access log, while pino's own response
* serializer recorded them honestly as `"statusCode": null` (it emits `headersSent ? statusCode :
* null`). Trust `headersSent` for the same reason pino does.
*/
function outcomeOf(res: ServerResponse): number | typeof NO_RESPONSE_STATUS {
return res.headersSent ? res.statusCode : NO_RESPONSE_STATUS;
}
function isFailure(res: ServerResponse): boolean {
return !res.headersSent || res.statusCode >= 400;
}
export const loggerConfig: Params = {
exclude: [],
pinoHttp: {
@@ -25,19 +42,22 @@ export const loggerConfig: Params = {
},
},
customSuccessMessage: (req: IncomingMessage, res: ServerResponse, responseTime: number) => {
return `[HTTP] ${req.method} ${req.url} ${res.statusCode} +${Math.round(responseTime)}ms`;
return `[HTTP] ${req.method} ${req.url} ${outcomeOf(res)} +${Math.round(responseTime)}ms`;
},
customErrorMessage: (req: IncomingMessage, res: ServerResponse, err: Error) => {
return `[HTTP] ${req.method} ${req.url} ${res.statusCode} - ${err?.message ?? 'error'}`;
return `[HTTP] ${req.method} ${req.url} ${outcomeOf(res)} - ${err?.message ?? 'error'}`;
},
customLogLevel: (_req: IncomingMessage, res: ServerResponse, err?: Error) => {
if (err || res.statusCode >= 500) return 'error';
if (res.statusCode >= 400) return 'warn';
// A response that never reached the client is a failed request, not a quiet success.
if (isFailure(res)) return 'warn';
return 'debug';
},
serializers: {
req: (req: IncomingMessage & { id?: string }) => ({ id: req.id, method: req.method, url: req.url }),
res: (res: ServerResponse) => ({ statusCode: res.statusCode }),
// pino-http wraps this with `wrapResponseSerializer`, so the argument is pino's already
// serialized response rather than the raw one: `statusCode` is null when nothing was sent.
res: (res: { statusCode: number | null }) => ({ statusCode: res.statusCode }),
},
redact: {
paths: [
@@ -14,8 +14,37 @@ import {
isSecureProtocol,
applyConditionalHsts,
registerConditionalHsts,
isStaticAssetPath,
shouldServeSpaFallback,
} from './bootstrap.utils';
describe('SPA fallback routing', () => {
it.each([
'/assets/index-BMAlyH9T.js',
'/assets/index-D5CelejF.css',
'/sw.js',
'/manifest.webmanifest',
'/favicon.ico',
'/pwa-192x192.png',
'/assets/foliate/reader.js?v=2',
])('treats %s as a static asset so a missing file 404s instead of returning index.html', (url) => {
expect(isStaticAssetPath(url)).toBe(true);
expect(shouldServeSpaFallback(url)).toBe(false);
});
it.each(['/', '/dashboard', '/book/123/files', '/library/12', '/settings/admin/audit-log', '/authors/42', '/read/1/2', '/collections?sort=name'])(
'serves the SPA shell for client route %s',
(url) => {
expect(shouldServeSpaFallback(url)).toBe(true);
},
);
it('ignores the query string and hash when deciding', () => {
expect(shouldServeSpaFallback('/assets/app-abc123.js?import&t=1')).toBe(false);
expect(shouldServeSpaFallback('/dashboard?tab=recent#top')).toBe(true);
});
});
describe('parseBooleanEnv', () => {
it('returns fallback when value is undefined', () => {
expect(parseBooleanEnv(undefined)).toBe(false);
@@ -84,6 +84,27 @@ export function buildHelmetOptions(options: CspOptions = {}) {
};
}
/**
* Decides whether an unmatched request should be answered with the SPA shell.
*
* Serving `index.html` for everything means a hashed bundle that no longer exists after an upgrade
* answers `200 OK` with an HTML body under a `.js` URL. A 200 is storable, so both the browser HTTP
* cache and the Workbox precache keep it, dynamic imports of that chunk fail, and the broken state
* survives reloads until the user clears site data. Anything that names a file gets a real 404.
*
* Every BookOrbit route parameter is numeric, so "has an extension" reliably separates asset
* requests from client-side routes such as `/book/123/files`.
*/
export function isStaticAssetPath(url: string): boolean {
const pathname = url.split('?')[0].split('#')[0];
const lastSegment = pathname.slice(pathname.lastIndexOf('/') + 1);
return lastSegment.includes('.');
}
export function shouldServeSpaFallback(url: string): boolean {
return !isStaticAssetPath(url);
}
export function shouldInjectEmptyJsonBody(method: string, headers: IncomingHttpHeaders): boolean {
const contentType = getHeaderValue(headers['content-type'])?.toLowerCase();
if (!BODY_METHODS.has(method.toUpperCase()) || !contentType?.startsWith('application/json')) {
+4
View File
@@ -22,6 +22,7 @@ import {
registerConditionalHsts,
registerEmptyBodyContentTypeParser,
shouldInjectEmptyJsonBody,
shouldServeSpaFallback,
} from './common/utils/bootstrap.utils';
const MAX_COVER_BYTES = 20 * 1024 * 1024;
@@ -100,6 +101,9 @@ async function bootstrap() {
if (request.url.startsWith('/api')) {
return nestHandler(request, reply);
}
if (!shouldServeSpaFallback(request.url)) {
return reply.status(404).send({ statusCode: 404, message: 'Not Found', path: request.url });
}
return reply.sendFile('index.html');
});
};
@@ -444,4 +444,35 @@ describe('DashboardWidgetService', () => {
expect(widgetRepo.getLibraryOverview).toHaveBeenCalledTimes(2);
});
});
describe('getWidgets', () => {
it('resolves several widgets in one call', async () => {
const { service, widgetRepo, libraryService } = makeService();
libraryService.findAccessibleLibraryIds.mockResolvedValue([1]);
widgetRepo.getCompletedBooksThisYear.mockResolvedValue(4);
widgetRepo.getLibraryOverview.mockResolvedValue({ totalBooks: 100, formats: [] });
const response = await service.getWidgets(['reading-goal', 'library-overview'], makeUser());
expect(response.items).toHaveLength(2);
expect(response.items.map((item) => item.type)).toEqual(['reading-goal', 'library-overview']);
expect(response.items.every((item) => item.failed)).toBe(false);
expect(response.items[0]?.data).toMatchObject({ completedBooks: 4 });
});
it('isolates a failing widget so the rest of the dashboard still loads', async () => {
const { service, widgetRepo, libraryService } = makeService();
libraryService.findAccessibleLibraryIds.mockResolvedValue([1]);
widgetRepo.getCompletedBooksThisYear.mockRejectedValue(new Error('statement timeout'));
widgetRepo.getLibraryOverview.mockResolvedValue({ totalBooks: 100, formats: [] });
const response = await service.getWidgets(['reading-goal', 'library-overview'], makeUser());
const readingGoal = response.items.find((item) => item.type === 'reading-goal');
const libraryOverview = response.items.find((item) => item.type === 'library-overview');
expect(readingGoal).toMatchObject({ failed: true, data: null });
expect(libraryOverview?.failed).toBe(false);
expect(libraryOverview?.data).toBeTruthy();
});
});
});
@@ -1,7 +1,9 @@
import { Injectable } from '@nestjs/common';
import { Injectable, Logger } from '@nestjs/common';
import type {
CurrentlyReadingWidgetData,
DashboardWidgetBatchResponse,
DashboardWidgetBatchResult,
DiversityScoreWidgetData,
HighlightOfTheDayWidgetData,
LibraryOverviewWidgetData,
@@ -13,11 +15,14 @@ import type {
ReadingRhythmWidgetData,
ReadingStreakWidgetData,
UserSettings,
WidgetType,
YearProjectionWidgetData,
} from '@bookorbit/types';
import type { RequestUser } from '../../common/types/request-user';
import { StatsCache } from '../../common/cache/stats-cache';
import { mapWithConcurrency } from '../../common/utils/batch.utils';
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
import { LibraryService } from '../library/library.service';
import {
buildDaysSeries,
@@ -36,9 +41,12 @@ import { DashboardWidgetRepository } from './dashboard-widget.repository';
const DASHBOARD_LIVE_TTL_MS = 120_000;
const DASHBOARD_STALE_TTL_MS = 300_000;
const DASHBOARD_CACHE_MAX_ENTRIES = 200;
// Matches the scroller batch: enough to overlap query latency without flooding the connection pool.
const WIDGET_QUERY_CONCURRENCY = 3;
@Injectable()
export class DashboardWidgetService {
private readonly logger = new Logger(DashboardWidgetService.name);
private readonly liveCache = new StatsCache({ ttlMs: DASHBOARD_LIVE_TTL_MS, maxEntries: DASHBOARD_CACHE_MAX_ENTRIES });
private readonly staleCache = new StatsCache({ ttlMs: DASHBOARD_STALE_TTL_MS, maxEntries: DASHBOARD_CACHE_MAX_ENTRIES });
@@ -216,4 +224,53 @@ export class DashboardWidgetService {
return { days, ...rhythm };
});
}
private readonly widgetLoaders: Record<WidgetType, (user: RequestUser) => Promise<DashboardWidgetBatchResult['data']>> = {
'reading-goal': (user) => this.getReadingGoal(user),
'currently-reading': (user) => this.getCurrentlyReading(user),
'reading-streak': (user) => this.getReadingStreak(user),
'library-overview': (user) => this.getLibraryOverview(user),
'highlight-of-the-day': (user) => this.getHighlightOfTheDay(user),
'monthly-challenge': (user) => this.getMonthlyChallenge(user),
'year-projection': (user) => this.getYearProjection(user),
'neglected-gems': (user) => this.getNeglectedGems(user),
'reading-dna': (user) => this.getReadingDna(user),
'long-wait': (user) => this.getLongWait(user),
'diversity-score': (user) => this.getDiversityScore(user),
'reading-rhythm': (user) => this.getReadingRhythm(user),
};
/**
* Resolves a whole dashboard's widgets over one request.
*
* Twelve separate widget calls plus the shelves and the sidebar put a page load well past the six
* connections a browser will open to one origin, and every widget fetches once on mount with no
* retry, so a request lost in that crowd leaves a tile stuck on "Failed to load" for the life of
* the page. Failures are reported per widget rather than failing the batch.
*/
async getWidgets(types: readonly WidgetType[], user: RequestUser): Promise<DashboardWidgetBatchResponse> {
const startedAt = Date.now();
this.logger.debug(`[dashboard.widget_batch] [start] userId=${user.id} widgetCount=${types.length} - widget batch started`);
const items = await mapWithConcurrency(types, WIDGET_QUERY_CONCURRENCY, async (type): Promise<DashboardWidgetBatchResult> => {
const widgetStartedAt = Date.now();
try {
return { type, data: await this.widgetLoaders[type](user), failed: false };
} catch (error) {
const errorClass = error instanceof Error ? error.constructor.name : typeof error;
const message = sanitizeLogValue(error instanceof Error ? error.message : error);
this.logger.warn(
`[dashboard.widget_query] [fail] userId=${user.id} type=${type} durationMs=${Date.now() - widgetStartedAt} errorClass=${errorClass} error="${message}" - widget query failed`,
);
return { type, data: null, failed: true };
}
});
const failedCount = items.filter((item) => item.failed).length;
this.logger.debug(
`[dashboard.widget_batch] [end] userId=${user.id} durationMs=${Date.now() - startedAt} widgetCount=${items.length} failedCount=${failedCount} - widget batch completed`,
);
return { items };
}
}
@@ -40,6 +40,7 @@ function makeController() {
getLongWait: vi.fn(),
getDiversityScore: vi.fn(),
getReadingRhythm: vi.fn(),
getWidgets: vi.fn(),
};
const controller = new DashboardController(dashboardService as never, widgetService as never);
@@ -79,6 +80,28 @@ describe('DashboardController', () => {
});
describe('widget endpoints', () => {
it('getWidgets delegates the batch to widgetService', async () => {
const { controller, widgetService } = makeController();
const user = makeUser();
const mockResult = { items: [{ type: 'reading-goal' as const, data: null, failed: false }] };
widgetService.getWidgets.mockResolvedValue(mockResult);
const result = await controller.getWidgets(user, { widgets: ['reading-goal', 'reading-dna'] });
expect(widgetService.getWidgets).toHaveBeenCalledWith(['reading-goal', 'reading-dna'], user);
expect(result).toBe(mockResult);
});
it('getWidgets asks for each widget once even when the client repeats one', async () => {
const { controller, widgetService } = makeController();
const user = makeUser();
widgetService.getWidgets.mockResolvedValue({ items: [] });
await controller.getWidgets(user, { widgets: ['reading-goal', 'reading-goal', 'reading-dna'] });
expect(widgetService.getWidgets).toHaveBeenCalledWith(['reading-goal', 'reading-dna'], user);
});
it('getReadingGoal delegates to widgetService', async () => {
const { controller, widgetService } = makeController();
const user = makeUser();
@@ -5,6 +5,7 @@ import type { RequestUser } from '../../common/types/request-user';
import { DashboardService } from './dashboard.service';
import { DashboardWidgetService } from './dashboard-widget.service';
import { DashboardScrollerBatchDto } from './dto/dashboard-scroller-batch.dto';
import { DashboardWidgetBatchDto } from './dto/dashboard-widget-batch.dto';
import { ScrollerType } from './dto/scroller-type.enum';
@Controller('dashboard')
@@ -30,6 +31,12 @@ export class DashboardController {
return this.dashboardService.getScrollers(dto, user);
}
@Post('widgets/batch')
@HttpCode(HttpStatus.OK)
getWidgets(@CurrentUser() user: RequestUser, @Body() dto: DashboardWidgetBatchDto) {
return this.widgetService.getWidgets([...new Set(dto.widgets)], user);
}
@Get('widgets/reading-goal')
getReadingGoal(@CurrentUser() user: RequestUser) {
return this.widgetService.getReadingGoal(user);
@@ -0,0 +1,39 @@
import 'reflect-metadata';
import { plainToInstance } from 'class-transformer';
import { validate, type ValidationError } from 'class-validator';
import { DASHBOARD_WIDGET_BATCH_MAX, WIDGET_TYPES } from '@bookorbit/types';
import { DashboardWidgetBatchDto } from './dashboard-widget-batch.dto';
function constraintNames(errors: ValidationError[]): string[] {
return errors.flatMap((error) => [...Object.keys(error.constraints ?? {}), ...constraintNames(error.children ?? [])]);
}
describe('DashboardWidgetBatchDto', () => {
it('accepts the widgets a dashboard can hold', async () => {
const dto = plainToInstance(DashboardWidgetBatchDto, { widgets: [...WIDGET_TYPES] });
await expect(validate(dto)).resolves.toEqual([]);
});
it('rejects an empty batch', async () => {
const dto = plainToInstance(DashboardWidgetBatchDto, { widgets: [] });
expect(constraintNames(await validate(dto))).toContain('arrayMinSize');
});
it('rejects batches above the widget-count limit', async () => {
const dto = plainToInstance(DashboardWidgetBatchDto, {
widgets: Array.from({ length: DASHBOARD_WIDGET_BATCH_MAX + 1 }, () => 'reading-goal'),
});
expect(constraintNames(await validate(dto))).toContain('arrayMaxSize');
});
it('rejects an unknown widget type', async () => {
const dto = plainToInstance(DashboardWidgetBatchDto, { widgets: ['reading-goal', 'podcast-streak'] });
expect(constraintNames(await validate(dto))).toContain('isIn');
});
});
@@ -0,0 +1,11 @@
import { ArrayMaxSize, ArrayMinSize, IsArray, IsIn } from 'class-validator';
import { DASHBOARD_WIDGET_BATCH_MAX, WIDGET_TYPES, type WidgetType } from '@bookorbit/types';
export class DashboardWidgetBatchDto {
@IsArray()
@ArrayMinSize(1)
@ArrayMaxSize(DASHBOARD_WIDGET_BATCH_MAX)
@IsIn(WIDGET_TYPES, { each: true })
widgets!: WidgetType[];
}