From bbd7d286d92f2afe807b7be05506006e3e5f8657 Mon Sep 17 00:00:00 2001 From: Gregor Vostrak Date: Tue, 28 Jul 2026 15:11:03 +0200 Subject: [PATCH] insert breaks into work entries instead of carving them out; rollback on failure system --- e2e/timesheet.spec.ts | 271 +++++++--- .../Timesheet/BreakPlacementModal.vue | 15 +- resources/js/Pages/Timesheet.vue | 8 +- .../timesheet/breakPlacementMath.test.ts | 278 ++++++++++- .../js/utils/timesheet/breakPlacementMath.ts | 314 +++++++++--- .../utils/timesheet/useBreakPlacement.test.ts | 427 +++++++++++++++- .../js/utils/timesheet/useBreakPlacement.ts | 468 +++++++++++------- .../useTimesheetCellMutations.test.ts | 19 + .../timesheet/useTimesheetCellMutations.ts | 52 +- 9 files changed, 1483 insertions(+), 369 deletions(-) diff --git a/e2e/timesheet.spec.ts b/e2e/timesheet.spec.ts index f9c29c86..5d50aef1 100644 --- a/e2e/timesheet.spec.ts +++ b/e2e/timesheet.spec.ts @@ -9,6 +9,7 @@ import { createTimeEntryWithTimestampsViaApi, getTimeEntriesViaApi, updateOrganizationSettingViaApi, + type TestContext, } from './utils/api'; // ────────────────────────────────────────────────── @@ -65,6 +66,34 @@ function addRowButton(page: Page) { return page.getByRole('button', { name: /Add row/i }).first(); } +async function fillBreakCell(page: Page, hours: string, dayIndex = 0) { + const input = page + .locator('[data-testid="timesheet_row"]') + .filter({ has: page.getByText('Break', { exact: true }) }) + .locator('[data-testid="timesheet_cell"]') + .nth(dayIndex) + .locator('input'); + await input.click(); + await input.fill(hours); + return input; +} + +function waitForBreakCreated(page: Page) { + return page.waitForResponse( + async (resp) => + resp.url().includes('/time-entries') && + resp.request().method() === 'POST' && + resp.status() === 201 && + (await resp.json()).data.type === 'break' + ); +} + +async function getDayEntriesViaApi(ctx: TestContext, day: string) { + return (await getTimeEntriesViaApi(ctx)) + .filter((e) => e.start.startsWith(day)) + .sort((a, b) => a.start.localeCompare(b.start)); +} + async function chooseRowIdentity(page: Page, optionName: string) { await addRowButton(page).click(); @@ -665,37 +694,128 @@ test('test that adding a timesheet break to a full day splits the work entry via await expect(page.getByTestId('timesheet_view')).toBeVisible(); // The break row is always present — enter a 30m break on Monday - const breakRow = page - .locator('[data-testid="timesheet_row"]') - .filter({ has: page.getByText('Break', { exact: true }) }); - const breakCell = breakRow.locator('[data-testid="timesheet_cell"]').nth(0).locator('input'); - await breakCell.click(); - await breakCell.fill('0.5'); + const breakCell = await fillBreakCell(page, '0.5'); await breakCell.press('Enter'); // The placement modal opens with the split preview await expect(page.getByTestId('break_placement_summary')).toBeVisible(); await Promise.all([ - page.waitForResponse( - async (resp) => - resp.url().includes('/time-entries') && - resp.request().method() === 'POST' && - resp.status() === 201 && - (await resp.json()).data.type === 'break' - ), + waitForBreakCreated(page), page.getByRole('button', { name: 'Add break' }).click(), ]); - // The day now has two work halves and one break, none overlapping - const entries = await getTimeEntriesViaApi(ctx); - const dayEntries = entries - .filter((e) => e.start.startsWith(day)) - .sort((a, b) => a.start.localeCompare(b.start)); - expect(dayEntries).toHaveLength(3); - expect(dayEntries.map((e) => e.type)).toEqual(['work', 'break', 'work']); - // The break sits flush between the two halves - expect(dayEntries[0].end).toBe(dayEntries[1].start); - expect(dayEntries[1].end).toBe(dayEntries[2].start); + // The break is inserted without reducing the eight hours of work. + const dayEntries = await getDayEntriesViaApi(ctx, day); + expect(dayEntries.map((e) => [e.type, e.start, e.end])).toEqual([ + ['work', `${day}T09:00:00Z`, `${day}T13:00:00Z`], + ['break', `${day}T13:00:00Z`, `${day}T13:30:00Z`], + ['work', `${day}T13:30:00Z`, `${day}T17:30:00Z`], + ]); +}); + +test('test that inserting a break as long as the work keeps the tracked work time', async ({ + page, + ctx, +}) => { + // A one-hour break splits but does not reduce the one-hour work entry. + await updateOrganizationSettingViaApi(ctx, { breaks_enabled: true }); + const day = getCurrentWeekMonday().toISOString().slice(0, 10); + await createTimeEntryWithTimestampsViaApi(ctx, { + start: `${day}T09:00:00Z`, + end: `${day}T10:00:00Z`, + description: 'One hour', + }); + + await goToTimesheet(page); + await expect(page.getByTestId('timesheet_view')).toBeVisible(); + + const breakCell = await fillBreakCell(page, '1'); + await breakCell.press('Enter'); + + await expect(page.getByTestId('break_placement_summary')).toBeVisible(); + await expect(page.getByTestId('break_placement_infeasible')).not.toBeVisible(); + await Promise.all([ + waitForBreakCreated(page), + page.getByRole('button', { name: 'Add break' }).click(), + ]); + + const dayEntries = await getDayEntriesViaApi(ctx, day); + expect(dayEntries.map((e) => [e.type, e.start, e.end])).toEqual([ + ['work', `${day}T09:00:00Z`, `${day}T09:30:00Z`], + ['break', `${day}T09:30:00Z`, `${day}T10:30:00Z`], + ['work', `${day}T10:30:00Z`, `${day}T11:00:00Z`], + ]); + const workRow = page + .locator('[data-testid="timesheet_row"]') + .filter({ hasNot: page.getByText('Break', { exact: true }) }) + .first(); + await expect( + workRow.locator('[data-testid="timesheet_cell"]').nth(0).locator('input') + ).toHaveValue('1h 00min'); +}); + +test('test that a late work entry starts earlier so the break still fits the day', async ({ + page, + ctx, +}) => { + // Move the block earlier rather than extending it into the next day. + await updateOrganizationSettingViaApi(ctx, { breaks_enabled: true }); + const day = getCurrentWeekMonday().toISOString().slice(0, 10); + await createTimeEntryWithTimestampsViaApi(ctx, { + start: `${day}T22:00:00Z`, + end: `${day}T23:30:00Z`, + description: 'Late shift', + }); + + await goToTimesheet(page); + await expect(page.getByTestId('timesheet_view')).toBeVisible(); + + const breakCell = await fillBreakCell(page, '1'); + await breakCell.press('Enter'); + + await expect(page.getByTestId('break_placement_summary')).toBeVisible(); + await expect(page.getByTestId('break_placement_infeasible')).not.toBeVisible(); + await Promise.all([ + waitForBreakCreated(page), + page.getByRole('button', { name: 'Add break' }).click(), + ]); + + const dayEntries = await getDayEntriesViaApi(ctx, day); + const nextDay = new Date(`${day}T00:00:00Z`); + nextDay.setUTCDate(nextDay.getUTCDate() + 1); + const midnight = `${nextDay.toISOString().slice(0, 10)}T00:00:00Z`; + expect(dayEntries.map((e) => [e.type, e.start, e.end])).toEqual([ + ['work', `${day}T21:30:00Z`, `${day}T22:15:00Z`], + ['break', `${day}T22:15:00Z`, `${day}T23:15:00Z`], + ['work', `${day}T23:15:00Z`, midnight], + ]); +}); + +test('test that a work entry too short to split gets the break placed next to it', async ({ + page, + ctx, +}) => { + // The one-minute work entry is too short to split. + await updateOrganizationSettingViaApi(ctx, { breaks_enabled: true }); + const day = getCurrentWeekMonday().toISOString().slice(0, 10); + await createTimeEntryWithTimestampsViaApi(ctx, { + start: `${day}T09:00:00Z`, + end: `${day}T09:01:00Z`, + description: 'Quick note', + }); + + await goToTimesheet(page); + await expect(page.getByTestId('timesheet_view')).toBeVisible(); + + const breakCell = await fillBreakCell(page, '0.5'); + await Promise.all([waitForBreakCreated(page), breakCell.press('Enter')]); + + await expect(page.getByTestId('break_placement_summary')).not.toBeVisible(); + const dayEntries = await getDayEntriesViaApi(ctx, day); + expect(dayEntries.map((e) => [e.type, e.start, e.end])).toEqual([ + ['work', `${day}T09:00:00Z`, `${day}T09:01:00Z`], + ['break', `${day}T09:01:00Z`, `${day}T09:31:00Z`], + ]); }); test('test that adding a break into an oversized gap places it without moving other entries', async ({ @@ -722,28 +842,11 @@ test('test that adding a break into an oversized gap places it without moving ot await goToTimesheet(page); await expect(page.getByTestId('timesheet_view')).toBeVisible(); - const breakRow = page - .locator('[data-testid="timesheet_row"]') - .filter({ has: page.getByText('Break', { exact: true }) }); - const breakCell = breakRow.locator('[data-testid="timesheet_cell"]').nth(0).locator('input'); - await breakCell.click(); - await breakCell.fill('0.5'); - await Promise.all([ - page.waitForResponse( - async (resp) => - resp.url().includes('/time-entries') && - resp.request().method() === 'POST' && - resp.status() === 201 && - (await resp.json()).data.type === 'break' - ), - breakCell.press('Enter'), - ]); + const breakCell = await fillBreakCell(page, '0.5'); + await Promise.all([waitForBreakCreated(page), breakCell.press('Enter')]); await expect(page.getByTestId('break_placement_summary')).not.toBeVisible(); - const entries = await getTimeEntriesViaApi(ctx); - const dayEntries = entries - .filter((e) => e.start.startsWith(day)) - .sort((a, b) => a.start.localeCompare(b.start)); + const dayEntries = await getDayEntriesViaApi(ctx, day); expect(dayEntries.map((e) => [e.type, e.start, e.end])).toEqual([ ['work', `${day}T09:00:00Z`, `${day}T12:00:00Z`], ['break', `${day}T12:00:00Z`, `${day}T12:30:00Z`], @@ -775,12 +878,7 @@ test('test that the placement modal warns when the chosen time would leave the b await goToTimesheet(page); await expect(page.getByTestId('timesheet_view')).toBeVisible(); - const breakRow = page - .locator('[data-testid="timesheet_row"]') - .filter({ has: page.getByText('Break', { exact: true }) }); - const breakCell = breakRow.locator('[data-testid="timesheet_cell"]').nth(0).locator('input'); - await breakCell.click(); - await breakCell.fill('0.5'); + const breakCell = await fillBreakCell(page, '0.5'); await breakCell.press('Enter'); // Default suggestion sits flush between work → no warning @@ -804,20 +902,11 @@ test('test that the placement modal warns when the chosen time would leave the b // The warning is non-blocking: the break can still be added as chosen await Promise.all([ - page.waitForResponse( - async (resp) => - resp.url().includes('/time-entries') && - resp.request().method() === 'POST' && - resp.status() === 201 && - (await resp.json()).data.type === 'break' - ), + waitForBreakCreated(page), page.getByRole('button', { name: 'Add break' }).click(), ]); - const entries = await getTimeEntriesViaApi(ctx); - const dayEntries = entries - .filter((e) => e.start.startsWith(day)) - .sort((a, b) => a.start.localeCompare(b.start)); + const dayEntries = await getDayEntriesViaApi(ctx, day); expect(dayEntries.map((e) => [e.type, e.start, e.end])).toEqual([ ['break', `${day}T07:00:00Z`, `${day}T07:30:00Z`], ['work', `${day}T09:00:00Z`, `${day}T12:00:00Z`], @@ -894,12 +983,7 @@ test('test that editing a timesheet break re-places it as one entry instead of f await goToTimesheet(page); await expect(page.getByTestId('timesheet_view')).toBeVisible(); - const breakRow = page - .locator('[data-testid="timesheet_row"]') - .filter({ has: page.getByText('Break', { exact: true }) }); - const breakCell = breakRow.locator('[data-testid="timesheet_cell"]').nth(0).locator('input'); - await breakCell.click(); - await breakCell.fill('0.75'); // 45 minutes — still fits the 1h gap, so it stays anchored + const breakCell = await fillBreakCell(page, '0.75'); await Promise.all([ // A break that still fits its gap is re-placed in place (PUT on the same entry), // not deleted and recreated — that's what keeps it a single entry. @@ -915,10 +999,61 @@ test('test that editing a timesheet break re-places it as one entry instead of f // Still exactly one break on the day (not fragmented). It stays anchored at its current // start (12:15) rather than re-centering, growing its end to 13:00 to reach 45 minutes. - const after = await getTimeEntriesViaApi(ctx); - const breaks = after.filter((e) => e.start.startsWith(day) && e.type === 'break'); + const breaks = (await getDayEntriesViaApi(ctx, day)).filter((e) => e.type === 'break'); expect(breaks).toHaveLength(1); expect(breaks[0].duration).toBe(2700); expect(breaks[0].start).toBe(`${day}T12:15:00Z`); expect(breaks[0].end).toBe(`${day}T13:00:00Z`); }); + +test('test that editing an adjacent break vacates its old slot before extending work', async ({ + page, + ctx, +}) => { + // The existing break must move before work can extend through its old slot. + await updateOrganizationSettingViaApi(ctx, { + breaks_enabled: true, + prevent_overlapping_time_entries: true, + }); + const day = getCurrentWeekMonday().toISOString().slice(0, 10); + await createTimeEntryWithTimestampsViaApi(ctx, { + start: `${day}T09:00:00Z`, + end: `${day}T17:00:00Z`, + description: 'Work before break', + }); + const breakEntry = await createTimeEntryWithTimestampsViaApi(ctx, { + start: `${day}T17:00:00Z`, + end: `${day}T17:30:00Z`, + type: 'break', + }); + + await goToTimesheet(page); + await expect(page.getByTestId('timesheet_view')).toBeVisible(); + const breakCell = await fillBreakCell(page, '1'); + await breakCell.press('Enter'); + + await expect(page.getByTestId('break_placement_summary')).toBeVisible(); + await Promise.all([ + page.waitForResponse( + (resp) => + resp.url().includes(`/time-entries/${breakEntry.id}`) && + resp.request().method() === 'PUT' && + resp.status() === 200 + ), + page.waitForResponse( + async (resp) => + resp.url().includes('/time-entries') && + resp.request().method() === 'POST' && + resp.status() === 201 && + (await resp.json()).data.type === 'work' + ), + page.getByRole('button', { name: 'Add break' }).click(), + ]); + + const entries = await getDayEntriesViaApi(ctx, day); + expect(entries.map((entry) => [entry.id, entry.type, entry.start, entry.end])).toEqual([ + [expect.any(String), 'work', `${day}T09:00:00Z`, `${day}T13:00:00Z`], + [breakEntry.id, 'break', `${day}T13:00:00Z`, `${day}T14:00:00Z`], + [expect.any(String), 'work', `${day}T14:00:00Z`, `${day}T18:00:00Z`], + ]); +}); diff --git a/resources/js/Components/Timesheet/BreakPlacementModal.vue b/resources/js/Components/Timesheet/BreakPlacementModal.vue index 25b00a4a..345b6988 100644 --- a/resources/js/Components/Timesheet/BreakPlacementModal.vue +++ b/resources/js/Components/Timesheet/BreakPlacementModal.vue @@ -54,7 +54,11 @@ const durationSeconds = computed(() => const splitPlan = computed(() => { if (!props.request || mode.value !== 'split' || durationSeconds.value <= 0) return null; - return planSplitEntry(props.request.workEntries[0]!, durationSeconds.value, utcStart.value); + return planSplitEntry(props.request.workEntries[0]!, durationSeconds.value, utcStart.value, { + dayStart: props.request.dayStart, + dayEnd: props.request.dayEnd, + otherEntries: props.request.otherEntries, + }); }); const movePlan = computed(() => { @@ -116,11 +120,10 @@ function fmt(iso: string): string { const explanation = computed(() => { if (!props.request) return ''; return mode.value === 'split' - ? "There's no free gap that fits this break, so the work entry will be split and the break placed inside it." + ? "There's no free gap that fits this break, so the work entry will be split around it. The work moves to make room and keeps its full length." : "There's no free gap that fits this break, so the surrounding entries will be shifted to make room."; }); -// Human-readable summary of what will change, so the user can confirm the edit. const changeSummary = computed(() => { if (mode.value === 'split') { const plan = splitPlan.value; @@ -129,6 +132,10 @@ const changeSummary = computed(() => { `${fmt(plan.firstHalf.start)}–${fmt(plan.firstHalf.end)} (work)`, `${fmt(plan.breakSlot.start)}–${fmt(plan.breakSlot.end)} (break)`, `${fmt(plan.secondHalf.start)}–${fmt(plan.secondHalf.end)} (work)`, + ...plan.shifted.map((shift) => { + const original = props.request!.otherEntries.find((e) => e.id === shift.id)!; + return `${fmt(original.start)}–${fmt(original.end)} → ${fmt(shift.start)}–${fmt(shift.end)} (break)`; + }), ]; } const plan = movePlan.value; @@ -202,7 +209,7 @@ async function submit() { class="rounded-lg border border-red-500/30 bg-red-500/10 px-3 py-2 text-sm text-red-600 dark:text-red-400"> {{ mode === 'split' - ? "This break doesn't fit there — it must lie inside the work entry, leaving at least a minute of work on each side." + ? "This break doesn't fit there. It has to sit inside the work, leaving at least a minute of work on each side, and the work around it has to stay inside the day." : "This break doesn't fit at that time without pushing an entry outside the day. Try a shorter break or a different time." }} diff --git a/resources/js/Pages/Timesheet.vue b/resources/js/Pages/Timesheet.vue index ad0c3a53..7fdfcf55 100644 --- a/resources/js/Pages/Timesheet.vue +++ b/resources/js/Pages/Timesheet.vue @@ -125,7 +125,13 @@ const { breakPlacementRequest, applyBreakPlacement, dismissBreakPlacement, -} = useTimesheetCellMutations(weekDays, allTimeEntries, rows, removeSlot); +} = useTimesheetCellMutations( + weekDays, + allTimeEntries, + rows, + removeSlot, + () => organization.value?.prevent_overlapping_time_entries ?? false +); // Local dates (YYYY-MM-DD) that have a misplaced break. There is only one break // row, so a flat set is enough — its cells show a warning for dates in the set. diff --git a/resources/js/utils/timesheet/breakPlacementMath.test.ts b/resources/js/utils/timesheet/breakPlacementMath.test.ts index 456a7412..cc78ac96 100644 --- a/resources/js/utils/timesheet/breakPlacementMath.test.ts +++ b/resources/js/utils/timesheet/breakPlacementMath.test.ts @@ -4,6 +4,9 @@ import { describe, expect, it } from 'vitest'; import { BREAK_GAP_TOLERANCE_SECONDS, buildDayPlacementContext, + decideBreakPlacement, + findAdjacentBreakSlot, + findBreakSlotNearInDay, findValidBreakGap, findValidBreakGapNear, planMoveInsert, @@ -19,6 +22,7 @@ const HOUR = 3600; const DAY = '2026-07-14'; const dayStart = `${DAY}T00:00:00Z`; const dayEnd = `${DAY}T24:00:00Z`; +const MIDNIGHT = '2026-07-15T00:00:00Z'; function iv(startH: number, endH: number) { const h = (n: number) => { @@ -30,6 +34,71 @@ function iv(startH: number, endH: number) { return { start: h(startH), end: h(endH) }; } +describe('decideBreakPlacement', () => { + const context = (work: MovableInterval[] = []) => ({ + work, + breaks: [], + blocked: [], + dayStart, + dayEnd: MIDNIGHT, + }); + const movable = (id: string, startH: number, endH: number): MovableInterval => ({ + id, + ...iv(startH, endH), + }); + + it('returns a direct save when an existing gap fits', () => { + const decision = decideBreakPlacement({ + date: DAY, + durationSeconds: HOUR, + context: context([movable('morning', 9, 12), movable('afternoon', 13, 17)]), + }); + + expect(decision).toEqual({ + kind: 'save', + slot: { start: `${DAY}T12:00:00Z`, end: `${DAY}T13:00:00Z` }, + }); + }); + + it('delegates an empty day to the default cell placement', () => { + expect( + decideBreakPlacement({ + date: DAY, + durationSeconds: HOUR, + context: context(), + }) + ).toEqual({ kind: 'place-in-free-window' }); + }); + + it('returns a modal request when work must be rearranged', () => { + const decision = decideBreakPlacement({ + date: DAY, + durationSeconds: HOUR, + context: context([movable('work', 9, 17)]), + }); + + expect(decision).toEqual( + expect.objectContaining({ + kind: 'needs-input', + request: expect.objectContaining({ + date: DAY, + defaultBreakStart: `${DAY}T13:00:00Z`, + }), + }) + ); + }); + + it('rejects a day that cannot fit or rearrange the break', () => { + expect( + decideBreakPlacement({ + date: DAY, + durationSeconds: HOUR, + context: context([movable('all-day', 0, 24)]), + }) + ).toEqual({ kind: 'reject' }); + }); +}); + describe('findValidBreakGap', () => { it('centers the break in a gap that fits within tolerance', () => { // 09-12 and 13-17 → 1h gap, 30m break → centered at 12:15-12:45 @@ -106,53 +175,230 @@ describe('findValidBreakGap', () => { }); describe('planSplitEntry', () => { - it('splits a single entry and centers the break', () => { + const workSeconds = (plan: NonNullable>) => + dayjs.utc(plan.firstHalf.end).diff(dayjs.utc(plan.firstHalf.start), 'second') + + dayjs.utc(plan.secondHalf.end).diff(dayjs.utc(plan.secondHalf.start), 'second'); + + it('splits a single entry in the middle and keeps the work length', () => { const plan = planSplitEntry(iv(9, 17), HALF_HOUR); expect(plan).not.toBeNull(); expect(plan!.firstHalf.start).toBe(`${DAY}T09:00:00Z`); expect(plan!.breakSlot.start).toBe(plan!.firstHalf.end); expect(plan!.secondHalf.start).toBe(plan!.breakSlot.end); - expect(plan!.secondHalf.end).toBe(`${DAY}T17:00:00Z`); - // break is 30m and centered → 12:45-13:15 - expect(plan!.breakSlot).toEqual({ start: `${DAY}T12:45:00Z`, end: `${DAY}T13:15:00Z` }); + expect(plan!.breakSlot).toEqual({ start: `${DAY}T13:00:00Z`, end: `${DAY}T13:30:00Z` }); + expect(plan!.secondHalf.end).toBe(`${DAY}T17:30:00Z`); + expect(workSeconds(plan!)).toBe(8 * HOUR); + }); + + it('inserts a break as long as the work without shortening it', () => { + const plan = planSplitEntry(iv(9, 10), HOUR); + expect(plan).not.toBeNull(); + expect(plan!.firstHalf).toEqual({ start: `${DAY}T09:00:00Z`, end: `${DAY}T09:30:00Z` }); + expect(plan!.breakSlot).toEqual({ start: `${DAY}T09:30:00Z`, end: `${DAY}T10:30:00Z` }); + expect(plan!.secondHalf).toEqual({ start: `${DAY}T10:30:00Z`, end: `${DAY}T11:00:00Z` }); + expect(workSeconds(plan!)).toBe(HOUR); + }); + + it('accepts a break far longer than the work entry', () => { + const plan = planSplitEntry(iv(9, 9.5), 4 * HOUR); + expect(plan).not.toBeNull(); + expect(plan!.firstHalf).toEqual({ start: `${DAY}T09:00:00Z`, end: `${DAY}T09:15:00Z` }); + expect(plan!.breakSlot).toEqual({ start: `${DAY}T09:15:00Z`, end: `${DAY}T13:15:00Z` }); + expect(plan!.secondHalf).toEqual({ start: `${DAY}T13:15:00Z`, end: `${DAY}T13:30:00Z` }); + expect(workSeconds(plan!)).toBe(HALF_HOUR); }); it('honors an explicit break start', () => { const plan = planSplitEntry(iv(9, 17), HALF_HOUR, `${DAY}T10:00:00Z`); expect(plan!.firstHalf).toEqual({ start: `${DAY}T09:00:00Z`, end: `${DAY}T10:00:00Z` }); - expect(plan!.secondHalf.start).toBe(`${DAY}T10:30:00Z`); + expect(plan!.breakSlot).toEqual({ start: `${DAY}T10:00:00Z`, end: `${DAY}T10:30:00Z` }); + expect(plan!.secondHalf).toEqual({ start: `${DAY}T10:30:00Z`, end: `${DAY}T17:30:00Z` }); + expect(workSeconds(plan!)).toBe(8 * HOUR); }); it('returns null when the entry is too short to leave work on both sides', () => { - expect(planSplitEntry(iv(9, 9.25), HALF_HOUR)).toBeNull(); + expect( + planSplitEntry({ start: `${DAY}T09:00:00Z`, end: `${DAY}T09:01:30Z` }, HALF_HOUR) + ).toBeNull(); }); it('rejects an explicit break start before the entry instead of clamping it', () => { - // 07:00 lies before the 09:00-17:00 entry — relocating it silently would - // leave a hair-thin first fragment at a time the user never picked. expect(planSplitEntry(iv(9, 17), HALF_HOUR, `${DAY}T07:00:00Z`)).toBeNull(); }); - it('rejects an explicit break start whose break would reach past the entry end', () => { - expect(planSplitEntry(iv(9, 17), HALF_HOUR, `${DAY}T16:45:00Z`)).toBeNull(); + it('rejects an explicit break start at or after the end of the entry', () => { + expect(planSplitEntry(iv(9, 17), HALF_HOUR, `${DAY}T17:00:00Z`)).toBeNull(); + expect(planSplitEntry(iv(9, 17), HALF_HOUR, `${DAY}T18:00:00Z`)).toBeNull(); }); it('rejects an explicit break start that leaves less than the minimum fragment', () => { - // 09:00:30 would leave only 30s of work before the break. expect(planSplitEntry(iv(9, 17), HALF_HOUR, `${DAY}T09:00:30Z`)).toBeNull(); + expect(planSplitEntry(iv(9, 17), HALF_HOUR, `${DAY}T16:59:30Z`)).toBeNull(); }); it('accepts an explicit break start leaving exactly the minimum fragment on each side', () => { - // 09:01 leaves 60s before; on a 09:00-09:32 entry a 30m break also leaves 60s after. - const plan = planSplitEntry(iv(9, 9 + 32 / 60), HALF_HOUR, `${DAY}T09:01:00Z`); + const plan = planSplitEntry(iv(9, 9 + 2 / 60), HALF_HOUR, `${DAY}T09:01:00Z`); expect(plan).not.toBeNull(); expect(plan!.firstHalf).toEqual({ start: `${DAY}T09:00:00Z`, end: `${DAY}T09:01:00Z` }); expect(plan!.secondHalf).toEqual({ start: `${DAY}T09:31:00Z`, end: `${DAY}T09:32:00Z` }); }); - it('returns null when the entry cannot hold the break plus a minimum fragment per side', () => { - // 31 minutes of work cannot hold a 30m break with 60s of work on each side. - expect(planSplitEntry(iv(9, 9 + 31 / 60), HALF_HOUR)).toBeNull(); + it('leaves later entries alone when the extended work does not reach them', () => { + const plan = planSplitEntry(iv(9, 17), HALF_HOUR, `${DAY}T12:00:00Z`, { + otherEntries: [ + { id: 'early', ...iv(8, 8.5) }, + { id: 'later', ...iv(18, 19) }, + ], + }); + expect(plan!.secondHalf).toEqual({ start: `${DAY}T12:30:00Z`, end: `${DAY}T17:30:00Z` }); + expect(plan!.shifted).toEqual([]); + }); + + it('pushes only the collision chain and preserves later entry times', () => { + const plan = planSplitEntry(iv(9, 17), HALF_HOUR, `${DAY}T12:00:00Z`, { + otherEntries: [ + { id: 'first', ...iv(17.25, 17.5) }, + { id: 'second', ...iv(17.6, 17.9) }, + { id: 'distant', ...iv(20, 21) }, + ], + }); + + // Moving the first break also collides with the second. + expect(plan!.shifted).toEqual([ + { id: 'first', ...iv(17.5, 17.75) }, + { id: 'second', ...iv(17.75, 18.05) }, + ]); + }); + + it('starts the work earlier when pushing it later would leave the day', () => { + const plan = planSplitEntry(iv(22, 23.5), HOUR, undefined, { dayStart, dayEnd }); + expect(plan).not.toBeNull(); + expect(plan!.firstHalf).toEqual({ start: `${DAY}T21:30:00Z`, end: `${DAY}T22:15:00Z` }); + expect(plan!.breakSlot).toEqual({ start: `${DAY}T22:15:00Z`, end: `${DAY}T23:15:00Z` }); + expect(plan!.secondHalf).toEqual({ start: `${DAY}T23:15:00Z`, end: MIDNIGHT }); + expect(workSeconds(plan!)).toBe(1.5 * HOUR); + }); + + it('fits a break that is longer than the work left in the day', () => { + const plan = planSplitEntry(iv(23, 24), HOUR, undefined, { dayStart, dayEnd }); + expect(plan).not.toBeNull(); + expect(plan!.firstHalf).toEqual({ start: `${DAY}T22:00:00Z`, end: `${DAY}T22:30:00Z` }); + expect(plan!.breakSlot).toEqual({ start: `${DAY}T22:30:00Z`, end: `${DAY}T23:30:00Z` }); + expect(plan!.secondHalf).toEqual({ start: `${DAY}T23:30:00Z`, end: MIDNIGHT }); + expect(workSeconds(plan!)).toBe(HOUR); + }); + + it('reproduces its own plan when re-planned from the break start it chose', () => { + const options = { dayStart, dayEnd }; + const suggested = planSplitEntry(iv(22, 23.5), HOUR, undefined, options); + const replanned = planSplitEntry(iv(22, 23.5), HOUR, suggested!.breakSlot.start, options); + expect(replanned).toEqual(suggested); + }); + + it('returns null when the block cannot fit in the day at all', () => { + expect(planSplitEntry(iv(0, 23), 2 * HOUR, undefined, { dayStart, dayEnd })).toBeNull(); + }); + + it('returns null when starting earlier would run into an entry that stays put', () => { + expect( + planSplitEntry(iv(22, 23.75), HOUR, undefined, { + dayStart, + dayEnd, + otherEntries: [{ id: 'earlier', ...iv(21, 21.5) }], + }) + ).toBeNull(); + }); + + it('returns null when the actual collision chain would leave the day', () => { + expect( + planSplitEntry(iv(22, 23.5), HOUR, undefined, { + dayStart, + dayEnd, + otherEntries: [{ id: 'late', ...iv(23.75, 24) }], + }) + ).toBeNull(); + }); + + it('does not reject a valid split because of an unrelated late entry', () => { + const plan = planSplitEntry(iv(9, 10), HOUR, `${DAY}T09:30:00Z`, { + dayStart, + dayEnd, + otherEntries: [{ id: 'late', ...iv(23.5, 24) }], + }); + + expect(plan).not.toBeNull(); + expect(plan!.secondHalf.end).toBe(`${DAY}T11:00:00Z`); + expect(plan!.shifted).toEqual([]); + }); +}); + +describe('findBreakSlotNearInDay', () => { + const anchor = `${DAY}T23:00:00Z`; + + it('keeps the break where it is when the new duration still fits', () => { + expect(findBreakSlotNearInDay(dayStart, dayEnd, HALF_HOUR, anchor)).toEqual({ + start: anchor, + end: `${DAY}T23:30:00Z`, + }); + }); + + it('slides the break earlier instead of growing past midnight', () => { + expect(findBreakSlotNearInDay(dayStart, dayEnd, 2 * HOUR, anchor)).toEqual({ + start: `${DAY}T22:00:00Z`, + end: MIDNIGHT, + }); + }); + + it('slides only as far as the nearest free window allows', () => { + expect(findBreakSlotNearInDay(dayStart, dayEnd, HOUR, anchor, [iv(21.5, 22)])).toEqual({ + start: `${DAY}T23:00:00Z`, + end: MIDNIGHT, + }); + expect(findBreakSlotNearInDay(dayStart, dayEnd, 2 * HOUR, anchor, [iv(21.5, 22)])).toEqual({ + start: `${DAY}T22:00:00Z`, + end: MIDNIGHT, + }); + }); + + it('respects a day window shortened by a running entry', () => { + expect(findBreakSlotNearInDay(dayStart, `${DAY}T12:00:00Z`, HOUR, anchor)).toEqual({ + start: `${DAY}T11:00:00Z`, + end: `${DAY}T12:00:00Z`, + }); + }); + + it('returns null when the day has no free window big enough', () => { + expect(findBreakSlotNearInDay(dayStart, dayEnd, 20 * HOUR, anchor, [iv(6, 18)])).toBeNull(); + }); +}); + +describe('findAdjacentBreakSlot', () => { + it('parks the break flush after the last work entry', () => { + expect(findAdjacentBreakSlot([iv(9, 9.5)], dayStart, dayEnd, HALF_HOUR)).toEqual({ + start: `${DAY}T09:30:00Z`, + end: `${DAY}T10:00:00Z`, + }); + }); + + it('falls back to flush before the first work entry', () => { + expect(findAdjacentBreakSlot([iv(9, 24)], dayStart, dayEnd, HALF_HOUR)).toEqual({ + start: `${DAY}T08:30:00Z`, + end: `${DAY}T09:00:00Z`, + }); + }); + + it('skips a candidate blocked by an existing break', () => { + expect( + findAdjacentBreakSlot([iv(9, 24)], dayStart, dayEnd, HALF_HOUR, [iv(8.75, 9)]) + ).toBeNull(); + }); + + it('returns null when neither side fits inside the day', () => { + expect(findAdjacentBreakSlot([iv(0, 24)], dayStart, dayEnd, HALF_HOUR)).toBeNull(); + }); + + it('returns null without any work to sit next to', () => { + expect(findAdjacentBreakSlot([], dayStart, dayEnd, HALF_HOUR)).toBeNull(); }); }); diff --git a/resources/js/utils/timesheet/breakPlacementMath.ts b/resources/js/utils/timesheet/breakPlacementMath.ts index 5f7cf20a..b87dcb1b 100644 --- a/resources/js/utils/timesheet/breakPlacementMath.ts +++ b/resources/js/utils/timesheet/breakPlacementMath.ts @@ -40,6 +40,8 @@ export interface SplitPlan { firstHalf: Interval; breakSlot: Interval; secondHalf: Interval; + // Entries pushed later to clear the extended second half. + shifted: MovableInterval[]; } /** @@ -96,6 +98,79 @@ export interface DayPlacementContext { dayEnd: string; } +export type BreakPlacementDecision = + | { kind: 'save'; slot: Interval } + | { kind: 'place-in-free-window' } + | { kind: 'needs-input'; request: BreakPlacementRequest } + | { kind: 'reject' }; + +/** + * Decide how a requested break should be placed without performing any writes. + * Keeping this policy pure lets the composable focus on UI state and persistence. + */ +export function decideBreakPlacement({ + date, + durationSeconds, + context, + anchorStart = null, + replaceBreakId = null, +}: { + date: string; + durationSeconds: number; + context: DayPlacementContext; + anchorStart?: string | null; + replaceBreakId?: string | null; +}): BreakPlacementDecision { + const { work, breaks, blocked, dayStart, dayEnd } = context; + const obstacles = [...breaks, ...blocked]; + const gap = + (anchorStart !== null + ? findValidBreakGapNear(work, durationSeconds, anchorStart, obstacles) + : null) ?? findValidBreakGap(work, durationSeconds, obstacles); + + if (gap) return { kind: 'save', slot: gap }; + + if (work.length === 0) { + if (anchorStart === null) return { kind: 'place-in-free-window' }; + const slot = findBreakSlotNearInDay( + dayStart, + dayEnd, + durationSeconds, + anchorStart, + obstacles + ); + return slot ? { kind: 'save', slot } : { kind: 'reject' }; + } + + const defaultPlan = + work.length === 1 + ? planSplitEntry(work[0]!, durationSeconds, undefined, { + dayStart, + dayEnd, + otherEntries: breaks, + }) + : suggestMovePlan(work, dayStart, dayEnd, durationSeconds, breaks); + + if (!defaultPlan) { + const adjacent = findAdjacentBreakSlot(work, dayStart, dayEnd, durationSeconds, obstacles); + return adjacent ? { kind: 'save', slot: adjacent } : { kind: 'reject' }; + } + + return { + kind: 'needs-input', + request: { + date, + durationSeconds, + dayStart, + dayEnd, + workEntries: work, + otherEntries: breaks, + defaultBreakStart: defaultPlan.breakSlot.start, + replaceBreakId, + }, + }; +} + export function buildDayPlacementContext( entries: DayEntryLike[], dayStart: string, @@ -169,6 +244,14 @@ function toIntervalMs(interval: Interval): IntervalMs { }; } +function slotAt(startMs: number, durationMs: number): Interval { + const dayjs = getDayJsInstance(); + return { + start: dayjs.utc(startMs).format(), + end: dayjs.utc(startMs + durationMs).format(), + }; +} + /** * Merge overlapping/touching work intervals so the space between two * consecutive merged intervals is genuinely work-free. Without this, an entry @@ -218,17 +301,12 @@ export function findValidBreakGap( toleranceSeconds: number = BREAK_GAP_TOLERANCE_SECONDS ): Interval | null { if (durationSeconds <= 0) return null; - const dayjs = getDayJsInstance(); const durationMs = durationSeconds * 1000; const gaps = workFreeGapsMs(work); const obstaclesMs = obstacles.map(toIntervalMs); const blockers = (startMs: number): IntervalMs[] => obstaclesMs.filter((o) => startMs < o.endMs && o.startMs < startMs + durationMs); - const slot = (startMs: number): Interval => ({ - start: dayjs.utc(startMs).format(), - end: dayjs.utc(startMs + durationMs).format(), - }); // Pass 1: a gap where the centered break keeps both sides within tolerance. for (const gap of gaps) { @@ -236,7 +314,7 @@ export function findValidBreakGap( if (gapMs < durationMs || gapMs > durationMs + 2 * toleranceSeconds * 1000) continue; const startMs = gap.startMs + Math.floor((gapMs - durationMs) / 2000) * 1000; if (blockers(startMs).length > 0) continue; - return slot(startMs); + return slotAt(startMs, durationMs); } // Pass 2: any gap that can physically hold the break. Start flush after the @@ -245,7 +323,7 @@ export function findValidBreakGap( let startMs = gap.startMs; while (startMs + durationMs <= gap.endMs) { const blocking = blockers(startMs); - if (blocking.length === 0) return slot(startMs); + if (blocking.length === 0) return slotAt(startMs, durationMs); startMs = Math.max(...blocking.map((o) => o.endMs)); } } @@ -274,98 +352,198 @@ export function findValidBreakGapNear( const dayjs = getDayJsInstance(); const durationMs = durationSeconds * 1000; const anchorMs = dayjs.utc(anchorStart).valueOf(); + const obstaclesMs = obstacles.map(toIntervalMs); for (const gap of workFreeGapsMs(work)) { // The anchor must fall inside this gap for it to be "where the break is". if (anchorMs < gap.startMs || anchorMs >= gap.endMs) continue; - if (gap.endMs - gap.startMs < durationMs) return null; - - // Walk the gap's free windows around obstacles and pick the start - // closest to the anchor, so the break moves as little as possible - // from where the user left it. - const blockers = obstacles - .map(toIntervalMs) - .filter((o) => o.startMs < gap.endMs && o.endMs > gap.startMs) - .sort((a, b) => a.startMs - b.startMs); - let best: number | null = null; - const consider = (winStartMs: number, winEndMs: number) => { - if (winEndMs - winStartMs < durationMs) return; - const candidate = Math.min(Math.max(anchorMs, winStartMs), winEndMs - durationMs); - if (best === null || Math.abs(candidate - anchorMs) < Math.abs(best - anchorMs)) { - best = candidate; - } - }; - let cursor = gap.startMs; - for (const blocker of blockers) { - consider(cursor, blocker.startMs); - cursor = Math.max(cursor, blocker.endMs); - } - consider(cursor, gap.endMs); - + const best = closestFreeStartMs(gap, durationMs, anchorMs, obstaclesMs); if (best === null) return null; - return { start: dayjs.utc(best).format(), end: dayjs.utc(best + durationMs).format() }; + return slotAt(best, durationMs); } return null; } -// A split must leave a meaningful chunk of work on each side of the break; -// hair-thin fragments would only exist to make a bad placement "fit". +/** Closest obstacle-free start to `anchorMs` that fits inside `range`. */ +function closestFreeStartMs( + range: IntervalMs, + durationMs: number, + anchorMs: number, + obstaclesMs: IntervalMs[] +): number | null { + if (range.endMs - range.startMs < durationMs) return null; + const blockers = obstaclesMs + .filter((o) => o.startMs < range.endMs && o.endMs > range.startMs) + .sort((a, b) => a.startMs - b.startMs); + + let best: number | null = null; + const consider = (winStartMs: number, winEndMs: number) => { + if (winEndMs - winStartMs < durationMs) return; + const candidate = Math.min(Math.max(anchorMs, winStartMs), winEndMs - durationMs); + if (best === null || Math.abs(candidate - anchorMs) < Math.abs(best - anchorMs)) { + best = candidate; + } + }; + let cursor = range.startMs; + for (const blocker of blockers) { + consider(cursor, blocker.startMs); + cursor = Math.max(cursor, blocker.endMs); + } + consider(cursor, range.endMs); + return best; +} + +/** Keep a workless-day break near its current start without leaving the day. */ +export function findBreakSlotNearInDay( + dayStart: string, + dayEnd: string, + durationSeconds: number, + anchorStart: string, + obstacles: Interval[] = [] +): Interval | null { + if (durationSeconds <= 0) return null; + const dayjs = getDayJsInstance(); + const durationMs = durationSeconds * 1000; + const range = { + startMs: dayjs.utc(dayStart).valueOf(), + endMs: dayjs.utc(dayEnd).valueOf(), + }; + const best = closestFreeStartMs( + range, + durationMs, + dayjs.utc(anchorStart).valueOf(), + obstacles.map(toIntervalMs) + ); + if (best === null) return null; + return slotAt(best, durationMs); +} + export const MIN_SPLIT_FRAGMENT_SECONDS = 60; -/** - * Split a single work entry to insert a break. `breakStart` (UTC ISO) lets the - * caller position it; without one the break is centered. Returns null when the - * entry is too short to leave at least MIN_SPLIT_FRAGMENT_SECONDS of work on - * both sides of the break, or when an explicit `breakStart` would not — an - * out-of-range request is rejected rather than clamped, because silently - * relocating the break would contradict the time the user picked. - */ +export interface SplitOptions { + // Omitted bounds are unbounded. + dayStart?: string; + dayEnd?: string; + // Existing breaks that may block or move with the split. + otherEntries?: MovableInterval[]; +} + +/** Insert a break while preserving work duration and respecting the supplied bounds. */ export function planSplitEntry( entry: Interval, durationSeconds: number, - breakStart?: string + breakStart?: string, + options: SplitOptions = {} ): SplitPlan | null { if (durationSeconds <= 0) return null; const dayjs = getDayJsInstance(); - const entryStart = dayjs.utc(entry.start); - const entryEnd = dayjs.utc(entry.end); - const total = entryEnd.diff(entryStart, 'second'); - if (total < durationSeconds + 2 * MIN_SPLIT_FRAGMENT_SECONDS) return null; + const toMs = (iso: string) => dayjs.utc(iso).valueOf(); + const iso = (ms: number) => dayjs.utc(ms).format(); - const earliest = entryStart.add(MIN_SPLIT_FRAGMENT_SECONDS, 'second'); - const latest = entryEnd.subtract(durationSeconds + MIN_SPLIT_FRAGMENT_SECONDS, 'second'); + const entryStartMs = toMs(entry.start); + const totalMs = toMs(entry.end) - entryStartMs; + const minMs = MIN_SPLIT_FRAGMENT_SECONDS * 1000; + if (totalMs < 2 * minMs) return null; + const durationMs = durationSeconds * 1000; + const dayEndMs = options.dayEnd !== undefined ? toMs(options.dayEnd) : Infinity; - let bStart = breakStart - ? dayjs.utc(breakStart) - : entryStart.add(Math.floor((total - durationSeconds) / 2), 'second'); + // Pull the block earlier only enough to keep it inside the day. + const blockStartMs = Math.min(entryStartMs, dayEndMs - totalMs - durationMs); + let bStartMs = breakStart ? toMs(breakStart) : blockStartMs + Math.floor(totalMs / 2000) * 1000; if (breakStart) { - if (bStart.isBefore(earliest) || bStart.isAfter(latest)) return null; + if (bStartMs < blockStartMs + minMs || bStartMs > blockStartMs + totalMs - minMs) { + return null; + } } else { // Safety net for rounding of the centered position only. - if (bStart.isBefore(earliest)) bStart = earliest; - if (bStart.isAfter(latest)) bStart = latest; + bStartMs = Math.min( + Math.max(bStartMs, blockStartMs + minMs), + blockStartMs + totalMs - minMs + ); + } + const bEndMs = bStartMs + durationMs; + const firstHalfMs = bStartMs - blockStartMs; + + const secondHalfEndMs = bEndMs + totalMs - firstHalfMs; + + // Carry the occupied end through the collision chain; stop at the first gap. + const others = options.otherEntries ?? []; + const later = others + .filter((other) => toMs(other.start) >= bStartMs) + .sort((a, b) => toMs(a.start) - toMs(b.start)); + const shifted: MovableInterval[] = []; + let occupiedEndMs = secondHalfEndMs; + for (const other of later) { + const otherStartMs = toMs(other.start); + if (otherStartMs >= occupiedEndMs) break; + + const otherEndMs = toMs(other.end); + const shiftMs = occupiedEndMs - otherStartMs; + const shiftedEndMs = otherEndMs + shiftMs; + if (shiftedEndMs > dayEndMs) return null; + shifted.push({ + id: other.id, + start: iso(otherStartMs + shiftMs), + end: iso(shiftedEndMs), + }); + occupiedEndMs = shiftedEndMs; } - const bEnd = bStart.add(durationSeconds, 'second'); - if (!bStart.isAfter(entryStart) || !bEnd.isBefore(entryEnd)) return null; + // Earlier breaks constrain how far the block may move back. + let earliestStartMs = options.dayStart !== undefined ? toMs(options.dayStart) : -Infinity; + for (const other of others) { + const otherEndMs = toMs(other.end); + if ( + toMs(other.start) < bStartMs && + otherEndMs <= bStartMs && + otherEndMs > earliestStartMs + ) { + earliestStartMs = otherEndMs; + } + } + if (blockStartMs < earliestStartMs) return null; return { - firstHalf: { start: entryStart.format(), end: bStart.format() }, - breakSlot: { start: bStart.format(), end: bEnd.format() }, - secondHalf: { start: bEnd.format(), end: entryEnd.format() }, + firstHalf: { start: iso(blockStartMs), end: iso(bStartMs) }, + breakSlot: { start: iso(bStartMs), end: iso(bEndMs) }, + secondHalf: { start: iso(bEndMs), end: iso(secondHalfEndMs) }, + shifted, }; } +/** Last resort: place the break after the last work entry or before the first. */ +export function findAdjacentBreakSlot( + work: Interval[], + dayStart: string, + dayEnd: string, + durationSeconds: number, + obstacles: Interval[] = [] +): Interval | null { + if (durationSeconds <= 0 || work.length === 0) return null; + const dayjs = getDayJsInstance(); + const durationMs = durationSeconds * 1000; + const dayStartMs = dayjs.utc(dayStart).valueOf(); + const dayEndMs = dayjs.utc(dayEnd).valueOf(); + + const merged = mergedWorkMs(work); + const blockers = [...merged, ...obstacles.map(toIntervalMs)]; + const candidates = [merged[merged.length - 1]!.endMs, merged[0]!.startMs - durationMs]; + + for (const startMs of candidates) { + if (startMs < dayStartMs || startMs + durationMs > dayEndMs) continue; + const clear = blockers.every( + (blocker) => blocker.endMs <= startMs || blocker.startMs >= startMs + durationMs + ); + if (clear) return slotAt(startMs, durationMs); + } + return null; +} + /** - * Insert a break at `breakStart`, shifting the surrounding entries only as much - * as needed to clear the slot. Entries starting before the break form the left - * block: when it reaches into the slot it is translated earlier so its latest - * end meets the break start. The rest form the right block: when the slot - * reaches into it, it is translated later so its earliest start meets the break - * end. Blocks that already clear the slot are left untouched — existing gaps - * are preserved, never tightened. Returns null if a required shift would push - * an entry outside `[dayStart, dayEnd]`. + * Insert a break by translating overlapping entries on either side just enough + * to clear it. Existing gaps remain unchanged. Returns null if a shift would + * leave the day. */ export function planMoveInsert( entries: MovableInterval[], diff --git a/resources/js/utils/timesheet/useBreakPlacement.test.ts b/resources/js/utils/timesheet/useBreakPlacement.test.ts index cd7c73ed..1da80266 100644 --- a/resources/js/utils/timesheet/useBreakPlacement.test.ts +++ b/resources/js/utils/timesheet/useBreakPlacement.test.ts @@ -1,12 +1,14 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { ref } from 'vue'; import { createPinia, setActivePinia } from 'pinia'; -import { useBreakPlacement, BreakPlacementDeferred } from './useBreakPlacement'; +import { useBreakPlacement } from './useBreakPlacement'; +import { NoFreeWindowError } from './cellMath'; import { api } from '@/packages/api/src'; import type { TimeEntry } from '@/packages/api/src'; import type { TimesheetRow } from '@/utils/useTimesheetGrid'; const addNotification = vi.fn(); +const mutationOptionsSpy = vi.hoisted(() => vi.fn()); vi.mock('@/utils/useUser', () => ({ getCurrentOrganizationId: vi.fn(() => 'org-1'), @@ -15,6 +17,28 @@ vi.mock('@/utils/useUser', () => ({ vi.mock('@tanstack/vue-query', () => ({ useQueryClient: () => ({ invalidateQueries: vi.fn() }), + useMutation: (options: { + mutationFn: (variables: unknown) => Promise; + onSuccess?: (data: unknown, variables: unknown) => void | Promise; + onError?: (error: unknown, variables: unknown) => void | Promise; + onSettled?: () => void | Promise; + }) => { + mutationOptionsSpy(options); + return { + mutateAsync: async (variables: unknown) => { + try { + const data = await options.mutationFn(variables); + await options.onSuccess?.(data, variables); + return data; + } catch (error) { + await options.onError?.(error, variables); + throw error; + } finally { + await options.onSettled?.(); + } + }, + }; + }, })); vi.mock('@/utils/notification', () => ({ @@ -61,24 +85,29 @@ const breakRow: TimesheetRow = { totalSeconds: 0, }; -function setup(allEntries: TimeEntry[]) { +function setup(allEntries: TimeEntry[], preventOverlaps = false) { const createCell = vi.fn(async () => undefined); - const updateEntry = vi.fn(async () => undefined); + const updateEntry = vi.fn(async (_entry: TimeEntry) => undefined); + const deleteEntry = vi.fn(async (_id: string) => undefined); const bp = useBreakPlacement({ weekDays: ref([DATE, '2026-04-11', '2026-04-12']), timeEntries: ref(allEntries), requireOrgId: () => 'org-1', createCell, updateEntry, + deleteEntry, + preventOverlappingTimeEntries: () => preventOverlaps, }); - return { bp, createCell, updateEntry }; + return { bp, createCell, updateEntry, deleteEntry }; } beforeEach(() => { setActivePinia(createPinia()); apiMocks.createTimeEntry.mockClear(); apiMocks.updateTimeEntry.mockClear(); + apiMocks.deleteTimeEntry.mockClear(); addNotification.mockClear(); + mutationOptionsSpy.mockClear(); }); describe('useBreakPlacement.placeBreak', () => { @@ -126,13 +155,114 @@ describe('useBreakPlacement.placeBreak', () => { ); }); - it('defers to the split modal when a single work entry blocks every gap', async () => { + it('parks the break next to work too short to split instead of refusing it', async () => { + const work = entry('2026-04-10T09:00:00Z', '2026-04-10T09:01:00Z', { id: 'w1' }); + const { bp } = setup([work]); + + await bp.placeBreak(breakRow, 0, HOUR / 2); + + expect(apiMocks.createTimeEntry).toHaveBeenCalledTimes(1); + expect(apiMocks.createTimeEntry.mock.calls[0]![0]).toEqual( + expect.objectContaining({ + type: 'break', + start: '2026-04-10T09:01:00Z', + end: '2026-04-10T09:31:00Z', + }) + ); + expect(bp.breakPlacementRequest.value).toBeNull(); + }); + + it('still offers a split when a running entry caps the end of the day', async () => { + const work = entry('2026-04-10T09:00:00Z', '2026-04-10T12:00:00Z', { id: 'w1' }); + const running = entry('2026-04-10T12:00:00Z', null, { id: 'running' }); + const { bp } = setup([work, running]); + + await expect(bp.placeBreak(breakRow, 0, HOUR)).resolves.toBe('needs-input'); + expect(bp.breakPlacementRequest.value?.defaultBreakStart).toBe('2026-04-10T09:30:00Z'); + + await bp.applyBreakPlacement('2026-04-10T09:30:00Z', HOUR); + const created = apiMocks.createTimeEntry.mock.calls.map((c) => c[0]); + expect(created).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + type: 'work', + start: '2026-04-10T10:30:00Z', + end: '2026-04-10T12:00:00Z', + }), + expect.objectContaining({ + type: 'break', + start: '2026-04-10T09:30:00Z', + end: '2026-04-10T10:30:00Z', + }), + ]) + ); + }); + + it('drops a new break into the first free window when the day has no work', async () => { + const { bp, createCell } = setup([]); + await bp.placeBreak(breakRow, 0, HOUR); + expect(createCell).toHaveBeenCalledWith(breakRow, 0, HOUR); + expect(apiMocks.createTimeEntry).not.toHaveBeenCalled(); + }); + + it('resizes a break on a workless day in place while it still fits', async () => { + const brk = entry('2026-04-10T20:00:00Z', '2026-04-10T20:30:00Z', { + id: 'b1', + type: 'break', + }); + const { bp, updateEntry } = setup([brk]); + + await bp.placeBreak(breakRow, 0, HOUR, 'b1'); + + expect(updateEntry).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'b1', + start: '2026-04-10T20:00:00Z', + end: '2026-04-10T21:00:00Z', + }) + ); + }); + + it('slides a workless-day break earlier rather than growing it past midnight', async () => { + const brk = entry('2026-04-10T23:00:00Z', '2026-04-10T23:30:00Z', { + id: 'b1', + type: 'break', + }); + const { bp, updateEntry } = setup([brk]); + + await bp.placeBreak(breakRow, 0, 2 * HOUR, 'b1'); + + expect(updateEntry).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'b1', + start: '2026-04-10T22:00:00Z', + end: '2026-04-11T00:00:00Z', + }) + ); + }); + + it('refuses a workless-day resize the day genuinely cannot hold', async () => { + const brk = entry('2026-04-10T23:00:00Z', '2026-04-10T23:30:00Z', { + id: 'b1', + type: 'break', + }); + const other = entry('2026-04-10T06:00:00Z', '2026-04-10T18:00:00Z', { + id: 'b2', + type: 'break', + }); + const { bp, updateEntry } = setup([brk, other]); + + await expect(bp.placeBreak(breakRow, 0, 20 * HOUR, 'b1')).rejects.toBeInstanceOf( + NoFreeWindowError + ); + expect(updateEntry).not.toHaveBeenCalled(); + }); + + it('requests input in the split modal when a single work entry blocks every gap', async () => { const work = entry('2026-04-10T09:00:00Z', '2026-04-10T17:00:00Z', { id: 'w1' }); const { bp } = setup([work]); - await expect(bp.placeBreak(breakRow, 0, HOUR)).rejects.toBeInstanceOf( - BreakPlacementDeferred - ); + await expect(bp.placeBreak(breakRow, 0, HOUR)).resolves.toBe('needs-input'); expect(bp.breakPlacementRequest.value).toEqual( expect.objectContaining({ durationSeconds: HOUR, @@ -145,15 +275,25 @@ describe('useBreakPlacement.placeBreak', () => { }); describe('useBreakPlacement.applyBreakPlacement (split)', () => { - it('shrinks the original, creates the second half, and saves the break', async () => { + it('configures the logical placement as a serialized, non-retrying mutation', () => { + setup([]); + + expect(mutationOptionsSpy).toHaveBeenCalledWith( + expect.objectContaining({ + mutationKey: ['timesheet', 'break-placement'], + scope: { id: 'timesheet-break-placement' }, + retry: false, + }) + ); + }); + + it('shrinks the original, pushes the rest of the work out, and saves the break', async () => { const work = entry('2026-04-10T09:00:00Z', '2026-04-10T17:00:00Z', { id: 'w1' }); const { bp, updateEntry } = setup([work]); - // Open the placement request, then commit the break at noon. - await bp.placeBreak(breakRow, 0, HOUR).catch(() => undefined); + await bp.placeBreak(breakRow, 0, HOUR); await bp.applyBreakPlacement('2026-04-10T12:00:00Z', HOUR); - // Original work shrunk to its first half. expect(updateEntry).toHaveBeenCalledWith( expect.objectContaining({ id: 'w1', @@ -161,14 +301,13 @@ describe('useBreakPlacement.applyBreakPlacement (split)', () => { end: '2026-04-10T12:00:00Z', }) ); - // Second half of work + the break both created. const created = apiMocks.createTimeEntry.mock.calls.map((c) => c[0]); expect(created).toEqual( expect.arrayContaining([ expect.objectContaining({ type: 'work', start: '2026-04-10T13:00:00Z', - end: '2026-04-10T17:00:00Z', + end: '2026-04-10T18:00:00Z', }), expect.objectContaining({ type: 'break', @@ -177,11 +316,245 @@ describe('useBreakPlacement.applyBreakPlacement (split)', () => { }), ]) ); - // Request cleared and a success toast surfaced. expect(bp.breakPlacementRequest.value).toBeNull(); expect(addNotification).toHaveBeenCalledWith('success', 'Break added', expect.any(String)); }); + it('keeps the work length when the break is as long as the work entry', async () => { + const work = entry('2026-04-10T09:00:00Z', '2026-04-10T10:00:00Z', { id: 'w1' }); + const { bp, updateEntry } = setup([work]); + + await bp.placeBreak(breakRow, 0, HOUR); + expect(bp.breakPlacementRequest.value?.defaultBreakStart).toBe('2026-04-10T09:30:00Z'); + await bp.applyBreakPlacement('2026-04-10T09:30:00Z', HOUR); + + expect(updateEntry).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'w1', + start: '2026-04-10T09:00:00Z', + end: '2026-04-10T09:30:00Z', + }) + ); + const created = apiMocks.createTimeEntry.mock.calls.map((c) => c[0]); + expect(created).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + type: 'work', + start: '2026-04-10T10:30:00Z', + end: '2026-04-10T11:00:00Z', + }), + expect.objectContaining({ + type: 'break', + start: '2026-04-10T09:30:00Z', + end: '2026-04-10T10:30:00Z', + }), + ]) + ); + }); + + it('accepts a break longer than the work entry', async () => { + const work = entry('2026-04-10T09:00:00Z', '2026-04-10T10:00:00Z', { id: 'w1' }); + const { bp } = setup([work]); + + await bp.placeBreak(breakRow, 0, 3 * HOUR); + await bp.applyBreakPlacement('2026-04-10T09:30:00Z', 3 * HOUR); + + const created = apiMocks.createTimeEntry.mock.calls.map((c) => c[0]); + expect(created).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + type: 'work', + start: '2026-04-10T12:30:00Z', + end: '2026-04-10T13:00:00Z', + }), + expect.objectContaining({ + type: 'break', + start: '2026-04-10T09:30:00Z', + end: '2026-04-10T12:30:00Z', + }), + ]) + ); + expect(addNotification).toHaveBeenCalledWith('success', 'Break added', expect.any(String)); + }); + + it('moves an existing break out of the way of the pushed-out work', async () => { + const work = entry('2026-04-10T09:00:00Z', '2026-04-10T12:00:00Z', { id: 'w1' }); + const existingBreak = entry('2026-04-10T12:00:00Z', '2026-04-10T12:15:00Z', { + id: 'b1', + type: 'break', + }); + const { bp, updateEntry } = setup([work, existingBreak]); + + await bp.placeBreak(breakRow, 0, HOUR); + await bp.applyBreakPlacement('2026-04-10T10:00:00Z', HOUR); + + expect(updateEntry).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + id: 'w1', + start: '2026-04-10T09:00:00Z', + end: '2026-04-10T10:00:00Z', + }) + ); + expect(updateEntry).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + id: 'b1', + start: '2026-04-10T13:00:00Z', + end: '2026-04-10T13:15:00Z', + }) + ); + const created = apiMocks.createTimeEntry.mock.calls.map((c) => c[0]); + expect(created).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + type: 'work', + start: '2026-04-10T11:00:00Z', + end: '2026-04-10T13:00:00Z', + }), + expect.objectContaining({ + type: 'break', + start: '2026-04-10T10:00:00Z', + end: '2026-04-10T11:00:00Z', + }), + ]) + ); + }); + + it('moves a replacement break before creating work through its old slot', async () => { + const work = entry('2026-04-10T09:00:00Z', '2026-04-10T17:00:00Z', { id: 'w1' }); + const existingBreak = entry('2026-04-10T17:00:00Z', '2026-04-10T17:30:00Z', { + id: 'b1', + type: 'break', + }); + const { bp, updateEntry } = setup([work, existingBreak], true); + + await bp.placeBreak(breakRow, 0, HOUR, 'b1'); + await bp.applyBreakPlacement('2026-04-10T12:00:00Z', HOUR); + + expect(updateEntry).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ id: 'w1', end: '2026-04-10T12:00:00Z' }) + ); + expect(updateEntry).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + id: 'b1', + start: '2026-04-10T12:00:00Z', + end: '2026-04-10T13:00:00Z', + }) + ); + expect(apiMocks.createTimeEntry).toHaveBeenCalledTimes(1); + expect(apiMocks.createTimeEntry.mock.calls[0]![0]).toEqual( + expect.objectContaining({ + type: 'work', + start: '2026-04-10T13:00:00Z', + end: '2026-04-10T18:00:00Z', + }) + ); + }); + + it('restores the original entry when creating the second half fails', async () => { + const work = entry('2026-04-10T09:00:00Z', '2026-04-10T17:00:00Z', { id: 'w1' }); + const { bp, updateEntry } = setup([work]); + apiMocks.createTimeEntry.mockRejectedValueOnce(new Error('boom')); + + await bp.placeBreak(breakRow, 0, HOUR); + await expect(bp.applyBreakPlacement('2026-04-10T12:00:00Z', HOUR)).rejects.toThrow('boom'); + + expect(updateEntry).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + id: 'w1', + start: '2026-04-10T09:00:00Z', + end: '2026-04-10T12:00:00Z', + }) + ); + expect(updateEntry).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + id: 'w1', + start: '2026-04-10T09:00:00Z', + end: '2026-04-10T17:00:00Z', + }) + ); + expect(addNotification).toHaveBeenCalledWith( + 'error', + 'Failed to add break', + expect.any(String) + ); + }); + + it('restores shifted breaks as well as the original when creating work fails', async () => { + const work = entry('2026-04-10T09:00:00Z', '2026-04-10T12:00:00Z', { id: 'w1' }); + const existingBreak = entry('2026-04-10T12:00:00Z', '2026-04-10T12:15:00Z', { + id: 'b1', + type: 'break', + }); + const { bp, updateEntry } = setup([work, existingBreak]); + apiMocks.createTimeEntry.mockRejectedValueOnce(new Error('work create failed')); + + await bp.placeBreak(breakRow, 0, HOUR); + await expect(bp.applyBreakPlacement('2026-04-10T10:00:00Z', HOUR)).rejects.toThrow( + 'work create failed' + ); + + expect(updateEntry.mock.calls.map((call) => call[0])).toEqual([ + expect.objectContaining({ id: 'w1', end: '2026-04-10T10:00:00Z' }), + expect.objectContaining({ + id: 'b1', + start: '2026-04-10T13:00:00Z', + end: '2026-04-10T13:15:00Z', + }), + expect.objectContaining({ + id: 'b1', + start: '2026-04-10T12:00:00Z', + end: '2026-04-10T12:15:00Z', + }), + expect.objectContaining({ id: 'w1', end: '2026-04-10T12:00:00Z' }), + ]); + }); + + it('deletes the created second half and restores updates when saving the break fails', async () => { + const work = entry('2026-04-10T09:00:00Z', '2026-04-10T17:00:00Z', { id: 'w1' }); + const { bp, updateEntry, deleteEntry } = setup([work]); + apiMocks.createTimeEntry + .mockResolvedValueOnce({ data: { id: 'second-half' } } as never) + .mockRejectedValueOnce(new Error('break create failed')); + + await bp.placeBreak(breakRow, 0, HOUR); + await expect(bp.applyBreakPlacement('2026-04-10T12:00:00Z', HOUR)).rejects.toThrow( + 'break create failed' + ); + + expect(deleteEntry).toHaveBeenCalledWith('second-half'); + expect(updateEntry).toHaveBeenLastCalledWith( + expect.objectContaining({ id: 'w1', end: '2026-04-10T17:00:00Z' }) + ); + expect(bp.breakPlacementRequest.value).not.toBeNull(); + }); + + it('surfaces incomplete recovery and closes a stale placement request', async () => { + const work = entry('2026-04-10T09:00:00Z', '2026-04-10T17:00:00Z', { id: 'w1' }); + const { bp, updateEntry } = setup([work]); + updateEntry + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('restore failed')); + apiMocks.createTimeEntry.mockRejectedValueOnce(new Error('create failed')); + + await bp.placeBreak(breakRow, 0, HOUR); + await expect(bp.applyBreakPlacement('2026-04-10T12:00:00Z', HOUR)).rejects.toThrow( + 'create failed' + ); + + expect(addNotification).toHaveBeenCalledWith( + 'error', + 'Break placement needs attention', + expect.any(String) + ); + expect(bp.breakPlacementRequest.value).toBeNull(); + }); + it('does nothing when there is no pending placement request', async () => { const { bp, updateEntry } = setup([]); await bp.applyBreakPlacement('2026-04-10T12:00:00Z', HOUR); @@ -189,3 +562,27 @@ describe('useBreakPlacement.applyBreakPlacement (split)', () => { expect(apiMocks.createTimeEntry).not.toHaveBeenCalled(); }); }); + +describe('useBreakPlacement.applyBreakPlacement (move)', () => { + it('rejects a replacement dependency cycle before changing any entry', async () => { + const morning = entry('2026-04-10T09:00:00Z', '2026-04-10T12:00:00Z', { + id: 'morning', + }); + const afternoon = entry('2026-04-10T12:00:00Z', '2026-04-10T17:00:00Z', { + id: 'afternoon', + }); + const existingBreak = entry('2026-04-10T17:00:00Z', '2026-04-10T17:30:00Z', { + id: 'b1', + type: 'break', + }); + const { bp, updateEntry } = setup([morning, afternoon, existingBreak], true); + + await bp.placeBreak(breakRow, 0, HOUR, 'b1'); + await expect(bp.applyBreakPlacement('2026-04-10T12:00:00Z', HOUR)).rejects.toBeInstanceOf( + NoFreeWindowError + ); + + expect(updateEntry).not.toHaveBeenCalled(); + expect(apiMocks.createTimeEntry).not.toHaveBeenCalled(); + }); +}); diff --git a/resources/js/utils/timesheet/useBreakPlacement.ts b/resources/js/utils/timesheet/useBreakPlacement.ts index 815142aa..ff10e30b 100644 --- a/resources/js/utils/timesheet/useBreakPlacement.ts +++ b/resources/js/utils/timesheet/useBreakPlacement.ts @@ -1,7 +1,6 @@ import { ref, type Ref } from 'vue'; -import { useQueryClient } from '@tanstack/vue-query'; -import { api, type TimeEntry } from '@/packages/api/src'; -import { getDayJsInstance } from '@/packages/ui/src/utils/time'; +import { useMutation, useQueryClient } from '@tanstack/vue-query'; +import { api, type CreateTimeEntryBody, type TimeEntry } from '@/packages/api/src'; import { getUserTimezone } from '@/packages/ui/src/utils/settings'; import { getCurrentMembershipId } from '@/utils/useUser'; import type { TimesheetRow } from '@/utils/useTimesheetGrid'; @@ -9,24 +8,15 @@ import { useNotificationsStore } from '@/utils/notification'; import { localDayBounds, NoFreeWindowError } from './cellMath'; import { buildDayPlacementContext, - findValidBreakGap, - findValidBreakGapNear, + decideBreakPlacement, placementMode, planMoveInsert, planSplitEntry, - suggestMovePlan, type BreakPlacementRequest, type DayPlacementContext, + type MovableInterval, } from './breakPlacementMath'; -/** Signals the caller that a break create/edit is waiting on the placement modal. */ -export class BreakPlacementDeferred extends Error { - constructor() { - super('Break placement deferred to modal'); - this.name = 'BreakPlacementDeferred'; - } -} - /** * Generic entry primitives the break subsystem borrows from the cell-mutation * layer. `createCell` drops an entry in the first free window (used when there @@ -45,17 +35,51 @@ export interface BreakPlacementDeps { afterCursor?: string ) => Promise; updateEntry: (entry: TimeEntry) => Promise; + deleteEntry: (id: string) => Promise; + preventOverlappingTimeEntries?: () => boolean; +} + +interface EntryChange { + original: TimeEntry; + next: TimeEntry; +} + +interface PlacementCommit { + updates: EntryChange[]; + creates: CreateTimeEntryBody[]; + entriesAdjusted: boolean; +} + +export type PlaceBreakResult = 'committed' | 'needs-input'; + +type UndoOperation = () => Promise; + +export class BreakPlacementSagaError extends Error { + constructor( + public readonly operationError: unknown, + public readonly rollbackErrors: unknown[] + ) { + super(operationError instanceof Error ? operationError.message : 'Break placement failed'); + this.name = 'BreakPlacementSagaError'; + } } /** * Break-placement subsystem for the timesheet. Owns the placement-modal request * state and everything that positions a break relative to work — auto-placing it - * into a valid gap when one exists, or deferring to the split/move modal when the + * into a valid gap when one exists, or asking for input in the split/move modal when the * day has to be rearranged. */ export function useBreakPlacement(deps: BreakPlacementDeps) { - const { weekDays, timeEntries, requireOrgId, createCell, updateEntry } = deps; - const dayjs = getDayJsInstance(); + const { + weekDays, + timeEntries, + requireOrgId, + createCell, + updateEntry, + deleteEntry, + preventOverlappingTimeEntries = () => false, + } = deps; const queryClient = useQueryClient(); const notifications = useNotificationsStore(); @@ -82,24 +106,35 @@ export function useBreakPlacement(deps: BreakPlacementDeps) { ); } - async function createBreakEntry(start: string, end: string, memberId?: string): Promise { - const orgId = requireOrgId(); + function breakEntryBody(start: string, end: string, memberId?: string): CreateTimeEntryBody { const member = memberId ?? getCurrentMembershipId(); if (!member) throw new Error('No member context'); - await api.createTimeEntry( - { - member_id: member, - project_id: null, - task_id: null, - start, - end, - billable: false, - type: 'break', - description: null, - tags: [], - }, - { params: { organization: orgId } } - ); + return { + member_id: member, + project_id: null, + task_id: null, + start, + end, + billable: false, + type: 'break', + description: null, + tags: [], + }; + } + + async function createEntry(body: CreateTimeEntryBody): Promise { + const response = await api.createTimeEntry(body, { + params: { organization: requireOrgId() }, + }); + return response.data.id; + } + + async function createBreakEntry( + start: string, + end: string, + memberId?: string + ): Promise { + return createEntry(breakEntryBody(start, end, memberId)); } async function saveBreakEntry( @@ -119,116 +154,169 @@ export function useBreakPlacement(deps: BreakPlacementDeps) { await createBreakEntry(start, end, memberId); } - /** - * Place a break on the day (new, or re-placing an existing one when `replaceBreakId` - * is given). Prefers a gap that already satisfies the placement tolerance; otherwise - * raises BreakPlacementDeferred so the page opens the modal. With no work to anchor to, - * the break is just dropped in / resized in the first free window. - */ + /** Place or resize a break directly when possible, otherwise open the placement modal. */ async function placeBreak( row: TimesheetRow, dayIndex: number, durationSeconds: number, replaceBreakId?: string - ): Promise { + ): Promise { const date = weekDays.value[dayIndex]!; const tz = getUserTimezone(); - const { work, breaks, blocked, dayStart, dayEnd } = dayPlacementContext( - date, - tz, - replaceBreakId - ); - // Existing breaks block auto-placement into a gap (obstacles), but move - // along with the surrounding work when a move plan shifts entries. - // Running entries block everything from their start (never movable). - const obstacles = [...breaks, ...blocked]; - - // On edit, keep the break where it is when its current gap still fits it; only - // fall back to the first-gap-centered placement when it can't stay put. + const context = dayPlacementContext(date, tz, replaceBreakId); const anchorStart = replaceBreakId ? (timeEntries.value.find((e) => e.id === replaceBreakId)?.start ?? null) : null; - const validGap = - (anchorStart !== null - ? findValidBreakGapNear(work, durationSeconds, anchorStart, obstacles) - : null) ?? findValidBreakGap(work, durationSeconds, obstacles); - if (validGap) { - await saveBreakEntry(validGap.start, validGap.end, replaceBreakId); - return; - } - - if (work.length === 0) { - // No work to sit between: for an edit, resize the break in place; for a new - // break, drop it in the first free window. Nothing to align to either way. - if (replaceBreakId) { - const existing = timeEntries.value.find((e) => e.id === replaceBreakId); - if (existing) { - const newEnd = dayjs - .utc(existing.start) - .add(durationSeconds, 'second') - .format(); - await updateEntry({ ...existing, end: newEnd }); - return; - } - } - await createCell(row, dayIndex, durationSeconds); - return; - } - - const mode: 'split' | 'move' = work.length === 1 ? 'split' : 'move'; - const defaultBreakStart = - mode === 'split' - ? (planSplitEntry(work[0]!, durationSeconds)?.breakSlot.start ?? null) - : (suggestMovePlan(work, dayStart, dayEnd, durationSeconds, breaks)?.breakSlot - .start ?? null); - - if (!defaultBreakStart) { - // Even splitting/moving can't open a slot on this day. - throw new NoFreeWindowError(date, durationSeconds); - } - - breakPlacementRequest.value = { + const decision = decideBreakPlacement({ date, durationSeconds, - dayStart, - dayEnd, - workEntries: work, - otherEntries: breaks, - defaultBreakStart, - replaceBreakId: replaceBreakId ?? null, - }; - throw new BreakPlacementDeferred(); + context, + anchorStart, + replaceBreakId, + }); + + switch (decision.kind) { + case 'save': + await saveBreakEntry(decision.slot.start, decision.slot.end, replaceBreakId); + return 'committed'; + case 'place-in-free-window': + await createCell(row, dayIndex, durationSeconds); + return 'committed'; + case 'needs-input': + breakPlacementRequest.value = decision.request; + return 'needs-input'; + case 'reject': + throw new NoFreeWindowError(date, durationSeconds); + } } function dismissBreakPlacement(): void { breakPlacementRequest.value = null; } - /** - * Commit a break at `breakStart` by executing the split or move plan. Shifts - * happen before the break is saved so its target slot is free first. - */ - async function applyBreakPlacement(breakStart: string, durationSeconds: number): Promise { - const req = breakPlacementRequest.value; - if (!req) return; + function shiftChanges(shifted: MovableInterval[]): EntryChange[] { + return shifted.map((shift) => { + const original = timeEntries.value.find((e) => e.id === shift.id); + if (!original) throw new Error('An entry to move no longer exists'); + return { + original, + next: { ...original, start: shift.start, end: shift.end }, + }; + }); + } - // The timesheet is the current member's own, so all created/edited entries stay with them. - const memberId = getCurrentMembershipId(); - if (!memberId) throw new Error('No member context'); + function intervalsOverlap( + left: Pick, + right: Pick + ): boolean { + const leftEndMs = left.end === null ? Infinity : Date.parse(left.end); + const rightEndMs = right.end === null ? Infinity : Date.parse(right.end); + return Date.parse(left.start) < rightEndMs && Date.parse(right.start) < leftEndMs; + } - let entriesAdjusted = true; + /** Order updates so each target is free when overlap prevention is enabled. */ + function nonOverlappingUpdateOrder( + changes: EntryChange[], + memberId: string + ): EntryChange[] | null { + if (!preventOverlappingTimeEntries()) return changes; + + const current = new Map( + timeEntries.value + .filter((entry) => entry.member_id === memberId) + .map((entry) => [entry.id, entry] as const) + ); + const pending = [...changes]; + const ordered: EntryChange[] = []; + + while (pending.length > 0) { + const index = pending.findIndex((change) => + [...current.values()].every( + (entry) => + entry.id === change.original.id || !intervalsOverlap(change.next, entry) + ) + ); + if (index === -1) return null; + + const [change] = pending.splice(index, 1); + ordered.push(change!); + current.set(change!.original.id, change!.next); + } + return ordered; + } + + async function applyUpdates(changes: EntryChange[], undo: UndoOperation[]): Promise { + for (const change of changes) { + await updateEntry(change.next); + undo.push(() => updateEntry(change.original)); + } + } + + async function withCompensation( + operation: (undo: UndoOperation[]) => Promise + ): Promise { + const undo: UndoOperation[] = []; try { - if (placementMode(req) === 'split') { - const original = timeEntries.value.find((e) => e.id === req.workEntries[0]!.id); - const plan = planSplitEntry(req.workEntries[0]!, durationSeconds, breakStart); - if (!original || !plan) throw new NoFreeWindowError(req.date, durationSeconds); - // Shrink the original to the first half, then add the second half + break. - await updateEntry({ - ...original, - start: plan.firstHalf.start, - end: plan.firstHalf.end, - }); - await api.createTimeEntry( + return await operation(undo); + } catch (operationError) { + const rollbackErrors: unknown[] = []; + for (const rollback of [...undo].reverse()) { + try { + await rollback(); + } catch (rollbackError) { + rollbackErrors.push(rollbackError); + } + } + throw new BreakPlacementSagaError(operationError, rollbackErrors); + } + } + + function replacementChange( + req: BreakPlacementRequest, + start: string, + end: string + ): EntryChange | null { + if (!req.replaceBreakId) return null; + const original = timeEntries.value.find((entry) => entry.id === req.replaceBreakId); + if (!original) throw new Error('Break to update no longer exists'); + return { original, next: { ...original, start, end } }; + } + + function buildPlacementCommit({ + req, + breakStart, + durationSeconds, + memberId, + }: { + req: BreakPlacementRequest; + breakStart: string; + durationSeconds: number; + memberId: string; + }): PlacementCommit { + if (placementMode(req) === 'split') { + const original = timeEntries.value.find((e) => e.id === req.workEntries[0]!.id); + const plan = planSplitEntry(req.workEntries[0]!, durationSeconds, breakStart, { + dayStart: req.dayStart, + dayEnd: req.dayEnd, + otherEntries: req.otherEntries, + }); + if (!original || !plan) throw new NoFreeWindowError(req.date, durationSeconds); + + const replacement = replacementChange(req, plan.breakSlot.start, plan.breakSlot.end); + return { + updates: [ + { + original, + next: { + ...original, + start: plan.firstHalf.start, + end: plan.firstHalf.end, + }, + }, + ...(replacement ? [replacement] : []), + ...shiftChanges(plan.shifted), + ], + creates: [ { member_id: memberId, project_id: original.project_id, @@ -240,52 +328,65 @@ export function useBreakPlacement(deps: BreakPlacementDeps) { description: original.description ?? null, tags: original.tags ?? [], }, - { params: { organization: requireOrgId() } } - ); - await saveBreakEntry( - plan.breakSlot.start, - plan.breakSlot.end, - req.replaceBreakId ?? undefined, - memberId - ); - } else { - const plan = planMoveInsert( - [...req.workEntries, ...req.otherEntries], - req.dayStart, - req.dayEnd, - breakStart, - durationSeconds - ); - if (!plan) throw new NoFreeWindowError(req.date, durationSeconds); - entriesAdjusted = plan.shifted.length > 0; - // Order the shifts so no intermediate step overlaps (matters when the org - // prevents overlapping entries): entries moving earlier are updated left-to-right, - // entries moving later right-to-left, so each one vacates before its neighbour moves. - const shifts = plan.shifted - .map((shift) => ({ - shift, - original: timeEntries.value.find((e) => e.id === shift.id), - })) - .filter( - (x): x is { shift: (typeof plan.shifted)[number]; original: TimeEntry } => - !!x.original - ); - const movingEarlier = shifts - .filter((x) => x.shift.start < x.original.start) - .sort((a, b) => a.original.start.localeCompare(b.original.start)); - const movingLater = shifts - .filter((x) => x.shift.start >= x.original.start) - .sort((a, b) => b.original.start.localeCompare(a.original.start)); - for (const { shift, original } of [...movingEarlier, ...movingLater]) { - await updateEntry({ ...original, start: shift.start, end: shift.end }); - } - await saveBreakEntry( - plan.breakSlot.start, - plan.breakSlot.end, - req.replaceBreakId ?? undefined, - memberId - ); + ...(!replacement + ? [breakEntryBody(plan.breakSlot.start, plan.breakSlot.end, memberId)] + : []), + ], + entriesAdjusted: true, + }; + } + + const plan = planMoveInsert( + [...req.workEntries, ...req.otherEntries], + req.dayStart, + req.dayEnd, + breakStart, + durationSeconds + ); + if (!plan) throw new NoFreeWindowError(req.date, durationSeconds); + + const replacement = replacementChange(req, plan.breakSlot.start, plan.breakSlot.end); + return { + updates: [...shiftChanges(plan.shifted), ...(replacement ? [replacement] : [])], + creates: replacement + ? [] + : [breakEntryBody(plan.breakSlot.start, plan.breakSlot.end, memberId)], + entriesAdjusted: plan.shifted.length > 0, + }; + } + + /** Apply a declarative commit with reverse-order compensation if a later write fails. */ + async function executePlacement({ + req, + breakStart, + durationSeconds, + memberId, + }: { + req: BreakPlacementRequest; + breakStart: string; + durationSeconds: number; + memberId: string; + }): Promise<{ entriesAdjusted: boolean }> { + const commit = buildPlacementCommit({ req, breakStart, durationSeconds, memberId }); + const updates = nonOverlappingUpdateOrder(commit.updates, memberId); + if (!updates) throw new NoFreeWindowError(req.date, durationSeconds); + + return withCompensation(async (undo) => { + await applyUpdates(updates, undo); + for (const body of commit.creates) { + const id = await createEntry(body); + undo.push(() => deleteEntry(id)); } + return { entriesAdjusted: commit.entriesAdjusted }; + }); + } + + const { mutateAsync: commitPlacement } = useMutation({ + mutationKey: ['timesheet', 'break-placement'], + scope: { id: 'timesheet-break-placement' }, + retry: false, + mutationFn: executePlacement, + onSuccess: ({ entriesAdjusted }, { req }) => { notifications.addNotification( 'success', req.replaceBreakId ? 'Break updated' : 'Break added', @@ -293,8 +394,19 @@ export function useBreakPlacement(deps: BreakPlacementDeps) { ? 'Your entries were adjusted to make room for the break.' : 'The break was added at the selected time.' ); - } catch (err) { - if (err instanceof NoFreeWindowError) { + if (breakPlacementRequest.value === req) breakPlacementRequest.value = null; + }, + onError: (error, { req }) => { + const operationError = + error instanceof BreakPlacementSagaError ? error.operationError : error; + if (error instanceof BreakPlacementSagaError && error.rollbackErrors.length > 0) { + notifications.addNotification( + 'error', + 'Break placement needs attention', + 'Some entries could not be restored. The timesheet has been refreshed.' + ); + if (breakPlacementRequest.value === req) breakPlacementRequest.value = null; + } else if (operationError instanceof NoFreeWindowError) { notifications.addNotification( 'error', "This day can't fit the break", @@ -303,15 +415,23 @@ export function useBreakPlacement(deps: BreakPlacementDeps) { } else { notifications.addNotification( 'error', - 'Failed to add break', + req.replaceBreakId ? 'Failed to update break' : 'Failed to add break', 'Please try again later.' ); } - throw err; - } finally { - breakPlacementRequest.value = null; + }, + onSettled: () => { queryClient.invalidateQueries({ queryKey: ['timeEntries'] }); - } + }, + }); + + async function applyBreakPlacement(breakStart: string, durationSeconds: number): Promise { + const req = breakPlacementRequest.value; + if (!req) return; + const memberId = getCurrentMembershipId(); + if (!memberId) throw new Error('No member context'); + + await commitPlacement({ req, breakStart, durationSeconds, memberId }); } return { diff --git a/resources/js/utils/timesheet/useTimesheetCellMutations.test.ts b/resources/js/utils/timesheet/useTimesheetCellMutations.test.ts index 4a91197a..77f813c7 100644 --- a/resources/js/utils/timesheet/useTimesheetCellMutations.test.ts +++ b/resources/js/utils/timesheet/useTimesheetCellMutations.test.ts @@ -17,6 +17,25 @@ vi.mock('@tanstack/vue-query', () => ({ useQueryClient: () => ({ invalidateQueries: vi.fn(), }), + useMutation: (options: { + mutationFn: (variables: unknown) => Promise; + onSuccess?: (data: unknown, variables: unknown) => void | Promise; + onError?: (error: unknown, variables: unknown) => void | Promise; + onSettled?: () => void | Promise; + }) => ({ + mutateAsync: async (variables: unknown) => { + try { + const data = await options.mutationFn(variables); + await options.onSuccess?.(data, variables); + return data; + } catch (error) { + await options.onError?.(error, variables); + throw error; + } finally { + await options.onSettled?.(); + } + }, + }), })); vi.mock('@/utils/notification', () => ({ diff --git a/resources/js/utils/timesheet/useTimesheetCellMutations.ts b/resources/js/utils/timesheet/useTimesheetCellMutations.ts index f0c3f64f..edc4fa84 100644 --- a/resources/js/utils/timesheet/useTimesheetCellMutations.ts +++ b/resources/js/utils/timesheet/useTimesheetCellMutations.ts @@ -18,7 +18,7 @@ import { workDayStartOn, type FreeWindow, } from './cellMath'; -import { useBreakPlacement, BreakPlacementDeferred } from './useBreakPlacement'; +import { useBreakPlacement, type PlaceBreakResult } from './useBreakPlacement'; export type CellSaveStatus = 'saving' | 'saved' | 'error'; @@ -54,7 +54,8 @@ export function useTimesheetCellMutations( weekDays: Ref, timeEntries: Ref, rows: Ref, - removeSlot: (key: TimesheetRowKey) => void + removeSlot: (key: TimesheetRowKey) => void, + preventOverlappingTimeEntries: () => boolean = () => false ) { const dayjs = getDayJsInstance(); const queryClient = useQueryClient(); @@ -70,7 +71,15 @@ export function useTimesheetCellMutations( // modal flow) is its own subsystem — it borrows the generic entry primitives // below (hoisted function declarations, so referenceable here). const { breakPlacementRequest, placeBreak, dismissBreakPlacement, applyBreakPlacement } = - useBreakPlacement({ weekDays, timeEntries, requireOrgId, createCell, updateEntry }); + useBreakPlacement({ + weekDays, + timeEntries, + requireOrgId, + createCell, + updateEntry, + deleteEntry, + preventOverlappingTimeEntries, + }); function clearStatusTimer(key: string): void { clearTimeout(statusClearTimers[key]); @@ -125,7 +134,14 @@ export function useTimesheetCellMutations( const wasEmpty = row.totalSeconds === 0; try { - await dispatchCellUpdate(row, dayIndex, newTotalSeconds); + const result = await dispatchCellUpdate(row, dayIndex, newTotalSeconds); + if (result === 'needs-input') { + // The placement modal owns the actual save, so return the cell to idle. + clearStatusTimer(statusKey); + delete cellStatus.value[statusKey]; + delete cellPendingSeconds.value[statusKey]; + return; + } if (wasEmpty && newTotalSeconds > 0 && hasDuplicateIdentitySlot(row)) { removeSlot(row.key); @@ -137,14 +153,6 @@ export function useTimesheetCellMutations( } markSaved(statusKey); } catch (err) { - if (err instanceof BreakPlacementDeferred) { - // The break needs manual placement — revert the cell to idle (the - // modal drives the actual save) instead of showing an error. - clearStatusTimer(statusKey); - delete cellStatus.value[statusKey]; - delete cellPendingSeconds.value[statusKey]; - return; - } markError(statusKey); if (err instanceof NoFreeWindowError) { const friendlyDuration = formatHumanReadableDuration( @@ -182,25 +190,24 @@ export function useTimesheetCellMutations( row: TimesheetRow, dayIndex: number, newTotalSeconds: number - ): Promise { + ): Promise { const cell = row.cells.get(dayIndex); const existingSeconds = cell?.totalSeconds ?? 0; const diff = newTotalSeconds - existingSeconds; if (newTotalSeconds === 0 && cell) { await deleteCell(cell); - return; + return 'committed'; } if (!cell || existingSeconds === 0) { // Breaks are placed relative to work (within tolerance), not just in the // first free slot, and may need the placement modal to resolve. if (row.type === 'break' && newTotalSeconds > 0) { - await placeBreak(row, dayIndex, newTotalSeconds); - return; + return placeBreak(row, dayIndex, newTotalSeconds); } await createCell(row, dayIndex, newTotalSeconds); - return; + return 'committed'; } // Re-place breaks rather than extend/shrink them, which would fragment a break into @@ -209,24 +216,23 @@ export function useTimesheetCellMutations( // become obstacles); shrinking just trims the tail, which can't fragment. if (row.type === 'break') { if (cell.entries.length === 1) { - await placeBreak(row, dayIndex, newTotalSeconds, cell.entries[0]!.id); - return; + return placeBreak(row, dayIndex, newTotalSeconds, cell.entries[0]!.id); } const tail = pickLatestEndedEntry(cell); if (diff > 0 && tail?.end) { - await placeBreak(row, dayIndex, (tail.duration ?? 0) + diff, tail.id); - return; + return placeBreak(row, dayIndex, (tail.duration ?? 0) + diff, tail.id); } await shrinkFromEnd(cell, -diff); - return; + return 'committed'; } if (diff > 0) { await extendCell(row, dayIndex, cell, diff); - return; + return 'committed'; } await shrinkFromEnd(cell, -diff); + return 'committed'; } async function deleteCell(cell: TimesheetCell): Promise {