From 7e7277beb071e2eb1a9e436decd4fe26ef41879c Mon Sep 17 00:00:00 2001 From: jubnl Date: Wed, 12 Aug 2026 00:38:41 +0200 Subject: [PATCH] fix(vacay): enforce weekend blocking in toggleEntry service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With block_weekends on, toggling a weekend day via MCP or the plugin RPC still added the entry and burned allowance — the guard lived only in the web UI (mcp-smoke-test I-02). toggleEntry now rejects inserts and in-place conversions on blocked days with { error: 'weekend_blocked' } (the votePoll pattern); removing an existing entry stays allowed so stray weekend entries remain cleanable. The controller maps the code to a 400, the MCP tool to an isError result. --- server/src/nest/vacay/vacay.controller.ts | 6 +- server/src/nest/vacay/vacay.mcp.ts | 1 + server/src/nest/vacay/vacay.service.ts | 20 ++- server/tests/integration/vacay.test.ts | 7 +- server/tests/unit/mcp/tools-vacay.test.ts | 8 +- .../tests/unit/nest/vacay.controller.test.ts | 6 + server/tests/unit/nest/vacay.service.test.ts | 116 +++++++++++++++++- 7 files changed, 149 insertions(+), 15 deletions(-) diff --git a/server/src/nest/vacay/vacay.controller.ts b/server/src/nest/vacay/vacay.controller.ts index a5103f118..435152cd1 100644 --- a/server/src/nest/vacay/vacay.controller.ts +++ b/server/src/nest/vacay/vacay.controller.ts @@ -229,7 +229,11 @@ export class VacayController { } userId = tid; } - return this.vacay.toggleEntry(userId, planId, body.date, body.fraction, body.kind, socketId); + const result = this.vacay.toggleEntry(userId, planId, body.date, body.fraction, body.kind, socketId); + if (result.error === 'weekend_blocked') { + throw new HttpException({ error: 'Weekend days are blocked on this plan' }, 400); + } + return result; } @Post('entries/company-holiday') diff --git a/server/src/nest/vacay/vacay.mcp.ts b/server/src/nest/vacay/vacay.mcp.ts index a6c81b558..d02e5176b 100644 --- a/server/src/nest/vacay/vacay.mcp.ts +++ b/server/src/nest/vacay/vacay.mcp.ts @@ -272,6 +272,7 @@ export class VacayMcp { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const planId = this.vacay.getActivePlanId(ctx.userId); const result = this.vacay.toggleEntry(ctx.userId, planId, date, 1, 'vacation', undefined); + if (result.error) return errorResult(result.error); return ok(result); } diff --git a/server/src/nest/vacay/vacay.service.ts b/server/src/nest/vacay/vacay.service.ts index f97a283ff..5619e8e6b 100644 --- a/server/src/nest/vacay/vacay.service.ts +++ b/server/src/nest/vacay/vacay.service.ts @@ -16,6 +16,8 @@ export interface VacayPlan { school_holidays_enabled: number; company_holidays_enabled: number; carry_over_enabled: number; + weekend_days: string | null; + week_start: number | null; } export interface VacayUserYear { @@ -120,6 +122,16 @@ function normalizeKind(value: unknown): 'vacation' | 'comp' { return value === 'comp' ? 'comp' : 'vacation'; } +/** + * Same semantics as the client's isWeekend guard (holidays.ts): UTC weekday, + * weekend_days falls back to Sat/Sun when the column is NULL or empty. + */ +function isBlockedWeekend(plan: Pick, date: string): boolean { + if (!plan.block_weekends) return false; + const days = plan.weekend_days ? String(plan.weekend_days).split(',').map(Number) : [0, 6]; + return days.includes(new Date(date + 'T00:00:00Z').getUTCDay()); +} + const pad2 = (n: number) => String(n).padStart(2, '0'); /** Days a month has in every year — February caps at 28 so no boundary lands on a date a common year lacks. */ @@ -1064,9 +1076,11 @@ export class VacayService { return { entries, companyHolidays }; } - toggleEntry(userId: number, planId: number, date: string, fraction?: unknown, kind?: unknown, socketId?: string): { action: string; fraction?: number; kind?: string } { + toggleEntry(userId: number, planId: number, date: string, fraction?: unknown, kind?: unknown, socketId?: string): { action?: string; fraction?: number; kind?: string; error?: string } { const frac = normalizeFraction(fraction); const knd = normalizeKind(kind); + const plan = this.db.get('SELECT * FROM vacay_plans WHERE id = ?', planId); + const weekendBlocked = plan ? isBlockedWeekend(plan, date) : false; const existing = this.db.get<{ id: number; fraction: number; kind: string | null }>('SELECT id, fraction, kind FROM vacay_entries WHERE user_id = ? AND date = ? AND plan_id = ?', userId, date, planId); if (existing) { // Clicking the exact same day again (same type AND same fraction) clears it; @@ -1076,10 +1090,14 @@ export class VacayService { this.notifyPlanUsers(planId, socketId); return { action: 'removed' }; } + // Removing a stray entry on a blocked day stays possible; keeping one + // there (converted in place) does not. + if (weekendBlocked) return { error: 'weekend_blocked' }; this.db.run('UPDATE vacay_entries SET fraction = ?, kind = ? WHERE id = ?', frac, knd, existing.id); this.notifyPlanUsers(planId, socketId); return { action: 'updated', fraction: frac, kind: knd }; } + if (weekendBlocked) return { error: 'weekend_blocked' }; this.db.run('INSERT INTO vacay_entries (plan_id, user_id, date, note, fraction, kind) VALUES (?, ?, ?, ?, ?, ?)', planId, userId, date, '', frac, knd); this.notifyPlanUsers(planId, socketId); return { action: 'added', fraction: frac, kind: knd }; diff --git a/server/tests/integration/vacay.test.ts b/server/tests/integration/vacay.test.ts index 8e8167472..b10450733 100644 --- a/server/tests/integration/vacay.test.ts +++ b/server/tests/integration/vacay.test.ts @@ -187,17 +187,18 @@ describe('Vacay entries', () => { expect(res.status).toBe(200); }); - it('VACAY-004 — POST /api/addons/vacay/entries/toggle on weekend is allowed (no server-side weekend blocking)', async () => { + it('VACAY-004 — POST /api/addons/vacay/entries/toggle on a weekend is rejected when block_weekends is on', async () => { const { user } = createUser(testDb); await request(app).get('/api/addons/vacay/plan').set('Cookie', authCookie(user.id)); await request(app).post('/api/addons/vacay/years').set('Cookie', authCookie(user.id)).send({ year: 2025 }); - // 2025-06-21 is a Saturday — server does not block weekends; client-side only + // 2025-06-21 is a Saturday and plans default to block_weekends = 1 (I-02) const res = await request(app) .post('/api/addons/vacay/entries/toggle') .set('Cookie', authCookie(user.id)) .send({ date: '2025-06-21', year: 2025, type: 'vacation' }); - expect(res.status).toBe(200); + expect(res.status).toBe(400); + expect(res.body).toEqual({ error: 'Weekend days are blocked on this plan' }); }); it('VACAY-006 — GET /api/addons/vacay/entries/:year returns vacation entries', async () => { diff --git a/server/tests/unit/mcp/tools-vacay.test.ts b/server/tests/unit/mcp/tools-vacay.test.ts index 7d13c3ae1..0bfc372fb 100644 --- a/server/tests/unit/mcp/tools-vacay.test.ts +++ b/server/tests/unit/mcp/tools-vacay.test.ts @@ -252,7 +252,7 @@ describe('Tool: toggle_vacay_entry', () => { it('toggles entry and returns action', async () => { const { user } = createUser(testDb); await withHarness(user.id, async (h) => { - const result = await h.client.callTool({ name: 'toggle_vacay_entry', arguments: { date: '2025-06-15' } }); + const result = await h.client.callTool({ name: 'toggle_vacay_entry', arguments: { date: '2025-06-16' } }); const data = parseToolResult(result) as any; expect(data.action).toBeDefined(); }); @@ -262,7 +262,7 @@ describe('Tool: toggle_vacay_entry', () => { process.env.DEMO_MODE = 'true'; const { user } = createUser(testDb, { email: 'demo@nomad.app' }); await withHarness(user.id, async (h) => { - const result = await h.client.callTool({ name: 'toggle_vacay_entry', arguments: { date: '2025-06-15' } }); + const result = await h.client.callTool({ name: 'toggle_vacay_entry', arguments: { date: '2025-06-16' } }); expect(result.isError).toBe(true); }); }); @@ -561,7 +561,7 @@ describe('Tool: get_shared_vacay_calendars', () => { const { user: owner } = createUser(testDb); const { user: viewer } = createUser(testDb); await withHarness(owner.id, async (h) => { - await h.client.callTool({ name: 'toggle_vacay_entry', arguments: { date: '2025-06-15' } }); + await h.client.callTool({ name: 'toggle_vacay_entry', arguments: { date: '2025-06-16' } }); await h.client.callTool({ name: 'share_vacay_calendar', arguments: { targetUserId: viewer.id } }); }); await withHarness(viewer.id, async (h) => { @@ -570,7 +570,7 @@ describe('Tool: get_shared_vacay_calendars', () => { expect(Array.isArray(data.calendars)).toBe(true); expect(data.calendars).toHaveLength(1); expect(data.calendars[0].owner_id).toBe(owner.id); - expect(data.calendars[0].entries.map((e: any) => e.date)).toContain('2025-06-15'); + expect(data.calendars[0].entries.map((e: any) => e.date)).toContain('2025-06-16'); }); }); }); diff --git a/server/tests/unit/nest/vacay.controller.test.ts b/server/tests/unit/nest/vacay.controller.test.ts index e366ed205..dcd58399c 100644 --- a/server/tests/unit/nest/vacay.controller.test.ts +++ b/server/tests/unit/nest/vacay.controller.test.ts @@ -144,6 +144,12 @@ describe('VacayController (parity with the legacy /api/addons/vacay route)', () expect(toggleEntry).toHaveBeenCalledWith(1, 10, '2026-07-01', 0.5, undefined, 'sock'); }); + it('400 with the bespoke message when the service blocks a weekend day (I-02)', () => { + const toggleEntry = vi.fn().mockReturnValue({ error: 'weekend_blocked' }); + return thrown(() => makeController({ ...planBase, toggleEntry }).toggleEntry(user, { date: '2026-07-04' })).then((r) => + expect(r).toEqual({ status: 400, body: { error: 'Weekend days are blocked on this plan' } })); + }); + it('forwards the comp/flex leave type (#1074)', () => { const toggleEntry = vi.fn().mockReturnValue({ action: 'added', kind: 'comp' }); makeController({ ...planBase, toggleEntry }).toggleEntry(user, { date: '2026-07-01', kind: 'comp' }, 'sock'); diff --git a/server/tests/unit/nest/vacay.service.test.ts b/server/tests/unit/nest/vacay.service.test.ts index 7bcb2f147..9b963c2f7 100644 --- a/server/tests/unit/nest/vacay.service.test.ts +++ b/server/tests/unit/nest/vacay.service.test.ts @@ -83,6 +83,14 @@ function setupUserWithPlan() { return { user, plan }; } +/** + * Lift the (default-on) weekend blocking for tests whose dates are derived + * from the current year and can land on any weekday. + */ +function allowWeekends(planId: number) { + testDb.prepare('UPDATE vacay_plans SET block_weekends = 0 WHERE id = ?').run(planId); +} + // ── getOwnPlan ──────────────────────────────────────────────────────────────── describe('getOwnPlan', () => { @@ -508,13 +516,13 @@ describe('toggleEntry', () => { it('VACAY-SVC-033: removes the entry on second call (action: removed)', () => { const { user, plan } = setupUserWithPlan(); - svc.toggleEntry(user.id, plan.id, '2025-08-02', undefined); - const result = svc.toggleEntry(user.id, plan.id, '2025-08-02', undefined); + svc.toggleEntry(user.id, plan.id, '2025-08-04', undefined); + const result = svc.toggleEntry(user.id, plan.id, '2025-08-04', undefined); expect(result.action).toBe('removed'); const row = testDb .prepare('SELECT * FROM vacay_entries WHERE user_id = ? AND plan_id = ? AND date = ?') - .get(user.id, plan.id, '2025-08-02'); + .get(user.id, plan.id, '2025-08-04'); expect(row).toBeUndefined(); }); @@ -555,6 +563,81 @@ describe('toggleEntry', () => { .get(user.id, plan.id, '2025-08-07'); expect(row).toBeUndefined(); }); + + // Weekend blocking (I-02): plans default to block_weekends = 1 / weekend_days '0,6'. + it('VACAY-SVC-033d: rejects a blocked weekend day with error weekend_blocked (I-02)', () => { + const { user, plan } = setupUserWithPlan(); + + const result = svc.toggleEntry(user.id, plan.id, '2025-07-19', undefined); // Saturday + + expect(result).toEqual({ error: 'weekend_blocked' }); + const row = testDb + .prepare('SELECT id FROM vacay_entries WHERE user_id = ? AND plan_id = ? AND date = ?') + .get(user.id, plan.id, '2025-07-19'); + expect(row).toBeUndefined(); + }); + + it('VACAY-SVC-033e: accepts a non-weekend day on a blocking plan', () => { + const { user, plan } = setupUserWithPlan(); + + const result = svc.toggleEntry(user.id, plan.id, '2025-07-16', undefined); // Wednesday + + expect(result).toMatchObject({ action: 'added' }); + }); + + it('VACAY-SVC-033f: accepts a weekend day when block_weekends is off', () => { + const { user, plan } = setupUserWithPlan(); + testDb.prepare('UPDATE vacay_plans SET block_weekends = 0 WHERE id = ?').run(plan.id); + + const result = svc.toggleEntry(user.id, plan.id, '2025-07-19', undefined); // Saturday + + expect(result).toMatchObject({ action: 'added' }); + }); + + it('VACAY-SVC-033g: honours custom weekend_days (5,6 blocks Friday, frees Sunday)', () => { + const { user, plan } = setupUserWithPlan(); + testDb.prepare("UPDATE vacay_plans SET weekend_days = '5,6' WHERE id = ?").run(plan.id); + + expect(svc.toggleEntry(user.id, plan.id, '2025-07-18', undefined)).toEqual({ error: 'weekend_blocked' }); // Friday + expect(svc.toggleEntry(user.id, plan.id, '2025-07-20', undefined)).toMatchObject({ action: 'added' }); // Sunday + }); + + it('VACAY-SVC-033h: a NULL weekend_days column falls back to Sat/Sun', () => { + const { user, plan } = setupUserWithPlan(); + testDb.prepare('UPDATE vacay_plans SET weekend_days = NULL WHERE id = ?').run(plan.id); + + expect(svc.toggleEntry(user.id, plan.id, '2025-07-19', undefined)).toEqual({ error: 'weekend_blocked' }); // Saturday + }); + + it('VACAY-SVC-033i: still removes an existing entry on a blocked day (stray-data cleanup)', () => { + const { user, plan } = setupUserWithPlan(); + testDb + .prepare('INSERT INTO vacay_entries (plan_id, user_id, date, note, fraction, kind) VALUES (?, ?, ?, ?, ?, ?)') + .run(plan.id, user.id, '2025-07-19', '', 1, 'vacation'); + + const result = svc.toggleEntry(user.id, plan.id, '2025-07-19', 1, 'vacation'); + + expect(result.action).toBe('removed'); + const row = testDb + .prepare('SELECT id FROM vacay_entries WHERE user_id = ? AND plan_id = ? AND date = ?') + .get(user.id, plan.id, '2025-07-19'); + expect(row).toBeUndefined(); + }); + + it('VACAY-SVC-033j: refuses to convert an existing entry in place on a blocked day', () => { + const { user, plan } = setupUserWithPlan(); + testDb + .prepare('INSERT INTO vacay_entries (plan_id, user_id, date, note, fraction, kind) VALUES (?, ?, ?, ?, ?, ?)') + .run(plan.id, user.id, '2025-07-19', '', 0.5, 'vacation'); + + const result = svc.toggleEntry(user.id, plan.id, '2025-07-19', 1, 'vacation'); + + expect(result).toEqual({ error: 'weekend_blocked' }); + const row = testDb + .prepare('SELECT fraction FROM vacay_entries WHERE user_id = ? AND plan_id = ? AND date = ?') + .get(user.id, plan.id, '2025-07-19') as { fraction: number }; + expect(row.fraction).toBe(0.5); + }); }); // ── toggleCompanyHoliday ────────────────────────────────────────────────────── @@ -721,6 +804,7 @@ describe('getStats', () => { const { user, plan } = setupUserWithPlan(); const yr = new Date().getFullYear(); + allowWeekends(plan.id); svc.toggleEntry(user.id, plan.id, `${yr}-09-10`, undefined); svc.toggleEntry(user.id, plan.id, `${yr}-09-11`, undefined); @@ -734,6 +818,7 @@ describe('getStats', () => { const { user, plan } = setupUserWithPlan(); const yr = new Date().getFullYear(); + allowWeekends(plan.id); svc.toggleEntry(user.id, plan.id, `${yr}-09-12`, 1); // full day svc.toggleEntry(user.id, plan.id, `${yr}-09-13`, 0.5); // half day @@ -747,6 +832,7 @@ describe('getStats', () => { const { user, plan } = setupUserWithPlan(); const yr = new Date().getFullYear(); + allowWeekends(plan.id); svc.toggleEntry(user.id, plan.id, `${yr}-09-14`, 1, 'vacation'); svc.toggleEntry(user.id, plan.id, `${yr}-09-15`, 1, 'comp'); @@ -760,6 +846,7 @@ describe('getStats', () => { const { user, plan } = setupUserWithPlan(); const yr = new Date().getFullYear(); + allowWeekends(plan.id); svc.toggleEntry(user.id, plan.id, `${yr}-09-16`, 0.5, 'comp'); const stats = svc.getStats(plan.id, yr); @@ -772,6 +859,7 @@ describe('getStats', () => { const { user, plan } = setupUserWithPlan(); const yr = new Date().getFullYear(); + allowWeekends(plan.id); svc.toggleEntry(user.id, plan.id, `${yr}-09-17`, 1, 'comp'); svc.toggleEntry(user.id, plan.id, `${yr}-09-18`, 0.5, 'comp'); svc.toggleEntry(user.id, plan.id, `${yr}-09-19`, 1, 'vacation'); @@ -786,6 +874,7 @@ describe('getStats', () => { const { user, plan } = setupUserWithPlan(); const yr = new Date().getFullYear(); + allowWeekends(plan.id); svc.toggleEntry(user.id, plan.id, `${yr}-09-20`, 1, 'vacation'); expect(svc.getStats(plan.id, yr)[0].used).toBe(1); @@ -795,6 +884,20 @@ describe('getStats', () => { expect(svc.getStats(plan.id, yr)[0].used).toBe(0); }); + it('VACAY-SVC-045g: a rejected weekend toggle leaves used/remaining untouched (I-02)', () => { + const { user, plan } = setupUserWithPlan(); + svc.toggleEntry(user.id, plan.id, '2025-07-16', 1, 'vacation'); // Wednesday + const before = svc.getStats(plan.id, 2025)[0]; + expect(before.used).toBe(1); + + const result = svc.toggleEntry(user.id, plan.id, '2025-07-19', 1, 'vacation'); // Saturday + + expect(result).toEqual({ error: 'weekend_blocked' }); + const after = svc.getStats(plan.id, 2025)[0]; + expect(after.used).toBe(before.used); + expect(after.remaining).toBe(before.remaining); + }); + it('VACAY-SVC-045f: getStats reports the window it counted over (#737)', () => { const { user, plan } = setupUserWithPlan(); svc.updateYearSettings(user.id, { year_type: 'fiscal', year_start_month: 7, year_start_day: 1 }); @@ -970,8 +1073,8 @@ describe('usage over a shifted window (#737)', () => { `).run(user.id, plan.id); // Two days inside the 2030 period (Jul 2030 – Jun 2031), one of them in 2031. - svc.toggleEntry(user.id, plan.id, '2030-09-01', 1, 'vacation'); - svc.toggleEntry(user.id, plan.id, '2031-02-01', 1, 'vacation'); + svc.toggleEntry(user.id, plan.id, '2030-09-02', 1, 'vacation'); + svc.toggleEntry(user.id, plan.id, '2031-02-03', 1, 'vacation'); svc.addYear(plan.id, 2031, undefined); @@ -990,7 +1093,7 @@ describe('usage over a shifted window (#737)', () => { VALUES (?, ?, 2030, 10, 0) `).run(user.id, plan.id); - svc.toggleEntry(user.id, plan.id, '2031-02-01', 1, 'comp'); + svc.toggleEntry(user.id, plan.id, '2031-02-03', 1, 'comp'); svc.addYear(plan.id, 2031, undefined); @@ -1393,6 +1496,7 @@ describe('quirk fixes', () => { it('VACAY-SVC-069: deleteYear is atomic — a failure mid-flow keeps the year and its entries', () => { const { user, plan } = setupUserWithPlan(); const year = new Date().getFullYear(); + allowWeekends(plan.id); svc.toggleEntry(user.id, plan.id, `${year}-03-03`, 1); const broken = failingService('DELETE FROM vacay_user_years');