fix(settings): add optional dashboard shelf sync

Keep dashboard layouts browser-local by default and let users opt in to account storage.
Group library selection and shelf sync in one dropdown, preserve legacy client settings,
and block sync changes for demo-restricted accounts.

Fixes #1646

server/src/modules/user/user.controller.test.ts

@@ -67,9 +67,10 @@ describe('UserController', () => {
     const settings = { dashboardConfig: { readingGoal: 12 } };
     userService.updateMySettings.mockResolvedValue({ id: 7, settings });
 
-    const result = await controller.updateMySettings({ id: 7 } as any, { settings } as any);
+    const user = { id: 7, permissions: [] } as any;
+    const result = await controller.updateMySettings(user, { settings } as any);
 
-    expect(userService.updateMySettings).toHaveBeenCalledWith(7, { settings });
+    expect(userService.updateMySettings).toHaveBeenCalledWith(user, { settings });
     expect(result).toEqual({ id: 7, settings });
   });
 

server/src/modules/user/user.controller.ts

@@ -91,7 +91,7 @@ export class UserController {
 
   @Patch('me/settings')
   updateMySettings(@CurrentUser() user: RequestUser, @Body() dto: UpdateMeSettingsDto) {
-    return this.userService.updateMySettings(user.id, dto);
+    return this.userService.updateMySettings(user, dto);
   }
 
   @Patch('me/reader-storage-mode')

server/src/modules/user/user.dto.test.ts

@@ -1,6 +1,7 @@
 import 'reflect-metadata';
 
 import { plainToInstance } from 'class-transformer';
+import { ValidationPipe } from '@nestjs/common';
 import { validate } from 'class-validator';
 import { Permission } from '@bookorbit/types';
 
@@ -17,6 +18,17 @@ async function hasErrors(dto: object): Promise<boolean> {
 }
 
 describe('User DTO validation', () => {
+  it('accepts optional shelf sync settings through the global validation contract', async () => {
+    const pipe = new ValidationPipe({ whitelist: true, forbidNonWhitelisted: true, transform: true });
+    for (const settings of [
+      { dashboardConfig: { readingGoal: 12 } },
+      { dashboardShelfConfig: { syncAcrossSessions: true, shelfLayout: 'two-columns', scrollers: [] } },
+      { dashboardShelfConfig: { syncAcrossSessions: false } },
+    ]) {
+      const result = await pipe.transform({ settings }, { type: 'body', metatype: UpdateMeSettingsDto });
+      expect(result.settings).toEqual(settings);
+    }
+  });
   it('CreateUserDto requires email, enforces username minimum length, and validates permission enums', async () => {
     const bad = plainToInstance(CreateUserDto, { username: 'ab', name: 'n', permissionNames: [1, 2] });
     expect(await hasErrors(bad)).toBe(true);

server/src/modules/user/user.service.test.ts

@@ -280,23 +280,40 @@ describe('UserService', () => {
     const updated = { id: 2, settings: { dashboardConfig: { readingGoal: 12 } } };
     userRepo.update.mockResolvedValue(updated);
 
-    const result = await service.updateMySettings(2, { settings: { dashboardConfig: { readingGoal: 12 } } });
+    const result = await service.updateMySettings(reqUser({ id: 2 }), { settings: { dashboardConfig: { readingGoal: 12 } } });
 
     expect(userRepo.update).toHaveBeenCalledWith(2, { settings: { dashboardConfig: { readingGoal: 12 } } });
     expect(result).toEqual(updated);
   });
 
+  it.each([false, true])('rejects shelf sync settings from demo-restricted accounts with isSuperuser=%s', async (isSuperuser) => {
+    const user = reqUser({ isSuperuser, permissions: [Permission.DemoRestricted] });
+    for (const dashboardShelfConfig of [{ syncAcrossSessions: true }, { syncAcrossSessions: false }, null]) {
+      await expect(service.updateMySettings(user, { settings: { dashboardShelfConfig } })).rejects.toBeInstanceOf(ForbiddenException);
+    }
+    expect(userRepo.update).not.toHaveBeenCalled();
+  });
+
+  it('allows demo-restricted accounts to save existing dashboard widget settings', async () => {
+    const user = reqUser({ permissions: [Permission.DemoRestricted] });
+    const settings = { dashboardConfig: { readingGoal: 12 } };
+    userRepo.update.mockResolvedValue({ id: user.id, settings });
+
+    await expect(service.updateMySettings(user, { settings })).resolves.toEqual({ id: user.id, settings });
+    expect(userRepo.update).toHaveBeenCalledWith(user.id, { settings });
+  });
+
   it('updateMySettings throws NotFoundException when user does not exist', async () => {
     userRepo.update.mockResolvedValue(null);
 
-    await expect(service.updateMySettings(99, { settings: { theme: 'dark' } })).rejects.toBeInstanceOf(NotFoundException);
+    await expect(service.updateMySettings(reqUser({ id: 99 }), { settings: { theme: 'dark' } })).rejects.toBeInstanceOf(NotFoundException);
   });
 
   it('updateMySettings passes arbitrary nested settings without modification', async () => {
     const settings = { nested: { deeply: { value: true } }, arr: [1, 2, 3] };
     userRepo.update.mockResolvedValue({ id: 5, settings });
 
-    await service.updateMySettings(5, { settings });
+    await service.updateMySettings(reqUser({ id: 5 }), { settings });
 
     expect(userRepo.update).toHaveBeenCalledWith(5, { settings });
   });
@@ -306,7 +323,7 @@ describe('UserService', () => {
     userRepo.update.mockResolvedValue({ id: 5, settings: { timezone: 'America/Halifax' } });
     userStatistics.rebuildDailyStatsForUser.mockResolvedValue({ deleted: 12, inserted: 9, libraries: 1 });
 
-    await service.updateMySettings(5, { settings: { timezone: 'America/Halifax' } });
+    await service.updateMySettings(reqUser({ id: 5 }), { settings: { timezone: 'America/Halifax' } });
 
     expect(userStatistics.rebuildDailyStatsForUser).toHaveBeenCalledWith(5, 'America/Halifax');
   });
@@ -317,7 +334,7 @@ describe('UserService', () => {
     userRepo.findSettingsById.mockResolvedValue({ timezone: 'America/Halifax' });
     userRepo.update.mockResolvedValue({ id: 5, settings: { timezone: 'America/Halifax' } });
 
-    await service.updateMySettings(5, { settings: { timezone: 'America/Halifax' } });
+    await service.updateMySettings(reqUser({ id: 5 }), { settings: { timezone: 'America/Halifax' } });
 
     expect(userStatistics.rebuildDailyStatsForUser).toHaveBeenCalledWith(5, 'America/Halifax');
   });
@@ -327,12 +344,12 @@ describe('UserService', () => {
     userRepo.update.mockResolvedValue({ id: 5, settings: { timezone: 'America/Halifax' } });
     userStatistics.rebuildDailyStatsForUser.mockRejectedValueOnce(new Error('deadlock detected'));
 
-    await service.updateMySettings(5, { settings: { timezone: 'America/Halifax' } });
+    await service.updateMySettings(reqUser({ id: 5 }), { settings: { timezone: 'America/Halifax' } });
 
     userStatistics.rebuildDailyStatsForUser.mockResolvedValue({ deleted: 4, inserted: 3, libraries: 1 });
     userRepo.findSettingsById.mockResolvedValue({ timezone: 'America/Halifax' });
 
-    await service.updateMySettings(5, { settings: { timezone: 'America/Halifax' } });
+    await service.updateMySettings(reqUser({ id: 5 }), { settings: { timezone: 'America/Halifax' } });
 
     expect(userStatistics.rebuildDailyStatsForUser).toHaveBeenCalledTimes(2);
   });
@@ -350,7 +367,7 @@ describe('UserService', () => {
       }),
     );
 
-    await expect(service.updateMySettings(5, { settings: { timezone: 'America/Halifax' } })).resolves.toEqual({
+    await expect(service.updateMySettings(reqUser({ id: 5 }), { settings: { timezone: 'America/Halifax' } })).resolves.toEqual({
       id: 5,
       settings: { timezone: 'America/Halifax' },
     });
@@ -362,7 +379,7 @@ describe('UserService', () => {
   it('leaves stats alone for a settings write that does not touch the timezone', async () => {
     userRepo.update.mockResolvedValue({ id: 5, settings: { timezone: 'America/Halifax', theme: 'dark' } });
 
-    await service.updateMySettings(5, { settings: { theme: 'dark' } });
+    await service.updateMySettings(reqUser({ id: 5 }), { settings: { theme: 'dark' } });
 
     // Reading the stored settings is only worth a query when this write can replace the zone.
     expect(userRepo.findSettingsById).not.toHaveBeenCalled();
@@ -374,7 +391,7 @@ describe('UserService', () => {
     userRepo.findSettingsById.mockResolvedValue({});
     userRepo.update.mockResolvedValue({ id: 5, settings: { timezone: 'Europe/Berlin' } });
 
-    await service.updateMySettings(5, { settings: { timezone: 'Europe/Berlin' } });
+    await service.updateMySettings(reqUser({ id: 5 }), { settings: { timezone: 'Europe/Berlin' } });
     expect(userStatistics.rebuildDailyStatsForUser).toHaveBeenCalledWith(5, 'Europe/Berlin');
 
     userStatistics.rebuildDailyStatsForUser.mockClear();
@@ -382,7 +399,7 @@ describe('UserService', () => {
     userRepo.findSettingsById.mockResolvedValue({ timezone: 'Europe/Berlin' });
     userRepo.update.mockResolvedValue({ id: 5, settings: { timezone: 'Not/AZone' } });
 
-    await service.updateMySettings(5, { settings: { timezone: 'Not/AZone' } });
+    await service.updateMySettings(reqUser({ id: 5 }), { settings: { timezone: 'Not/AZone' } });
     expect(userStatistics.rebuildDailyStatsForUser).toHaveBeenCalledWith(5, 'UTC');
   });
 
@@ -392,13 +409,13 @@ describe('UserService', () => {
     userRepo.update.mockResolvedValue(updated);
     userStatistics.rebuildDailyStatsForUser.mockRejectedValue(new Error('deadlock detected'));
 
-    await expect(service.updateMySettings(5, { settings: { timezone: 'America/Halifax' } })).resolves.toEqual(updated);
+    await expect(service.updateMySettings(reqUser({ id: 5 }), { settings: { timezone: 'America/Halifax' } })).resolves.toEqual(updated);
   });
 
   it('caches an explicit achievement preference when settings are updated', async () => {
     userRepo.update.mockResolvedValue({ id: 5, settings: { achievementPreferences: { enabled: false } } });
 
-    await service.updateMySettings(5, { settings: { achievementPreferences: { enabled: false } } });
+    await service.updateMySettings(reqUser({ id: 5 }), { settings: { achievementPreferences: { enabled: false } } });
 
     await expect(service.isAchievementEnabled(5)).resolves.toBe(false);
     expect(userRepo.findSettingsById).not.toHaveBeenCalled();

server/src/modules/user/user.service.ts

@@ -228,7 +228,14 @@ export class UserService {
     return user;
   }
 
-  async updateMySettings(userId: number, dto: UpdateMeSettingsDto) {
+  async updateMySettings(requestingUser: RequestUser, dto: UpdateMeSettingsDto) {
+    if (
+      requestingUser.permissions.includes(Permission.DemoRestricted) &&
+      Object.prototype.hasOwnProperty.call(dto.settings, 'dashboardShelfConfig')
+    ) {
+      throw new ForbiddenException('Demo-restricted account cannot change dashboard shelf sync settings');
+    }
+    const userId = requestingUser.id;
     // Settings are merged server-side, so the stored row is the only place the outgoing
     // timezone still exists. Read only when this write is the one that can replace it.
     const touchesTimeZone = Object.prototype.hasOwnProperty.call(dto.settings, 'timezone');

server/test/auth-session-security.e2e-spec.ts

@@ -5,6 +5,7 @@ import { Test } from '@nestjs/testing';
 import { FastifyAdapter, type NestFastifyApplication } from '@nestjs/platform-fastify';
 import { and, eq, gt, isNull } from 'drizzle-orm';
 import { hash } from 'bcryptjs';
+import { Permission } from '@bookorbit/types';
 
 import { AppModule } from '../src/app.module';
 import { DB } from '../src/db';
@@ -295,6 +296,96 @@ describe('Auth session security (e2e)', () => {
     });
   });
 
+  describe('dashboard shelf settings', () => {
+    it.each([false, true])(
+      'rejects shelf sync writes for demo-restricted accounts with isSuperuser=%s',
+      async (isSuperuser) => {
+        const demo = await createLocalUser(context.db);
+        await context.db.update(schema.users).set({ isSuperuser }).where(eq(schema.users.id, demo.userId));
+        await context.db.insert(schema.userPermissions).values({ userId: demo.userId, permissionName: Permission.DemoRestricted });
+        const session = await login(context.app, demo.username, demo.password);
+        const headers = { authorization: `Bearer ${session.accessToken}` };
+
+        for (const dashboardShelfConfig of [{ syncAcrossSessions: true }, { syncAcrossSessions: false }, null]) {
+          const response = await context.app.inject({
+            method: 'PATCH',
+            url: '/api/v1/users/me/settings',
+            headers,
+            payload: { settings: { dashboardShelfConfig } },
+          });
+          expect(response.statusCode).toBe(403);
+        }
+
+        const legacySave = await context.app.inject({
+          method: 'PATCH',
+          url: '/api/v1/users/me/settings',
+          headers,
+          payload: { settings: { dashboardConfig: { readingGoal: 24 } } },
+        });
+        expect(legacySave.statusCode).toBe(200);
+        expect(legacySave.json().settings.dashboardConfig.readingGoal).toBe(24);
+        expect(legacySave.json().settings.dashboardShelfConfig).toBeUndefined();
+      },
+      15_000,
+    );
+
+    it('persists opt-in shelves across sessions, preserves them through legacy saves, and isolates accounts', async () => {
+      const owner = await createLocalUser(context.db);
+      const session = await login(context.app, owner.username, owner.password);
+      const shelfConfig = {
+        syncAcrossSessions: true,
+        shelfLayout: 'two-columns',
+        scrollers: [{ id: '1', type: 'random', label: 'Discover Something New', enabled: true, order: 1, limit: 20, rows: 2 }],
+      };
+      const save = await context.app.inject({
+        method: 'PATCH',
+        url: '/api/v1/users/me/settings',
+        headers: { authorization: `Bearer ${session.accessToken}` },
+        payload: { settings: { dashboardShelfConfig: shelfConfig } },
+      });
+      expect(save.statusCode).toBe(200);
+
+      const legacySave = await context.app.inject({
+        method: 'PATCH',
+        url: '/api/v1/users/me/settings',
+        headers: { authorization: `Bearer ${session.accessToken}` },
+        payload: { settings: { dashboardConfig: { readingGoal: 24, widgets: [], libraryIds: [1] } } },
+      });
+      expect(legacySave.statusCode).toBe(200);
+      expect(legacySave.json().settings.dashboardShelfConfig).toEqual(shelfConfig);
+
+      const fresh = await login(context.app, owner.username, owner.password);
+      const me = await context.app.inject({
+        method: 'GET',
+        url: '/api/v1/auth/me',
+        headers: { authorization: `Bearer ${fresh.accessToken}` },
+      });
+      expect(me.statusCode).toBe(200);
+      expect(me.json().settings.dashboardShelfConfig).toEqual(shelfConfig);
+      expect(me.json().settings.dashboardConfig.readingGoal).toBe(24);
+
+      const other = await createLocalUser(context.db);
+      const otherSession = await login(context.app, other.username, other.password);
+      const otherMe = await context.app.inject({
+        method: 'GET',
+        url: '/api/v1/auth/me',
+        headers: { authorization: `Bearer ${otherSession.accessToken}` },
+      });
+      expect(otherMe.statusCode).toBe(200);
+      expect(otherMe.json().settings.dashboardShelfConfig).toBeUndefined();
+
+      const optOut = await context.app.inject({
+        method: 'PATCH',
+        url: '/api/v1/users/me/settings',
+        headers: { authorization: `Bearer ${fresh.accessToken}` },
+        payload: { settings: { dashboardShelfConfig: { ...shelfConfig, syncAcrossSessions: false } } },
+      });
+      expect(optOut.statusCode).toBe(200);
+      expect(optOut.json().settings.dashboardShelfConfig.syncAcrossSessions).toBe(false);
+      expect(optOut.json().settings.dashboardConfig.readingGoal).toBe(24);
+    }, 15_000);
+  });
+
   describe('login lockout', () => {
     async function attemptLogin(username: string, password: string) {
       return context.app.inject({ method: 'POST', url: '/api/v1/auth/login', payload: { username, password } });